[PATCH v13 7/7] vhost-user-device: Add shared memory BAR

Albert Esteve posted 7 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v13 7/7] vhost-user-device: Add shared memory BAR
Posted by Albert Esteve 1 month, 3 weeks ago
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
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 <stefanha@redhat.com>
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 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);
+                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;
 
+    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);
+
+    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) {
+        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) {
+            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
Re: [PATCH v13 7/7] vhost-user-device: Add shared memory BAR
Posted by Stefano Garzarella 1 month, 1 week ago
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 <stefanha@redhat.com>
>Signed-off-by: Albert Esteve <aesteve@redhat.com>
>---
> 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?

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

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

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

In any case:

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>


>+            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
>
Re: [PATCH v13 7/7] vhost-user-device: Add shared memory BAR
Posted by Albert Esteve 1 month, 1 week ago
On Wed, Mar 4, 2026 at 2:26 PM Stefano Garzarella <sgarzare@redhat.com> 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 <stefanha@redhat.com>
> >Signed-off-by: Albert Esteve <aesteve@redhat.com>
> >---
> > 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 <sgarzare@redhat.com>
>
>
> >+            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
> >
>