[PATCH v3 0/3] vhost-user-gpu: Add blob resource and shared memory support

[PATCH v3 0/3] vhost-user-gpu: Add blob resource and shared memory support

[Dorinda Bassey]

Add blob resource support to the vhost-user-gpu device, enabling
Vulkan pass-through via Venus with a vhost-user backend such as
vhost-device-gpu.

Patch 1 fixes a deadlock that occurs when the memory region
transaction commit triggers ADD_MEM_REG back to the backend while
it is still blocked waiting for the SHMEM_MAP reply.

Patches 2-3 register a shared memory region for SHMEM_MAP/UNMAP
operations and forward the RESOURCE_BLOB and CONTEXT_INIT feature
flags from the backend to the guest.

Dorinda Bassey (1):
  vhost: Fix SHMEM_MAP transaction ordering and resulting deadlock

Matej Hrica (2):
  vhost-user-gpu: Add shared memory region support
  vhost-user-gpu: Forward RESOURCE_BLOB and CONTEXT_INIT flags

 hw/display/vhost-user-gpu.c | 17 +++++++++++++++++
 hw/virtio/vhost-user.c      | 20 ++------------------
 hw/virtio/vhost.c           | 10 ++++++++++
 3 files changed, 29 insertions(+), 18 deletions(-)

Changes in v3:
- Reordered patches: bugfix first, then features
- Added Fixes tag

Changes in v2:
- Reordered patches: shmem support before feature flags
- Use virtio_new_shmem_region() helper
- Drop explicit transaction begin/commit wrapping

--
2.52.0

[PATCH v3 1/3] vhost: Fix SHMEM_MAP transaction ordering and resulting deadlock

[Dorinda Bassey]

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().

Fixes: b52e1896e764 ("vhost-user: Add VirtIO Shared Memory map request")
Suggested-by: Albert Esteve <aesteve@redhat.com>
Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
---
 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

[PATCH v3 2/3] vhost-user-gpu: Add shared memory region support

[Dorinda Bassey]

From: Matej Hrica <mhrica@redhat.com>

Add a 'hostmem' property to the vhost-user-gpu which enables the shared
memory in the parent object.

Register a container MR in the shmem_list for SHMEM_MAP/UNMAP
operations. A container MR is used so that SHMEM_MAP FD-backed
subregions each get their own KVM memory slot with the correct
backing fd.

Suggested-by: Albert Esteve <aesteve@redhat.com>
Signed-off-by: Matej Hrica <mhrica@redhat.com>
Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
---
 hw/display/vhost-user-gpu.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index 57360898ca0..dbba80c7783 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -649,6 +649,16 @@ vhost_user_gpu_device_realize(DeviceState *qdev, Error **errp)
     }
 
     g->vhost_gpu_fd = -1;
+
+    /*
+     * If shared memory is enabled, register the hostmem region for
+     * SHMEM_MAP/UNMAP operations.
+     */
+    if (virtio_gpu_hostmem_enabled(g->parent_obj.conf)) {
+        VirtioSharedMemory *shmem = virtio_new_shmem_region(
+            vdev, VIRTIO_GPU_SHM_ID_HOST_VISIBLE, g->parent_obj.conf.hostmem);
+        memory_region_add_subregion(&g->parent_obj.hostmem, 0, &shmem->mr);
+    }
 }
 
 static struct vhost_dev *vhost_user_gpu_get_vhost(VirtIODevice *vdev)
@@ -659,6 +669,7 @@ static struct vhost_dev *vhost_user_gpu_get_vhost(VirtIODevice *vdev)
 
 static const Property vhost_user_gpu_properties[] = {
     VIRTIO_GPU_BASE_PROPERTIES(VhostUserGPU, parent_obj.conf),
+    DEFINE_PROP_SIZE("hostmem", VhostUserGPU, parent_obj.conf.hostmem, 0),
 };
 
 static void
-- 
2.52.0

[PATCH v3 3/3] vhost-user-gpu: Forward RESOURCE_BLOB and CONTEXT_INIT flags

[Dorinda Bassey]

From: Matej Hrica <mhrica@redhat.com>

Forward VIRTIO_GPU_F_RESOURCE_BLOB and VIRTIO_GPU_F_CONTEXT_INIT
received from a vhost backend to expose it to the guest.

Use vhost_dev_has_feature() to query the backend features, matching
the existing pattern for RESOURCE_UUID.

This allows the guest to negotiate blob resource support and context
initialization when the backend (e.g., vhost-device-gpu with Venus)
advertises these capabilities.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Signed-off-by: Matej Hrica <mhrica@redhat.com>
Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
---
 hw/display/vhost-user-gpu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index dbba80c7783..4022a3026ca 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -643,6 +643,12 @@ vhost_user_gpu_device_realize(DeviceState *qdev, Error **errp)
     if (vhost_dev_has_feature(&g->vhost->dev, VIRTIO_GPU_F_RESOURCE_UUID)) {
         g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_RESOURCE_UUID_ENABLED;
     }
+    if (vhost_dev_has_feature(&g->vhost->dev, VIRTIO_GPU_F_RESOURCE_BLOB)) {
+        g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_BLOB_ENABLED;
+    }
+    if (vhost_dev_has_feature(&g->vhost->dev, VIRTIO_GPU_F_CONTEXT_INIT)) {
+        g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED;
+    }
 
     if (!virtio_gpu_base_device_realize(qdev, NULL, NULL, errp)) {
         return;
-- 
2.52.0

Re: [PATCH v3 1/3] vhost: Fix SHMEM_MAP transaction ordering and resulting deadlock

[Michael S. Tsirkin]

On Fri, Jul 17, 2026 at 12:32:59PM +0200, Dorinda Bassey 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().
> 
> Fixes: b52e1896e764 ("vhost-user: Add VirtIO Shared Memory map request")
> Suggested-by: Albert Esteve <aesteve@redhat.com>
> Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
> ---
>  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;


This all seems risky i have to say.  Is this always map never unmap?

> -- 
> 2.52.0

Re: [PATCH v3 1/3] vhost: Fix SHMEM_MAP transaction ordering and resulting deadlock

[Albert Esteve]

On Fri, Jul 17, 2026 at 12:33 PM Dorinda Bassey <dbassey@redhat.com> 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().
>
> Fixes: b52e1896e764 ("vhost-user: Add VirtIO Shared Memory map request")
> Suggested-by: Albert Esteve <aesteve@redhat.com>
> Signed-off-by: Dorinda Bassey <dbassey@redhat.com>

I tested this patch on top of [1] with the rust-vmm backend [2] as
described in the linked series cover letter.

Tested-by: Albert Esteve <aesteve@redhat.com>
Reviewed-by: Albert Esteve <aesteve@redhat.com>

[1] https://lore.kernel.org/all/20260630112310.552606-1-aesteve@redhat.com/
[2] https://github.com/rust-vmm/vhost-device/tree/main/vhost-device-media

> ---
>  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
>

Re: [PATCH v3 2/3] vhost-user-gpu: Add shared memory region support

[Albert Esteve]

On Fri, Jul 17, 2026 at 12:33 PM Dorinda Bassey <dbassey@redhat.com> wrote:
>
> From: Matej Hrica <mhrica@redhat.com>
>
> Add a 'hostmem' property to the vhost-user-gpu which enables the shared
> memory in the parent object.
>
> Register a container MR in the shmem_list for SHMEM_MAP/UNMAP
> operations. A container MR is used so that SHMEM_MAP FD-backed
> subregions each get their own KVM memory slot with the correct
> backing fd.
>
> Suggested-by: Albert Esteve <aesteve@redhat.com>
> Signed-off-by: Matej Hrica <mhrica@redhat.com>
> Signed-off-by: Dorinda Bassey <dbassey@redhat.com>

Reviewed-by: Albert Esteve <aesteve@redhat.com>

Thanks!

> ---
>  hw/display/vhost-user-gpu.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
> index 57360898ca0..dbba80c7783 100644
> --- a/hw/display/vhost-user-gpu.c
> +++ b/hw/display/vhost-user-gpu.c
> @@ -649,6 +649,16 @@ vhost_user_gpu_device_realize(DeviceState *qdev, Error **errp)
>      }
>
>      g->vhost_gpu_fd = -1;
> +
> +    /*
> +     * If shared memory is enabled, register the hostmem region for
> +     * SHMEM_MAP/UNMAP operations.
> +     */
> +    if (virtio_gpu_hostmem_enabled(g->parent_obj.conf)) {
> +        VirtioSharedMemory *shmem = virtio_new_shmem_region(
> +            vdev, VIRTIO_GPU_SHM_ID_HOST_VISIBLE, g->parent_obj.conf.hostmem);
> +        memory_region_add_subregion(&g->parent_obj.hostmem, 0, &shmem->mr);
> +    }
>  }
>
>  static struct vhost_dev *vhost_user_gpu_get_vhost(VirtIODevice *vdev)
> @@ -659,6 +669,7 @@ static struct vhost_dev *vhost_user_gpu_get_vhost(VirtIODevice *vdev)
>
>  static const Property vhost_user_gpu_properties[] = {
>      VIRTIO_GPU_BASE_PROPERTIES(VhostUserGPU, parent_obj.conf),
> +    DEFINE_PROP_SIZE("hostmem", VhostUserGPU, parent_obj.conf.hostmem, 0),
>  };
>
>  static void
> --
> 2.52.0
>

Re: [PATCH v3 1/3] vhost: Fix SHMEM_MAP transaction ordering and resulting deadlock

[Albert Esteve]

On Fri, Jul 17, 2026 at 12:37 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Fri, Jul 17, 2026 at 12:32:59PM +0200, Dorinda Bassey 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().
> >
> > Fixes: b52e1896e764 ("vhost-user: Add VirtIO Shared Memory map request")
> > Suggested-by: Albert Esteve <aesteve@redhat.com>
> > Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
> > ---
> >  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;
>
>
> This all seems risky i have to say.  Is this always map never unmap?

The filter covers both, and this is a good point. For symmetry the
handle_shmem_unmap() should call virtio_del_shmem_map() before
replying.

BR,
Albert.

>
> > --
> > 2.52.0
>

Re: [PATCH v3 1/3] vhost: Fix SHMEM_MAP transaction ordering and resulting deadlock

[Dorinda Bassey]

[-- Attachment #1: Type: text/plain, Size: 4468 bytes --]

Good point, will move virtio_del_shmem_map() before the reply in the next
version.

On Fri, Jul 17, 2026 at 1:50 PM Albert Esteve <aesteve@redhat.com> wrote:

> On Fri, Jul 17, 2026 at 12:37 PM Michael S. Tsirkin <mst@redhat.com>
> wrote:
> >
> > On Fri, Jul 17, 2026 at 12:32:59PM +0200, Dorinda Bassey 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().
> > >
> > > Fixes: b52e1896e764 ("vhost-user: Add VirtIO Shared Memory map
> request")
> > > Suggested-by: Albert Esteve <aesteve@redhat.com>
> > > Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
> > > ---
> > >  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;
> >
> >
> > This all seems risky i have to say.  Is this always map never unmap?
>
> The filter covers both, and this is a good point. For symmetry the
> handle_shmem_unmap() should call virtio_del_shmem_map() before
> replying.
>
> BR,
> Albert.
>
> >
> > > --
> > > 2.52.0
> >
>
>

[-- Attachment #2: Type: text/html, Size: 5938 bytes --]

Re: [PATCH v3 1/3] vhost: Fix SHMEM_MAP transaction ordering and resulting deadlock

[Michael S. Tsirkin]

On Fri, Jul 17, 2026 at 01:50:31PM +0200, Albert Esteve wrote:
> On Fri, Jul 17, 2026 at 12:37 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Fri, Jul 17, 2026 at 12:32:59PM +0200, Dorinda Bassey 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().
> > >
> > > Fixes: b52e1896e764 ("vhost-user: Add VirtIO Shared Memory map request")
> > > Suggested-by: Albert Esteve <aesteve@redhat.com>
> > > Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
> > > ---
> > >  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;
> >
> >
> > This all seems risky i have to say.  Is this always map never unmap?
> 
> The filter covers both, and this is a good point. For symmetry the
> handle_shmem_unmap() should call virtio_del_shmem_map() before
> replying.
> 
> BR,
> Albert.

the reverse order I would say?

if we add, first we commit then update the backend

if we delete first update the backend then commit?



> >
> > > --
> > > 2.52.0
> >

Re: [PATCH v3 1/3] vhost: Fix SHMEM_MAP transaction ordering and resulting deadlock

[Albert Esteve]

On Fri, Jul 17, 2026 at 2:58 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Fri, Jul 17, 2026 at 01:50:31PM +0200, Albert Esteve wrote:
> > On Fri, Jul 17, 2026 at 12:37 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Fri, Jul 17, 2026 at 12:32:59PM +0200, Dorinda Bassey 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().
> > > >
> > > > Fixes: b52e1896e764 ("vhost-user: Add VirtIO Shared Memory map request")
> > > > Suggested-by: Albert Esteve <aesteve@redhat.com>
> > > > Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
> > > > ---
> > > >  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;
> > >
> > >
> > > This all seems risky i have to say.  Is this always map never unmap?
> >
> > The filter covers both, and this is a good point. For symmetry the
> > handle_shmem_unmap() should call virtio_del_shmem_map() before
> > replying.
> >
> > BR,
> > Albert.
>
> the reverse order I would say?
>
> if we add, first we commit then update the backend
>
> if we delete first update the backend then commit?

I think it could be argued both ways, so I'd be ok with what you
described (which is what's done in current mainline for the unmap
side).

The reason the commit (the whole del_shmem_map() op) was delayed
before on unmap was to avoid the deadlock (the same as for map).

With the filter, that is no longer a problem. So I would say that if
the backend sends a SHMEM_UNMAP request, by the time it receives the
reply the unmap operation is already commited. It's more symmetrical:
the frontend only replies when the requested memory operation (map or
unmap) is finished and committed.

>
>
>
> > >
> > > > --
> > > > 2.52.0
> > >
>

Re: [PATCH v3 1/3] vhost: Fix SHMEM_MAP transaction ordering and resulting deadlock

[Dorinda Bassey]

[-- Attachment #1: Type: text/plain, Size: 5870 bytes --]

So I will keep the current mainline ordering for unmap (reply before
delete) as Michael suggested.

On Fri, Jul 17, 2026 at 3:11 PM Albert Esteve <aesteve@redhat.com> wrote:

> On Fri, Jul 17, 2026 at 2:58 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Fri, Jul 17, 2026 at 01:50:31PM +0200, Albert Esteve wrote:
> > > On Fri, Jul 17, 2026 at 12:37 PM Michael S. Tsirkin <mst@redhat.com>
> wrote:
> > > >
> > > > On Fri, Jul 17, 2026 at 12:32:59PM +0200, Dorinda Bassey 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().
> > > > >
> > > > > Fixes: b52e1896e764 ("vhost-user: Add VirtIO Shared Memory map
> request")
> > > > > Suggested-by: Albert Esteve <aesteve@redhat.com>
> > > > > Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
> > > > > ---
> > > > >  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;
> > > >
> > > >
> > > > This all seems risky i have to say.  Is this always map never unmap?
> > >
> > > The filter covers both, and this is a good point. For symmetry the
> > > handle_shmem_unmap() should call virtio_del_shmem_map() before
> > > replying.
> > >
> > > BR,
> > > Albert.
> >
> > the reverse order I would say?
> >
> > if we add, first we commit then update the backend
> >
> > if we delete first update the backend then commit?
>
> I think it could be argued both ways, so I'd be ok with what you
> described (which is what's done in current mainline for the unmap
> side).
>
> The reason the commit (the whole del_shmem_map() op) was delayed
> before on unmap was to avoid the deadlock (the same as for map).
>
> With the filter, that is no longer a problem. So I would say that if
> the backend sends a SHMEM_UNMAP request, by the time it receives the
> reply the unmap operation is already commited. It's more symmetrical:
> the frontend only replies when the requested memory operation (map or
> unmap) is finished and committed.
>
> >
> >
> >
> > > >
> > > > > --
> > > > > 2.52.0
> > > >
> >
>
>

[-- Attachment #2: Type: text/html, Size: 8097 bytes --]

Re: [PATCH v3 1/3] vhost: Fix SHMEM_MAP transaction ordering and resulting deadlock

[Michael S. Tsirkin]

On Fri, Jul 17, 2026 at 03:11:12PM +0200, Albert Esteve wrote:
> On Fri, Jul 17, 2026 at 2:58 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Fri, Jul 17, 2026 at 01:50:31PM +0200, Albert Esteve wrote:
> > > On Fri, Jul 17, 2026 at 12:37 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Fri, Jul 17, 2026 at 12:32:59PM +0200, Dorinda Bassey 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().
> > > > >
> > > > > Fixes: b52e1896e764 ("vhost-user: Add VirtIO Shared Memory map request")
> > > > > Suggested-by: Albert Esteve <aesteve@redhat.com>
> > > > > Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
> > > > > ---
> > > > >  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;
> > > >
> > > >
> > > > This all seems risky i have to say.  Is this always map never unmap?
> > >
> > > The filter covers both, and this is a good point. For symmetry the
> > > handle_shmem_unmap() should call virtio_del_shmem_map() before
> > > replying.
> > >
> > > BR,
> > > Albert.
> >
> > the reverse order I would say?
> >
> > if we add, first we commit then update the backend
> >
> > if we delete first update the backend then commit?
> 
> I think it could be argued both ways, so I'd be ok with what you
> described (which is what's done in current mainline for the unmap
> side).
> 
> The reason the commit (the whole del_shmem_map() op) was delayed
> before on unmap was to avoid the deadlock (the same as for map).
> 
> With the filter, that is no longer a problem. So I would say that if
> the backend sends a SHMEM_UNMAP request, by the time it receives the
> reply the unmap operation is already commited. It's more symmetrical:
> the frontend only replies when the requested memory operation (map or
> unmap) is finished and committed.

I don't know, this thing makes my head hurt.
Pls document ordering for both in the commit log.

> >
> >
> >
> > > >
> > > > > --
> > > > > 2.52.0
> > > >
> >