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

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Mon, Oct 13, 2025 at 05:37:52PM +0200, Albert Esteve wrote:
> On Mon, Sep 29, 2025 at 9:00 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Wed, Sep 10, 2025 at 01:54:20PM +0200, 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
> > > 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.
> > >
> > > 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..82f49500e4 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,9 @@ 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];
> > > +    g_autofree char *name = NULL;
> > > +    int i, ret, nregions, regions_processed = 0;
> > >
> > >      if (!vub->chardev.chr) {
> > >          error_setg(errp, "vhost-user-base: missing chardev");
> > > @@ -319,7 +322,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 +336,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-device-pci.c b/hw/virtio/vhost-user-device-pci.c
> > > index f10bac874e..bac99e7c60 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 3
> >
> > 64-bit PCI memory BARs require two contiguous 32-bit BARs. Choosing BAR
> > 3 won't work because modern_mem_bar_idx = 4 (and 5) and there is a
> > collision.
> >
> > virtio-gpu-hostmem uses BAR 2 and 3 for VIRTIO Shared Memory Regions. In
> > order to do this, it rearranges the other VIRTIO PCI BARs in
> > virtio_vga_base_realize(). Following that approach should be safe.
> 
> I see. But doing the same strategy would collide with
> virtio-gpu-hostmem if both devices are used simultaneously? Would that
> be ok?

virtio-gpu-* and vhost-user-device-pci are separate PCI devices with
their own BARs, how would they conflict?

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