On Wed, Nov 14, 2018 at 09:38:31PM -0200, 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 this multi-purpose device > type, 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 Am I right in thinking that this is basically identical to virtio-*-pci, aside from only being valid for PCI buses ? IOW, libvirt can expose this device even if QEMU does not support it, by simply using the existing device type and only ever placing it in a PCI bus ? If libvirt did this compatibility approach, can you confirm this would be live migration state compatible. ie can live migrate virtio-*-pci -> virtio-*-pci-transitional, provided only PCI bus was used. > - virtio-*-pci-non-transitional: modern-only > - Supports both Conventional PCI and PCI Express buses IIUC, libvirt can again provide compatibility with old QEMU by simply using the existing device type and setting disable-legacy ? Can you confirm this would be live migration compatible virtio-*-pci + disable-legacy -> virtio-*pci-non-transitional > Reference to previous discussion that originated this idea: > https://www.mail-archive.com/qemu-devel@xxxxxxxxxx/msg558389.html > > Changes v1 -> v2: > * Removed *-0.9 devices. Nobody will want to use them, if > transitional devices work with legacy drivers > (Gerd Hoffmann, Michael S. Tsirkin) > * Drop virtio version from name: rename -1.0-transitional to > -transitional (Michael S. Tsirkin) > * Renamed -1.0 to -non-transitional > * Don't add any extra variants to modern-only device types > (they don't need it) > * Fix typo on TYPE_VIRTIO_INPUT_HOST_PCI (crash reported by > Cornelia Huck) > * No need to change cast macros for modern-only devices > * Rename virtio_register_types() to virtio_pci_types_register() > > Note about points discussed in the v1 thread: > > Andrea suggested making separate transitional Conventional PCi > and transitional PCI Express device types[1]. I didn't do that > because I don't see which problems this solves. We have many > other device types that are hybrid (support both PCI Express and > Conventional PCI) and this was never a problem for us[2]. > > [1] http://mid.mail-archive.com/6f8d59b8ee2d92d73d2957b520bd8bac989fc796.camel@xxxxxxxxxx > [2] http://mid.mail-archive.com/20181017155637.GC31060@xxxxxxxxxxx > --- > hw/virtio/virtio-pci.h | 80 +++++++++-- > hw/display/virtio-gpu-pci.c | 8 +- > hw/display/virtio-vga.c | 8 +- > hw/virtio/virtio-crypto-pci.c | 8 +- > hw/virtio/virtio-pci.c | 215 ++++++++++++++++++++--------- > tests/acceptance/virtio_version.py | 177 ++++++++++++++++++++++++ > 6 files changed, 406 insertions(+), 90 deletions(-) > create mode 100644 tests/acceptance/virtio_version.py > > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h > index 813082b0d7..1d2a11504f 100644 > --- a/hw/virtio/virtio-pci.h > +++ b/hw/virtio/virtio-pci.h > @@ -216,7 +216,8 @@ static inline void virtio_pci_disable_modern(VirtIOPCIProxy *proxy) > /* > * virtio-scsi-pci: This extends VirtioPCIProxy. > */ > -#define TYPE_VIRTIO_SCSI_PCI "virtio-scsi-pci" > +#define TYPE_VIRTIO_SCSI_PCI_PREFIX "virtio-scsi-pci" > +#define TYPE_VIRTIO_SCSI_PCI (TYPE_VIRTIO_SCSI_PCI_PREFIX "-base") > #define VIRTIO_SCSI_PCI(obj) \ > OBJECT_CHECK(VirtIOSCSIPCI, (obj), TYPE_VIRTIO_SCSI_PCI) > > @@ -229,7 +230,8 @@ struct VirtIOSCSIPCI { > /* > * vhost-scsi-pci: This extends VirtioPCIProxy. > */ > -#define TYPE_VHOST_SCSI_PCI "vhost-scsi-pci" > +#define TYPE_VHOST_SCSI_PCI_PREFIX "vhost-scsi-pci" > +#define TYPE_VHOST_SCSI_PCI (TYPE_VHOST_SCSI_PCI_PREFIX "-base") > #define VHOST_SCSI_PCI(obj) \ > OBJECT_CHECK(VHostSCSIPCI, (obj), TYPE_VHOST_SCSI_PCI) > > @@ -239,7 +241,8 @@ struct VHostSCSIPCI { > }; > #endif > > -#define TYPE_VHOST_USER_SCSI_PCI "vhost-user-scsi-pci" > +#define TYPE_VHOST_USER_SCSI_PCI_PREFIX "vhost-user-scsi-pci" > +#define TYPE_VHOST_USER_SCSI_PCI (TYPE_VHOST_USER_SCSI_PCI_PREFIX "-base") > #define VHOST_USER_SCSI_PCI(obj) \ > OBJECT_CHECK(VHostUserSCSIPCI, (obj), TYPE_VHOST_USER_SCSI_PCI) > > @@ -252,7 +255,8 @@ struct VHostUserSCSIPCI { > /* > * vhost-user-blk-pci: This extends VirtioPCIProxy. > */ > -#define TYPE_VHOST_USER_BLK_PCI "vhost-user-blk-pci" > +#define TYPE_VHOST_USER_BLK_PCI_PREFIX "vhost-user-blk-pci" > +#define TYPE_VHOST_USER_BLK_PCI (TYPE_VHOST_USER_BLK_PCI_PREFIX "-base") > #define VHOST_USER_BLK_PCI(obj) \ > OBJECT_CHECK(VHostUserBlkPCI, (obj), TYPE_VHOST_USER_BLK_PCI) > > @@ -265,7 +269,8 @@ struct VHostUserBlkPCI { > /* > * virtio-blk-pci: This extends VirtioPCIProxy. > */ > -#define TYPE_VIRTIO_BLK_PCI "virtio-blk-pci" > +#define TYPE_VIRTIO_BLK_PCI_PREFIX "virtio-blk-pci" > +#define TYPE_VIRTIO_BLK_PCI (TYPE_VIRTIO_BLK_PCI_PREFIX "-base") > #define VIRTIO_BLK_PCI(obj) \ > OBJECT_CHECK(VirtIOBlkPCI, (obj), TYPE_VIRTIO_BLK_PCI) > > @@ -277,7 +282,8 @@ struct VirtIOBlkPCI { > /* > * virtio-balloon-pci: This extends VirtioPCIProxy. > */ > -#define TYPE_VIRTIO_BALLOON_PCI "virtio-balloon-pci" > +#define TYPE_VIRTIO_BALLOON_PCI_PREFIX "virtio-balloon-pci" > +#define TYPE_VIRTIO_BALLOON_PCI (TYPE_VIRTIO_BALLOON_PCI_PREFIX "-base") > #define VIRTIO_BALLOON_PCI(obj) \ > OBJECT_CHECK(VirtIOBalloonPCI, (obj), TYPE_VIRTIO_BALLOON_PCI) > > @@ -289,7 +295,8 @@ struct VirtIOBalloonPCI { > /* > * virtio-serial-pci: This extends VirtioPCIProxy. > */ > -#define TYPE_VIRTIO_SERIAL_PCI "virtio-serial-pci" > +#define TYPE_VIRTIO_SERIAL_PCI_PREFIX "virtio-serial-pci" > +#define TYPE_VIRTIO_SERIAL_PCI (TYPE_VIRTIO_SERIAL_PCI_PREFIX "-base") > #define VIRTIO_SERIAL_PCI(obj) \ > OBJECT_CHECK(VirtIOSerialPCI, (obj), TYPE_VIRTIO_SERIAL_PCI) > > @@ -301,7 +308,8 @@ struct VirtIOSerialPCI { > /* > * virtio-net-pci: This extends VirtioPCIProxy. > */ > -#define TYPE_VIRTIO_NET_PCI "virtio-net-pci" > +#define TYPE_VIRTIO_NET_PCI_PREFIX "virtio-net-pci" > +#define TYPE_VIRTIO_NET_PCI (TYPE_VIRTIO_NET_PCI_PREFIX "-base") > #define VIRTIO_NET_PCI(obj) \ > OBJECT_CHECK(VirtIONetPCI, (obj), TYPE_VIRTIO_NET_PCI) > > @@ -316,7 +324,8 @@ struct VirtIONetPCI { > > #ifdef CONFIG_VIRTFS > > -#define TYPE_VIRTIO_9P_PCI "virtio-9p-pci" > +#define TYPE_VIRTIO_9P_PCI_PREFIX "virtio-9p-pci" > +#define TYPE_VIRTIO_9P_PCI (TYPE_VIRTIO_9P_PCI_PREFIX "-base") > #define VIRTIO_9P_PCI(obj) \ > OBJECT_CHECK(V9fsPCIState, (obj), TYPE_VIRTIO_9P_PCI) > > @@ -330,7 +339,8 @@ typedef struct V9fsPCIState { > /* > * virtio-rng-pci: This extends VirtioPCIProxy. > */ > -#define TYPE_VIRTIO_RNG_PCI "virtio-rng-pci" > +#define TYPE_VIRTIO_RNG_PCI_PREFIX "virtio-rng-pci" > +#define TYPE_VIRTIO_RNG_PCI (TYPE_VIRTIO_RNG_PCI_PREFIX "-base") > #define VIRTIO_RNG_PCI(obj) \ > OBJECT_CHECK(VirtIORngPCI, (obj), TYPE_VIRTIO_RNG_PCI) > > @@ -365,7 +375,8 @@ struct VirtIOInputHIDPCI { > > #ifdef CONFIG_LINUX > > -#define TYPE_VIRTIO_INPUT_HOST_PCI "virtio-input-host-pci" > +#define TYPE_VIRTIO_INPUT_HOST_PCI_PREFIX "virtio-input-host-pci" > +#define TYPE_VIRTIO_INPUT_HOST_PCI (TYPE_VIRTIO_INPUT_HOST_PCI_PREFIX "-base") > #define VIRTIO_INPUT_HOST_PCI(obj) \ > OBJECT_CHECK(VirtIOInputHostPCI, (obj), TYPE_VIRTIO_INPUT_HOST_PCI) > > @@ -392,7 +403,8 @@ struct VirtIOGPUPCI { > /* > * vhost-vsock-pci: This extends VirtioPCIProxy. > */ > -#define TYPE_VHOST_VSOCK_PCI "vhost-vsock-pci" > +#define TYPE_VHOST_VSOCK_PCI_PREFIX "vhost-vsock-pci" > +#define TYPE_VHOST_VSOCK_PCI (TYPE_VHOST_VSOCK_PCI_PREFIX "-base") > #define VHOST_VSOCK_PCI(obj) \ > OBJECT_CHECK(VHostVSockPCI, (obj), TYPE_VHOST_VSOCK_PCI) > > @@ -417,4 +429,48 @@ struct VirtIOCryptoPCI { > /* Virtio ABI version, if we increment this, we break the guest driver. */ > #define VIRTIO_PCI_ABI_VERSION 0 > > +/** > + * VirtioPCIDeviceTypeInfo: > + * > + * Template for virtio PCI device types. See virtio_pci_types_register() > + * for details. > + */ > +typedef struct VirtioPCIDeviceTypeInfo { > + /* Prefix for the class names */ > + const char *name_prefix; > + /* Parent type. If NULL, TYPE_VIRTIO_PCI is used */ > + const char *parent; > + /* If modern_only is true, only <name_prefix> type will be registered */ > + bool modern_only; > + > + /* Same as TypeInfo fields: */ > + size_t instance_size; > + void (*instance_init)(Object *obj); > + void (*class_init)(ObjectClass *klass, void *data); > +} VirtioPCIDeviceTypeInfo; > + > +/* > + * Register virtio-pci types. Each virtio-pci device type will be available > + * in 3 flavors: > + * - <prefix>-transitional: modern device supporting legacy drivers > + * - Supports Conventional PCI buses only > + * - <prefix>-non-transitional: modern-only > + * - Supports Conventional PCI and PCI Express buses > + * - <prefix>: virtio version configurable, legacy driver support > + * automatically selected when device is plugged > + * _ This was the only flavor supported until QEMU 3.1 > + * - Supports Conventional PCI and PCI Express buses > + * > + * All the types above will inherit from "<prefix>-base", so generic > + * code can use "<prefix>-base" on type casts. > + * > + * We need a separate "<prefix>-base" type instead of making all types inherit > + * from <prefix> for two reasons: > + * 1) <prefix> will implement INTERFACE_PCIE_DEVICE, but > + * <prefix>-transitional won't. > + * 2) <prefix> will have the "disable-legacy" and "disable-modern" properties, > + * <prefix>-transitional and <prefix>-non-transitional won't. > + */ > +void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t); > + > #endif > diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c > index cece4aa495..ca47063b70 100644 > --- a/hw/display/virtio-gpu-pci.c > +++ b/hw/display/virtio-gpu-pci.c > @@ -69,9 +69,9 @@ static void virtio_gpu_initfn(Object *obj) > TYPE_VIRTIO_GPU); > } > > -static const TypeInfo virtio_gpu_pci_info = { > - .name = TYPE_VIRTIO_GPU_PCI, > - .parent = TYPE_VIRTIO_PCI, > +static const VirtioPCIDeviceTypeInfo virtio_gpu_pci_info = { > + .name_prefix = TYPE_VIRTIO_GPU_PCI, > + .modern_only = true, > .instance_size = sizeof(VirtIOGPUPCI), > .instance_init = virtio_gpu_initfn, > .class_init = virtio_gpu_pci_class_init, > @@ -79,6 +79,6 @@ static const TypeInfo virtio_gpu_pci_info = { > > static void virtio_gpu_pci_register_types(void) > { > - type_register_static(&virtio_gpu_pci_info); > + virtio_pci_types_register(&virtio_gpu_pci_info); > } > type_init(virtio_gpu_pci_register_types) > diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c > index ab2e369b28..aaa42a8c31 100644 > --- a/hw/display/virtio-vga.c > +++ b/hw/display/virtio-vga.c > @@ -207,9 +207,9 @@ static void virtio_vga_inst_initfn(Object *obj) > TYPE_VIRTIO_GPU); > } > > -static TypeInfo virtio_vga_info = { > - .name = TYPE_VIRTIO_VGA, > - .parent = TYPE_VIRTIO_PCI, > +static VirtioPCIDeviceTypeInfo virtio_vga_info = { > + .name_prefix = TYPE_VIRTIO_VGA, > + .modern_only = true, > .instance_size = sizeof(struct VirtIOVGA), > .instance_init = virtio_vga_inst_initfn, > .class_init = virtio_vga_class_init, > @@ -217,7 +217,7 @@ static TypeInfo virtio_vga_info = { > > static void virtio_vga_register_types(void) > { > - type_register_static(&virtio_vga_info); > + virtio_pci_types_register(&virtio_vga_info); > } > > type_init(virtio_vga_register_types) > diff --git a/hw/virtio/virtio-crypto-pci.c b/hw/virtio/virtio-crypto-pci.c > index bf64996e48..c41dc11f0d 100644 > --- a/hw/virtio/virtio-crypto-pci.c > +++ b/hw/virtio/virtio-crypto-pci.c > @@ -64,9 +64,9 @@ static void virtio_crypto_initfn(Object *obj) > TYPE_VIRTIO_CRYPTO); > } > > -static const TypeInfo virtio_crypto_pci_info = { > - .name = TYPE_VIRTIO_CRYPTO_PCI, > - .parent = TYPE_VIRTIO_PCI, > +static const VirtioPCIDeviceTypeInfo virtio_crypto_pci_info = { > + .name_prefix = TYPE_VIRTIO_CRYPTO_PCI, > + .modern_only = true, > .instance_size = sizeof(VirtIOCryptoPCI), > .instance_init = virtio_crypto_initfn, > .class_init = virtio_crypto_pci_class_init, > @@ -74,6 +74,6 @@ static const TypeInfo virtio_crypto_pci_info = { > > static void virtio_crypto_pci_register_types(void) > { > - type_register_static(&virtio_crypto_pci_info); > + virtio_pci_types_register(&virtio_crypto_pci_info); > } > type_init(virtio_crypto_pci_register_types) > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index a954799267..0fa248d68c 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1119,9 +1119,8 @@ static void virtio_9p_pci_instance_init(Object *obj) > TYPE_VIRTIO_9P); > } > > -static const TypeInfo virtio_9p_pci_info = { > - .name = TYPE_VIRTIO_9P_PCI, > - .parent = TYPE_VIRTIO_PCI, > +static const VirtioPCIDeviceTypeInfo virtio_9p_pci_info = { > + .name_prefix = TYPE_VIRTIO_9P_PCI_PREFIX, > .instance_size = sizeof(V9fsPCIState), > .instance_init = virtio_9p_pci_instance_init, > .class_init = virtio_9p_pci_class_init, > @@ -1877,9 +1876,6 @@ static void virtio_pci_reset(DeviceState *qdev) > static Property virtio_pci_properties[] = { > DEFINE_PROP_BIT("virtio-pci-bus-master-bug-migration", VirtIOPCIProxy, flags, > VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT, false), > - DEFINE_PROP_ON_OFF_AUTO("disable-legacy", VirtIOPCIProxy, disable_legacy, > - ON_OFF_AUTO_AUTO), > - DEFINE_PROP_BOOL("disable-modern", VirtIOPCIProxy, disable_modern, false), > DEFINE_PROP_BIT("migrate-extra", VirtIOPCIProxy, flags, > VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, true), > DEFINE_PROP_BIT("modern-pio-notify", VirtIOPCIProxy, flags, > @@ -1939,13 +1935,104 @@ static const TypeInfo virtio_pci_info = { > .class_init = virtio_pci_class_init, > .class_size = sizeof(VirtioPCIClass), > .abstract = true, > - .interfaces = (InterfaceInfo[]) { > - { INTERFACE_PCIE_DEVICE }, > - { INTERFACE_CONVENTIONAL_PCI_DEVICE }, > - { } > - }, > }; > > +static Property virtio_pci_generic_properties[] = { > + DEFINE_PROP_ON_OFF_AUTO("disable-legacy", VirtIOPCIProxy, disable_legacy, > + ON_OFF_AUTO_AUTO), > + DEFINE_PROP_BOOL("disable-modern", VirtIOPCIProxy, disable_modern, false), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void virtio_pci_generic_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->props = virtio_pci_generic_properties; > +} > + > +static void virtio_pci_transitional_instance_init(Object *obj) > +{ > + VirtIOPCIProxy *proxy = VIRTIO_PCI(obj); > + > + proxy->disable_legacy = ON_OFF_AUTO_OFF; > + proxy->disable_modern = false; > +} > + > +static void virtio_pci_1_0_instance_init(Object *obj) > +{ > + VirtIOPCIProxy *proxy = VIRTIO_PCI(obj); > + > + proxy->disable_legacy = ON_OFF_AUTO_ON; > + proxy->disable_modern = false; > +} > + > + > +void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t) > +{ > + char *base_name = g_strdup_printf("%s-base", t->name_prefix); > + const TypeInfo base_type_info = { > + .name = base_name, > + .parent = t->parent ? t->parent : TYPE_VIRTIO_PCI, > + .instance_size = t->instance_size, > + .instance_init = t->instance_init, > + .class_init = t->class_init, > + .abstract = true, > + }; > + > + const TypeInfo generic_type_info = { > + .name = t->name_prefix, > + .parent = base_name, > + .class_init = virtio_pci_generic_class_init, > + .interfaces = (InterfaceInfo[]) { > + { INTERFACE_PCIE_DEVICE }, > + { INTERFACE_CONVENTIONAL_PCI_DEVICE }, > + { } > + }, > + }; > + > + type_register(&base_type_info); > + type_register(&generic_type_info); > + > + if (!t->modern_only) { > + char *non_transitional_name = > + g_strdup_printf("%s-non-transitional", t->name_prefix); > + char *transitional_name = > + g_strdup_printf("%s-transitional", t->name_prefix); > + const TypeInfo non_transitional_type_info = { > + .name = non_transitional_name, > + .parent = base_name, > + .instance_init = virtio_pci_1_0_instance_init, > + .interfaces = (InterfaceInfo[]) { > + { INTERFACE_PCIE_DEVICE }, > + { INTERFACE_CONVENTIONAL_PCI_DEVICE }, > + { } > + }, > + }; > + const TypeInfo transitional_type_info = { > + .name = transitional_name, > + .parent = base_name, > + .instance_init = virtio_pci_transitional_instance_init, > + .interfaces = (InterfaceInfo[]) { > + /* > + * Transitional virtio devices work only as Conventional PCI > + * devices because they require PIO ports. > + */ > + { INTERFACE_CONVENTIONAL_PCI_DEVICE }, > + { } > + }, > + }; > + > + type_register(&non_transitional_type_info); > + type_register(&transitional_type_info); > + > + g_free(non_transitional_name); > + g_free(transitional_name); > + } > + > + g_free(base_name); > +} > + > /* virtio-blk-pci */ > > static Property virtio_blk_pci_properties[] = { > @@ -1995,9 +2082,8 @@ static void virtio_blk_pci_instance_init(Object *obj) > "bootindex", &error_abort); > } > > -static const TypeInfo virtio_blk_pci_info = { > - .name = TYPE_VIRTIO_BLK_PCI, > - .parent = TYPE_VIRTIO_PCI, > +static const VirtioPCIDeviceTypeInfo virtio_blk_pci_info = { > + .name_prefix = TYPE_VIRTIO_BLK_PCI_PREFIX, > .instance_size = sizeof(VirtIOBlkPCI), > .instance_init = virtio_blk_pci_instance_init, > .class_init = virtio_blk_pci_class_init, > @@ -2051,9 +2137,8 @@ static void vhost_user_blk_pci_instance_init(Object *obj) > "bootindex", &error_abort); > } > > -static const TypeInfo vhost_user_blk_pci_info = { > - .name = TYPE_VHOST_USER_BLK_PCI, > - .parent = TYPE_VIRTIO_PCI, > +static const VirtioPCIDeviceTypeInfo vhost_user_blk_pci_info = { > + .name_prefix = TYPE_VHOST_USER_BLK_PCI_PREFIX, > .instance_size = sizeof(VHostUserBlkPCI), > .instance_init = vhost_user_blk_pci_instance_init, > .class_init = vhost_user_blk_pci_class_init, > @@ -2119,9 +2204,8 @@ static void virtio_scsi_pci_instance_init(Object *obj) > TYPE_VIRTIO_SCSI); > } > > -static const TypeInfo virtio_scsi_pci_info = { > - .name = TYPE_VIRTIO_SCSI_PCI, > - .parent = TYPE_VIRTIO_PCI, > +static const VirtioPCIDeviceTypeInfo virtio_scsi_pci_info = { > + .name_prefix = TYPE_VIRTIO_SCSI_PCI_PREFIX, > .instance_size = sizeof(VirtIOSCSIPCI), > .instance_init = virtio_scsi_pci_instance_init, > .class_init = virtio_scsi_pci_class_init, > @@ -2174,9 +2258,8 @@ static void vhost_scsi_pci_instance_init(Object *obj) > "bootindex", &error_abort); > } > > -static const TypeInfo vhost_scsi_pci_info = { > - .name = TYPE_VHOST_SCSI_PCI, > - .parent = TYPE_VIRTIO_PCI, > +static const VirtioPCIDeviceTypeInfo vhost_scsi_pci_info = { > + .name_prefix = TYPE_VHOST_SCSI_PCI_PREFIX, > .instance_size = sizeof(VHostSCSIPCI), > .instance_init = vhost_scsi_pci_instance_init, > .class_init = vhost_scsi_pci_class_init, > @@ -2229,9 +2312,8 @@ static void vhost_user_scsi_pci_instance_init(Object *obj) > "bootindex", &error_abort); > } > > -static const TypeInfo vhost_user_scsi_pci_info = { > - .name = TYPE_VHOST_USER_SCSI_PCI, > - .parent = TYPE_VIRTIO_PCI, > +static const VirtioPCIDeviceTypeInfo vhost_user_scsi_pci_info = { > + .name_prefix = TYPE_VHOST_USER_SCSI_PCI_PREFIX, > .instance_size = sizeof(VHostUserSCSIPCI), > .instance_init = vhost_user_scsi_pci_instance_init, > .class_init = vhost_user_scsi_pci_class_init, > @@ -2277,9 +2359,8 @@ static void vhost_vsock_pci_instance_init(Object *obj) > TYPE_VHOST_VSOCK); > } > > -static const TypeInfo vhost_vsock_pci_info = { > - .name = TYPE_VHOST_VSOCK_PCI, > - .parent = TYPE_VIRTIO_PCI, > +static const VirtioPCIDeviceTypeInfo vhost_vsock_pci_info = { > + .name_prefix = TYPE_VHOST_VSOCK_PCI_PREFIX, > .instance_size = sizeof(VHostVSockPCI), > .instance_init = vhost_vsock_pci_instance_init, > .class_init = vhost_vsock_pci_class_init, > @@ -2334,9 +2415,8 @@ static void virtio_balloon_pci_instance_init(Object *obj) > "guest-stats-polling-interval", &error_abort); > } > > -static const TypeInfo virtio_balloon_pci_info = { > - .name = TYPE_VIRTIO_BALLOON_PCI, > - .parent = TYPE_VIRTIO_PCI, > +static const VirtioPCIDeviceTypeInfo virtio_balloon_pci_info = { > + .name_prefix = TYPE_VIRTIO_BALLOON_PCI_PREFIX, > .instance_size = sizeof(VirtIOBalloonPCI), > .instance_init = virtio_balloon_pci_instance_init, > .class_init = virtio_balloon_pci_class_init, > @@ -2407,9 +2487,8 @@ static void virtio_serial_pci_instance_init(Object *obj) > TYPE_VIRTIO_SERIAL); > } > > -static const TypeInfo virtio_serial_pci_info = { > - .name = TYPE_VIRTIO_SERIAL_PCI, > - .parent = TYPE_VIRTIO_PCI, > +static const VirtioPCIDeviceTypeInfo virtio_serial_pci_info = { > + .name_prefix = TYPE_VIRTIO_SERIAL_PCI_PREFIX, > .instance_size = sizeof(VirtIOSerialPCI), > .instance_init = virtio_serial_pci_instance_init, > .class_init = virtio_serial_pci_class_init, > @@ -2462,9 +2541,8 @@ static void virtio_net_pci_instance_init(Object *obj) > "bootindex", &error_abort); > } > > -static const TypeInfo virtio_net_pci_info = { > - .name = TYPE_VIRTIO_NET_PCI, > - .parent = TYPE_VIRTIO_PCI, > +static const VirtioPCIDeviceTypeInfo virtio_net_pci_info = { > + .name_prefix = TYPE_VIRTIO_NET_PCI_PREFIX, > .instance_size = sizeof(VirtIONetPCI), > .instance_init = virtio_net_pci_instance_init, > .class_init = virtio_net_pci_class_init, > @@ -2513,9 +2591,8 @@ static void virtio_rng_initfn(Object *obj) > TYPE_VIRTIO_RNG); > } > > -static const TypeInfo virtio_rng_pci_info = { > - .name = TYPE_VIRTIO_RNG_PCI, > - .parent = TYPE_VIRTIO_PCI, > +static const VirtioPCIDeviceTypeInfo virtio_rng_pci_info = { > + .name_prefix = TYPE_VIRTIO_RNG_PCI_PREFIX, > .instance_size = sizeof(VirtIORngPCI), > .instance_init = virtio_rng_initfn, > .class_init = virtio_rng_pci_class_init, > @@ -2605,25 +2682,28 @@ static const TypeInfo virtio_input_hid_pci_info = { > .abstract = true, > }; > > -static const TypeInfo virtio_keyboard_pci_info = { > - .name = TYPE_VIRTIO_KEYBOARD_PCI, > +static const VirtioPCIDeviceTypeInfo virtio_keyboard_pci_info = { > + .name_prefix = TYPE_VIRTIO_KEYBOARD_PCI, > .parent = TYPE_VIRTIO_INPUT_HID_PCI, > + .modern_only = true, > .class_init = virtio_input_hid_kbd_pci_class_init, > .instance_size = sizeof(VirtIOInputHIDPCI), > .instance_init = virtio_keyboard_initfn, > }; > > -static const TypeInfo virtio_mouse_pci_info = { > - .name = TYPE_VIRTIO_MOUSE_PCI, > +static const VirtioPCIDeviceTypeInfo virtio_mouse_pci_info = { > + .name_prefix = TYPE_VIRTIO_MOUSE_PCI, > .parent = TYPE_VIRTIO_INPUT_HID_PCI, > + .modern_only = true, > .class_init = virtio_input_hid_mouse_pci_class_init, > .instance_size = sizeof(VirtIOInputHIDPCI), > .instance_init = virtio_mouse_initfn, > }; > > -static const TypeInfo virtio_tablet_pci_info = { > - .name = TYPE_VIRTIO_TABLET_PCI, > +static const VirtioPCIDeviceTypeInfo virtio_tablet_pci_info = { > + .name_prefix = TYPE_VIRTIO_TABLET_PCI, > .parent = TYPE_VIRTIO_INPUT_HID_PCI, > + .modern_only = true, > .instance_size = sizeof(VirtIOInputHIDPCI), > .instance_init = virtio_tablet_initfn, > }; > @@ -2637,8 +2717,8 @@ static void virtio_host_initfn(Object *obj) > TYPE_VIRTIO_INPUT_HOST); > } > > -static const TypeInfo virtio_host_pci_info = { > - .name = TYPE_VIRTIO_INPUT_HOST_PCI, > +static const VirtioPCIDeviceTypeInfo virtio_host_pci_info = { > + .name_prefix = TYPE_VIRTIO_INPUT_HOST_PCI_PREFIX, > .parent = TYPE_VIRTIO_INPUT_PCI, > .instance_size = sizeof(VirtIOInputHostPCI), > .instance_init = virtio_host_initfn, > @@ -2692,36 +2772,39 @@ static const TypeInfo virtio_pci_bus_info = { > > static void virtio_pci_register_types(void) > { > - type_register_static(&virtio_rng_pci_info); > + /* Base types: */ > + type_register_static(&virtio_pci_bus_info); > + type_register_static(&virtio_pci_info); > type_register_static(&virtio_input_pci_info); > type_register_static(&virtio_input_hid_pci_info); > - type_register_static(&virtio_keyboard_pci_info); > - type_register_static(&virtio_mouse_pci_info); > - type_register_static(&virtio_tablet_pci_info); > + > + /* Implementations: */ > + virtio_pci_types_register(&virtio_rng_pci_info); > + virtio_pci_types_register(&virtio_keyboard_pci_info); > + virtio_pci_types_register(&virtio_mouse_pci_info); > + virtio_pci_types_register(&virtio_tablet_pci_info); > #ifdef CONFIG_LINUX > - type_register_static(&virtio_host_pci_info); > + virtio_pci_types_register(&virtio_host_pci_info); > #endif > - type_register_static(&virtio_pci_bus_info); > - type_register_static(&virtio_pci_info); > #ifdef CONFIG_VIRTFS > - type_register_static(&virtio_9p_pci_info); > + virtio_pci_types_register(&virtio_9p_pci_info); > #endif > - type_register_static(&virtio_blk_pci_info); > + virtio_pci_types_register(&virtio_blk_pci_info); > #if defined(CONFIG_VHOST_USER) && defined(CONFIG_LINUX) > - type_register_static(&vhost_user_blk_pci_info); > + virtio_pci_types_register(&vhost_user_blk_pci_info); > #endif > - type_register_static(&virtio_scsi_pci_info); > - type_register_static(&virtio_balloon_pci_info); > - type_register_static(&virtio_serial_pci_info); > - type_register_static(&virtio_net_pci_info); > + virtio_pci_types_register(&virtio_scsi_pci_info); > + virtio_pci_types_register(&virtio_balloon_pci_info); > + virtio_pci_types_register(&virtio_serial_pci_info); > + virtio_pci_types_register(&virtio_net_pci_info); > #ifdef CONFIG_VHOST_SCSI > - type_register_static(&vhost_scsi_pci_info); > + virtio_pci_types_register(&vhost_scsi_pci_info); > #endif > #if defined(CONFIG_VHOST_USER) && defined(CONFIG_LINUX) > - type_register_static(&vhost_user_scsi_pci_info); > + virtio_pci_types_register(&vhost_user_scsi_pci_info); > #endif > #ifdef CONFIG_VHOST_VSOCK > - type_register_static(&vhost_vsock_pci_info); > + virtio_pci_types_register(&vhost_vsock_pci_info); > #endif > } > > diff --git a/tests/acceptance/virtio_version.py b/tests/acceptance/virtio_version.py > new file mode 100644 > index 0000000000..a8d0b20b75 > --- /dev/null > +++ b/tests/acceptance/virtio_version.py > @@ -0,0 +1,177 @@ > +""" > +Check compatibility of virtio device types > +""" > +# Copyright (c) 2018 Red Hat, Inc. > +# > +# Author: > +# Eduardo Habkost <ehabkost@xxxxxxxxxx> > +# > +# This work is licensed under the terms of the GNU GPL, version 2 or > +# later. See the COPYING file in the top-level directory. > +import sys > +import os > + > +sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "scripts")) > +from qemu import QEMUMachine > +from avocado_qemu import Test > + > +# Virtio Device IDs: > +VIRTIO_NET = 1 > +VIRTIO_BLOCK = 2 > +VIRTIO_CONSOLE = 3 > +VIRTIO_RNG = 4 > +VIRTIO_BALLOON = 5 > +VIRTIO_RPMSG = 7 > +VIRTIO_SCSI = 8 > +VIRTIO_9P = 9 > +VIRTIO_RPROC_SERIAL = 11 > +VIRTIO_CAIF = 12 > +VIRTIO_GPU = 16 > +VIRTIO_INPUT = 18 > +VIRTIO_VSOCK = 19 > +VIRTIO_CRYPTO = 20 > + > +PCI_VENDOR_ID_REDHAT_QUMRANET = 0x1af4 > + > +# Device IDs for legacy/transitional devices: > +PCI_LEGACY_DEVICE_IDS = { > + VIRTIO_NET: 0x1000, > + VIRTIO_BLOCK: 0x1001, > + VIRTIO_BALLOON: 0x1002, > + VIRTIO_CONSOLE: 0x1003, > + VIRTIO_SCSI: 0x1004, > + VIRTIO_RNG: 0x1005, > + VIRTIO_9P: 0x1009, > + VIRTIO_VSOCK: 0x1012, > +} > + > +def pci_modern_device_id(virtio_devid): > + return virtio_devid + 0x1040 > + > +def devtype_implements(vm, devtype, implements): > + return devtype in [d['name'] for d in vm.command('qom-list-types', implements=implements)] > + > +def get_interfaces(vm, devtype, interfaces=['pci-express-device', 'conventional-pci-device']): > + return [i for i in interfaces if devtype_implements(vm, devtype, i)] > + > +class VirtioVersionCheck(Test): > + """ > + Check if virtio-version-specific device types result in the > + same device tree created by `disable-modern` and > + `disable-legacy`. > + > + :avocado: enable > + :avocado: tags=x86_64 > + """ > + > + # just in case there are failures, show larger diff: > + maxDiff = 4096 > + > + def run_device(self, devtype, opts=None, machine='pc'): > + """ > + Run QEMU with `-device DEVTYPE`, return device info from `query-pci` > + """ > + with QEMUMachine(self.qemu_bin) as vm: > + vm.set_machine(machine) > + if opts: > + devtype += ',' + opts > + vm.add_args('-device', '%s,id=devfortest' % (devtype)) > + vm.add_args('-S') > + vm.launch() > + > + pcibuses = vm.command('query-pci') > + alldevs = [dev for bus in pcibuses for dev in bus['devices']] > + import pprint > + pprint.pprint(alldevs) > + devfortest = [dev for dev in alldevs > + if dev['qdev_id'] == 'devfortest'] > + return devfortest[0], get_interfaces(vm, devtype) > + > + > + def assert_devids(self, dev, devid, non_transitional=False): > + self.assertEqual(dev['id']['vendor'], PCI_VENDOR_ID_REDHAT_QUMRANET) > + self.assertEqual(dev['id']['device'], devid) > + if non_transitional: > + self.assertTrue(0x1040 <= dev['id']['device'] <= 0x107f) > + self.assertGreaterEqual(dev['id']['subsystem'], 0x40) > + > + def check_all_variants(self, qemu_devtype, virtio_devid): > + """Check if a virtio device type and its variants behave as expected""" > + # Force modern mode: > + dev_modern,_ = self.run_device(qemu_devtype, > + 'disable-modern=off,disable-legacy=on') > + self.assert_devids(dev_modern, pci_modern_device_id(virtio_devid), > + non_transitional=True) > + > + # <prefix>-non-transitional device types should be 100% equivalent to > + # <prefix>,disable-modern=off,disable-legacy=on > + dev_1_0,nt_ifaces = self.run_device('%s-non-transitional' % (qemu_devtype)) > + self.assertEqual(dev_modern, dev_1_0) > + > + # Force transitional mode: > + dev_trans,_ = self.run_device(qemu_devtype, > + 'disable-modern=off,disable-legacy=off') > + self.assert_devids(dev_trans, PCI_LEGACY_DEVICE_IDS[virtio_devid]) > + > + # Force legacy mode: > + dev_legacy,_ = self.run_device(qemu_devtype, > + 'disable-modern=on,disable-legacy=off') > + self.assert_devids(dev_legacy, PCI_LEGACY_DEVICE_IDS[virtio_devid]) > + > + # No options: default to transitional on PC machine-type: > + no_opts_pc,generic_ifaces = self.run_device(qemu_devtype) > + self.assertEqual(dev_trans, no_opts_pc) > + > + #TODO: check if plugging on a PCI Express bus will make the > + # device non-transitional > + #no_opts_q35 = self.run_device(qemu_devtype, machine='q35') > + #self.assertEqual(dev_modern, no_opts_q35) > + > + # <prefix>-transitional device types should be 100% equivalent to > + # <prefix>,disable-modern=off,disable-legacy=off > + dev_trans,trans_ifaces = self.run_device('%s-transitional' % (qemu_devtype)) > + self.assertEqual(dev_trans, dev_trans) > + > + # ensure the interface information is correct: > + self.assertIn('conventional-pci-device', generic_ifaces) > + self.assertIn('pci-express-device', generic_ifaces) > + > + self.assertIn('conventional-pci-device', nt_ifaces) > + self.assertIn('pci-express-device', nt_ifaces) > + > + self.assertIn('conventional-pci-device', trans_ifaces) > + self.assertNotIn('pci-express-device', trans_ifaces) > + > + > + def test_conventional_devs(self): > + self.check_all_variants('virtio-net-pci', VIRTIO_NET) > + # virtio-blk requires 'driver' parameter > + #self.check_all_variants('virtio-blk-pci', VIRTIO_BLOCK) > + self.check_all_variants('virtio-serial-pci', VIRTIO_CONSOLE) > + self.check_all_variants('virtio-rng-pci', VIRTIO_RNG) > + self.check_all_variants('virtio-balloon-pci', VIRTIO_BALLOON) > + self.check_all_variants('virtio-scsi-pci', VIRTIO_SCSI) > + # virtio-9p requires 'fsdev' parameter > + #self.check_all_variants('virtio-9p-pci', VIRTIO_9P) > + > + def check_modern_only(self, qemu_devtype, virtio_devid): > + """Check if a modern-only virtio device type behaves as expected""" > + # Force modern mode: > + dev_modern,_ = self.run_device(qemu_devtype, > + 'disable-modern=off,disable-legacy=on') > + self.assert_devids(dev_modern, pci_modern_device_id(virtio_devid), > + non_transitional=True) > + > + # No options: should be modern anyway > + dev_no_opts,ifaces = self.run_device(qemu_devtype) > + self.assertEqual(dev_modern, dev_no_opts) > + > + self.assertIn('conventional-pci-device', ifaces) > + self.assertIn('pci-express-device', ifaces) > + > + def test_modern_only_devs(self): > + self.check_modern_only('virtio-vga', VIRTIO_GPU) > + self.check_modern_only('virtio-gpu-pci', VIRTIO_GPU) > + self.check_modern_only('virtio-mouse-pci', VIRTIO_INPUT) > + self.check_modern_only('virtio-tablet-pci', VIRTIO_INPUT) > + self.check_modern_only('virtio-keyboard-pci', VIRTIO_INPUT) > -- > 2.18.0.rc1.1.g3f1ff2140 > Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list