Re: [PATCH 09/31] vhost-vdpa: Take into account SVQ in vhost_vdpa_set_vring_call

[Jason Wang <jasowang@redhat.com>]

在 2022/1/31 下午11:34, Eugenio Perez Martin 写道:
> On Sat, Jan 29, 2022 at 9:06 AM Jason Wang <jasowang@redhat.com> wrote:
>>
>> 在 2022/1/22 上午4:27, Eugenio Pérez 写道:
>>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>>> ---
>>>    hw/virtio/vhost-vdpa.c | 20 ++++++++++++++++++--
>>>    1 file changed, 18 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
>>> index 18de14f0fb..029f98feee 100644
>>> --- a/hw/virtio/vhost-vdpa.c
>>> +++ b/hw/virtio/vhost-vdpa.c
>>> @@ -687,13 +687,29 @@ static int vhost_vdpa_set_vring_kick(struct vhost_dev *dev,
>>>        }
>>>    }
>>>
>>> -static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
>>> -                                       struct vhost_vring_file *file)
>>> +static int vhost_vdpa_set_vring_dev_call(struct vhost_dev *dev,
>>> +                                         struct vhost_vring_file *file)
>>>    {
>>>        trace_vhost_vdpa_set_vring_call(dev, file->index, file->fd);
>>>        return vhost_vdpa_call(dev, VHOST_SET_VRING_CALL, file);
>>>    }
>>>
>>> +static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
>>> +                                     struct vhost_vring_file *file)
>>> +{
>>> +    struct vhost_vdpa *v = dev->opaque;
>>> +
>>> +    if (v->shadow_vqs_enabled) {
>>> +        int vdpa_idx = vhost_vdpa_get_vq_index(dev, file->index);
>>> +        VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, vdpa_idx);
>>> +
>>> +        vhost_svq_set_guest_call_notifier(svq, file->fd);
>>
>> Two questions here (had similar questions for vring kick):
>>
>> 1) Any reason that we setup the eventfd for vhost-vdpa in
>> vhost_vdpa_svq_setup() not here?
>>
> I'm not sure what you mean.
>
> The guest->SVQ call and kick fds are set here and at
> vhost_vdpa_set_vring_kick. The event notifier handler of the guest ->
> SVQ kick_fd is set at vhost_vdpa_set_vring_kick /
> vhost_svq_set_svq_kick_fd. The guest -> SVQ call fd has no event
> notifier handler since we don't poll it.
>
> On the other hand, the connection SVQ <-> device uses the same fds
> from the beginning to the end, and they will not change with, for
> example, call fd masking. That's why it's setup from
> vhost_vdpa_svq_setup. Delaying to vhost_vdpa_set_vring_call would make
> us add way more logic there.


More logic in general shadow vq code but less codes for vhost-vdpa 
specific code I think.

E.g for we can move the kick set logic from vhost_vdpa_svq_set_fds() to 
here.

Thanks


>
>> 2) The call could be disabled by using -1 as the fd, I don't see any
>> code to deal with that.
>>
> Right, I didn't take that into account. vhost-kernel takes also -1 as
> kick_fd to unbind, so SVQ can be reworked to take that into account
> for sure.
>
> Thanks!
>
>> Thanks
>>
>>
>>> +        return 0;
>>> +    } else {
>>> +        return vhost_vdpa_set_vring_dev_call(dev, file);
>>> +    }
>>> +}
>>> +
>>>    /**
>>>     * Set shadow virtqueue descriptors to the device
>>>     *