Re: [RFC PATCH 2/3] vfio: mmap sparse regions backed by multiple FDs

On Fri, Aug 14, 2026 at 1:52 AM John Levon <[email protected]> wrote:
>
> On Tue, Aug 11, 2026 at 11:39:29PM +0000, Naman Gulati wrote:
>
> > Generalize QEMU's VFIO core layer to support mapping sparse memory
> > subregions from distinct file descriptors within a single device
> > region.
> >
> > Replace list of region_fds in VFIODevice with a collection of struct
> > VFIORegionFDs that internally holds a collection of FDs to mmap per
> > region.
>
> This is getting a bit non-obvious to me, we now have a region_fds array where
> each member is also an array.
>
> Given that you've introduced a dependency on VFIORegion in the
> include/hw/vfio/vfio-device.h for the other callback, I'm wondering why we don't
> just store an array of VFIORegions in VFIODevice instead of reginfo+region_fds ?

I tried to maintain the idiom that the VFIORegion region is synonymous
with one single BAR region. As before each VFIORegion has multiple
mmap areas underneath, the fd for each is now held in the region_fds
list.

>
> And I'm wondering if we really have to have two separate callbacks for getting
> region info.
>
>
> > @@ -184,6 +184,12 @@ static int vfio_user_device_io_get_region_info(VFIODevice *vbasedev,
> >          return ret;
> >      }
> >
> > +    if (fds.recv_fds > 0 && info->index < vbasedev->num_initial_regions) {
> > +        vbasedev->region_fds[info->index].fds = g_new0(int, 1);
> > +        vbasedev->region_fds[info->index].nr_fds = 1;
> > +        vbasedev->region_fds[info->index].fds[0] = fd;
> > +    }
> > +
>
> This seems quite surprising behaviour in particular - if we happen to be asking
> about a "standard" region, then we'll store its fd info in vbasedev. But we
> might not be setting device->reginfo[index] at the same time.
>
> I'd expect vbasedev to be const in these callbacks.
>
> And what about the special regions > num_initial_regions?
>
> I feel like this callback should be passing in a VFIORegion to be filled in,
> then the caller can do as they wish with it. We could even embed the
> vfio_region_info into VFIORegion for convenience for the callers that want caps
> etc.

This approach makes sense to me I can refactor accordingly & respin.

>
> Alternatively, if we don't want to do this, then at least get_region_info()
> should take a VFIORegionFds out parameter, that the caller can then do what they
> like with.
>
> Thoughts Cédric ?
>
> I'm also not sure why we have a sparse mmaps callback - why can't that be
> generic code that handles both capabilities? Are you concerned about the cap
> number clashing in future vfio kernel versions perhaps?

Yes I wanted to encapsulate vfio-user specific handling within the
vfio-user logic so that it doesn't conflict with the pure vfio device
handling. It felt odd to me to introduce generic handling of a
capability that's only exposed by one flavor of device and not the
other. However, no strong opinions, I can refactor and generalize the
cap handling if that's preferred, and the risks of cap number clashes
are okay.

>
> regards
> john