From f06e65cb747cbd32f4d12f1d880625c14a8bd9da Mon Sep 17 00:00:00 2001 From: Xueming Li Date: Mon, 7 Feb 2022 15:19:28 +0800 Subject: [PATCH 1/2] vhost-user: remove VirtQ notifier restore Notifier set when vhost-user backend asks qemu to mmap an FD and offset. When vhost-user backend restart or getting killed, VQ notifier FD and mmap addresses become invalid. After backend restart, MR contains the invalid address will be restored and fail on notifier access. On the other hand, qemu should munmap the notifier, release underlying hardware resources to enable backend restart and allocate hardware notifier resources correctly. Qemu shouldn't reference and use resources of disconnected backend. This patch removes VQ notifier restore, uses the default vhost-user notifier to avoid invalid address access. After backend restart, the backend should ask qemu to install a hardware notifier if needed. Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") Cc: qemu-stable@nongnu.org Signed-off-by: Xueming Li Message-Id: <20220207071929.527149-2-xuemingl@nvidia.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin (cherry picked from commit e867144b73b3c5009266b6df07d5ff44acfb82c3) --- 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 176cae9244..7b21e09723 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -1145,19 +1145,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) { @@ -1165,17 +1152,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); } @@ -1540,7 +1524,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 { -- Gitee From 0d022f741ab510b52453bb9e9e07b968e7d5e0df Mon Sep 17 00:00:00 2001 From: Xueming Li Date: Mon, 7 Feb 2022 15:19:29 +0800 Subject: [PATCH 2/2] vhost-user: fix VirtQ notifier cleanup When vhost-user device cleanup, remove notifier MR and munmaps notifier address in the event-handling thread, VM CPU thread writing the notifier in concurrent fails with an error of accessing invalid address. It happens because MR is still being referenced and accessed in another thread while the underlying notifier mmap address is being freed and becomes invalid. This patch calls RCU and munmap notifiers in the callback after the memory flatview update finish. Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers") Cc: qemu-stable@nongnu.org Signed-off-by: Xueming Li Message-Id: <20220207071929.527149-3-xuemingl@nvidia.com> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin (cherry picked from commit 0b0af4d62f7002b31cd7b2762b26d2fcb76bb2ba) --- hw/virtio/vhost-user.c | 48 ++++++++++++++++++++-------------- include/hw/virtio/vhost-user.h | 2 ++ 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 7b21e09723..42a9e16cd9 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -26,6 +26,7 @@ #include "migration/postcopy-ram.h" #include "migration/register.h" #include "trace.h" +#include "exec/ramblock.h" #include #include @@ -1145,15 +1146,26 @@ 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_remove(struct vhost_dev *dev, - int queue_idx) +static void vhost_user_host_notifier_free(VhostUserHostNotifier *n) { - struct vhost_user *u = dev->opaque; - VhostUserHostNotifier *n = &u->user->notifier[queue_idx]; - VirtIODevice *vdev = dev->vdev; + assert(n && n->unmap_addr); + munmap(n->unmap_addr, qemu_real_host_page_size); + n->unmap_addr = NULL; +} + +static void vhost_user_host_notifier_remove(VhostUserState *user, + VirtIODevice *vdev, int queue_idx) +{ + VhostUserHostNotifier *n = &user->notifier[queue_idx]; if (n->addr) { - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); + if (vdev) { + virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); + } + assert(!n->unmap_addr); + n->unmap_addr = n->addr; + n->addr = NULL; + call_rcu(n, vhost_user_host_notifier_free, rcu); } } @@ -1192,8 +1204,9 @@ static int vhost_user_get_vring_base(struct vhost_dev *dev, .payload.state = *ring, .hdr.size = sizeof(msg.payload.state), }; + struct vhost_user *u = dev->opaque; - vhost_user_host_notifier_remove(dev, ring->index); + vhost_user_host_notifier_remove(u->user, dev->vdev, ring->index); if (vhost_user_write(dev, &msg, NULL, 0) < 0) { return -1; @@ -1488,12 +1501,7 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, n = &user->notifier[queue_idx]; - if (n->addr) { - virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, false); - object_unparent(OBJECT(&n->mr)); - munmap(n->addr, page_size); - n->addr = NULL; - } + vhost_user_host_notifier_remove(user, vdev, queue_idx); if (area->u64 & VHOST_USER_VRING_NOFD_MASK) { return 0; @@ -1512,9 +1520,12 @@ static int vhost_user_slave_handle_vring_host_notifier(struct vhost_dev *dev, name = g_strdup_printf("vhost-user/host-notifier@%p mmaps[%d]", user, queue_idx); - if (!n->mr.ram) /* Don't init again after suspend. */ + if (!n->mr.ram) { /* Don't init again after suspend. */ memory_region_init_ram_device_ptr(&n->mr, OBJECT(vdev), name, page_size, addr); + } else { + n->mr.ram_block->host = addr; + } g_free(name); if (virtio_queue_set_host_notifier_mr(vdev, queue_idx, &n->mr, true)) { @@ -2492,17 +2503,16 @@ bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp) void vhost_user_cleanup(VhostUserState *user) { int i; + VhostUserHostNotifier *n; if (!user->chr) { return; } memory_region_transaction_begin(); for (i = 0; i < VIRTIO_QUEUE_MAX; i++) { - if (user->notifier[i].addr) { - object_unparent(OBJECT(&user->notifier[i].mr)); - munmap(user->notifier[i].addr, qemu_real_host_page_size); - user->notifier[i].addr = NULL; - } + n = &user->notifier[i]; + vhost_user_host_notifier_remove(user, NULL, i); + object_unparent(OBJECT(&n->mr)); } memory_region_transaction_commit(); user->chr = NULL; diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h index f6012b2078..e44a41bb70 100644 --- a/include/hw/virtio/vhost-user.h +++ b/include/hw/virtio/vhost-user.h @@ -12,8 +12,10 @@ #include "hw/virtio/virtio.h" typedef struct VhostUserHostNotifier { + struct rcu_head rcu; MemoryRegion mr; void *addr; + void *unmap_addr; } VhostUserHostNotifier; typedef struct VhostUserState { -- Gitee