On Mon, Aug 18, 2025 at 10:41 PM Stefan Hajnoczi <stefa...@redhat.com> wrote:
>
> On Mon, Aug 18, 2025 at 12:03:46PM +0200, Albert Esteve wrote:
>
> (I haven't fully reviewed this yet, but here are my current comments.)
>
> > Add SHMEM_MAP/UNMAP requests to vhost-user for
> > dynamic management of VIRTIO Shared Memory mappings.
> >
> > This implementation introduces VhostUserShmemObject
> > as an intermediate QOM parent for MemoryRegions
> > created for SHMEM_MAP requests. This object
> > provides reference-counted lifecycle management
> > with automatic cleanup.
> >
> > This request allows backends to dynamically map
> > file descriptors into a VIRTIO Shared Memory
> > Regions identified by their shmid. Maps are created
> > using memory_region_init_ram_device_ptr() with
> > configurable read/write permissions, and the resulting
> > MemoryRegions are added as subregions to the shmem
> > container region. The mapped memory is then advertised
> > to the guest VIRTIO drivers as a base address plus
> > offset for reading and writting according
> > to the requested mmap flags.
> >
> > The backend can unmap memory ranges within a given
> > VIRTIO Shared Memory Region to free resources.
> > Upon receiving this message, the frontend removes
> > the MemoryRegion as a subregion and automatically
> > unreferences the associated VhostUserShmemObject,
> > triggering cleanup if no other references exist.
> >
> > Error handling has been improved to ensure consistent
> > behavior across handlers that manage their own
> > vhost_user_send_resp() calls. Since these handlers
> > clear the VHOST_USER_NEED_REPLY_MASK flag, explicit
> > error checking ensures proper connection closure on
> > failures, maintaining the expected error flow.
> >
> > Note the memory region commit for these
> > operations needs to be delayed until after we
> > respond to the backend to avoid deadlocks.
> >
> > Signed-off-by: Albert Esteve <aest...@redhat.com>
> > ---
> > hw/virtio/meson.build | 1 +
> > hw/virtio/vhost-user-shmem.c | 134 ++++++++++++++
> > hw/virtio/vhost-user.c | 207 +++++++++++++++++++++-
> > hw/virtio/virtio.c | 109 ++++++++++++
> > include/hw/virtio/vhost-user-shmem.h | 75 ++++++++
> > include/hw/virtio/virtio.h | 93 ++++++++++
> > subprojects/libvhost-user/libvhost-user.c | 70 ++++++++
> > subprojects/libvhost-user/libvhost-user.h | 54 ++++++
> > 8 files changed, 741 insertions(+), 2 deletions(-)
> > create mode 100644 hw/virtio/vhost-user-shmem.c
> > create mode 100644 include/hw/virtio/vhost-user-shmem.h
> >
> > diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
> > index 3ea7b3cec8..5efcf70b75 100644
> > --- a/hw/virtio/meson.build
> > +++ b/hw/virtio/meson.build
> > @@ -20,6 +20,7 @@ if have_vhost
> > # fixme - this really should be generic
> > specific_virtio_ss.add(files('vhost-user.c'))
> > system_virtio_ss.add(files('vhost-user-base.c'))
> > + system_virtio_ss.add(files('vhost-user-shmem.c'))
> >
> > # MMIO Stubs
> > system_virtio_ss.add(files('vhost-user-device.c'))
> > diff --git a/hw/virtio/vhost-user-shmem.c b/hw/virtio/vhost-user-shmem.c
> > new file mode 100644
> > index 0000000000..1d763b56b6
> > --- /dev/null
> > +++ b/hw/virtio/vhost-user-shmem.c
> > @@ -0,0 +1,134 @@
> > +/*
> > + * VHost-user Shared Memory Object
> > + *
> > + * Copyright Red Hat, Inc. 2025
> > + *
> > + * Authors:
> > + * Albert Esteve <aest...@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/virtio/vhost-user-shmem.h"
> > +#include "system/memory.h"
> > +#include "qapi/error.h"
> > +#include "qemu/error-report.h"
> > +#include "trace.h"
> > +
> > +/**
> > + * VhostUserShmemObject
> > + *
> > + * An intermediate QOM object that manages individual shared memory
> > mappings
> > + * created by VHOST_USER_BACKEND_SHMEM_MAP requests. It acts as a parent
> > for
> > + * MemoryRegion objects, providing proper lifecycle management with
> > reference
> > + * counting. When the object is unreferenced and its reference count drops
> > + * to zero, it automatically cleans up the MemoryRegion and unmaps the
> > memory.
> > + */
> > +
> > +static void vhost_user_shmem_object_finalize(Object *obj);
> > +static void vhost_user_shmem_object_instance_init(Object *obj);
> > +
> > +static const TypeInfo vhost_user_shmem_object_info = {
> > + .name = TYPE_VHOST_USER_SHMEM_OBJECT,
> > + .parent = TYPE_OBJECT,
> > + .instance_size = sizeof(VhostUserShmemObject),
> > + .instance_init = vhost_user_shmem_object_instance_init,
> > + .instance_finalize = vhost_user_shmem_object_finalize,
> > +};
> > +
> > +static void vhost_user_shmem_object_instance_init(Object *obj)
> > +{
> > + VhostUserShmemObject *shmem_obj = VHOST_USER_SHMEM_OBJECT(obj);
> > +
> > + shmem_obj->shmid = 0;
> > + shmem_obj->fd = -1;
> > + shmem_obj->shm_offset = 0;
> > + shmem_obj->len = 0;
> > + shmem_obj->mr = NULL;
> > +}
> > +
> > +static void vhost_user_shmem_object_finalize(Object *obj)
> > +{
> > + VhostUserShmemObject *shmem_obj = VHOST_USER_SHMEM_OBJECT(obj);
> > +
> > + /* Clean up MemoryRegion if it exists */
> > + if (shmem_obj->mr) {
> > + /* Unparent the MemoryRegion to trigger cleanup */
> > + object_unparent(OBJECT(shmem_obj->mr));
> > + shmem_obj->mr = NULL;
> > + }
> > +
> > + /* Close file descriptor */
> > + if (shmem_obj->fd >= 0) {
> > + close(shmem_obj->fd);
> > + shmem_obj->fd = -1;
> > + }
> > +}
> > +
> > +VhostUserShmemObject *vhost_user_shmem_object_new(uint8_t shmid,
> > + int fd,
> > + uint64_t fd_offset,
> > + uint64_t shm_offset,
> > + uint64_t len,
> > + uint16_t flags)
> > +{
> > + VhostUserShmemObject *shmem_obj;
> > + MemoryRegion *mr;
> > + g_autoptr(GString) mr_name = g_string_new(NULL);
> > + uint32_t ram_flags;
> > + Error *local_err = NULL;
> > +
> > + if (len == 0) {
> > + error_report("Shared memory mapping size cannot be zero");
> > + return NULL;
> > + }
> > +
> > + fd = dup(fd);
> > + if (fd < 0) {
> > + error_report("Failed to duplicate fd: %s", strerror(errno));
> > + return NULL;
> > + }
> > +
> > + /* Determine RAM flags */
> > + ram_flags = RAM_SHARED;
> > + if (!(flags & VHOST_USER_FLAG_MAP_RW)) {
> > + ram_flags |= RAM_READONLY_FD;
> > + }
> > +
> > + /* Create the VhostUserShmemObject */
> > + shmem_obj = VHOST_USER_SHMEM_OBJECT(
> > + object_new(TYPE_VHOST_USER_SHMEM_OBJECT));
> > +
> > + /* Set up object properties */
> > + shmem_obj->shmid = shmid;
> > + shmem_obj->fd = fd;
> > + shmem_obj->shm_offset = shm_offset;
> > + shmem_obj->len = len;
> > +
> > + /* Create MemoryRegion as a child of this object */
> > + mr = g_new0(MemoryRegion, 1);
> > + g_string_printf(mr_name, "vhost-user-shmem-%d-%" PRIx64, shmid,
> > shm_offset);
> > +
> > + /* Initialize MemoryRegion with file descriptor */
> > + if (!memory_region_init_ram_from_fd(mr, OBJECT(shmem_obj),
> > mr_name->str,
> > + len, ram_flags, fd, fd_offset,
> > + &local_err)) {
> > + error_report_err(local_err);
> > + g_free(mr);
> > + close(fd);
> > + object_unref(OBJECT(shmem_obj));
> > + return NULL;
> > + }
> > +
> > + shmem_obj->mr = mr;
> > + return shmem_obj;
> > +}
> > +
> > +static void vhost_user_shmem_register_types(void)
> > +{
> > + type_register_static(&vhost_user_shmem_object_info);
> > +}
> > +
> > +type_init(vhost_user_shmem_register_types)
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 1e1d6b0d6e..eb3ad728b0 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -11,6 +11,7 @@
> > #include "qemu/osdep.h"
> > #include "qapi/error.h"
> > #include "hw/virtio/virtio-dmabuf.h"
> > +#include "hw/virtio/vhost-user-shmem.h"
> > #include "hw/virtio/vhost.h"
> > #include "hw/virtio/virtio-crypto.h"
> > #include "hw/virtio/vhost-user.h"
> > @@ -115,6 +116,8 @@ typedef enum VhostUserBackendRequest {
> > VHOST_USER_BACKEND_SHARED_OBJECT_ADD = 6,
> > VHOST_USER_BACKEND_SHARED_OBJECT_REMOVE = 7,
> > VHOST_USER_BACKEND_SHARED_OBJECT_LOOKUP = 8,
> > + VHOST_USER_BACKEND_SHMEM_MAP = 9,
> > + VHOST_USER_BACKEND_SHMEM_UNMAP = 10,
> > VHOST_USER_BACKEND_MAX
> > } VhostUserBackendRequest;
> >
> > @@ -192,6 +195,23 @@ typedef struct VhostUserShared {
> > unsigned char uuid[16];
> > } VhostUserShared;
> >
> > +/* For the flags field of VhostUserMMap */
> > +#define VHOST_USER_FLAG_MAP_RW (1u << 0)
> > +
> > +typedef struct {
> > + /* VIRTIO Shared Memory Region ID */
> > + uint8_t shmid;
> > + uint8_t padding[7];
> > + /* File offset */
> > + uint64_t fd_offset;
> > + /* Offset within the VIRTIO Shared Memory Region */
> > + uint64_t shm_offset;
> > + /* Size of the mapping */
> > + uint64_t len;
> > + /* Flags for the mmap operation, from VHOST_USER_FLAG_MAP_* */
> > + uint16_t flags;
> > +} VhostUserMMap;
> > +
> > typedef struct {
> > VhostUserRequest request;
> >
> > @@ -224,6 +244,7 @@ typedef union {
> > VhostUserInflight inflight;
> > VhostUserShared object;
> > VhostUserTransferDeviceState transfer_state;
> > + VhostUserMMap mmap;
> > } VhostUserPayload;
> >
> > typedef struct VhostUserMsg {
> > @@ -1768,6 +1789,172 @@
> > vhost_user_backend_handle_shared_object_lookup(struct vhost_user *u,
> > return 0;
> > }
> >
> > +/**
> > + * vhost_user_backend_handle_shmem_map() - Handle SHMEM_MAP backend request
> > + * @dev: vhost device
> > + * @ioc: QIOChannel for communication
> > + * @hdr: vhost-user message header
> > + * @payload: message payload containing mapping details
> > + * @fd: file descriptor for the shared memory region
> > + *
> > + * Handles VHOST_USER_BACKEND_SHMEM_MAP requests from the backend. Creates
> > + * a VhostUserShmemObject to manage the shared memory mapping and adds it
> > + * to the appropriate VirtIO shared memory region. The VhostUserShmemObject
> > + * serves as an intermediate parent for the MemoryRegion, ensuring proper
> > + * lifecycle management with reference counting.
> > + *
> > + * Returns: 0 on success, negative errno on failure
> > + */
> > +static int
> > +vhost_user_backend_handle_shmem_map(struct vhost_dev *dev,
> > + QIOChannel *ioc,
> > + VhostUserHeader *hdr,
> > + VhostUserPayload *payload,
> > + int fd)
> > +{
> > + VirtioSharedMemory *shmem;
> > + VhostUserMMap *vu_mmap = &payload->mmap;
> > + Error *local_err = NULL;
> > + g_autoptr(GString) shm_name = g_string_new(NULL);
> > +
> > + if (fd < 0) {
> > + error_report("Bad fd for map");
> > + return -EBADF;
> > + }
> > +
> > + if (QSIMPLEQ_EMPTY(&dev->vdev->shmem_list)) {
> > + error_report("Device has no VIRTIO Shared Memory Regions. "
> > + "Requested ID: %d", vu_mmap->shmid);
> > + return -EFAULT;
> > + }
> > +
> > + shmem = virtio_find_shmem_region(dev->vdev, vu_mmap->shmid);
> > + if (!shmem) {
> > + error_report("VIRTIO Shared Memory Region at "
> > + "ID %d not found or unitialized", vu_mmap->shmid);
> > + return -EFAULT;
> > + }
> > +
> > + if ((vu_mmap->shm_offset + vu_mmap->len) < vu_mmap->len ||
> > + (vu_mmap->shm_offset + vu_mmap->len) > shmem->mr.size) {
> > + error_report("Bad offset/len for mmap %" PRIx64 "+%" PRIx64,
> > + vu_mmap->shm_offset, vu_mmap->len);
> > + return -EFAULT;
> > + }
> > +
> > + g_string_printf(shm_name, "virtio-shm%i-%lu",
> > + vu_mmap->shmid, vu_mmap->shm_offset);
> > +
> > + memory_region_transaction_begin();
> > +
> > + /* Create VhostUserShmemObject as intermediate parent for MemoryRegion
> > */
> > + VhostUserShmemObject *shmem_obj = vhost_user_shmem_object_new(
> > + vu_mmap->shmid, fd, vu_mmap->fd_offset, vu_mmap->shm_offset,
> > + vu_mmap->len, vu_mmap->flags);
> > +
> > + if (!shmem_obj) {
> > + memory_region_transaction_commit();
> > + return -EFAULT;
> > + }
> > +
> > + /* Add the mapping using our VhostUserShmemObject as the parent */
> > + if (virtio_add_shmem_map(shmem, shmem_obj) != 0) {
> > + error_report("Failed to add shared memory mapping");
> > + object_unref(OBJECT(shmem_obj));
> > + memory_region_transaction_commit();
> > + return -EFAULT;
> > + }
> > +
> > + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> > + payload->u64 = 0;
> > + hdr->size = sizeof(payload->u64);
> > + vhost_user_send_resp(ioc, hdr, payload, &local_err);
> > + if (local_err) {
> > + error_report_err(local_err);
> > + memory_region_transaction_commit();
> > + return -EFAULT;
> > + }
> > + }
> > +
> > + memory_region_transaction_commit();
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * vhost_user_backend_handle_shmem_unmap() - Handle SHMEM_UNMAP backend
> > request
> > + * @dev: vhost device
> > + * @ioc: QIOChannel for communication
> > + * @hdr: vhost-user message header
> > + * @payload: message payload containing unmapping details
> > + *
> > + * Handles VHOST_USER_BACKEND_SHMEM_UNMAP requests from the backend.
> > Removes
> > + * the specified memory mapping from the VirtIO shared memory region. This
> > + * automatically unreferences the associated VhostUserShmemObject, which
> > may
> > + * trigger its finalization and cleanup (munmap, close fd) if no other
> > + * references exist.
> > + *
> > + * Returns: 0 on success, negative errno on failure
> > + */
> > +static int
> > +vhost_user_backend_handle_shmem_unmap(struct vhost_dev *dev,
> > + QIOChannel *ioc,
> > + VhostUserHeader *hdr,
> > + VhostUserPayload *payload)
> > +{
> > + VirtioSharedMemory *shmem;
> > + VirtioSharedMemoryMapping *mmap = NULL;
> > + VhostUserMMap *vu_mmap = &payload->mmap;
> > + Error *local_err = NULL;
> > +
> > + if (QSIMPLEQ_EMPTY(&dev->vdev->shmem_list)) {
> > + error_report("Device has no VIRTIO Shared Memory Regions. "
> > + "Requested ID: %d", vu_mmap->shmid);
> > + return -EFAULT;
> > + }
> > +
> > + shmem = virtio_find_shmem_region(dev->vdev, vu_mmap->shmid);
> > + if (!shmem) {
> > + error_report("VIRTIO Shared Memory Region at "
> > + "ID %d not found or unitialized", vu_mmap->shmid);
> > + return -EFAULT;
> > + }
> > +
> > + if ((vu_mmap->shm_offset + vu_mmap->len) < vu_mmap->len ||
> > + (vu_mmap->shm_offset + vu_mmap->len) > shmem->mr.size) {
> > + error_report("Bad offset/len for unmmap %" PRIx64 "+%" PRIx64,
> > + vu_mmap->shm_offset, vu_mmap->len);
> > + return -EFAULT;
> > + }
> > +
> > + mmap = virtio_find_shmem_map(shmem, vu_mmap->shm_offset, vu_mmap->len);
> > + if (!mmap) {
> > + error_report("Shared memory mapping not found at offset %" PRIx64
> > + " with length %" PRIx64,
> > + vu_mmap->shm_offset, vu_mmap->len);
> > + return -EFAULT;
> > + }
> > +
> > + memory_region_transaction_begin();
> > + memory_region_del_subregion(&shmem->mr, mmap->mem);
> > + if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
> > + payload->u64 = 0;
> > + hdr->size = sizeof(payload->u64);
> > + vhost_user_send_resp(ioc, hdr, payload, &local_err);
> > + if (local_err) {
> > + error_report_err(local_err);
> > + memory_region_transaction_commit();
> > + return -EFAULT;
> > + }
> > + }
> > + memory_region_transaction_commit();
> > +
> > + /* Free the MemoryRegion only after vhost_commit */
> > + virtio_del_shmem_map(shmem, vu_mmap->shm_offset, vu_mmap->len);
> > +
> > + return 0;
> > +}
> > +
> > static void close_backend_channel(struct vhost_user *u)
> > {
> > g_source_destroy(u->backend_src);
> > @@ -1833,8 +2020,24 @@ static gboolean backend_read(QIOChannel *ioc,
> > GIOCondition condition,
> >
> > &payload.object);
> > break;
> > case VHOST_USER_BACKEND_SHARED_OBJECT_LOOKUP:
> > - ret = vhost_user_backend_handle_shared_object_lookup(dev->opaque,
> > ioc,
> > - &hdr,
> > &payload);
> > + /* Handler manages its own response, check error and close
> > connection */
> > + if (vhost_user_backend_handle_shared_object_lookup(dev->opaque,
> > ioc,
> > + &hdr, &payload)
> > < 0) {
> > + goto err;
> > + }
> > + break;
> > + case VHOST_USER_BACKEND_SHMEM_MAP:
> > + /* Handler manages its own response, check error and close
> > connection */
> > + if (vhost_user_backend_handle_shmem_map(dev, ioc, &hdr, &payload,
> > + fd ? fd[0] : -1) < 0) {
> > + goto err;
> > + }
> > + break;
> > + case VHOST_USER_BACKEND_SHMEM_UNMAP:
> > + /* Handler manages its own response, check error and close
> > connection */
> > + if (vhost_user_backend_handle_shmem_unmap(dev, ioc, &hdr,
> > &payload) < 0) {
> > + goto err;
> > + }
> > break;
> > default:
> > error_report("Received unexpected msg type: %d.", hdr.request);
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 9a81ad912e..1ead5f653f 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -14,6 +14,7 @@
> > #include "qemu/osdep.h"
> > #include "qapi/error.h"
> > #include "qapi/qapi-commands-virtio.h"
> > +#include "hw/virtio/vhost-user-shmem.h"
> > #include "trace.h"
> > #include "qemu/defer-call.h"
> > #include "qemu/error-report.h"
> > @@ -3045,6 +3046,100 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
> > return vmstate_save_state(f, &vmstate_virtio, vdev, NULL);
> > }
> >
> > +VirtioSharedMemory *virtio_new_shmem_region(VirtIODevice *vdev, uint8_t
> > shmid)
> > +{
> > + VirtioSharedMemory *elem;
> > + g_autofree char *name = NULL;
> > +
> > + elem = g_new0(VirtioSharedMemory, 1);
> > + elem->shmid = shmid;
> > +
> > + /* Initialize embedded MemoryRegion as container for shmem mappings */
> > + name = g_strdup_printf("virtio-shmem-%d", shmid);
> > + memory_region_init(&elem->mr, OBJECT(vdev), name, UINT64_MAX);
> > + QTAILQ_INIT(&elem->mmaps);
> > + QSIMPLEQ_INSERT_TAIL(&vdev->shmem_list, elem, entry);
> > + return QSIMPLEQ_LAST(&vdev->shmem_list, VirtioSharedMemory, entry);
>
> "return elem;" is simpler.
This sounds familiar. I hope is not a change that got lost, but simply
that I changed
it somewhere else but I repeated the pattern here. I will change it.
>
> > +}
> > +
> > +VirtioSharedMemory *virtio_find_shmem_region(VirtIODevice *vdev, uint8_t
> > shmid)
> > +{
> > + VirtioSharedMemory *shmem, *next;
> > + QSIMPLEQ_FOREACH_SAFE(shmem, &vdev->shmem_list, entry, next) {
> > + if (shmem->shmid == shmid) {
> > + return shmem;
> > + }
> > + }
> > + return NULL;
> > +}
> > +
> > +int virtio_add_shmem_map(VirtioSharedMemory *shmem,
> > + VhostUserShmemObject *shmem_obj)
> > +{
> > + VirtioSharedMemoryMapping *mmap;
> > + if (!shmem_obj) {
> > + error_report("VhostUserShmemObject cannot be NULL");
> > + return -1;
> > + }
> > + if (!shmem_obj->mr) {
> > + error_report("VhostUserShmemObject has no MemoryRegion");
> > + return -1;
> > + }
> > +
> > + /* Validate boundaries against the VIRTIO shared memory region */
> > + if (shmem_obj->shm_offset + shmem_obj->len > shmem->mr.size) {
>
> From above:
>
> memory_region_init(&elem->mr, OBJECT(vdev), name, UINT64_MAX);
>
> shmem->mr's size is UINT64_MAX and this if statement doesn't handle
> integer overflow. What is the purpose of this size check?
>
> > + error_report("Memory exceeds the shared memory boundaries");
> > + return -1;
> > + }
> > +
> > + /* Create the VirtioSharedMemoryMapping wrapper */
> > + mmap = g_new0(VirtioSharedMemoryMapping, 1);
> > + mmap->mem = shmem_obj->mr;
> > + mmap->offset = shmem_obj->shm_offset;
> > + mmap->shmem_obj = shmem_obj;
> > +
> > + /* Take a reference on the VhostUserShmemObject */
> > + object_ref(OBJECT(shmem_obj));
>
> Why is the reference count incremented here? The caller seems to pass
> ownership to this function...at least
> vhost_user_backend_handle_shmem_map() doesn't touch shmem_obj afterwards
> and doesn't unref it.
You are right. transfering ownership is better and does not require
the count increment. I'll remove this.
>
> > +
> > + /* Add as subregion to the VIRTIO shared memory */
> > + memory_region_add_subregion(&shmem->mr, mmap->offset, mmap->mem);
> > +
> > + /* Add to the mapped regions list */
> > + QTAILQ_INSERT_TAIL(&shmem->mmaps, mmap, link);
> > +
> > + return 0;
> > +}
> > +
> > +VirtioSharedMemoryMapping *virtio_find_shmem_map(VirtioSharedMemory *shmem,
> > + hwaddr offset, uint64_t size)
> > +{
> > + VirtioSharedMemoryMapping *mmap;
> > + QTAILQ_FOREACH(mmap, &shmem->mmaps, link) {
> > + if (mmap->offset == offset && mmap->mem->size == size) {
> > + return mmap;
> > + }
> > + }
> > + return NULL;
> > +}
> > +
> > +void virtio_del_shmem_map(VirtioSharedMemory *shmem, hwaddr offset,
> > + uint64_t size)
> > +{
> > + VirtioSharedMemoryMapping *mmap = virtio_find_shmem_map(shmem, offset,
> > size);
> > + if (mmap == NULL) {
> > + return;
> > + }
> > +
> > + /*
> > + * Unref the VhostUserShmemObject which will trigger automatic cleanup
> > + * when the reference count reaches zero.
> > + */
> > + object_unref(OBJECT(mmap->shmem_obj));
> > +
> > + QTAILQ_REMOVE(&shmem->mmaps, mmap, link);
> > + g_free(mmap);
> > +}
> > +
> > /* A wrapper for use as a VMState .put function */
> > static int virtio_device_put(QEMUFile *f, void *opaque, size_t size,
> > const VMStateField *field, JSONWriter
> > *vmdesc)
> > @@ -3521,6 +3616,7 @@ void virtio_init(VirtIODevice *vdev, uint16_t
> > device_id, size_t config_size)
> > NULL, virtio_vmstate_change, vdev);
> > vdev->device_endian = virtio_default_endian();
> > vdev->use_guest_notifier_mask = true;
> > + QSIMPLEQ_INIT(&vdev->shmem_list);
> > }
> >
> > /*
> > @@ -4032,11 +4128,24 @@ static void
> > virtio_device_free_virtqueues(VirtIODevice *vdev)
> > static void virtio_device_instance_finalize(Object *obj)
> > {
> > VirtIODevice *vdev = VIRTIO_DEVICE(obj);
> > + VirtioSharedMemory *shmem;
> >
> > virtio_device_free_virtqueues(vdev);
> >
> > g_free(vdev->config);
> > g_free(vdev->vector_queues);
> > + while (!QSIMPLEQ_EMPTY(&vdev->shmem_list)) {
> > + shmem = QSIMPLEQ_FIRST(&vdev->shmem_list);
> > + while (!QTAILQ_EMPTY(&shmem->mmaps)) {
> > + VirtioSharedMemoryMapping *mmap_reg =
> > QTAILQ_FIRST(&shmem->mmaps);
> > + virtio_del_shmem_map(shmem, mmap_reg->offset,
> > mmap_reg->mem->size);
> > + }
> > +
> > + /* Clean up the embedded MemoryRegion */
> > + object_unparent(OBJECT(&shmem->mr));
> > + QSIMPLEQ_REMOVE_HEAD(&vdev->shmem_list, entry);
> > + g_free(shmem);
> > + }
> > }
> >
> > static const Property virtio_properties[] = {
> > diff --git a/include/hw/virtio/vhost-user-shmem.h
> > b/include/hw/virtio/vhost-user-shmem.h
> > new file mode 100644
> > index 0000000000..1f8c7bdc1f
> > --- /dev/null
> > +++ b/include/hw/virtio/vhost-user-shmem.h
> > @@ -0,0 +1,75 @@
> > +/*
> > + * VHost-user Shared Memory Object
> > + *
> > + * Copyright Red Hat, Inc. 2025
> > + *
> > + * Authors:
> > + * Albert Esteve <aest...@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#ifndef VHOST_USER_SHMEM_H
> > +#define VHOST_USER_SHMEM_H
> > +
> > +#include "qemu/osdep.h"
> > +#include "qom/object.h"
> > +#include "system/memory.h"
> > +#include "qapi/error.h"
> > +
> > +/* vhost-user memory mapping flags */
> > +#define VHOST_USER_FLAG_MAP_RW (1u << 0)
>
> This constant is part of the vhost-user protocol. It would be nicer to
> keep that all in one file instead of spreading protocol definitions
> across multiple files.
>
> In this case you could replace vhost_user_shmem_object_new()'s flags
> argument with a bool allow_write argument. That way the vhost-user
> protocol parsing happens in vhost-user.c and not vhost-user-shmem.c.
I'll go for this option.
>
> Alternatively, you could move the protocol definitions from vhost-user.c
> into a header file and include them from vhost-user-shmem.c.
>
> > +
> > +#define TYPE_VHOST_USER_SHMEM_OBJECT "vhost-user-shmem"
> > +OBJECT_DECLARE_SIMPLE_TYPE(VhostUserShmemObject, VHOST_USER_SHMEM_OBJECT)
> > +
> > +/**
> > + * VhostUserShmemObject:
> > + * @parent: Parent object
> > + * @shmid: VIRTIO Shared Memory Region ID
> > + * @fd: File descriptor for the shared memory region
> > + * @shm_offset: Offset within the VIRTIO Shared Memory Region
> > + * @len: Size of the mapping
> > + * @mr: MemoryRegion associated with this shared memory mapping
> > + *
> > + * An intermediate QOM object that manages individual shared memory
> > mappings
> > + * created by VHOST_USER_BACKEND_SHMEM_MAP requests. It acts as a parent
> > for
> > + * MemoryRegion objects, providing proper lifecycle management with
> > reference
> > + * counting. When the object is unreferenced and its reference count drops
> > + * to zero, it automatically cleans up the MemoryRegion and unmaps the
> > memory.
> > + */
> > +struct VhostUserShmemObject {
> > + Object parent;
> > +
> > + uint8_t shmid;
> > + int fd;
> > + uint64_t shm_offset;
> > + uint64_t len;
> > + MemoryRegion *mr;
> > +};
> > +
> > +/**
> > + * vhost_user_shmem_object_new() - Create a new VhostUserShmemObject
> > + * @shmid: VIRTIO Shared Memory Region ID
> > + * @fd: File descriptor for the shared memory
> > + * @fd_offset: Offset within the file descriptor
> > + * @shm_offset: Offset within the VIRTIO Shared Memory Region
> > + * @len: Size of the mapping
> > + * @flags: Mapping flags (VHOST_USER_FLAG_MAP_*)
> > + *
> > + * Creates a new VhostUserShmemObject that manages a shared memory mapping.
> > + * The object will create a MemoryRegion using
> > memory_region_init_ram_from_fd()
> > + * as a child object. When the object is finalized, it will automatically
> > + * clean up the MemoryRegion and close the file descriptor.
> > + *
> > + * Return: A new VhostUserShmemObject on success, NULL on error.
> > + */
> > +VhostUserShmemObject *vhost_user_shmem_object_new(uint8_t shmid,
> > + int fd,
> > + uint64_t fd_offset,
> > + uint64_t shm_offset,
> > + uint64_t len,
> > + uint16_t flags);
> > +
> > +#endif /* VHOST_USER_SHMEM_H */
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index c594764f23..a563bbac2c 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -98,6 +98,26 @@ enum virtio_device_endian {
> > VIRTIO_DEVICE_ENDIAN_BIG,
> > };
> >
> > +struct VhostUserShmemObject;
> > +
> > +struct VirtioSharedMemoryMapping {
> > + MemoryRegion *mem;
> > + hwaddr offset;
> > + QTAILQ_ENTRY(VirtioSharedMemoryMapping) link;
> > + struct VhostUserShmemObject *shmem_obj; /* Intermediate parent object
> > */
> > +};
> > +
> > +typedef struct VirtioSharedMemoryMapping VirtioSharedMemoryMapping;
> > +
> > +struct VirtioSharedMemory {
> > + uint8_t shmid;
> > + MemoryRegion mr;
> > + QTAILQ_HEAD(, VirtioSharedMemoryMapping) mmaps;
> > + QSIMPLEQ_ENTRY(VirtioSharedMemory) entry;
> > +};
>
> VirtioSharedMemoryMapping and VirtioSharedMemory duplicate information
> from VhostUserShmemObject (shmid, memory region pointers, offsets). This
> makes the relationship between VIRTIO and vhost-user code confusing.
>
> I wonder if VhostUserShmemObject is specific to the vhost-user protocol
> or if any VIRTIO device implementation that needs a VIRTIO Shared Memory
> Region with an fd, offset, etc should be able to use it? If yes, then it
> should be renamed and made part of the core hw/virtio/ code rather than
> vhost-user.
>
> > +
> > +typedef struct VirtioSharedMemory VirtioSharedMemory;
> > +
> > /**
> > * struct VirtIODevice - common VirtIO structure
> > * @name: name of the device
> > @@ -167,6 +187,8 @@ struct VirtIODevice
> > */
> > EventNotifier config_notifier;
> > bool device_iotlb_enabled;
> > + /* Shared memory region for mappings. */
> > + QSIMPLEQ_HEAD(, VirtioSharedMemory) shmem_list;
> > };
> >
> > struct VirtioDeviceClass {
> > @@ -295,6 +317,77 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
> >
> > int virtio_save(VirtIODevice *vdev, QEMUFile *f);
> >
> > +/**
> > + * virtio_new_shmem_region() - Create a new shared memory region
> > + * @vdev: VirtIODevice
> > + * @shmid: Shared memory ID
> > + *
> > + * Creates a new VirtioSharedMemory region for the given device and ID.
> > + * The returned VirtioSharedMemory is owned by the VirtIODevice and will
> > + * be automatically freed when the device is destroyed. The caller
> > + * should not free the returned pointer.
> > + *
> > + * Returns: Pointer to the new VirtioSharedMemory region, or NULL on
> > failure
> > + */
> > +VirtioSharedMemory *virtio_new_shmem_region(VirtIODevice *vdev, uint8_t
> > shmid);
> > +
> > +/**
> > + * virtio_find_shmem_region() - Find an existing shared memory region
> > + * @vdev: VirtIODevice
> > + * @shmid: Shared memory ID to find
> > + *
> > + * Finds an existing VirtioSharedMemory region by ID. The returned pointer
> > + * is owned by the VirtIODevice and should not be freed by the caller.
> > + *
> > + * Returns: Pointer to the VirtioSharedMemory region, or NULL if not found
> > + */
> > +VirtioSharedMemory *virtio_find_shmem_region(VirtIODevice *vdev, uint8_t
> > shmid);
> > +
> > +/**
> > + * virtio_add_shmem_map() - Add a memory mapping to a shared region
> > + * @shmem: VirtioSharedMemory region
> > + * @shmem_obj: VhostUserShmemObject to add (takes a reference)
> > + *
> > + * Adds a memory mapping to the shared memory region. The
> > VhostUserShmemObject
> > + * is added as a child of the mapping and will be automatically managed
> > through
> > + * QOM reference counting. The mapping will be removed when
> > + * virtio_del_shmem_map() is called or when the shared memory region is
> > + * destroyed.
> > + *
> > + * Returns: 0 on success, negative errno on failure
> > + */
> > +int virtio_add_shmem_map(VirtioSharedMemory *shmem,
> > + struct VhostUserShmemObject *shmem_obj);
>
> This API suggests the answer to my question above about whether
> VhostUserShmemObject is really a core hw/virtio/ concept rather than a
> vhost-user protocol concept is "yes". I think VhostUserShmemObject
> should be renamed and maybe unified with VirtioSharedMemoryMapping.
The answer would yes, you are right. The messages to map/unmap are
vhost-user-specific, but shared memory is a core virtio concept. I
will rename it.
>
> > +
> > +/**
> > + * virtio_find_shmem_map() - Find a memory mapping in a shared region
> > + * @shmem: VirtioSharedMemory region
> > + * @offset: Offset within the shared memory region
> > + * @size: Size of the mapping to find
> > + *
> > + * Finds an existing memory mapping that covers the specified range.
> > + * The returned VirtioSharedMemoryMapping is owned by the
> > VirtioSharedMemory
> > + * region and should not be freed by the caller.
> > + *
> > + * Returns: Pointer to the VirtioSharedMemoryMapping, or NULL if not found
> > + */
> > +VirtioSharedMemoryMapping *virtio_find_shmem_map(VirtioSharedMemory *shmem,
> > + hwaddr offset, uint64_t size);
> > +
> > +/**
> > + * virtio_del_shmem_map() - Remove a memory mapping from a shared region
> > + * @shmem: VirtioSharedMemory region
> > + * @offset: Offset of the mapping to remove
> > + * @size: Size of the mapping to remove
> > + *
> > + * Removes a memory mapping from the shared memory region. This will
> > + * automatically unref the associated VhostUserShmemObject, which may
> > + * trigger its finalization and cleanup if no other references exist.
> > + * The mapping's MemoryRegion will be properly unmapped and cleaned up.
> > + */
> > +void virtio_del_shmem_map(VirtioSharedMemory *shmem, hwaddr offset,
> > + uint64_t size);
> > +
> > extern const VMStateInfo virtio_vmstate_info;
> >
> > #define VMSTATE_VIRTIO_DEVICE \
> > diff --git a/subprojects/libvhost-user/libvhost-user.c
> > b/subprojects/libvhost-user/libvhost-user.c
> > index 9c630c2170..034cbfdc3c 100644
> > --- a/subprojects/libvhost-user/libvhost-user.c
> > +++ b/subprojects/libvhost-user/libvhost-user.c
> > @@ -1592,6 +1592,76 @@ vu_rm_shared_object(VuDev *dev, unsigned char
> > uuid[UUID_LEN])
> > return vu_send_message(dev, &msg);
> > }
> >
> > +bool
> > +vu_shmem_map(VuDev *dev, uint8_t shmid, uint64_t fd_offset,
> > + uint64_t shm_offset, uint64_t len, uint64_t flags, int fd)
> > +{
> > + VhostUserMsg vmsg = {
> > + .request = VHOST_USER_BACKEND_SHMEM_MAP,
> > + .size = sizeof(vmsg.payload.mmap),
> > + .flags = VHOST_USER_VERSION,
> > + .payload.mmap = {
> > + .shmid = shmid,
> > + .fd_offset = fd_offset,
> > + .shm_offset = shm_offset,
> > + .len = len,
> > + .flags = flags,
> > + },
> > + .fd_num = 1,
> > + .fds[0] = fd,
> > + };
> > +
> > + if (!vu_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_SHMEM)) {
> > + return false;
> > + }
> > +
> > + if (vu_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_REPLY_ACK)) {
> > + vmsg.flags |= VHOST_USER_NEED_REPLY_MASK;
> > + }
> > +
> > + pthread_mutex_lock(&dev->backend_mutex);
> > + if (!vu_message_write(dev, dev->backend_fd, &vmsg)) {
> > + pthread_mutex_unlock(&dev->backend_mutex);
> > + return false;
> > + }
> > +
> > + /* Also unlocks the backend_mutex */
> > + return vu_process_message_reply(dev, &vmsg);
> > +}
> > +
> > +bool
> > +vu_shmem_unmap(VuDev *dev, uint8_t shmid, uint64_t shm_offset, uint64_t
> > len)
> > +{
> > + VhostUserMsg vmsg = {
> > + .request = VHOST_USER_BACKEND_SHMEM_UNMAP,
> > + .size = sizeof(vmsg.payload.mmap),
> > + .flags = VHOST_USER_VERSION,
> > + .payload.mmap = {
> > + .shmid = shmid,
> > + .fd_offset = 0,
> > + .shm_offset = shm_offset,
> > + .len = len,
> > + },
> > + };
> > +
> > + if (!vu_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_SHMEM)) {
> > + return false;
> > + }
> > +
> > + if (vu_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_REPLY_ACK)) {
> > + vmsg.flags |= VHOST_USER_NEED_REPLY_MASK;
> > + }
> > +
> > + pthread_mutex_lock(&dev->backend_mutex);
> > + if (!vu_message_write(dev, dev->backend_fd, &vmsg)) {
> > + pthread_mutex_unlock(&dev->backend_mutex);
> > + return false;
> > + }
> > +
> > + /* Also unlocks the backend_mutex */
> > + return vu_process_message_reply(dev, &vmsg);
> > +}
> > +
> > static bool
> > vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
> > {
> > diff --git a/subprojects/libvhost-user/libvhost-user.h
> > b/subprojects/libvhost-user/libvhost-user.h
> > index 2ffc58c11b..26b710c92d 100644
> > --- a/subprojects/libvhost-user/libvhost-user.h
> > +++ b/subprojects/libvhost-user/libvhost-user.h
> > @@ -69,6 +69,8 @@ enum VhostUserProtocolFeature {
> > /* Feature 16 is reserved for VHOST_USER_PROTOCOL_F_STATUS. */
> > /* Feature 17 reserved for VHOST_USER_PROTOCOL_F_XEN_MMAP. */
> > VHOST_USER_PROTOCOL_F_SHARED_OBJECT = 18,
> > + /* Feature 19 is reserved for VHOST_USER_PROTOCOL_F_DEVICE_STATE */
> > + VHOST_USER_PROTOCOL_F_SHMEM = 20,
> > VHOST_USER_PROTOCOL_F_MAX
> > };
> >
> > @@ -127,6 +129,8 @@ typedef enum VhostUserBackendRequest {
> > VHOST_USER_BACKEND_SHARED_OBJECT_ADD = 6,
> > VHOST_USER_BACKEND_SHARED_OBJECT_REMOVE = 7,
> > VHOST_USER_BACKEND_SHARED_OBJECT_LOOKUP = 8,
> > + VHOST_USER_BACKEND_SHMEM_MAP = 9,
> > + VHOST_USER_BACKEND_SHMEM_UNMAP = 10,
> > VHOST_USER_BACKEND_MAX
> > } VhostUserBackendRequest;
> >
> > @@ -186,6 +190,23 @@ typedef struct VhostUserShared {
> > unsigned char uuid[UUID_LEN];
> > } VhostUserShared;
> >
> > +/* For the flags field of VhostUserMMap */
> > +#define VHOST_USER_FLAG_MAP_RW (1u << 0)
> > +
> > +typedef struct {
> > + /* VIRTIO Shared Memory Region ID */
> > + uint8_t shmid;
> > + uint8_t padding[7];
> > + /* File offset */
> > + uint64_t fd_offset;
> > + /* Offset within the VIRTIO Shared Memory Region */
> > + uint64_t shm_offset;
> > + /* Size of the mapping */
> > + uint64_t len;
> > + /* Flags for the mmap operation, from VHOST_USER_FLAG_MAP_* */
> > + uint16_t flags;
> > +} VhostUserMMap;
> > +
> > #define VU_PACKED __attribute__((packed))
> >
> > typedef struct VhostUserMsg {
> > @@ -210,6 +231,7 @@ typedef struct VhostUserMsg {
> > VhostUserVringArea area;
> > VhostUserInflight inflight;
> > VhostUserShared object;
> > + VhostUserMMap mmap;
> > } payload;
> >
> > int fds[VHOST_MEMORY_BASELINE_NREGIONS];
> > @@ -593,6 +615,38 @@ bool vu_add_shared_object(VuDev *dev, unsigned char
> > uuid[UUID_LEN]);
> > */
> > bool vu_rm_shared_object(VuDev *dev, unsigned char uuid[UUID_LEN]);
> >
> > +/**
> > + * vu_shmem_map:
> > + * @dev: a VuDev context
> > + * @shmid: VIRTIO Shared Memory Region ID
> > + * @fd_offset: File offset
> > + * @shm_offset: Offset within the VIRTIO Shared Memory Region
> > + * @len: Size of the mapping
> > + * @flags: Flags for the mmap operation
> > + * @fd: A file descriptor
> > + *
> > + * Advertises a new mapping to be made in a given VIRTIO Shared Memory
> > Region.
> > + *
> > + * Returns: TRUE on success, FALSE on failure.
> > + */
> > +bool vu_shmem_map(VuDev *dev, uint8_t shmid, uint64_t fd_offset,
> > + uint64_t shm_offset, uint64_t len, uint64_t flags, int
> > fd);
> > +
> > +/**
> > + * vu_shmem_unmap:
> > + * @dev: a VuDev context
> > + * @shmid: VIRTIO Shared Memory Region ID
> > + * @fd_offset: File offset
> > + * @len: Size of the mapping
> > + *
> > + * The front-end un-mmaps a given range in the VIRTIO Shared Memory Region
> > + * with the requested `shmid`.
> > + *
> > + * Returns: TRUE on success, FALSE on failure.
> > + */
> > +bool vu_shmem_unmap(VuDev *dev, uint8_t shmid, uint64_t shm_offset,
> > + uint64_t len);
> > +
> > /**
> > * vu_queue_set_notification:
> > * @dev: a VuDev context
> > --
> > 2.49.0
> >
