Re: [PATCH RFC 12/15] hw/virtio/vhost-user: send isolation regions to device

Stefan Hajnoczi <[email protected]>
Newsgroups gmane.comp.emulators.qemu.block,gmane.comp.emulators.qemu
Message-ID <20260728191620.GJ371693@fedora>
On Thu, Jul 23, 2026 at 03:30:11PM -0700, Connor Kite wrote:
> Adds features to fill a vhost_user_set_mem_table message with the
> addresses of isolation memory regions corresponding to bounce buffers
> and vrings.
> 
> Signed-off-by: Connor Kite <[email protected]>
> ---
>  hw/virtio/vhost-user.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 79 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 710cf966f8..acabfb7f1c 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -625,6 +625,21 @@ static void vhost_user_fill_msg_region(struct vhost_dev *dev,
>      dst->mmap_offset = mmap_offset;
>  }
>  
> +static void vhost_user_fill_msg_region_iso(VhostUserMemoryRegion *dst,
> +                                           const struct vhost_user *u,
> +                                           const struct vhost_memory_region
> +                                           *iova_reg)
> +{
> +    assert(u != NULL && dst != NULL && iova_reg != NULL);
> +    uint64_t offset;
> +
> +    offset = iova_reg->userspace_addr - u->iso_memory.base_addr;
> +    dst->userspace_addr = iova_reg->userspace_addr;
> +    dst->memory_size = iova_reg->memory_size;
> +    dst->guest_phys_addr = iova_reg->userspace_addr;
> +    dst->mmap_offset = offset;
> +}
> +
>  static int vhost_user_fill_set_mem_table_msg(struct vhost_user *u,
>                                               struct vhost_dev *dev,
>                                               VhostUserMsg *msg,
> @@ -1136,7 +1151,33 @@ static void cleanup_isolation_regions(struct vhost_dev *dev)
>      }
>  }
>  
> -__attribute__((unused))
> +struct iova_tree_traversal_args {

QEMU coding style:

  typedef struct {
      ...
  } IOVATreeTraversalArgs;

> +    VhostUserMsg *msg;
> +    struct vhost_user *u;
> +    int *fds;
> +    size_t *fd_num;

This field is also used to index into msg->payload.memory.regions[], so
"fd_num" is a misnomer. I suggest something like "region_idx" or just
"idx".

> +};
> +
> +static gboolean vhost_user_iova_tree_traverse_funct(gpointer key,
> +                                                    gpointer value,
> +                                                    gpointer data)
> +{
> +    struct iova_tree_traversal_args *args = data;
> +    struct vhost_memory_region msg_region;
> +    VhostUserMemoryRegion region_buffer;
> +    DMAMap *map = key;
> +    args->fds[*args->fd_num] = args->u->iso_memory.iso_fd;
> +
> +    msg_region.guest_phys_addr = map->iova;
> +    msg_region.memory_size = map->size + 1;
> +    msg_region.userspace_addr = map->iova;
> +    vhost_user_fill_msg_region_iso(&region_buffer, args->u, &msg_region);
> +    args->msg->payload.memory.regions[*args->fd_num] = region_buffer;

I'm confused by this code. VhostUserMemoryRegion region_buffer is the
vhost-user protocol struct that is being filled in, but there is also a
struct vhost_memory_region msg_region from the Linux kernel headers?

msg_region and vhost_user_fill_msg_region_iso() make it harder to see
what is going on. Can you open code the region_buffer struct field
assignments instead?

> +    (*args->fd_num)++;
> +
> +    return false;
> +}
> +
>  static int init_isolation_regions(struct vhost_dev *dev,
>                                    VhostUserMsg *msg,
>                                    int *fds, size_t *fd_num)
> @@ -1234,6 +1275,24 @@ static int init_isolation_regions(struct vhost_dev *dev,
>                                        map->translated_addr);
>      }
>  
> +    struct iova_tree_traversal_args args = {
> +        .fd_num = fd_num,
> +        .fds = fds,
> +        .msg = msg,
> +        .u = u
> +    };
> +
> +    vhost_iova_tree_foreach(u->iso_iova_tree,
> +                            vhost_user_iova_tree_traverse_funct, &args);
> +
> +    msg->payload.memory.nregions = *fd_num;
> +
> +    assert(*fd_num != 0);
> +
> +    msg->hdr.size = sizeof(msg->payload.memory.nregions);
> +    msg->hdr.size += sizeof(msg->payload.memory.padding);
> +    msg->hdr.size += *fd_num * sizeof(VhostUserMemoryRegion);
> +
>      return 0;
>  }
>  
> @@ -1241,6 +1300,7 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
>                                      struct vhost_memory *mem)
>  {
>      struct vhost_user *u = dev->opaque;
> +    bool memory_isolation = u->user->memory_isolation;
>      int fds[VHOST_MEMORY_BASELINE_NREGIONS];
>      size_t fd_num = 0;
>      bool do_postcopy = u->postcopy_listen && u->postcopy_fd.handler;
> @@ -1268,6 +1328,24 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
>          msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
>      }
>  
> +    if (memory_isolation) {
> +        ret = init_isolation_regions(dev, &msg, fds, &fd_num);
> +        if (ret < 0) {
> +            return ret;
> +        }
> +
> +        ret = vhost_user_write(dev, &msg, fds, fd_num);
> +        if (ret < 0) {
> +            return ret;
> +        }
> +
> +        if (reply_supported) {
> +            return process_message_reply(dev, &msg);
> +        }
> +
> +        return 0;
> +    }
> +
>      if (config_mem_slots) {
>          ret = vhost_user_add_remove_regions(dev, &msg, reply_supported, false);
>          if (ret < 0) {
> 
> -- 
> 2.43.0
>
signature.asc (application/pgp-signature, 488 B)
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEhpWov9P5fNqsNXdanKSrs4Grc8gFAmppAAQACgkQnKSrs4Gr
c8iXlAf/TVBhkZrgVsUI+z+qyzvmD3DJfqr3zuyFtJyyZzkwc4laZEugQGhEUtdS
BAqMG+biQ4Qyb0eII/yzy+ieWUtOj2uDoZx+nvmKRE6iJmwqhn7jZdKHjLbUsNIG
WFWV4Ym/AmMXs1wgnz2n+/L8BQ6qag0KhEc24GTHnuC81+wH7sU1LE5kSSh33x5v
QFJRrPh+4tZpntjGbv/hs3AOOeOiALR8uR4VIPaL6Jz/MeB0iSloE/96bcOrRZDK
JloH1f0QrFhiVM8auiKG56AvGo5hE+JVvYRZaUSspiyna9AaKhptGQ9UPsB07Hc+
P1qOsQtYqgto7jMemu20LWYXo1VAyw==
=vR+H
-----END PGP SIGNATURE-----
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.