Re: [PATCH v5 3/9] vfio/pci: Add a helper to look up PFNs for DMABUFs

[Pranjal Shrivastava <praan@google.com>]

On Mon, Aug 03, 2026 at 07:22:06PM +0100, Matt Evans wrote:

Hi Matt,

> Hi Praan,
> 
> On 31/07/2026 18:43, Matt Evans wrote:
> > Hi Praan,
> > 
> > On 30/07/2026 23:55, Pranjal Shrivastava wrote:
> >> On Wed, Jul 15, 2026 at 06:47:26PM +0100, Matt Evans wrote:
> >>> Add vfio_pci_dma_buf_find_pfn(), which a VMA fault handler can use to
> >>> find a PFN.
> >>>
> >>> This supports multi-range DMABUFs, which typically would be used to
> >>> represent scattered spans but might even represent overlapping or
> >>> aliasing spans of PFNs.
> >>>
> >>> Because this is intended to be used in vfio_pci_core.c, we also need
> >>> to expose the struct vfio_pci_dma_buf in the vfio_pci_priv.h header.
> >>>
> >>> Signed-off-by: Matt Evans <matt@ozlabs.org>
> >>> ---
> >>>  drivers/vfio/pci/vfio_pci_dmabuf.c | 153 ++++++++++++++++++++++++++---
> >>>  drivers/vfio/pci/vfio_pci_priv.h   |  20 ++++
> >>>  2 files changed, 160 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
> >>> index c16f460c01d6..7c047400dfd1 100644
> >>> --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> >>> +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
> >>> @@ -9,19 +9,6 @@
> >>>  
> >>>  MODULE_IMPORT_NS("DMA_BUF");
> >>>  
> >>> -struct vfio_pci_dma_buf {
> >>> -	struct dma_buf *dmabuf;
> >>> -	struct vfio_pci_core_device *vdev;
> >>> -	struct list_head dmabufs_elm;
> >>> -	size_t size;
> >>> -	struct phys_vec *phys_vec;
> >>> -	struct p2pdma_provider *provider;
> >>> -	u32 nr_ranges;
> >>> -	struct kref kref;
> >>> -	struct completion comp;
> >>> -	u8 revoked : 1;
> >>> -};
> >>> -
> >>>  static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf,
> >>>  				   struct dma_buf_attachment *attachment)
> >>>  {
> >>> @@ -106,6 +93,146 @@ static const struct dma_buf_ops vfio_pci_dmabuf_ops = {
> >>>  	.release = vfio_pci_dma_buf_release,
> >>>  };
> >>>  
> >>> +int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *priv,
> >>> +			      struct vm_area_struct *vma,
> >>> +			      unsigned long fault_addr,
> >>> +			      unsigned int order,
> >>> +			      unsigned long *out_pfn)
> >>> +{
> >>> +	/*
> >>> +	 * Given a VMA (start, end, pgoffs) and a fault address,
> >>> +	 * search the corresponding DMABUF's phys_vec[] to find the
> >>> +	 * range representing the address's offset into the VMA, and
> >>> +	 * its PFN.
> >>> +	 *
> >>> +	 * The phys_vec[] ranges represent contiguous spans of VAs
> >>> +	 * upwards from the buffer offset 0; the actual PFNs might be
> >>> +	 * in any order, overlap/alias, etc.  Calculate an offset of
> >>> +	 * the desired page given VMA start/pgoff and address, then
> >>> +	 * search upwards from 0 to find which span contains it.
> >>> +	 *
> >>> +	 * On success, a valid PFN for a page sized by 'order' is
> >>> +	 * returned into out_pfn.
> >>> +	 *
> >>> +	 * Failure occurs if:
> >>> +	 * - A hugepage would cross the edge of the VMA,
> >>> +	 * - A hugepage isn't entirely contained within a range
> >>> +	 *   (including where it straddles the boundary between
> >>> +	 *   ranges),
> >>> +	 * - We find a range, but the final PFN isn't aligned to the
> >>> +	 *   requested order.
> >>> +	 *
> >>> +	 * Upon failure, -EAGAIN is returned and the caller is
> >>> +	 * expected to try again with a smaller order, which will
> >>> +	 * eventually succeed (order=0 will always work).
> >>> +	 *
> >>> +	 * It's suboptimal if DMABUFs are created with neighbouring
> >>> +	 * ranges that are physically contiguous, since hugepages
> >>> +	 * can't straddle range boundaries.  (The construction of the
> >>> +	 * ranges should merge them in this case.)
> >>> +	 *
> >>> +	 * Finally, vma_pgoff_adjust is used with a DMABUF created for
> >>> +	 * a VFIO BAR mmap: a BAR mapped with vm_pgoff > 0 creates a
> >>> +	 * DMABUF such that byte 0 of the VMA corresponds to byte 0 of
> >>> +	 * the DMABUF and byte 'vm_pgoff << PAGE_SHIFT' into the BAR.
> >>> +	 * To avoid double-offsetting in this scenario, subtracting
> >>> +	 * vma_pgoff_adjust from this (non-zero) vm_pgoff generates
> >>> +	 * the effective offset.
> >>> +	 */
> >>> +
> >>> +	const unsigned long pagesize = PAGE_SIZE << order;
> >>> +	unsigned long vma_off = ((vma->vm_pgoff - priv->vma_pgoff_adjust) <<
> >>> +				 PAGE_SHIFT) & VFIO_PCI_OFFSET_MASK;
> >>
> >> Maybe I'm getting ahead of myself here.. but it seems like this
> >> restricts us to only mapping DMABUFs at offsets < 1TB due to the 
> >> VFIO_PCI_OFFSET_MASK (since we have HBMs on PCI devices now, hitting 1TB
> >> may not be a very distant future).
> > 
> > This is a really good question, thanks for raisiing it.  It is not too
> > forward-thinking at all.
> > 
> >> While I understand this mask is needed to drop the BAR encoding in the
> >> high bits. 
> >>
> >> My worry is, if in the future a user were to export a massive
> >> contiguous DMABUF (e.g., >1TB of aggregated HBM) and tried to mmap deep
> >> into it (passing an offset >= 1TB), this bitwise AND would silently drop
> >> the high bits, leading to silent data corruption.
> > 
> > One of the big advantages of DMABUF export was that the range could be
> > huge and unencumbered by the VFIO_PCI_OFFSET_SHIFT of the traditional
> > mmap() interface.  It's a way to mmap huge BARs without having to change
> > the user-visible shift).  So definitely this is a relevant concern.
> > 
> >> I think we should explicitly reject such an mmap with -EINVAL like:
> >>
> >> +const unsigned long pagesize = PAGE_SIZE << order;
> >> +unsigned long vma_off = (vma->vm_pgoff - priv->vma_pgoff_adjust) << PAGE_SHIFT;
> >>
> >> +/*
> >> + * Prevent silent wrap-around if the user mmaps a DMABUF at an
> >> + * offset greater than the VFIO index mask allows.
> >> + */
> >> +if (unlikely(vma_off > VFIO_PCI_OFFSET_MASK))
> >> +    return -EINVAL;
> >>
> >> +vma_off &= VFIO_PCI_OFFSET_MASK;
> > 
> > Agreed, for now this absolutely should not silently wrap if the offset
> > is > 1TB, and we live with the restriction that a DMABUF sized >1TB
> > can't be mapped with such an offset.  (We can still, say, map all of a
> > 16TB DMABUF with offset=0, which is good.).  I'll add a check, thanks
> > for pointing this out.
> > 
> > I suggest as something to revisit later, we flag for a DMABUF (maybe an
> > evolution of vma_pgoff_adjust) to differentiate whether this masking
> > needs to be applied (traditional mmap() path) or not (DMABUF mmap()),
> > and then offsets can be arbitrarily large.
> 
> Having tinkered, the two-step approach I suggested isn't great because
> userspace has no easy way to know that step 2 has occurred and that
> large offsets are "now supported".
> 
> The second problem is this isn't just a limitation to not do an mmap()
> with offset approaching 1TB, because you could run into trouble just
> mapping, say, a 2TB BAR and then splitting it by unmapping a hole in the
> middle:  the second VMA now has a huge offset.
> 
> It seems a proper fix is easy though:
> 
>  unsigned long vma_off = (vma->vm_pgoff - priv->vma_pgoff_adjust) <<
> 	 		  PAGE_SHIFT;  /* No masking! */
> 
> Then in vfio_pci_core_mmap_prep_dmabuf(),
> 
>  priv->vma_pgoff_adjust = vma->vm_pgoff;
> 
> I.e., if vma_pgoff_adjust just includes the region index up high, it
> cancels out the (same) index in the vm_pgoff.  Thus regular mmap should
> work (up to the 1TB offset), and DMABUF VMAs can have arbitrarily large
> offsets.

This looks good to me! I traced through both mmap paths to verify how 
vma_pgoff_adjust handles the offsets and it looks like this works 
perfectly for both.

Cheers,
Praan