On Fri, Jul 17, 2026 at 3:49 PM Dorinda Bassey <[email protected]> wrote:
>
> 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:
Minor idea for follow-up, not a blocker: with nothing left to do after
the reply, we could drop reply_ack = false and the handler’s
send_reply block, and rely on backend_read()’s default reply handling.
> 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
>
