On Tue, Nov 27, 2018 at 11:45:23AM +0100, Cornelia Huck wrote:
On Tue, 27 Nov 2018 00:49:58 -0200 Eduardo Habkost <ehabkost@redhat.com> wrote:
Introduce a helper for registering different flavours of virtio devices. Convert code to use the helper, but keep only the existing generic types. Transitional and non-transitional device types will be added by another patch.
Acked-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v2 -> v3: * Split into two separate patches (type registration helper and introduction of new types) * Rewrote comments describing each flavour * Rewrote virtio_pci_types_register() completely: * Replaced magic generation of type names with explicit fields in VirtioPCIDeviceTypeInfo * Removed modern_only field (won't be used anymore) * Don't register a separate base type unless it's required by the caller --- hw/virtio/virtio-pci.h | 54 ++++++++ hw/display/virtio-gpu-pci.c | 7 +- hw/display/virtio-vga.c | 7 +- hw/virtio/virtio-crypto-pci.c | 7 +- hw/virtio/virtio-pci.c | 231 ++++++++++++++++++++++++---------- 5 files changed, 228 insertions(+), 78 deletions(-)
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index 813082b0d7..b26731a305 100644 --- a/hw/virtio/virtio-pci.h +++ b/hw/virtio/virtio-pci.h @@ -417,4 +417,58 @@ struct VirtIOCryptoPCI { /* Virtio ABI version, if we increment this, we break the guest driver. */ #define VIRTIO_PCI_ABI_VERSION 0
+/* Input for virtio_pci_types_register() */ +typedef struct VirtioPCIDeviceTypeInfo { + /* + * Common base class for the subclasses below. + * + * Required only if transitional_name or non_transitional_name is set. + * + * We need a separate base type instead of making all types + * inherit from generic_name for two reasons: + * 1) generic_name implements INTERFACE_PCIE_DEVICE, but + * transitional_name don't.
s/don't/does not/
Fixed, thanks!
+ * 2) generic_name has the "disable-legacy" and "disable-modern" + * properties, transitional_name and non_transitional name don't. + */ + const char *base_name; + /* + * Generic device type. Optional. + * + * Supports both transitional and non-transitional modes, + * using the disable-legacy and disable-modern properties. + * If disable-legacy=auto, (non-)transitional mode is selected + * depending on the bus where the device is plugged. + * + * Implements both INTERFACE_PCIE_DEVICE and INTERFACE_CONVENTIONAL_PCI_DEVICE, + * but PCI Express is supported only in non-transitional mode. + * + * The only type implemented by QEMU 3.1 and older. + */ + const char *generic_name; + /* + * The non-transitional device type. Optional.
That's transitional...
Oops. Fixed, thanks!
+ * + * Implements both INTERFACE_PCIE_DEVICE and INTERFACE_CONVENTIONAL_PCI_DEVICE. + */ + const char *transitional_name; + /* + * The transitional device type. Optional.
...and that's non-transitional :)
Oops. Fixed, thanks!
+ * + * Implements INTERFACE_CONVENTIONAL_PCI_DEVICE only. + */ + const char *non_transitional_name; + + /* Parent type. If NULL, TYPE_VIRTIO_PCI is used */ + const char *parent; + + /* Same as TypeInfo fields: */ + size_t instance_size; + void (*instance_init)(Object *obj); + void (*class_init)(ObjectClass *klass, void *data); +} VirtioPCIDeviceTypeInfo;
That's a bit of boilerplate, but the end result seems to be more greppable.
Right. Also, only old devices that have transitional versions will need to use all fields. The modern-only devices now have less boilerplate (because parent is optional).
+ +/* Register virtio-pci type(s). @t must be static. */ +void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t); + #endif
(...)
Looks sane to me. With the typos fixed,
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Thanks! -- Eduardo