On Wed, Mar 4, 2026 at 2:26 PM Stefano Garzarella <[email protected]> wrote:
>
> On Thu, Feb 19, 2026 at 02:03:34PM +0100, Albert Esteve wrote:
> >Add shared memory BAR support to vhost-user-device-pci
> >to enable direct file mapping for VIRTIO Shared
> >Memory Regions.
> >
> >The implementation creates a consolidated shared
> >memory BAR that contains all VIRTIO Shared
> >Memory Regions as subregions. Each region is
> >configured with its proper shmid, size, and
> >offset within the BAR. The number and size of
> >regions are retrieved via VHOST_USER_GET_SHMEM_CONFIG
> >message sent by vhost-user-base during realization
> >after virtio_init().
> >
> >Specifiically, it uses BAR 3 to avoid conflicts, as
>
> nit: s/Specifiically/Specifically
>
> Discussed offline, this should be BAR 4 IIUC.
>
> >it is currently unused.
> >
> >The shared memory BAR is only created when the
> >backend supports VHOST_USER_PROTOCOL_F_SHMEM and
> >has configured shared memory regions. This maintains
> >backward compatibility with backends that do not
> >support shared memory functionality.
> >
> >Reviewed-by: Stefan Hajnoczi <[email protected]>
> >Signed-off-by: Albert Esteve <[email protected]>
> >---
> > hw/virtio/vhost-user-base.c | 46 ++++++++++++++++++++++++--
> > hw/virtio/vhost-user-test-device-pci.c | 38 +++++++++++++++++++--
> > 2 files changed, 79 insertions(+), 5 deletions(-)
> >
> >diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
> >index 01ab9ca56b..e5e8abae96 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, regions_processed = 0;
> >
> > 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,49 @@ 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 < VIRTIO_MAX_SHMEM_REGIONS && regions_processed <
> >nregions; i++) {
> >+ if (memory_sizes[i]) {
> >+ regions_processed++;
> >+ 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);
>
> Is this error message updated?
It is a bit misleading indeed. I will reword it, the size must be a
multiple of the host page size.
>
> >+ goto err;
> >+ }
> >+
> >+ virtio_new_shmem_region(vdev, 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-test-device-pci.c
> >b/hw/virtio/vhost-user-test-device-pci.c
> >index 7f6d751690..22678e3277 100644
> >--- a/hw/virtio/vhost-user-test-device-pci.c
> >+++ b/hw/virtio/vhost-user-test-device-pci.c
> >@@ -12,10 +12,13 @@
> > #include "hw/virtio/vhost-user-base.h"
> > #include "hw/virtio/virtio-pci.h"
> >
> >+#define VIRTIO_DEVICE_PCI_SHMEM_BAR 4
> >+
> > struct VHostUserDevicePCI {
> > VirtIOPCIProxy parent_obj;
> >
> > VHostUserBase vub;
> >+ MemoryRegion shmembar;
> > };
> >
> > #define TYPE_VHOST_USER_TEST_DEVICE_PCI "vhost-user-test-device-pci-base"
> >@@ -25,10 +28,41 @@ OBJECT_DECLARE_SIMPLE_TYPE(VHostUserDevicePCI,
> >VHOST_USER_TEST_DEVICE_PCI)
> > static void vhost_user_device_pci_realize(VirtIOPCIProxy *vpci_dev, Error
> > **errp)
> > {
> > VHostUserDevicePCI *dev = VHOST_USER_TEST_DEVICE_PCI(vpci_dev);
> >- DeviceState *vdev = DEVICE(&dev->vub);
> >+ DeviceState *dev_state = DEVICE(&dev->vub);
> >+ VirtIODevice *vdev = VIRTIO_DEVICE(dev_state);
> >+ VirtioSharedMemory *shmem, *next;
> >+ uint64_t offset = 0, shmem_size = 0;
> >
>
> I'd add a comment here to explain this since it's not immediately clear
> IMHO.
>
> >+ vpci_dev->modern_mem_bar_idx = 2;
> > vpci_dev->nvectors = 1;
> >- qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> >+ qdev_realize(dev_state, BUS(&vpci_dev->bus), errp);
>
> IIUC before this patch qdev_realize() was the last step in this
> function, so we didn't check for any error, but now should we return
> earlier if it fails?
Agreed.
>
> >+
> >+ QSIMPLEQ_FOREACH_SAFE(shmem, &vdev->shmem_list, entry, next) {
>
> Is QSIMPLEQ_FOREACH_SAFE really needed here? IIUC we are not modifying
> the list, so can we use QSIMPLEQ_FOREACH ?
Agreed.
>
> >+ if (shmem->mr.size > UINT64_MAX - shmem_size) {
>
> Same here about using memory_region_size().
>
> >+ error_setg(errp, "Total shared memory required overflow");
> >+ return;
> >+ }
> >+ shmem_size = shmem_size + shmem->mr.size;
>
> Ditto about memory_region_size().
>
> >+ }
> >+ if (shmem_size) {
> >+ if (vpci_dev->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY) {
> >+ error_setg(errp, "modern-pio-notify is not supported due to PCI
> >BAR layout limitations");
> >+ return;
> >+ }
> >+ memory_region_init(&dev->shmembar, OBJECT(vpci_dev),
> >+ "vhost-device-pci-shmembar", shmem_size);
> >+ QSIMPLEQ_FOREACH_SAFE(shmem, &vdev->shmem_list, entry, next) {
>
> Ditto about QSIMPLEQ_FOREACH_SAFE.
>
> BTW my comments can eventually be fixed with a followup, overall LGTM,
> but if you need to resend, maybe we can fix them now.
I'm already applying these, I will resend them shortly. Thanks!
>
> In any case:
>
> Reviewed-by: Stefano Garzarella <[email protected]>
>
>
> >+ 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, shmem->shmid);
> >+ 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.52.0
> >
>
