Re: [PATCH v2 2/2] vfio: selftests: Test BAR write faults after clear_refs

[David Matlack <dmatlack@google.com>]

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
>