On Sat, Mar 05, 2022 at 07:07:54AM +0000, Longpeng (Mike, Cloud Infrastructure Service Product Dept.) wrote:
-----Original Message----- From: Stefano Garzarella [mailto:[email protected]] Sent: Wednesday, January 19, 2022 7:31 PM To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) <[email protected]> Cc: [email protected]; [email protected]; [email protected]; [email protected]; Gonglei (Arei) <[email protected]>; Yechuan <[email protected]>; Huangzhichao <[email protected]>; [email protected] 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 <[email protected]> > >Implements the .realize interface. > >Signed-off-by: Longpeng <[email protected]> >--- > 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
