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
