Re: [PATCH 3/9] vfio/pci: Add a helper to create a DMABUF for a BAR-map VMA

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, May 06, 2026 at 08:03:08PM +0100, Matt Evans wrote:
> > > > > > +    /*
> > > > > > +     * The mmap() request's vma->vm_offs might be non-zero, but
> > > > > > +     * the DMABUF is created from _offset zero_ of the BAR.  The
> > > > > > +     * portion between zero and the vm_offs is inaccessible
> > > > > > +     * through this VMA, but this approach keeps the
> > > > > > +     * /proc/<pid>/maps offset somewhat consistent with the
> > > > > > +     * pre-DMABUF code.  Size includes the offset portion.
> > > > > 
> > > > > I'm not sure I understand this comment?
> > > > > 
> > > > > For the old path vm_pgoff for byte 0 of the bar starts at some large
> > > > > offset
> > > > > 
> > > > > For the new path vm_pgoff for byte 0 of the first range starts at 0
> > > > 
> > > > Glad you asked.  :)
> > > > 
> > > > This is trying to achieve keeping /proc/<pid>/maps (or similar) somewhat
> > > > as informative as pre-DMABUF BAR mmap, in terms of keeping the VMA
> > > > vm_offs column useful.  Before this patch, say you mmap() two slices A
> > > > and B of the same BAR:
> > > > 
> > > >   struct vfio_region_info bar_region;
> > > > 
> > > >   vm_a = mmap(0, 0x1000, ..., device_fd, bar_region.offset + 0);
> > > >   vm_b = mmap(0, 0x1000, ..., device_fd, bar_region.offset + 0x4000);
> > > > 
> > > > ...you'd see something like this in /proc/blah/maps:
> > > > 
> > > > fffff4000000-fffff4001000 rw-s 10000000000 00:07 148    
> > > > /dev/vfio/ devices/vfio0
> > > > fffff5000000-fffff5001000 rw-s 10000004000 00:07 148    
> > > > /dev/vfio/ devices/vfio0
> 
> Looking at this again, I/we got this backwards and I mixed up two things:
> 
> The goal of this patch _is already_ to make sure the VMA's vm_pgoff (whether
> viewed in /proc/<pid>/maps or elsewhere) still matches the mmap()'s offset.
> 
> (For a mo, ignore the resource index encoded into the offset.  Consider just
> the offset into the BAR itself, inside the VFIO_PCI_OFFSET_MASK. I'll come
> back to the index encoded into the upper bits.)
> 
> > > > then the VMA's vm_offs would need to be thunked back down to 0 (since
> > > > the fault handler then treats vm_b + 0 as the first byte of the DMABUF).
> > > > That works/adds up, but then the vm_offs of both VMAs A & B both have
> > > > offset 0, and it's harder to differentiate in /proc/blah/maps.
> > > 
> > > Yes, and that would be correct.
> 
> Why?  This paragraph was outlining a hypothetical alternative implementation
> that creates the DMABUF the size of the VMA and starting from an offset into
> the BAR based on vm_pgoff, and then compensates by setting vma->vm_pgoff = 0
> so that the fault doesn't re-apply the offset again.  That would make byte 0
> of the VMA access correct:

I see, I mis understood what you were suggesting

> This patch is supporting that property by instead creating the DMABUF so
> that the VMA's vm_pgoff (which is maintained and the same* as passed from
> mmap()!) indexes the DMABUF so that byte 0 of the VMA accesses the same
> address above in [1].  The DMABUF spans from the start of the BAR so the
> fault handler maths (which indexes the DMABUF by vm_pgoffs) is common for
> all buffers.
> 
>  a = mmap(0, 0x10000, ..., device_fd, 0x4000);

>          +0           +0x4000
>          +------------v------------------------------------------+
>          |               BAR                                     |
>          |                                                       |
>          +------------^------------------------------------------+
>          .            .
>          .            +--------------------------+
>          .            |  VMA                     |
>          .            |  vma->vm_pgoff = 4       |
>          .            +--------------------------+
>          .            .                          .
>          +------------+--------------------------+
>          | invisible  |  DMABUF                  |
>          |            |                          |
>          +------------+--------------------------+
> 
> Same* externally-observable behaviour as the old mmap().

Sure, but it is a mess..

You should create the dma_buf that is the narrow one that only covers
the requested mmap. The vma_pgoff should be exactly what is passed to mmap.

And then have a simple 'vma_pgoff_adjust' that fixes up the pgoff to
be 0 based for internal operation of the fault handler.

It is nonsense stuff like this:
+	priv->size = (vma->vm_pgoff << PAGE_SHIFT) + req_len;

That is really objectionable, the size should never have anything to
do with a pgoff.

Jason