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]>
---
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 517cc4ca716..01468f7441a 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1977,8 +1977,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,
@@ -1986,7 +1984,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 */
@@ -1994,22 +1992,8 @@ 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;
- }
-
-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;
- }
+ goto send_reply;
}
- memory_region_transaction_commit();
- return 0;
send_reply:
if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index af41841b529..e5c219be095 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;
--
2.52.0
