Re: [Qemu-devel] [PATCH for-4.0 v4 2/2] virtio: Provide version-specific variants of virtio PCI devices

[Thomas Huth]

On 2019-01-03 10:38, Thomas Huth wrote:
> On 2018-12-05 20:57, Eduardo Habkost wrote:
>> Many of the current virtio-*-pci device types actually represent
>> 3 different types of devices:
>> * virtio 1.0 non-transitional devices
>> * virtio 1.0 transitional devices
>> * virtio 0.9 ("legacy device" in virtio 1.0 terminology)
>>
>> That would be just an annoyance if it didn't break our device/bus
>> compatibility QMP interfaces.  With these multi-purpose device
>> types, there's no way to tell management software that
>> transitional devices and legacy devices require a Conventional
>> PCI bus.
>>
>> The multi-purpose device types would also prevent us from telling
>> management software what's the PCI vendor/device ID for them,
>> because their PCI IDs change at runtime depending on the bus
>> where they were plugged.
>>
>> This patch adds separate device types for each of those virtio
>> device flavors:
>>
>> - virtio-*-pci: the existing multi-purpose device types
>>   - Configurable using `disable-legacy` and `disable-modern`
>>     properties
>>   - Legacy driver support is automatically enabled/disabled
>>     depending on the bus where it is plugged
>>   - Supports Conventional PCI and PCI Express buses
>>     (but Conventional PCI is incompatible with
>>     disable-legacy=off)
>>   - Changes PCI vendor/device IDs at runtime
>> - virtio-*-pci-transitional: virtio-1.0 device supporting legacy drivers
>>   - Supports Conventional PCI buses only, because
>>     it has a PIO BAR
>> - virtio-*-pci-non-transitional: modern-only
>>   - Supports both Conventional PCI and PCI Express buses
>>
>> The existing TYPE_* macros for these types will point to an
>> abstract base type, so existing casts in the code will keep
>> working for all variants.
>>
>> A simple test script (tests/acceptance/virtio_version.py) is
>> included, to check if the new device types are equivalent to
>> using the `disable-legacy` and `disable-modern` options.
>>
>> Acked-by: Andrea Bolognani <address@hidden>
>> Reviewed-by: Cornelia Huck <address@hidden>
>> Signed-off-by: Eduardo Habkost <address@hidden>
> 
>  Hi Eduardo,
> 
> with these new devices, I can trigger an abort on s390x:
> 
> $ qemu-system-s390x -M s390-ccw-virtio-2.5 -monitor stdio -no-shutdown
> QEMU 3.1.50 monitor - type 'help' for more information
> (qemu) device_add vhost-scsi-pci-non-transitional
> qemu-system-s390x: hw/core/qdev-properties.c:1236:
> qdev_prop_set_globals: Assertion `prop->user_provided' failed.
> Aborted (core dumped)
FWIW, it happens with x86, too:

$ qemu-system-x86_64 -M pc-i440fx-2.6 -monitor stdio
QEMU 3.1.50 monitor - type 'help' for more information
(qemu) device_add vhost-scsi-pci-non-transitional
qemu-system-x86_64: hw/core/qdev-properties.c:1236:
qdev_prop_set_globals: Assertion `prop->user_provided' failed.
Aborted (core dumped)

Only machine types newer than 2.7 seem to be OK.

 Thomas