Not stopping them leave the device in a bad state when virtio-net
fronted device is unplugged with device_del monitor command.
This is not triggable in regular poweroff or qemu forces shutdown
because cleanup is called right after vhost_vdpa_dev_start(false). But
devices hot unplug does not call vdpa device cleanups. This lead to all
the vhost_vdpa devices without stop the SVQ but the last.
Fix it and clean the code, making it symmetric with
vhost_vdpa_svqs_start.
Fixes: dff4426fa656 ("vhost: Add Shadow VirtQueue kick forwarding capabilities")
Reported-by: Lei Yang <leiyang@redhat.com>
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
hw/virtio/vhost-vdpa.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 542e003101..df3a1e92ac 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -689,26 +689,11 @@ static int vhost_vdpa_get_device_id(struct vhost_dev *dev,
return ret;
}
-static void vhost_vdpa_reset_svq(struct vhost_vdpa *v)
-{
- if (!v->shadow_vqs_enabled) {
- return;
- }
-
- for (unsigned i = 0; i < v->shadow_vqs->len; ++i) {
- VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i);
- vhost_svq_stop(svq);
- }
-}
-
static int vhost_vdpa_reset_device(struct vhost_dev *dev)
{
- struct vhost_vdpa *v = dev->opaque;
int ret;
uint8_t status = 0;
- vhost_vdpa_reset_svq(v);
-
ret = vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &status);
trace_vhost_vdpa_reset_device(dev, status);
return ret;
@@ -1100,6 +1085,8 @@ static void vhost_vdpa_svqs_stop(struct vhost_dev *dev)
for (unsigned i = 0; i < v->shadow_vqs->len; ++i) {
VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i);
+
+ vhost_svq_stop(svq);
vhost_vdpa_svq_unmap_rings(dev, svq);
event_notifier_cleanup(&svq->hdev_kick);
--
2.31.1
Hi, this patch fixes a QEMU crash, could it be merged? Thanks, Laurent On 2/9/23 18:00, Eugenio Pérez wrote: > Not stopping them leave the device in a bad state when virtio-net > fronted device is unplugged with device_del monitor command. > > This is not triggable in regular poweroff or qemu forces shutdown > because cleanup is called right after vhost_vdpa_dev_start(false). But > devices hot unplug does not call vdpa device cleanups. This lead to all > the vhost_vdpa devices without stop the SVQ but the last. > > Fix it and clean the code, making it symmetric with > vhost_vdpa_svqs_start. > > Fixes: dff4426fa656 ("vhost: Add Shadow VirtQueue kick forwarding capabilities") > Reported-by: Lei Yang <leiyang@redhat.com> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > --- > hw/virtio/vhost-vdpa.c | 17 ++--------------- > 1 file changed, 2 insertions(+), 15 deletions(-) > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > index 542e003101..df3a1e92ac 100644 > --- a/hw/virtio/vhost-vdpa.c > +++ b/hw/virtio/vhost-vdpa.c > @@ -689,26 +689,11 @@ static int vhost_vdpa_get_device_id(struct vhost_dev *dev, > return ret; > } > > -static void vhost_vdpa_reset_svq(struct vhost_vdpa *v) > -{ > - if (!v->shadow_vqs_enabled) { > - return; > - } > - > - for (unsigned i = 0; i < v->shadow_vqs->len; ++i) { > - VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i); > - vhost_svq_stop(svq); > - } > -} > - > static int vhost_vdpa_reset_device(struct vhost_dev *dev) > { > - struct vhost_vdpa *v = dev->opaque; > int ret; > uint8_t status = 0; > > - vhost_vdpa_reset_svq(v); > - > ret = vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &status); > trace_vhost_vdpa_reset_device(dev, status); > return ret; > @@ -1100,6 +1085,8 @@ static void vhost_vdpa_svqs_stop(struct vhost_dev *dev) > > for (unsigned i = 0; i < v->shadow_vqs->len; ++i) { > VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i); > + > + vhost_svq_stop(svq); > vhost_vdpa_svq_unmap_rings(dev, svq); > > event_notifier_cleanup(&svq->hdev_kick);
On Thu, Feb 23, 2023 at 4:51 PM Laurent Vivier <lvivier@redhat.com> wrote: > > Hi, > > this patch fixes a QEMU crash, could it be merged? Acked-by: Jason Wang <jasowang@redhat.com> I think it should go with Michael's tree. Thanks > > Thanks, > Laurent > > On 2/9/23 18:00, Eugenio Pérez wrote: > > Not stopping them leave the device in a bad state when virtio-net > > fronted device is unplugged with device_del monitor command. > > > > This is not triggable in regular poweroff or qemu forces shutdown > > because cleanup is called right after vhost_vdpa_dev_start(false). But > > devices hot unplug does not call vdpa device cleanups. This lead to all > > the vhost_vdpa devices without stop the SVQ but the last. > > > > Fix it and clean the code, making it symmetric with > > vhost_vdpa_svqs_start. > > > > Fixes: dff4426fa656 ("vhost: Add Shadow VirtQueue kick forwarding capabilities") > > Reported-by: Lei Yang <leiyang@redhat.com> > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > > --- > > hw/virtio/vhost-vdpa.c | 17 ++--------------- > > 1 file changed, 2 insertions(+), 15 deletions(-) > > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > > index 542e003101..df3a1e92ac 100644 > > --- a/hw/virtio/vhost-vdpa.c > > +++ b/hw/virtio/vhost-vdpa.c > > @@ -689,26 +689,11 @@ static int vhost_vdpa_get_device_id(struct vhost_dev *dev, > > return ret; > > } > > > > -static void vhost_vdpa_reset_svq(struct vhost_vdpa *v) > > -{ > > - if (!v->shadow_vqs_enabled) { > > - return; > > - } > > - > > - for (unsigned i = 0; i < v->shadow_vqs->len; ++i) { > > - VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i); > > - vhost_svq_stop(svq); > > - } > > -} > > - > > static int vhost_vdpa_reset_device(struct vhost_dev *dev) > > { > > - struct vhost_vdpa *v = dev->opaque; > > int ret; > > uint8_t status = 0; > > > > - vhost_vdpa_reset_svq(v); > > - > > ret = vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &status); > > trace_vhost_vdpa_reset_device(dev, status); > > return ret; > > @@ -1100,6 +1085,8 @@ static void vhost_vdpa_svqs_stop(struct vhost_dev *dev) > > > > for (unsigned i = 0; i < v->shadow_vqs->len; ++i) { > > VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i); > > + > > + vhost_svq_stop(svq); > > vhost_vdpa_svq_unmap_rings(dev, svq); > > > > event_notifier_cleanup(&svq->hdev_kick); >
On 2/9/23 18:00, Eugenio Pérez wrote: > Not stopping them leave the device in a bad state when virtio-net > fronted device is unplugged with device_del monitor command. > > This is not triggable in regular poweroff or qemu forces shutdown > because cleanup is called right after vhost_vdpa_dev_start(false). But > devices hot unplug does not call vdpa device cleanups. This lead to all > the vhost_vdpa devices without stop the SVQ but the last. > > Fix it and clean the code, making it symmetric with > vhost_vdpa_svqs_start. > > Fixes: dff4426fa656 ("vhost: Add Shadow VirtQueue kick forwarding capabilities") > Reported-by: Lei Yang <leiyang@redhat.com> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > --- > hw/virtio/vhost-vdpa.c | 17 ++--------------- > 1 file changed, 2 insertions(+), 15 deletions(-) > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > index 542e003101..df3a1e92ac 100644 > --- a/hw/virtio/vhost-vdpa.c > +++ b/hw/virtio/vhost-vdpa.c > @@ -689,26 +689,11 @@ static int vhost_vdpa_get_device_id(struct vhost_dev *dev, > return ret; > } > > -static void vhost_vdpa_reset_svq(struct vhost_vdpa *v) > -{ > - if (!v->shadow_vqs_enabled) { > - return; > - } > - > - for (unsigned i = 0; i < v->shadow_vqs->len; ++i) { > - VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i); > - vhost_svq_stop(svq); > - } > -} > - > static int vhost_vdpa_reset_device(struct vhost_dev *dev) > { > - struct vhost_vdpa *v = dev->opaque; > int ret; > uint8_t status = 0; > > - vhost_vdpa_reset_svq(v); > - > ret = vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &status); > trace_vhost_vdpa_reset_device(dev, status); > return ret; > @@ -1100,6 +1085,8 @@ static void vhost_vdpa_svqs_stop(struct vhost_dev *dev) > > for (unsigned i = 0; i < v->shadow_vqs->len; ++i) { > VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i); > + > + vhost_svq_stop(svq); > vhost_vdpa_svq_unmap_rings(dev, svq); > > event_notifier_cleanup(&svq->hdev_kick); Tested-by: Laurent Vivier <lvivier@redhat.com>
© 2016 - 2024 Red Hat, Inc.