[PATCH v5 0/7] vhost-user: Add SHMEM_MAP/UNMAP requests

[PATCH v5 0/7] vhost-user: Add SHMEM_MAP/UNMAP requests

[Albert Esteve]

Hi all,

v4->v5
- Change vdev->shmem_list to QSIMPLEQ type
- Dropped MEM_READ/WRITE messages
- Improved message handling for
  SHMEM_MAP/UNMAP (fix leaks, fix
  response logic, reworked field
  checks, etc.)
- Changed VHOST_USER_FLAG_MAP_* to
  have only read-write flag

The goal of this patch is to support
dynamic fd-backed memory maps initiated
from vhost-user backends.
There are many devices that could already
benefit of this feature, e.g.,
virtiofs, virtio-gpu, or the recently
added to the spec, virtio-media device.

After receiving the SHMEM_MAP/UNMAP request,
the frontend creates the RAMBlock form the
fd and maps it by adding it as a subregion
of the shared memory region container.

The VIRTIO Shared Memory Region list is
declared in the `VirtIODevice` struct
to make it generic.

The message handling code has been tested
with a rust-vmm device, which entails
some level of validation.

This patch also includes:
- SHMEM_CONFIG frontend request that is
specifically meant to allow generic
vhost-user-device frontend to be able to
query VIRTIO Shared Memory settings from the
backend (as this device is generic and agnostic
of the actual backend configuration).

Albert Esteve (7):
  vhost-user: Add VirtIO Shared Memory map request
  vhost_user.rst: Align VhostUserMsg excerpt members
  vhost_user.rst: Add SHMEM_MAP/_UNMAP to spec
  vhost_user: Add frontend get_shmem_config command
  vhost_user.rst: Add GET_SHMEM_CONFIG message
  qmp: add shmem feature map
  vhost-user-devive: Add shmem BAR

 docs/interop/vhost-user.rst               |  98 +++++++++++
 hw/virtio/vhost-user-base.c               |  47 +++++-
 hw/virtio/vhost-user-device-pci.c         |  34 +++-
 hw/virtio/vhost-user.c                    | 193 ++++++++++++++++++++++
 hw/virtio/virtio-qmp.c                    |   3 +
 hw/virtio/virtio.c                        |  97 +++++++++++
 include/hw/virtio/vhost-backend.h         |  10 ++
 include/hw/virtio/vhost-user.h            |   1 +
 include/hw/virtio/virtio.h                |  31 ++++
 subprojects/libvhost-user/libvhost-user.c |  70 ++++++++
 subprojects/libvhost-user/libvhost-user.h |  54 ++++++
 11 files changed, 633 insertions(+), 5 deletions(-)

-- 
2.49.0

[PATCH v5 1/7] vhost-user: Add VirtIO Shared Memory map request

[Albert Esteve]

Add SHMEM_MAP/UNMAP requests to vhost-user to
handle VIRTIO Shared Memory mappings.

This request allows backends to dynamically map
fds into a VIRTIO Shared Memory Region indentified
by its `shmid`. The map is performed by calling
`memory_region_init_ram_from_fd` and adding the
new region as a subregion of the shmem container
MR. Then, the fd memory is advertised to the
driver as a base addres + offset, so it can be
read/written (depending on the mmap flags
requested) while it is valid.

The backend can unmap the memory range
in a given VIRTIO Shared Memory Region (again,
identified by its `shmid`), to free it.
Upon receiving this message, the front-end
must delete the MR as a subregion of
the shmem container region and free its
resources.

Note that commit all these operations need
to be delayed to after we respond the request
to the backend to avoid deadlocks.

The device model needs to create VirtSharedMemory
instances for the VirtIO Shared Memory Regions
and add them to the `VirtIODevice` instance.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 hw/virtio/vhost-user.c                    | 150 ++++++++++++++++++++++
 hw/virtio/virtio.c                        |  97 ++++++++++++++
 include/hw/virtio/virtio.h                |  29 +++++
 subprojects/libvhost-user/libvhost-user.c |  70 ++++++++++
 subprojects/libvhost-user/libvhost-user.h |  54 ++++++++
 5 files changed, 400 insertions(+)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 1e1d6b0d6e..9c635fb928 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -115,6 +115,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 +194,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 +243,7 @@ typedef union {
         VhostUserInflight inflight;
         VhostUserShared object;
         VhostUserTransferDeviceState transfer_state;
+        VhostUserMMap mmap;
 } VhostUserPayload;
 
 typedef struct VhostUserMsg {
@@ -1768,6 +1788,129 @@ vhost_user_backend_handle_shared_object_lookup(struct vhost_user *u,
     return 0;
 }
 
+static int
+vhost_user_backend_handle_shmem_map(struct vhost_dev *dev,
+                                    QIOChannel *ioc,
+                                    VhostUserHeader *hdr,
+                                    VhostUserPayload *payload,
+                                    int fd)
+{
+    uint32_t ram_flags;
+    VirtSharedMemory *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();
+    ram_flags = RAM_SHARED |
+                ((vu_mmap->flags & VHOST_USER_FLAG_MAP_RW) ? 0 : RAM_READONLY);
+    if (virtio_add_shmem_map(shmem, shm_name->str, vu_mmap->shm_offset,
+                             vu_mmap->fd_offset, vu_mmap->len, ram_flags,
+                             fd) != 0) {
+        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;
+}
+
+static int
+vhost_user_backend_handle_shmem_unmap(struct vhost_dev *dev,
+                                      QIOChannel *ioc,
+                                      VhostUserHeader *hdr,
+                                      VhostUserPayload *payload)
+{
+    VirtSharedMemory *shmem;
+    MappedMemoryRegion *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) {
+        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);
@@ -1836,6 +1979,13 @@ static gboolean backend_read(QIOChannel *ioc, GIOCondition condition,
         ret = vhost_user_backend_handle_shared_object_lookup(dev->opaque, ioc,
                                                              &hdr, &payload);
         break;
+    case VHOST_USER_BACKEND_SHMEM_MAP:
+        ret = vhost_user_backend_handle_shmem_map(dev, ioc, &hdr, &payload,
+                                                  fd ? fd[0] : -1);
+        break;
+    case VHOST_USER_BACKEND_SHMEM_UNMAP:
+        ret = vhost_user_backend_handle_shmem_unmap(dev, ioc, &hdr, &payload);
+        break;
     default:
         error_report("Received unexpected msg type: %d.", hdr.request);
         ret = -EINVAL;
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5534251e01..208ad11685 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3035,6 +3035,92 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
     return vmstate_save_state(f, &vmstate_virtio, vdev, NULL);
 }
 
+VirtSharedMemory *virtio_new_shmem_region(VirtIODevice *vdev, uint8_t shmid)
+{
+    VirtSharedMemory *elem;
+    elem = g_new0(VirtSharedMemory, 1);
+    elem->shmid = shmid;
+    elem->mr = g_new0(MemoryRegion, 1);
+    QTAILQ_INIT(&elem->mmaps);
+    QSIMPLEQ_INSERT_TAIL(&vdev->shmem_list, elem, entry);
+    return QSIMPLEQ_LAST(&vdev->shmem_list, VirtSharedMemory, entry);
+}
+
+VirtSharedMemory *virtio_find_shmem_region(VirtIODevice *vdev, uint8_t shmid)
+{
+    VirtSharedMemory *shmem, *next;
+    QSIMPLEQ_FOREACH_SAFE(shmem, &vdev->shmem_list, entry, next) {
+        if (shmem->shmid == shmid) {
+            return shmem;
+        }
+    }
+    return NULL;
+}
+
+int virtio_add_shmem_map(VirtSharedMemory *shmem, const char *shm_name,
+                          hwaddr shm_offset, hwaddr fd_offset, uint64_t size,
+                          uint32_t ram_flags, int fd)
+{
+    Error *err = NULL;
+    MappedMemoryRegion *mmap;
+    fd = dup(fd);
+    if (fd < 0) {
+        error_report("Failed to duplicate fd: %s", strerror(errno));
+        return -1;
+    }
+
+    if (shm_offset + size > shmem->mr->size) {
+        error_report("Memory exceeds the shared memory boundaries");
+        close(fd);
+        return -1;
+    }
+
+    mmap = g_new0(MappedMemoryRegion, 1);
+    mmap->mem = g_new0(MemoryRegion, 1);
+    mmap->offset = shm_offset;
+    memory_region_init_ram_from_fd(mmap->mem,
+                                   OBJECT(shmem->mr),
+                                   shm_name, size, ram_flags,
+                                   fd, fd_offset, &err);
+    if (err) {
+        error_report_err(err);
+        close(fd);
+        g_free(mmap->mem);
+        g_free(mmap);
+        return -1;
+    }
+    memory_region_add_subregion(shmem->mr, shm_offset, mmap->mem);
+
+    QTAILQ_INSERT_TAIL(&shmem->mmaps, mmap, link);
+
+    return 0;
+}
+
+MappedMemoryRegion *virtio_find_shmem_map(VirtSharedMemory *shmem,
+                                          hwaddr offset, uint64_t size)
+{
+    MappedMemoryRegion *mmap;
+    QTAILQ_FOREACH(mmap, &shmem->mmaps, link) {
+        if (mmap->offset == offset && mmap->mem->size == size) {
+            return mmap;
+        }
+    }
+    return NULL;
+}
+
+void virtio_del_shmem_map(VirtSharedMemory *shmem, hwaddr offset,
+                          uint64_t size)
+{
+    MappedMemoryRegion *mmap = virtio_find_shmem_map(shmem, offset, size);
+    if (mmap == NULL) {
+        return;
+    }
+
+    object_unparent(OBJECT(mmap->mem));
+    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)
@@ -3511,6 +3597,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);
 }
 
 /*
@@ -4022,11 +4109,21 @@ static void virtio_device_free_virtqueues(VirtIODevice *vdev)
 static void virtio_device_instance_finalize(Object *obj)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(obj);
+    VirtSharedMemory *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)) {
+            MappedMemoryRegion *mmap_reg = QTAILQ_FIRST(&shmem->mmaps);
+            virtio_del_shmem_map(shmem, mmap_reg->offset, mmap_reg->mem->size);
+        }
+        QSIMPLEQ_REMOVE_HEAD(&vdev->shmem_list, entry);
+        g_free(shmem);
+    }
 }
 
 static const Property virtio_properties[] = {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 214d4a77e9..331dbcfbe0 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -98,6 +98,23 @@ enum virtio_device_endian {
     VIRTIO_DEVICE_ENDIAN_BIG,
 };
 
+struct MappedMemoryRegion {
+    MemoryRegion *mem;
+    hwaddr offset;
+    QTAILQ_ENTRY(MappedMemoryRegion) link;
+};
+
+typedef struct MappedMemoryRegion MappedMemoryRegion;
+
+struct VirtSharedMemory {
+    uint8_t shmid;
+    MemoryRegion *mr;
+    QTAILQ_HEAD(, MappedMemoryRegion) mmaps;
+    QSIMPLEQ_ENTRY(VirtSharedMemory) entry;
+};
+
+typedef struct VirtSharedMemory VirtSharedMemory;
+
 /**
  * struct VirtIODevice - common VirtIO structure
  * @name: name of the device
@@ -167,6 +184,8 @@ struct VirtIODevice
      */
     EventNotifier config_notifier;
     bool device_iotlb_enabled;
+    /* Shared memory region for mappings. */
+    QSIMPLEQ_HEAD(, VirtSharedMemory) shmem_list;
 };
 
 struct VirtioDeviceClass {
@@ -289,6 +308,16 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
 
 int virtio_save(VirtIODevice *vdev, QEMUFile *f);
 
+VirtSharedMemory *virtio_new_shmem_region(VirtIODevice *vdev, uint8_t shmid);
+VirtSharedMemory *virtio_find_shmem_region(VirtIODevice *vdev, uint8_t shmid);
+int virtio_add_shmem_map(VirtSharedMemory *shmem, const char *shm_name,
+                          hwaddr shm_offset, hwaddr fd_offset, uint64_t size,
+                          uint32_t ram_flags, int fd);
+MappedMemoryRegion *virtio_find_shmem_map(VirtSharedMemory *shmem,
+                                          hwaddr offset, uint64_t size);
+void virtio_del_shmem_map(VirtSharedMemory *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

Re: [PATCH v5 1/7] vhost-user: Add VirtIO Shared Memory map request

[David Hildenbrand]

On 09.06.25 16:47, Albert Esteve wrote:
> Add SHMEM_MAP/UNMAP requests to vhost-user to
> handle VIRTIO Shared Memory mappings.
> 
> This request allows backends to dynamically map
> fds into a VIRTIO Shared Memory Region indentified
> by its `shmid`. The map is performed by calling
> `memory_region_init_ram_from_fd` and adding the
> new region as a subregion of the shmem container
> MR. Then, the fd memory is advertised to the
> driver as a base addres + offset, so it can be

s/addres/address/

> read/written (depending on the mmap flags
> requested) while it is valid.
> 
> The backend can unmap the memory range
> in a given VIRTIO Shared Memory Region (again,
> identified by its `shmid`), to free it.
> Upon receiving this message, the front-end
> must delete the MR as a subregion of
> the shmem container region and free its
> resources.
> 
> Note that commit all these operations need
> to be delayed to after we respond the request
> to the backend to avoid deadlocks.
> 
> The device model needs to create VirtSharedMemory
> instances for the VirtIO Shared Memory Regions
> and add them to the `VirtIODevice` instance.

Just a general comment: you can use more characters per line in the 
patch desription.


-- 
Cheers,

David / dhildenb

Re: [PATCH v5 1/7] vhost-user: Add VirtIO Shared Memory map request

[Stefan Hajnoczi]

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

On Mon, Jun 09, 2025 at 04:47:23PM +0200, Albert Esteve wrote:
> Add SHMEM_MAP/UNMAP requests to vhost-user to
> handle VIRTIO Shared Memory mappings.
> 
> This request allows backends to dynamically map
> fds into a VIRTIO Shared Memory Region indentified

identified

> by its `shmid`. The map is performed by calling
> `memory_region_init_ram_from_fd` and adding the
> new region as a subregion of the shmem container
> MR. Then, the fd memory is advertised to the
> driver as a base addres + offset, so it can be

address

> read/written (depending on the mmap flags
> requested) while it is valid.

I'm not sure what the last sentence is referring to. Does "driver" mean
a QEMU device model or a VIRTIO driver? Neither of these make a lot of
sense to me.

> 
> The backend can unmap the memory range
> in a given VIRTIO Shared Memory Region (again,
> identified by its `shmid`), to free it.
> Upon receiving this message, the front-end
> must delete the MR as a subregion of
> the shmem container region and free its
> resources.
> 
> Note that commit all these operations need
> to be delayed to after we respond the request
> to the backend to avoid deadlocks.

Rephrased:
"Note the memory region commit for these operations needs to be delayed
until after we respond to the backend to avoid deadlocks."

By the way, what is the reason for the deadlock?

> 
> The device model needs to create VirtSharedMemory
> instances for the VirtIO Shared Memory Regions
> and add them to the `VirtIODevice` instance.
> 
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  hw/virtio/vhost-user.c                    | 150 ++++++++++++++++++++++
>  hw/virtio/virtio.c                        |  97 ++++++++++++++
>  include/hw/virtio/virtio.h                |  29 +++++
>  subprojects/libvhost-user/libvhost-user.c |  70 ++++++++++
>  subprojects/libvhost-user/libvhost-user.h |  54 ++++++++
>  5 files changed, 400 insertions(+)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 1e1d6b0d6e..9c635fb928 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -115,6 +115,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 +194,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 +243,7 @@ typedef union {
>          VhostUserInflight inflight;
>          VhostUserShared object;
>          VhostUserTransferDeviceState transfer_state;
> +        VhostUserMMap mmap;
>  } VhostUserPayload;
>  
>  typedef struct VhostUserMsg {
> @@ -1768,6 +1788,129 @@ vhost_user_backend_handle_shared_object_lookup(struct vhost_user *u,
>      return 0;
>  }
>  
> +static int
> +vhost_user_backend_handle_shmem_map(struct vhost_dev *dev,
> +                                    QIOChannel *ioc,
> +                                    VhostUserHeader *hdr,
> +                                    VhostUserPayload *payload,
> +                                    int fd)
> +{
> +    uint32_t ram_flags;
> +    VirtSharedMemory *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();
> +    ram_flags = RAM_SHARED |
> +                ((vu_mmap->flags & VHOST_USER_FLAG_MAP_RW) ? 0 : RAM_READONLY);
> +    if (virtio_add_shmem_map(shmem, shm_name->str, vu_mmap->shm_offset,
> +                             vu_mmap->fd_offset, vu_mmap->len, ram_flags,
> +                             fd) != 0) {
> +        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;
> +}
> +
> +static int
> +vhost_user_backend_handle_shmem_unmap(struct vhost_dev *dev,
> +                                      QIOChannel *ioc,
> +                                      VhostUserHeader *hdr,
> +                                      VhostUserPayload *payload)
> +{
> +    VirtSharedMemory *shmem;
> +    MappedMemoryRegion *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) {
> +        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);
> @@ -1836,6 +1979,13 @@ static gboolean backend_read(QIOChannel *ioc, GIOCondition condition,
>          ret = vhost_user_backend_handle_shared_object_lookup(dev->opaque, ioc,
>                                                               &hdr, &payload);
>          break;
> +    case VHOST_USER_BACKEND_SHMEM_MAP:
> +        ret = vhost_user_backend_handle_shmem_map(dev, ioc, &hdr, &payload,
> +                                                  fd ? fd[0] : -1);
> +        break;
> +    case VHOST_USER_BACKEND_SHMEM_UNMAP:
> +        ret = vhost_user_backend_handle_shmem_unmap(dev, ioc, &hdr, &payload);
> +        break;
>      default:
>          error_report("Received unexpected msg type: %d.", hdr.request);
>          ret = -EINVAL;
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 5534251e01..208ad11685 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3035,6 +3035,92 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
>      return vmstate_save_state(f, &vmstate_virtio, vdev, NULL);
>  }
>  
> +VirtSharedMemory *virtio_new_shmem_region(VirtIODevice *vdev, uint8_t shmid)

The caller is not responsible for freeing the returned VirtSharedMemory.
Please add doc comments for the new APIs explaining these things.

> +{
> +    VirtSharedMemory *elem;
> +    elem = g_new0(VirtSharedMemory, 1);
> +    elem->shmid = shmid;
> +    elem->mr = g_new0(MemoryRegion, 1);

Maybe this will become clear in later commits, but at this point I'm
confused about elem->mr's lifecycle:
- memory_region_init() is not called?
- Where is this freed?
- Does it really need to be allocated on the heap instead of being
  embedded as a field within VirtSharedMemory?

> +    QTAILQ_INIT(&elem->mmaps);
> +    QSIMPLEQ_INSERT_TAIL(&vdev->shmem_list, elem, entry);
> +    return QSIMPLEQ_LAST(&vdev->shmem_list, VirtSharedMemory, entry);

More concise:

  return elem;

> +}
> +
> +VirtSharedMemory *virtio_find_shmem_region(VirtIODevice *vdev, uint8_t shmid)
> +{
> +    VirtSharedMemory *shmem, *next;
> +    QSIMPLEQ_FOREACH_SAFE(shmem, &vdev->shmem_list, entry, next) {
> +        if (shmem->shmid == shmid) {
> +            return shmem;
> +        }
> +    }
> +    return NULL;
> +}
> +
> +int virtio_add_shmem_map(VirtSharedMemory *shmem, const char *shm_name,
> +                          hwaddr shm_offset, hwaddr fd_offset, uint64_t size,
> +                          uint32_t ram_flags, int fd)
> +{
> +    Error *err = NULL;
> +    MappedMemoryRegion *mmap;
> +    fd = dup(fd);
> +    if (fd < 0) {
> +        error_report("Failed to duplicate fd: %s", strerror(errno));
> +        return -1;
> +    }
> +
> +    if (shm_offset + size > shmem->mr->size) {
> +        error_report("Memory exceeds the shared memory boundaries");
> +        close(fd);
> +        return -1;
> +    }
> +
> +    mmap = g_new0(MappedMemoryRegion, 1);
> +    mmap->mem = g_new0(MemoryRegion, 1);
> +    mmap->offset = shm_offset;
> +    memory_region_init_ram_from_fd(mmap->mem,
> +                                   OBJECT(shmem->mr),
> +                                   shm_name, size, ram_flags,
> +                                   fd, fd_offset, &err);
> +    if (err) {
> +        error_report_err(err);

There is an opportunity here to propagate errors to the caller by
adjusting the function prototype:

  bool virtio_add_shmem_map(..., Error **errp)

I don't know if the caller is going to do anything with the error
though, so I guess we can leave things as they are for now.

> +        close(fd);
> +        g_free(mmap->mem);
> +        g_free(mmap);
> +        return -1;
> +    }
> +    memory_region_add_subregion(shmem->mr, shm_offset, mmap->mem);
> +
> +    QTAILQ_INSERT_TAIL(&shmem->mmaps, mmap, link);
> +
> +    return 0;
> +}
> +
> +MappedMemoryRegion *virtio_find_shmem_map(VirtSharedMemory *shmem,
> +                                          hwaddr offset, uint64_t size)
> +{
> +    MappedMemoryRegion *mmap;
> +    QTAILQ_FOREACH(mmap, &shmem->mmaps, link) {
> +        if (mmap->offset == offset && mmap->mem->size == size) {
> +            return mmap;
> +        }
> +    }
> +    return NULL;
> +}
> +
> +void virtio_del_shmem_map(VirtSharedMemory *shmem, hwaddr offset,
> +                          uint64_t size)
> +{
> +    MappedMemoryRegion *mmap = virtio_find_shmem_map(shmem, offset, size);
> +    if (mmap == NULL) {
> +        return;
> +    }
> +
> +    object_unparent(OBJECT(mmap->mem));

I'm not 100% confident about the lifecycle of the MemoryRegions in this
patch. There is a section in docs/devel/memory.rst with warnings about
dynamically destroying MemoryRegions at runtime.

I think a reference count is leaked because
memory_region_add_subregion() increments the refcount rather than taking
over the caller's refcount.

Using memory_region_del_subregion() + memory_region_unref() would be
clearer, but may violate the rule in the memory.rst documentation,
depending on when this function is called.

> +    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)
> @@ -3511,6 +3597,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);
>  }
>  
>  /*
> @@ -4022,11 +4109,21 @@ static void virtio_device_free_virtqueues(VirtIODevice *vdev)
>  static void virtio_device_instance_finalize(Object *obj)
>  {
>      VirtIODevice *vdev = VIRTIO_DEVICE(obj);
> +    VirtSharedMemory *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)) {
> +            MappedMemoryRegion *mmap_reg = QTAILQ_FIRST(&shmem->mmaps);
> +            virtio_del_shmem_map(shmem, mmap_reg->offset, mmap_reg->mem->size);
> +        }
> +        QSIMPLEQ_REMOVE_HEAD(&vdev->shmem_list, entry);
> +        g_free(shmem);
> +    }
>  }
>  
>  static const Property virtio_properties[] = {
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 214d4a77e9..331dbcfbe0 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -98,6 +98,23 @@ enum virtio_device_endian {
>      VIRTIO_DEVICE_ENDIAN_BIG,
>  };
>  
> +struct MappedMemoryRegion {
> +    MemoryRegion *mem;
> +    hwaddr offset;
> +    QTAILQ_ENTRY(MappedMemoryRegion) link;
> +};
> +
> +typedef struct MappedMemoryRegion MappedMemoryRegion;
> +
> +struct VirtSharedMemory {
> +    uint8_t shmid;
> +    MemoryRegion *mr;
> +    QTAILQ_HEAD(, MappedMemoryRegion) mmaps;
> +    QSIMPLEQ_ENTRY(VirtSharedMemory) entry;
> +};
> +
> +typedef struct VirtSharedMemory VirtSharedMemory;

It would be nice to prefix the type names for namespacing reasons:
VirtSharedMemory -> VirtioSharedMemory
MappedMemoryRegion -> VirtioSharedMemoryMapping

> +
>  /**
>   * struct VirtIODevice - common VirtIO structure
>   * @name: name of the device
> @@ -167,6 +184,8 @@ struct VirtIODevice
>       */
>      EventNotifier config_notifier;
>      bool device_iotlb_enabled;
> +    /* Shared memory region for mappings. */
> +    QSIMPLEQ_HEAD(, VirtSharedMemory) shmem_list;
>  };
>  
>  struct VirtioDeviceClass {
> @@ -289,6 +308,16 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
>  
>  int virtio_save(VirtIODevice *vdev, QEMUFile *f);
>  
> +VirtSharedMemory *virtio_new_shmem_region(VirtIODevice *vdev, uint8_t shmid);
> +VirtSharedMemory *virtio_find_shmem_region(VirtIODevice *vdev, uint8_t shmid);
> +int virtio_add_shmem_map(VirtSharedMemory *shmem, const char *shm_name,
> +                          hwaddr shm_offset, hwaddr fd_offset, uint64_t size,
> +                          uint32_t ram_flags, int fd);
> +MappedMemoryRegion *virtio_find_shmem_map(VirtSharedMemory *shmem,
> +                                          hwaddr offset, uint64_t size);
> +void virtio_del_shmem_map(VirtSharedMemory *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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v5 1/7] vhost-user: Add VirtIO Shared Memory map request

[Albert Esteve]

Sorry I did not reply-all.

Albert Esteve

Principal Software Engineer

Red Hat

aesteve@redhat.com



On Mon, Jun 16, 2025 at 5:28 PM Albert Esteve <aesteve@redhat.com> wrote:
>
>
>
> On Thu, Jun 12, 2025 at 6:19 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Mon, Jun 09, 2025 at 04:47:23PM +0200, Albert Esteve wrote:
> > > Add SHMEM_MAP/UNMAP requests to vhost-user to
> > > handle VIRTIO Shared Memory mappings.
> > >
> > > This request allows backends to dynamically map
> > > fds into a VIRTIO Shared Memory Region indentified
> >
> > identified
> >
> > > by its `shmid`. The map is performed by calling
> > > `memory_region_init_ram_from_fd` and adding the
> > > new region as a subregion of the shmem container
> > > MR. Then, the fd memory is advertised to the
> > > driver as a base addres + offset, so it can be
> >
> > address
> >
> > > read/written (depending on the mmap flags
> > > requested) while it is valid.
> >
> > I'm not sure what the last sentence is referring to. Does "driver" mean
> > a QEMU device model or a VIRTIO driver? Neither of these make a lot of
> > sense to me.
> >
> If I remember correctly, I mean the VIRTIO driver. What I was trying to say, I think, is that in order to access the mmap'd memory from the driver, we need the full address+offset and not only the offset.
> > >
> > > The backend can unmap the memory range
> > > in a given VIRTIO Shared Memory Region (again,
> > > identified by its `shmid`), to free it.
> > > Upon receiving this message, the front-end
> > > must delete the MR as a subregion of
> > > the shmem container region and free its
> > > resources.
> > >
> > > Note that commit all these operations need
> > > to be delayed to after we respond the request
> > > to the backend to avoid deadlocks.
> >
> > Rephrased:
> > "Note the memory region commit for these operations needs to be delayed
> > until after we respond to the backend to avoid deadlocks."
> >
> > By the way, what is the reason for the deadlock?
>
> I may not explain it correctly, but I'll try. Maybe @David Hildenbrand can keep me honest on this one, or correct any misspeak.
>
> So the problem with committing the memory operation after the add_subregion, is that it requires a VIRTIO message and an ACK. But if the VIRTIO device that sends the message that requested the mmap is blocked waiting for an ACK to the mmap from the VMM. As a result, it will not process the commit message, it will not send its ACK, and thus the deadlock. The commit will never finish, because the device is waiting for the mmap operation to finish, which is locked on the commit.
>
> >
> > >
> > > The device model needs to create VirtSharedMemory
> > > instances for the VirtIO Shared Memory Regions
> > > and add them to the `VirtIODevice` instance.
> > >
> > > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > > ---
> > >  hw/virtio/vhost-user.c                    | 150 ++++++++++++++++++++++
> > >  hw/virtio/virtio.c                        |  97 ++++++++++++++
> > >  include/hw/virtio/virtio.h                |  29 +++++
> > >  subprojects/libvhost-user/libvhost-user.c |  70 ++++++++++
> > >  subprojects/libvhost-user/libvhost-user.h |  54 ++++++++
> > >  5 files changed, 400 insertions(+)
> > >
> > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > index 1e1d6b0d6e..9c635fb928 100644
> > > --- a/hw/virtio/vhost-user.c
> > > +++ b/hw/virtio/vhost-user.c
> > > @@ -115,6 +115,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 +194,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 +243,7 @@ typedef union {
> > >          VhostUserInflight inflight;
> > >          VhostUserShared object;
> > >          VhostUserTransferDeviceState transfer_state;
> > > +        VhostUserMMap mmap;
> > >  } VhostUserPayload;
> > >
> > >  typedef struct VhostUserMsg {
> > > @@ -1768,6 +1788,129 @@ vhost_user_backend_handle_shared_object_lookup(struct vhost_user *u,
> > >      return 0;
> > >  }
> > >
> > > +static int
> > > +vhost_user_backend_handle_shmem_map(struct vhost_dev *dev,
> > > +                                    QIOChannel *ioc,
> > > +                                    VhostUserHeader *hdr,
> > > +                                    VhostUserPayload *payload,
> > > +                                    int fd)
> > > +{
> > > +    uint32_t ram_flags;
> > > +    VirtSharedMemory *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();
> > > +    ram_flags = RAM_SHARED |
> > > +                ((vu_mmap->flags & VHOST_USER_FLAG_MAP_RW) ? 0 : RAM_READONLY);
> > > +    if (virtio_add_shmem_map(shmem, shm_name->str, vu_mmap->shm_offset,
> > > +                             vu_mmap->fd_offset, vu_mmap->len, ram_flags,
> > > +                             fd) != 0) {
> > > +        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;
> > > +}
> > > +
> > > +static int
> > > +vhost_user_backend_handle_shmem_unmap(struct vhost_dev *dev,
> > > +                                      QIOChannel *ioc,
> > > +                                      VhostUserHeader *hdr,
> > > +                                      VhostUserPayload *payload)
> > > +{
> > > +    VirtSharedMemory *shmem;
> > > +    MappedMemoryRegion *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) {
> > > +        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);
> > > @@ -1836,6 +1979,13 @@ static gboolean backend_read(QIOChannel *ioc, GIOCondition condition,
> > >          ret = vhost_user_backend_handle_shared_object_lookup(dev->opaque, ioc,
> > >                                                               &hdr, &payload);
> > >          break;
> > > +    case VHOST_USER_BACKEND_SHMEM_MAP:
> > > +        ret = vhost_user_backend_handle_shmem_map(dev, ioc, &hdr, &payload,
> > > +                                                  fd ? fd[0] : -1);
> > > +        break;
> > > +    case VHOST_USER_BACKEND_SHMEM_UNMAP:
> > > +        ret = vhost_user_backend_handle_shmem_unmap(dev, ioc, &hdr, &payload);
> > > +        break;
> > >      default:
> > >          error_report("Received unexpected msg type: %d.", hdr.request);
> > >          ret = -EINVAL;
> > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > > index 5534251e01..208ad11685 100644
> > > --- a/hw/virtio/virtio.c
> > > +++ b/hw/virtio/virtio.c
> > > @@ -3035,6 +3035,92 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
> > >      return vmstate_save_state(f, &vmstate_virtio, vdev, NULL);
> > >  }
> > >
> > > +VirtSharedMemory *virtio_new_shmem_region(VirtIODevice *vdev, uint8_t shmid)
> >
> > The caller is not responsible for freeing the returned VirtSharedMemory.
> > Please add doc comments for the new APIs explaining these things.
>
> Sure, I'll do in the next version.
>
> >
> > > +{
> > > +    VirtSharedMemory *elem;
> > > +    elem = g_new0(VirtSharedMemory, 1);
> > > +    elem->shmid = shmid;
> > > +    elem->mr = g_new0(MemoryRegion, 1);
> >
> > Maybe this will become clear in later commits, but at this point I'm
> > confused about elem->mr's lifecycle:
> > - memory_region_init() is not called?
> You are right? I did not see it. Then, I thought that as it is a container region it may not need to be initialised. But I checked the memory API document and indeed it does.
>
> I tested this patch and worked. Not sure why. I will initialise it for the next version.
>
> > - Where is this freed?
> WIthin `virtio_device_instance_finalize`.
>
> > - Does it really need to be allocated on the heap instead of being
> >   embedded as a field within VirtSharedMemory?
> Not sure? It is already a field within `VirtSharedMemory`. How would you prefer it to be allocated?
>
> >
> > > +    QTAILQ_INIT(&elem->mmaps);
> > > +    QSIMPLEQ_INSERT_TAIL(&vdev->shmem_list, elem, entry);
> > > +    return QSIMPLEQ_LAST(&vdev->shmem_list, VirtSharedMemory, entry);
> >
> > More concise:
> >
> >   return elem;
> >
> > > +}
> > > +
> > > +VirtSharedMemory *virtio_find_shmem_region(VirtIODevice *vdev, uint8_t shmid)
> > > +{
> > > +    VirtSharedMemory *shmem, *next;
> > > +    QSIMPLEQ_FOREACH_SAFE(shmem, &vdev->shmem_list, entry, next) {
> > > +        if (shmem->shmid == shmid) {
> > > +            return shmem;
> > > +        }
> > > +    }
> > > +    return NULL;
> > > +}
> > > +
> > > +int virtio_add_shmem_map(VirtSharedMemory *shmem, const char *shm_name,
> > > +                          hwaddr shm_offset, hwaddr fd_offset, uint64_t size,
> > > +                          uint32_t ram_flags, int fd)
> > > +{
> > > +    Error *err = NULL;
> > > +    MappedMemoryRegion *mmap;
> > > +    fd = dup(fd);
> > > +    if (fd < 0) {
> > > +        error_report("Failed to duplicate fd: %s", strerror(errno));
> > > +        return -1;
> > > +    }
> > > +
> > > +    if (shm_offset + size > shmem->mr->size) {
> > > +        error_report("Memory exceeds the shared memory boundaries");
> > > +        close(fd);
> > > +        return -1;
> > > +    }
> > > +
> > > +    mmap = g_new0(MappedMemoryRegion, 1);
> > > +    mmap->mem = g_new0(MemoryRegion, 1);
> > > +    mmap->offset = shm_offset;
> > > +    memory_region_init_ram_from_fd(mmap->mem,
> > > +                                   OBJECT(shmem->mr),
> > > +                                   shm_name, size, ram_flags,
> > > +                                   fd, fd_offset, &err);
> > > +    if (err) {
> > > +        error_report_err(err);
> >
> > There is an opportunity here to propagate errors to the caller by
> > adjusting the function prototype:
> >
> >   bool virtio_add_shmem_map(..., Error **errp)
> >
> > I don't know if the caller is going to do anything with the error
> > though, so I guess we can leave things as they are for now.
>
> I had some back and forth myself on what can or cannot be propagated. I changed that prototype a couple times. I will give it another iteration and see if it is worth changing again.
>
> >
> > > +        close(fd);
> > > +        g_free(mmap->mem);
> > > +        g_free(mmap);
> > > +        return -1;
> > > +    }
> > > +    memory_region_add_subregion(shmem->mr, shm_offset, mmap->mem);
> > > +
> > > +    QTAILQ_INSERT_TAIL(&shmem->mmaps, mmap, link);
> > > +
> > > +    return 0;
> > > +}
> > > +
> > > +MappedMemoryRegion *virtio_find_shmem_map(VirtSharedMemory *shmem,
> > > +                                          hwaddr offset, uint64_t size)
> > > +{
> > > +    MappedMemoryRegion *mmap;
> > > +    QTAILQ_FOREACH(mmap, &shmem->mmaps, link) {
> > > +        if (mmap->offset == offset && mmap->mem->size == size) {
> > > +            return mmap;
> > > +        }
> > > +    }
> > > +    return NULL;
> > > +}
> > > +
> > > +void virtio_del_shmem_map(VirtSharedMemory *shmem, hwaddr offset,
> > > +                          uint64_t size)
> > > +{
> > > +    MappedMemoryRegion *mmap = virtio_find_shmem_map(shmem, offset, size);
> > > +    if (mmap == NULL) {
> > > +        return;
> > > +    }
> > > +
> > > +    object_unparent(OBJECT(mmap->mem));
> >
> > I'm not 100% confident about the lifecycle of the MemoryRegions in this
> > patch. There is a section in docs/devel/memory.rst with warnings about
> > dynamically destroying MemoryRegions at runtime.
>
> Yes, I remember reading that part. I think I was focused on making it work, and probably disregarded the warning assuming the MR is not reference anywhere.
>
> >
> > I think a reference count is leaked because
> > memory_region_add_subregion() increments the refcount rather than taking
> > over the caller's refcount.
>
> I did not know that.
>
> >
> > Using memory_region_del_subregion() + memory_region_unref() would be
> > clearer, but may violate the rule in the memory.rst documentation,
> > depending on when this function is called.
>
> To be honest I am not sure what's the best option moving forward. The feature definitively needs dynamically allocated data structures. Maybe using subregions was not he right strategy after all?
>
> Can we have some compromise solution here?
>
> >
> > > +    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)
> > > @@ -3511,6 +3597,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);
> > >  }
> > >
> > >  /*
> > > @@ -4022,11 +4109,21 @@ static void virtio_device_free_virtqueues(VirtIODevice *vdev)
> > >  static void virtio_device_instance_finalize(Object *obj)
> > >  {
> > >      VirtIODevice *vdev = VIRTIO_DEVICE(obj);
> > > +    VirtSharedMemory *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)) {
> > > +            MappedMemoryRegion *mmap_reg = QTAILQ_FIRST(&shmem->mmaps);
> > > +            virtio_del_shmem_map(shmem, mmap_reg->offset, mmap_reg->mem->size);
> > > +        }
> > > +        QSIMPLEQ_REMOVE_HEAD(&vdev->shmem_list, entry);
> > > +        g_free(shmem);
> > > +    }
> > >  }
> > >
> > >  static const Property virtio_properties[] = {
> > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > > index 214d4a77e9..331dbcfbe0 100644
> > > --- a/include/hw/virtio/virtio.h
> > > +++ b/include/hw/virtio/virtio.h
> > > @@ -98,6 +98,23 @@ enum virtio_device_endian {
> > >      VIRTIO_DEVICE_ENDIAN_BIG,
> > >  };
> > >
> > > +struct MappedMemoryRegion {
> > > +    MemoryRegion *mem;
> > > +    hwaddr offset;
> > > +    QTAILQ_ENTRY(MappedMemoryRegion) link;
> > > +};
> > > +
> > > +typedef struct MappedMemoryRegion MappedMemoryRegion;
> > > +
> > > +struct VirtSharedMemory {
> > > +    uint8_t shmid;
> > > +    MemoryRegion *mr;
> > > +    QTAILQ_HEAD(, MappedMemoryRegion) mmaps;
> > > +    QSIMPLEQ_ENTRY(VirtSharedMemory) entry;
> > > +};
> > > +
> > > +typedef struct VirtSharedMemory VirtSharedMemory;
> >
> > It would be nice to prefix the type names for namespacing reasons:
> > VirtSharedMemory -> VirtioSharedMemory
> > MappedMemoryRegion -> VirtioSharedMemoryMapping
> >
> > > +
> > >  /**
> > >   * struct VirtIODevice - common VirtIO structure
> > >   * @name: name of the device
> > > @@ -167,6 +184,8 @@ struct VirtIODevice
> > >       */
> > >      EventNotifier config_notifier;
> > >      bool device_iotlb_enabled;
> > > +    /* Shared memory region for mappings. */
> > > +    QSIMPLEQ_HEAD(, VirtSharedMemory) shmem_list;
> > >  };
> > >
> > >  struct VirtioDeviceClass {
> > > @@ -289,6 +308,16 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
> > >
> > >  int virtio_save(VirtIODevice *vdev, QEMUFile *f);
> > >
> > > +VirtSharedMemory *virtio_new_shmem_region(VirtIODevice *vdev, uint8_t shmid);
> > > +VirtSharedMemory *virtio_find_shmem_region(VirtIODevice *vdev, uint8_t shmid);
> > > +int virtio_add_shmem_map(VirtSharedMemory *shmem, const char *shm_name,
> > > +                          hwaddr shm_offset, hwaddr fd_offset, uint64_t size,
> > > +                          uint32_t ram_flags, int fd);
> > > +MappedMemoryRegion *virtio_find_shmem_map(VirtSharedMemory *shmem,
> > > +                                          hwaddr offset, uint64_t size);
> > > +void virtio_del_shmem_map(VirtSharedMemory *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
> > >

[PATCH v5 2/7] vhost_user.rst: Align VhostUserMsg excerpt members

[Albert Esteve]

Add missing members to the VhostUserMsg excerpt in
the vhost-user spec documentation.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 docs/interop/vhost-user.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 2e50f2ddfa..436a94c0ee 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -366,11 +366,15 @@ In QEMU the vhost-user message is implemented with the following struct:
           struct vhost_vring_state state;
           struct vhost_vring_addr addr;
           VhostUserMemory memory;
+          VhostUserMemRegMsg mem_reg;
           VhostUserLog log;
           struct vhost_iotlb_msg iotlb;
           VhostUserConfig config;
+          VhostUserCryptoSession session;
           VhostUserVringArea area;
           VhostUserInflight inflight;
+          VhostUserShared object;
+          VhostUserTransferDeviceState transfer_state;
       };
   } QEMU_PACKED VhostUserMsg;
 
-- 
2.49.0

Re: [PATCH v5 2/7] vhost_user.rst: Align VhostUserMsg excerpt members

[David Hildenbrand]

On 09.06.25 16:47, Albert Esteve wrote:
> Add missing members to the VhostUserMsg excerpt in
> the vhost-user spec documentation.
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

[PATCH v5 3/7] vhost_user.rst: Add SHMEM_MAP/_UNMAP to spec

[Albert Esteve]

Add SHMEM_MAP/_UNMAP request to the vhost-user
spec documentation.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 docs/interop/vhost-user.rst | 55 +++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 436a94c0ee..b623284819 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -350,6 +350,27 @@ Device state transfer parameters
   In the future, additional phases might be added e.g. to allow
   iterative migration while the device is running.
 
+MMAP request
+^^^^^^^^^^^^
+
++-------+---------+-----------+------------+-----+-------+
+| shmid | padding | fd_offset | shm_offset | len | flags |
++-------+---------+-----------+------------+-----+-------+
+
+:shmid: a 8-bit shared memory region identifier
+
+:fd_offset: a 64-bit offset of this area from the start
+            of the supplied file descriptor
+
+:shm_offset: a 64-bit offset from the start of the
+             pointed shared memory region
+
+:len: a 64-bit size of the memory to map
+
+:flags: a 64-bit value:
+  - 0: Pages are mapped read-only
+  - 1: Pages are mapped read-write
+
 C structure
 -----------
 
@@ -375,6 +396,7 @@ In QEMU the vhost-user message is implemented with the following struct:
           VhostUserInflight inflight;
           VhostUserShared object;
           VhostUserTransferDeviceState transfer_state;
+          VhostUserMMap mmap;
       };
   } QEMU_PACKED VhostUserMsg;
 
@@ -1057,6 +1079,7 @@ Protocol features
   #define VHOST_USER_PROTOCOL_F_XEN_MMAP             17
   #define VHOST_USER_PROTOCOL_F_SHARED_OBJECT        18
   #define VHOST_USER_PROTOCOL_F_DEVICE_STATE         19
+  #define VHOST_USER_PROTOCOL_F_SHMEM                20
 
 Front-end message types
 -----------------------
@@ -1865,6 +1888,38 @@ is sent by the front-end.
   when the operation is successful, or non-zero otherwise. Note that if the
   operation fails, no fd is sent to the backend.
 
+``VHOST_USER_BACKEND_SHMEM_MAP``
+  :id: 9
+  :equivalent ioctl: N/A
+  :request payload: fd and ``struct VhostUserMMap``
+  :reply payload: N/A
+
+  When the ``VHOST_USER_PROTOCOL_F_SHMEM`` protocol feature has been
+  successfully negotiated, this message can be submitted by the backends to
+  advertise a new mapping to be made in a given VIRTIO Shared Memory Region.
+  Upon receiving the message, the front-end will mmap the given fd into the
+  VIRTIO Shared Memory Region with the requested ``shmid``. A reply is
+  generated indicating whether mapping succeeded.
+
+  Mapping over an already existing map is not allowed and request shall fail.
+  Therefore, the memory range in the request must correspond with a valid,
+  free region of the VIRTIO Shared Memory Region. Also, note that mappings
+  consume resources and that the request can fail when there are no resources
+  available.
+
+``VHOST_USER_BACKEND_SHMEM_UNMAP``
+  :id: 10
+  :equivalent ioctl: N/A
+  :request payload: ``struct VhostUserMMap``
+  :reply payload: N/A
+
+  When the ``VHOST_USER_PROTOCOL_F_SHMEM`` protocol feature has been
+  successfully negotiated, this message can be submitted by the backends so
+  that the front-end un-mmap a given range (``shm_offset``, ``len``) in the
+  VIRTIO Shared Memory Region with the requested ``shmid``. Note that the
+  given range shall correspond to the entirety of a valid mapped region.
+  A reply is generated indicating whether unmapping succeeded.
+
 .. _reply_ack:
 
 VHOST_USER_PROTOCOL_F_REPLY_ACK
-- 
2.49.0

Re: [PATCH v5 3/7] vhost_user.rst: Add SHMEM_MAP/_UNMAP to spec

[Alyssa Ross]

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

Albert Esteve <aesteve@redhat.com> writes:

> Add SHMEM_MAP/_UNMAP request to the vhost-user
> spec documentation.
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  docs/interop/vhost-user.rst | 55 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index 436a94c0ee..b623284819 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -350,6 +350,27 @@ Device state transfer parameters
>    In the future, additional phases might be added e.g. to allow
>    iterative migration while the device is running.
>  
> +MMAP request
> +^^^^^^^^^^^^
> +
> ++-------+---------+-----------+------------+-----+-------+
> +| shmid | padding | fd_offset | shm_offset | len | flags |
> ++-------+---------+-----------+------------+-----+-------+
> +
> +:shmid: a 8-bit shared memory region identifier
> +
> +:fd_offset: a 64-bit offset of this area from the start
> +            of the supplied file descriptor
> +
> +:shm_offset: a 64-bit offset from the start of the
> +             pointed shared memory region
> +
> +:len: a 64-bit size of the memory to map
> +
> +:flags: a 64-bit value:
> +  - 0: Pages are mapped read-only
> +  - 1: Pages are mapped read-write
> +
>  C structure
>  -----------
>  
> @@ -375,6 +396,7 @@ In QEMU the vhost-user message is implemented with the following struct:
>            VhostUserInflight inflight;
>            VhostUserShared object;
>            VhostUserTransferDeviceState transfer_state;
> +          VhostUserMMap mmap;
>        };
>    } QEMU_PACKED VhostUserMsg;
>  
> @@ -1057,6 +1079,7 @@ Protocol features
>    #define VHOST_USER_PROTOCOL_F_XEN_MMAP             17
>    #define VHOST_USER_PROTOCOL_F_SHARED_OBJECT        18
>    #define VHOST_USER_PROTOCOL_F_DEVICE_STATE         19
> +  #define VHOST_USER_PROTOCOL_F_SHMEM                20
>  
>  Front-end message types
>  -----------------------
> @@ -1865,6 +1888,38 @@ is sent by the front-end.
>    when the operation is successful, or non-zero otherwise. Note that if the
>    operation fails, no fd is sent to the backend.
>  
> +``VHOST_USER_BACKEND_SHMEM_MAP``
> +  :id: 9
> +  :equivalent ioctl: N/A
> +  :request payload: fd and ``struct VhostUserMMap``
> +  :reply payload: N/A
> +
> +  When the ``VHOST_USER_PROTOCOL_F_SHMEM`` protocol feature has been
> +  successfully negotiated, this message can be submitted by the backends to
> +  advertise a new mapping to be made in a given VIRTIO Shared Memory Region.
> +  Upon receiving the message, the front-end will mmap the given fd into the
> +  VIRTIO Shared Memory Region with the requested ``shmid``. A reply is
> +  generated indicating whether mapping succeeded.

Should this be phrased to make it clear replies are only generated in
some cases, like how e.g. VHOST_USER_BACKEND_IOTLB_MSG phrases it?

	If ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` is negotiated, and
	back-end set the ``VHOST_USER_NEED_REPLY`` flag, the front-end
	must respond with zero when operation is successfully completed,
	or non-zero otherwise.

> +
> +  Mapping over an already existing map is not allowed and request shall fail.

request*s* shall fail

> +  Therefore, the memory range in the request must correspond with a valid,
> +  free region of the VIRTIO Shared Memory Region. Also, note that mappings
> +  consume resources and that the request can fail when there are no resources
> +  available.
> +
> +``VHOST_USER_BACKEND_SHMEM_UNMAP``
> +  :id: 10
> +  :equivalent ioctl: N/A
> +  :request payload: ``struct VhostUserMMap``
> +  :reply payload: N/A
> +
> +  When the ``VHOST_USER_PROTOCOL_F_SHMEM`` protocol feature has been
> +  successfully negotiated, this message can be submitted by the backends so
> +  that the front-end un-mmap a given range (``shm_offset``, ``len``) in the

un-mmaps?

> +  VIRTIO Shared Memory Region with the requested ``shmid``. Note that the
> +  given range shall correspond to the entirety of a valid mapped region.
> +  A reply is generated indicating whether unmapping succeeded.
> +
>  .. _reply_ack:
>  
>  VHOST_USER_PROTOCOL_F_REPLY_ACK
> -- 
> 2.49.0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH v5 3/7] vhost_user.rst: Add SHMEM_MAP/_UNMAP to spec

[Albert Esteve]

On Wed, Jun 11, 2025 at 8:21 AM Alyssa Ross <hi@alyssa.is> wrote:
>
> Albert Esteve <aesteve@redhat.com> writes:
>
> > Add SHMEM_MAP/_UNMAP request to the vhost-user
> > spec documentation.
> >
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
> >  docs/interop/vhost-user.rst | 55 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 55 insertions(+)
> >
> > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> > index 436a94c0ee..b623284819 100644
> > --- a/docs/interop/vhost-user.rst
> > +++ b/docs/interop/vhost-user.rst
> > @@ -350,6 +350,27 @@ Device state transfer parameters
> >    In the future, additional phases might be added e.g. to allow
> >    iterative migration while the device is running.
> >
> > +MMAP request
> > +^^^^^^^^^^^^
> > +
> > ++-------+---------+-----------+------------+-----+-------+
> > +| shmid | padding | fd_offset | shm_offset | len | flags |
> > ++-------+---------+-----------+------------+-----+-------+
> > +
> > +:shmid: a 8-bit shared memory region identifier
> > +
> > +:fd_offset: a 64-bit offset of this area from the start
> > +            of the supplied file descriptor
> > +
> > +:shm_offset: a 64-bit offset from the start of the
> > +             pointed shared memory region
> > +
> > +:len: a 64-bit size of the memory to map
> > +
> > +:flags: a 64-bit value:
> > +  - 0: Pages are mapped read-only
> > +  - 1: Pages are mapped read-write
> > +
> >  C structure
> >  -----------
> >
> > @@ -375,6 +396,7 @@ In QEMU the vhost-user message is implemented with the following struct:
> >            VhostUserInflight inflight;
> >            VhostUserShared object;
> >            VhostUserTransferDeviceState transfer_state;
> > +          VhostUserMMap mmap;
> >        };
> >    } QEMU_PACKED VhostUserMsg;
> >
> > @@ -1057,6 +1079,7 @@ Protocol features
> >    #define VHOST_USER_PROTOCOL_F_XEN_MMAP             17
> >    #define VHOST_USER_PROTOCOL_F_SHARED_OBJECT        18
> >    #define VHOST_USER_PROTOCOL_F_DEVICE_STATE         19
> > +  #define VHOST_USER_PROTOCOL_F_SHMEM                20
> >
> >  Front-end message types
> >  -----------------------
> > @@ -1865,6 +1888,38 @@ is sent by the front-end.
> >    when the operation is successful, or non-zero otherwise. Note that if the
> >    operation fails, no fd is sent to the backend.
> >
> > +``VHOST_USER_BACKEND_SHMEM_MAP``
> > +  :id: 9
> > +  :equivalent ioctl: N/A
> > +  :request payload: fd and ``struct VhostUserMMap``
> > +  :reply payload: N/A
> > +
> > +  When the ``VHOST_USER_PROTOCOL_F_SHMEM`` protocol feature has been
> > +  successfully negotiated, this message can be submitted by the backends to
> > +  advertise a new mapping to be made in a given VIRTIO Shared Memory Region.
> > +  Upon receiving the message, the front-end will mmap the given fd into the
> > +  VIRTIO Shared Memory Region with the requested ``shmid``. A reply is
> > +  generated indicating whether mapping succeeded.
>
> Should this be phrased to make it clear replies are only generated in
> some cases, like how e.g. VHOST_USER_BACKEND_IOTLB_MSG phrases it?
>
>         If ``VHOST_USER_PROTOCOL_F_REPLY_ACK`` is negotiated, and
>         back-end set the ``VHOST_USER_NEED_REPLY`` flag, the front-end
>         must respond with zero when operation is successfully completed,
>         or non-zero otherwise.
>

Right! Thanks for the catch. I will fix it (and the others in your
comment) in the next version.

BR,
Albert.

> > +
> > +  Mapping over an already existing map is not allowed and request shall fail.
>
> request*s* shall fail
>
> > +  Therefore, the memory range in the request must correspond with a valid,
> > +  free region of the VIRTIO Shared Memory Region. Also, note that mappings
> > +  consume resources and that the request can fail when there are no resources
> > +  available.
> > +
> > +``VHOST_USER_BACKEND_SHMEM_UNMAP``
> > +  :id: 10
> > +  :equivalent ioctl: N/A
> > +  :request payload: ``struct VhostUserMMap``
> > +  :reply payload: N/A
> > +
> > +  When the ``VHOST_USER_PROTOCOL_F_SHMEM`` protocol feature has been
> > +  successfully negotiated, this message can be submitted by the backends so
> > +  that the front-end un-mmap a given range (``shm_offset``, ``len``) in the
>
> un-mmaps?
>
> > +  VIRTIO Shared Memory Region with the requested ``shmid``. Note that the
> > +  given range shall correspond to the entirety of a valid mapped region.
> > +  A reply is generated indicating whether unmapping succeeded.
> > +
> >  .. _reply_ack:
> >
> >  VHOST_USER_PROTOCOL_F_REPLY_ACK
> > --
> > 2.49.0

[PATCH v5 4/7] vhost_user: Add frontend get_shmem_config command

[Albert Esteve]

The frontend can use this command to retrieve
VirtIO Shared Memory Regions configuration from
the backend. The response contains the number of
shared memory regions, their size, and shmid.

This is useful when the frontend is unaware of
specific backend type and configuration,
for example, in the `vhost-user-device` case.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 hw/virtio/vhost-user.c            | 43 +++++++++++++++++++++++++++++++
 include/hw/virtio/vhost-backend.h | 10 +++++++
 include/hw/virtio/vhost-user.h    |  1 +
 include/hw/virtio/virtio.h        |  2 ++
 4 files changed, 56 insertions(+)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 9c635fb928..f32da60ac4 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -104,6 +104,7 @@ typedef enum VhostUserRequest {
     VHOST_USER_GET_SHARED_OBJECT = 41,
     VHOST_USER_SET_DEVICE_STATE_FD = 42,
     VHOST_USER_CHECK_DEVICE_STATE = 43,
+    VHOST_USER_GET_SHMEM_CONFIG = 44,
     VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -138,6 +139,12 @@ typedef struct VhostUserMemRegMsg {
     VhostUserMemoryRegion region;
 } VhostUserMemRegMsg;
 
+typedef struct VhostUserShMemConfig {
+    uint32_t nregions;
+    uint32_t padding;
+    uint64_t memory_sizes[VIRTIO_MAX_SHMEM_REGIONS];
+} VhostUserShMemConfig;
+
 typedef struct VhostUserLog {
     uint64_t mmap_size;
     uint64_t mmap_offset;
@@ -244,6 +251,7 @@ typedef union {
         VhostUserShared object;
         VhostUserTransferDeviceState transfer_state;
         VhostUserMMap mmap;
+        VhostUserShMemConfig shmem;
 } VhostUserPayload;
 
 typedef struct VhostUserMsg {
@@ -3160,6 +3168,40 @@ static int vhost_user_check_device_state(struct vhost_dev *dev, Error **errp)
     return 0;
 }
 
+static int vhost_user_get_shmem_config(struct vhost_dev *dev,
+                                       int *nregions,
+                                       uint64_t *memory_sizes,
+                                       Error **errp)
+{
+    int ret;
+    VhostUserMsg msg = {
+        .hdr.request = VHOST_USER_GET_SHMEM_CONFIG,
+        .hdr.flags = VHOST_USER_VERSION,
+    };
+
+    if (!virtio_has_feature(dev->protocol_features,
+                            VHOST_USER_PROTOCOL_F_SHMEM)) {
+        return 0;
+    }
+
+    ret = vhost_user_write(dev, &msg, NULL, 0);
+    if (ret < 0) {
+        return ret;
+    }
+
+    ret = vhost_user_read(dev, &msg);
+    if (ret < 0) {
+        return ret;
+    }
+
+    assert(msg.payload.shmem.nregions <= VIRTIO_MAX_SHMEM_REGIONS);
+    *nregions = msg.payload.shmem.nregions;
+    memcpy(memory_sizes,
+           &msg.payload.shmem.memory_sizes,
+           sizeof(uint64_t) * msg.payload.shmem.nregions);
+    return 0;
+}
+
 const VhostOps user_ops = {
         .backend_type = VHOST_BACKEND_TYPE_USER,
         .vhost_backend_init = vhost_user_backend_init,
@@ -3198,4 +3240,5 @@ const VhostOps user_ops = {
         .vhost_supports_device_state = vhost_user_supports_device_state,
         .vhost_set_device_state_fd = vhost_user_set_device_state_fd,
         .vhost_check_device_state = vhost_user_check_device_state,
+        .vhost_get_shmem_config = vhost_user_get_shmem_config,
 };
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index d6df209a2f..42400b276e 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -159,6 +159,15 @@ typedef int (*vhost_set_device_state_fd_op)(struct vhost_dev *dev,
                                             int *reply_fd,
                                             Error **errp);
 typedef int (*vhost_check_device_state_op)(struct vhost_dev *dev, Error **errp);
+/*
+ * Max regions is VIRTIO_MAX_SHMEM_REGIONS, so that is the maximum
+ * number of memory_sizes that will be accepted.
+ */
+typedef int (*vhost_get_shmem_config_op)(struct vhost_dev *dev,
+                                         int *nregions,
+                                         uint64_t *memory_sizes,
+                                         Error **errp);
+
 
 typedef struct VhostOps {
     VhostBackendType backend_type;
@@ -214,6 +223,7 @@ typedef struct VhostOps {
     vhost_supports_device_state_op vhost_supports_device_state;
     vhost_set_device_state_fd_op vhost_set_device_state_fd;
     vhost_check_device_state_op vhost_check_device_state;
+    vhost_get_shmem_config_op vhost_get_shmem_config;
 } VhostOps;
 
 int vhost_backend_update_device_iotlb(struct vhost_dev *dev,
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index 9a3f238b43..bacc7d184c 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -32,6 +32,7 @@ enum VhostUserProtocolFeature {
     /* Feature 17 reserved for VHOST_USER_PROTOCOL_F_XEN_MMAP. */
     VHOST_USER_PROTOCOL_F_SHARED_OBJECT = 18,
     VHOST_USER_PROTOCOL_F_DEVICE_STATE = 19,
+    VHOST_USER_PROTOCOL_F_SHMEM = 20,
     VHOST_USER_PROTOCOL_F_MAX
 };
 
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 331dbcfbe0..5d0c084f57 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -81,6 +81,8 @@ typedef struct VirtQueueElement
 
 #define VIRTIO_NO_VECTOR 0xffff
 
+#define VIRTIO_MAX_SHMEM_REGIONS 256
+
 /* special index value used internally for config irqs */
 #define VIRTIO_CONFIG_IRQ_IDX -1
 
-- 
2.49.0

Re: [PATCH v5 4/7] vhost_user: Add frontend get_shmem_config command

[Stefan Hajnoczi]

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

On Mon, Jun 09, 2025 at 04:47:26PM +0200, Albert Esteve wrote:
> The frontend can use this command to retrieve
> VirtIO Shared Memory Regions configuration from
> the backend. The response contains the number of
> shared memory regions, their size, and shmid.
> 
> This is useful when the frontend is unaware of
> specific backend type and configuration,
> for example, in the `vhost-user-device` case.
> 
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  hw/virtio/vhost-user.c            | 43 +++++++++++++++++++++++++++++++
>  include/hw/virtio/vhost-backend.h | 10 +++++++
>  include/hw/virtio/vhost-user.h    |  1 +
>  include/hw/virtio/virtio.h        |  2 ++
>  4 files changed, 56 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v5 5/7] vhost_user.rst: Add GET_SHMEM_CONFIG message

[Albert Esteve]

Add GET_SHMEM_CONFIG vhost-user frontend
message to the spec documentation.

Reviewed-by: Alyssa Ross <hi@alyssa.is>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 docs/interop/vhost-user.rst | 39 +++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index b623284819..223bfe7cd7 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -371,6 +371,20 @@ MMAP request
   - 0: Pages are mapped read-only
   - 1: Pages are mapped read-write
 
+VIRTIO Shared Memory Region configuration
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
++-------------+---------+------------+----+--------------+
+| num regions | padding | mem size 0 | .. | mem size 255 |
++-------------+---------+------------+----+--------------+
+
+:num regions: a 32-bit number of regions
+
+:padding: 32-bit
+
+:mem size: contains ``num regions`` 64-bit fields representing the size of each
+           VIRTIO Shared Memory Region
+
 C structure
 -----------
 
@@ -397,6 +411,7 @@ In QEMU the vhost-user message is implemented with the following struct:
           VhostUserShared object;
           VhostUserTransferDeviceState transfer_state;
           VhostUserMMap mmap;
+          VhostUserShMemConfig shmem;
       };
   } QEMU_PACKED VhostUserMsg;
 
@@ -1754,6 +1769,30 @@ Front-end message types
   Using this function requires prior negotiation of the
   ``VHOST_USER_PROTOCOL_F_DEVICE_STATE`` feature.
 
+``VHOST_USER_GET_SHMEM_CONFIG``
+  :id: 44
+  :equivalent ioctl: N/A
+  :request payload: N/A
+  :reply payload: ``struct VhostUserShMemConfig``
+
+  When the ``VHOST_USER_PROTOCOL_F_SHMEM`` protocol feature has been
+  successfully negotiated, this message can be submitted by the front-end
+  to gather the VIRTIO Shared Memory Region configuration. The back-end will
+  respond with the number of VIRTIO Shared Memory Regions it requires, and
+  each shared memory region size in an array. The shared memory IDs are
+  represented by the array index. The information returned shall comply
+  with the following rules:
+
+  * The shared information will remain valid and unchanged for the entire
+    lifetime of the connection.
+
+  * The Shared Memory Region size must be a multiple of the page size
+    supported by mmap(2).
+
+  * The size may be 0 if the region is unused. This can happen when the
+    device does not support an optional feature but does support a feature
+    that uses a higher shmid.
+
 Back-end message types
 ----------------------
 
-- 
2.49.0

[PATCH v5 6/7] qmp: add shmem feature map

[Albert Esteve]

Add new vhost-user protocol
VHOST_USER_PROTOCOL_F_SHMEM feature to
feature map.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 hw/virtio/virtio-qmp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index 3b6377cf0d..8c2cfd0916 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -127,6 +127,9 @@ static const qmp_virtio_feature_map_t vhost_user_protocol_map[] = {
     FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_DEVICE_STATE, \
             "VHOST_USER_PROTOCOL_F_DEVICE_STATE: Backend device state transfer "
             "supported"),
+    FEATURE_ENTRY(VHOST_USER_PROTOCOL_F_SHMEM, \
+                "VHOST_USER_PROTOCOL_F_SHMEM: Backend shared memory mapping "
+                "supported"),
     { -1, "" }
 };
 
-- 
2.49.0

[PATCH v5 7/7] vhost-user-devive: Add shmem BAR

[Albert Esteve]

Add a shmem BAR block in the vhost-user-device,
which files can be directly mapped into.

The number, shmid, and size of the VIRTIO Shared
Memory subregions is retrieved through a
get_shmem_config message sent by the
vhost-user-base module on the realize step,
after virtio_init().

By default, if VHOST_USER_PROTOCOL_F_SHMEM
feature is not supported by the backend,
there is no cache.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 hw/virtio/vhost-user-base.c       | 47 +++++++++++++++++++++++++++++--
 hw/virtio/vhost-user-device-pci.c | 34 ++++++++++++++++++++--
 2 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index ff67a020b4..e86e391fa5 100644
--- a/hw/virtio/vhost-user-base.c
+++ b/hw/virtio/vhost-user-base.c
@@ -16,6 +16,7 @@
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/vhost-user-base.h"
 #include "qemu/error-report.h"
+#include "migration/blocker.h"
 
 static void vub_start(VirtIODevice *vdev)
 {
@@ -276,7 +277,8 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VHostUserBase *vub = VHOST_USER_BASE(dev);
-    int ret;
+    uint64_t memory_sizes[VIRTIO_MAX_SHMEM_REGIONS];
+    int i, ret, nregions;
 
     if (!vub->chardev.chr) {
         error_setg(errp, "vhost-user-base: missing chardev");
@@ -319,7 +321,7 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
 
     /* Allocate queues */
     vub->vqs = g_ptr_array_sized_new(vub->num_vqs);
-    for (int i = 0; i < vub->num_vqs; i++) {
+    for (i = 0; i < vub->num_vqs; i++) {
         g_ptr_array_add(vub->vqs,
                         virtio_add_queue(vdev, vub->vq_size,
                                          vub_handle_output));
@@ -333,11 +335,50 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
                          VHOST_BACKEND_TYPE_USER, 0, errp);
 
     if (ret < 0) {
-        do_vhost_user_cleanup(vdev, vub);
+        goto err;
+    }
+
+    ret = vub->vhost_dev.vhost_ops->vhost_get_shmem_config(&vub->vhost_dev,
+                                                           &nregions,
+                                                           memory_sizes,
+                                                           errp);
+
+    if (ret < 0) {
+        goto err;
+    }
+
+    for (i = 0; i < nregions; i++) {
+        if (memory_sizes[i]) {
+            if (vub->vhost_dev.migration_blocker == NULL) {
+                error_setg(&vub->vhost_dev.migration_blocker,
+                       "Migration disabled: devices with VIRTIO Shared Memory "
+                       "Regions do not support migration yet.");
+                ret = migrate_add_blocker_normal(
+                    &vub->vhost_dev.migration_blocker,
+                    errp);
+
+                if (ret < 0) {
+                    goto err;
+                }
+            }
+
+            if (memory_sizes[i] % qemu_real_host_page_size() != 0) {
+                error_setg(errp, "Shared memory %d size must be a power of 2 "
+                                 "no smaller than the page size", i);
+                goto err;
+            }
+
+            memory_region_init(virtio_new_shmem_region(vdev, i)->mr,
+                               OBJECT(vdev), "vub-shm-" + i,
+                               memory_sizes[i]);
+        }
     }
 
     qemu_chr_fe_set_handlers(&vub->chardev, NULL, NULL, vub_event, NULL,
                              dev, NULL, true);
+    return;
+err:
+    do_vhost_user_cleanup(vdev, vub);
 }
 
 static void vub_device_unrealize(DeviceState *dev)
diff --git a/hw/virtio/vhost-user-device-pci.c b/hw/virtio/vhost-user-device-pci.c
index f10bac874e..eeb52671a0 100644
--- a/hw/virtio/vhost-user-device-pci.c
+++ b/hw/virtio/vhost-user-device-pci.c
@@ -8,14 +8,18 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/vhost-user-base.h"
 #include "hw/virtio/virtio-pci.h"
 
+#define VIRTIO_DEVICE_PCI_SHMEM_BAR 2
+
 struct VHostUserDevicePCI {
     VirtIOPCIProxy parent_obj;
 
     VHostUserBase vub;
+    MemoryRegion shmembar;
 };
 
 #define TYPE_VHOST_USER_DEVICE_PCI "vhost-user-device-pci-base"
@@ -25,10 +29,36 @@ OBJECT_DECLARE_SIMPLE_TYPE(VHostUserDevicePCI, VHOST_USER_DEVICE_PCI)
 static void vhost_user_device_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
 {
     VHostUserDevicePCI *dev = VHOST_USER_DEVICE_PCI(vpci_dev);
-    DeviceState *vdev = DEVICE(&dev->vub);
+    DeviceState *dev_state = DEVICE(&dev->vub);
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev_state);
+    VirtSharedMemory *shmem, *next;
+    uint64_t offset = 0, shmem_size = 0;
 
     vpci_dev->nvectors = 1;
-    qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
+    qdev_realize(dev_state, BUS(&vpci_dev->bus), errp);
+
+    QSIMPLEQ_FOREACH_SAFE(shmem, &vdev->shmem_list, entry, next) {
+        if (shmem->mr->size > UINT64_MAX - shmem_size) {
+            error_setg(errp, "Total shared memory required overflow");
+            return;
+        }
+        shmem_size = shmem_size + shmem->mr->size;
+    }
+    if (shmem_size) {
+        memory_region_init(&dev->shmembar, OBJECT(vpci_dev),
+                           "vhost-device-pci-shmembar", shmem_size);
+        QSIMPLEQ_FOREACH_SAFE(shmem, &vdev->shmem_list, entry, next) {
+            memory_region_add_subregion(&dev->shmembar, offset, shmem->mr);
+            virtio_pci_add_shm_cap(vpci_dev, VIRTIO_DEVICE_PCI_SHMEM_BAR,
+                                   offset, shmem->mr->size, 0);
+            offset = offset + shmem->mr->size;
+        }
+        pci_register_bar(&vpci_dev->pci_dev, VIRTIO_DEVICE_PCI_SHMEM_BAR,
+                        PCI_BASE_ADDRESS_SPACE_MEMORY |
+                        PCI_BASE_ADDRESS_MEM_PREFETCH |
+                        PCI_BASE_ADDRESS_MEM_TYPE_64,
+                        &dev->shmembar);
+    }
 }
 
 static void vhost_user_device_pci_class_init(ObjectClass *klass,
-- 
2.49.0

Re: [PATCH v5 7/7] vhost-user-devive: Add shmem BAR

[Alyssa Ross]

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

Subject should say vhost-user-devi*c*e

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH v5 7/7] vhost-user-devive: Add shmem BAR

[Stefan Hajnoczi]

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

On Mon, Jun 09, 2025 at 04:47:29PM +0200, Albert Esteve wrote:
> diff --git a/hw/virtio/vhost-user-device-pci.c b/hw/virtio/vhost-user-device-pci.c
> index f10bac874e..eeb52671a0 100644
> --- a/hw/virtio/vhost-user-device-pci.c
> +++ b/hw/virtio/vhost-user-device-pci.c
> @@ -8,14 +8,18 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qapi/error.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/virtio/vhost-user-base.h"
>  #include "hw/virtio/virtio-pci.h"
>  
> +#define VIRTIO_DEVICE_PCI_SHMEM_BAR 2

This BAR number conflicts with `--device
vhost-user-device-pci,modern-pio-notify=on`. Please take a look at the
how other devices using VIRTIO Shared Memory arrange BARs.

> +
>  struct VHostUserDevicePCI {
>      VirtIOPCIProxy parent_obj;
>  
>      VHostUserBase vub;
> +    MemoryRegion shmembar;
>  };
>  
>  #define TYPE_VHOST_USER_DEVICE_PCI "vhost-user-device-pci-base"
> @@ -25,10 +29,36 @@ OBJECT_DECLARE_SIMPLE_TYPE(VHostUserDevicePCI, VHOST_USER_DEVICE_PCI)
>  static void vhost_user_device_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>  {
>      VHostUserDevicePCI *dev = VHOST_USER_DEVICE_PCI(vpci_dev);
> -    DeviceState *vdev = DEVICE(&dev->vub);
> +    DeviceState *dev_state = DEVICE(&dev->vub);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev_state);
> +    VirtSharedMemory *shmem, *next;
> +    uint64_t offset = 0, shmem_size = 0;
>  
>      vpci_dev->nvectors = 1;
> -    qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> +    qdev_realize(dev_state, BUS(&vpci_dev->bus), errp);
> +
> +    QSIMPLEQ_FOREACH_SAFE(shmem, &vdev->shmem_list, entry, next) {
> +        if (shmem->mr->size > UINT64_MAX - shmem_size) {
> +            error_setg(errp, "Total shared memory required overflow");
> +            return;
> +        }
> +        shmem_size = shmem_size + shmem->mr->size;
> +    }
> +    if (shmem_size) {
> +        memory_region_init(&dev->shmembar, OBJECT(vpci_dev),
> +                           "vhost-device-pci-shmembar", shmem_size);
> +        QSIMPLEQ_FOREACH_SAFE(shmem, &vdev->shmem_list, entry, next) {
> +            memory_region_add_subregion(&dev->shmembar, offset, shmem->mr);
> +            virtio_pci_add_shm_cap(vpci_dev, VIRTIO_DEVICE_PCI_SHMEM_BAR,
> +                                   offset, shmem->mr->size, 0);

The last argument is the shmid. It cannot be hardcoded to 0.

> +            offset = offset + shmem->mr->size;
> +        }
> +        pci_register_bar(&vpci_dev->pci_dev, VIRTIO_DEVICE_PCI_SHMEM_BAR,
> +                        PCI_BASE_ADDRESS_SPACE_MEMORY |
> +                        PCI_BASE_ADDRESS_MEM_PREFETCH |
> +                        PCI_BASE_ADDRESS_MEM_TYPE_64,
> +                        &dev->shmembar);
> +    }
>  }
>  
>  static void vhost_user_device_pci_class_init(ObjectClass *klass,
> -- 
> 2.49.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v5 7/7] vhost-user-devive: Add shmem BAR

[Dorinda Bassey]

Hi Albert,

Thank you for these patches, just a small issue you could add to your
next series.

>
> +
> +            memory_region_init(virtio_new_shmem_region(vdev, i)->mr,
> +                               OBJECT(vdev), "vub-shm-" + i,
> +                               memory_sizes[i]);

Here, `"vub-shm-" + i` looks like string concatenation but uses
pointer arithmetic,
which could cause a crash at runtime for e.g., when `i` is > 0. I
believe you can use
g_strdup_printf("vub-shm-%d", i) to fix this, it constructs a new
null-terminated
string like "vub-shm-0", "vub-shm-1", "vub-shm-2", etc. If that was
your intention.

BR,
Dorinda.

Re: [PATCH v5 7/7] vhost-user-devive: Add shmem BAR

[Albert Esteve]

On Fri, Jun 20, 2025 at 11:08 AM Dorinda Bassey <dbassey@redhat.com> wrote:
>
> Hi Albert,
>
> Thank you for these patches, just a small issue you could add to your
> next series.
>
> >
> > +
> > +            memory_region_init(virtio_new_shmem_region(vdev, i)->mr,
> > +                               OBJECT(vdev), "vub-shm-" + i,
> > +                               memory_sizes[i]);
>
> Here, `"vub-shm-" + i` looks like string concatenation but uses
> pointer arithmetic,
> which could cause a crash at runtime for e.g., when `i` is > 0. I
> believe you can use
> g_strdup_printf("vub-shm-%d", i) to fix this, it constructs a new
> null-terminated
> string like "vub-shm-0", "vub-shm-1", "vub-shm-2", etc. If that was
> your intention.

Nice catch! Looks like I just didn't completely switch to the right
syntax when working on this patch.

Luckily, on patch 1 of this series I used the correct string
manipulation. I had to double check.

>
> BR,
> Dorinda.
>