-----Original Message-----
From: Stefano Garzarella [mailto:sgarz...@redhat.com]
Sent: Monday, March 7, 2022 4:24 PM
To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
<longpe...@huawei.com>
Cc: stefa...@redhat.com; m...@redhat.com; coh...@redhat.com;
pbonz...@redhat.com; Gonglei (Arei) <arei.gong...@huawei.com>; Yechuan
<yech...@huawei.com>; Huangzhichao <huangzhic...@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:sgarz...@redhat.com]
>> Sent: Wednesday, January 19, 2022 7:31 PM
>> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
>> <longpe...@huawei.com>
>> Cc: stefa...@redhat.com; m...@redhat.com; coh...@redhat.com;
>> pbonz...@redhat.com; Gonglei (Arei) <arei.gong...@huawei.com>; Yechuan
>> <yech...@huawei.com>; Huangzhichao <huangzhic...@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 <longpe...@huawei.com>
>> >
>> >Implements the .realize interface.
>> >
>> >Signed-off-by: Longpeng <longpe...@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.
