On Thu, Jul 23, 2026 at 03:30:13PM -0700, Connor Kite wrote: > - Add start logic for shadow virtqueues, which sets vring addresses. > - Update logic for sending vring addresses to backend to point > to the shadow vrings when isolation mode is active. > - Implement handlers for intercepted avail and used descriptors. These > handlers copy buffer contents between bounce buffers in the isolation > region and the buffers made available by the guest > - Implement logic to stop svqs > > Signed-off-by: Connor Kite <[email protected]> > --- > hw/virtio/vhost-user.c | 167 > ++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 166 insertions(+), 1 deletion(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 75858289a2..e65f877f9a 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -1299,6 +1299,97 @@ static int init_isolation_regions(struct vhost_dev > *dev, > return 0; > } > > +static int vhost_user_memory_lookup(struct vhost_dev *dev, hwaddr gpa, > + hwaddr *hva) > +{ > + int i; > + hwaddr offset; > + > + for (i = 0; i < dev->mem->nregions; i++) { > + struct vhost_memory_region *reg = dev->mem->regions + i; > + > + if (gpa >= reg->guest_phys_addr && > + reg->guest_phys_addr + reg->memory_size > gpa) { > + offset = gpa - reg->guest_phys_addr; > + *hva = reg->userspace_addr + offset; > + return 0; > + } > + } > + > + return -EFAULT; > +} > + > +static int vhost_user_svq_handle_used(VhostShadowVirtqueue *svq, > + VirtQueueElement *elem, > + void *opaque) > +{ > + hwaddr hva; > + int r; > + struct vhost_dev *dev = opaque; > + > + for (int i = 0; i < elem->in_num; i++) { > + r = vhost_user_memory_lookup(dev, elem->in_addr[i], &hva); > + if (r < 0) { > + return r; > + } > + > + memcpy((void *) hva, elem->in_sg[i].iov_base, > elem->in_sg[i].iov_len); > + elem->in_sg[i].iov_base = (void *) hva; > + } > + > + for (int i = 0; i < elem->out_num; i++) { > + r = vhost_user_memory_lookup(dev, elem->out_addr[i], &hva); > + if (r < 0) { > + return r; > + } > + > + memcpy((void *) hva, elem->out_sg[i].iov_base, > elem->out_sg[i].iov_len); > + elem->out_sg[i].iov_base = (void *) hva; > + } > + > + return 0; > +} > + > +static int vhost_user_svq_handle_avail(VhostShadowVirtqueue *svq, > + VirtQueueElement *elem, > + void *opaque) > +{ > + hwaddr offset; > + const DMAMap *map; > + DMAMap needle; > + hwaddr *iova_base; > + > + for (int i = 0; i < elem->out_num; i++) { > + needle.translated_addr = elem->out_addr[i]; > + needle.size = elem->out_sg[i].iov_len - 1; > + map = vhost_iova_tree_find_gpa(svq->iova_tree, &needle);
Mapping failure must be handled.
> + offset = needle.translated_addr - map->translated_addr;
> + iova_base = (void *)(map->iova + offset);
> +
> + elem->out_sg[i].iov_base = iova_base;
> + memcpy(iova_base, elem->out_sg[i].iov_base, needle.size + 1);
iova_base is an iova, not QEMU memory (HVA). Memcpy cannot be used with
IOVAs. The address of the mapped shared memory is needed as the
destination instead.
> + }
> +
> + for (int i = 0; i < elem->in_num; i++) {
> + needle.translated_addr = elem->in_addr[i];
> + needle.size = elem->in_sg[i].iov_len - 1;
> + map = vhost_iova_tree_find_gpa(svq->iova_tree, &needle);
Mapping failure must be handled.
> + offset = needle.translated_addr - map->translated_addr;
> + iova_base = (void *)(map->iova + offset);
> +
> + elem->in_sg[i].iov_base = iova_base;
> + memcpy(iova_base, elem->in_sg[i].iov_base, needle.size + 1);
No memcpy is necessary before vhost_svq_add() since this is data that
will be read from the device upon I/O completion.
> + }
elem->out_sg[] and elem->in_sg[] are modified in this function. Have we
lost the original I/O buffer memory address from the guest virtqueue?
This is a problem because they will be needed when completing the
request.
> +
> + vhost_svq_add(svq, elem->out_sg, elem->out_num, elem->out_addr,
> + elem->in_sg, elem->in_num, elem->in_addr, elem);
> +
> + return 0;
> +}
> +
> +
> +
> +
> static int vhost_user_set_mem_table(struct vhost_dev *dev,
> struct vhost_memory *mem)
> {
> @@ -1772,6 +1863,8 @@ static int vhost_user_set_vring_err(struct vhost_dev
> *dev,
> static int vhost_user_set_vring_addr(struct vhost_dev *dev,
> struct vhost_vring_addr *addr)
> {
> + struct vhost_user *u = dev->opaque;
> +
> VhostUserMsg msg = {
> .hdr.request = VHOST_USER_SET_VRING_ADDR,
> .hdr.flags = VHOST_USER_VERSION,
> @@ -1779,6 +1872,25 @@ static int vhost_user_set_vring_addr(struct vhost_dev
> *dev,
> .hdr.size = sizeof(msg.payload.addr),
> };
>
> + if (u->user->memory_isolation) {
> + if (!u->svqs_allocated) {
> + return 0;
> + }
Is the idea that this returns silently when called before
vhost_user_dev_start()? State make code harder to understand. It would
be cleaner to set up the vring addresses without relying on
svqs_allocated. What is the reason for deferring the vring address
setup to vhost_user_dev_start()?
> +
> + int svq_idx = addr->index - dev->vq_index;
> + VhostShadowVirtqueue *svq = g_ptr_array_index(u->shadow_vqs,
> + svq_idx);
> +
> + struct vhost_vring_addr svq_addr = {
> + .avail_user_addr = (uint64_t)(uintptr_t)svq->vring.avail,
> + .desc_user_addr = (uint64_t)(uintptr_t)svq->vring.desc,
> + .used_user_addr = (uint64_t)(uintptr_t)svq->vring.used,
> + .index = addr->index,
> + };
> +
> + msg.payload.addr = svq_addr;
> + }
> +
> /*
> * wait for a reply if logging is enabled to make sure
> * backend is actually logging changes
> @@ -2754,13 +2866,18 @@ static int
> vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
> return 0;
> }
>
> +static const VhostShadowVirtqueueOps vhost_user_svq_ops = {
> + .avail_handler = vhost_user_svq_handle_avail,
> + .used_handler = vhost_user_svq_handle_used
> +};
> +
> static void vhost_user_init_svq(struct vhost_dev *dev, struct vhost_user *u)
> {
> /*Modified from vhost-vdpa*/
> u->shadow_vqs = g_ptr_array_new_full(dev->nvqs, vhost_svq_free);
> for (int i = 0; i < dev->nvqs; i++) {
> VhostShadowVirtqueue *svq;
> - svq = vhost_svq_new(NULL, NULL);
> + svq = vhost_svq_new(&vhost_user_svq_ops, dev);
> g_ptr_array_add(u->shadow_vqs, svq);
> }
> }
> @@ -3466,8 +3583,56 @@ void vhost_user_async_close(DeviceState *d,
> }
> }
>
> +static bool vhost_user_svqs_start(struct vhost_dev *dev)
> +{
> + struct vhost_user *u = dev->opaque;
> + uint64_t vring_base = u->iso_memory.vring_base_addr;
> + u->svqs_allocated = true;
Where is this field cleared to false on reset?
> +
> + for (int i = 0; i < u->shadow_vqs->len; i++) {
> + VirtQueue *vq = virtio_get_queue(dev->vdev, dev->vq_index + i);
> + VhostShadowVirtqueue *svq = g_ptr_array_index(u->shadow_vqs, i);
> + svq->base_addr = (hwaddr *) vring_base;
Does this support multiple virtqueues, it looks like they will all use
the same base_addr?
> + vhost_svq_start(svq, dev->vdev, vq, u->iso_iova_tree);
> +
> + struct vhost_vring_addr addr = {
> + .index = dev->vq_index + i,
> + .desc_user_addr = vring_base,
> + .avail_user_addr = vring_base + sizeof(vring_desc_t) *
> + svq->vring.num,
> + .used_user_addr = vring_base + vhost_svq_driver_area_size(svq)
> + };
> +
> + vhost_user_set_vring_addr(dev, &addr);
Also mentioned above in vhost_user_set_vring_addr():
Sending a vhost-user message here is strange since that should already
be done by vhost.c:vhost_virtqueue_start() ->
vhost_user_set_vring_addr().
> +
> + vring_base += vhost_svq_device_area_size(svq) +
> + vhost_svq_driver_area_size(svq);
> + }
> +
> + return false;
> +}
> +
> +static void vhost_user_svqs_stop(struct vhost_dev *dev)
> +{
> + struct vhost_user *u = dev->opaque;
> +
> + for (int i = 0; i < u->shadow_vqs->len; i++) {
> + vhost_svq_stop(g_ptr_array_index(u->shadow_vqs, i));
> + }
> +}
> +
> +
> static int vhost_user_dev_start(struct vhost_dev *dev, bool started)
> {
> + struct vhost_user *u = dev->opaque;
> + if (u->user->memory_isolation) {
> + if (started) {
> + vhost_user_svqs_start(dev);
> + } else {
> + vhost_user_svqs_stop(dev);
> + }
> + }
> +
> if (!vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_STATUS))
> {
> return 0;
> }
>
> --
> 2.43.0
>
signature.asc
Description: PGP signature
