On Wed, Jun 14, 2023 at 10:56:22PM +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 discard
> command.
>
> This patch adds a new qdev property called "apple-type" to virtio-blk-pci.
> When that property is set, we assume the virtio-blk device is an Apple one
> of the specific type and act accordingly.
I wonder if we should treat these as two separate devices. ie define
a 'apple-virtio-blk' device name, and have it be a subclass of the
main virtio blk impl. That would allow distros to drop the apple
forked impl in their downstream when they seek to minimize their
support matrix.
>
> Signed-off-by: Alexander Graf <g...@amazon.com>
> ---
> hw/block/virtio-blk.c | 23 +++++++++++++++++++++
> hw/virtio/virtio-blk-pci.c | 7 +++++++
> include/hw/pci/pci_ids.h | 1 +
> include/hw/virtio/virtio-blk.h | 1 +
> include/standard-headers/linux/virtio_blk.h | 3 +++
> 5 files changed, 35 insertions(+)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 39e7f23fab..76b85bb3cb 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -1120,6 +1120,20 @@ static int virtio_blk_handle_request(VirtIOBlockReq
> *req, MultiReqBuffer *mrb)
>
> break;
> }
> + case VIRTIO_BLK_T_APPLE1:
> + {
> + if (s->conf.x_apple_type) {
> + /* Only valid on Apple Virtio */
> + char buf[iov_size(in_iov, in_num)];
> + memset(buf, 0, sizeof(buf));
> + iov_from_buf(in_iov, in_num, 0, buf, sizeof(buf));
> + virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
> + } else {
> + virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
> + }
> + virtio_blk_free_request(req);
> + break;
> + }
> default:
> virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
> virtio_blk_free_request(req);
> @@ -1351,6 +1365,10 @@ static void virtio_blk_update_config(VirtIODevice
> *vdev, uint8_t *config)
> } else {
> blkcfg.zoned.model = VIRTIO_BLK_Z_NONE;
> }
> + if (s->conf.x_apple_type) {
> + /* Apple abuses the same location for its type id */
> + blkcfg.max_secure_erase_sectors = s->conf.x_apple_type;
> + }
> memcpy(config, &blkcfg, s->config_size);
> }
>
> @@ -1625,6 +1643,10 @@ static void virtio_blk_device_realize(DeviceState
> *dev, Error **errp)
>
> s->config_size = virtio_get_config_size(&virtio_blk_cfg_size_params,
> s->host_features);
> + if (s->conf.x_apple_type) {
> + /* Apple Virtio puts the blk type at 0x3c, make sure we have space.
> */
> + s->config_size = MAX(s->config_size, 0x3d);
> + }
> virtio_init(vdev, VIRTIO_ID_BLOCK, s->config_size);
>
> s->blk = conf->conf.blk;
> @@ -1734,6 +1756,7 @@ static Property virtio_blk_properties[] = {
> conf.max_write_zeroes_sectors,
> BDRV_REQUEST_MAX_SECTORS),
> DEFINE_PROP_BOOL("x-enable-wce-if-config-wce", VirtIOBlock,
> conf.x_enable_wce_if_config_wce, true),
> + DEFINE_PROP_UINT32("x-apple-type", VirtIOBlock, conf.x_apple_type, 0),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/hw/virtio/virtio-blk-pci.c b/hw/virtio/virtio-blk-pci.c
> index 9743bee965..5fbf98f750 100644
> --- a/hw/virtio/virtio-blk-pci.c
> +++ b/hw/virtio/virtio-blk-pci.c
> @@ -62,6 +62,13 @@ static void virtio_blk_pci_realize(VirtIOPCIProxy
> *vpci_dev, Error **errp)
> }
>
> qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> +
> + if (conf->x_apple_type) {
> + /* Apple virtio-blk uses a different vendor/device id */
> + 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 virtio_blk_pci_class_init(ObjectClass *klass, void *data)
> 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..7117ce754c 100644
> --- a/include/hw/virtio/virtio-blk.h
> +++ b/include/hw/virtio/virtio-blk.h
> @@ -46,6 +46,7 @@ struct VirtIOBlkConf
> uint32_t max_discard_sectors;
> uint32_t max_write_zeroes_sectors;
> bool x_enable_wce_if_config_wce;
> + uint32_t x_apple_type;
> };
>
> struct VirtIOBlockDataPlane;
> diff --git a/include/standard-headers/linux/virtio_blk.h
> b/include/standard-headers/linux/virtio_blk.h
> index 7155b1a470..bbea5d50b9 100644
> --- a/include/standard-headers/linux/virtio_blk.h
> +++ b/include/standard-headers/linux/virtio_blk.h
> @@ -204,6 +204,9 @@ struct virtio_blk_config {
> /* Reset All zones command */
> #define VIRTIO_BLK_T_ZONE_RESET_ALL 26
>
> +/* Write zeroes command */
> +#define VIRTIO_BLK_T_APPLE1 0x10000
> +
> #ifndef VIRTIO_BLK_NO_LEGACY
> /* Barrier before this op. */
> #define VIRTIO_BLK_T_BARRIER 0x80000000
> --
> 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
>
>
>
>
With 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 :|
