* Re: [PATCH v2 2/2] vfio: selftests: Test BAR write faults after clear_refs
2026-11-01 6:20 ` [PATCH v2 2/2] vfio: selftests: Test BAR write faults after clear_refs Mina Almasry
@ 2026-11-02 21:45 ` David Matlack
2026-11-03 0:11 ` Alex Williamson
1 sibling, 0 replies; 5+ messages in thread
From: David Matlack @ 2026-11-02 21:45 UTC (permalink / raw)
To: Mina Almasry
Cc: Peter Xu, Andrew Morton, Alex Williamson, kvm, linux-kernel,
linux-kselftest, Shuah Khan, Kevin Tian, Jason Gunthorpe,
Matt Evans
On 2026-11-01 06:20 AM, Mina Almasry wrote:
> +TEST_F(vfio_pci_device_test, bar_write_fault_after_clear_refs)
> +{
> + const u32 rw_mmap_flags = VFIO_REGION_INFO_FLAG_MMAP |
> + VFIO_REGION_INFO_FLAG_READ |
> + VFIO_REGION_INFO_FLAG_WRITE;
> + struct vfio_pci_bar *bar;
> + bool tested = false;
> + int ret, i;
> + u32 val;
> +
> + ret = clear_soft_dirty_refs();
> + if (ret == -ENOENT || ret == -EINVAL)
> + SKIP(return, "soft-dirty clear_refs unsupported (%d)\n", ret);
> + ASSERT_EQ(0, ret);
> +
> + vfio_pci_cmd_set(self->device, PCI_COMMAND_MEMORY);
> +
> + for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> + void *vaddr;
> +
> + bar = &self->device->bars[i];
> + if ((bar->info.flags & rw_mmap_flags) != rw_mmap_flags)
> + continue;
> +
> + ASSERT_NE(NULL, bar->vaddr);
> +
> + /*
> + * clear_soft_dirty_refs() stripped write permissions from
> + * vma->vm_page_prot on the unpopulated BAR mapping created by
Sashiko's finding here is right. The BAR mappings may be populated
already at this point. You can reliably zap the BAR mappings by clearing
PCI_COMMAND_MEMORY. e.g.
static void zap_bar_mappings(struct vfio_pci_device *device)
{
vfio_pci_cmd_clear(device, PCI_COMMAND_MEMORY);
vfio_pci_cmd_set(device, PCI_COMMAND_MEMORY);
}
Then you can also use this instead of mmap()+madvise() below.
> + * vfio_pci_device_init(). Fault it read-only first, then write
> + * to exercise in-place write-upgrade (wp_huge_pmd/wp_huge_pud).
> + */
> + val = READ_ONCE(*(u32 *)bar->vaddr);
> + WRITE_ONCE(*(u32 *)bar->vaddr, val);
Writing to offset 0 of every read/write BAR isn't necessarily safe. It
could be a doorbell, a write-1-to-clear status register, the MSI-X
table, etc.
There's no way to trigger the write fault without actually writing to
the BAR (e.g. MADV_POPULATE_WRITE fails on VM_PFNMAP VMAs), so the test
needs to know where it's safe to write.
To solve this, I propose adding a helper to the VFIO selftests library
that finds a BAR (or range within a BAR) that behaves like memory, i.e.
every location can be read and written with arbitrary values without
side effects. Something like:
bool vfio_pci_find_mem_bar(struct vfio_pci_device *device, u64 min_size,
struct vfio_pci_mem_bar *mem_bar);
Detection would be based on a table of known devices, and tests SKIP()
when no such BAR is found. Your test would then ask for a mem BAR of at
least PMD_SIZE and run both cases on it.
This came up recently with Matt's DMABUF mmap() test as well [1], so I
think it belongs in the library rather than in vfio_pci_device_test.c.
It shouldn't be much more work than a test-local helper (but please add
it in a separate commit).
My suggestion would be for QEMU's pci-testdev (1b36:0005) to be the
first device in the table, since anyone can run it in a VM without any
special hardware. With membar-backed=on, pci-testdev's BAR2 is a 64-bit
prefetchable BAR backed by plain RAM, so it's safe to read and write
anywhere in it. membar-backed requires QEMU 10.1 or newer. For example:
qemu-system-x86_64 \
-machine q35,accel=kvm,kernel-irqchip=split \
-device intel-iommu,intremap=on \
-device pci-testdev,membar=1G,membar-backed=on \
...
Then boot the guest and run the test against the device:
$ BDF=$(lspci -D -d 1b36:0005 | awk '{print $1}')
$ ./scripts/setup.sh $BDF
$ ./vfio_pci_device_test $BDF
membar=1G is large enough to exercise both PMD and PUD mappings, since
vfio_pci_bar_map() aligns the mapping to min(size, 1G). If 1G of host
RAM for the BAR is too much, membar=2M still covers the PMD case.
If you want to also add the device you reproduced on to the table, that
works for me too. More devices can be added later as needed.
This could also be added to the vfio_pci_driver framework, but I'm not
sure that is a good idea. Devices that have large memory-like BARs
aren't necessarily the same devices that can implement memcpy and send
MSIs.
Would you be open to adding vfio_pci_find_mem_bar() as part of this
series?
[1] https://lore.kernel.org/kvm/CALzav=cGtmkBBpfUA8nP7AQ3azB_Hn7_8jJL=J+E+h61CpOFrw@mail.gmail.com/
> +
> + /*
> + * Replace the VMA with a fresh unpopulated mapping at the same
> + * hugepage-aligned address, clear soft-dirty again, and fault
> + * with an immediate store to exercise initial write insertion
> + * (vmf_insert_pfn_pmd/pud with write=true).
> + */
> + vaddr = mmap(bar->vaddr, bar->info.size, PROT_READ | PROT_WRITE,
> + MAP_SHARED | MAP_FIXED, self->device->fd,
> + bar->info.offset);
> + ASSERT_EQ(bar->vaddr, vaddr);
> + madvise(bar->vaddr, bar->info.size, MADV_HUGEPAGE);
> +
> + ASSERT_EQ(0, clear_soft_dirty_refs());
> +
> + WRITE_ONCE(*(u32 *)bar->vaddr, val);
> + (void)READ_ONCE(*(u32 *)bar->vaddr);
> +
> + tested = true;
Should only BARs >= PMD_SIZE be considered tested?
> + }
> +
> + vfio_pci_cmd_clear(self->device, PCI_COMMAND_MEMORY);
This should restore PCI_COMMAND_MEMORY to whatever it was at the start
of the test. Or just leave PCI_COMMAND_MEMORY enabled (that would also
be harmless).
> +
> + if (!tested)
> + SKIP(return, "No mappable read/write BAR on %s\n", device_bdf);
> +}
> +
> FIXTURE(vfio_pci_irq_test) {
> struct iommu *iommu;
> struct vfio_pci_device *device;
> --
> 2.56.0.385.gac671c819a-goog
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2 2/2] vfio: selftests: Test BAR write faults after clear_refs
2026-11-01 6:20 ` [PATCH v2 2/2] vfio: selftests: Test BAR write faults after clear_refs Mina Almasry
2026-11-02 21:45 ` David Matlack
@ 2026-11-03 0:11 ` Alex Williamson
1 sibling, 0 replies; 5+ messages in thread
From: Alex Williamson @ 2026-11-03 0:11 UTC (permalink / raw)
To: Mina Almasry
Cc: Peter Xu, Andrew Morton, kvm, linux-kernel, linux-kselftest,
David Matlack, Shuah Khan, Kevin Tian, Jason Gunthorpe, alex
On Sun, 1 Nov 2026 06:20:49 +0000
Mina Almasry <almasrymina@google.com> wrote:
> Add a regression test in vfio_pci_device_test to verify that huge_fault
> on writable MAP_SHARED VFIO PCI BAR mappings handles write faults after
> soft-dirty tracking clears VM_SOFTDIRTY and downgrades vma->vm_page_prot
> to read-only via /proc/self/clear_refs ("4").
>
> Specifically, for each mappable read/write BAR (already aligned up to
> SZ_1G by vfio_pci_bar_map() with MADV_HUGEPAGE), verify both:
>
> 1. Read-then-write fault sequence after clear_refs, exercising in-place
> huge PMD/PUD write-protection upgrade via wp_huge_pmd()/wp_huge_pud().
> 2. Direct initial write fault on a freshly mapped unpopulated BAR VMA
> after clear_refs, exercising vmf_insert_pfn_pmd()/vmf_insert_pfn_pud()
> with FAULT_FLAG_WRITE set.
>
> Skip gracefully when /proc/self/clear_refs is absent or soft-dirty
> tracking (CONFIG_MEM_SOFT_DIRTY) is disabled, or when the device exposes
> no mappable read/write BARs.
>
> Cc: David Matlack <dmatlack@google.com>
> Signed-off-by: Mina Almasry <almasrymina@google.com>
> ---
> .../selftests/vfio/vfio_pci_device_test.c | 83 +++++++++++++++++++
> 1 file changed, 83 insertions(+)
>
> diff --git a/tools/testing/selftests/vfio/vfio_pci_device_test.c b/tools/testing/selftests/vfio/vfio_pci_device_test.c
> index 93c11fd5e081..75ccc557efb9 100644
> --- a/tools/testing/selftests/vfio/vfio_pci_device_test.c
> +++ b/tools/testing/selftests/vfio/vfio_pci_device_test.c
> @@ -1,10 +1,13 @@
> // SPDX-License-Identifier: GPL-2.0-only
> +#include <errno.h>
> #include <fcntl.h>
> #include <stdlib.h>
> +#include <unistd.h>
>
> #include <sys/ioctl.h>
> #include <sys/mman.h>
>
> +#include <linux/compiler.h>
> #include <linux/limits.h>
> #include <linux/pci_regs.h>
> #include <linux/sizes.h>
> @@ -102,6 +105,86 @@ TEST_F(vfio_pci_device_test, validate_bars)
> }
> }
>
> +static int clear_soft_dirty_refs(void)
> +{
> + ssize_t ret;
> + int fd;
> +
> + fd = open("/proc/self/clear_refs", O_WRONLY);
> + if (fd < 0)
> + return -errno;
> +
> + ret = write(fd, "4\n", 2);
> + if (ret < 0)
> + ret = -errno;
> + else
> + VFIO_ASSERT_EQ(ret, 2);
> +
> + VFIO_ASSERT_EQ(close(fd), 0);
> + return ret < 0 ? ret : 0;
> +}
> +
> +TEST_F(vfio_pci_device_test, bar_write_fault_after_clear_refs)
> +{
> + const u32 rw_mmap_flags = VFIO_REGION_INFO_FLAG_MMAP |
> + VFIO_REGION_INFO_FLAG_READ |
> + VFIO_REGION_INFO_FLAG_WRITE;
> + struct vfio_pci_bar *bar;
> + bool tested = false;
> + int ret, i;
> + u32 val;
> +
> + ret = clear_soft_dirty_refs();
> + if (ret == -ENOENT || ret == -EINVAL)
> + SKIP(return, "soft-dirty clear_refs unsupported (%d)\n", ret);
-ENOENT seems to map to the open() when /proc/self/clear_refs is not
present. We can't clear soft-dirty by virtue that the file is absent.
Fine.
-EINVAL seems to be intended to catch:
clear_refs_write():
if (type < CLEAR_REFS_ALL || type >= CLEAR_REFS_LAST)
return -EINVAL;
But that's only testing for a known type, not that it's supported.
This is never triggered, which can lead to a false pass if soft-dirty
really isn't supported and silently does nothing.
> + ASSERT_EQ(0, ret);
> +
> + vfio_pci_cmd_set(self->device, PCI_COMMAND_MEMORY);
> +
> + for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> + void *vaddr;
> +
> + bar = &self->device->bars[i];
> + if ((bar->info.flags & rw_mmap_flags) != rw_mmap_flags)
> + continue;
Where do we filter out BARs that wouldn't trigger a PMD/PUD fault? If
we're only testing sub-PMD BARs, this is another false pass.
If we do find a BAR to trigger a PMD/PUD mapping, what's the value in
continuing to exhaustively search and test the remaining BARs?
> +
> + ASSERT_NE(NULL, bar->vaddr);
> +
> + /*
> + * clear_soft_dirty_refs() stripped write permissions from
> + * vma->vm_page_prot on the unpopulated BAR mapping created by
> + * vfio_pci_device_init(). Fault it read-only first, then write
> + * to exercise in-place write-upgrade (wp_huge_pmd/wp_huge_pud).
> + */
> + val = READ_ONCE(*(u32 *)bar->vaddr);
> + WRITE_ONCE(*(u32 *)bar->vaddr, val);
Sashiko notes[1] an issue here that might be a false positive, but the
comment should at least note that a matching driver could already have
touched the BAR via read/write but doesn't invalidate the test... if so.
Thanks,
Alex
[1]https://sashiko.dev/#/patchset/20261101062049.998160-1-almasrymina@google.com
> +
> + /*
> + * Replace the VMA with a fresh unpopulated mapping at the same
> + * hugepage-aligned address, clear soft-dirty again, and fault
> + * with an immediate store to exercise initial write insertion
> + * (vmf_insert_pfn_pmd/pud with write=true).
> + */
> + vaddr = mmap(bar->vaddr, bar->info.size, PROT_READ | PROT_WRITE,
> + MAP_SHARED | MAP_FIXED, self->device->fd,
> + bar->info.offset);
> + ASSERT_EQ(bar->vaddr, vaddr);
> + madvise(bar->vaddr, bar->info.size, MADV_HUGEPAGE);
> +
> + ASSERT_EQ(0, clear_soft_dirty_refs());
> +
> + WRITE_ONCE(*(u32 *)bar->vaddr, val);
> + (void)READ_ONCE(*(u32 *)bar->vaddr);
> +
> + tested = true;
> + }
> +
> + vfio_pci_cmd_clear(self->device, PCI_COMMAND_MEMORY);
> +
> + if (!tested)
> + SKIP(return, "No mappable read/write BAR on %s\n", device_bdf);
> +}
> +
> FIXTURE(vfio_pci_irq_test) {
> struct iommu *iommu;
> struct vfio_pci_device *device;
^ permalink raw reply [flat|nested] 5+ messages in thread