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 | 36 +++++++++++++++++++++--
2 files changed, 78 insertions(+), 5 deletions(-)
diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index 2bc3423326..8d4bca98a8 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)
{
@@ -271,7 +272,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");
@@ -314,7 +316,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));
@@ -328,11 +330,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)->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 efaf55d3dd..f215cae925 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,38 @@ 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);
+ MemoryRegion *mr;
+ uint64_t offset = 0, shmem_size = 0;
+ int i;
vpci_dev->nvectors = 1;
- qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
+ qdev_realize(dev_state, BUS(&vpci_dev->bus), errp);
+
+ for (i = 0; i < vdev->n_shmem_regions; i++) {
+ mr = vdev->shmem_list[i].mr;
+ if (mr->size > UINT64_MAX - shmem_size) {
+ error_setg(errp, "Total shared memory required overflow");
+ return;
+ }
+ shmem_size = shmem_size + mr->size;
+ }
+ if (shmem_size) {
+ memory_region_init(&dev->shmembar, OBJECT(vpci_dev),
+ "vhost-device-pci-shmembar", shmem_size);
+ for (i = 0; i < vdev->n_shmem_regions; i++) {
+ memory_region_add_subregion(&dev->shmembar, offset, mr);
+ virtio_pci_add_shm_cap(vpci_dev, VIRTIO_DEVICE_PCI_SHMEM_BAR,
+ offset, mr->size, i);
+ offset = offset + 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, void *data)
--
2.48.1On Mon, Feb 17, 2025 at 05:40:10PM +0100, Albert Esteve wrote:
> 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 | 36 +++++++++++++++++++++--
> 2 files changed, 78 insertions(+), 5 deletions(-)
>
> diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
> index 2bc3423326..8d4bca98a8 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)
> {
> @@ -271,7 +272,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");
> @@ -314,7 +316,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));
> @@ -328,11 +330,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)->mr,
Does this code support non-contiguous shmids? For example, if a device
has two Shared Memory Regions defined in its spec but the first one is
optional, then the device might have memory_sizes[0] == 0 and
memory_sizes[1] > 0. In that case the Shared Memory Region must have
shmid 1 and not shmid 0.
> + 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 efaf55d3dd..f215cae925 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,38 @@ 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);
> + MemoryRegion *mr;
> + uint64_t offset = 0, shmem_size = 0;
> + int i;
>
> vpci_dev->nvectors = 1;
> - qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> + qdev_realize(dev_state, BUS(&vpci_dev->bus), errp);
> +
> + for (i = 0; i < vdev->n_shmem_regions; i++) {
> + mr = vdev->shmem_list[i].mr;
> + if (mr->size > UINT64_MAX - shmem_size) {
> + error_setg(errp, "Total shared memory required overflow");
> + return;
> + }
> + shmem_size = shmem_size + mr->size;
> + }
> + if (shmem_size) {
> + memory_region_init(&dev->shmembar, OBJECT(vpci_dev),
> + "vhost-device-pci-shmembar", shmem_size);
> + for (i = 0; i < vdev->n_shmem_regions; i++) {
> + memory_region_add_subregion(&dev->shmembar, offset, mr);
> + virtio_pci_add_shm_cap(vpci_dev, VIRTIO_DEVICE_PCI_SHMEM_BAR,
> + offset, mr->size, i);
> + offset = offset + 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, void *data)
> --
> 2.48.1
>
On Tue, Feb 18, 2025 at 11:41 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Mon, Feb 17, 2025 at 05:40:10PM +0100, Albert Esteve wrote:
> > 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 | 36 +++++++++++++++++++++--
> > 2 files changed, 78 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
> > index 2bc3423326..8d4bca98a8 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)
> > {
> > @@ -271,7 +272,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");
> > @@ -314,7 +316,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));
> > @@ -328,11 +330,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)->mr,
>
> Does this code support non-contiguous shmids? For example, if a device
> has two Shared Memory Regions defined in its spec but the first one is
> optional, then the device might have memory_sizes[0] == 0 and
> memory_sizes[1] > 0. In that case the Shared Memory Region must have
> shmid 1 and not shmid 0.
Yes, it does. That is guarded by ` if (memory_sizes[i]) {`, which only
initializes the region if memory_sizes[i] > 0. The main downsize of
that, is that it requires to send as many `memory_sizes` elements as
the highest shmid for the device. But as it is, it is supported by
this code.
>
> > + 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 efaf55d3dd..f215cae925 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,38 @@ 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);
> > + MemoryRegion *mr;
> > + uint64_t offset = 0, shmem_size = 0;
> > + int i;
> >
> > vpci_dev->nvectors = 1;
> > - qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> > + qdev_realize(dev_state, BUS(&vpci_dev->bus), errp);
> > +
> > + for (i = 0; i < vdev->n_shmem_regions; i++) {
> > + mr = vdev->shmem_list[i].mr;
> > + if (mr->size > UINT64_MAX - shmem_size) {
> > + error_setg(errp, "Total shared memory required overflow");
> > + return;
> > + }
> > + shmem_size = shmem_size + mr->size;
> > + }
> > + if (shmem_size) {
> > + memory_region_init(&dev->shmembar, OBJECT(vpci_dev),
> > + "vhost-device-pci-shmembar", shmem_size);
> > + for (i = 0; i < vdev->n_shmem_regions; i++) {
> > + memory_region_add_subregion(&dev->shmembar, offset, mr);
> > + virtio_pci_add_shm_cap(vpci_dev, VIRTIO_DEVICE_PCI_SHMEM_BAR,
> > + offset, mr->size, i);
> > + offset = offset + 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, void *data)
> > --
> > 2.48.1
> >
On Tue, Feb 18, 2025 at 11:55:33AM +0100, Albert Esteve wrote:
> On Tue, Feb 18, 2025 at 11:41 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Mon, Feb 17, 2025 at 05:40:10PM +0100, Albert Esteve wrote:
> > > 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 | 36 +++++++++++++++++++++--
> > > 2 files changed, 78 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
> > > index 2bc3423326..8d4bca98a8 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)
> > > {
> > > @@ -271,7 +272,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");
> > > @@ -314,7 +316,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));
> > > @@ -328,11 +330,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)->mr,
> >
> > Does this code support non-contiguous shmids? For example, if a device
> > has two Shared Memory Regions defined in its spec but the first one is
> > optional, then the device might have memory_sizes[0] == 0 and
> > memory_sizes[1] > 0. In that case the Shared Memory Region must have
> > shmid 1 and not shmid 0.
>
> Yes, it does. That is guarded by ` if (memory_sizes[i]) {`, which only
> initializes the region if memory_sizes[i] > 0. The main downsize of
> that, is that it requires to send as many `memory_sizes` elements as
> the highest shmid for the device. But as it is, it is supported by
> this code.
shmids are not preserved when there are gaps:
for (i = 0; i < vdev->n_shmem_regions; i++) {
memory_region_add_subregion(&dev->shmembar, offset, mr);
virtio_pci_add_shm_cap(vpci_dev, VIRTIO_DEVICE_PCI_SHMEM_BAR,
offset, mr->size, i);
^
vdev->n_shmem_regions is incremented by virtio_new_shmem_region().
virtio_new_shmem_region() is only called on non-empty Shared Memory
Regions.
In the example I gave with empty shmid 0 and non-empty shmid 1 I think
we end up with vdev->n_shmem_regions == 1. shmdid 1 is exposed to the
guest with shmid 0.
Have I missed something?
> >
> > > + 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 efaf55d3dd..f215cae925 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,38 @@ 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);
> > > + MemoryRegion *mr;
> > > + uint64_t offset = 0, shmem_size = 0;
> > > + int i;
> > >
> > > vpci_dev->nvectors = 1;
> > > - qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> > > + qdev_realize(dev_state, BUS(&vpci_dev->bus), errp);
> > > +
> > > + for (i = 0; i < vdev->n_shmem_regions; i++) {
> > > + mr = vdev->shmem_list[i].mr;
> > > + if (mr->size > UINT64_MAX - shmem_size) {
> > > + error_setg(errp, "Total shared memory required overflow");
> > > + return;
> > > + }
> > > + shmem_size = shmem_size + mr->size;
> > > + }
> > > + if (shmem_size) {
> > > + memory_region_init(&dev->shmembar, OBJECT(vpci_dev),
> > > + "vhost-device-pci-shmembar", shmem_size);
> > > + for (i = 0; i < vdev->n_shmem_regions; i++) {
> > > + memory_region_add_subregion(&dev->shmembar, offset, mr);
> > > + virtio_pci_add_shm_cap(vpci_dev, VIRTIO_DEVICE_PCI_SHMEM_BAR,
> > > + offset, mr->size, i);
> > > + offset = offset + 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, void *data)
> > > --
> > > 2.48.1
> > >
>
On Tue, Feb 18, 2025 at 2:29 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Tue, Feb 18, 2025 at 11:55:33AM +0100, Albert Esteve wrote:
> > On Tue, Feb 18, 2025 at 11:41 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > >
> > > On Mon, Feb 17, 2025 at 05:40:10PM +0100, Albert Esteve wrote:
> > > > 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 | 36 +++++++++++++++++++++--
> > > > 2 files changed, 78 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
> > > > index 2bc3423326..8d4bca98a8 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)
> > > > {
> > > > @@ -271,7 +272,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");
> > > > @@ -314,7 +316,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));
> > > > @@ -328,11 +330,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)->mr,
> > >
> > > Does this code support non-contiguous shmids? For example, if a device
> > > has two Shared Memory Regions defined in its spec but the first one is
> > > optional, then the device might have memory_sizes[0] == 0 and
> > > memory_sizes[1] > 0. In that case the Shared Memory Region must have
> > > shmid 1 and not shmid 0.
> >
> > Yes, it does. That is guarded by ` if (memory_sizes[i]) {`, which only
> > initializes the region if memory_sizes[i] > 0. The main downsize of
> > that, is that it requires to send as many `memory_sizes` elements as
> > the highest shmid for the device. But as it is, it is supported by
> > this code.
>
> shmids are not preserved when there are gaps:
>
> for (i = 0; i < vdev->n_shmem_regions; i++) {
> memory_region_add_subregion(&dev->shmembar, offset, mr);
> virtio_pci_add_shm_cap(vpci_dev, VIRTIO_DEVICE_PCI_SHMEM_BAR,
> offset, mr->size, i);
> ^
>
> vdev->n_shmem_regions is incremented by virtio_new_shmem_region().
> virtio_new_shmem_region() is only called on non-empty Shared Memory
> Regions.
>
> In the example I gave with empty shmid 0 and non-empty shmid 1 I think
> we end up with vdev->n_shmem_regions == 1. shmdid 1 is exposed to the
> guest with shmid 0.
Ah, right. I considered your example when I was doing it, but the code
is buggy indeed. The code that I tested is mostly on the shm map API
part, with a custom pci device.
As mentioned in the initial message, I will add tests for the next
iteration. Thanks for finding this one!
>
> Have I missed something?
>
> > >
> > > > + 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 efaf55d3dd..f215cae925 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,38 @@ 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);
> > > > + MemoryRegion *mr;
> > > > + uint64_t offset = 0, shmem_size = 0;
> > > > + int i;
> > > >
> > > > vpci_dev->nvectors = 1;
> > > > - qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> > > > + qdev_realize(dev_state, BUS(&vpci_dev->bus), errp);
> > > > +
> > > > + for (i = 0; i < vdev->n_shmem_regions; i++) {
> > > > + mr = vdev->shmem_list[i].mr;
> > > > + if (mr->size > UINT64_MAX - shmem_size) {
> > > > + error_setg(errp, "Total shared memory required overflow");
> > > > + return;
> > > > + }
> > > > + shmem_size = shmem_size + mr->size;
> > > > + }
> > > > + if (shmem_size) {
> > > > + memory_region_init(&dev->shmembar, OBJECT(vpci_dev),
> > > > + "vhost-device-pci-shmembar", shmem_size);
> > > > + for (i = 0; i < vdev->n_shmem_regions; i++) {
> > > > + memory_region_add_subregion(&dev->shmembar, offset, mr);
> > > > + virtio_pci_add_shm_cap(vpci_dev, VIRTIO_DEVICE_PCI_SHMEM_BAR,
> > > > + offset, mr->size, i);
> > > > + offset = offset + 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, void *data)
> > > > --
> > > > 2.48.1
> > > >
> >
© 2016 - 2025 Red Hat, Inc.