Message ID | 20241110215519.49150-17-phil@philjordan.eu |
---|---|
State | New |
Headers | show |
Series | macOS PV Graphics and new vmapple machine type | expand |
On 2024/11/11 6:55, Phil Dennis-Jordan wrote: > This reduces the type hierarchy of the vmapple-virtio-blk-pci-base > type and vmapple-virtio-root/vmapple-virtio-aux leaf types with a single > vmapple-virtio-blk-pci type which exposes a 'variant' enum property > which can be set to 'aux' or 'root'. > > This change removes a bunch of device type boilerplate at the cost of > defining a new qapi enum and qdev property type. > > Documentation for the vmapple machine type is also updated. Please merge this change into "[PATCH v9 13/16] hw/vmapple/virtio-blk: Add support for apple virtio-blk". > > Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu> > --- > docs/system/arm/vmapple.rst | 4 +-- > hw/core/qdev-properties-system.c | 8 +++++ > hw/vmapple/virtio-blk.c | 50 +++++++---------------------- > include/hw/qdev-properties-system.h | 6 ++++ > include/hw/vmapple/vmapple.h | 4 +-- > qapi/virtio.json | 14 ++++++++ > 6 files changed, 43 insertions(+), 43 deletions(-) > > diff --git a/docs/system/arm/vmapple.rst b/docs/system/arm/vmapple.rst > index 67942474b93..efe39b68dc4 100644 > --- a/docs/system/arm/vmapple.rst > +++ b/docs/system/arm/vmapple.rst > @@ -56,8 +56,8 @@ to get better interactive access into the target system: > -drive file="$DISK",if=pflash,format=raw \ > -drive file="$AUX",if=none,id=aux,format=raw \ > -drive file="$DISK",if=none,id=root,format=raw \ > - -device vmapple-virtio-aux,drive=aux \ > - -device vmapple-virtio-root,drive=root \ > + -device vmapple-virtio-blk-pci,variant=aux,drive=aux \ > + -device vmapple-virtio-blk-pci,variant=root,drive=root \ > -netdev user,id=net0,ipv6=off,hostfwd=tcp::2222-:22,hostfwd=tcp::5901-:5900 \ > -device virtio-net-pci,netdev=net0 > > diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c > index 35deef05f32..8bf8a3442d6 100644 > --- a/hw/core/qdev-properties-system.c > +++ b/hw/core/qdev-properties-system.c > @@ -1239,3 +1239,11 @@ const PropertyInfo qdev_prop_iothread_vq_mapping_list = { > .set = set_iothread_vq_mapping_list, > .release = release_iothread_vq_mapping_list, > }; > + > +const PropertyInfo qdev_prop_vmapple_virtio_blk_variant = { > + .name = "VMAppleVirtioBlkVariant", > + .enum_table = &VMAppleVirtioBlkVariant_lookup, > + .get = qdev_propinfo_get_enum, > + .set = qdev_propinfo_set_enum, > + .set_default_value = qdev_propinfo_set_default_value_enum, > +}; > diff --git a/hw/vmapple/virtio-blk.c b/hw/vmapple/virtio-blk.c > index 40b33bbdac5..f883e65a5de 100644 > --- a/hw/vmapple/virtio-blk.c > +++ b/hw/vmapple/virtio-blk.c > @@ -24,6 +24,7 @@ > #include "qemu/module.h" > #include "qapi/error.h" > > +#define TYPE_VMAPPLE_VIRTIO_BLK "vmapple-virtio-blk" > OBJECT_DECLARE_TYPE(VMAppleVirtIOBlk, VMAppleVirtIOBlkClass, VMAPPLE_VIRTIO_BLK) > > typedef struct VMAppleVirtIOBlkClass { > @@ -41,14 +42,10 @@ typedef struct VMAppleVirtIOBlk { > /* > * vmapple-virtio-blk-pci: This extends VirtioPCIProxy. > */ > -#define TYPE_VMAPPLE_VIRTIO_BLK_PCI "vmapple-virtio-blk-pci-base" > OBJECT_DECLARE_SIMPLE_TYPE(VMAppleVirtIOBlkPCI, VMAPPLE_VIRTIO_BLK_PCI) > > #define VIRTIO_BLK_T_APPLE_BARRIER 0x10000 > > -#define VIRTIO_APPLE_TYPE_ROOT 1 > -#define VIRTIO_APPLE_TYPE_AUX 2 > - > static bool vmapple_virtio_blk_handle_unknown_request(VirtIOBlockReq *req, > MultiReqBuffer *mrb, > uint32_t type) > @@ -109,7 +106,7 @@ static const TypeInfo vmapple_virtio_blk_info = { > struct VMAppleVirtIOBlkPCI { > VirtIOPCIProxy parent_obj; > VMAppleVirtIOBlk vdev; > - uint32_t apple_type; > + VMAppleVirtioBlkVariant variant; > }; > > > @@ -119,6 +116,8 @@ static Property vmapple_virtio_blk_pci_properties[] = { > VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, > DEV_NVECTORS_UNSPECIFIED), > + DEFINE_PROP_VMAPPLE_VIRTIO_BLK_VARIANT("variant", VMAppleVirtIOBlkPCI, variant, > + VM_APPLE_VIRTIO_BLK_VARIANT_UNSPECIFIED), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -128,6 +127,12 @@ static void vmapple_virtio_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **err > DeviceState *vdev = DEVICE(&dev->vdev); > VirtIOBlkConf *conf = &dev->vdev.parent_obj.conf; > > + if (dev->variant == VM_APPLE_VIRTIO_BLK_VARIANT_UNSPECIFIED) { > + error_setg(errp, "Device " TYPE_VMAPPLE_VIRTIO_BLK_PCI ": must specify " > + "a variant, 'aux' or 'root'"); > + return; > + } > + > if (conf->num_queues == VIRTIO_BLK_AUTO_NUM_QUEUES) { > conf->num_queues = virtio_pci_optimal_num_queues(0); > } > @@ -143,7 +148,7 @@ static void vmapple_virtio_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **err > */ > virtio_add_feature(&dev->vdev.parent_obj.host_features, VIRTIO_BLK_F_ZONED); > /* Propagate the apple type down to the virtio-blk device */ > - dev->vdev.apple_type = dev->apple_type; > + dev->vdev.apple_type = dev->variant; > /* and spawn the virtio-blk device */ > qdev_realize(vdev, BUS(&vpci_dev->bus), errp); > > @@ -181,47 +186,16 @@ static void vmapple_virtio_blk_pci_instance_init(Object *obj) > } > > static const VirtioPCIDeviceTypeInfo vmapple_virtio_blk_pci_info = { > - .base_name = TYPE_VMAPPLE_VIRTIO_BLK_PCI, > - .generic_name = "vmapple-virtio-blk-pci", > + .generic_name = TYPE_VMAPPLE_VIRTIO_BLK_PCI, > .instance_size = sizeof(VMAppleVirtIOBlkPCI), > .instance_init = vmapple_virtio_blk_pci_instance_init, > .class_init = vmapple_virtio_blk_pci_class_init, > }; > > -static void vmapple_virtio_root_instance_init(Object *obj) > -{ > - VMAppleVirtIOBlkPCI *dev = VMAPPLE_VIRTIO_BLK_PCI(obj); > - > - dev->apple_type = VIRTIO_APPLE_TYPE_ROOT; > -} > - > -static const TypeInfo vmapple_virtio_root_info = { > - .name = TYPE_VMAPPLE_VIRTIO_ROOT, > - .parent = "vmapple-virtio-blk-pci", > - .instance_size = sizeof(VMAppleVirtIOBlkPCI), > - .instance_init = vmapple_virtio_root_instance_init, > -}; > - > -static void vmapple_virtio_aux_instance_init(Object *obj) > -{ > - VMAppleVirtIOBlkPCI *dev = VMAPPLE_VIRTIO_BLK_PCI(obj); > - > - dev->apple_type = VIRTIO_APPLE_TYPE_AUX; > -} > - > -static const TypeInfo vmapple_virtio_aux_info = { > - .name = TYPE_VMAPPLE_VIRTIO_AUX, > - .parent = "vmapple-virtio-blk-pci", > - .instance_size = sizeof(VMAppleVirtIOBlkPCI), > - .instance_init = vmapple_virtio_aux_instance_init, > -}; > - > static void vmapple_virtio_blk_register_types(void) > { > type_register_static(&vmapple_virtio_blk_info); > virtio_pci_types_register(&vmapple_virtio_blk_pci_info); > - type_register_static(&vmapple_virtio_root_info); > - type_register_static(&vmapple_virtio_aux_info); > } > > type_init(vmapple_virtio_blk_register_types) > diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h > index cdcc63056e5..6e428f3fcad 100644 > --- a/include/hw/qdev-properties-system.h > +++ b/include/hw/qdev-properties-system.h > @@ -27,6 +27,8 @@ extern const PropertyInfo qdev_prop_pcie_link_speed; > extern const PropertyInfo qdev_prop_pcie_link_width; > extern const PropertyInfo qdev_prop_cpus390entitlement; > extern const PropertyInfo qdev_prop_iothread_vq_mapping_list; > +extern const PropertyInfo qdev_prop_vmapple_virtio_blk_variant; > + Here is an extra line. > > #define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d) \ > DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t) > @@ -94,4 +96,8 @@ extern const PropertyInfo qdev_prop_iothread_vq_mapping_list; > DEFINE_PROP(_name, _state, _field, qdev_prop_iothread_vq_mapping_list, \ > IOThreadVirtQueueMappingList *) > > +#define DEFINE_PROP_VMAPPLE_VIRTIO_BLK_VARIANT(_n, _s, _f, _d) \ > + DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_vmapple_virtio_blk_variant, \ > + VMAppleVirtioBlkVariant) > + > #endif > diff --git a/include/hw/vmapple/vmapple.h b/include/hw/vmapple/vmapple.h > index b20956e1286..9c1ad1bd8c3 100644 > --- a/include/hw/vmapple/vmapple.h > +++ b/include/hw/vmapple/vmapple.h > @@ -18,8 +18,6 @@ > > #define TYPE_VMAPPLE_CFG "vmapple-cfg" > > -#define TYPE_VMAPPLE_VIRTIO_BLK "vmapple-virtio-blk" > -#define TYPE_VMAPPLE_VIRTIO_ROOT "vmapple-virtio-root" > -#define TYPE_VMAPPLE_VIRTIO_AUX "vmapple-virtio-aux" > +#define TYPE_VMAPPLE_VIRTIO_BLK_PCI "vmapple-virtio-blk-pci" > > #endif /* HW_VMAPPLE_VMAPPLE_H */ > diff --git a/qapi/virtio.json b/qapi/virtio.json > index 2529c2d8b20..d351d2166ef 100644 > --- a/qapi/virtio.json > +++ b/qapi/virtio.json > @@ -992,3 +992,17 @@ > ## > { 'enum': 'GranuleMode', > 'data': [ '4k', '8k', '16k', '64k', 'host' ] } > + > +## > +# @VMAppleVirtioBlkVariant: > +# > +# @unspecified: The default, not a valid setting. > +# > +# @root: Block device holding the root volume > +# > +# @aux: Block device holding auxiliary data required for boot > +# > +# Since: 9.2 > +## > +{ 'enum': 'VMAppleVirtioBlkVariant', > + 'data': [ 'unspecified', 'root', 'aux' ] }
diff --git a/docs/system/arm/vmapple.rst b/docs/system/arm/vmapple.rst index 67942474b93..efe39b68dc4 100644 --- a/docs/system/arm/vmapple.rst +++ b/docs/system/arm/vmapple.rst @@ -56,8 +56,8 @@ to get better interactive access into the target system: -drive file="$DISK",if=pflash,format=raw \ -drive file="$AUX",if=none,id=aux,format=raw \ -drive file="$DISK",if=none,id=root,format=raw \ - -device vmapple-virtio-aux,drive=aux \ - -device vmapple-virtio-root,drive=root \ + -device vmapple-virtio-blk-pci,variant=aux,drive=aux \ + -device vmapple-virtio-blk-pci,variant=root,drive=root \ -netdev user,id=net0,ipv6=off,hostfwd=tcp::2222-:22,hostfwd=tcp::5901-:5900 \ -device virtio-net-pci,netdev=net0 diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index 35deef05f32..8bf8a3442d6 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -1239,3 +1239,11 @@ const PropertyInfo qdev_prop_iothread_vq_mapping_list = { .set = set_iothread_vq_mapping_list, .release = release_iothread_vq_mapping_list, }; + +const PropertyInfo qdev_prop_vmapple_virtio_blk_variant = { + .name = "VMAppleVirtioBlkVariant", + .enum_table = &VMAppleVirtioBlkVariant_lookup, + .get = qdev_propinfo_get_enum, + .set = qdev_propinfo_set_enum, + .set_default_value = qdev_propinfo_set_default_value_enum, +}; diff --git a/hw/vmapple/virtio-blk.c b/hw/vmapple/virtio-blk.c index 40b33bbdac5..f883e65a5de 100644 --- a/hw/vmapple/virtio-blk.c +++ b/hw/vmapple/virtio-blk.c @@ -24,6 +24,7 @@ #include "qemu/module.h" #include "qapi/error.h" +#define TYPE_VMAPPLE_VIRTIO_BLK "vmapple-virtio-blk" OBJECT_DECLARE_TYPE(VMAppleVirtIOBlk, VMAppleVirtIOBlkClass, VMAPPLE_VIRTIO_BLK) typedef struct VMAppleVirtIOBlkClass { @@ -41,14 +42,10 @@ typedef struct VMAppleVirtIOBlk { /* * vmapple-virtio-blk-pci: This extends VirtioPCIProxy. */ -#define TYPE_VMAPPLE_VIRTIO_BLK_PCI "vmapple-virtio-blk-pci-base" OBJECT_DECLARE_SIMPLE_TYPE(VMAppleVirtIOBlkPCI, VMAPPLE_VIRTIO_BLK_PCI) #define VIRTIO_BLK_T_APPLE_BARRIER 0x10000 -#define VIRTIO_APPLE_TYPE_ROOT 1 -#define VIRTIO_APPLE_TYPE_AUX 2 - static bool vmapple_virtio_blk_handle_unknown_request(VirtIOBlockReq *req, MultiReqBuffer *mrb, uint32_t type) @@ -109,7 +106,7 @@ static const TypeInfo vmapple_virtio_blk_info = { struct VMAppleVirtIOBlkPCI { VirtIOPCIProxy parent_obj; VMAppleVirtIOBlk vdev; - uint32_t apple_type; + VMAppleVirtioBlkVariant variant; }; @@ -119,6 +116,8 @@ static Property vmapple_virtio_blk_pci_properties[] = { VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, DEV_NVECTORS_UNSPECIFIED), + DEFINE_PROP_VMAPPLE_VIRTIO_BLK_VARIANT("variant", VMAppleVirtIOBlkPCI, variant, + VM_APPLE_VIRTIO_BLK_VARIANT_UNSPECIFIED), DEFINE_PROP_END_OF_LIST(), }; @@ -128,6 +127,12 @@ static void vmapple_virtio_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **err DeviceState *vdev = DEVICE(&dev->vdev); VirtIOBlkConf *conf = &dev->vdev.parent_obj.conf; + if (dev->variant == VM_APPLE_VIRTIO_BLK_VARIANT_UNSPECIFIED) { + error_setg(errp, "Device " TYPE_VMAPPLE_VIRTIO_BLK_PCI ": must specify " + "a variant, 'aux' or 'root'"); + return; + } + if (conf->num_queues == VIRTIO_BLK_AUTO_NUM_QUEUES) { conf->num_queues = virtio_pci_optimal_num_queues(0); } @@ -143,7 +148,7 @@ static void vmapple_virtio_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **err */ virtio_add_feature(&dev->vdev.parent_obj.host_features, VIRTIO_BLK_F_ZONED); /* Propagate the apple type down to the virtio-blk device */ - dev->vdev.apple_type = dev->apple_type; + dev->vdev.apple_type = dev->variant; /* and spawn the virtio-blk device */ qdev_realize(vdev, BUS(&vpci_dev->bus), errp); @@ -181,47 +186,16 @@ static void vmapple_virtio_blk_pci_instance_init(Object *obj) } static const VirtioPCIDeviceTypeInfo vmapple_virtio_blk_pci_info = { - .base_name = TYPE_VMAPPLE_VIRTIO_BLK_PCI, - .generic_name = "vmapple-virtio-blk-pci", + .generic_name = TYPE_VMAPPLE_VIRTIO_BLK_PCI, .instance_size = sizeof(VMAppleVirtIOBlkPCI), .instance_init = vmapple_virtio_blk_pci_instance_init, .class_init = vmapple_virtio_blk_pci_class_init, }; -static void vmapple_virtio_root_instance_init(Object *obj) -{ - VMAppleVirtIOBlkPCI *dev = VMAPPLE_VIRTIO_BLK_PCI(obj); - - dev->apple_type = VIRTIO_APPLE_TYPE_ROOT; -} - -static const TypeInfo vmapple_virtio_root_info = { - .name = TYPE_VMAPPLE_VIRTIO_ROOT, - .parent = "vmapple-virtio-blk-pci", - .instance_size = sizeof(VMAppleVirtIOBlkPCI), - .instance_init = vmapple_virtio_root_instance_init, -}; - -static void vmapple_virtio_aux_instance_init(Object *obj) -{ - VMAppleVirtIOBlkPCI *dev = VMAPPLE_VIRTIO_BLK_PCI(obj); - - dev->apple_type = VIRTIO_APPLE_TYPE_AUX; -} - -static const TypeInfo vmapple_virtio_aux_info = { - .name = TYPE_VMAPPLE_VIRTIO_AUX, - .parent = "vmapple-virtio-blk-pci", - .instance_size = sizeof(VMAppleVirtIOBlkPCI), - .instance_init = vmapple_virtio_aux_instance_init, -}; - static void vmapple_virtio_blk_register_types(void) { type_register_static(&vmapple_virtio_blk_info); virtio_pci_types_register(&vmapple_virtio_blk_pci_info); - type_register_static(&vmapple_virtio_root_info); - type_register_static(&vmapple_virtio_aux_info); } type_init(vmapple_virtio_blk_register_types) diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h index cdcc63056e5..6e428f3fcad 100644 --- a/include/hw/qdev-properties-system.h +++ b/include/hw/qdev-properties-system.h @@ -27,6 +27,8 @@ extern const PropertyInfo qdev_prop_pcie_link_speed; extern const PropertyInfo qdev_prop_pcie_link_width; extern const PropertyInfo qdev_prop_cpus390entitlement; extern const PropertyInfo qdev_prop_iothread_vq_mapping_list; +extern const PropertyInfo qdev_prop_vmapple_virtio_blk_variant; + #define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d) \ DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t) @@ -94,4 +96,8 @@ extern const PropertyInfo qdev_prop_iothread_vq_mapping_list; DEFINE_PROP(_name, _state, _field, qdev_prop_iothread_vq_mapping_list, \ IOThreadVirtQueueMappingList *) +#define DEFINE_PROP_VMAPPLE_VIRTIO_BLK_VARIANT(_n, _s, _f, _d) \ + DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_vmapple_virtio_blk_variant, \ + VMAppleVirtioBlkVariant) + #endif diff --git a/include/hw/vmapple/vmapple.h b/include/hw/vmapple/vmapple.h index b20956e1286..9c1ad1bd8c3 100644 --- a/include/hw/vmapple/vmapple.h +++ b/include/hw/vmapple/vmapple.h @@ -18,8 +18,6 @@ #define TYPE_VMAPPLE_CFG "vmapple-cfg" -#define TYPE_VMAPPLE_VIRTIO_BLK "vmapple-virtio-blk" -#define TYPE_VMAPPLE_VIRTIO_ROOT "vmapple-virtio-root" -#define TYPE_VMAPPLE_VIRTIO_AUX "vmapple-virtio-aux" +#define TYPE_VMAPPLE_VIRTIO_BLK_PCI "vmapple-virtio-blk-pci" #endif /* HW_VMAPPLE_VMAPPLE_H */ diff --git a/qapi/virtio.json b/qapi/virtio.json index 2529c2d8b20..d351d2166ef 100644 --- a/qapi/virtio.json +++ b/qapi/virtio.json @@ -992,3 +992,17 @@ ## { 'enum': 'GranuleMode', 'data': [ '4k', '8k', '16k', '64k', 'host' ] } + +## +# @VMAppleVirtioBlkVariant: +# +# @unspecified: The default, not a valid setting. +# +# @root: Block device holding the root volume +# +# @aux: Block device holding auxiliary data required for boot +# +# Since: 9.2 +## +{ 'enum': 'VMAppleVirtioBlkVariant', + 'data': [ 'unspecified', 'root', 'aux' ] }
This reduces the type hierarchy of the vmapple-virtio-blk-pci-base type and vmapple-virtio-root/vmapple-virtio-aux leaf types with a single vmapple-virtio-blk-pci type which exposes a 'variant' enum property which can be set to 'aux' or 'root'. This change removes a bunch of device type boilerplate at the cost of defining a new qapi enum and qdev property type. Documentation for the vmapple machine type is also updated. Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu> --- docs/system/arm/vmapple.rst | 4 +-- hw/core/qdev-properties-system.c | 8 +++++ hw/vmapple/virtio-blk.c | 50 +++++++---------------------- include/hw/qdev-properties-system.h | 6 ++++ include/hw/vmapple/vmapple.h | 4 +-- qapi/virtio.json | 14 ++++++++ 6 files changed, 43 insertions(+), 43 deletions(-)