| Message ID | 20220117124331.1642-6-longpeng2@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add generic vDPA device support | expand |
On Mon, Jan 17, 2022 at 08:43:26PM +0800, Longpeng(Mike) via wrote: >From: Longpeng <longpeng2@huawei.com> > >Implements the .realize interface. > >Signed-off-by: Longpeng <longpeng2@huawei.com> >--- > hw/virtio/vdpa-dev.c | 101 +++++++++++++++++++++++++++++++++++ > include/hw/virtio/vdpa-dev.h | 8 +++ > 2 files changed, 109 insertions(+) > >diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c >index b103768f33..bd28cf7a15 100644 >--- a/hw/virtio/vdpa-dev.c >+++ b/hw/virtio/vdpa-dev.c >@@ -27,9 +27,109 @@ uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long int cmd, Error **errp) > return val; > } > >+static void >+vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) >+{ >+ /* Nothing to do */ >+} >+ > static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp) > { >+ VirtIODevice *vdev = VIRTIO_DEVICE(dev); >+ VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev); >+ uint32_t vdev_id, max_queue_size; >+ struct vhost_virtqueue *vqs; >+ int i, ret; >+ >+ if (s->vdpa_dev_fd == -1) { >+ s->vdpa_dev_fd = qemu_open(s->vdpa_dev, O_RDWR, errp); So, here we are re-opening the `vdpa_dev` again (without checking if it is NULL). And we re-do the same ioctls already done in vhost_vdpa_device_pci_realize(), so I think we should do them in a single place, and that place should be here. So, what about doing all the ioctls here, setting appropriate fields in VhostVdpaDevice, then using that fields in vhost_vdpa_device_pci_realize() after qdev_realize() to set `class_code`, `trans_devid`, and `nvectors`? >+ if (*errp) { >+ return; >+ } >+ } >+ s->vdpa.device_fd = s->vdpa_dev_fd; >+ >+ max_queue_size = vhost_vdpa_device_get_u32(s->vdpa_dev_fd, >+ VHOST_VDPA_GET_VRING_NUM, errp); >+ if (*errp) { >+ goto out; >+ } >+ >+ if (s->queue_size > max_queue_size) { >+ error_setg(errp, "vhost-vdpa-device: invalid queue_size: %d (max:%d)", >+ s->queue_size, max_queue_size); >+ goto out; >+ } else if (!s->queue_size) { >+ s->queue_size = max_queue_size; >+ } >+ >+ s->num_queues = vhost_vdpa_device_get_u32(s->vdpa_dev_fd, >+ VHOST_VDPA_GET_VQS_NUM, errp); ^ VHOST_VDPA_GET_VQS_COUNT >+ if (*errp) { >+ goto out; >+ } >+ >+ if (!s->num_queues || s->num_queues > VIRTIO_QUEUE_MAX) { >+ error_setg(errp, "invalid number of virtqueues: %u (max:%u)", >+ s->num_queues, VIRTIO_QUEUE_MAX); >+ goto out; >+ } >+ >+ s->dev.nvqs = s->num_queues; >+ vqs = g_new0(struct vhost_virtqueue, s->dev.nvqs); >+ s->dev.vqs = vqs; >+ s->dev.vq_index = 0; >+ s->dev.vq_index_end = s->dev.nvqs; >+ s->dev.backend_features = 0; >+ s->started = false; >+ >+ ret = vhost_dev_init(&s->dev, &s->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; >+ } >+ >+ vdev_id = vhost_vdpa_device_get_u32(s->vdpa_dev_fd, >+ VHOST_VDPA_GET_DEVICE_ID, errp); >+ if (ret < 0) { >+ error_setg(errp, "vhost-vdpa-device: vhost get device id failed: %s", >+ strerror(-ret)); >+ goto vhost_cleanup; >+ } >+ >+ s->config_size = vhost_vdpa_device_get_u32(s->vdpa_dev_fd, >+ VHOST_VDPA_GET_CONFIG_SIZE, errp); >+ if (*errp) { >+ goto vhost_cleanup; >+ } >+ s->config = g_malloc0(s->config_size); >+ >+ ret = vhost_dev_get_config(&s->dev, s->config, s->config_size, NULL); >+ if (ret < 0) { >+ error_setg(errp, "vhost-vdpa-device: get config failed"); >+ goto free_config; >+ } >+ >+ virtio_init(vdev, "vhost-vdpa", vdev_id, s->config_size); >+ >+ s->virtqs = g_new0(VirtQueue *, s->dev.nvqs); >+ for (i = 0; i < s->dev.nvqs; i++) { >+ s->virtqs[i] = virtio_add_queue(vdev, s->queue_size, >+ vhost_vdpa_device_dummy_handle_output); >+ } >+ > return; >+ >+free_config: >+ g_free(s->config); >+vhost_cleanup: >+ vhost_dev_cleanup(&s->dev); >+free_vqs: >+ g_free(vqs); >+out: >+ qemu_close(s->vdpa_dev_fd); >+ s->vdpa_dev_fd = -1; > } > > static void vhost_vdpa_device_unrealize(DeviceState *dev) >@@ -64,6 +164,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 e7ad349113..e0482035cf 100644 >--- a/include/hw/virtio/vdpa-dev.h >+++ b/include/hw/virtio/vdpa-dev.h >@@ -14,6 +14,14 @@ struct VhostVdpaDevice { > char *vdpa_dev; > int vdpa_dev_fd; > int32_t bootindex; >+ struct vhost_dev dev; >+ struct vhost_vdpa vdpa; >+ VirtQueue **virtqs; >+ uint8_t *config; >+ int config_size; >+ uint32_t num_queues; >+ uint16_t queue_size; >+ bool started; > }; > > uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long int cmd, Error **errp); >-- >2.23.0 > >
> -----Original Message----- > From: Stefano Garzarella [mailto:sgarzare@redhat.com] > Sent: Wednesday, January 19, 2022 7:31 PM > To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) > <longpeng2@huawei.com> > Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; > pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan > <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; > qemu-devel@nongnu.org > Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface > > On Mon, Jan 17, 2022 at 08:43:26PM +0800, Longpeng(Mike) via wrote: > >From: Longpeng <longpeng2@huawei.com> > > > >Implements the .realize interface. > > > >Signed-off-by: Longpeng <longpeng2@huawei.com> > >--- > > hw/virtio/vdpa-dev.c | 101 +++++++++++++++++++++++++++++++++++ > > include/hw/virtio/vdpa-dev.h | 8 +++ > > 2 files changed, 109 insertions(+) > > > >diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c > >index b103768f33..bd28cf7a15 100644 > >--- a/hw/virtio/vdpa-dev.c > >+++ b/hw/virtio/vdpa-dev.c > >@@ -27,9 +27,109 @@ uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long > int cmd, Error **errp) > > return val; > > } > > > >+static void > >+vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) > >+{ > >+ /* Nothing to do */ > >+} > >+ > > static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp) > > { > >+ VirtIODevice *vdev = VIRTIO_DEVICE(dev); > >+ VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev); > >+ uint32_t vdev_id, max_queue_size; > >+ struct vhost_virtqueue *vqs; > >+ int i, ret; > >+ > >+ if (s->vdpa_dev_fd == -1) { > >+ s->vdpa_dev_fd = qemu_open(s->vdpa_dev, O_RDWR, errp); > > So, here we are re-opening the `vdpa_dev` again (without checking if it > is NULL). > > And we re-do the same ioctls already done in > vhost_vdpa_device_pci_realize(), so I think we should do them in a > single place, and that place should be here. > > So, what about doing all the ioctls here, setting appropriate fields in > VhostVdpaDevice, then using that fields in > vhost_vdpa_device_pci_realize() after qdev_realize() to set > `class_code`, `trans_devid`, and `nvectors`? > vhost_vdpa_device_pci_realize() qdev_realize() virtio_device_realize() vhost_vdpa_device_realize() virtio_bus_device_plugged() virtio_pci_device_plugged() These three fields would be used in virtio_pci_device_plugged(), so it's too late to set them after qdev_realize(). And they belong to VirtIOPCIProxy, so we cannot set them in vhost_vdpa_device_realize() which is transport layer independent. > >+ if (*errp) { > >+ return; > >+ } > >+ } > >+ s->vdpa.device_fd = s->vdpa_dev_fd; > >+ > >+ max_queue_size = vhost_vdpa_device_get_u32(s->vdpa_dev_fd, > >+ VHOST_VDPA_GET_VRING_NUM, errp); > >+ if (*errp) { > >+ goto out; > >+ } > >+ > >+ if (s->queue_size > max_queue_size) { > >+ error_setg(errp, "vhost-vdpa-device: invalid queue_size: %d > (max:%d)", > >+ s->queue_size, max_queue_size); > >+ goto out; > >+ } else if (!s->queue_size) { > >+ s->queue_size = max_queue_size; > >+ } > >+ > >+ s->num_queues = vhost_vdpa_device_get_u32(s->vdpa_dev_fd, > >+ VHOST_VDPA_GET_VQS_NUM, errp); > ^ > VHOST_VDPA_GET_VQS_COUNT > OK, thanks :) > >+ if (*errp) { > >+ goto out; > >+ } > >+ > >+ if (!s->num_queues || s->num_queues > VIRTIO_QUEUE_MAX) { > >+ error_setg(errp, "invalid number of virtqueues: %u (max:%u)", > >+ s->num_queues, VIRTIO_QUEUE_MAX); > >+ goto out; > >+ } > >+ > >+ s->dev.nvqs = s->num_queues; > >+ vqs = g_new0(struct vhost_virtqueue, s->dev.nvqs); > >+ s->dev.vqs = vqs; > >+ s->dev.vq_index = 0; > >+ s->dev.vq_index_end = s->dev.nvqs; > >+ s->dev.backend_features = 0; > >+ s->started = false; > >+ > >+ ret = vhost_dev_init(&s->dev, &s->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; > >+ } > >+ > >+ vdev_id = vhost_vdpa_device_get_u32(s->vdpa_dev_fd, > >+ VHOST_VDPA_GET_DEVICE_ID, errp); > >+ if (ret < 0) { > >+ error_setg(errp, "vhost-vdpa-device: vhost get device id failed: %s", > >+ strerror(-ret)); > >+ goto vhost_cleanup; > >+ } > >+ > >+ s->config_size = vhost_vdpa_device_get_u32(s->vdpa_dev_fd, > >+ VHOST_VDPA_GET_CONFIG_SIZE, > errp); > >+ if (*errp) { > >+ goto vhost_cleanup; > >+ } > >+ s->config = g_malloc0(s->config_size); > >+ > >+ ret = vhost_dev_get_config(&s->dev, s->config, s->config_size, NULL); > >+ if (ret < 0) { > >+ error_setg(errp, "vhost-vdpa-device: get config failed"); > >+ goto free_config; > >+ } > >+ > >+ virtio_init(vdev, "vhost-vdpa", vdev_id, s->config_size); > >+ > >+ s->virtqs = g_new0(VirtQueue *, s->dev.nvqs); > >+ for (i = 0; i < s->dev.nvqs; i++) { > >+ s->virtqs[i] = virtio_add_queue(vdev, s->queue_size, > >+ > vhost_vdpa_device_dummy_handle_output); > >+ } > >+ > > return; > >+ > >+free_config: > >+ g_free(s->config); > >+vhost_cleanup: > >+ vhost_dev_cleanup(&s->dev); > >+free_vqs: > >+ g_free(vqs); > >+out: > >+ qemu_close(s->vdpa_dev_fd); > >+ s->vdpa_dev_fd = -1; > > } > > > > static void vhost_vdpa_device_unrealize(DeviceState *dev) > >@@ -64,6 +164,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 e7ad349113..e0482035cf 100644 > >--- a/include/hw/virtio/vdpa-dev.h > >+++ b/include/hw/virtio/vdpa-dev.h > >@@ -14,6 +14,14 @@ struct VhostVdpaDevice { > > char *vdpa_dev; > > int vdpa_dev_fd; > > int32_t bootindex; > >+ struct vhost_dev dev; > >+ struct vhost_vdpa vdpa; > >+ VirtQueue **virtqs; > >+ uint8_t *config; > >+ int config_size; > >+ uint32_t num_queues; > >+ uint16_t queue_size; > >+ bool started; > > }; > > > > uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long int cmd, Error > **errp); > >-- > >2.23.0 > > > >
On Sat, Mar 05, 2022 at 07:07:54AM +0000, Longpeng (Mike, Cloud Infrastructure Service Product Dept.) wrote: > > >> -----Original Message----- >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] >> Sent: Wednesday, January 19, 2022 7:31 PM >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) >> <longpeng2@huawei.com> >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; >> qemu-devel@nongnu.org >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface >> >> On Mon, Jan 17, 2022 at 08:43:26PM +0800, Longpeng(Mike) via wrote: >> >From: Longpeng <longpeng2@huawei.com> >> > >> >Implements the .realize interface. >> > >> >Signed-off-by: Longpeng <longpeng2@huawei.com> >> >--- >> > hw/virtio/vdpa-dev.c | 101 +++++++++++++++++++++++++++++++++++ >> > include/hw/virtio/vdpa-dev.h | 8 +++ >> > 2 files changed, 109 insertions(+) >> > >> >diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c >> >index b103768f33..bd28cf7a15 100644 >> >--- a/hw/virtio/vdpa-dev.c >> >+++ b/hw/virtio/vdpa-dev.c >> >@@ -27,9 +27,109 @@ uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long >> int cmd, Error **errp) >> > return val; >> > } >> > >> >+static void >> >+vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) >> >+{ >> >+ /* Nothing to do */ >> >+} >> >+ >> > static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp) >> > { >> >+ VirtIODevice *vdev = VIRTIO_DEVICE(dev); >> >+ VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev); >> >+ uint32_t vdev_id, max_queue_size; >> >+ struct vhost_virtqueue *vqs; >> >+ int i, ret; >> >+ >> >+ if (s->vdpa_dev_fd == -1) { >> >+ s->vdpa_dev_fd = qemu_open(s->vdpa_dev, O_RDWR, errp); >> >> So, here we are re-opening the `vdpa_dev` again (without checking if it >> is NULL). >> >> And we re-do the same ioctls already done in >> vhost_vdpa_device_pci_realize(), so I think we should do them in a >> single place, and that place should be here. >> >> So, what about doing all the ioctls here, setting appropriate fields in >> VhostVdpaDevice, then using that fields in >> vhost_vdpa_device_pci_realize() after qdev_realize() to set >> `class_code`, `trans_devid`, and `nvectors`? >> > >vhost_vdpa_device_pci_realize() > qdev_realize() > virtio_device_realize() > vhost_vdpa_device_realize() > virtio_bus_device_plugged() > virtio_pci_device_plugged() > >These three fields would be used in virtio_pci_device_plugged(), so it's too >late to set them after qdev_realize(). And they belong to VirtIOPCIProxy, so >we cannot set them in vhost_vdpa_device_realize() which is transport layer >independent. Maybe I expressed myself wrong, I was saying to open the file and make ioctls in vhost_vdpa_device_realize(). Save the values we use on both sides in VhostVdpaDevice (e.g. num_queues, queue_size) and use these saved values in virtio_pci_device_plugged() without re-opening the file again. Can't we set `class_code`, `trans_devid`, and `nvectors` after calling qdev_realize()? Thanks, Stefano
> -----Original Message----- > From: Stefano Garzarella [mailto:sgarzare@redhat.com] > Sent: Monday, March 7, 2022 4:24 PM > To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) > <longpeng2@huawei.com> > Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; > pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan > <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; > qemu-devel@nongnu.org > Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface > > On Sat, Mar 05, 2022 at 07:07:54AM +0000, Longpeng (Mike, Cloud Infrastructure > Service Product Dept.) wrote: > > > > > >> -----Original Message----- > >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] > >> Sent: Wednesday, January 19, 2022 7:31 PM > >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) > >> <longpeng2@huawei.com> > >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; > >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan > >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; > >> qemu-devel@nongnu.org > >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface > >> > >> On Mon, Jan 17, 2022 at 08:43:26PM +0800, Longpeng(Mike) via wrote: > >> >From: Longpeng <longpeng2@huawei.com> > >> > > >> >Implements the .realize interface. > >> > > >> >Signed-off-by: Longpeng <longpeng2@huawei.com> > >> >--- > >> > hw/virtio/vdpa-dev.c | 101 +++++++++++++++++++++++++++++++++++ > >> > include/hw/virtio/vdpa-dev.h | 8 +++ > >> > 2 files changed, 109 insertions(+) > >> > > >> >diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c > >> >index b103768f33..bd28cf7a15 100644 > >> >--- a/hw/virtio/vdpa-dev.c > >> >+++ b/hw/virtio/vdpa-dev.c > >> >@@ -27,9 +27,109 @@ uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long > >> int cmd, Error **errp) > >> > return val; > >> > } > >> > > >> >+static void > >> >+vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) > >> >+{ > >> >+ /* Nothing to do */ > >> >+} > >> >+ > >> > static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp) > >> > { > >> >+ VirtIODevice *vdev = VIRTIO_DEVICE(dev); > >> >+ VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev); > >> >+ uint32_t vdev_id, max_queue_size; > >> >+ struct vhost_virtqueue *vqs; > >> >+ int i, ret; > >> >+ > >> >+ if (s->vdpa_dev_fd == -1) { > >> >+ s->vdpa_dev_fd = qemu_open(s->vdpa_dev, O_RDWR, errp); > >> > >> So, here we are re-opening the `vdpa_dev` again (without checking if it > >> is NULL). > >> > >> And we re-do the same ioctls already done in > >> vhost_vdpa_device_pci_realize(), so I think we should do them in a > >> single place, and that place should be here. > >> > >> So, what about doing all the ioctls here, setting appropriate fields in > >> VhostVdpaDevice, then using that fields in > >> vhost_vdpa_device_pci_realize() after qdev_realize() to set > >> `class_code`, `trans_devid`, and `nvectors`? > >> > > > >vhost_vdpa_device_pci_realize() > > qdev_realize() > > virtio_device_realize() > > vhost_vdpa_device_realize() > > virtio_bus_device_plugged() > > virtio_pci_device_plugged() > > > >These three fields would be used in virtio_pci_device_plugged(), so it's too > >late to set them after qdev_realize(). And they belong to VirtIOPCIProxy, so > >we cannot set them in vhost_vdpa_device_realize() which is transport layer > >independent. > > Maybe I expressed myself wrong, I was saying to open the file and make > ioctls in vhost_vdpa_device_realize(). Save the values we use on both > sides in VhostVdpaDevice (e.g. num_queues, queue_size) and use these > saved values in virtio_pci_device_plugged() without re-opening the file > again. > This means we need to access VhostVdpaDevice in virtio_pci_device_plugged()? > Can't we set `class_code`, `trans_devid`, and `nvectors` after calling > qdev_realize()? > > Thanks, > Stefano
On Mon, Mar 07, 2022 at 11:13:02AM +0000, Longpeng (Mike, Cloud Infrastructure Service Product Dept.) wrote: > > >> -----Original Message----- >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] >> Sent: Monday, March 7, 2022 4:24 PM >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) >> <longpeng2@huawei.com> >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; >> qemu-devel@nongnu.org >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface >> >> On Sat, Mar 05, 2022 at 07:07:54AM +0000, Longpeng (Mike, Cloud Infrastructure >> Service Product Dept.) wrote: >> > >> > >> >> -----Original Message----- >> >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] >> >> Sent: Wednesday, January 19, 2022 7:31 PM >> >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) >> >> <longpeng2@huawei.com> >> >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; >> >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan >> >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; >> >> qemu-devel@nongnu.org >> >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface >> >> >> >> On Mon, Jan 17, 2022 at 08:43:26PM +0800, Longpeng(Mike) via wrote: >> >> >From: Longpeng <longpeng2@huawei.com> >> >> > >> >> >Implements the .realize interface. >> >> > >> >> >Signed-off-by: Longpeng <longpeng2@huawei.com> >> >> >--- >> >> > hw/virtio/vdpa-dev.c | 101 +++++++++++++++++++++++++++++++++++ >> >> > include/hw/virtio/vdpa-dev.h | 8 +++ >> >> > 2 files changed, 109 insertions(+) >> >> > >> >> >diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c >> >> >index b103768f33..bd28cf7a15 100644 >> >> >--- a/hw/virtio/vdpa-dev.c >> >> >+++ b/hw/virtio/vdpa-dev.c >> >> >@@ -27,9 +27,109 @@ uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long >> >> int cmd, Error **errp) >> >> > return val; >> >> > } >> >> > >> >> >+static void >> >> >+vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) >> >> >+{ >> >> >+ /* Nothing to do */ >> >> >+} >> >> >+ >> >> > static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp) >> >> > { >> >> >+ VirtIODevice *vdev = VIRTIO_DEVICE(dev); >> >> >+ VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev); >> >> >+ uint32_t vdev_id, max_queue_size; >> >> >+ struct vhost_virtqueue *vqs; >> >> >+ int i, ret; >> >> >+ >> >> >+ if (s->vdpa_dev_fd == -1) { >> >> >+ s->vdpa_dev_fd = qemu_open(s->vdpa_dev, O_RDWR, errp); >> >> >> >> So, here we are re-opening the `vdpa_dev` again (without checking if it >> >> is NULL). >> >> >> >> And we re-do the same ioctls already done in >> >> vhost_vdpa_device_pci_realize(), so I think we should do them in a >> >> single place, and that place should be here. >> >> >> >> So, what about doing all the ioctls here, setting appropriate fields in >> >> VhostVdpaDevice, then using that fields in >> >> vhost_vdpa_device_pci_realize() after qdev_realize() to set >> >> `class_code`, `trans_devid`, and `nvectors`? >> >> >> > >> >vhost_vdpa_device_pci_realize() >> > qdev_realize() >> > virtio_device_realize() >> > vhost_vdpa_device_realize() >> > virtio_bus_device_plugged() >> > virtio_pci_device_plugged() >> > >> >These three fields would be used in virtio_pci_device_plugged(), so it's too >> >late to set them after qdev_realize(). And they belong to VirtIOPCIProxy, so >> >we cannot set them in vhost_vdpa_device_realize() which is transport layer >> >independent. >> >> Maybe I expressed myself wrong, I was saying to open the file and make >> ioctls in vhost_vdpa_device_realize(). Save the values we use on both >> sides in VhostVdpaDevice (e.g. num_queues, queue_size) and use these >> saved values in virtio_pci_device_plugged() without re-opening the file >> again. >> > >This means we need to access VhostVdpaDevice in virtio_pci_device_plugged()? Yep, or implement some functions to get those values. Stefano
> -----Original Message----- > From: Stefano Garzarella [mailto:sgarzare@redhat.com] > Sent: Monday, March 7, 2022 8:14 PM > To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) > <longpeng2@huawei.com> > Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; > pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan > <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; > qemu-devel@nongnu.org > Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface > > On Mon, Mar 07, 2022 at 11:13:02AM +0000, Longpeng (Mike, Cloud Infrastructure > Service Product Dept.) wrote: > > > > > >> -----Original Message----- > >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] > >> Sent: Monday, March 7, 2022 4:24 PM > >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) > >> <longpeng2@huawei.com> > >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; > >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan > >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; > >> qemu-devel@nongnu.org > >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface > >> > >> On Sat, Mar 05, 2022 at 07:07:54AM +0000, Longpeng (Mike, Cloud Infrastructure > >> Service Product Dept.) wrote: > >> > > >> > > >> >> -----Original Message----- > >> >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] > >> >> Sent: Wednesday, January 19, 2022 7:31 PM > >> >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) > >> >> <longpeng2@huawei.com> > >> >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; > >> >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan > >> >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; > >> >> qemu-devel@nongnu.org > >> >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface > >> >> > >> >> On Mon, Jan 17, 2022 at 08:43:26PM +0800, Longpeng(Mike) via wrote: > >> >> >From: Longpeng <longpeng2@huawei.com> > >> >> > > >> >> >Implements the .realize interface. > >> >> > > >> >> >Signed-off-by: Longpeng <longpeng2@huawei.com> > >> >> >--- > >> >> > hw/virtio/vdpa-dev.c | 101 +++++++++++++++++++++++++++++++++++ > >> >> > include/hw/virtio/vdpa-dev.h | 8 +++ > >> >> > 2 files changed, 109 insertions(+) > >> >> > > >> >> >diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c > >> >> >index b103768f33..bd28cf7a15 100644 > >> >> >--- a/hw/virtio/vdpa-dev.c > >> >> >+++ b/hw/virtio/vdpa-dev.c > >> >> >@@ -27,9 +27,109 @@ uint32_t vhost_vdpa_device_get_u32(int fd, unsigned > long > >> >> int cmd, Error **errp) > >> >> > return val; > >> >> > } > >> >> > > >> >> >+static void > >> >> >+vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue > *vq) > >> >> >+{ > >> >> >+ /* Nothing to do */ > >> >> >+} > >> >> >+ > >> >> > static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp) > >> >> > { > >> >> >+ VirtIODevice *vdev = VIRTIO_DEVICE(dev); > >> >> >+ VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev); > >> >> >+ uint32_t vdev_id, max_queue_size; > >> >> >+ struct vhost_virtqueue *vqs; > >> >> >+ int i, ret; > >> >> >+ > >> >> >+ if (s->vdpa_dev_fd == -1) { > >> >> >+ s->vdpa_dev_fd = qemu_open(s->vdpa_dev, O_RDWR, errp); > >> >> > >> >> So, here we are re-opening the `vdpa_dev` again (without checking if it > >> >> is NULL). > >> >> > >> >> And we re-do the same ioctls already done in > >> >> vhost_vdpa_device_pci_realize(), so I think we should do them in a > >> >> single place, and that place should be here. > >> >> > >> >> So, what about doing all the ioctls here, setting appropriate fields in > >> >> VhostVdpaDevice, then using that fields in > >> >> vhost_vdpa_device_pci_realize() after qdev_realize() to set > >> >> `class_code`, `trans_devid`, and `nvectors`? > >> >> > >> > > >> >vhost_vdpa_device_pci_realize() > >> > qdev_realize() > >> > virtio_device_realize() > >> > vhost_vdpa_device_realize() > >> > virtio_bus_device_plugged() > >> > virtio_pci_device_plugged() > >> > > >> >These three fields would be used in virtio_pci_device_plugged(), so it's > too > >> >late to set them after qdev_realize(). And they belong to VirtIOPCIProxy, > so > >> >we cannot set them in vhost_vdpa_device_realize() which is transport layer > >> >independent. > >> > >> Maybe I expressed myself wrong, I was saying to open the file and make > >> ioctls in vhost_vdpa_device_realize(). Save the values we use on both > >> sides in VhostVdpaDevice (e.g. num_queues, queue_size) and use these > >> saved values in virtio_pci_device_plugged() without re-opening the file > >> again. > >> > > > >This means we need to access VhostVdpaDevice in virtio_pci_device_plugged()? > > Yep, or implement some functions to get those values. > I prefer not to modify the VIRTIO or the VIRTIO_PCI core too much. How about the following proposal? struct VhostVdpaDevice { ... void (*post_init)(VhostVdpaDevice *vdpa_dev); ... } vhost_vdpa_device_pci_post_init(VhostVdpaDevice *vdpa_dev) { ... vpci_dev->class_code = virtio_pci_get_class_id(vdpa_dev->vdev_id); vpci_dev->trans_devid = virtio_pci_get_trans_devid(vdpa_dev->vdev_id); vpci_dev->nvectors = vdpa_dev->num_queues + 1; ... } vhost_vdpa_device_pci_realize(): post_init = vhost_vdpa_device_pci_post_init; vhost_vdpa_device_realize() { ... Open the file. Set vdpa_dev->vdev_id, vdpa_dev->vdev_id, vdpa_dev->num_queues ... if (vdpa_dev->post_init) { vdpa_dev->post_init(vdpa_dev); } ... } > Stefano
On Tue, Mar 08, 2022 at 03:19:55AM +0000, Longpeng (Mike, Cloud Infrastructure Service Product Dept.) wrote: > > >> -----Original Message----- >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] >> Sent: Monday, March 7, 2022 8:14 PM >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) >> <longpeng2@huawei.com> >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; >> qemu-devel@nongnu.org >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface >> >> On Mon, Mar 07, 2022 at 11:13:02AM +0000, Longpeng (Mike, Cloud Infrastructure >> Service Product Dept.) wrote: >> > >> > >> >> -----Original Message----- >> >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] >> >> Sent: Monday, March 7, 2022 4:24 PM >> >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) >> >> <longpeng2@huawei.com> >> >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; >> >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan >> >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; >> >> qemu-devel@nongnu.org >> >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface >> >> >> >> On Sat, Mar 05, 2022 at 07:07:54AM +0000, Longpeng (Mike, Cloud Infrastructure >> >> Service Product Dept.) wrote: >> >> > >> >> > >> >> >> -----Original Message----- >> >> >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] >> >> >> Sent: Wednesday, January 19, 2022 7:31 PM >> >> >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) >> >> >> <longpeng2@huawei.com> >> >> >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; >> >> >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan >> >> >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; >> >> >> qemu-devel@nongnu.org >> >> >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface >> >> >> >> >> >> On Mon, Jan 17, 2022 at 08:43:26PM +0800, Longpeng(Mike) via wrote: >> >> >> >From: Longpeng <longpeng2@huawei.com> >> >> >> > >> >> >> >Implements the .realize interface. >> >> >> > >> >> >> >Signed-off-by: Longpeng <longpeng2@huawei.com> >> >> >> >--- >> >> >> > hw/virtio/vdpa-dev.c | 101 +++++++++++++++++++++++++++++++++++ >> >> >> > include/hw/virtio/vdpa-dev.h | 8 +++ >> >> >> > 2 files changed, 109 insertions(+) >> >> >> > >> >> >> >diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c >> >> >> >index b103768f33..bd28cf7a15 100644 >> >> >> >--- a/hw/virtio/vdpa-dev.c >> >> >> >+++ b/hw/virtio/vdpa-dev.c >> >> >> >@@ -27,9 +27,109 @@ uint32_t vhost_vdpa_device_get_u32(int fd, unsigned >> long >> >> >> int cmd, Error **errp) >> >> >> > return val; >> >> >> > } >> >> >> > >> >> >> >+static void >> >> >> >+vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue >> *vq) >> >> >> >+{ >> >> >> >+ /* Nothing to do */ >> >> >> >+} >> >> >> >+ >> >> >> > static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp) >> >> >> > { >> >> >> >+ VirtIODevice *vdev = VIRTIO_DEVICE(dev); >> >> >> >+ VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev); >> >> >> >+ uint32_t vdev_id, max_queue_size; >> >> >> >+ struct vhost_virtqueue *vqs; >> >> >> >+ int i, ret; >> >> >> >+ >> >> >> >+ if (s->vdpa_dev_fd == -1) { >> >> >> >+ s->vdpa_dev_fd = qemu_open(s->vdpa_dev, O_RDWR, errp); >> >> >> >> >> >> So, here we are re-opening the `vdpa_dev` again (without checking if it >> >> >> is NULL). >> >> >> >> >> >> And we re-do the same ioctls already done in >> >> >> vhost_vdpa_device_pci_realize(), so I think we should do them in a >> >> >> single place, and that place should be here. >> >> >> >> >> >> So, what about doing all the ioctls here, setting appropriate fields in >> >> >> VhostVdpaDevice, then using that fields in >> >> >> vhost_vdpa_device_pci_realize() after qdev_realize() to set >> >> >> `class_code`, `trans_devid`, and `nvectors`? >> >> >> >> >> > >> >> >vhost_vdpa_device_pci_realize() >> >> > qdev_realize() >> >> > virtio_device_realize() >> >> > vhost_vdpa_device_realize() >> >> > virtio_bus_device_plugged() >> >> > virtio_pci_device_plugged() >> >> > >> >> >These three fields would be used in virtio_pci_device_plugged(), so it's >> too >> >> >late to set them after qdev_realize(). And they belong to VirtIOPCIProxy, >> so >> >> >we cannot set them in vhost_vdpa_device_realize() which is >> >> >transport layer >> >> >independent. >> >> >> >> Maybe I expressed myself wrong, I was saying to open the file and make >> >> ioctls in vhost_vdpa_device_realize(). Save the values we use on both >> >> sides in VhostVdpaDevice (e.g. num_queues, queue_size) and use these >> >> saved values in virtio_pci_device_plugged() without re-opening the file >> >> again. >> >> >> > >> >This means we need to access VhostVdpaDevice in virtio_pci_device_plugged()? >> >> Yep, or implement some functions to get those values. >> > >I prefer not to modify the VIRTIO or the VIRTIO_PCI core too much. Yeah, I was not thinking of modifying virtio or virtio_pci core either. >How about the following proposal? > >struct VhostVdpaDevice { > ... > void (*post_init)(VhostVdpaDevice *vdpa_dev); > ... >} > >vhost_vdpa_device_pci_post_init(VhostVdpaDevice *vdpa_dev) >{ > ... > vpci_dev->class_code = virtio_pci_get_class_id(vdpa_dev->vdev_id); > vpci_dev->trans_devid = > virtio_pci_get_trans_devid(vdpa_dev->vdev_id); > vpci_dev->nvectors = vdpa_dev->num_queues + 1; > ... >} > >vhost_vdpa_device_pci_realize(): > post_init = vhost_vdpa_device_pci_post_init; > >vhost_vdpa_device_realize() >{ > ... > Open the file. > Set vdpa_dev->vdev_id, vdpa_dev->vdev_id, vdpa_dev->num_queues > ... > if (vdpa_dev->post_init) { > vdpa_dev->post_init(vdpa_dev); > } > ... >} I was honestly thinking of something simpler: call qdev_realize() to realize the VhostVdpaDevice object and then query VhostVdpaDevice for the id and number of queues. Something like this (untested): diff --git a/include/hw/virtio/vdpa-dev.h b/include/hw/virtio/vdpa-dev.h index e0482035cf..9d5f90eacc 100644 --- a/include/hw/virtio/vdpa-dev.h +++ b/include/hw/virtio/vdpa-dev.h @@ -25,5 +25,7 @@ struct VhostVdpaDevice { }; uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long int cmd, Error **errp); +uint32_t vhost_vdpa_device_get_vdev_id(VhostVdpaDevice *s); +uint32_t vhost_vdpa_device_get_num_queues(VhostVdpaDevice *s); #endif diff --git a/hw/virtio/vdpa-dev-pci.c b/hw/virtio/vdpa-dev-pci.c index 257538dbdd..5eace2f79e 100644 --- a/hw/virtio/vdpa-dev-pci.c +++ b/hw/virtio/vdpa-dev-pci.c @@ -43,32 +43,16 @@ vhost_vdpa_device_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) VhostVdpaDevicePCI *dev = VHOST_VDPA_DEVICE_PCI(vpci_dev); DeviceState *vdev = DEVICE(&dev->vdev); uint32_t vdev_id; - uint32_t num_queues; - int fd; - fd = qemu_open(dev->vdev.vdpa_dev, O_RDWR, errp); - if (*errp) { + if (!qdev_realize(vdev, BUS(&vpci_dev->bus), errp)) { return; } - vdev_id = vhost_vdpa_device_get_u32(fd, VHOST_VDPA_GET_DEVICE_ID, errp); - if (*errp) { - qemu_close(fd); - return; - } - - num_queues = vhost_vdpa_device_get_u32(fd, VHOST_VDPA_GET_VQS_NUM, errp); - if (*errp) { - qemu_close(fd); - return; - } - - dev->vdev.vdpa_dev_fd = fd; + vdev_id = vhost_vdpa_device_get_vdev_id(&dev->vdev); vpci_dev->class_code = virtio_pci_get_class_id(vdev_id); vpci_dev->trans_devid = virtio_pci_get_trans_devid(vdev_id); /* one for config interrupt, one per vq */ - vpci_dev->nvectors = num_queues + 1; - qdev_realize(vdev, BUS(&vpci_dev->bus), errp); + vpci_dev->nvectors = vhost_vdpa_device_get_num_queues(&dev->vdev) + 1; } 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 65511243f9..3bf3040e26 100644 --- a/hw/virtio/vdpa-dev.c +++ b/hw/virtio/vdpa-dev.c @@ -27,6 +27,14 @@ uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long int cmd, Error **errp) return val; } +uint32_t vhost_vdpa_device_get_vdev_id(VhostVdpaDevice *s) { + return s->vdev_id; +} + +uint32_t vhost_vdpa_device_get_num_queues(VhostVdpaDevice *s) { + return s->num_queues; +} + static void vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) { Cheers, Stefano
> -----Original Message----- > From: Stefano Garzarella [mailto:sgarzare@redhat.com] > Sent: Tuesday, March 8, 2022 4:41 PM > To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) > <longpeng2@huawei.com> > Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; > pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan > <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; > qemu-devel@nongnu.org > Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface > > On Tue, Mar 08, 2022 at 03:19:55AM +0000, Longpeng (Mike, Cloud Infrastructure > Service Product Dept.) wrote: > > > > > >> -----Original Message----- > >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] > >> Sent: Monday, March 7, 2022 8:14 PM > >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) > >> <longpeng2@huawei.com> > >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; > >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan > >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; > >> qemu-devel@nongnu.org > >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface > >> > >> On Mon, Mar 07, 2022 at 11:13:02AM +0000, Longpeng (Mike, Cloud Infrastructure > >> Service Product Dept.) wrote: > >> > > >> > > >> >> -----Original Message----- > >> >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] > >> >> Sent: Monday, March 7, 2022 4:24 PM > >> >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) > >> >> <longpeng2@huawei.com> > >> >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; > >> >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan > >> >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; > >> >> qemu-devel@nongnu.org > >> >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface > >> >> > >> >> On Sat, Mar 05, 2022 at 07:07:54AM +0000, Longpeng (Mike, Cloud > Infrastructure > >> >> Service Product Dept.) wrote: > >> >> > > >> >> > > >> >> >> -----Original Message----- > >> >> >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] > >> >> >> Sent: Wednesday, January 19, 2022 7:31 PM > >> >> >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) > >> >> >> <longpeng2@huawei.com> > >> >> >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; > >> >> >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan > >> >> >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; > >> >> >> qemu-devel@nongnu.org > >> >> >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface > >> >> >> > >> >> >> On Mon, Jan 17, 2022 at 08:43:26PM +0800, Longpeng(Mike) via wrote: > >> >> >> >From: Longpeng <longpeng2@huawei.com> > >> >> >> > > >> >> >> >Implements the .realize interface. > >> >> >> > > >> >> >> >Signed-off-by: Longpeng <longpeng2@huawei.com> > >> >> >> >--- > >> >> >> > hw/virtio/vdpa-dev.c | 101 > +++++++++++++++++++++++++++++++++++ > >> >> >> > include/hw/virtio/vdpa-dev.h | 8 +++ > >> >> >> > 2 files changed, 109 insertions(+) > >> >> >> > > >> >> >> >diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c > >> >> >> >index b103768f33..bd28cf7a15 100644 > >> >> >> >--- a/hw/virtio/vdpa-dev.c > >> >> >> >+++ b/hw/virtio/vdpa-dev.c > >> >> >> >@@ -27,9 +27,109 @@ uint32_t vhost_vdpa_device_get_u32(int fd, unsigned > >> long > >> >> >> int cmd, Error **errp) > >> >> >> > return val; > >> >> >> > } > >> >> >> > > >> >> >> >+static void > >> >> >> >+vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue > >> *vq) > >> >> >> >+{ > >> >> >> >+ /* Nothing to do */ > >> >> >> >+} > >> >> >> >+ > >> >> >> > static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp) > >> >> >> > { > >> >> >> >+ VirtIODevice *vdev = VIRTIO_DEVICE(dev); > >> >> >> >+ VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev); > >> >> >> >+ uint32_t vdev_id, max_queue_size; > >> >> >> >+ struct vhost_virtqueue *vqs; > >> >> >> >+ int i, ret; > >> >> >> >+ > >> >> >> >+ if (s->vdpa_dev_fd == -1) { > >> >> >> >+ s->vdpa_dev_fd = qemu_open(s->vdpa_dev, O_RDWR, errp); > >> >> >> > >> >> >> So, here we are re-opening the `vdpa_dev` again (without checking if > it > >> >> >> is NULL). > >> >> >> > >> >> >> And we re-do the same ioctls already done in > >> >> >> vhost_vdpa_device_pci_realize(), so I think we should do them in a > >> >> >> single place, and that place should be here. > >> >> >> > >> >> >> So, what about doing all the ioctls here, setting appropriate fields > in > >> >> >> VhostVdpaDevice, then using that fields in > >> >> >> vhost_vdpa_device_pci_realize() after qdev_realize() to set > >> >> >> `class_code`, `trans_devid`, and `nvectors`? > >> >> >> > >> >> > > >> >> >vhost_vdpa_device_pci_realize() > >> >> > qdev_realize() > >> >> > virtio_device_realize() > >> >> > vhost_vdpa_device_realize() > >> >> > virtio_bus_device_plugged() > >> >> > virtio_pci_device_plugged() > >> >> > > >> >> >These three fields would be used in virtio_pci_device_plugged(), so it's > >> too > >> >> >late to set them after qdev_realize(). And they belong to VirtIOPCIProxy, > >> so > >> >> >we cannot set them in vhost_vdpa_device_realize() which is > >> >> >transport layer > >> >> >independent. > >> >> > >> >> Maybe I expressed myself wrong, I was saying to open the file and make > >> >> ioctls in vhost_vdpa_device_realize(). Save the values we use on both > >> >> sides in VhostVdpaDevice (e.g. num_queues, queue_size) and use these > >> >> saved values in virtio_pci_device_plugged() without re-opening the file > >> >> again. > >> >> > >> > > >> >This means we need to access VhostVdpaDevice in virtio_pci_device_plugged()? > >> > >> Yep, or implement some functions to get those values. > >> > > > >I prefer not to modify the VIRTIO or the VIRTIO_PCI core too much. > > Yeah, I was not thinking of modifying virtio or virtio_pci core either. > > >How about the following proposal? > > > >struct VhostVdpaDevice { > > ... > > void (*post_init)(VhostVdpaDevice *vdpa_dev); > > ... > >} > > > >vhost_vdpa_device_pci_post_init(VhostVdpaDevice *vdpa_dev) > >{ > > ... > > vpci_dev->class_code = virtio_pci_get_class_id(vdpa_dev->vdev_id); > > vpci_dev->trans_devid = > > virtio_pci_get_trans_devid(vdpa_dev->vdev_id); > > vpci_dev->nvectors = vdpa_dev->num_queues + 1; > > ... > >} > > > >vhost_vdpa_device_pci_realize(): > > post_init = vhost_vdpa_device_pci_post_init; > > > >vhost_vdpa_device_realize() > >{ > > ... > > Open the file. > > Set vdpa_dev->vdev_id, vdpa_dev->vdev_id, vdpa_dev->num_queues > > ... > > if (vdpa_dev->post_init) { > > vdpa_dev->post_init(vdpa_dev); > > } > > ... > >} > > I was honestly thinking of something simpler: call qdev_realize() to > realize the VhostVdpaDevice object and then query VhostVdpaDevice for > the id and number of queues. > > Something like this (untested): > > diff --git a/include/hw/virtio/vdpa-dev.h b/include/hw/virtio/vdpa-dev.h > index e0482035cf..9d5f90eacc 100644 > --- a/include/hw/virtio/vdpa-dev.h > +++ b/include/hw/virtio/vdpa-dev.h > @@ -25,5 +25,7 @@ struct VhostVdpaDevice { > }; > > uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long int cmd, Error > **errp); > +uint32_t vhost_vdpa_device_get_vdev_id(VhostVdpaDevice *s); > +uint32_t vhost_vdpa_device_get_num_queues(VhostVdpaDevice *s); > > #endif > diff --git a/hw/virtio/vdpa-dev-pci.c b/hw/virtio/vdpa-dev-pci.c > index 257538dbdd..5eace2f79e 100644 > --- a/hw/virtio/vdpa-dev-pci.c > +++ b/hw/virtio/vdpa-dev-pci.c > @@ -43,32 +43,16 @@ vhost_vdpa_device_pci_realize(VirtIOPCIProxy *vpci_dev, > Error **errp) > VhostVdpaDevicePCI *dev = VHOST_VDPA_DEVICE_PCI(vpci_dev); > DeviceState *vdev = DEVICE(&dev->vdev); > uint32_t vdev_id; > - uint32_t num_queues; > - int fd; > > - fd = qemu_open(dev->vdev.vdpa_dev, O_RDWR, errp); > - if (*errp) { > + if (!qdev_realize(vdev, BUS(&vpci_dev->bus), errp)) { > return; > } > > - vdev_id = vhost_vdpa_device_get_u32(fd, VHOST_VDPA_GET_DEVICE_ID, errp); > - if (*errp) { > - qemu_close(fd); > - return; > - } > - > - num_queues = vhost_vdpa_device_get_u32(fd, VHOST_VDPA_GET_VQS_NUM, errp); > - if (*errp) { > - qemu_close(fd); > - return; > - } > - > - dev->vdev.vdpa_dev_fd = fd; > + vdev_id = vhost_vdpa_device_get_vdev_id(&dev->vdev); > vpci_dev->class_code = virtio_pci_get_class_id(vdev_id); > vpci_dev->trans_devid = virtio_pci_get_trans_devid(vdev_id); > /* one for config interrupt, one per vq */ > - vpci_dev->nvectors = num_queues + 1; > - qdev_realize(vdev, BUS(&vpci_dev->bus), errp); > + vpci_dev->nvectors = vhost_vdpa_device_get_num_queues(&dev->vdev) + 1; > } > It may be too late to set these fields. In fact, qdev_realize() calls vhost_vdpa_device_realize() first and then calls virtio_pci_device_plugged() which would use class_code, trans_devid and nvectors, so we need to make sure they're set before invoking virtio_pci_device_plugged(). > 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 65511243f9..3bf3040e26 100644 > --- a/hw/virtio/vdpa-dev.c > +++ b/hw/virtio/vdpa-dev.c > @@ -27,6 +27,14 @@ uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long int > cmd, Error **errp) > return val; > } > > +uint32_t vhost_vdpa_device_get_vdev_id(VhostVdpaDevice *s) { > + return s->vdev_id; > +} > + > +uint32_t vhost_vdpa_device_get_num_queues(VhostVdpaDevice *s) { > + return s->num_queues; > +} > + > static void > vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) > { > > Cheers, > Stefano
On Tue, Mar 08, 2022 at 09:42:25AM +0000, Longpeng (Mike, Cloud Infrastructure Service Product Dept.) wrote: > > >> -----Original Message----- >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] >> Sent: Tuesday, March 8, 2022 4:41 PM >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) >> <longpeng2@huawei.com> >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; >> qemu-devel@nongnu.org >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface >> >> On Tue, Mar 08, 2022 at 03:19:55AM +0000, Longpeng (Mike, Cloud Infrastructure >> Service Product Dept.) wrote: >> > >> > >> >> -----Original Message----- >> >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] >> >> Sent: Monday, March 7, 2022 8:14 PM >> >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) >> >> <longpeng2@huawei.com> >> >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; >> >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan >> >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; >> >> qemu-devel@nongnu.org >> >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface >> >> >> >> On Mon, Mar 07, 2022 at 11:13:02AM +0000, Longpeng (Mike, Cloud Infrastructure >> >> Service Product Dept.) wrote: >> >> > >> >> > >> >> >> -----Original Message----- >> >> >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] >> >> >> Sent: Monday, March 7, 2022 4:24 PM >> >> >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) >> >> >> <longpeng2@huawei.com> >> >> >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; >> >> >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan >> >> >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; >> >> >> qemu-devel@nongnu.org >> >> >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface >> >> >> >> >> >> On Sat, Mar 05, 2022 at 07:07:54AM +0000, Longpeng (Mike, Cloud >> Infrastructure >> >> >> Service Product Dept.) wrote: >> >> >> > >> >> >> > >> >> >> >> -----Original Message----- >> >> >> >> From: Stefano Garzarella [mailto:sgarzare@redhat.com] >> >> >> >> Sent: Wednesday, January 19, 2022 7:31 PM >> >> >> >> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) >> >> >> >> <longpeng2@huawei.com> >> >> >> >> Cc: stefanha@redhat.com; mst@redhat.com; cohuck@redhat.com; >> >> >> >> pbonzini@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>; Yechuan >> >> >> >> <yechuan@huawei.com>; Huangzhichao <huangzhichao@huawei.com>; >> >> >> >> qemu-devel@nongnu.org >> >> >> >> Subject: Re: [PATCH v2 05/10] vdpa-dev: implement the realize interface >> >> >> >> >> >> >> >> On Mon, Jan 17, 2022 at 08:43:26PM +0800, Longpeng(Mike) via wrote: >> >> >> >> >From: Longpeng <longpeng2@huawei.com> >> >> >> >> > >> >> >> >> >Implements the .realize interface. >> >> >> >> > >> >> >> >> >Signed-off-by: Longpeng <longpeng2@huawei.com> >> >> >> >> >--- >> >> >> >> > hw/virtio/vdpa-dev.c | 101 >> +++++++++++++++++++++++++++++++++++ >> >> >> >> > include/hw/virtio/vdpa-dev.h | 8 +++ >> >> >> >> > 2 files changed, 109 insertions(+) >> >> >> >> > >> >> >> >> >diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c >> >> >> >> >index b103768f33..bd28cf7a15 100644 >> >> >> >> >--- a/hw/virtio/vdpa-dev.c >> >> >> >> >+++ b/hw/virtio/vdpa-dev.c >> >> >> >> >@@ -27,9 +27,109 @@ uint32_t vhost_vdpa_device_get_u32(int fd, unsigned >> >> long >> >> >> >> int cmd, Error **errp) >> >> >> >> > return val; >> >> >> >> > } >> >> >> >> > >> >> >> >> >+static void >> >> >> >> >+vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue >> >> *vq) >> >> >> >> >+{ >> >> >> >> >+ /* Nothing to do */ >> >> >> >> >+} >> >> >> >> >+ >> >> >> >> > static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp) >> >> >> >> > { >> >> >> >> >+ VirtIODevice *vdev = VIRTIO_DEVICE(dev); >> >> >> >> >+ VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev); >> >> >> >> >+ uint32_t vdev_id, max_queue_size; >> >> >> >> >+ struct vhost_virtqueue *vqs; >> >> >> >> >+ int i, ret; >> >> >> >> >+ >> >> >> >> >+ if (s->vdpa_dev_fd == -1) { >> >> >> >> >+ s->vdpa_dev_fd = qemu_open(s->vdpa_dev, O_RDWR, errp); >> >> >> >> >> >> >> >> So, here we are re-opening the `vdpa_dev` again (without checking if >> it >> >> >> >> is NULL). >> >> >> >> >> >> >> >> And we re-do the same ioctls already done in >> >> >> >> vhost_vdpa_device_pci_realize(), so I think we should do them in a >> >> >> >> single place, and that place should be here. >> >> >> >> >> >> >> >> So, what about doing all the ioctls here, setting appropriate fields >> in >> >> >> >> VhostVdpaDevice, then using that fields in >> >> >> >> vhost_vdpa_device_pci_realize() after qdev_realize() to set >> >> >> >> `class_code`, `trans_devid`, and `nvectors`? >> >> >> >> >> >> >> > >> >> >> >vhost_vdpa_device_pci_realize() >> >> >> > qdev_realize() >> >> >> > virtio_device_realize() >> >> >> > vhost_vdpa_device_realize() >> >> >> > virtio_bus_device_plugged() >> >> >> > virtio_pci_device_plugged() >> >> >> > >> >> >> >These three fields would be used in virtio_pci_device_plugged(), so it's >> >> too >> >> >> >late to set them after qdev_realize(). And they belong to VirtIOPCIProxy, >> >> so >> >> >> >we cannot set them in vhost_vdpa_device_realize() which is >> >> >> >transport layer >> >> >> >independent. >> >> >> >> >> >> Maybe I expressed myself wrong, I was saying to open the file and make >> >> >> ioctls in vhost_vdpa_device_realize(). Save the values we use on both >> >> >> sides in VhostVdpaDevice (e.g. num_queues, queue_size) and use these >> >> >> saved values in virtio_pci_device_plugged() without re-opening the file >> >> >> again. >> >> >> >> >> > >> >> >This means we need to access VhostVdpaDevice in virtio_pci_device_plugged()? >> >> >> >> Yep, or implement some functions to get those values. >> >> >> > >> >I prefer not to modify the VIRTIO or the VIRTIO_PCI core too much. >> >> Yeah, I was not thinking of modifying virtio or virtio_pci core either. >> >> >How about the following proposal? >> > >> >struct VhostVdpaDevice { >> > ... >> > void (*post_init)(VhostVdpaDevice *vdpa_dev); >> > ... >> >} >> > >> >vhost_vdpa_device_pci_post_init(VhostVdpaDevice *vdpa_dev) >> >{ >> > ... >> > vpci_dev->class_code = virtio_pci_get_class_id(vdpa_dev->vdev_id); >> > vpci_dev->trans_devid = >> > virtio_pci_get_trans_devid(vdpa_dev->vdev_id); >> > vpci_dev->nvectors = vdpa_dev->num_queues + 1; >> > ... >> >} >> > >> >vhost_vdpa_device_pci_realize(): >> > post_init = vhost_vdpa_device_pci_post_init; >> > >> >vhost_vdpa_device_realize() >> >{ >> > ... >> > Open the file. >> > Set vdpa_dev->vdev_id, vdpa_dev->vdev_id, vdpa_dev->num_queues >> > ... >> > if (vdpa_dev->post_init) { >> > vdpa_dev->post_init(vdpa_dev); >> > } >> > ... >> >} >> >> I was honestly thinking of something simpler: call qdev_realize() to >> realize the VhostVdpaDevice object and then query VhostVdpaDevice for >> the id and number of queues. >> >> Something like this (untested): >> >> diff --git a/include/hw/virtio/vdpa-dev.h b/include/hw/virtio/vdpa-dev.h >> index e0482035cf..9d5f90eacc 100644 >> --- a/include/hw/virtio/vdpa-dev.h >> +++ b/include/hw/virtio/vdpa-dev.h >> @@ -25,5 +25,7 @@ struct VhostVdpaDevice { >> }; >> >> uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long int cmd, Error >> **errp); >> +uint32_t vhost_vdpa_device_get_vdev_id(VhostVdpaDevice *s); >> +uint32_t vhost_vdpa_device_get_num_queues(VhostVdpaDevice *s); >> >> #endif >> diff --git a/hw/virtio/vdpa-dev-pci.c b/hw/virtio/vdpa-dev-pci.c >> index 257538dbdd..5eace2f79e 100644 >> --- a/hw/virtio/vdpa-dev-pci.c >> +++ b/hw/virtio/vdpa-dev-pci.c >> @@ -43,32 +43,16 @@ vhost_vdpa_device_pci_realize(VirtIOPCIProxy *vpci_dev, >> Error **errp) >> VhostVdpaDevicePCI *dev = VHOST_VDPA_DEVICE_PCI(vpci_dev); >> DeviceState *vdev = DEVICE(&dev->vdev); >> uint32_t vdev_id; >> - uint32_t num_queues; >> - int fd; >> >> - fd = qemu_open(dev->vdev.vdpa_dev, O_RDWR, errp); >> - if (*errp) { >> + if (!qdev_realize(vdev, BUS(&vpci_dev->bus), errp)) { >> return; >> } >> >> - vdev_id = vhost_vdpa_device_get_u32(fd, VHOST_VDPA_GET_DEVICE_ID, errp); >> - if (*errp) { >> - qemu_close(fd); >> - return; >> - } >> - >> - num_queues = vhost_vdpa_device_get_u32(fd, VHOST_VDPA_GET_VQS_NUM, errp); >> - if (*errp) { >> - qemu_close(fd); >> - return; >> - } >> - >> - dev->vdev.vdpa_dev_fd = fd; >> + vdev_id = vhost_vdpa_device_get_vdev_id(&dev->vdev); >> vpci_dev->class_code = virtio_pci_get_class_id(vdev_id); >> vpci_dev->trans_devid = virtio_pci_get_trans_devid(vdev_id); >> /* one for config interrupt, one per vq */ >> - vpci_dev->nvectors = num_queues + 1; >> - qdev_realize(vdev, BUS(&vpci_dev->bus), errp); >> + vpci_dev->nvectors = vhost_vdpa_device_get_num_queues(&dev->vdev) + 1; >> } >> > >It may be too late to set these fields. > >In fact, qdev_realize() calls vhost_vdpa_device_realize() first and then >calls virtio_pci_device_plugged() which would use class_code, trans_devid >and nvectors, so we need to make sure they're set before invoking >virtio_pci_device_plugged(). Right, so let's go with your solution unless someone has a better idea. Thanks, Stefano
diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c index b103768f33..bd28cf7a15 100644 --- a/hw/virtio/vdpa-dev.c +++ b/hw/virtio/vdpa-dev.c @@ -27,9 +27,109 @@ uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long int cmd, Error **errp) return val; } +static void +vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) +{ + /* Nothing to do */ +} + static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp) { + VirtIODevice *vdev = VIRTIO_DEVICE(dev); + VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev); + uint32_t vdev_id, max_queue_size; + struct vhost_virtqueue *vqs; + int i, ret; + + if (s->vdpa_dev_fd == -1) { + s->vdpa_dev_fd = qemu_open(s->vdpa_dev, O_RDWR, errp); + if (*errp) { + return; + } + } + s->vdpa.device_fd = s->vdpa_dev_fd; + + max_queue_size = vhost_vdpa_device_get_u32(s->vdpa_dev_fd, + VHOST_VDPA_GET_VRING_NUM, errp); + if (*errp) { + goto out; + } + + if (s->queue_size > max_queue_size) { + error_setg(errp, "vhost-vdpa-device: invalid queue_size: %d (max:%d)", + s->queue_size, max_queue_size); + goto out; + } else if (!s->queue_size) { + s->queue_size = max_queue_size; + } + + s->num_queues = vhost_vdpa_device_get_u32(s->vdpa_dev_fd, + VHOST_VDPA_GET_VQS_NUM, errp); + if (*errp) { + goto out; + } + + if (!s->num_queues || s->num_queues > VIRTIO_QUEUE_MAX) { + error_setg(errp, "invalid number of virtqueues: %u (max:%u)", + s->num_queues, VIRTIO_QUEUE_MAX); + goto out; + } + + s->dev.nvqs = s->num_queues; + vqs = g_new0(struct vhost_virtqueue, s->dev.nvqs); + s->dev.vqs = vqs; + s->dev.vq_index = 0; + s->dev.vq_index_end = s->dev.nvqs; + s->dev.backend_features = 0; + s->started = false; + + ret = vhost_dev_init(&s->dev, &s->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; + } + + vdev_id = vhost_vdpa_device_get_u32(s->vdpa_dev_fd, + VHOST_VDPA_GET_DEVICE_ID, errp); + if (ret < 0) { + error_setg(errp, "vhost-vdpa-device: vhost get device id failed: %s", + strerror(-ret)); + goto vhost_cleanup; + } + + s->config_size = vhost_vdpa_device_get_u32(s->vdpa_dev_fd, + VHOST_VDPA_GET_CONFIG_SIZE, errp); + if (*errp) { + goto vhost_cleanup; + } + s->config = g_malloc0(s->config_size); + + ret = vhost_dev_get_config(&s->dev, s->config, s->config_size, NULL); + if (ret < 0) { + error_setg(errp, "vhost-vdpa-device: get config failed"); + goto free_config; + } + + virtio_init(vdev, "vhost-vdpa", vdev_id, s->config_size); + + s->virtqs = g_new0(VirtQueue *, s->dev.nvqs); + for (i = 0; i < s->dev.nvqs; i++) { + s->virtqs[i] = virtio_add_queue(vdev, s->queue_size, + vhost_vdpa_device_dummy_handle_output); + } + return; + +free_config: + g_free(s->config); +vhost_cleanup: + vhost_dev_cleanup(&s->dev); +free_vqs: + g_free(vqs); +out: + qemu_close(s->vdpa_dev_fd); + s->vdpa_dev_fd = -1; } static void vhost_vdpa_device_unrealize(DeviceState *dev) @@ -64,6 +164,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 e7ad349113..e0482035cf 100644 --- a/include/hw/virtio/vdpa-dev.h +++ b/include/hw/virtio/vdpa-dev.h @@ -14,6 +14,14 @@ struct VhostVdpaDevice { char *vdpa_dev; int vdpa_dev_fd; int32_t bootindex; + struct vhost_dev dev; + struct vhost_vdpa vdpa; + VirtQueue **virtqs; + uint8_t *config; + int config_size; + uint32_t num_queues; + uint16_t queue_size; + bool started; }; uint32_t vhost_vdpa_device_get_u32(int fd, unsigned long int cmd, Error **errp);