| Message ID | 20210315194842.277740-11-eperezma@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vDPA software assisted live migration | expand |
在 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. 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,
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. > 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, >
在 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,
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; + } + + 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,
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. Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- hw/virtio/vhost-backend.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)