Re: [PATCH RFC 09/15] hw/virtio/vhost-shadow-virtqueue: specified vring placement

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Thu, Jul 23, 2026 at 03:30:08PM -0700, Connor Kite wrote:
> By default svq vrings are placed in an anonymous memory map. As svqs
> will be leveraged to enable memory isolation in vhost-user, it is useful
> to be able to place the vrings in a shared isolation memory region.
> 
> Adds the option to specify vring placement by providing a vring base
> address before starting the svq.
> 
> Signed-off-by: Connor Kite <connorkite@gmail.com>
> ---
>  hw/virtio/vhost-shadow-virtqueue.c | 22 +++++++++++++++-------
>  hw/virtio/vhost-shadow-virtqueue.h |  3 +++
>  2 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> index eb86c1ee37..9e3c359f50 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -857,14 +857,21 @@ void vhost_svq_start(VhostShadowVirtqueue *svq, VirtIODevice *vdev,
>  
>      svq->vring.num = virtio_queue_get_num(vdev, virtio_get_queue_index(vq));
>      svq->num_free = svq->vring.num;
> -    svq->vring.desc = mmap(NULL, vhost_svq_driver_area_size(svq),
> -                           PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS,
> -                           -1, 0);
>      desc_size = sizeof(vring_desc_t) * svq->vring.num;
> -    svq->vring.avail = (void *)((char *)svq->vring.desc + desc_size);
> -    svq->vring.used = mmap(NULL, vhost_svq_device_area_size(svq),
> -                           PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS,
> -                           -1, 0);
> +    if (svq->base_addr == NULL) {
> +        svq->vring.desc = mmap(NULL, vhost_svq_driver_area_size(svq),
> +                            PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS,
> +                            -1, 0);
> +        svq->vring.avail = (void *)((char *)svq->vring.desc + desc_size);
> +        svq->vring.used = mmap(NULL, vhost_svq_device_area_size(svq),
> +                            PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS,
> +                            -1, 0);
> +    } else {
> +        svq->vring.desc = (void *) svq->base_addr;
> +        svq->vring.avail = (void *)((char *)svq->vring.desc + desc_size);
> +        svq->vring.used = (void *)((char *)svq->base_addr +
> +                          vhost_svq_driver_area_size(svq));
> +    }
>      svq->desc_state = g_new0(SVQDescState, svq->vring.num);
>      if (virtio_vdev_has_feature(svq->vdev, VIRTIO_F_IN_ORDER)) {
>          svq->batch_last.id = VIRTIO_RING_NOT_IN_BATCH;
> @@ -929,6 +936,7 @@ VhostShadowVirtqueue *vhost_svq_new(const VhostShadowVirtqueueOps *ops,
>      event_notifier_init_fd(&svq->svq_kick, VHOST_FILE_UNBIND);
>      svq->ops = ops;
>      svq->ops_opaque = ops_opaque;
> +    svq->base_addr = NULL;
>      return svq;
>  }
>  
> diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
> index ccfeee36d7..39f69e6455 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.h
> +++ b/hw/virtio/vhost-shadow-virtqueue.h
> @@ -148,6 +148,9 @@ typedef struct VhostShadowVirtqueue {
>  
>      /* Size of SVQ vring free descriptors */
>      uint16_t num_free;
> +
> +    /* Location assigned to vrings if not in default anon memory map*/
> +    hwaddr *base_addr;

Why is base_addr a hwaddr? This is QEMU memory, not guest RAM. I
expected this to be void *.

A size needs to be at least documented here to reduce the chance of
memory bugs. Even better would be an interface like
vhost_svq_vring_total_size() so the caller can fetch the number of bytes
before allocating the memory.

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