From: Alex Williamson <alex@shazbot.org>
To: Mina Almasry <almasrymina@google.com>
Cc: Peter Xu <peterx@redhat.com>,
Andrew Morton <akpm@linux-foundation.org>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest@vger.kernel.org,
David Matlack <dmatlack@google.com>,
Shuah Khan <shuah@kernel.org>, Kevin Tian <kevin.tian@intel.com>,
Jason Gunthorpe <jgg@ziepe.ca>,
alex@shazbot.org
Subject: Re: [PATCH v2 2/2] vfio: selftests: Test BAR write faults after clear_refs
Date: Mon, 2 Nov 2026 17:11:49 -0700 [thread overview]
Message-ID: <20261102171149.5d4ed369@shazbot.org> (raw)
In-Reply-To: <20261101062049.998160-3-almasrymina@google.com>
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;
prev parent reply other threads:[~2026-11-03 0:11 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-01 6:20 [PATCH v2 0/2] vfio/pci: fix huge_fault write handling and add selftest Mina Almasry
2026-11-01 6:20 ` [PATCH v2 1/2] vfio/pci: pass FAULT_FLAG_WRITE to vmf_insert_pfn_pmd/pud() Mina Almasry
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 [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261102171149.5d4ed369@shazbot.org \
--to=alex@shazbot.org \
--cc=akpm@linux-foundation.org \
--cc=almasrymina@google.com \
--cc=dmatlack@google.com \
--cc=jgg@ziepe.ca \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=peterx@redhat.com \
--cc=shuah@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®