All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Mikhail Krasheninnikov <krashmisha@gmail.com>
Cc: qemu-devel@nongnu.org,
	Matwey Kornilov <matwey.kornilov@gmail.com>,
	qemu-block@nongnu.org
Subject: Re: [PATCH] virtio: Implement Virtio Backend for SD/MMC in QEMU
Date: Mon, 1 Jul 2024 16:53:41 -0400	[thread overview]
Message-ID: <20240701165239-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20240630134348.16830-1-krashmisha@gmail.com>

On Sun, Jun 30, 2024 at 01:43:48PM +0000, Mikhail Krasheninnikov wrote:
> Add a Virtio backend for SD/MMC devices. Confirmed interoperability with
> Linux.
> 
> Signed-off-by: Mikhail Krasheninnikov <krashmisha@gmail.com>
> CC: Matwey Kornilov <matwey.kornilov@gmail.com>
> CC: qemu-block@nongnu.org
> CC: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/virtio/Kconfig                           |   5 +
>  hw/virtio/meson.build                       |   2 +
>  hw/virtio/virtio-mmc-pci.c                  |  85 ++++++++++
>  hw/virtio/virtio-mmc.c                      | 165 ++++++++++++++++++++
>  hw/virtio/virtio.c                          |   3 +-
>  include/hw/virtio/virtio-mmc.h              |  20 +++
>  include/standard-headers/linux/virtio_ids.h |   1 +
>  7 files changed, 280 insertions(+), 1 deletion(-)
>  create mode 100644 hw/virtio/virtio-mmc-pci.c
>  create mode 100644 hw/virtio/virtio-mmc.c
>  create mode 100644 include/hw/virtio/virtio-mmc.h

I don't think we want these in virtio core directory.

Where does it belong? hw/block?

And CC people that you expect to maintain this, please.

Thanks!




> diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
> index 92c9cf6c96..e5fa7607c4 100644
> --- a/hw/virtio/Kconfig
> +++ b/hw/virtio/Kconfig
> @@ -105,3 +105,8 @@ config VHOST_USER_SCMI
>      bool
>      default y
>      depends on VIRTIO && VHOST_USER
> +
> +config VIRTIO_MMC
> +    bool
> +    default y
> +    depends on VIRTIO
> \ No newline at end of file
> diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
> index 47baf00366..1ff095c5bc 100644
> --- a/hw/virtio/meson.build
> +++ b/hw/virtio/meson.build
> @@ -41,6 +41,7 @@ specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: files('vhost-use
>  specific_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_GPIO'], if_true: files('vhost-user-gpio-pci.c'))
>  specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_SCMI', if_true: files('vhost-user-scmi.c'))
>  specific_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_SCMI'], if_true: files('vhost-user-scmi-pci.c'))
> +specific_virtio_ss.add(when: 'CONFIG_VIRTIO_MMC', if_true: files('virtio-mmc.c'))
>  
>  virtio_pci_ss = ss.source_set()
>  virtio_pci_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock-pci.c'))
> @@ -68,6 +69,7 @@ virtio_pci_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu-pci.
>  virtio_pci_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem-pci.c'))
>  virtio_pci_ss.add(when: 'CONFIG_VHOST_VDPA_DEV', if_true: files('vdpa-dev-pci.c'))
>  virtio_pci_ss.add(when: 'CONFIG_VIRTIO_MD', if_true: files('virtio-md-pci.c'))
> +virtio_pci_ss.add(when: 'CONFIG_VIRTIO_MMC', if_true: files('virtio-mmc-pci.c'))
>  
>  specific_virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
>  
> diff --git a/hw/virtio/virtio-mmc-pci.c b/hw/virtio/virtio-mmc-pci.c
> new file mode 100644
> index 0000000000..f0ed17d03b
> --- /dev/null
> +++ b/hw/virtio/virtio-mmc-pci.c
> @@ -0,0 +1,85 @@
> +#include "qemu/osdep.h"
> +
> +#include "hw/virtio/virtio-pci.h"
> +#include "hw/virtio/virtio-mmc.h"
> +#include "hw/qdev-properties-system.h"
> +#include "qemu/typedefs.h"
> +#include "qapi/error.h"
> +#include "sysemu/block-backend-global-state.h"
> +
> +typedef struct VirtIOMMCPCI VirtIOMMCPCI;
> +
> +/*
> + * virtio-mmc-pci: This extends VirtioPCIProxy.
> + */
> +#define TYPE_VIRTIO_MMC_PCI "virtio-mmc-pci-base"
> +DECLARE_INSTANCE_CHECKER(VirtIOMMCPCI, VIRTIO_MMC_PCI,
> +                         TYPE_VIRTIO_MMC_PCI)
> +
> +struct VirtIOMMCPCI {
> +    VirtIOPCIProxy parent_obj;
> +    VirtIOMMC vdev;
> +    BlockBackend *blk;
> +};
> +
> +static void virtio_mmc_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> +{
> +    VirtIOMMCPCI *vmmc = VIRTIO_MMC_PCI(vpci_dev);
> +    DeviceState *dev = DEVICE(&vmmc->vdev);
> +
> +    if (!vmmc->blk) {
> +        error_setg(errp, "Drive property not set");
> +        return;
> +    }
> +    VirtIOMMC *vmmc_dev = &vmmc->vdev;
> +    vmmc_dev->blk = vmmc->blk;
> +    blk_detach_dev(vmmc->blk, DEVICE(vpci_dev));
> +
> +    qdev_set_parent_bus(dev, BUS(&vpci_dev->bus), errp);
> +
> +    virtio_pci_force_virtio_1(vpci_dev);
> +    object_property_set_bool(OBJECT(dev), "realized", true, errp);
> +}
> +
> +static Property virtio_mmc_properties[] = {
> +    DEFINE_PROP_DRIVE("drive", VirtIOMMCPCI, blk),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void virtio_mmc_pci_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    VirtioPCIClass *virtio_pci_class = VIRTIO_PCI_CLASS(oc);
> +    PCIDeviceClass *pci_device_class = PCI_DEVICE_CLASS(oc);
> +
> +    device_class_set_props(dc, virtio_mmc_properties);
> +    set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> +
> +    virtio_pci_class->realize = virtio_mmc_pci_realize;
> +
> +    pci_device_class->revision = VIRTIO_PCI_ABI_VERSION;
> +    pci_device_class->class_id = PCI_CLASS_MEMORY_FLASH;
> +}
> +
> +static void virtio_mmc_pci_instance_init(Object *obj)
> +{
> +    VirtIOMMCPCI *dev = VIRTIO_MMC_PCI(obj);
> +
> +    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> +                                TYPE_VIRTIO_MMC);
> +}
> +
> +static const VirtioPCIDeviceTypeInfo virtio_mmc_pci_info = {
> +    .base_name     = TYPE_VIRTIO_MMC_PCI,
> +    .generic_name  = "virtio-mmc-pci",
> +    .instance_size = sizeof(VirtIOMMCPCI),
> +    .class_init    = virtio_mmc_pci_class_init,
> +    .instance_init = virtio_mmc_pci_instance_init,
> +};
> +
> +static void virtio_mmc_pci_register(void)
> +{
> +    virtio_pci_types_register(&virtio_mmc_pci_info);
> +}
> +
> +type_init(virtio_mmc_pci_register)
> diff --git a/hw/virtio/virtio-mmc.c b/hw/virtio/virtio-mmc.c
> new file mode 100644
> index 0000000000..50bd7113c5
> --- /dev/null
> +++ b/hw/virtio/virtio-mmc.c
> @@ -0,0 +1,165 @@
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +
> +#include "hw/virtio/virtio.h"
> +#include "hw/virtio/virtio-mmc.h"
> +#include "qemu/typedefs.h"
> +#include "sysemu/blockdev.h"
> +
> +typedef struct mmc_req {
> +    uint32_t opcode;
> +    uint32_t arg;
> +} mmc_req;
> +
> +typedef struct virtio_mmc_req {
> +    uint8_t flags;
> +
> +#define VIRTIO_MMC_REQUEST_DATA BIT(1)
> +#define VIRTIO_MMC_REQUEST_WRITE BIT(2)
> +#define VIRTIO_MMC_REQUEST_STOP BIT(3)
> +#define VIRTIO_MMC_REQUEST_SBC BIT(4)
> +
> +    mmc_req request;
> +
> +    uint8_t buf[4096];
> +    size_t buf_len;
> +
> +    mmc_req stop_req;
> +    mmc_req sbc_req;
> +} virtio_mmc_req;
> +
> +typedef struct virtio_mmc_resp {
> +    uint32_t response[4];
> +    int resp_len;
> +    uint8_t buf[4096];
> +} virtio_mmc_resp;
> +
> +static void send_command(SDBus *sdbus, mmc_req *mmc_request, uint8_t *response,
> +                         virtio_mmc_resp *virtio_resp)
> +{
> +    SDRequest sdreq;
> +    sdreq.cmd = (uint8_t)mmc_request->opcode;
> +    sdreq.arg = mmc_request->arg;
> +    int resp_len = sdbus_do_command(sdbus, &sdreq, response);
> +    virtio_resp->resp_len = resp_len;
> +
> +    for (int i = 0; i < resp_len / sizeof(uint32_t); i++) {
> +        virtio_resp->response[i] = ldl_be_p(&virtio_resp->response[i]);
> +    }
> +}
> +
> +static void send_command_without_response(SDBus *sdbus, mmc_req *mmc_request)
> +{
> +    SDRequest sdreq;
> +    sdreq.cmd = (uint8_t)mmc_request->opcode;
> +    sdreq.arg = mmc_request->arg;
> +    uint8_t response[4];
> +    sdbus_do_command(sdbus, &sdreq, response);
> +}
> +
> +static void handle_mmc_request(VirtIODevice *vdev, virtio_mmc_req *virtio_req,
> +                               virtio_mmc_resp *virtio_resp)
> +{
> +    VirtIOMMC *vmmc = VIRTIO_MMC(vdev);
> +    SDBus *sdbus = &vmmc->sdbus;
> +
> +    if (virtio_req->flags & VIRTIO_MMC_REQUEST_SBC) {
> +        send_command_without_response(sdbus, &virtio_req->sbc_req);
> +    }
> +
> +    send_command(sdbus, &virtio_req->request,
> +    (uint8_t *)virtio_resp->response, virtio_resp);
> +
> +    if (virtio_req->flags & VIRTIO_MMC_REQUEST_DATA) {
> +        if (virtio_req->flags & VIRTIO_MMC_REQUEST_WRITE) {
> +            sdbus_write_data(sdbus, virtio_req->buf, virtio_req->buf_len);
> +        } else {
> +            sdbus_read_data(sdbus, virtio_resp->buf, virtio_req->buf_len);
> +        }
> +    }
> +
> +    if (virtio_req->flags & VIRTIO_MMC_REQUEST_STOP) {
> +        send_command_without_response(sdbus, &virtio_req->stop_req);
> +    }
> +}
> +
> +static void handle_request(VirtIODevice *vdev, VirtQueue *vq)
> +{
> +    VirtQueueElement *elem;
> +    virtio_mmc_req virtio_req;
> +    virtio_mmc_resp virtio_resp;
> +
> +    elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> +
> +    iov_to_buf(elem->out_sg, elem->out_num, 0,
> +    &virtio_req, sizeof(virtio_mmc_req));
> +
> +    handle_mmc_request(vdev, &virtio_req, &virtio_resp);
> +
> +    iov_from_buf(elem->in_sg, elem->in_num, 0,
> +    &virtio_resp, sizeof(virtio_mmc_resp));
> +
> +    virtqueue_push(vq, elem, 1);
> +
> +    virtio_notify(vdev, vq);
> +}
> +
> +static void virtio_mmc_realize(DeviceState *dev, Error **errp)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VirtIOMMC *vmmc = VIRTIO_MMC(dev);
> +
> +    virtio_init(vdev, VIRTIO_ID_MMC, 0);
> +
> +    vmmc->vq = virtio_add_queue(vdev, 1, handle_request);
> +
> +    BlockBackend *blk = vmmc->blk;
> +    if (!blk) {
> +        error_setg(errp, "Block backend not found");
> +        return;
> +    }
> +
> +    qbus_init(&vmmc->sdbus, sizeof(vmmc->sdbus), TYPE_SD_BUS, dev, "sd-bus");
> +    DeviceState *card = qdev_new(TYPE_SD_CARD);
> +    qdev_prop_set_drive_err(card, "drive", blk, &error_fatal);
> +    qdev_realize_and_unref(card,
> +    qdev_get_child_bus(dev, "sd-bus"), &error_fatal);
> +}
> +
> +static void virtio_mmc_unrealize(DeviceState *dev)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    virtio_cleanup(vdev);
> +}
> +
> +static uint64_t virtio_mmc_get_features(VirtIODevice *vdev,
> +                                        uint64_t features, Error **errp)
> +{
> +    return features;
> +}
> +
> +static void virtio_mmc_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    VirtioDeviceClass *k = VIRTIO_DEVICE_CLASS(klass);
> +
> +    set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> +
> +    k->realize = virtio_mmc_realize;
> +    k->unrealize = virtio_mmc_unrealize;
> +    k->get_features = virtio_mmc_get_features;
> +}
> +
> +static const TypeInfo virtio_mmc_info = {
> +    .name = TYPE_VIRTIO_MMC,
> +    .parent = TYPE_VIRTIO_DEVICE,
> +    .instance_size = sizeof(VirtIOMMC),
> +    .class_init = virtio_mmc_class_init,
> +};
> +
> +static void virtio_register_types(void)
> +{
> +    type_register_static(&virtio_mmc_info);
> +}
> +
> +type_init(virtio_register_types)
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 7549094154..35f00f06aa 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -193,7 +193,8 @@ const char *virtio_device_names[] = {
>      [VIRTIO_ID_PARAM_SERV] = "virtio-param-serv",
>      [VIRTIO_ID_AUDIO_POLICY] = "virtio-audio-pol",
>      [VIRTIO_ID_BT] = "virtio-bluetooth",
> -    [VIRTIO_ID_GPIO] = "virtio-gpio"
> +    [VIRTIO_ID_GPIO] = "virtio-gpio",
> +    [VIRTIO_ID_MMC] = "virtio-mmc",
>  };
>  
>  static const char *virtio_id_to_name(uint16_t device_id)
> diff --git a/include/hw/virtio/virtio-mmc.h b/include/hw/virtio/virtio-mmc.h
> new file mode 100644
> index 0000000000..a68f45d7cb
> --- /dev/null
> +++ b/include/hw/virtio/virtio-mmc.h
> @@ -0,0 +1,20 @@
> +#pragma once
> +
> +#include "hw/virtio/virtio.h"
> +#include "hw/sd/sd.h"
> +#include "qemu/typedefs.h"
> +
> +#define VIRTIO_ID_MMC 42
> +
> +#define TYPE_VIRTIO_MMC "virtio-mmc-device"
> +#define VIRTIO_MMC(obj) \
> +    OBJECT_CHECK(VirtIOMMC, (obj), TYPE_VIRTIO_MMC)
> +#define VIRTIO_MMC_GET_PARENT_CLASS(obj) \
> +    OBJECT_GET_PARENT_CLASS(VIRTIO_MMC(obj), TYPE_VIRTIO_MMC)
> +
> +typedef struct VirtIOMMC {
> +    VirtIODevice parent_obj;
> +    VirtQueue *vq;
> +    SDBus sdbus;
> +    BlockBackend *blk;
> +} VirtIOMMC;
> diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h
> index 7aa2eb7662..0c67fbf709 100644
> --- a/include/standard-headers/linux/virtio_ids.h
> +++ b/include/standard-headers/linux/virtio_ids.h
> @@ -68,6 +68,7 @@
>  #define VIRTIO_ID_AUDIO_POLICY		39 /* virtio audio policy */
>  #define VIRTIO_ID_BT			40 /* virtio bluetooth */
>  #define VIRTIO_ID_GPIO			41 /* virtio gpio */
> +#define VIRTIO_ID_MMC			42 /* virtio mmc */
>  
>  /*
>   * Virtio Transitional IDs
> -- 
> 2.34.1



      reply	other threads:[~2024-07-01 20:54 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-30 13:43 [PATCH] virtio: Implement Virtio Backend for SD/MMC in QEMU Mikhail Krasheninnikov
2024-07-01 20:53 ` Michael S. Tsirkin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240701165239-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=krashmisha@gmail.com \
    --cc=matwey.kornilov@gmail.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.