Re: [PATCH v3] vfio/type1: optimize vfio_pin_pages_remote() for huge folio

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 26 May 2025 17:19:55 -0300
Jason Gunthorpe <jgg@ziepe.ca> wrote:

> On Wed, May 21, 2025 at 10:55:12AM -0600, Alex Williamson wrote:
> 
> > This optimization does rely on an assumption of consecutive _pages_ in
> > the array returned from GUP.  If we cannot assume the next array index
> > is the next page from the same folio (which afaict we have no basis to
> > do), we cannot use the folio as the basis for any optimization.  
> 
> Right! I was wondering why this code was messing with folios, it
> really can't learn anything from folios. The only advantage to folios
> is during unpinning where we can batch the atomics for all the folio
> sub pages, which the core mm helpers are doing.

I *think* all we're gaining is that comparing page pointers is
slightly more lightweight than iterating page_to_pfn() and that we can
skip checking whether we've crossed an inflection in pages considered
reserved within a folio.  I'm curious to see to what extent this
optimization is still worthwhile.

> Which brings me back to my first remark - this is all solved in
> iommufd, in a much better way :( I continue to think we should just
> leave this type1 stuff as-is upstream and encourage people to move
> forward.
> 
> Lots of CSPs are running iommufd now. There is a commonly used OOT
> patch to add the insecure P2P support like VFIO. I know lots of folks
> have backported iommufd.. No idea about libvirt, but you can run it in
> compatibility mode and then you don't need to change libvirt.

For distributions that don't have an upstream first policy, sure, they
can patch whatever they like.  I can't recommend that solution though.

Otherwise the problem with compatibility mode is that it's a compile
time choice.  A single kernel binary cannot interchangeably provide
either P2P DMA with legacy vfio or better IOMMUFD improvements without
P2P DMA.  If libvirt had IOMMUFD support, XML could specify the
interface on a per-device bases, and maybe even allow opt-in to a
system-wide policy choice.  Thanks,

Alex