On Wed, Nov 05, 2025 at 01:58:04PM -0700, Alex Williamson wrote: > On Mon, 3 Nov 2025 09:53:04 +0000 > Mostafa Saleh <[email protected]> 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
