| Message ID | 20211101083813.1224522-2-xuemingl@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | Improve vhost-user VQ notifier unmap | expand |
On Mon, Nov 01, 2021 at 04:38:12PM +0800, Xueming Li wrote: > When vhost-user vdpa client suspend, backend may close all resources, > VQ notifier mmap address become invalid, restore MR which contains > the invalid address is wrong. vdpa client will set VQ notifier after > reconnect. > > This patch removes VQ notifier restore and related flags to avoid reusing > invalid address. > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > Cc: qemu-stable@nongnu.org > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > --- > hw/virtio/vhost-user.c | 19 +------------------ > include/hw/virtio/vhost-user.h | 1 - > 2 files changed, 1 insertion(+), 19 deletions(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index bf6e50223c..c671719e9b 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -1143,19 +1143,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, > return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); > } > > -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, > - int queue_idx) > -{ > - struct vhost_user *u = dev->opaque; > - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > - VirtIODevice *vdev = dev->vdev; > - > - if (n->addr && !n->set) { > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); > - n->set = true; > - } > -} > - > static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > int queue_idx) > { > @@ -1163,17 +1150,14 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > VirtIODevice *vdev = dev->vdev; > > - if (n->addr && n->set) { > + if (n->addr) { > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > - n->set = false; > } > } > So on vq stop we still remove the notifier... > static int vhost_user_set_vring_base(struct vhost_dev *dev, > struct vhost_vring_state *ring) > { > - vhost_user_host_notifier_restore(dev, ring->index); > - > return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); > } > but on vq start we do not reinstate it? Does this not mean that notifiers won't work after stop then start? > @@ -1538,7 +1522,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > } > > n->addr = addr; > - n->set = true; > > return 0; > } > diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h > index a9abca3288..f6012b2078 100644 > --- a/include/hw/virtio/vhost-user.h > +++ b/include/hw/virtio/vhost-user.h > @@ -14,7 +14,6 @@ > typedef struct VhostUserHostNotifier { > MemoryRegion mr; > void *addr; > - bool set; > } VhostUserHostNotifier; > > typedef struct VhostUserState { > -- > 2.33.0
On Mon, 2021-11-01 at 17:06 -0400, Michael S. Tsirkin wrote: > On Mon, Nov 01, 2021 at 04:38:12PM +0800, Xueming Li wrote: > > When vhost-user vdpa client suspend, backend may close all resources, > > VQ notifier mmap address become invalid, restore MR which contains > > the invalid address is wrong. vdpa client will set VQ notifier after > > reconnect. > > > > This patch removes VQ notifier restore and related flags to avoid reusing > > invalid address. > > > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > > Cc: qemu-stable@nongnu.org > > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > --- > > hw/virtio/vhost-user.c | 19 +------------------ > > include/hw/virtio/vhost-user.h | 1 - > > 2 files changed, 1 insertion(+), 19 deletions(-) > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > index bf6e50223c..c671719e9b 100644 > > --- a/hw/virtio/vhost-user.c > > +++ b/hw/virtio/vhost-user.c > > @@ -1143,19 +1143,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); > > } > > > > -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, > > - int queue_idx) > > -{ > > - struct vhost_user *u = dev->opaque; > > - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > - VirtIODevice *vdev = dev->vdev; > > - > > - if (n->addr && !n->set) { > > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); > > - n->set = true; > > - } > > -} > > - > > static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > int queue_idx) > > { > > @@ -1163,17 +1150,14 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > VirtIODevice *vdev = dev->vdev; > > > > - if (n->addr && n->set) { > > + if (n->addr) { > > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > - n->set = false; > > } > > } > > > > So on vq stop we still remove the notifier... > > > static int vhost_user_set_vring_base(struct vhost_dev *dev, > > struct vhost_vring_state *ring) > > { > > - vhost_user_host_notifier_restore(dev, ring->index); > > - > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); > > } > > > > but on vq start we do not reinstate it? Does this not mean that > notifiers won't work after stop then start? Yes, backend initially work w/o host notifier, request VM driver to install notifier if needed - call this function through slave socket. > > > > @@ -1538,7 +1522,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > > } > > > > n->addr = addr; > > - n->set = true; > > > > return 0; > > } > > diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h > > index a9abca3288..f6012b2078 100644 > > --- a/include/hw/virtio/vhost-user.h > > +++ b/include/hw/virtio/vhost-user.h > > @@ -14,7 +14,6 @@ > > typedef struct VhostUserHostNotifier { > > MemoryRegion mr; > > void *addr; > > - bool set; > > } VhostUserHostNotifier; > > > > typedef struct VhostUserState { > > -- > > 2.33.0 >
On Tue, Nov 02, 2021 at 06:08:39AM +0000, Xueming(Steven) Li wrote: > On Mon, 2021-11-01 at 17:06 -0400, Michael S. Tsirkin wrote: > > On Mon, Nov 01, 2021 at 04:38:12PM +0800, Xueming Li wrote: > > > When vhost-user vdpa client suspend, backend may close all resources, > > > VQ notifier mmap address become invalid, restore MR which contains > > > the invalid address is wrong. vdpa client will set VQ notifier after > > > reconnect. > > > > > > This patch removes VQ notifier restore and related flags to avoid reusing > > > invalid address. > > > > > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > > > Cc: qemu-stable@nongnu.org > > > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > > --- > > > hw/virtio/vhost-user.c | 19 +------------------ > > > include/hw/virtio/vhost-user.h | 1 - > > > 2 files changed, 1 insertion(+), 19 deletions(-) > > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > > index bf6e50223c..c671719e9b 100644 > > > --- a/hw/virtio/vhost-user.c > > > +++ b/hw/virtio/vhost-user.c > > > @@ -1143,19 +1143,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); > > > } > > > > > > -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, > > > - int queue_idx) > > > -{ > > > - struct vhost_user *u = dev->opaque; > > > - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > - VirtIODevice *vdev = dev->vdev; > > > - > > > - if (n->addr && !n->set) { > > > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); > > > - n->set = true; > > > - } > > > -} > > > - > > > static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > int queue_idx) > > > { > > > @@ -1163,17 +1150,14 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > VirtIODevice *vdev = dev->vdev; > > > > > > - if (n->addr && n->set) { > > > + if (n->addr) { > > > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > > - n->set = false; > > > } > > > } > > > > > > > So on vq stop we still remove the notifier... > > > > > static int vhost_user_set_vring_base(struct vhost_dev *dev, > > > struct vhost_vring_state *ring) > > > { > > > - vhost_user_host_notifier_restore(dev, ring->index); > > > - > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); > > > } > > > > > > > but on vq start we do not reinstate it? Does this not mean that > > notifiers won't work after stop then start? > > > Yes, backend initially work w/o host notifier, request VM driver to > install notifier if needed - call this function through slave socket. I think it's cleaner if qemu handles this itself like it did before, it knows vm is stopped without getting called. > > > > > > > @@ -1538,7 +1522,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > > > } > > > > > > n->addr = addr; > > > - n->set = true; > > > > > > return 0; > > > } > > > diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h > > > index a9abca3288..f6012b2078 100644 > > > --- a/include/hw/virtio/vhost-user.h > > > +++ b/include/hw/virtio/vhost-user.h > > > @@ -14,7 +14,6 @@ > > > typedef struct VhostUserHostNotifier { > > > MemoryRegion mr; > > > void *addr; > > > - bool set; > > > } VhostUserHostNotifier; > > > > > > typedef struct VhostUserState { > > > -- > > > 2.33.0 > > >
On Tue, 2021-11-02 at 02:49 -0400, Michael S. Tsirkin wrote: > On Tue, Nov 02, 2021 at 06:08:39AM +0000, Xueming(Steven) Li wrote: > > On Mon, 2021-11-01 at 17:06 -0400, Michael S. Tsirkin wrote: > > > On Mon, Nov 01, 2021 at 04:38:12PM +0800, Xueming Li wrote: > > > > When vhost-user vdpa client suspend, backend may close all resources, > > > > VQ notifier mmap address become invalid, restore MR which contains > > > > the invalid address is wrong. vdpa client will set VQ notifier after > > > > reconnect. > > > > > > > > This patch removes VQ notifier restore and related flags to avoid reusing > > > > invalid address. > > > > > > > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > > > > Cc: qemu-stable@nongnu.org > > > > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > > > --- > > > > hw/virtio/vhost-user.c | 19 +------------------ > > > > include/hw/virtio/vhost-user.h | 1 - > > > > 2 files changed, 1 insertion(+), 19 deletions(-) > > > > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > > > index bf6e50223c..c671719e9b 100644 > > > > --- a/hw/virtio/vhost-user.c > > > > +++ b/hw/virtio/vhost-user.c > > > > @@ -1143,19 +1143,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, > > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); > > > > } > > > > > > > > -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, > > > > - int queue_idx) > > > > -{ > > > > - struct vhost_user *u = dev->opaque; > > > > - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > > - VirtIODevice *vdev = dev->vdev; > > > > - > > > > - if (n->addr && !n->set) { > > > > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); > > > > - n->set = true; > > > > - } > > > > -} > > > > - > > > > static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > int queue_idx) > > > > { > > > > @@ -1163,17 +1150,14 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > > VirtIODevice *vdev = dev->vdev; > > > > > > > > - if (n->addr && n->set) { > > > > + if (n->addr) { > > > > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > > > - n->set = false; > > > > } > > > > } > > > > > > > > > > So on vq stop we still remove the notifier... > > > > > > > static int vhost_user_set_vring_base(struct vhost_dev *dev, > > > > struct vhost_vring_state *ring) > > > > { > > > > - vhost_user_host_notifier_restore(dev, ring->index); > > > > - > > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); > > > > } > > > > > > > > > > but on vq start we do not reinstate it? Does this not mean that > > > notifiers won't work after stop then start? > > > > > > Yes, backend initially work w/o host notifier, request VM driver to > > install notifier if needed - call this function through slave socket. > > I think it's cleaner if qemu handles this itself like it did before, it > knows vm is stopped without getting called. If vhost play as server, there are 2 scenario that remove the notifier: 1. VM suspend: backend still there, it's okay to keep mmap address. 2. vhost backend stopped or process killed: resources from backend should be released. That's why patch 2/2 munmap in notifier remove function. Then the restore function get nothing to restore, maybe I shouldn't reverse patch order. > > > > > > > > > > > @@ -1538,7 +1522,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > > > > } > > > > > > > > n->addr = addr; > > > > - n->set = true; > > > > > > > > return 0; > > > > } > > > > diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h > > > > index a9abca3288..f6012b2078 100644 > > > > --- a/include/hw/virtio/vhost-user.h > > > > +++ b/include/hw/virtio/vhost-user.h > > > > @@ -14,7 +14,6 @@ > > > > typedef struct VhostUserHostNotifier { > > > > MemoryRegion mr; > > > > void *addr; > > > > - bool set; > > > > } VhostUserHostNotifier; > > > > > > > > typedef struct VhostUserState { > > > > -- > > > > 2.33.0 > > > > > >
On Wed, Nov 03, 2021 at 02:48:41PM +0000, Xueming(Steven) Li wrote: > On Tue, 2021-11-02 at 02:49 -0400, Michael S. Tsirkin wrote: > > On Tue, Nov 02, 2021 at 06:08:39AM +0000, Xueming(Steven) Li wrote: > > > On Mon, 2021-11-01 at 17:06 -0400, Michael S. Tsirkin wrote: > > > > On Mon, Nov 01, 2021 at 04:38:12PM +0800, Xueming Li wrote: > > > > > When vhost-user vdpa client suspend, backend may close all resources, > > > > > VQ notifier mmap address become invalid, restore MR which contains > > > > > the invalid address is wrong. vdpa client will set VQ notifier after > > > > > reconnect. > > > > > > > > > > This patch removes VQ notifier restore and related flags to avoid reusing > > > > > invalid address. > > > > > > > > > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > > > > > Cc: qemu-stable@nongnu.org > > > > > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > > > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > > > > --- > > > > > hw/virtio/vhost-user.c | 19 +------------------ > > > > > include/hw/virtio/vhost-user.h | 1 - > > > > > 2 files changed, 1 insertion(+), 19 deletions(-) > > > > > > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > > > > index bf6e50223c..c671719e9b 100644 > > > > > --- a/hw/virtio/vhost-user.c > > > > > +++ b/hw/virtio/vhost-user.c > > > > > @@ -1143,19 +1143,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, > > > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); > > > > > } > > > > > > > > > > -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, > > > > > - int queue_idx) > > > > > -{ > > > > > - struct vhost_user *u = dev->opaque; > > > > > - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > > > - VirtIODevice *vdev = dev->vdev; > > > > > - > > > > > - if (n->addr && !n->set) { > > > > > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); > > > > > - n->set = true; > > > > > - } > > > > > -} > > > > > - > > > > > static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > > int queue_idx) > > > > > { > > > > > @@ -1163,17 +1150,14 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > > VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > > > VirtIODevice *vdev = dev->vdev; > > > > > > > > > > - if (n->addr && n->set) { > > > > > + if (n->addr) { > > > > > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > > > > - n->set = false; > > > > > } > > > > > } > > > > > > > > > > > > > So on vq stop we still remove the notifier... > > > > > > > > > static int vhost_user_set_vring_base(struct vhost_dev *dev, > > > > > struct vhost_vring_state *ring) > > > > > { > > > > > - vhost_user_host_notifier_restore(dev, ring->index); > > > > > - > > > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); > > > > > } > > > > > > > > > > > > > but on vq start we do not reinstate it? Does this not mean that > > > > notifiers won't work after stop then start? > > > > > > > > > Yes, backend initially work w/o host notifier, request VM driver to > > > install notifier if needed - call this function through slave socket. > > > > I think it's cleaner if qemu handles this itself like it did before, it > > knows vm is stopped without getting called. > > If vhost play as server, there are 2 scenario that remove the notifier: > 1. VM suspend: backend still there, it's okay to keep mmap address. > 2. vhost backend stopped or process killed: resources from backend > should be released. That's why patch 2/2 munmap in notifier remove > function. Then the restore function get nothing to restore, maybe I > shouldn't reverse patch order. I can't say I understand what you mean here. Do you plan to change the patchset in some way? When you do, pls include a cover letter with a changelog and Cc all people you include on patches on the cover letter too. > > > > > > > > > > > > > > > @@ -1538,7 +1522,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > > > > > } > > > > > > > > > > n->addr = addr; > > > > > - n->set = true; > > > > > > > > > > return 0; > > > > > } > > > > > diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h > > > > > index a9abca3288..f6012b2078 100644 > > > > > --- a/include/hw/virtio/vhost-user.h > > > > > +++ b/include/hw/virtio/vhost-user.h > > > > > @@ -14,7 +14,6 @@ > > > > > typedef struct VhostUserHostNotifier { > > > > > MemoryRegion mr; > > > > > void *addr; > > > > > - bool set; > > > > > } VhostUserHostNotifier; > > > > > > > > > > typedef struct VhostUserState { > > > > > -- > > > > > 2.33.0 > > > > > > > > > >
On Wed, 2021-11-03 at 16:30 -0400, Michael S. Tsirkin wrote: > On Wed, Nov 03, 2021 at 02:48:41PM +0000, Xueming(Steven) Li wrote: > > On Tue, 2021-11-02 at 02:49 -0400, Michael S. Tsirkin wrote: > > > On Tue, Nov 02, 2021 at 06:08:39AM +0000, Xueming(Steven) Li wrote: > > > > On Mon, 2021-11-01 at 17:06 -0400, Michael S. Tsirkin wrote: > > > > > On Mon, Nov 01, 2021 at 04:38:12PM +0800, Xueming Li wrote: > > > > > > When vhost-user vdpa client suspend, backend may close all resources, > > > > > > VQ notifier mmap address become invalid, restore MR which contains > > > > > > the invalid address is wrong. vdpa client will set VQ notifier after > > > > > > reconnect. > > > > > > > > > > > > This patch removes VQ notifier restore and related flags to avoid reusing > > > > > > invalid address. > > > > > > > > > > > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > > > > > > Cc: qemu-stable@nongnu.org > > > > > > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > > > > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > > > > > --- > > > > > > hw/virtio/vhost-user.c | 19 +------------------ > > > > > > include/hw/virtio/vhost-user.h | 1 - > > > > > > 2 files changed, 1 insertion(+), 19 deletions(-) > > > > > > > > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > > > > > index bf6e50223c..c671719e9b 100644 > > > > > > --- a/hw/virtio/vhost-user.c > > > > > > +++ b/hw/virtio/vhost-user.c > > > > > > @@ -1143,19 +1143,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, > > > > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); > > > > > > } > > > > > > > > > > > > -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, > > > > > > - int queue_idx) > > > > > > -{ > > > > > > - struct vhost_user *u = dev->opaque; > > > > > > - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > > > > - VirtIODevice *vdev = dev->vdev; > > > > > > - > > > > > > - if (n->addr && !n->set) { > > > > > > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); > > > > > > - n->set = true; > > > > > > - } > > > > > > -} > > > > > > - > > > > > > static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > > > int queue_idx) > > > > > > { > > > > > > @@ -1163,17 +1150,14 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > > > VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > > > > VirtIODevice *vdev = dev->vdev; > > > > > > > > > > > > - if (n->addr && n->set) { > > > > > > + if (n->addr) { > > > > > > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > > > > > - n->set = false; > > > > > > } > > > > > > } > > > > > > > > > > > > > > > > So on vq stop we still remove the notifier... > > > > > > > > > > > static int vhost_user_set_vring_base(struct vhost_dev *dev, > > > > > > struct vhost_vring_state *ring) > > > > > > { > > > > > > - vhost_user_host_notifier_restore(dev, ring->index); > > > > > > - > > > > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); > > > > > > } > > > > > > > > > > > > > > > > but on vq start we do not reinstate it? Does this not mean that > > > > > notifiers won't work after stop then start? > > > > > > > > > > > > Yes, backend initially work w/o host notifier, request VM driver to > > > > install notifier if needed - call this function through slave socket. > > > > > > I think it's cleaner if qemu handles this itself like it did before, it > > > knows vm is stopped without getting called. > > > > If vhost play as server, there are 2 scenario that remove the notifier: > > 1. VM suspend: backend still there, it's okay to keep mmap address. > > 2. vhost backend stopped or process killed: resources from backend > > should be released. That's why patch 2/2 munmap in notifier remove > > function. Then the restore function get nothing to restore, maybe I > > shouldn't reverse patch order. > > I can't say I understand what you mean here. Do you plan to change > the patchset in some way? > When you do, pls include a cover letter with a changelog and > Cc all people you include on patches on the cover letter too. Here is the detail of the problem I encountered, my vhost backend is DPDK vdpa user space application. Notifier address is set when vdpa ask qemu to mmap a FD and an offset from vdpa, see function vhost_user_slave_handle_vring_host_notifier(). If the vdpa application restart of get killed for some reason, vhost_user_host_notifier_remove() is called and notifier MR is uninstalled, the notifier address that retrieved from mmap is referencing to an invalid FD, not released. This will cause HW resources on kernel side still referenced, most important, when vdpa connection restored, this invalid notifier will be be restored as notifier MR. To resolve it, have to remove the notifer restore mechanism, vDPA application will issue client socket request again to install notifier to VM, so no concern that the notifier will be lost after resume. Since vdpa might be killed, no chance to notify qemu to remove notifier. An alternative solution is to detect sock disconnection and unmmap notifier, but it looks more complex to me. How do you think? > > > > > > > > > > > > > > > > > > > > @@ -1538,7 +1522,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > > > > > > } > > > > > > > > > > > > n->addr = addr; > > > > > > - n->set = true; > > > > > > > > > > > > return 0; > > > > > > } > > > > > > diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h > > > > > > index a9abca3288..f6012b2078 100644 > > > > > > --- a/include/hw/virtio/vhost-user.h > > > > > > +++ b/include/hw/virtio/vhost-user.h > > > > > > @@ -14,7 +14,6 @@ > > > > > > typedef struct VhostUserHostNotifier { > > > > > > MemoryRegion mr; > > > > > > void *addr; > > > > > > - bool set; > > > > > > } VhostUserHostNotifier; > > > > > > > > > > > > typedef struct VhostUserState { > > > > > > -- > > > > > > 2.33.0 > > > > > > > > > > > > > > >
I dropped this for now as I'm a bit lost with this patchset. Cc Raphael maybe he'll understand it better. On Wed, Jan 12, 2022 at 03:05:15PM +0000, Xueming(Steven) Li wrote: > On Wed, 2021-11-03 at 16:30 -0400, Michael S. Tsirkin wrote: > > On Wed, Nov 03, 2021 at 02:48:41PM +0000, Xueming(Steven) Li wrote: > > > On Tue, 2021-11-02 at 02:49 -0400, Michael S. Tsirkin wrote: > > > > On Tue, Nov 02, 2021 at 06:08:39AM +0000, Xueming(Steven) Li wrote: > > > > > On Mon, 2021-11-01 at 17:06 -0400, Michael S. Tsirkin wrote: > > > > > > On Mon, Nov 01, 2021 at 04:38:12PM +0800, Xueming Li wrote: > > > > > > > When vhost-user vdpa client suspend, backend may close all resources, > > > > > > > VQ notifier mmap address become invalid, restore MR which contains > > > > > > > the invalid address is wrong. vdpa client will set VQ notifier after > > > > > > > reconnect. > > > > > > > > > > > > > > This patch removes VQ notifier restore and related flags to avoid reusing > > > > > > > invalid address. > > > > > > > > > > > > > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > > > > > > > Cc: qemu-stable@nongnu.org > > > > > > > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > > > > > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > > > > > > --- > > > > > > > hw/virtio/vhost-user.c | 19 +------------------ > > > > > > > include/hw/virtio/vhost-user.h | 1 - > > > > > > > 2 files changed, 1 insertion(+), 19 deletions(-) > > > > > > > > > > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > > > > > > index bf6e50223c..c671719e9b 100644 > > > > > > > --- a/hw/virtio/vhost-user.c > > > > > > > +++ b/hw/virtio/vhost-user.c > > > > > > > @@ -1143,19 +1143,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, > > > > > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); > > > > > > > } > > > > > > > > > > > > > > -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, > > > > > > > - int queue_idx) > > > > > > > -{ > > > > > > > - struct vhost_user *u = dev->opaque; > > > > > > > - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > > > > > - VirtIODevice *vdev = dev->vdev; > > > > > > > - > > > > > > > - if (n->addr && !n->set) { > > > > > > > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); > > > > > > > - n->set = true; > > > > > > > - } > > > > > > > -} > > > > > > > - > > > > > > > static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > > > > int queue_idx) > > > > > > > { > > > > > > > @@ -1163,17 +1150,14 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > > > > VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > > > > > VirtIODevice *vdev = dev->vdev; > > > > > > > > > > > > > > - if (n->addr && n->set) { > > > > > > > + if (n->addr) { > > > > > > > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > > > > > > - n->set = false; > > > > > > > } > > > > > > > } > > > > > > > > > > > > > > > > > > > So on vq stop we still remove the notifier... > > > > > > > > > > > > > static int vhost_user_set_vring_base(struct vhost_dev *dev, > > > > > > > struct vhost_vring_state *ring) > > > > > > > { > > > > > > > - vhost_user_host_notifier_restore(dev, ring->index); > > > > > > > - > > > > > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); > > > > > > > } > > > > > > > > > > > > > > > > > > > but on vq start we do not reinstate it? Does this not mean that > > > > > > notifiers won't work after stop then start? > > > > > > > > > > > > > > > Yes, backend initially work w/o host notifier, request VM driver to > > > > > install notifier if needed - call this function through slave socket. > > > > > > > > I think it's cleaner if qemu handles this itself like it did before, it > > > > knows vm is stopped without getting called. > > > > > > If vhost play as server, there are 2 scenario that remove the notifier: > > > 1. VM suspend: backend still there, it's okay to keep mmap address. > > > 2. vhost backend stopped or process killed: resources from backend > > > should be released. That's why patch 2/2 munmap in notifier remove > > > function. Then the restore function get nothing to restore, maybe I > > > shouldn't reverse patch order. > > > > I can't say I understand what you mean here. Do you plan to change > > the patchset in some way? > > When you do, pls include a cover letter with a changelog and > > Cc all people you include on patches on the cover letter too. > > Here is the detail of the problem I encountered, my vhost backend is > DPDK vdpa user space application. Notifier address is set when vdpa ask > qemu to mmap a FD and an offset from vdpa, see function > vhost_user_slave_handle_vring_host_notifier(). If the vdpa application > restart of get killed for some reason, > vhost_user_host_notifier_remove() is called and notifier MR is > uninstalled, the notifier address that retrieved from mmap is > referencing to an invalid FD, not released. This will cause HW > resources on kernel side still referenced, most important, when vdpa > connection restored, this invalid notifier will be be restored as > notifier MR. > > To resolve it, have to remove the notifer restore mechanism, vDPA > application will issue client socket request again to install notifier > to VM, so no concern that the notifier will be lost after resume. > > Since vdpa might be killed, no chance to notify qemu to remove > notifier. An alternative solution is to detect sock disconnection and > unmmap notifier, but it looks more complex to me. How do you think? > > > > > > > > > > > > > > > > > > > > > > > > > > > @@ -1538,7 +1522,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > > > > > > > } > > > > > > > > > > > > > > n->addr = addr; > > > > > > > - n->set = true; > > > > > > > > > > > > > > return 0; > > > > > > > } > > > > > > > diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h > > > > > > > index a9abca3288..f6012b2078 100644 > > > > > > > --- a/include/hw/virtio/vhost-user.h > > > > > > > +++ b/include/hw/virtio/vhost-user.h > > > > > > > @@ -14,7 +14,6 @@ > > > > > > > typedef struct VhostUserHostNotifier { > > > > > > > MemoryRegion mr; > > > > > > > void *addr; > > > > > > > - bool set; > > > > > > > } VhostUserHostNotifier; > > > > > > > > > > > > > > typedef struct VhostUserState { > > > > > > > -- > > > > > > > 2.33.0 > > > > > > > > > > > > > > > > > > > > >
v7 sent with more description, please check.
Thanks,
Xueming
On Fri, 2022-02-04 at 07:25 -0500, Michael S. Tsirkin wrote:
I dropped this for now as I'm a bit lost with this patchset.
Cc Raphael maybe he'll understand it better.
On Wed, Jan 12, 2022 at 03:05:15PM +0000, Xueming(Steven) Li wrote:
On Wed, 2021-11-03 at 16:30 -0400, Michael S. Tsirkin wrote:
On Wed, Nov 03, 2021 at 02:48:41PM +0000, Xueming(Steven) Li wrote:
On Tue, 2021-11-02 at 02:49 -0400, Michael S. Tsirkin wrote:
On Tue, Nov 02, 2021 at 06:08:39AM +0000, Xueming(Steven) Li wrote:
On Mon, 2021-11-01 at 17:06 -0400, Michael S. Tsirkin wrote:
On Mon, Nov 01, 2021 at 04:38:12PM +0800, Xueming Li wrote:
When vhost-user vdpa client suspend, backend may close all resources,
VQ notifier mmap address become invalid, restore MR which contains
the invalid address is wrong. vdpa client will set VQ notifier after
reconnect.
This patch removes VQ notifier restore and related flags to avoid reusing
invalid address.
Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers")
Cc: qemu-stable@nongnu.org<mailto:qemu-stable@nongnu.org>
Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com<mailto:zhangyuwei.9149@bytedance.com>>
Signed-off-by: Xueming Li <xuemingl@nvidia.com<mailto:xuemingl@nvidia.com>>
---
hw/virtio/vhost-user.c | 19 +------------------
include/hw/virtio/vhost-user.h | 1 -
2 files changed, 1 insertion(+), 19 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index bf6e50223c..c671719e9b 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1143,19 +1143,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev,
return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring);
}
-static void vhost_user_host_notifier_restore(struct vhost_dev *dev,
- int queue_idx)
-{
- struct vhost_user *u = dev->opaque;
- VhostUserHostNotifier *n = &u->user->notifier[queue_idx];
- VirtIODevice *vdev = dev->vdev;
-
- if (n->addr && !n->set) {
- virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true);
- n->set = true;
- }
-}
-
static void vhost_user_host_notifier_remove(struct vhost_dev *dev,
int queue_idx)
{
@@ -1163,17 +1150,14 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev,
VhostUserHostNotifier *n = &u->user->notifier[queue_idx];
VirtIODevice *vdev = dev->vdev;
- if (n->addr && n->set) {
+ if (n->addr) {
virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false);
- n->set = false;
}
}
So on vq stop we still remove the notifier...
static int vhost_user_set_vring_base(struct vhost_dev *dev,
struct vhost_vring_state *ring)
{
- vhost_user_host_notifier_restore(dev, ring->index);
-
return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring);
}
but on vq start we do not reinstate it? Does this not mean that
notifiers won't work after stop then start?
Yes, backend initially work w/o host notifier, request VM driver to
install notifier if needed - call this function through slave socket.
I think it's cleaner if qemu handles this itself like it did before, it
knows vm is stopped without getting called.
If vhost play as server, there are 2 scenario that remove the notifier:
1. VM suspend: backend still there, it's okay to keep mmap address.
2. vhost backend stopped or process killed: resources from backend
should be released. That's why patch 2/2 munmap in notifier remove
function. Then the restore function get nothing to restore, maybe I
shouldn't reverse patch order.
I can't say I understand what you mean here. Do you plan to change
the patchset in some way?
When you do, pls include a cover letter with a changelog and
Cc all people you include on patches on the cover letter too.
Here is the detail of the problem I encountered, my vhost backend is
DPDK vdpa user space application. Notifier address is set when vdpa ask
qemu to mmap a FD and an offset from vdpa, see function
vhost_user_slave_handle_vring_host_notifier(). If the vdpa application
restart of get killed for some reason,
vhost_user_host_notifier_remove() is called and notifier MR is
uninstalled, the notifier address that retrieved from mmap is
referencing to an invalid FD, not released. This will cause HW
resources on kernel side still referenced, most important, when vdpa
connection restored, this invalid notifier will be be restored as
notifier MR.
To resolve it, have to remove the notifer restore mechanism, vDPA
application will issue client socket request again to install notifier
to VM, so no concern that the notifier will be lost after resume.
Since vdpa might be killed, no chance to notify qemu to remove
notifier. An alternative solution is to detect sock disconnection and
unmmap notifier, but it looks more complex to me. How do you think?
@@ -1538,7 +1522,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev,
}
n->addr = addr;
- n->set = true;
return 0;
}
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index a9abca3288..f6012b2078 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -14,7 +14,6 @@
typedef struct VhostUserHostNotifier {
MemoryRegion mr;
void *addr;
- bool set;
} VhostUserHostNotifier;
typedef struct VhostUserState {
--
2.33.0
v7 sent with more description, please check. Thanks, Xueming On Fri, 2022-02-04 at 07:25 -0500, Michael S. Tsirkin wrote: > I dropped this for now as I'm a bit lost with this patchset. > Cc Raphael maybe he'll understand it better. > > On Wed, Jan 12, 2022 at 03:05:15PM +0000, Xueming(Steven) Li wrote: > > On Wed, 2021-11-03 at 16:30 -0400, Michael S. Tsirkin wrote: > > > On Wed, Nov 03, 2021 at 02:48:41PM +0000, Xueming(Steven) Li wrote: > > > > On Tue, 2021-11-02 at 02:49 -0400, Michael S. Tsirkin wrote: > > > > > On Tue, Nov 02, 2021 at 06:08:39AM +0000, Xueming(Steven) Li wrote: > > > > > > On Mon, 2021-11-01 at 17:06 -0400, Michael S. Tsirkin wrote: > > > > > > > On Mon, Nov 01, 2021 at 04:38:12PM +0800, Xueming Li wrote: > > > > > > > > When vhost-user vdpa client suspend, backend may close all resources, > > > > > > > > VQ notifier mmap address become invalid, restore MR which contains > > > > > > > > the invalid address is wrong. vdpa client will set VQ notifier after > > > > > > > > reconnect. > > > > > > > > > > > > > > > > This patch removes VQ notifier restore and related flags to avoid reusing > > > > > > > > invalid address. > > > > > > > > > > > > > > > > Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") > > > > > > > > Cc: qemu-stable@nongnu.org > > > > > > > > Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> > > > > > > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > > > > > > > --- > > > > > > > > hw/virtio/vhost-user.c | 19 +------------------ > > > > > > > > include/hw/virtio/vhost-user.h | 1 - > > > > > > > > 2 files changed, 1 insertion(+), 19 deletions(-) > > > > > > > > > > > > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > > > > > > > index bf6e50223c..c671719e9b 100644 > > > > > > > > --- a/hw/virtio/vhost-user.c > > > > > > > > +++ b/hw/virtio/vhost-user.c > > > > > > > > @@ -1143,19 +1143,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, > > > > > > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); > > > > > > > > } > > > > > > > > > > > > > > > > -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, > > > > > > > > - int queue_idx) > > > > > > > > -{ > > > > > > > > - struct vhost_user *u = dev->opaque; > > > > > > > > - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > > > > > > - VirtIODevice *vdev = dev->vdev; > > > > > > > > - > > > > > > > > - if (n->addr && !n->set) { > > > > > > > > - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); > > > > > > > > - n->set = true; > > > > > > > > - } > > > > > > > > -} > > > > > > > > - > > > > > > > > static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > > > > > int queue_idx) > > > > > > > > { > > > > > > > > @@ -1163,17 +1150,14 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, > > > > > > > > VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; > > > > > > > > VirtIODevice *vdev = dev->vdev; > > > > > > > > > > > > > > > > - if (n->addr && n->set) { > > > > > > > > + if (n->addr) { > > > > > > > > virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); > > > > > > > > - n->set = false; > > > > > > > > } > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > So on vq stop we still remove the notifier... > > > > > > > > > > > > > > > static int vhost_user_set_vring_base(struct vhost_dev *dev, > > > > > > > > struct vhost_vring_state *ring) > > > > > > > > { > > > > > > > > - vhost_user_host_notifier_restore(dev, ring->index); > > > > > > > > - > > > > > > > > return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > but on vq start we do not reinstate it? Does this not mean that > > > > > > > notifiers won't work after stop then start? > > > > > > > > > > > > > > > > > > Yes, backend initially work w/o host notifier, request VM driver to > > > > > > install notifier if needed - call this function through slave socket. > > > > > > > > > > I think it's cleaner if qemu handles this itself like it did before, it > > > > > knows vm is stopped without getting called. > > > > > > > > If vhost play as server, there are 2 scenario that remove the notifier: > > > > 1. VM suspend: backend still there, it's okay to keep mmap address. > > > > 2. vhost backend stopped or process killed: resources from backend > > > > should be released. That's why patch 2/2 munmap in notifier remove > > > > function. Then the restore function get nothing to restore, maybe I > > > > shouldn't reverse patch order. > > > > > > I can't say I understand what you mean here. Do you plan to change > > > the patchset in some way? > > > When you do, pls include a cover letter with a changelog and > > > Cc all people you include on patches on the cover letter too. > > > > Here is the detail of the problem I encountered, my vhost backend is > > DPDK vdpa user space application. Notifier address is set when vdpa ask > > qemu to mmap a FD and an offset from vdpa, see function > > vhost_user_slave_handle_vring_host_notifier(). If the vdpa application > > restart of get killed for some reason, > > vhost_user_host_notifier_remove() is called and notifier MR is > > uninstalled, the notifier address that retrieved from mmap is > > referencing to an invalid FD, not released. This will cause HW > > resources on kernel side still referenced, most important, when vdpa > > connection restored, this invalid notifier will be be restored as > > notifier MR. > > > > To resolve it, have to remove the notifer restore mechanism, vDPA > > application will issue client socket request again to install notifier > > to VM, so no concern that the notifier will be lost after resume. > > > > Since vdpa might be killed, no chance to notify qemu to remove > > notifier. An alternative solution is to detect sock disconnection and > > unmmap notifier, but it looks more complex to me. How do you think? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > @@ -1538,7 +1522,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, > > > > > > > > } > > > > > > > > > > > > > > > > n->addr = addr; > > > > > > > > - n->set = true; > > > > > > > > > > > > > > > > return 0; > > > > > > > > } > > > > > > > > diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h > > > > > > > > index a9abca3288..f6012b2078 100644 > > > > > > > > --- a/include/hw/virtio/vhost-user.h > > > > > > > > +++ b/include/hw/virtio/vhost-user.h > > > > > > > > @@ -14,7 +14,6 @@ > > > > > > > > typedef struct VhostUserHostNotifier { > > > > > > > > MemoryRegion mr; > > > > > > > > void *addr; > > > > > > > > - bool set; > > > > > > > > } VhostUserHostNotifier; > > > > > > > > > > > > > > > > typedef struct VhostUserState { > > > > > > > > -- > > > > > > > > 2.33.0 > > > > > > > > > > > > > > > > > > > > > > > > > > > >
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index bf6e50223c..c671719e9b 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -1143,19 +1143,6 @@ static int vhost_user_set_vring_num(struct vhost_dev *dev, return vhost_set_vring(dev, VHOST_USER_SET_VRING_NUM, ring); } -static void vhost_user_host_notifier_restore(struct vhost_dev *dev, - int queue_idx) -{ - struct vhost_user *u = dev->opaque; - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; - VirtIODevice *vdev = dev->vdev; - - if (n->addr && !n->set) { - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true); - n->set = true; - } -} - static void vhost_user_host_notifier_remove(struct vhost_dev *dev, int queue_idx) { @@ -1163,17 +1150,14 @@ static void vhost_user_host_notifier_remove(struct vhost_dev *dev, VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; VirtIODevice *vdev = dev->vdev; - if (n->addr && n->set) { + if (n->addr) { virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); - n->set = false; } } static int vhost_user_set_vring_base(struct vhost_dev *dev, struct vhost_vring_state *ring) { - vhost_user_host_notifier_restore(dev, ring->index); - return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring); } @@ -1538,7 +1522,6 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, } n->addr = addr; - n->set = true; return 0; } diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h index a9abca3288..f6012b2078 100644 --- a/include/hw/virtio/vhost-user.h +++ b/include/hw/virtio/vhost-user.h @@ -14,7 +14,6 @@ typedef struct VhostUserHostNotifier { MemoryRegion mr; void *addr; - bool set; } VhostUserHostNotifier; typedef struct VhostUserState {
When vhost-user vdpa client suspend, backend may close all resources, VQ notifier mmap address become invalid, restore MR which contains the invalid address is wrong. vdpa client will set VQ notifier after reconnect. This patch removes VQ notifier restore and related flags to avoid reusing invalid address. Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") Cc: qemu-stable@nongnu.org Cc: Yuwei Zhang <zhangyuwei.9149@bytedance.com> Signed-off-by: Xueming Li <xuemingl@nvidia.com> --- hw/virtio/vhost-user.c | 19 +------------------ include/hw/virtio/vhost-user.h | 1 - 2 files changed, 1 insertion(+), 19 deletions(-)