From: Dorinda Bassey <[email protected]>
Commit the memory region transaction before sending the SHMEM_MAP reply
so the KVM memory slot exists before the guest can access the mapped
region.
This moves the commit to while the backend is still blocked waiting for
the reply, which can trigger ADD_MEM_REG back to the backend and
deadlock. Filter shmem mapping regions out of vhost_section() by
checking the MR owner type to prevent this.
The explicit transaction begin/commit wrapping is dropped since
virtio_add_shmem_map() already commits via memory_region_add_subregion().
The unmap handler keeps the original ordering: reply before delete, so
the backend is unblocked before the transaction commit.
Fixes: b52e1896e764 ("vhost-user: Add VirtIO Shared Memory map request")
Suggested-by: Albert Esteve <[email protected]>
Reviewed-by: Albert Esteve <[email protected]>
Tested-by: Albert Esteve <[email protected]>
Signed-off-by: Dorinda Bassey <[email protected]>
Reviewed-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
Message-ID: <[email protected]>
---
hw/virtio/vhost-user.c | 20 ++------------------
hw/virtio/vhost.c | 10 ++++++++++
2 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 2881cec72d..fac02a1ffc 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1980,8 +1980,6 @@ vhost_user_backend_handle_shmem_map(struct vhost_dev *dev,
}
}
- memory_region_transaction_begin();
-
/* Create VirtioSharedMemoryMapping object */
VirtioSharedMemoryMapping *mapping = virtio_shared_memory_mapping_new(
vu_mmap->shmid, fd, vu_mmap->fd_offset, vu_mmap->shm_offset,
@@ -1989,7 +1987,7 @@ vhost_user_backend_handle_shmem_map(struct vhost_dev *dev,
if (!mapping) {
ret = -EFAULT;
- goto send_reply_commit;
+ goto send_reply;
}
/* Add the mapping to the shared memory region */
@@ -1997,23 +1995,9 @@ vhost_user_backend_handle_shmem_map(struct vhost_dev
*dev,
error_report("Failed to add shared memory mapping");
object_unref(OBJECT(mapping));
ret = -EFAULT;
- goto send_reply_commit;
+ goto send_reply;
}
-send_reply_commit:
- /* Send reply and commit after transaction started */
- if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
- payload->u64 = !!ret;
- hdr->size = sizeof(payload->u64);
- if (!vhost_user_send_resp(ioc, hdr, payload, &local_err)) {
- error_report_err(local_err);
- memory_region_transaction_commit();
- return -EFAULT;
- }
- }
- memory_region_transaction_commit();
- return 0;
-
send_reply:
if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
payload->u64 = !!ret;
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index a81cd94f90..838b1da739 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -636,6 +636,16 @@ static bool vhost_section(struct vhost_dev *dev,
MemoryRegionSection *section)
{
MemoryRegion *mr = section->mr;
+ /*
+ * Skip shmem mapping regions, they are managed via SHMEM_MAP/UNMAP.
+ * Including them triggers ADD_MEM_REG during the SHMEM_MAP transaction
+ * commit, deadlocking the backend.
+ */
+ if (object_dynamic_cast(memory_region_owner(mr),
+ TYPE_VIRTIO_SHARED_MEMORY_MAPPING)) {
+ return false;
+ }
+
if (memory_region_is_ram(mr) && !memory_region_is_rom(mr)) {
uint8_t dirty_mask = memory_region_get_dirty_log_mask(mr);
uint8_t handled_dirty;
--
MST
