Re: [PATCH 00/22] vfio: Give VFIO_DEVICE_GET_REGION_INFO its own op

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Nov 05, 2025 at 01:58:04PM -0700, Alex Williamson wrote:
> On Mon, 3 Nov 2025 09:53:04 +0000
> Mostafa Saleh <smostafa@google.com> wrote:
> 
> > On Thu, Oct 23, 2025 at 08:09:14PM -0300, Jason Gunthorpe wrote:
> > > There is alot of duplicated code in the drivers for processing
> > > VFIO_DEVICE_GET_REGION_INFO. Introduce a new op get_region_info_caps()
> > > which provides a struct vfio_info_cap and handles the cap chain logic
> > > to write the caps back to userspace and remove all of this duplication
> > > from drivers.
> > > 
> > > This is done in two steps, the first is a largely mechanical introduction
> > > of the get_region_info(). These patches are best viewed with the diff
> > > option to ignore whitespace (-b) as most of the lines are re-indending
> > > things.
> > > 
> > > Then drivers are updated to remove the duplicate cap related code. Some
> > > drivers are converted to use vfio_info_add_capability() instead of open
> > > coding a version of it.  
> > 
> > The series as a whole looks good.
> > However, I got confused walking through it as almost all non-PCI drivers
> > had to transition to get_region_info then get_region_info_caps then
> > removing get_region_info completely from core code after introducing
> > it in this series.

This makes it alot easier to read as most of the lines are just
re-indenting code. If you try to do it in one shot the patches would
be much more dense. The stopping point at get_region_info() lets it be
more incremental..

> > IMO, the series should start with just consolidating PCI based implementation
> > and then add get_region_info_caps for all drivers at the end.
> > Anyway, no really strong opinion as the final outcome makes sense.
> 
> I agree it was a bit indirect to get there, but the result still makes
> sense and I don't think it's worth reworking the series.
>
> I think Eric has some outstanding naming concerns and Praan noted that
> either a comment or gratuitous kfree(caps.buf) might be worthwhile to
> keep call-outs in check regarding cap buffer leaks.  I don't think we
> have any such cases, but it can't hurt to note the policy at least.
> 
> Otherwise, LGTM.  Should these be addressed as follow-ups rather than a
> re-spin?  Thanks,

I was planning to respin it but time keeps ticking away..

Jason