? 2021/3/16 ??6:43, Eugenio Perez Martin ??:> On Tue, Mar 16, 2021 at 8:30 AM Jason Wang <jasowang at 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 at 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,