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

On Fri, Jul 24, 2026 at 6:33 AM Akihiko Odaki
<[email protected]> wrote:
>
> > +static gboolean vhost_user_iova_tree_traverse_funct(gpointer key,
> > +                                                    gpointer value,
> > +                                                    gpointer data)
>
> It is a bit odd naming. Functions usually do not have "funct" or similar
> suffixes.
>

I was mostly trying to convey that this was getting registered as a callback
to be called on each node of the iova_tree.  For more clarity, I am renaming it
vhost_user_iova_tree_fill_msg_reg.

> > +{
> > +    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;
>
> This may overrun the buffer because *args->fd_num is not checked as done
> in vhost_user_fill_set_mem_table_msg() for the non-isolation case.

Got it!  I have added a check here.

...

> >       int fds[VHOST_MEMORY_BASELINE_NREGIONS];
>
> This needs one more element for vring.
>

Good catch.  Fixed!

> >       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;
> > +    }
> > +
>
> This does not cover the postcopy path.
>
> It also allocates memory that is never sent for vq_index != 0. The
> isolation regions need to be shared by all vqs. Please see
> vhost_user_write() for the context.
>
> Regards,
> Akihiko Odaki
>
> >       if (config_mem_slots) {
> >           ret = vhost_user_add_remove_regions(dev, &msg, reply_supported, false);
> >           if (ret < 0) {
> >
>

I am less familiar with postcopy, but I think it might make sense to
break out support
for that into a separate patch. For now, I will return -1 in the
postcopy path if isolation
is active.

I'll need to think about how to handle devices like virtio-net and
make some changes to
this and the previous patch so that:
1. I am not allocating a new shared region for each vhost_dev
2. The shared region has enough space for the svq vrings of all devices.