Re: [PATCH] vfio: selftests: Add vfio_dma_mapping_mmio_test

[Alex Mastro <amastro@fb.com>]

On Thu, Jan 08, 2026 at 10:38:04AM -0400, Jason Gunthorpe wrote:
> On Wed, Jan 07, 2026 at 07:36:44PM -0800, Alex Mastro wrote:
> > This was inspired by QEMU's hw/vfio/region.c which also does this rounding up
> > of size to the next power of two [1].
> > 
> > I'm now realizing that's only necessary for regions with
> > VFIO_REGION_INFO_CAP_SPARSE_MMAP where there are multiple mmaps per region, and
> > each mmap's size is less than the size of the BAR. Here, since we're mapping the
> > entire BAR which must be pow2, it shouldn't be necessary.
> 
> You only need to do this dance if you care about having large PTEs
> under the VMAs, which is probably something worth testing both
> scenarios.

Yep, makes sense. The test takes a long time to run without this due potentially
faulting a 128G BAR region 4K at a time during VFIO_IOMMU_MAP_DMA.

> 
> > The intent of QEMU's mmap alignment code is imperfect in the SPARE_MMAP case?
> > After a hole, the next mmap'able range could be some arbitrary page-aligned
> > offset into the region. It's not helpful mmap some region offset which is
> > maximally 4K-aligned at a 1G-aligned vaddr.
> > 
> > I think to be optimal, QEMU should be attempting to align the vaddr for bar
> > mmaps such that
> > 
> > vaddr % {2M,1G} == region_offset % {2M,1G}
> > 
> > Would love someone to sanity check me on this. Kind of a diversion.
> 
> What you write is correct. Ankit recently discovered this bug in
> qemu. It happens not just with SPARSE_MMAP but also when mmmaping
> around the MSI-X hole..

Is my mental model broken? I thought MSI-X holes in a VFIO-exposed BAR region
implied SPARSE_MMAP? I didn't think there was another way for the uapi to
express hole-yness.

> 
> I also advocated for what you write here that qemu should ensure:
> 
>   vaddr % region_size == region_offset % region_size

Why region_size out of curiosity? Assuming perfect knowledge of kernel internals
I would have expected something like this:

diff --git a/hw/vfio/region.c b/hw/vfio/region.c
index ca75ab1be4..1d8595e808 100644
--- a/hw/vfio/region.c
+++ b/hw/vfio/region.c
@@ -238,6 +238,18 @@ static void vfio_subregion_unmap(VFIORegion *region, int index)
     region->mmaps[index].mmap = NULL;
 }
 
+/*
+ * Return the next value greater than or equal to `input` such that
+ * (value % align) == offset.
+ */
+static size_t align_offset(size_t input, size_t offset, size_t align)
+{
+    size_t remainder = input % align;
+    size_t delta = (align + offset - remainder) % align;
+
+    return input + delta;
+}
+
 int vfio_region_mmap(VFIORegion *region)
 {
     int i, ret, prot = 0;
@@ -252,7 +264,11 @@ int vfio_region_mmap(VFIORegion *region)
     prot |= region->flags & VFIO_REGION_INFO_FLAG_WRITE ? PROT_WRITE : 0;
 
     for (i = 0; i < region->nr_mmaps; i++) {
-        size_t align = MIN(1ULL << ctz64(region->mmaps[i].size), 1 * GiB);
+        size_t size = region->mmaps[i].size;
+        size_t offs = region->mmaps[i].offset;
+        size_t align = size >= GiB ? GiB :
+                       size >= 2 * MiB ? 2 * MiB :
+                       getpagesize();
         void *map_base, *map_align;
 
         /*
@@ -275,7 +291,7 @@ int vfio_region_mmap(VFIORegion *region)
 
         fd = vfio_device_get_region_fd(region->vbasedev, region->nr);
 
-        map_align = (void *)ROUND_UP((uintptr_t)map_base, (uintptr_t)align);
+        map_align = (void *)align_offset((size_t)map_base, offs % align, align);
         munmap(map_base, map_align - map_base);
         munmap(map_align + region->mmaps[i].size,
                align - (map_align - map_base));