| Message ID | 20230830161425.91946-12-graf@amazon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce new vmapple machine type | expand |
On Wed, Aug 30, 2023 at 04:14:24PM +0000, Alexander Graf wrote: > Apple has its own virtio-blk PCI device ID where it deviates from the > official virtio-pci spec slightly: It puts a new "apple type" > field at a static offset in config space and introduces a new barrier > command. > > This patch first creates a mechanism for virtio-blk downstream classes to > handle unknown commands. It then creates such a downstream class and a new > vmapple-virtio-blk-pci class which support the additional apple type config > identifier as well as the barrier command. > > It then exposes 2 subclasses from that that we can use to expose root and > aux virtio-blk devices: "vmapple-virtio-root" and "vmapple-virtio-aux". > > Signed-off-by: Alexander Graf <graf@amazon.com> Aside from my comments below: Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > > --- > > v1 -> v2: > > - Rework to make all vmapple virtio-blk logic a subclass > --- > include/hw/pci/pci_ids.h | 1 + > include/hw/virtio/virtio-blk.h | 12 +- > include/hw/vmapple/virtio-blk.h | 39 ++++++ > hw/block/virtio-blk.c | 19 ++- > hw/vmapple/virtio-blk.c | 212 ++++++++++++++++++++++++++++++++ > hw/vmapple/Kconfig | 3 + > hw/vmapple/meson.build | 1 + > 7 files changed, 282 insertions(+), 5 deletions(-) > create mode 100644 include/hw/vmapple/virtio-blk.h > create mode 100644 hw/vmapple/virtio-blk.c > > diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h > index e4386ebb20..74e589a298 100644 > --- a/include/hw/pci/pci_ids.h > +++ b/include/hw/pci/pci_ids.h > @@ -188,6 +188,7 @@ > #define PCI_DEVICE_ID_APPLE_UNI_N_AGP 0x0020 > #define PCI_DEVICE_ID_APPLE_U3_AGP 0x004b > #define PCI_DEVICE_ID_APPLE_UNI_N_GMAC 0x0021 > +#define PCI_DEVICE_ID_APPLE_VIRTIO_BLK 0x1a00 > > #define PCI_VENDOR_ID_SUN 0x108e > #define PCI_DEVICE_ID_SUN_EBUS 0x1000 > diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h > index dafec432ce..381a906410 100644 > --- a/include/hw/virtio/virtio-blk.h > +++ b/include/hw/virtio/virtio-blk.h > @@ -23,7 +23,7 @@ > #include "qom/object.h" > > #define TYPE_VIRTIO_BLK "virtio-blk-device" > -OBJECT_DECLARE_SIMPLE_TYPE(VirtIOBlock, VIRTIO_BLK) > +OBJECT_DECLARE_TYPE(VirtIOBlock, VirtIOBlkClass, VIRTIO_BLK) > > /* This is the last element of the write scatter-gather list */ > struct virtio_blk_inhdr > @@ -91,6 +91,16 @@ typedef struct MultiReqBuffer { > bool is_write; > } MultiReqBuffer; > > +typedef struct VirtIOBlkClass { > + /*< private >*/ > + VirtioDeviceClass parent; > + /*< public >*/ > + bool (*handle_unknown_request)(VirtIOBlockReq *req, MultiReqBuffer *mrb, > + uint32_t type); Vendor-specific commands ;_; > +} VirtIOBlkClass; > + > void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq); > +void virtio_blk_free_request(VirtIOBlockReq *req); > +void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status); > > #endif > diff --git a/include/hw/vmapple/virtio-blk.h b/include/hw/vmapple/virtio-blk.h > new file mode 100644 > index 0000000000..b23106a3df > --- /dev/null > +++ b/include/hw/vmapple/virtio-blk.h > @@ -0,0 +1,39 @@ > +/* > + * VMApple specific VirtIO Block implementation > + * > + * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > + > +#ifndef HW_VMAPPLE_CFG_H > +#define HW_VMAPPLE_CFG_H > + > +#include "hw/sysbus.h" I'm surprised to see this header since this isn't a SysBus device. Is it really needed? > +#include "qom/object.h" > +#include "hw/virtio/virtio-pci.h" > +#include "hw/virtio/virtio-blk.h" > + > +#define TYPE_VMAPPLE_VIRTIO_BLK "vmapple-virtio-blk" > +#define TYPE_VMAPPLE_VIRTIO_ROOT "vmapple-virtio-root" > +#define TYPE_VMAPPLE_VIRTIO_AUX "vmapple-virtio-aux" > + > +OBJECT_DECLARE_TYPE(VMAppleVirtIOBlk, VMAppleVirtIOBlkClass, VMAPPLE_VIRTIO_BLK) > + > +typedef struct VMAppleVirtIOBlkClass { > + /*< private >*/ > + VirtIOBlkClass parent; > + /*< public >*/ > + void (*get_config)(VirtIODevice *vdev, uint8_t *config); > +} VMAppleVirtIOBlkClass; > + > +typedef struct VMAppleVirtIOBlk { > + /* <private> */ > + VirtIOBlock parent_obj; > + > + /* <public> */ > + uint32_t apple_type; > +} VMAppleVirtIOBlk; > + > +#endif /* HW_VMAPPLE_CFG_H */ > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 39e7f23fab..1645cdccbe 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -48,12 +48,12 @@ static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq, > req->mr_next = NULL; > } > > -static void virtio_blk_free_request(VirtIOBlockReq *req) > +void virtio_blk_free_request(VirtIOBlockReq *req) > { > g_free(req); > } > > -static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) > +void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) > { > VirtIOBlock *s = req->dev; > VirtIODevice *vdev = VIRTIO_DEVICE(s); > @@ -1121,8 +1121,18 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) > break; > } > default: > - virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); > - virtio_blk_free_request(req); > + { > + /* > + * Give subclasses a chance to handle unknown requests. This way the > + * class lookup is not in the hot path. > + */ > + VirtIOBlkClass *vbk = VIRTIO_BLK_GET_CLASS(s); > + if (!vbk->handle_unknown_request || > + !vbk->handle_unknown_request(req, mrb, type)) { > + virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); > + virtio_blk_free_request(req); > + } > + } > } > return 0; > } > @@ -1764,6 +1774,7 @@ static const TypeInfo virtio_blk_info = { > .instance_size = sizeof(VirtIOBlock), > .instance_init = virtio_blk_instance_init, > .class_init = virtio_blk_class_init, > + .class_size = sizeof(VirtIOBlkClass), > }; > > static void virtio_register_types(void) > diff --git a/hw/vmapple/virtio-blk.c b/hw/vmapple/virtio-blk.c > new file mode 100644 > index 0000000000..720eaa61a8 > --- /dev/null > +++ b/hw/vmapple/virtio-blk.c > @@ -0,0 +1,212 @@ > +/* > + * VMApple specific VirtIO Block implementation > + * > + * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + * VMApple uses almost standard VirtIO Block, but with a few key differences: > + * > + * - Different PCI device/vendor ID > + * - An additional "type" identifier to differentiate AUX and Root volumes Strange, virtio-blk has a serial string that is intended for differentiating between devices. Maybe that is already being used as a volume label that varies between root volumes, but I would have expected that identifier to live in the file system superblock, partition tables, etc. > + * - An additional BARRIER command > + */ > + > +#include "qemu/osdep.h" > +#include "hw/vmapple/virtio-blk.h" > +#include "qemu/log.h" > +#include "qemu/module.h" > +#include "qapi/error.h" > + > +#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) > +{ > + switch (type) { > + case VIRTIO_BLK_T_APPLE_BARRIER: > + /* We ignore barriers for now. YOLO. */ > + virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); > + virtio_blk_free_request(req); > + return true; This is fine when --blockdev/--drive have cache=unsafe. Can you add a check into .realize() that refuses to start when block_get_flags(blk) & BDRV_O_NO_FLUSH == 0? > + default: > + return false; > + } > +} > + > +/* > + * VMApple virtio-blk uses the same config format as normal virtio, with one > + * exception: It adds an "apple type" specififer at the same location that "specifier" > + * the spec reserves for max_secure_erase_sectors. Let's hook into the > + * get_config code path here, run it as usual and then patch in the apple type. > + */ > +static void vmapple_virtio_blk_get_config(VirtIODevice *vdev, uint8_t *config) > +{ > + VMAppleVirtIOBlk *dev = VMAPPLE_VIRTIO_BLK(vdev); > + VMAppleVirtIOBlkClass *vvbk = VMAPPLE_VIRTIO_BLK_GET_CLASS(dev); > + struct virtio_blk_config *blkcfg = (struct virtio_blk_config *)config; > + > + vvbk->get_config(vdev, config); > + > + g_assert(dev->parent_obj.config_size >= endof(struct virtio_blk_config, zoned)); > + > + /* Apple abuses the field for max_secure_erase_sectors as type id */ > + blkcfg->max_secure_erase_sectors = dev->apple_type; > +} > + > +static Property vmapple_virtio_blk_properties[] = { > + DEFINE_PROP_UINT32("apple-type", VMAppleVirtIOBlk, apple_type, 0), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void vmapple_virtio_blk_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + VirtIOBlkClass *vbk = VIRTIO_BLK_CLASS(klass); > + VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass); > + VMAppleVirtIOBlkClass *vvbk = VMAPPLE_VIRTIO_BLK_CLASS(klass); > + > + vbk->handle_unknown_request = vmapple_virtio_blk_handle_unknown_request; > + vvbk->get_config = vdc->get_config; > + vdc->get_config = vmapple_virtio_blk_get_config; > + device_class_set_props(dc, vmapple_virtio_blk_properties); > +} > + > +static const TypeInfo vmapple_virtio_blk_info = { > + .name = TYPE_VMAPPLE_VIRTIO_BLK, > + .parent = TYPE_VIRTIO_BLK, > + .instance_size = sizeof(VMAppleVirtIOBlk), > + .class_init = vmapple_virtio_blk_class_init, > +}; > + > +/* PCI Devices */ > + > +typedef struct VMAppleVirtIOBlkPCI { > + VirtIOPCIProxy parent_obj; > + VMAppleVirtIOBlk vdev; > + uint32_t apple_type; > +} VMAppleVirtIOBlkPCI; > + > +/* > + * vmapple-virtio-blk-pci: This extends VirtioPCIProxy. > + */ > +#define TYPE_VMAPPLE_VIRTIO_BLK_PCI "vmapple-virtio-blk-pci-base" > +DECLARE_INSTANCE_CHECKER(VMAppleVirtIOBlkPCI, VMAPPLE_VIRTIO_BLK_PCI, > + TYPE_VMAPPLE_VIRTIO_BLK_PCI) > + > +static Property vmapple_virtio_blk_pci_properties[] = { > + DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0), > + DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, > + VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > + DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, > + DEV_NVECTORS_UNSPECIFIED), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void vmapple_virtio_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > +{ > + VMAppleVirtIOBlkPCI *dev = VMAPPLE_VIRTIO_BLK_PCI(vpci_dev); > + DeviceState *vdev = DEVICE(&dev->vdev); > + VirtIOBlkConf *conf = &dev->vdev.parent_obj.conf; > + > + if (conf->num_queues == VIRTIO_BLK_AUTO_NUM_QUEUES) { > + conf->num_queues = virtio_pci_optimal_num_queues(0); > + } > + > + if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) { > + vpci_dev->nvectors = conf->num_queues + 1; > + } > + > + /* > + * We don't support zones, but we need the additional config space size. > + * Let's just expose the feature so the rest of the virtio-blk logic > + * allocates enough space for us. The guest will ignore zones anyway. > + */ > + virtio_add_feature(&dev->vdev.parent_obj.host_features, VIRTIO_BLK_F_ZONED); > + /* Propagate the apple type down to the virtio-blk device */ > + qdev_prop_set_uint32(DEVICE(&dev->vdev), "apple-type", dev->apple_type); > + /* and spawn the virtio-blk device */ > + qdev_realize(vdev, BUS(&vpci_dev->bus), errp); > + > + /* > + * The virtio-pci machinery adjusts its vendor/device ID based on whether > + * we support modern or legacy virtio. Let's patch it back to the Apple > + * identifiers here. > + */ > + pci_config_set_vendor_id(vpci_dev->pci_dev.config, PCI_VENDOR_ID_APPLE); > + pci_config_set_device_id(vpci_dev->pci_dev.config, PCI_DEVICE_ID_APPLE_VIRTIO_BLK); > +} > + > +static void vmapple_virtio_blk_pci_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass); > + PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass); > + > + set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > + device_class_set_props(dc, vmapple_virtio_blk_pci_properties); > + k->realize = vmapple_virtio_blk_pci_realize; > + pcidev_k->vendor_id = PCI_VENDOR_ID_APPLE; > + pcidev_k->device_id = PCI_DEVICE_ID_APPLE_VIRTIO_BLK; > + pcidev_k->revision = VIRTIO_PCI_ABI_VERSION; > + pcidev_k->class_id = PCI_CLASS_STORAGE_SCSI; > +} > + > +static void vmapple_virtio_blk_pci_instance_init(Object *obj) > +{ > + VMAppleVirtIOBlkPCI *dev = VMAPPLE_VIRTIO_BLK_PCI(obj); > + > + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev), > + TYPE_VMAPPLE_VIRTIO_BLK); > +} > + > +static const VirtioPCIDeviceTypeInfo vmapple_virtio_blk_pci_info = { > + .base_name = TYPE_VMAPPLE_VIRTIO_BLK_PCI, > + .generic_name = "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/hw/vmapple/Kconfig b/hw/vmapple/Kconfig > index ba37fc5b81..f06eae8039 100644 > --- a/hw/vmapple/Kconfig > +++ b/hw/vmapple/Kconfig > @@ -9,3 +9,6 @@ config VMAPPLE_CFG > > config VMAPPLE_PVG > bool > + > +config VMAPPLE_VIRTIO_BLK > + bool > diff --git a/hw/vmapple/meson.build b/hw/vmapple/meson.build > index 74a0d7a5f1..3b4a16f619 100644 > --- a/hw/vmapple/meson.build > +++ b/hw/vmapple/meson.build > @@ -2,3 +2,4 @@ system_ss.add(when: 'CONFIG_VMAPPLE_AES', if_true: files('aes.c')) > system_ss.add(when: 'CONFIG_VMAPPLE_BDIF', if_true: files('bdif.c')) > system_ss.add(when: 'CONFIG_VMAPPLE_CFG', if_true: files('cfg.c')) > system_ss.add(when: 'CONFIG_VMAPPLE_PVG', if_true: [files('apple-gfx.m'), pvg, metal]) > +system_ss.add(when: 'CONFIG_VMAPPLE_VIRTIO_BLK', if_true: files('virtio-blk.c')) > -- > 2.39.2 (Apple Git-143) > > > > > Amazon Development Center Germany GmbH > Krausenstr. 38 > 10117 Berlin > Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss > Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B > Sitz: Berlin > Ust-ID: DE 289 237 879 > >
On 31/8/23 22:03, Stefan Hajnoczi wrote: > On Wed, Aug 30, 2023 at 04:14:24PM +0000, Alexander Graf wrote: >> Apple has its own virtio-blk PCI device ID where it deviates from the >> official virtio-pci spec slightly: It puts a new "apple type" >> field at a static offset in config space and introduces a new barrier >> command. >> >> This patch first creates a mechanism for virtio-blk downstream classes to >> handle unknown commands. It then creates such a downstream class and a new >> vmapple-virtio-blk-pci class which support the additional apple type config >> identifier as well as the barrier command. >> >> It then exposes 2 subclasses from that that we can use to expose root and >> aux virtio-blk devices: "vmapple-virtio-root" and "vmapple-virtio-aux". >> >> Signed-off-by: Alexander Graf <graf@amazon.com> > > Aside from my comments below: > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > >> >> --- >> >> v1 -> v2: >> >> - Rework to make all vmapple virtio-blk logic a subclass >> --- >> include/hw/pci/pci_ids.h | 1 + >> include/hw/virtio/virtio-blk.h | 12 +- >> include/hw/vmapple/virtio-blk.h | 39 ++++++ >> hw/block/virtio-blk.c | 19 ++- >> hw/vmapple/virtio-blk.c | 212 ++++++++++++++++++++++++++++++++ >> hw/vmapple/Kconfig | 3 + >> hw/vmapple/meson.build | 1 + >> 7 files changed, 282 insertions(+), 5 deletions(-) >> create mode 100644 include/hw/vmapple/virtio-blk.h >> create mode 100644 hw/vmapple/virtio-blk.c >> diff --git a/include/hw/vmapple/virtio-blk.h b/include/hw/vmapple/virtio-blk.h >> new file mode 100644 >> index 0000000000..b23106a3df >> --- /dev/null >> +++ b/include/hw/vmapple/virtio-blk.h >> @@ -0,0 +1,39 @@ >> +/* >> + * VMApple specific VirtIO Block implementation >> + * >> + * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + */ >> + >> +#ifndef HW_VMAPPLE_CFG_H >> +#define HW_VMAPPLE_CFG_H >> + >> +#include "hw/sysbus.h" > > I'm surprised to see this header since this isn't a SysBus device. Is it > really needed? > >> +#include "qom/object.h" >> +#include "hw/virtio/virtio-pci.h" "virtio-pci.h" surprised me too. >> +#include "hw/virtio/virtio-blk.h" >> + >> +#define TYPE_VMAPPLE_VIRTIO_BLK "vmapple-virtio-blk" >> +#define TYPE_VMAPPLE_VIRTIO_ROOT "vmapple-virtio-root" >> +#define TYPE_VMAPPLE_VIRTIO_AUX "vmapple-virtio-aux" >> + >> +OBJECT_DECLARE_TYPE(VMAppleVirtIOBlk, VMAppleVirtIOBlkClass, VMAPPLE_VIRTIO_BLK) >> + >> +typedef struct VMAppleVirtIOBlkClass { >> + /*< private >*/ >> + VirtIOBlkClass parent; >> + /*< public >*/ >> + void (*get_config)(VirtIODevice *vdev, uint8_t *config); >> +} VMAppleVirtIOBlkClass; >> + >> +typedef struct VMAppleVirtIOBlk { >> + /* <private> */ >> + VirtIOBlock parent_obj; >> + >> + /* <public> */ >> + uint32_t apple_type; >> +} VMAppleVirtIOBlk; >> + >> +#endif /* HW_VMAPPLE_CFG_H */
On 30/08/2023 17:14, Alexander Graf wrote: Hi Alex, > Apple has its own virtio-blk PCI device ID where it deviates from the > official virtio-pci spec slightly: It puts a new "apple type" > field at a static offset in config space and introduces a new barrier > command. > > This patch first creates a mechanism for virtio-blk downstream classes to > handle unknown commands. It then creates such a downstream class and a new > vmapple-virtio-blk-pci class which support the additional apple type config > identifier as well as the barrier command. > > It then exposes 2 subclasses from that that we can use to expose root and > aux virtio-blk devices: "vmapple-virtio-root" and "vmapple-virtio-aux". > > Signed-off-by: Alexander Graf <graf@amazon.com> > > --- > > v1 -> v2: > > - Rework to make all vmapple virtio-blk logic a subclass > --- > include/hw/pci/pci_ids.h | 1 + > include/hw/virtio/virtio-blk.h | 12 +- > include/hw/vmapple/virtio-blk.h | 39 ++++++ > hw/block/virtio-blk.c | 19 ++- > hw/vmapple/virtio-blk.c | 212 ++++++++++++++++++++++++++++++++ > hw/vmapple/Kconfig | 3 + > hw/vmapple/meson.build | 1 + > 7 files changed, 282 insertions(+), 5 deletions(-) > create mode 100644 include/hw/vmapple/virtio-blk.h > create mode 100644 hw/vmapple/virtio-blk.c > > diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h > index e4386ebb20..74e589a298 100644 > --- a/include/hw/pci/pci_ids.h > +++ b/include/hw/pci/pci_ids.h > @@ -188,6 +188,7 @@ > #define PCI_DEVICE_ID_APPLE_UNI_N_AGP 0x0020 > #define PCI_DEVICE_ID_APPLE_U3_AGP 0x004b > #define PCI_DEVICE_ID_APPLE_UNI_N_GMAC 0x0021 > +#define PCI_DEVICE_ID_APPLE_VIRTIO_BLK 0x1a00 > > #define PCI_VENDOR_ID_SUN 0x108e > #define PCI_DEVICE_ID_SUN_EBUS 0x1000 > diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h > index dafec432ce..381a906410 100644 > --- a/include/hw/virtio/virtio-blk.h > +++ b/include/hw/virtio/virtio-blk.h > @@ -23,7 +23,7 @@ > #include "qom/object.h" > > #define TYPE_VIRTIO_BLK "virtio-blk-device" > -OBJECT_DECLARE_SIMPLE_TYPE(VirtIOBlock, VIRTIO_BLK) > +OBJECT_DECLARE_TYPE(VirtIOBlock, VirtIOBlkClass, VIRTIO_BLK) > > /* This is the last element of the write scatter-gather list */ > struct virtio_blk_inhdr > @@ -91,6 +91,16 @@ typedef struct MultiReqBuffer { > bool is_write; > } MultiReqBuffer; > > +typedef struct VirtIOBlkClass { > + /*< private >*/ > + VirtioDeviceClass parent; This should be parent_class for consistency. > + /*< public >*/ > + bool (*handle_unknown_request)(VirtIOBlockReq *req, MultiReqBuffer *mrb, > + uint32_t type); > +} VirtIOBlkClass; > + Same as before you can drop the typedef and public/private comments, and a newline after parent_class. > void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq); > +void virtio_blk_free_request(VirtIOBlockReq *req); > +void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status); > > #endif > diff --git a/include/hw/vmapple/virtio-blk.h b/include/hw/vmapple/virtio-blk.h > new file mode 100644 > index 0000000000..b23106a3df > --- /dev/null > +++ b/include/hw/vmapple/virtio-blk.h > @@ -0,0 +1,39 @@ > +/* > + * VMApple specific VirtIO Block implementation > + * > + * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > + > +#ifndef HW_VMAPPLE_CFG_H > +#define HW_VMAPPLE_CFG_H > + > +#include "hw/sysbus.h" > +#include "qom/object.h" > +#include "hw/virtio/virtio-pci.h" > +#include "hw/virtio/virtio-blk.h" > + > +#define TYPE_VMAPPLE_VIRTIO_BLK "vmapple-virtio-blk" > +#define TYPE_VMAPPLE_VIRTIO_ROOT "vmapple-virtio-root" > +#define TYPE_VMAPPLE_VIRTIO_AUX "vmapple-virtio-aux" > + > +OBJECT_DECLARE_TYPE(VMAppleVirtIOBlk, VMAppleVirtIOBlkClass, VMAPPLE_VIRTIO_BLK) > + > +typedef struct VMAppleVirtIOBlkClass { > + /*< private >*/ > + VirtIOBlkClass parent; > + /*< public >*/ > + void (*get_config)(VirtIODevice *vdev, uint8_t *config); > +} VMAppleVirtIOBlkClass; Same here. > +typedef struct VMAppleVirtIOBlk { > + /* <private> */ > + VirtIOBlock parent_obj; > + > + /* <public> */ > + uint32_t apple_type; > +} VMAppleVirtIOBlk; And here. > +#endif /* HW_VMAPPLE_CFG_H */ > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 39e7f23fab..1645cdccbe 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -48,12 +48,12 @@ static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq, > req->mr_next = NULL; > } > > -static void virtio_blk_free_request(VirtIOBlockReq *req) > +void virtio_blk_free_request(VirtIOBlockReq *req) > { > g_free(req); > } > > -static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) > +void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) > { > VirtIOBlock *s = req->dev; > VirtIODevice *vdev = VIRTIO_DEVICE(s); > @@ -1121,8 +1121,18 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) > break; > } > default: > - virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); > - virtio_blk_free_request(req); > + { > + /* > + * Give subclasses a chance to handle unknown requests. This way the > + * class lookup is not in the hot path. > + */ > + VirtIOBlkClass *vbk = VIRTIO_BLK_GET_CLASS(s); > + if (!vbk->handle_unknown_request || > + !vbk->handle_unknown_request(req, mrb, type)) { > + virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); > + virtio_blk_free_request(req); > + } > + } > } > return 0; > } > @@ -1764,6 +1774,7 @@ static const TypeInfo virtio_blk_info = { > .instance_size = sizeof(VirtIOBlock), > .instance_init = virtio_blk_instance_init, > .class_init = virtio_blk_class_init, > + .class_size = sizeof(VirtIOBlkClass), > }; > > static void virtio_register_types(void) > diff --git a/hw/vmapple/virtio-blk.c b/hw/vmapple/virtio-blk.c > new file mode 100644 > index 0000000000..720eaa61a8 > --- /dev/null > +++ b/hw/vmapple/virtio-blk.c > @@ -0,0 +1,212 @@ > +/* > + * VMApple specific VirtIO Block implementation > + * > + * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + * VMApple uses almost standard VirtIO Block, but with a few key differences: > + * > + * - Different PCI device/vendor ID > + * - An additional "type" identifier to differentiate AUX and Root volumes > + * - An additional BARRIER command > + */ > + > +#include "qemu/osdep.h" > +#include "hw/vmapple/virtio-blk.h" > +#include "qemu/log.h" > +#include "qemu/module.h" > +#include "qapi/error.h" > + > +#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) > +{ > + switch (type) { > + case VIRTIO_BLK_T_APPLE_BARRIER: > + /* We ignore barriers for now. YOLO. */ > + virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); > + virtio_blk_free_request(req); > + return true; > + default: > + return false; > + } > +} > + > +/* > + * VMApple virtio-blk uses the same config format as normal virtio, with one > + * exception: It adds an "apple type" specififer at the same location that > + * the spec reserves for max_secure_erase_sectors. Let's hook into the > + * get_config code path here, run it as usual and then patch in the apple type. > + */ > +static void vmapple_virtio_blk_get_config(VirtIODevice *vdev, uint8_t *config) > +{ > + VMAppleVirtIOBlk *dev = VMAPPLE_VIRTIO_BLK(vdev); > + VMAppleVirtIOBlkClass *vvbk = VMAPPLE_VIRTIO_BLK_GET_CLASS(dev); > + struct virtio_blk_config *blkcfg = (struct virtio_blk_config *)config; > + > + vvbk->get_config(vdev, config); > + > + g_assert(dev->parent_obj.config_size >= endof(struct virtio_blk_config, zoned)); You want to use a QOM cast macro instead of using parent_obj directly here. > + /* Apple abuses the field for max_secure_erase_sectors as type id */ > + blkcfg->max_secure_erase_sectors = dev->apple_type; > +} > + > +static Property vmapple_virtio_blk_properties[] = { > + DEFINE_PROP_UINT32("apple-type", VMAppleVirtIOBlk, apple_type, 0), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void vmapple_virtio_blk_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + VirtIOBlkClass *vbk = VIRTIO_BLK_CLASS(klass); > + VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass); > + VMAppleVirtIOBlkClass *vvbk = VMAPPLE_VIRTIO_BLK_CLASS(klass); > + > + vbk->handle_unknown_request = vmapple_virtio_blk_handle_unknown_request; > + vvbk->get_config = vdc->get_config; > + vdc->get_config = vmapple_virtio_blk_get_config; > + device_class_set_props(dc, vmapple_virtio_blk_properties); > +} > + > +static const TypeInfo vmapple_virtio_blk_info = { > + .name = TYPE_VMAPPLE_VIRTIO_BLK, > + .parent = TYPE_VIRTIO_BLK, > + .instance_size = sizeof(VMAppleVirtIOBlk), > + .class_init = vmapple_virtio_blk_class_init, > +}; > + > +/* PCI Devices */ > + > +typedef struct VMAppleVirtIOBlkPCI { > + VirtIOPCIProxy parent_obj; > + VMAppleVirtIOBlk vdev; > + uint32_t apple_type; > +} VMAppleVirtIOBlkPCI; Same here re: typedef, newline after parent_obj. And again this definition and the QOM declaration macro should be in a separate .h file. > +/* > + * vmapple-virtio-blk-pci: This extends VirtioPCIProxy. > + */ > +#define TYPE_VMAPPLE_VIRTIO_BLK_PCI "vmapple-virtio-blk-pci-base" > +DECLARE_INSTANCE_CHECKER(VMAppleVirtIOBlkPCI, VMAPPLE_VIRTIO_BLK_PCI, > + TYPE_VMAPPLE_VIRTIO_BLK_PCI) I believe you can just use OBJECT_DECLARE_SIMPLE_TYPE() here? > +static Property vmapple_virtio_blk_pci_properties[] = { > + DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0), > + DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, > + VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > + DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, > + DEV_NVECTORS_UNSPECIFIED), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void vmapple_virtio_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) > +{ > + VMAppleVirtIOBlkPCI *dev = VMAPPLE_VIRTIO_BLK_PCI(vpci_dev); > + DeviceState *vdev = DEVICE(&dev->vdev); > + VirtIOBlkConf *conf = &dev->vdev.parent_obj.conf; Here you should use the appropriate QOM cast macro instead of referencing parent_obj directly. > + > + if (conf->num_queues == VIRTIO_BLK_AUTO_NUM_QUEUES) { > + conf->num_queues = virtio_pci_optimal_num_queues(0); > + } > + > + if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) { > + vpci_dev->nvectors = conf->num_queues + 1; > + } > + > + /* > + * We don't support zones, but we need the additional config space size. > + * Let's just expose the feature so the rest of the virtio-blk logic > + * allocates enough space for us. The guest will ignore zones anyway. > + */ > + virtio_add_feature(&dev->vdev.parent_obj.host_features, VIRTIO_BLK_F_ZONED); And here too. > + /* Propagate the apple type down to the virtio-blk device */ > + qdev_prop_set_uint32(DEVICE(&dev->vdev), "apple-type", dev->apple_type); > + /* and spawn the virtio-blk device */ > + qdev_realize(vdev, BUS(&vpci_dev->bus), errp); It took me a while to figure out how virtio_blk_device_realize() could be called, but that appears to be done via VirtioDeviceClass::realize? > + /* > + * The virtio-pci machinery adjusts its vendor/device ID based on whether > + * we support modern or legacy virtio. Let's patch it back to the Apple > + * identifiers here. > + */ > + pci_config_set_vendor_id(vpci_dev->pci_dev.config, PCI_VENDOR_ID_APPLE); > + pci_config_set_device_id(vpci_dev->pci_dev.config, PCI_DEVICE_ID_APPLE_VIRTIO_BLK); > +} > + > +static void vmapple_virtio_blk_pci_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass); > + PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass); > + > + set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); > + device_class_set_props(dc, vmapple_virtio_blk_pci_properties); > + k->realize = vmapple_virtio_blk_pci_realize; > + pcidev_k->vendor_id = PCI_VENDOR_ID_APPLE; > + pcidev_k->device_id = PCI_DEVICE_ID_APPLE_VIRTIO_BLK; > + pcidev_k->revision = VIRTIO_PCI_ABI_VERSION; > + pcidev_k->class_id = PCI_CLASS_STORAGE_SCSI; > +} > + > +static void vmapple_virtio_blk_pci_instance_init(Object *obj) > +{ > + VMAppleVirtIOBlkPCI *dev = VMAPPLE_VIRTIO_BLK_PCI(obj); > + > + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev), > + TYPE_VMAPPLE_VIRTIO_BLK); > +} > + > +static const VirtioPCIDeviceTypeInfo vmapple_virtio_blk_pci_info = { > + .base_name = TYPE_VMAPPLE_VIRTIO_BLK_PCI, > + .generic_name = "vmapple-virtio-blk-pci", Does it make sense to use a macro for the generic name? > + .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, Is there any need to mention VIRTIO_BLK as opposed to VIRTIO in the typename? > + .parent = "vmapple-virtio-blk-pci", Same question here re: using a macro. > + .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, Similar question here re: VIRTIO vs. VIRTIO_BLK. > + .parent = "vmapple-virtio-blk-pci", Similar question here re: using a macro. > + .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) And again use DEFINE_TYPES(). > diff --git a/hw/vmapple/Kconfig b/hw/vmapple/Kconfig > index ba37fc5b81..f06eae8039 100644 > --- a/hw/vmapple/Kconfig > +++ b/hw/vmapple/Kconfig > @@ -9,3 +9,6 @@ config VMAPPLE_CFG > > config VMAPPLE_PVG > bool > + > +config VMAPPLE_VIRTIO_BLK > + bool > diff --git a/hw/vmapple/meson.build b/hw/vmapple/meson.build > index 74a0d7a5f1..3b4a16f619 100644 > --- a/hw/vmapple/meson.build > +++ b/hw/vmapple/meson.build > @@ -2,3 +2,4 @@ system_ss.add(when: 'CONFIG_VMAPPLE_AES', if_true: files('aes.c')) > system_ss.add(when: 'CONFIG_VMAPPLE_BDIF', if_true: files('bdif.c')) > system_ss.add(when: 'CONFIG_VMAPPLE_CFG', if_true: files('cfg.c')) > system_ss.add(when: 'CONFIG_VMAPPLE_PVG', if_true: [files('apple-gfx.m'), pvg, metal]) > +system_ss.add(when: 'CONFIG_VMAPPLE_VIRTIO_BLK', if_true: files('virtio-blk.c')) ATB, Mark.
diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h index e4386ebb20..74e589a298 100644 --- a/include/hw/pci/pci_ids.h +++ b/include/hw/pci/pci_ids.h @@ -188,6 +188,7 @@ #define PCI_DEVICE_ID_APPLE_UNI_N_AGP 0x0020 #define PCI_DEVICE_ID_APPLE_U3_AGP 0x004b #define PCI_DEVICE_ID_APPLE_UNI_N_GMAC 0x0021 +#define PCI_DEVICE_ID_APPLE_VIRTIO_BLK 0x1a00 #define PCI_VENDOR_ID_SUN 0x108e #define PCI_DEVICE_ID_SUN_EBUS 0x1000 diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index dafec432ce..381a906410 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -23,7 +23,7 @@ #include "qom/object.h" #define TYPE_VIRTIO_BLK "virtio-blk-device" -OBJECT_DECLARE_SIMPLE_TYPE(VirtIOBlock, VIRTIO_BLK) +OBJECT_DECLARE_TYPE(VirtIOBlock, VirtIOBlkClass, VIRTIO_BLK) /* This is the last element of the write scatter-gather list */ struct virtio_blk_inhdr @@ -91,6 +91,16 @@ typedef struct MultiReqBuffer { bool is_write; } MultiReqBuffer; +typedef struct VirtIOBlkClass { + /*< private >*/ + VirtioDeviceClass parent; + /*< public >*/ + bool (*handle_unknown_request)(VirtIOBlockReq *req, MultiReqBuffer *mrb, + uint32_t type); +} VirtIOBlkClass; + void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq); +void virtio_blk_free_request(VirtIOBlockReq *req); +void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status); #endif diff --git a/include/hw/vmapple/virtio-blk.h b/include/hw/vmapple/virtio-blk.h new file mode 100644 index 0000000000..b23106a3df --- /dev/null +++ b/include/hw/vmapple/virtio-blk.h @@ -0,0 +1,39 @@ +/* + * VMApple specific VirtIO Block implementation + * + * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#ifndef HW_VMAPPLE_CFG_H +#define HW_VMAPPLE_CFG_H + +#include "hw/sysbus.h" +#include "qom/object.h" +#include "hw/virtio/virtio-pci.h" +#include "hw/virtio/virtio-blk.h" + +#define TYPE_VMAPPLE_VIRTIO_BLK "vmapple-virtio-blk" +#define TYPE_VMAPPLE_VIRTIO_ROOT "vmapple-virtio-root" +#define TYPE_VMAPPLE_VIRTIO_AUX "vmapple-virtio-aux" + +OBJECT_DECLARE_TYPE(VMAppleVirtIOBlk, VMAppleVirtIOBlkClass, VMAPPLE_VIRTIO_BLK) + +typedef struct VMAppleVirtIOBlkClass { + /*< private >*/ + VirtIOBlkClass parent; + /*< public >*/ + void (*get_config)(VirtIODevice *vdev, uint8_t *config); +} VMAppleVirtIOBlkClass; + +typedef struct VMAppleVirtIOBlk { + /* <private> */ + VirtIOBlock parent_obj; + + /* <public> */ + uint32_t apple_type; +} VMAppleVirtIOBlk; + +#endif /* HW_VMAPPLE_CFG_H */ diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 39e7f23fab..1645cdccbe 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -48,12 +48,12 @@ static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq, req->mr_next = NULL; } -static void virtio_blk_free_request(VirtIOBlockReq *req) +void virtio_blk_free_request(VirtIOBlockReq *req) { g_free(req); } -static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) +void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) { VirtIOBlock *s = req->dev; VirtIODevice *vdev = VIRTIO_DEVICE(s); @@ -1121,8 +1121,18 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) break; } default: - virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); - virtio_blk_free_request(req); + { + /* + * Give subclasses a chance to handle unknown requests. This way the + * class lookup is not in the hot path. + */ + VirtIOBlkClass *vbk = VIRTIO_BLK_GET_CLASS(s); + if (!vbk->handle_unknown_request || + !vbk->handle_unknown_request(req, mrb, type)) { + virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); + virtio_blk_free_request(req); + } + } } return 0; } @@ -1764,6 +1774,7 @@ static const TypeInfo virtio_blk_info = { .instance_size = sizeof(VirtIOBlock), .instance_init = virtio_blk_instance_init, .class_init = virtio_blk_class_init, + .class_size = sizeof(VirtIOBlkClass), }; static void virtio_register_types(void) diff --git a/hw/vmapple/virtio-blk.c b/hw/vmapple/virtio-blk.c new file mode 100644 index 0000000000..720eaa61a8 --- /dev/null +++ b/hw/vmapple/virtio-blk.c @@ -0,0 +1,212 @@ +/* + * VMApple specific VirtIO Block implementation + * + * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + * VMApple uses almost standard VirtIO Block, but with a few key differences: + * + * - Different PCI device/vendor ID + * - An additional "type" identifier to differentiate AUX and Root volumes + * - An additional BARRIER command + */ + +#include "qemu/osdep.h" +#include "hw/vmapple/virtio-blk.h" +#include "qemu/log.h" +#include "qemu/module.h" +#include "qapi/error.h" + +#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) +{ + switch (type) { + case VIRTIO_BLK_T_APPLE_BARRIER: + /* We ignore barriers for now. YOLO. */ + virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); + virtio_blk_free_request(req); + return true; + default: + return false; + } +} + +/* + * VMApple virtio-blk uses the same config format as normal virtio, with one + * exception: It adds an "apple type" specififer at the same location that + * the spec reserves for max_secure_erase_sectors. Let's hook into the + * get_config code path here, run it as usual and then patch in the apple type. + */ +static void vmapple_virtio_blk_get_config(VirtIODevice *vdev, uint8_t *config) +{ + VMAppleVirtIOBlk *dev = VMAPPLE_VIRTIO_BLK(vdev); + VMAppleVirtIOBlkClass *vvbk = VMAPPLE_VIRTIO_BLK_GET_CLASS(dev); + struct virtio_blk_config *blkcfg = (struct virtio_blk_config *)config; + + vvbk->get_config(vdev, config); + + g_assert(dev->parent_obj.config_size >= endof(struct virtio_blk_config, zoned)); + + /* Apple abuses the field for max_secure_erase_sectors as type id */ + blkcfg->max_secure_erase_sectors = dev->apple_type; +} + +static Property vmapple_virtio_blk_properties[] = { + DEFINE_PROP_UINT32("apple-type", VMAppleVirtIOBlk, apple_type, 0), + DEFINE_PROP_END_OF_LIST(), +}; + +static void vmapple_virtio_blk_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + VirtIOBlkClass *vbk = VIRTIO_BLK_CLASS(klass); + VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass); + VMAppleVirtIOBlkClass *vvbk = VMAPPLE_VIRTIO_BLK_CLASS(klass); + + vbk->handle_unknown_request = vmapple_virtio_blk_handle_unknown_request; + vvbk->get_config = vdc->get_config; + vdc->get_config = vmapple_virtio_blk_get_config; + device_class_set_props(dc, vmapple_virtio_blk_properties); +} + +static const TypeInfo vmapple_virtio_blk_info = { + .name = TYPE_VMAPPLE_VIRTIO_BLK, + .parent = TYPE_VIRTIO_BLK, + .instance_size = sizeof(VMAppleVirtIOBlk), + .class_init = vmapple_virtio_blk_class_init, +}; + +/* PCI Devices */ + +typedef struct VMAppleVirtIOBlkPCI { + VirtIOPCIProxy parent_obj; + VMAppleVirtIOBlk vdev; + uint32_t apple_type; +} VMAppleVirtIOBlkPCI; + +/* + * vmapple-virtio-blk-pci: This extends VirtioPCIProxy. + */ +#define TYPE_VMAPPLE_VIRTIO_BLK_PCI "vmapple-virtio-blk-pci-base" +DECLARE_INSTANCE_CHECKER(VMAppleVirtIOBlkPCI, VMAPPLE_VIRTIO_BLK_PCI, + TYPE_VMAPPLE_VIRTIO_BLK_PCI) + +static Property vmapple_virtio_blk_pci_properties[] = { + DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0), + DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, + VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), + DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, + DEV_NVECTORS_UNSPECIFIED), + DEFINE_PROP_END_OF_LIST(), +}; + +static void vmapple_virtio_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) +{ + VMAppleVirtIOBlkPCI *dev = VMAPPLE_VIRTIO_BLK_PCI(vpci_dev); + DeviceState *vdev = DEVICE(&dev->vdev); + VirtIOBlkConf *conf = &dev->vdev.parent_obj.conf; + + if (conf->num_queues == VIRTIO_BLK_AUTO_NUM_QUEUES) { + conf->num_queues = virtio_pci_optimal_num_queues(0); + } + + if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) { + vpci_dev->nvectors = conf->num_queues + 1; + } + + /* + * We don't support zones, but we need the additional config space size. + * Let's just expose the feature so the rest of the virtio-blk logic + * allocates enough space for us. The guest will ignore zones anyway. + */ + virtio_add_feature(&dev->vdev.parent_obj.host_features, VIRTIO_BLK_F_ZONED); + /* Propagate the apple type down to the virtio-blk device */ + qdev_prop_set_uint32(DEVICE(&dev->vdev), "apple-type", dev->apple_type); + /* and spawn the virtio-blk device */ + qdev_realize(vdev, BUS(&vpci_dev->bus), errp); + + /* + * The virtio-pci machinery adjusts its vendor/device ID based on whether + * we support modern or legacy virtio. Let's patch it back to the Apple + * identifiers here. + */ + pci_config_set_vendor_id(vpci_dev->pci_dev.config, PCI_VENDOR_ID_APPLE); + pci_config_set_device_id(vpci_dev->pci_dev.config, PCI_DEVICE_ID_APPLE_VIRTIO_BLK); +} + +static void vmapple_virtio_blk_pci_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass); + PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass); + + set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); + device_class_set_props(dc, vmapple_virtio_blk_pci_properties); + k->realize = vmapple_virtio_blk_pci_realize; + pcidev_k->vendor_id = PCI_VENDOR_ID_APPLE; + pcidev_k->device_id = PCI_DEVICE_ID_APPLE_VIRTIO_BLK; + pcidev_k->revision = VIRTIO_PCI_ABI_VERSION; + pcidev_k->class_id = PCI_CLASS_STORAGE_SCSI; +} + +static void vmapple_virtio_blk_pci_instance_init(Object *obj) +{ + VMAppleVirtIOBlkPCI *dev = VMAPPLE_VIRTIO_BLK_PCI(obj); + + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev), + TYPE_VMAPPLE_VIRTIO_BLK); +} + +static const VirtioPCIDeviceTypeInfo vmapple_virtio_blk_pci_info = { + .base_name = TYPE_VMAPPLE_VIRTIO_BLK_PCI, + .generic_name = "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/hw/vmapple/Kconfig b/hw/vmapple/Kconfig index ba37fc5b81..f06eae8039 100644 --- a/hw/vmapple/Kconfig +++ b/hw/vmapple/Kconfig @@ -9,3 +9,6 @@ config VMAPPLE_CFG config VMAPPLE_PVG bool + +config VMAPPLE_VIRTIO_BLK + bool diff --git a/hw/vmapple/meson.build b/hw/vmapple/meson.build index 74a0d7a5f1..3b4a16f619 100644 --- a/hw/vmapple/meson.build +++ b/hw/vmapple/meson.build @@ -2,3 +2,4 @@ system_ss.add(when: 'CONFIG_VMAPPLE_AES', if_true: files('aes.c')) system_ss.add(when: 'CONFIG_VMAPPLE_BDIF', if_true: files('bdif.c')) system_ss.add(when: 'CONFIG_VMAPPLE_CFG', if_true: files('cfg.c')) system_ss.add(when: 'CONFIG_VMAPPLE_PVG', if_true: [files('apple-gfx.m'), pvg, metal]) +system_ss.add(when: 'CONFIG_VMAPPLE_VIRTIO_BLK', if_true: files('virtio-blk.c'))
Apple has its own virtio-blk PCI device ID where it deviates from the official virtio-pci spec slightly: It puts a new "apple type" field at a static offset in config space and introduces a new barrier command. This patch first creates a mechanism for virtio-blk downstream classes to handle unknown commands. It then creates such a downstream class and a new vmapple-virtio-blk-pci class which support the additional apple type config identifier as well as the barrier command. It then exposes 2 subclasses from that that we can use to expose root and aux virtio-blk devices: "vmapple-virtio-root" and "vmapple-virtio-aux". Signed-off-by: Alexander Graf <graf@amazon.com> --- v1 -> v2: - Rework to make all vmapple virtio-blk logic a subclass --- include/hw/pci/pci_ids.h | 1 + include/hw/virtio/virtio-blk.h | 12 +- include/hw/vmapple/virtio-blk.h | 39 ++++++ hw/block/virtio-blk.c | 19 ++- hw/vmapple/virtio-blk.c | 212 ++++++++++++++++++++++++++++++++ hw/vmapple/Kconfig | 3 + hw/vmapple/meson.build | 1 + 7 files changed, 282 insertions(+), 5 deletions(-) create mode 100644 include/hw/vmapple/virtio-blk.h create mode 100644 hw/vmapple/virtio-blk.c