[v3,05/10] vdpa-dev: implement the realize interface

Commit Message

Zhijian Li (Fujitsu)" via March 19, 2022, 7:20 a.m. UTC

From: Longpeng <[e-mail]>

Implements the .realize interface.

Signed-off-by: Longpeng <[e-mail]>
---
 hw/virtio/vdpa-dev-pci.c     |  18 ++++-
 hw/virtio/vdpa-dev.c         | 132 +++++++++++++++++++++++++++++++++++
 include/hw/virtio/vdpa-dev.h |  10 +++
 3 files changed, 159 insertions(+), 1 deletion(-)

Comments

Stefano Garzarella March 28, 2022, 2:59 p.m. UTC | #1

On Sat, Mar 19, 2022 at 03:20:07PM +0800, Longpeng(Mike) wrote:
>From: Longpeng <[e-mail]>
>
>Implements the .realize interface.
>
>Signed-off-by: Longpeng <[e-mail]>
>---
> hw/virtio/vdpa-dev-pci.c     |  18 ++++-
> hw/virtio/vdpa-dev.c         | 132 +++++++++++++++++++++++++++++++++++
> include/hw/virtio/vdpa-dev.h |  10 +++
> 3 files changed, 159 insertions(+), 1 deletion(-)
>
>diff --git a/hw/virtio/vdpa-dev-pci.c b/hw/virtio/vdpa-dev-pci.c
>index 9eb590ce8c..31bd17353a 100644
>--- a/hw/virtio/vdpa-dev-pci.c
>+++ b/hw/virtio/vdpa-dev-pci.c
>@@ -51,10 +51,26 @@ static Property vhost_vdpa_device_pci_properties[] = {
>     DEFINE_PROP_END_OF_LIST(),
> };
>
>+static int vhost_vdpa_device_pci_post_init(VhostVdpaDevice *v, Error **errp)
>+{
>+    VhostVdpaDevicePCI *dev = container_of(v, VhostVdpaDevicePCI, vdev);
>+    VirtIOPCIProxy *vpci_dev = &dev->parent_obj;
>+
>+    vpci_dev->class_code = virtio_pci_get_class_id(v->vdev_id);
>+    vpci_dev->trans_devid = virtio_pci_get_trans_devid(v->vdev_id);
>+    /* one for config vector */
>+    vpci_dev->nvectors = v->num_queues + 1;
>+
>+    return 0;
>+}
>+
> static void
> vhost_vdpa_device_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> {
>-    return;
>+    VhostVdpaDevicePCI *dev = VHOST_VDPA_DEVICE_PCI(vpci_dev);
>+
>+    dev->vdev.post_init = vhost_vdpa_device_pci_post_init;
>+    qdev_realize(DEVICE(&dev->vdev), BUS(&vpci_dev->bus), errp);
> }
>
> static void vhost_vdpa_device_pci_class_init(ObjectClass *klass, void *data)
>diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
>index 993cbc7d11..4defe6c33d 100644
>--- a/hw/virtio/vdpa-dev.c
>+++ b/hw/virtio/vdpa-dev.c
>@@ -29,9 +29,140 @@
> #include "sysemu/sysemu.h"
> #include "sysemu/runstate.h"
>
>+static void
>+vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq)
>+{
>+    /* Nothing to do */
>+}
>+
>+static uint32_t
>+vhost_vdpa_device_get_u32(int fd, unsigned long int cmd, Error **errp)
>+{
>+    uint32_t val = (uint32_t)-1;
>+
>+    if (ioctl(fd, cmd, &val) < 0) {
>+        error_setg(errp, "vhost-vdpa-device: cmd 0x%lx failed: %s",
>+                   cmd, strerror(errno));
>+    }
>+
>+    return val;
>+}
>+
> static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp)
> {
>+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>+    VhostVdpaDevice *v = VHOST_VDPA_DEVICE(vdev);
>+    uint32_t max_queue_size;
>+    struct vhost_virtqueue *vqs;
>+    int i, ret;
>+
>+    if (!v->vdpa_dev || v->vdpa_dev_fd == -1) {
                        ^
Should we use && operator here?

I can't start QEMU, how did you test this series?

$ ./qemu-system-x86_64 -m 512M -smp 2 -M q35,accel=kvm \
   ...
   -device vhost-vdpa-device-pci,vdpa-dev=/dev/vhost-vdpa-0
qemu-system-x86_64: -device
vhost-vdpa-device-pci,vdpa-dev=/dev/vhost-vdpa-0: both vpda-dev and
vpda-dev-fd are missing

>+        error_setg(errp, "both vpda-dev and vpda-dev-fd are missing");

Typo s/vpda/vdpa

>+        return;
>+    }
>+
>+    if (v->vdpa_dev && v->vdpa_dev_fd != -1) {
>+        error_setg(errp, "both vpda-dev and vpda-dev-fd are set");

Ditto

>+        return;
>+    }
>+
>+    if (v->vdpa_dev_fd == -1) {
>+        v->vdpa_dev_fd = qemu_open(v->vdpa_dev, O_RDWR, errp);
>+        if (*errp) {
>+            return;
>+        }
>+    }
>+    v->vdpa.device_fd = v->vdpa_dev_fd;
>+
>+    v->vdev_id = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
>+                                           VHOST_VDPA_GET_DEVICE_ID, errp);
>+    if (*errp) {
>+        goto out;
>+    }
>+
>+    max_queue_size = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
>+                                               VHOST_VDPA_GET_VRING_NUM, errp);
>+    if (*errp) {
>+        goto out;
>+    }
>+
>+    if (v->queue_size > max_queue_size) {
>+        error_setg(errp, "vhost-vdpa-device: invalid queue_size: %u (max:%u)",
>+                   v->queue_size, max_queue_size);
>+        goto out;
>+    } else if (!v->queue_size) {
>+        v->queue_size = max_queue_size;
>+    }
>+
>+    v->num_queues = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
>+                                              VHOST_VDPA_GET_VQS_COUNT, errp);
>+    if (*errp) {
>+        goto out;
>+    }
>+
>+    if (!v->num_queues || v->num_queues > VIRTIO_QUEUE_MAX) {
>+        error_setg(errp, "invalid number of virtqueues: %u (max:%u)",
>+                   v->num_queues, VIRTIO_QUEUE_MAX);
>+        goto out;
>+    }
>+
>+    v->dev.nvqs = v->num_queues;
>+    vqs = g_new0(struct vhost_virtqueue, v->dev.nvqs);
>+    v->dev.vqs = vqs;
>+    v->dev.vq_index = 0;
>+    v->dev.vq_index_end = v->dev.nvqs;
>+    v->dev.backend_features = 0;
>+    v->started = false;
>+
>+    ret = vhost_dev_init(&v->dev, &v->vdpa, VHOST_BACKEND_TYPE_VDPA, 0, NULL);
>+    if (ret < 0) {
>+        error_setg(errp, "vhost-vdpa-device: vhost initialization failed: %s",
>+                   strerror(-ret));
>+        goto free_vqs;
>+    }
>+
>+    v->config_size = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
>+                                               VHOST_VDPA_GET_CONFIG_SIZE, errp);
>+    if (*errp) {
>+        goto vhost_cleanup;
>+    }
>+    v->config = g_malloc0(v->config_size);
>+
>+    ret = vhost_dev_get_config(&v->dev, v->config, v->config_size, NULL);
>+    if (ret < 0) {
>+        error_setg(errp, "vhost-vdpa-device: get config failed");
>+        goto free_config;
>+    }
>+
>+    virtio_init(vdev, "vhost-vdpa", v->vdev_id, v->config_size);
>+
>+    v->virtqs = g_new0(VirtQueue *, v->dev.nvqs);
>+    for (i = 0; i < v->dev.nvqs; i++) {
>+        v->virtqs[i] = virtio_add_queue(vdev, v->queue_size,
>+                                        vhost_vdpa_device_dummy_handle_output);
>+    }
>+
>+    if (v->post_init && v->post_init(v, errp) < 0) {
>+        goto free_virtio;
>+    }
>+
>     return;
>+
>+free_virtio:
>+    for (i = 0; i < v->num_queues; i++) {
>+        virtio_delete_queue(v->virtqs[i]);
>+    }
>+    g_free(v->virtqs);
>+    virtio_cleanup(vdev);
>+free_config:
>+    g_free(v->config);
>+vhost_cleanup:
>+    vhost_dev_cleanup(&v->dev);
>+free_vqs:
>+    g_free(vqs);
>+out:
>+    qemu_close(v->vdpa_dev_fd);
>+    v->vdpa_dev_fd = -1;
> }
>
> static void vhost_vdpa_device_unrealize(DeviceState *dev)
>@@ -66,6 +197,7 @@ static void vhost_vdpa_device_set_status(VirtIODevice *vdev, uint8_t status)
> static Property vhost_vdpa_device_properties[] = {
>     DEFINE_PROP_STRING("vdpa-dev", VhostVdpaDevice, vdpa_dev),
>     DEFINE_PROP_INT32("vdpa-dev-fd", VhostVdpaDevice, vdpa_dev_fd, -1),

Other vhost devices use the `vhostfd` property, maybe we should use the
same name.

If we go for this change, then maybe we also need to change `vdpa-dev`
to `vhostpath` or something like that

Thanks,
Stefano

>+    DEFINE_PROP_UINT16("queue-size", VhostVdpaDevice, queue_size, 0),
>     DEFINE_PROP_END_OF_LIST(),
> };
>
>diff --git a/include/hw/virtio/vdpa-dev.h b/include/hw/virtio/vdpa-dev.h
>index 476bda0873..cf11abd0f7 100644
>--- a/include/hw/virtio/vdpa-dev.h
>+++ b/include/hw/virtio/vdpa-dev.h
>@@ -28,6 +28,16 @@ struct VhostVdpaDevice {
>     char *vdpa_dev;
>     int vdpa_dev_fd;
>     int32_t bootindex;
>+    uint32_t vdev_id;
>+    uint32_t num_queues;
>+    struct vhost_dev dev;
>+    struct vhost_vdpa vdpa;
>+    VirtQueue **virtqs;
>+    uint8_t *config;
>+    int config_size;
>+    uint16_t queue_size;
>+    bool started;
>+    int (*post_init)(VhostVdpaDevice *v, Error **errp);
> };
>
> #endif
>--
>2.23.0
>

Zhijian Li (Fujitsu)" via March 29, 2022, 5:01 a.m. UTC | #2

> -----Original Message-----
> From: Stefano Garzarella [mailto:[e-mail]]
> Sent: Monday, March 28, 2022 11:00 PM
> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
> <[e-mail]>
> Cc: Stefan Hajnoczi <[e-mail]>; Michael Tsirkin <[e-mail]>;
> Cornelia Huck <[e-mail]>; Paolo Bonzini <[e-mail]>;
> Gonglei (Arei) <[e-mail]>; Yechuan <[e-mail]>;
> Huangzhichao <[e-mail]>; qemu devel list <[e-mail]>
> Subject: Re: [PATCH v3 05/10] vdpa-dev: implement the realize interface
> 
> On Sat, Mar 19, 2022 at 03:20:07PM +0800, Longpeng(Mike) wrote:
> >From: Longpeng <[e-mail]>
> >
> >Implements the .realize interface.
> >
> >Signed-off-by: Longpeng <[e-mail]>
> >---
> > hw/virtio/vdpa-dev-pci.c     |  18 ++++-
> > hw/virtio/vdpa-dev.c         | 132 +++++++++++++++++++++++++++++++++++
> > include/hw/virtio/vdpa-dev.h |  10 +++
> > 3 files changed, 159 insertions(+), 1 deletion(-)
> >
> >diff --git a/hw/virtio/vdpa-dev-pci.c b/hw/virtio/vdpa-dev-pci.c
> >index 9eb590ce8c..31bd17353a 100644
> >--- a/hw/virtio/vdpa-dev-pci.c
> >+++ b/hw/virtio/vdpa-dev-pci.c
> >@@ -51,10 +51,26 @@ static Property vhost_vdpa_device_pci_properties[] = {
> >     DEFINE_PROP_END_OF_LIST(),
> > };
> >
> >+static int vhost_vdpa_device_pci_post_init(VhostVdpaDevice *v, Error **errp)
> >+{
> >+    VhostVdpaDevicePCI *dev = container_of(v, VhostVdpaDevicePCI, vdev);
> >+    VirtIOPCIProxy *vpci_dev = &dev->parent_obj;
> >+
> >+    vpci_dev->class_code = virtio_pci_get_class_id(v->vdev_id);
> >+    vpci_dev->trans_devid = virtio_pci_get_trans_devid(v->vdev_id);
> >+    /* one for config vector */
> >+    vpci_dev->nvectors = v->num_queues + 1;
> >+
> >+    return 0;
> >+}
> >+
> > static void
> > vhost_vdpa_device_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> > {
> >-    return;
> >+    VhostVdpaDevicePCI *dev = VHOST_VDPA_DEVICE_PCI(vpci_dev);
> >+
> >+    dev->vdev.post_init = vhost_vdpa_device_pci_post_init;
> >+    qdev_realize(DEVICE(&dev->vdev), BUS(&vpci_dev->bus), errp);
> > }
> >
> > static void vhost_vdpa_device_pci_class_init(ObjectClass *klass, void *data)
> >diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
> >index 993cbc7d11..4defe6c33d 100644
> >--- a/hw/virtio/vdpa-dev.c
> >+++ b/hw/virtio/vdpa-dev.c
> >@@ -29,9 +29,140 @@
> > #include "sysemu/sysemu.h"
> > #include "sysemu/runstate.h"
> >
> >+static void
> >+vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq)
> >+{
> >+    /* Nothing to do */
> >+}
> >+
> >+static uint32_t
> >+vhost_vdpa_device_get_u32(int fd, unsigned long int cmd, Error **errp)
> >+{
> >+    uint32_t val = (uint32_t)-1;
> >+
> >+    if (ioctl(fd, cmd, &val) < 0) {
> >+        error_setg(errp, "vhost-vdpa-device: cmd 0x%lx failed: %s",
> >+                   cmd, strerror(errno));
> >+    }
> >+
> >+    return val;
> >+}
> >+
> > static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp)
> > {
> >+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> >+    VhostVdpaDevice *v = VHOST_VDPA_DEVICE(vdev);
> >+    uint32_t max_queue_size;
> >+    struct vhost_virtqueue *vqs;
> >+    int i, ret;
> >+
> >+    if (!v->vdpa_dev || v->vdpa_dev_fd == -1) {
>                         ^
> Should we use && operator here?
> 
> I can't start QEMU, how did you test this series?
> 

Oh! I changed to && operator in my local environment when I test this series 
but forgot to include it.

I'll fix it in the next version.

> $ ./qemu-system-x86_64 -m 512M -smp 2 -M q35,accel=kvm \
>    ...
>    -device vhost-vdpa-device-pci,vdpa-dev=/dev/vhost-vdpa-0
> qemu-system-x86_64: -device
> vhost-vdpa-device-pci,vdpa-dev=/dev/vhost-vdpa-0: both vpda-dev and
> vpda-dev-fd are missing
> 
> >+        error_setg(errp, "both vpda-dev and vpda-dev-fd are missing");
> 
> Typo s/vpda/vdpa
> 

OK.

> >+        return;
> >+    }
> >+
> >+    if (v->vdpa_dev && v->vdpa_dev_fd != -1) {
> >+        error_setg(errp, "both vpda-dev and vpda-dev-fd are set");
> 
> Ditto
> 

OK.

> >+        return;
> >+    }
> >+
> >+    if (v->vdpa_dev_fd == -1) {
> >+        v->vdpa_dev_fd = qemu_open(v->vdpa_dev, O_RDWR, errp);
> >+        if (*errp) {
> >+            return;
> >+        }
> >+    }
> >+    v->vdpa.device_fd = v->vdpa_dev_fd;
> >+
> >+    v->vdev_id = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
> >+                                           VHOST_VDPA_GET_DEVICE_ID, errp);
> >+    if (*errp) {
> >+        goto out;
> >+    }
> >+
> >+    max_queue_size = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
> >+                                               VHOST_VDPA_GET_VRING_NUM, errp);
> >+    if (*errp) {
> >+        goto out;
> >+    }
> >+
> >+    if (v->queue_size > max_queue_size) {
> >+        error_setg(errp, "vhost-vdpa-device: invalid queue_size: %u
> (max:%u)",
> >+                   v->queue_size, max_queue_size);
> >+        goto out;
> >+    } else if (!v->queue_size) {
> >+        v->queue_size = max_queue_size;
> >+    }
> >+
> >+    v->num_queues = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
> >+                                              VHOST_VDPA_GET_VQS_COUNT, errp);
> >+    if (*errp) {
> >+        goto out;
> >+    }
> >+
> >+    if (!v->num_queues || v->num_queues > VIRTIO_QUEUE_MAX) {
> >+        error_setg(errp, "invalid number of virtqueues: %u (max:%u)",
> >+                   v->num_queues, VIRTIO_QUEUE_MAX);
> >+        goto out;
> >+    }
> >+
> >+    v->dev.nvqs = v->num_queues;
> >+    vqs = g_new0(struct vhost_virtqueue, v->dev.nvqs);
> >+    v->dev.vqs = vqs;
> >+    v->dev.vq_index = 0;
> >+    v->dev.vq_index_end = v->dev.nvqs;
> >+    v->dev.backend_features = 0;
> >+    v->started = false;
> >+
> >+    ret = vhost_dev_init(&v->dev, &v->vdpa, VHOST_BACKEND_TYPE_VDPA, 0,
> NULL);
> >+    if (ret < 0) {
> >+        error_setg(errp, "vhost-vdpa-device: vhost initialization
> failed: %s",
> >+                   strerror(-ret));
> >+        goto free_vqs;
> >+    }
> >+
> >+    v->config_size = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
> >+                                               VHOST_VDPA_GET_CONFIG_SIZE,
> errp);
> >+    if (*errp) {
> >+        goto vhost_cleanup;
> >+    }
> >+    v->config = g_malloc0(v->config_size);
> >+
> >+    ret = vhost_dev_get_config(&v->dev, v->config, v->config_size, NULL);
> >+    if (ret < 0) {
> >+        error_setg(errp, "vhost-vdpa-device: get config failed");
> >+        goto free_config;
> >+    }
> >+
> >+    virtio_init(vdev, "vhost-vdpa", v->vdev_id, v->config_size);
> >+
> >+    v->virtqs = g_new0(VirtQueue *, v->dev.nvqs);
> >+    for (i = 0; i < v->dev.nvqs; i++) {
> >+        v->virtqs[i] = virtio_add_queue(vdev, v->queue_size,
> >+
> vhost_vdpa_device_dummy_handle_output);
> >+    }
> >+
> >+    if (v->post_init && v->post_init(v, errp) < 0) {
> >+        goto free_virtio;
> >+    }
> >+
> >     return;
> >+
> >+free_virtio:
> >+    for (i = 0; i < v->num_queues; i++) {
> >+        virtio_delete_queue(v->virtqs[i]);
> >+    }
> >+    g_free(v->virtqs);
> >+    virtio_cleanup(vdev);
> >+free_config:
> >+    g_free(v->config);
> >+vhost_cleanup:
> >+    vhost_dev_cleanup(&v->dev);
> >+free_vqs:
> >+    g_free(vqs);
> >+out:
> >+    qemu_close(v->vdpa_dev_fd);
> >+    v->vdpa_dev_fd = -1;
> > }
> >
> > static void vhost_vdpa_device_unrealize(DeviceState *dev)
> >@@ -66,6 +197,7 @@ static void vhost_vdpa_device_set_status(VirtIODevice *vdev,
> uint8_t status)
> > static Property vhost_vdpa_device_properties[] = {
> >     DEFINE_PROP_STRING("vdpa-dev", VhostVdpaDevice, vdpa_dev),
> >     DEFINE_PROP_INT32("vdpa-dev-fd", VhostVdpaDevice, vdpa_dev_fd, -1),
> 
> Other vhost devices use the `vhostfd` property, maybe we should use the
> same name.
> 
> If we go for this change, then maybe we also need to change `vdpa-dev`
> to `vhostpath` or something like that
> 
> Thanks,
> Stefano
> 
> >+    DEFINE_PROP_UINT16("queue-size", VhostVdpaDevice, queue_size, 0),
> >     DEFINE_PROP_END_OF_LIST(),
> > };
> >
> >diff --git a/include/hw/virtio/vdpa-dev.h b/include/hw/virtio/vdpa-dev.h
> >index 476bda0873..cf11abd0f7 100644
> >--- a/include/hw/virtio/vdpa-dev.h
> >+++ b/include/hw/virtio/vdpa-dev.h
> >@@ -28,6 +28,16 @@ struct VhostVdpaDevice {
> >     char *vdpa_dev;
> >     int vdpa_dev_fd;
> >     int32_t bootindex;
> >+    uint32_t vdev_id;
> >+    uint32_t num_queues;
> >+    struct vhost_dev dev;
> >+    struct vhost_vdpa vdpa;
> >+    VirtQueue **virtqs;
> >+    uint8_t *config;
> >+    int config_size;
> >+    uint16_t queue_size;
> >+    bool started;
> >+    int (*post_init)(VhostVdpaDevice *v, Error **errp);
> > };
> >
> > #endif
> >--
> >2.23.0
> >

Zhijian Li (Fujitsu)" via March 29, 2022, 5:15 a.m. UTC | #3

> -----Original Message-----
> From: Stefano Garzarella [mailto:[e-mail]]
> Sent: Monday, March 28, 2022 11:00 PM
> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
> <[e-mail]>
> Cc: Stefan Hajnoczi <[e-mail]>; Michael Tsirkin <[e-mail]>;
> Cornelia Huck <[e-mail]>; Paolo Bonzini <[e-mail]>;
> Gonglei (Arei) <[e-mail]>; Yechuan <[e-mail]>;
> Huangzhichao <[e-mail]>; qemu devel list <[e-mail]>
> Subject: Re: [PATCH v3 05/10] vdpa-dev: implement the realize interface
> 
> On Sat, Mar 19, 2022 at 03:20:07PM +0800, Longpeng(Mike) wrote:
> >From: Longpeng <[e-mail]>
> >

[snap]

> > static void vhost_vdpa_device_unrealize(DeviceState *dev)
> >@@ -66,6 +197,7 @@ static void vhost_vdpa_device_set_status(VirtIODevice *vdev,
> uint8_t status)
> > static Property vhost_vdpa_device_properties[] = {
> >     DEFINE_PROP_STRING("vdpa-dev", VhostVdpaDevice, vdpa_dev),
> >     DEFINE_PROP_INT32("vdpa-dev-fd", VhostVdpaDevice, vdpa_dev_fd, -1),
> 
> Other vhost devices use the `vhostfd` property, maybe we should use the
> same name.
> 
> If we go for this change, then maybe we also need to change `vdpa-dev`
> to `vhostpath` or something like that
> 

Make sense.

I prefer to use 'vhostdev' for the file path, just like the usage in Netdev:
  -netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-0,id=vhostvdpa1

> Thanks,
> Stefano
> 
> >+    DEFINE_PROP_UINT16("queue-size", VhostVdpaDevice, queue_size, 0),
> >     DEFINE_PROP_END_OF_LIST(),
> > };
> >
> >diff --git a/include/hw/virtio/vdpa-dev.h b/include/hw/virtio/vdpa-dev.h
> >index 476bda0873..cf11abd0f7 100644
> >--- a/include/hw/virtio/vdpa-dev.h
> >+++ b/include/hw/virtio/vdpa-dev.h
> >@@ -28,6 +28,16 @@ struct VhostVdpaDevice {
> >     char *vdpa_dev;
> >     int vdpa_dev_fd;
> >     int32_t bootindex;
> >+    uint32_t vdev_id;
> >+    uint32_t num_queues;
> >+    struct vhost_dev dev;
> >+    struct vhost_vdpa vdpa;
> >+    VirtQueue **virtqs;
> >+    uint8_t *config;
> >+    int config_size;
> >+    uint16_t queue_size;
> >+    bool started;
> >+    int (*post_init)(VhostVdpaDevice *v, Error **errp);
> > };
> >
> > #endif
> >--
> >2.23.0
> >

Stefano Garzarella March 29, 2022, 7:05 a.m. UTC | #4

On Tue, Mar 29, 2022 at 7:16 AM Longpeng (Mike, Cloud Infrastructure
Service Product Dept.) <[e-mail]> wrote:
> > On Sat, Mar 19, 2022 at 03:20:07PM +0800, Longpeng(Mike) wrote:
> > >From: Longpeng <[e-mail]>
> > >
>
> [snap]
>
> > > static void vhost_vdpa_device_unrealize(DeviceState *dev)
> > >@@ -66,6 +197,7 @@ static void vhost_vdpa_device_set_status(VirtIODevice *vdev,
> > uint8_t status)
> > > static Property vhost_vdpa_device_properties[] = {
> > >     DEFINE_PROP_STRING("vdpa-dev", VhostVdpaDevice, vdpa_dev),
> > >     DEFINE_PROP_INT32("vdpa-dev-fd", VhostVdpaDevice, vdpa_dev_fd, -1),
> >
> > Other vhost devices use the `vhostfd` property, maybe we should use the
> > same name.
> >
> > If we go for this change, then maybe we also need to change `vdpa-dev`
> > to `vhostpath` or something like that
> >
>
> Make sense.
>
> I prefer to use 'vhostdev' for the file path, just like the usage in Netdev:
>   -netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-0,id=vhostvdpa1
>

Yep, I agree with 'vhostdev'.

Thanks,
Stefano

Patch

diff --git a/hw/virtio/vdpa-dev-pci.c b/hw/virtio/vdpa-dev-pci.c
index 9eb590ce8c..31bd17353a 100644
--- a/hw/virtio/vdpa-dev-pci.c
+++ b/hw/virtio/vdpa-dev-pci.c
@@ -51,10 +51,26 @@  static Property vhost_vdpa_device_pci_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static int vhost_vdpa_device_pci_post_init(VhostVdpaDevice *v, Error **errp)
+{
+    VhostVdpaDevicePCI *dev = container_of(v, VhostVdpaDevicePCI, vdev);
+    VirtIOPCIProxy *vpci_dev = &dev->parent_obj;
+
+    vpci_dev->class_code = virtio_pci_get_class_id(v->vdev_id);
+    vpci_dev->trans_devid = virtio_pci_get_trans_devid(v->vdev_id);
+    /* one for config vector */
+    vpci_dev->nvectors = v->num_queues + 1;
+
+    return 0;
+}
+
 static void
 vhost_vdpa_device_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
 {
-    return;
+    VhostVdpaDevicePCI *dev = VHOST_VDPA_DEVICE_PCI(vpci_dev);
+
+    dev->vdev.post_init = vhost_vdpa_device_pci_post_init;
+    qdev_realize(DEVICE(&dev->vdev), BUS(&vpci_dev->bus), errp);
 }
 
 static void vhost_vdpa_device_pci_class_init(ObjectClass *klass, void *data)
diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
index 993cbc7d11..4defe6c33d 100644
--- a/hw/virtio/vdpa-dev.c
+++ b/hw/virtio/vdpa-dev.c
@@ -29,9 +29,140 @@ 
 #include "sysemu/sysemu.h"
 #include "sysemu/runstate.h"
 
+static void
+vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq)
+{
+    /* Nothing to do */
+}
+
+static uint32_t
+vhost_vdpa_device_get_u32(int fd, unsigned long int cmd, Error **errp)
+{
+    uint32_t val = (uint32_t)-1;
+
+    if (ioctl(fd, cmd, &val) < 0) {
+        error_setg(errp, "vhost-vdpa-device: cmd 0x%lx failed: %s",
+                   cmd, strerror(errno));
+    }
+
+    return val;
+}
+
 static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp)
 {
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VhostVdpaDevice *v = VHOST_VDPA_DEVICE(vdev);
+    uint32_t max_queue_size;
+    struct vhost_virtqueue *vqs;
+    int i, ret;
+
+    if (!v->vdpa_dev || v->vdpa_dev_fd == -1) {
+        error_setg(errp, "both vpda-dev and vpda-dev-fd are missing");
+        return;
+    }
+
+    if (v->vdpa_dev && v->vdpa_dev_fd != -1) {
+        error_setg(errp, "both vpda-dev and vpda-dev-fd are set");
+        return;
+    }
+
+    if (v->vdpa_dev_fd == -1) {
+        v->vdpa_dev_fd = qemu_open(v->vdpa_dev, O_RDWR, errp);
+        if (*errp) {
+            return;
+        }
+    }
+    v->vdpa.device_fd = v->vdpa_dev_fd;
+
+    v->vdev_id = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
+                                           VHOST_VDPA_GET_DEVICE_ID, errp);
+    if (*errp) {
+        goto out;
+    }
+
+    max_queue_size = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
+                                               VHOST_VDPA_GET_VRING_NUM, errp);
+    if (*errp) {
+        goto out;
+    }
+
+    if (v->queue_size > max_queue_size) {
+        error_setg(errp, "vhost-vdpa-device: invalid queue_size: %u (max:%u)",
+                   v->queue_size, max_queue_size);
+        goto out;
+    } else if (!v->queue_size) {
+        v->queue_size = max_queue_size;
+    }
+
+    v->num_queues = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
+                                              VHOST_VDPA_GET_VQS_COUNT, errp);
+    if (*errp) {
+        goto out;
+    }
+
+    if (!v->num_queues || v->num_queues > VIRTIO_QUEUE_MAX) {
+        error_setg(errp, "invalid number of virtqueues: %u (max:%u)",
+                   v->num_queues, VIRTIO_QUEUE_MAX);
+        goto out;
+    }
+
+    v->dev.nvqs = v->num_queues;
+    vqs = g_new0(struct vhost_virtqueue, v->dev.nvqs);
+    v->dev.vqs = vqs;
+    v->dev.vq_index = 0;
+    v->dev.vq_index_end = v->dev.nvqs;
+    v->dev.backend_features = 0;
+    v->started = false;
+
+    ret = vhost_dev_init(&v->dev, &v->vdpa, VHOST_BACKEND_TYPE_VDPA, 0, NULL);
+    if (ret < 0) {
+        error_setg(errp, "vhost-vdpa-device: vhost initialization failed: %s",
+                   strerror(-ret));
+        goto free_vqs;
+    }
+
+    v->config_size = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
+                                               VHOST_VDPA_GET_CONFIG_SIZE, errp);
+    if (*errp) {
+        goto vhost_cleanup;
+    }
+    v->config = g_malloc0(v->config_size);
+
+    ret = vhost_dev_get_config(&v->dev, v->config, v->config_size, NULL);
+    if (ret < 0) {
+        error_setg(errp, "vhost-vdpa-device: get config failed");
+        goto free_config;
+    }
+
+    virtio_init(vdev, "vhost-vdpa", v->vdev_id, v->config_size);
+
+    v->virtqs = g_new0(VirtQueue *, v->dev.nvqs);
+    for (i = 0; i < v->dev.nvqs; i++) {
+        v->virtqs[i] = virtio_add_queue(vdev, v->queue_size,
+                                        vhost_vdpa_device_dummy_handle_output);
+    }
+
+    if (v->post_init && v->post_init(v, errp) < 0) {
+        goto free_virtio;
+    }
+
     return;
+
+free_virtio:
+    for (i = 0; i < v->num_queues; i++) {
+        virtio_delete_queue(v->virtqs[i]);
+    }
+    g_free(v->virtqs);
+    virtio_cleanup(vdev);
+free_config:
+    g_free(v->config);
+vhost_cleanup:
+    vhost_dev_cleanup(&v->dev);
+free_vqs:
+    g_free(vqs);
+out:
+    qemu_close(v->vdpa_dev_fd);
+    v->vdpa_dev_fd = -1;
 }
 
 static void vhost_vdpa_device_unrealize(DeviceState *dev)
@@ -66,6 +197,7 @@  static void vhost_vdpa_device_set_status(VirtIODevice *vdev, uint8_t status)
 static Property vhost_vdpa_device_properties[] = {
     DEFINE_PROP_STRING("vdpa-dev", VhostVdpaDevice, vdpa_dev),
     DEFINE_PROP_INT32("vdpa-dev-fd", VhostVdpaDevice, vdpa_dev_fd, -1),
+    DEFINE_PROP_UINT16("queue-size", VhostVdpaDevice, queue_size, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/vdpa-dev.h b/include/hw/virtio/vdpa-dev.h
index 476bda0873..cf11abd0f7 100644
--- a/include/hw/virtio/vdpa-dev.h
+++ b/include/hw/virtio/vdpa-dev.h
@@ -28,6 +28,16 @@  struct VhostVdpaDevice {
     char *vdpa_dev;
     int vdpa_dev_fd;
     int32_t bootindex;
+    uint32_t vdev_id;
+    uint32_t num_queues;
+    struct vhost_dev dev;
+    struct vhost_vdpa vdpa;
+    VirtQueue **virtqs;
+    uint8_t *config;
+    int config_size;
+    uint16_t queue_size;
+    bool started;
+    int (*post_init)(VhostVdpaDevice *v, Error **errp);
 };
 
 #endif