Re: [PATCH v3 03/23] vfio: add per-region fd support

[John Levon <john.levon@nutanix.com>]

On Tue, Jun 10, 2025 at 09:42:41AM +0200, Cédric Le Goater wrote:

> On 6/7/25 02:10, John Levon wrote:
> > For vfio-user, each region has its own fd rather than sharing
> > vbasedev's. Add the necessary plumbing to support this, and use the
> > correct fd in vfio_region_mmap().
> > 
> > @@ -172,10 +174,11 @@ struct VFIODeviceIOOps {
> >       /**
> >        * @get_region_info
> >        *
> > -     * Fill in @info with information on the region given by @info->index.
> > +     * Fill in @info (and optionally @fd) with information on the region given
> > +     * by @info->index.
> >        */
> 
> Could you please update the whole documentation of the VFIODeviceIOOps
> struct and document each parameter ?

Sorry, not sure what you're asking for. This struct was fully documented in
38bf025d ("vfio: add device IO ops vector")
and its subsequent patches.

> > @@ -360,6 +369,7 @@ void vfio_device_init(VFIODevice *vbasedev, int type, VFIODeviceOps *ops,
> >       vbasedev->io_ops = &vfio_device_io_ops_ioctl;
> >       vbasedev->dev = dev;
> >       vbasedev->fd = -1;
> > +    vbasedev->use_region_fds = false;
> 
> why not extend vfio_device_init() with a 'region_fd_cache' bool
> parameter instead ?

I could do, but you previously asked me not to add an "io_ops" parameter to this
function and instead directly change it here - why is this parameter different?

> and avoid the extra VFIODevice attribute.

I don't get this - we still need to record the boolean in the vbasedev.

regards
john