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. > > 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, Alex
