| Message ID | 20250217164012.246727-8-aesteve@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | vhost-user: Add SHMEM_MAP/UNMAP requests | expand |
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. > + 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 > > > > > >
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)
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(-)