Re: [RFC v2 10/13] vhost: add vhost_kernel_set_vring_enable

[Jason Wang <jasowang@redhat.com>]

在 2021/3/16 下午6:43, Eugenio Perez Martin 写道:
> On Tue, Mar 16, 2021 at 8:30 AM Jason Wang <jasowang@redhat.com> wrote:
>>
>> 在 2021/3/16 上午3:48, Eugenio Pérez 写道:
>>> This method is already present in vhost-user. This commit adapts it to
>>> vhost-net, so SVQ can use.
>>>
>>> vhost_kernel_set_enable stops the device, so qemu can ask for its status
>>> (next available idx the device was going to consume). When SVQ starts it
>>> can resume consuming the guest's driver ring, without notice from the
>>> latter. Not stopping the device before of the swapping could imply that
>>> it process more buffers than reported, what would duplicate the device
>>> action.
>>
>> Note that it might not be the case of vDPA (virtio) or at least virtio
>> needs some extension to achieve something similar like this. One example
>> is virtio-pci which forbids 0 to be wrote to queue_enable.
>>
>> This is another reason to start from vhost-vDPA.
>>
>>
>>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>>> ---
>>>    hw/virtio/vhost-backend.c | 29 +++++++++++++++++++++++++++++
>>>    1 file changed, 29 insertions(+)
>>>
>>> diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
>>> index 31b33bde37..1ac5c574a9 100644
>>> --- a/hw/virtio/vhost-backend.c
>>> +++ b/hw/virtio/vhost-backend.c
>>> @@ -201,6 +201,34 @@ static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
>>>        return idx - dev->vq_index;
>>>    }
>>>
>>> +static int vhost_kernel_set_vq_enable(struct vhost_dev *dev, unsigned idx,
>>> +                                      bool enable)
>>> +{
>>> +    struct vhost_vring_file file = {
>>> +        .index = idx,
>>> +    };
>>> +
>>> +    if (!enable) {
>>> +        file.fd = -1; /* Pass -1 to unbind from file. */
>>> +    } else {
>>> +        struct vhost_net *vn_dev = container_of(dev, struct vhost_net, dev);
>>> +        file.fd = vn_dev->backend;
>>
>> This can only work with vhost-net devices but not vsock/scsi etc.
>>
> Right. Shadow virtqueue code should also check the return value of the
> vhost_set_vring_enable call.
>
> I'm not sure how to solve it without resorting to some ifelse/switch
> chain, checking for specific net/vsock/... features, or relaying on
> some other qemu class facilities. However, since the main use case is
> vDPA live migration, this commit could be left out and SVQ operation
> would only be valid for vhost-vdpa and vhost-user.


Yes, that's why I think we can start with vhost-vDPA first.

Thanks


>
>> Thanks
>>
>>
>>> +    }
>>> +
>>> +    return vhost_kernel_net_set_backend(dev, &file);
>>> +}
>>> +
>>> +static int vhost_kernel_set_vring_enable(struct vhost_dev *dev, int enable)
>>> +{
>>> +    int i;
>>> +
>>> +    for (i = 0; i < dev->nvqs; ++i) {
>>> +        vhost_kernel_set_vq_enable(dev, i, enable);
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>    #ifdef CONFIG_VHOST_VSOCK
>>>    static int vhost_kernel_vsock_set_guest_cid(struct vhost_dev *dev,
>>>                                                uint64_t guest_cid)
>>> @@ -317,6 +345,7 @@ static const VhostOps kernel_ops = {
>>>            .vhost_set_owner = vhost_kernel_set_owner,
>>>            .vhost_reset_device = vhost_kernel_reset_device,
>>>            .vhost_get_vq_index = vhost_kernel_get_vq_index,
>>> +        .vhost_set_vring_enable = vhost_kernel_set_vring_enable,
>>>    #ifdef CONFIG_VHOST_VSOCK
>>>            .vhost_vsock_set_guest_cid = vhost_kernel_vsock_set_guest_cid,
>>>            .vhost_vsock_set_running = vhost_kernel_vsock_set_running,