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.
