[PATCH 0/2] KVM: Bound KVM_PRE_FAULT_MEMORY by the vCPU's dirty ring

[PATCH 0/2] KVM: Bound KVM_PRE_FAULT_MEMORY by the vCPU's dirty ring

[Fuad Tabba]

Hi folks,

KVM_PRE_FAULT_MEMORY never checks the vCPU's dirty ring, so one call
over enough populated, dirty-logged pages overflows it. The first patch
stops the loop at the soft limit, as KVM_RUN does. A stopped call
returns its partial progress, or, if it stopped before any, -ENOSPC
rather than the retryable -EAGAIN, since no retry can succeed until the
ring is harvested. The second patch adds the case to
pre_fault_memory_test.

On x86 a read fault normally maps a host-writable page writable and logs
it at once, so pre-faulting overflows the ring. Sean's auto-prefetch
series [1] keeps that behaviour for a pre-fault, so this series bounds
the loop rather than have x86 write-protect a pre-faulted page. On
arm64, which gained the ioctl in 7.4, pre-faulting RAM logged with the
ring alone overflows it too, until Wei-Lin's fix [2] is in mainline.

Pre-faulting isn't the only ioctl that can push past the ring's
reserve. On x86, KVM_SET_MSRS does too when it writes the wall clock or
the Hyper-V hypercall page: up to a few hundred entries in a single
call, with no bound across calls. KVM_SET_MSRS can't take the same
stop, since a short count from it means the MSR failed and QEMU asserts
on it.

What those paths should do when the ring fills is an open question for
the dirty ring API, which I don't think belongs in this series.
Pre-faulting a large logged range is a supported use of the ioctl, so
the loop should handle a full ring rather than lose entries.

Tested on x86 and arm64 under QEMU: without the first patch, the new
case hits the WARN on both.

Based on Linux 7.4-rc2 (7475ea8b58616).

Cheers,
/fuad

[1] https://lore.kernel.org/all/20261028202607.1844883-1-seanjc@google.com/
[2] https://lore.kernel.org/all/20261029175238.3214788-2-weilin.chang@arm.com/

Fuad Tabba (2):
  KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full
  KVM: selftests: Test KVM_PRE_FAULT_MEMORY against a full dirty ring

 Documentation/virt/kvm/api.rst                |  9 +-
 include/linux/kvm_dirty_ring.h                |  6 ++
 .../selftests/kvm/pre_fault_memory_test.c     | 85 +++++++++++++++++++
 virt/kvm/dirty_ring.c                         |  2 +-
 virt/kvm/kvm_main.c                           |  6 ++
 5 files changed, 105 insertions(+), 3 deletions(-)


base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
-- 
2.47.3

[PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full

[Fuad Tabba]

Pre-faulting can map a dirty-logged page writable, which pushes it to
the vCPU's dirty ring. KVM_RUN exits with KVM_EXIT_DIRTY_RING_FULL
before the ring overflows, but the pre-fault loop never checks it, so
one call over more such pages than the ring has free trips the WARN in
kvm_dirty_ring_push() and overwrites entries userspace hasn't
harvested. On x86:

  WARNING: arch/x86/kvm/../../../virt/kvm/dirty_ring.c:225 at kvm_dirty_ring_push+0x7a/0xd0
  Call Trace:
   make_spte+0x2cc/0x450
   kvm_tdp_mmu_map+0x5e1/0x7f0
   kvm_tdp_page_fault+0x12c/0x1b0
   kvm_mmu_do_page_fault+0x181/0x250
   kvm_arch_vcpu_pre_fault_memory+0x136/0x1d0
   kvm_vcpu_pre_fault_memory+0xcf/0x180
   kvm_vcpu_ioctl+0x455/0x770

Stop the loop once the ring is soft full, returning the pages already
processed, or -ENOSPC when there are none, since a retry can't succeed
until userspace harvests the ring. kvm_dirty_ring_check_request() fills
in a KVM_RUN exit, so test the ring's state directly. The stub for
!HAVE_KVM_DIRTY_RING is for s390, which has the ioctl but no ring.

Fixes: bc1a5cd002116 ("KVM: Add KVM_PRE_FAULT_MEMORY vcpu ioctl to pre-populate guest memory")
Cc: stable@vger.kernel.org
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 Documentation/virt/kvm/api.rst | 9 +++++++--
 include/linux/kvm_dirty_ring.h | 6 ++++++
 virt/kvm/dirty_ring.c          | 2 +-
 virt/kvm/kvm_main.c            | 6 ++++++
 4 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index e95c9cda085d9..89eb6be9bc6e3 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6544,6 +6544,7 @@ Errors:
   ENOENT     The specified `gpa` is outside defined memslots.
   ENOEXEC    The vCPU has not been initialised (arm64).
   EINTR      An unmasked signal is pending and no page was processed.
+  ENOSPC     The vCPU's dirty ring is full and no page was processed.
   EFAULT     The parameter address was invalid.
   EHWPOISON  A poisoned host page was encountered.
   EOPNOTSUPP Mapping memory for a GPA is unsupported by the
@@ -6588,7 +6589,9 @@ case, the ioctl can be called in parallel.
 
 When the ioctl returns, the input values are updated to point to the
 remaining range.  If `size` > 0 on return, the caller can just issue
-the ioctl again with the same `struct kvm_map_memory` argument.
+the ioctl again with the same `struct kvm_pre_fault_memory` argument.
+If that fails with ENOSPC, the vCPU's dirty ring has to be harvested
+and reset first.
 
 On x86, shadow page tables cannot support this ioctl because they
 are indexed by virtual address or nested guest physical address.
@@ -8957,7 +8960,9 @@ Therefore, the ioctl must be called *before* reading the content of
 the dirty pages.
 
 The dirty ring can get full.  When it happens, the KVM_RUN of the
-vcpu will return with exit reason KVM_EXIT_DIRTY_RING_FULL.
+vcpu will return with exit reason KVM_EXIT_DIRTY_RING_FULL, and
+KVM_PRE_FAULT_MEMORY on the vcpu will process no more pages until the
+ring has been harvested and reset.
 
 The dirty ring interface has a major difference comparing to the
 KVM_GET_DIRTY_LOG interface in that, when reading the dirty ring from
diff --git a/include/linux/kvm_dirty_ring.h b/include/linux/kvm_dirty_ring.h
index eb10d87adf7d5..88b4f2a009274 100644
--- a/include/linux/kvm_dirty_ring.h
+++ b/include/linux/kvm_dirty_ring.h
@@ -60,6 +60,11 @@ static inline void kvm_dirty_ring_push(struct kvm_vcpu *vcpu,
 {
 }
 
+static inline bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring)
+{
+	return false;
+}
+
 static inline struct page *kvm_dirty_ring_get_page(struct kvm_dirty_ring *ring,
 						   u32 offset)
 {
@@ -81,6 +86,7 @@ int kvm_dirty_ring_alloc(struct kvm *kvm, struct kvm_dirty_ring *ring,
 int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring,
 			 int *nr_entries_reset);
 void kvm_dirty_ring_push(struct kvm_vcpu *vcpu, u32 slot, u64 offset);
+bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring);
 
 bool kvm_dirty_ring_check_request(struct kvm_vcpu *vcpu);
 
diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c
index 572b854edf740..f73f92102cb7a 100644
--- a/virt/kvm/dirty_ring.c
+++ b/virt/kvm/dirty_ring.c
@@ -40,7 +40,7 @@ static u32 kvm_dirty_ring_used(struct kvm_dirty_ring *ring)
 	return READ_ONCE(ring->dirty_index) - READ_ONCE(ring->reset_index);
 }
 
-static bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring)
+bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring)
 {
 	return kvm_dirty_ring_used(ring) >= ring->soft_limit;
 }
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cf4cb586f1407..da1b76ceb65e3 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4461,6 +4461,12 @@ static int kvm_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu,
 			break;
 		}
 
+		/* Like KVM_RUN: only the iteration crossing the soft limit uses the reserve. */
+		if (vcpu->kvm->dirty_ring_size && kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) {
+			r = -ENOSPC;
+			break;
+		}
+
 		r = kvm_arch_vcpu_pre_fault_memory(vcpu, range);
 		if (WARN_ON_ONCE(r == 0 || r == -EIO))
 			break;
-- 
2.47.3

Re: [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full

[Lorenzo Stoakes (ARM)]

On Tue, Nov 10, 2026 at 11:07:23AM +0000, Fuad Tabba wrote:
> Pre-faulting can map a dirty-logged page writable, which pushes it to
> the vCPU's dirty ring. KVM_RUN exits with KVM_EXIT_DIRTY_RING_FULL
> before the ring overflows, but the pre-fault loop never checks it, so
> one call over more such pages than the ring has free trips the WARN in
> kvm_dirty_ring_push() and overwrites entries userspace hasn't
> harvested. On x86:
>
>   WARNING: arch/x86/kvm/../../../virt/kvm/dirty_ring.c:225 at kvm_dirty_ring_push+0x7a/0xd0
>   Call Trace:
>    make_spte+0x2cc/0x450
>    kvm_tdp_mmu_map+0x5e1/0x7f0
>    kvm_tdp_page_fault+0x12c/0x1b0
>    kvm_mmu_do_page_fault+0x181/0x250
>    kvm_arch_vcpu_pre_fault_memory+0x136/0x1d0
>    kvm_vcpu_pre_fault_memory+0xcf/0x180
>    kvm_vcpu_ioctl+0x455/0x770

If you have a local repro maybe worth sending a regression test for this?... and
I type that, then check the 2/2 and see that you already did :) ok haha thanks!

>
> Stop the loop once the ring is soft full, returning the pages already
> processed, or -ENOSPC when there are none, since a retry can't succeed
> until userspace harvests the ring. kvm_dirty_ring_check_request() fills
> in a KVM_RUN exit, so test the ring's state directly. The stub for
> !HAVE_KVM_DIRTY_RING is for s390, which has the ioctl but no ring.
>
> Fixes: bc1a5cd002116 ("KVM: Add KVM_PRE_FAULT_MEMORY vcpu ioctl to pre-populate guest memory")
> Cc: stable@vger.kernel.org
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>

Yikes, nasty. Thanks for addressing this!

LGTM so:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
>  Documentation/virt/kvm/api.rst | 9 +++++++--
>  include/linux/kvm_dirty_ring.h | 6 ++++++
>  virt/kvm/dirty_ring.c          | 2 +-
>  virt/kvm/kvm_main.c            | 6 ++++++
>  4 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index e95c9cda085d9..89eb6be9bc6e3 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6544,6 +6544,7 @@ Errors:
>    ENOENT     The specified `gpa` is outside defined memslots.
>    ENOEXEC    The vCPU has not been initialised (arm64).
>    EINTR      An unmasked signal is pending and no page was processed.
> +  ENOSPC     The vCPU's dirty ring is full and no page was processed.
>    EFAULT     The parameter address was invalid.
>    EHWPOISON  A poisoned host page was encountered.
>    EOPNOTSUPP Mapping memory for a GPA is unsupported by the
> @@ -6588,7 +6589,9 @@ case, the ioctl can be called in parallel.
>
>  When the ioctl returns, the input values are updated to point to the
>  remaining range.  If `size` > 0 on return, the caller can just issue
> -the ioctl again with the same `struct kvm_map_memory` argument.
> +the ioctl again with the same `struct kvm_pre_fault_memory` argument.
> +If that fails with ENOSPC, the vCPU's dirty ring has to be harvested
> +and reset first.
>
>  On x86, shadow page tables cannot support this ioctl because they
>  are indexed by virtual address or nested guest physical address.
> @@ -8957,7 +8960,9 @@ Therefore, the ioctl must be called *before* reading the content of
>  the dirty pages.
>
>  The dirty ring can get full.  When it happens, the KVM_RUN of the
> -vcpu will return with exit reason KVM_EXIT_DIRTY_RING_FULL.
> +vcpu will return with exit reason KVM_EXIT_DIRTY_RING_FULL, and
> +KVM_PRE_FAULT_MEMORY on the vcpu will process no more pages until the
> +ring has been harvested and reset.
>
>  The dirty ring interface has a major difference comparing to the
>  KVM_GET_DIRTY_LOG interface in that, when reading the dirty ring from
> diff --git a/include/linux/kvm_dirty_ring.h b/include/linux/kvm_dirty_ring.h
> index eb10d87adf7d5..88b4f2a009274 100644
> --- a/include/linux/kvm_dirty_ring.h
> +++ b/include/linux/kvm_dirty_ring.h
> @@ -60,6 +60,11 @@ static inline void kvm_dirty_ring_push(struct kvm_vcpu *vcpu,
>  {
>  }
>
> +static inline bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring)
> +{
> +	return false;
> +}
> +
>  static inline struct page *kvm_dirty_ring_get_page(struct kvm_dirty_ring *ring,
>  						   u32 offset)
>  {
> @@ -81,6 +86,7 @@ int kvm_dirty_ring_alloc(struct kvm *kvm, struct kvm_dirty_ring *ring,
>  int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring,
>  			 int *nr_entries_reset);
>  void kvm_dirty_ring_push(struct kvm_vcpu *vcpu, u32 slot, u64 offset);
> +bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring);
>
>  bool kvm_dirty_ring_check_request(struct kvm_vcpu *vcpu);
>
> diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c
> index 572b854edf740..f73f92102cb7a 100644
> --- a/virt/kvm/dirty_ring.c
> +++ b/virt/kvm/dirty_ring.c
> @@ -40,7 +40,7 @@ static u32 kvm_dirty_ring_used(struct kvm_dirty_ring *ring)
>  	return READ_ONCE(ring->dirty_index) - READ_ONCE(ring->reset_index);
>  }
>
> -static bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring)
> +bool kvm_dirty_ring_soft_full(struct kvm_dirty_ring *ring)
>  {
>  	return kvm_dirty_ring_used(ring) >= ring->soft_limit;
>  }
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index cf4cb586f1407..da1b76ceb65e3 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4461,6 +4461,12 @@ static int kvm_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu,
>  			break;
>  		}
>
> +		/* Like KVM_RUN: only the iteration crossing the soft limit uses the reserve. */
> +		if (vcpu->kvm->dirty_ring_size && kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) {
> +			r = -ENOSPC;
> +			break;
> +		}
> +
>  		r = kvm_arch_vcpu_pre_fault_memory(vcpu, range);
>  		if (WARN_ON_ONCE(r == 0 || r == -EIO))
>  			break;
> --
> 2.47.3
>

--
Cheers, Lorenzo

Re: [PATCH 1/2] KVM: Stop KVM_PRE_FAULT_MEMORY when the vCPU's dirty ring is full

[Sean Christopherson]

On Tue, Nov 10, 2026, Fuad Tabba wrote:
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index cf4cb586f1407..da1b76ceb65e3 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4461,6 +4461,12 @@ static int kvm_vcpu_pre_fault_memory(struct kvm_vcpu *vcpu,
>  			break;
>  		}
>  
> +		/* Like KVM_RUN: only the iteration crossing the soft limit uses the reserve. */

Please don't include AI comments verbatim.

> +		if (vcpu->kvm->dirty_ring_size && kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) {

Eh, I vote to just use kvm_dirty_ring_check_request(), I don't see any harm in
clobbering vcpu->run->exit_reason.

Side topic, kvm_dirty_ring_check_request() is rather confusing (clears and then
re-sets the requests).  AFAICT, it can be this instead:

	lockdep_assert_held(vcpu->mutex);

	/*
	 * The vCPU isn't runnable when the dirty ring becomes soft full.  Keep
	 * the requests set until the dirty pages are harvested and the dirty
	 * ring is reset by userspace.  Note, checking if the ring is soft-full
	 * can get a false positive, but not a false negative, because only the
	 * vCPU can add entries to the ring, and its mutex is held.
	 */
	if (kvm_test_request(KVM_REQ_DIRTY_RING_SOFT_FULL, vcpu) &&
	    kvm_dirty_ring_soft_full(&vcpu->dirty_ring)) {
		vcpu->run->exit_reason = KVM_EXIT_DIRTY_RING_FULL;
		trace_kvm_dirty_ring_exit(vcpu);
		return true;
	}

	kvm_clear_request(KVM_REQ_DIRTY_RING_SOFT_FULL, vcpu);
	return false;

> +			r = -ENOSPC;
> +			break;
> +		}
> +
>  		r = kvm_arch_vcpu_pre_fault_memory(vcpu, range);
>  		if (WARN_ON_ONCE(r == 0 || r == -EIO))
>  			break;
> -- 
> 2.47.3
> 

[PATCH 2/2] KVM: selftests: Test KVM_PRE_FAULT_MEMORY against a full dirty ring

[Fuad Tabba]

Pre-fault more populated, dirty-logged pages than the vCPU's dirty ring
holds, and check that each call stops before the ring overflows, that a
retry returns ENOSPC without progress, and that the harvested entries
are the pages in order. A read pre-fault can map a page read-only, or
access-tracked on x86 without EPT A/D bits, so skip the case if
pre-faulting dirties nothing.

Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 .../selftests/kvm/pre_fault_memory_test.c     | 85 +++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/tools/testing/selftests/kvm/pre_fault_memory_test.c b/tools/testing/selftests/kvm/pre_fault_memory_test.c
index 3082fe09b95fa..18fc1d69acbf0 100644
--- a/tools/testing/selftests/kvm/pre_fault_memory_test.c
+++ b/tools/testing/selftests/kvm/pre_fault_memory_test.c
@@ -5,6 +5,7 @@
  * Author:
  * Isaku Yamahata <isaku.yamahata at gmail.com>
  */
+#include <asm/barrier.h>
 #include <linux/sizes.h>
 
 #include <test_util.h>
@@ -17,6 +18,8 @@
 /* Arbitrarily chosen values */
 #define TEST_BASE_SIZE		SZ_2M
 #define TEST_SLOT		10
+/* Large enough for the entries every architecture reserves. */
+#define TEST_RING_SIZE		SZ_64K
 
 /* Storage of test info to share with guest code */
 struct test_config {
@@ -281,6 +284,87 @@ static void test_pre_fault_memory(unsigned long vm_type, enum vm_mem_backing_src
 	for_each_guest_mode(__test_pre_fault_memory, &p);
 }
 
+/*
+ * Pre-fault more dirty-logged pages than the ring holds: each call has to stop
+ * before the ring overflows, and no entry may be lost.
+ */
+static void test_pre_fault_dirty_ring(void)
+{
+	const u64 entries = TEST_RING_SIZE / sizeof(struct kvm_dirty_gfn);
+	const u64 npages = entries + entries / 4;
+	struct kvm_pre_fault_memory range, left;
+	struct kvm_dirty_gfn *gfns, *cur;
+	u64 harvested = 0, n, size;
+	struct kvm_vcpu *vcpu;
+	int ret, stops = 0;
+	struct kvm_vm *vm;
+	u32 fetch = 0;
+	gpa_t gpa;
+
+	if (!kvm_has_cap(KVM_CAP_DIRTY_LOG_RING) && !kvm_has_cap(KVM_CAP_DIRTY_LOG_RING_ACQ_REL)) {
+		pr_info("Skipping the dirty ring test: no dirty ring\n");
+		return;
+	}
+
+	vm = vm_create(1);
+	vm_enable_dirty_ring(vm, TEST_RING_SIZE);
+	vcpu = vm_vcpu_add(vm, 0, NULL);
+	gfns = vcpu_map_dirty_ring(vcpu);
+
+	size = npages * vm->page_size;
+	gpa = align_down(vm->max_gfn * vm->page_size - size, SZ_2M);
+	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, gpa, TEST_SLOT, npages,
+				    KVM_MEM_LOG_DIRTY_PAGES);
+
+	/* Populate the pages, so that pre-faulting can map them writable. */
+	memset(addr_gpa2hva(vm, gpa), 1, size);
+
+	range = (struct kvm_pre_fault_memory) { .gpa = gpa, .size = size };
+	while (range.size) {
+		vcpu_ioctl(vcpu, KVM_PRE_FAULT_MEMORY, &range);
+
+		if (range.size) {
+			left = range;
+			ret = __vcpu_ioctl(vcpu, KVM_PRE_FAULT_MEMORY, &range);
+			TEST_ASSERT_VM_VCPU_IOCTL(ret == -1 && errno == ENOSPC,
+						  KVM_PRE_FAULT_MEMORY, ret, vm);
+			TEST_ASSERT(range.gpa == left.gpa && range.size == left.size,
+				    "KVM_PRE_FAULT_MEMORY made progress on a full ring");
+			stops++;
+		}
+
+		for (n = 0; ; n++) {
+			cur = &gfns[fetch % entries];
+			if (smp_load_acquire(&cur->flags) != KVM_DIRTY_GFN_F_DIRTY)
+				break;
+
+			TEST_ASSERT(cur->slot == TEST_SLOT && cur->offset == harvested + n,
+				    "Ring entry %u: slot %u offset 0x%llx, expected slot %u offset 0x%lx",
+				    fetch, cur->slot, cur->offset, TEST_SLOT, harvested + n);
+			smp_store_release(&cur->flags, KVM_DIRTY_GFN_F_RESET);
+			fetch++;
+		}
+
+		/* A read pre-fault can map a page read-only, or access-tracked on x86. */
+		if (!n && !harvested && !range.size) {
+			pr_info("Skipping the dirty ring test: pre-faulting dirtied no pages\n");
+			kvm_vm_free(vm);
+			return;
+		}
+		TEST_ASSERT(n, "KVM_PRE_FAULT_MEMORY pushed nothing to the ring");
+		TEST_ASSERT(kvm_vm_reset_dirty_ring(vm) == n,
+			    "KVM_RESET_DIRTY_RINGS didn't reset %lu entries", n);
+		harvested += n;
+	}
+
+	TEST_ASSERT(harvested == npages, "%lu ring entries for %lu pages", harvested, npages);
+	TEST_ASSERT(stops, "KVM_PRE_FAULT_MEMORY never stopped for a %lu-entry ring", entries);
+	pr_info("Pre-faulted %lu pages through a %lu-entry ring, stopping %d times\n",
+		npages, entries, stops);
+
+	kvm_vm_free(vm);
+}
+
 static void help(char *name)
 {
 	puts("");
@@ -320,5 +404,6 @@ int main(int argc, char *argv[])
 	test_pre_fault_memory(KVM_X86_SW_PROTECTED_VM, backing, false);
 	test_pre_fault_memory(KVM_X86_SW_PROTECTED_VM, backing, true);
 #endif
+	test_pre_fault_dirty_ring();
 	return 0;
 }
-- 
2.47.3

Re: [PATCH 2/2] KVM: selftests: Test KVM_PRE_FAULT_MEMORY against a full dirty ring

[Lorenzo Stoakes (ARM)]

On Tue, Nov 10, 2026 at 11:07:24AM +0000, Fuad Tabba wrote:
> Pre-fault more populated, dirty-logged pages than the vCPU's dirty ring
> holds, and check that each call stops before the ring overflows, that a
> retry returns ENOSPC without progress, and that the harvested entries
> are the pages in order. A read pre-fault can map a page read-only, or

Are we asserting an invariant that prefaulting happens in address order
here?  Is that explicitly presumed in the prefault API, or just how it
happens to pan out?

I wonder if we might decide for whatever reason in future to do things
differently there. Bit nitty I know, but wonder if it's asserting an
implementation detail?

> access-tracked on x86 without EPT A/D bits, so skip the case if
> pre-faulting dirties nothing.
>
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---
>  .../selftests/kvm/pre_fault_memory_test.c     | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/pre_fault_memory_test.c b/tools/testing/selftests/kvm/pre_fault_memory_test.c
> index 3082fe09b95fa..18fc1d69acbf0 100644
> --- a/tools/testing/selftests/kvm/pre_fault_memory_test.c
> +++ b/tools/testing/selftests/kvm/pre_fault_memory_test.c
> @@ -5,6 +5,7 @@
>   * Author:
>   * Isaku Yamahata <isaku.yamahata at gmail.com>
>   */
> +#include <asm/barrier.h>
>  #include <linux/sizes.h>
>
>  #include <test_util.h>
> @@ -17,6 +18,8 @@
>  /* Arbitrarily chosen values */
>  #define TEST_BASE_SIZE		SZ_2M
>  #define TEST_SLOT		10
> +/* Large enough for the entries every architecture reserves. */
> +#define TEST_RING_SIZE		SZ_64K
>
>  /* Storage of test info to share with guest code */
>  struct test_config {
> @@ -281,6 +284,87 @@ static void test_pre_fault_memory(unsigned long vm_type, enum vm_mem_backing_src
>  	for_each_guest_mode(__test_pre_fault_memory, &p);
>  }
>
> +/*
> + * Pre-fault more dirty-logged pages than the ring holds: each call has to stop
> + * before the ring overflows, and no entry may be lost.
> + */
> +static void test_pre_fault_dirty_ring(void)
> +{
> +	const u64 entries = TEST_RING_SIZE / sizeof(struct kvm_dirty_gfn);
> +	const u64 npages = entries + entries / 4;
> +	struct kvm_pre_fault_memory range, left;
> +	struct kvm_dirty_gfn *gfns, *cur;
> +	u64 harvested = 0, n, size;
> +	struct kvm_vcpu *vcpu;
> +	int ret, stops = 0;
> +	struct kvm_vm *vm;
> +	u32 fetch = 0;
> +	gpa_t gpa;
> +
> +	if (!kvm_has_cap(KVM_CAP_DIRTY_LOG_RING) && !kvm_has_cap(KVM_CAP_DIRTY_LOG_RING_ACQ_REL)) {
> +		pr_info("Skipping the dirty ring test: no dirty ring\n");
> +		return;
> +	}
> +
> +	vm = vm_create(1);
> +	vm_enable_dirty_ring(vm, TEST_RING_SIZE);
> +	vcpu = vm_vcpu_add(vm, 0, NULL);
> +	gfns = vcpu_map_dirty_ring(vcpu);
> +
> +	size = npages * vm->page_size;
> +	gpa = align_down(vm->max_gfn * vm->page_size - size, SZ_2M);
> +	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, gpa, TEST_SLOT, npages,
> +				    KVM_MEM_LOG_DIRTY_PAGES);
> +
> +	/* Populate the pages, so that pre-faulting can map them writable. */

Reviewed below, but I wonder how x86 could then read fault these?

> +	memset(addr_gpa2hva(vm, gpa), 1, size);
> +
> +	range = (struct kvm_pre_fault_memory) { .gpa = gpa, .size = size };
> +	while (range.size) {

NIT: n could be scoped to this block instead.

> +		vcpu_ioctl(vcpu, KVM_PRE_FAULT_MEMORY, &range);
> +
> +		if (range.size) {
> +			left = range;
> +			ret = __vcpu_ioctl(vcpu, KVM_PRE_FAULT_MEMORY, &range);
> +			TEST_ASSERT_VM_VCPU_IOCTL(ret == -1 && errno == ENOSPC,
> +						  KVM_PRE_FAULT_MEMORY, ret, vm);
> +			TEST_ASSERT(range.gpa == left.gpa && range.size == left.size,
> +				    "KVM_PRE_FAULT_MEMORY made progress on a full ring");
> +			stops++;
> +		}
> +
> +		for (n = 0; ; n++) {

Could this loop forever if cur->flags always == KVM_DIRTY_GFN_F_DIRTY? Or
is that guaranteed not to be the case?

> +			cur = &gfns[fetch % entries];
> +			if (smp_load_acquire(&cur->flags) != KVM_DIRTY_GFN_F_DIRTY)

Is this and the below release paired with equivalent stuff in the dirty ring code?

> +				break;
> +
> +			TEST_ASSERT(cur->slot == TEST_SLOT && cur->offset == harvested + n,
> +				    "Ring entry %u: slot %u offset 0x%llx, expected slot %u offset 0x%lx",
> +				    fetch, cur->slot, cur->offset, TEST_SLOT, harvested + n);
> +			smp_store_release(&cur->flags, KVM_DIRTY_GFN_F_RESET);
> +			fetch++;
> +		}
> +
> +		/* A read pre-fault can map a page read-only, or access-tracked on x86. */

Hmm under what circumstances would that happen? Is there no way to ensure pages
are dirtied by prefaulting? I imagine not though!

> +		if (!n && !harvested && !range.size) {

Hmm, so !n && (harvested || range_size) will pass this, but then you assert
n immediately below, so it seems like n is an unconditional requirement.

So I wonder whether this should just be !n in this conditional or maybe the
assert afterwards should be dropped?

> +			pr_info("Skipping the dirty ring test: pre-faulting dirtied no pages\n");
> +			kvm_vm_free(vm);
> +			return;
> +		}
> +		TEST_ASSERT(n, "KVM_PRE_FAULT_MEMORY pushed nothing to the ring");
> +		TEST_ASSERT(kvm_vm_reset_dirty_ring(vm) == n,
> +			    "KVM_RESET_DIRTY_RINGS didn't reset %lu entries", n);
> +		harvested += n;
> +	}
> +
> +	TEST_ASSERT(harvested == npages, "%lu ring entries for %lu pages", harvested, npages);
> +	TEST_ASSERT(stops, "KVM_PRE_FAULT_MEMORY never stopped for a %lu-entry ring", entries);
> +	pr_info("Pre-faulted %lu pages through a %lu-entry ring, stopping %d times\n",
> +		npages, entries, stops);
> +
> +	kvm_vm_free(vm);
> +}
> +
>  static void help(char *name)
>  {
>  	puts("");
> @@ -320,5 +404,6 @@ int main(int argc, char *argv[])
>  	test_pre_fault_memory(KVM_X86_SW_PROTECTED_VM, backing, false);
>  	test_pre_fault_memory(KVM_X86_SW_PROTECTED_VM, backing, true);
>  #endif
> +	test_pre_fault_dirty_ring();
>  	return 0;
>  }
> --
> 2.47.3
>

--
Cheers, Lorenzo

Re: [PATCH 2/2] KVM: selftests: Test KVM_PRE_FAULT_MEMORY against a full dirty ring

[Fuad Tabba]

Hi Lorenzo,

On Tue, 10 Nov 2026 14:38:27 +0000, "Lorenzo Stoakes (ARM)"
<ljs@kernel.org> wrote:
> On Tue, Nov 10, 2026 at 11:07:24AM +0000, Fuad Tabba wrote:
> > Pre-fault more populated, dirty-logged pages than the vCPU's dirty ring
> > holds, and check that each call stops before the ring overflows, that a
> > retry returns ENOSPC without progress, and that the harvested entries
> > are the pages in order. A read pre-fault can map a page read-only, or
>
> Are we asserting an invariant that prefaulting happens in address order
> here?  Is that explicitly presumed in the prefault API, or just how it
> happens to pan out?
>
> I wonder if we might decide for whatever reason in future to do things
> differently there. Bit nitty I know, but wonder if it's asserting an
> implementation detail?

Yes, the API doesn't guarantee an order. I'll assert that every page
is reported exactly once instead.

[...]
> > diff --git a/tools/testing/selftests/kvm/pre_fault_memory_test.c b/tools/testing/selftests/kvm/pre_fault_memory_test.c
[...]
> > +static void test_pre_fault_dirty_ring(void)
[...]
> > +    /* Populate the pages, so that pre-faulting can map them writable. */
>
> Reviewed below, but I wonder how x86 could then read fault these?

KVM_PRE_FAULT_MEMORY is defined as a read fault, but x86 maps the page
writable when the host page is, and marks it dirty, unless it falls
back to access tracking (below). Populating the pages makes sure they
aren't backed by the zero page.

[...]
> > +    while (range.size) {
>
> NIT: n could be scoped to this block instead.

Will do.

[...]
> > +        for (n = 0; ; n++) {
>
> Could this loop forever if cur->flags always == KVM_DIRTY_GFN_F_DIRTY? Or
> is that guaranteed not to be the case?

It can't. Every harvested entry is set to KVM_DIRTY_GFN_F_RESET and
stays that way until KVM_RESET_DIRTY_RINGS, which runs after the loop,
so one pass stops after at most a full ring.

> > +            cur = &gfns[fetch % entries];
> > +            if (smp_load_acquire(&cur->flags) != KVM_DIRTY_GFN_F_DIRTY)
>
> Is this and the below release paired with equivalent stuff in the dirty ring code?

Yes, the acquire pairs with the smp_wmb() in kvm_dirty_ring_push()
ahead of the flag store, and the release with the smp_load_acquire()
in kvm_dirty_gfn_harvested(). dirty_log_test does the same.

[...]
> > +        /* A read pre-fault can map a page read-only, or access-tracked on x86. */
>
> Hmm under what circumstances would that happen? Is there no way to ensure pages
> are dirtied by prefaulting? I imagine not though!

No, it's up to the architecture. arm64 maps a read fault read-only
while the slot is logged with a bitmap, and x86 with EPT but no A/D
bits maps it access-tracked. Neither pushes anything to the ring.

> > +        if (!n && !harvested && !range.size) {
>
> Hmm, so !n && (harvested || range_size) will pass this, but then you assert
> n immediately below, so it seems like n is an unconditional requirement.
>
> So I wonder whether this should just be !n in this conditional or maybe the
> assert afterwards should be dropped?

The skip is for when pre-faulting dirties nothing at all: the first
call maps the whole range without pushing a single entry, so there's
nothing to test. Any other empty harvest is a bug, which the assert
reports, so I'd rather keep both as they are.

Cheers,
/fuad

Re: [PATCH 2/2] KVM: selftests: Test KVM_PRE_FAULT_MEMORY against a full dirty ring

[Sean Christopherson]

On Tue, Nov 10, 2026, Fuad Tabba wrote:
> Hi Lorenzo,
> 
> On Tue, 10 Nov 2026 14:38:27 +0000, "Lorenzo Stoakes (ARM)"
> <ljs@kernel.org> wrote:
> > On Tue, Nov 10, 2026 at 11:07:24AM +0000, Fuad Tabba wrote:
> > > Pre-fault more populated, dirty-logged pages than the vCPU's dirty ring
> > > holds, and check that each call stops before the ring overflows, that a
> > > retry returns ENOSPC without progress, and that the harvested entries
> > > are the pages in order. A read pre-fault can map a page read-only, or
> >
> > Are we asserting an invariant that prefaulting happens in address order
> > here?  Is that explicitly presumed in the prefault API, or just how it
> > happens to pan out?
> >
> > I wonder if we might decide for whatever reason in future to do things
> > differently there. Bit nitty I know, but wonder if it's asserting an
> > implementation detail?
> 
> Yes, the API doesn't guarantee an order. I'll assert that every page
> is reported exactly once instead.
> 
> [...]
> > > diff --git a/tools/testing/selftests/kvm/pre_fault_memory_test.c b/tools/testing/selftests/kvm/pre_fault_memory_test.c
> [...]
> > > +static void test_pre_fault_dirty_ring(void)
> [...]
> > > +    /* Populate the pages, so that pre-faulting can map them writable. */
> >
> > Reviewed below, but I wonder how x86 could then read fault these?
> 
> KVM_PRE_FAULT_MEMORY is defined as a read fault, but x86 maps the page
> writable when the host page is, and marks it dirty, unless it falls
> back to access tracking (below). Populating the pages makes sure they
> aren't backed by the zero page.

Before we effectively solidify the behavior, I think we should have an explicit
discussion about the desired behavior of KVM_PRE_FAULT_MEMORY when dirty logging
is enabled.  E.g. the only reason I kept the existing behavior in my auto-prefault
series was because I couldn't think of a good reason to change it.  But we never
made a concious decision in the first place, i.e. we never considered what would
happen if prefaulting collided with dirty logging (as evidenced by this bug).

Re: [PATCH 2/2] KVM: selftests: Test KVM_PRE_FAULT_MEMORY against a full dirty ring

[Lorenzo Stoakes (ARM)]

On Tue, Nov 10, 2026 at 07:53:35AM -0800, Sean Christopherson wrote:
> On Tue, Nov 10, 2026, Fuad Tabba wrote:
> > Hi Lorenzo,
> >
> > On Tue, 10 Nov 2026 14:38:27 +0000, "Lorenzo Stoakes (ARM)"
> > <ljs@kernel.org> wrote:
> > > On Tue, Nov 10, 2026 at 11:07:24AM +0000, Fuad Tabba wrote:
> > > > Pre-fault more populated, dirty-logged pages than the vCPU's dirty ring
> > > > holds, and check that each call stops before the ring overflows, that a
> > > > retry returns ENOSPC without progress, and that the harvested entries
> > > > are the pages in order. A read pre-fault can map a page read-only, or
> > >
> > > Are we asserting an invariant that prefaulting happens in address order
> > > here?  Is that explicitly presumed in the prefault API, or just how it
> > > happens to pan out?
> > >
> > > I wonder if we might decide for whatever reason in future to do things
> > > differently there. Bit nitty I know, but wonder if it's asserting an
> > > implementation detail?
> >
> > Yes, the API doesn't guarantee an order. I'll assert that every page
> > is reported exactly once instead.
> >
> > [...]
> > > > diff --git a/tools/testing/selftests/kvm/pre_fault_memory_test.c b/tools/testing/selftests/kvm/pre_fault_memory_test.c
> > [...]
> > > > +static void test_pre_fault_dirty_ring(void)
> > [...]
> > > > +    /* Populate the pages, so that pre-faulting can map them writable. */
> > >
> > > Reviewed below, but I wonder how x86 could then read fault these?
> >
> > KVM_PRE_FAULT_MEMORY is defined as a read fault, but x86 maps the page
> > writable when the host page is, and marks it dirty, unless it falls
> > back to access tracking (below). Populating the pages makes sure they
> > aren't backed by the zero page.
>
> Before we effectively solidify the behavior, I think we should have an explicit
> discussion about the desired behavior of KVM_PRE_FAULT_MEMORY when dirty logging
> is enabled.  E.g. the only reason I kept the existing behavior in my auto-prefault
> series was because I couldn't think of a good reason to change it.  But we never
> made a concious decision in the first place, i.e. we never considered what would
> happen if prefaulting collided with dirty logging (as evidenced by this bug).

Yeah, I do worry about implicit assumptions (also commented re: ordering).

I guess what makes sense, given the general approach for prefaulting = injecting
a synthetic read fault, is for the expectation to be that it matches the
behaviour of an ordinary read fault.

Or do you feel that it makes sense for the pre-faulting to do something special
in these cases and establish a sensible invariant?

If the pre-faulting does indeed dirty the pages, you'd need the dirty logging to
do its usual thing (not familiar with the code, so I'm guessing logging the page
:) though right?

--
Cheers, Lorenzo

Re: [PATCH 2/2] KVM: selftests: Test KVM_PRE_FAULT_MEMORY against a full dirty ring

[Lorenzo Stoakes (ARM)]

On Tue, Nov 10, 2026 at 03:37:05PM +0000, Fuad Tabba wrote:
> Hi Lorenzo,
>
> On Tue, 10 Nov 2026 14:38:27 +0000, "Lorenzo Stoakes (ARM)"
> <ljs@kernel.org> wrote:
> > On Tue, Nov 10, 2026 at 11:07:24AM +0000, Fuad Tabba wrote:
> > > Pre-fault more populated, dirty-logged pages than the vCPU's dirty ring
> > > holds, and check that each call stops before the ring overflows, that a
> > > retry returns ENOSPC without progress, and that the harvested entries
> > > are the pages in order. A read pre-fault can map a page read-only, or
> >
> > Are we asserting an invariant that prefaulting happens in address order
> > here?  Is that explicitly presumed in the prefault API, or just how it
> > happens to pan out?
> >
> > I wonder if we might decide for whatever reason in future to do things
> > differently there. Bit nitty I know, but wonder if it's asserting an
> > implementation detail?
>
> Yes, the API doesn't guarantee an order. I'll assert that every page
> is reported exactly once instead.

THanks!

>
> [...]
> > > diff --git a/tools/testing/selftests/kvm/pre_fault_memory_test.c b/tools/testing/selftests/kvm/pre_fault_memory_test.c
> [...]
> > > +static void test_pre_fault_dirty_ring(void)
> [...]
> > > +    /* Populate the pages, so that pre-faulting can map them writable. */
> >
> > Reviewed below, but I wonder how x86 could then read fault these?
>
> KVM_PRE_FAULT_MEMORY is defined as a read fault, but x86 maps the page
> writable when the host page is, and marks it dirty, unless it falls
> back to access tracking (below). Populating the pages makes sure they
> aren't backed by the zero page.

Ack.

>
> [...]
> > > +    while (range.size) {
> >
> > NIT: n could be scoped to this block instead.
>
> Will do.

Thanks!

>
> [...]
> > > +        for (n = 0; ; n++) {
> >
> > Could this loop forever if cur->flags always == KVM_DIRTY_GFN_F_DIRTY? Or
> > is that guaranteed not to be the case?
>
> It can't. Every harvested entry is set to KVM_DIRTY_GFN_F_RESET and
> stays that way until KVM_RESET_DIRTY_RINGS, which runs after the loop,
> so one pass stops after at most a full ring.

Ack OK.

>
> > > +            cur = &gfns[fetch % entries];
> > > +            if (smp_load_acquire(&cur->flags) != KVM_DIRTY_GFN_F_DIRTY)
> >
> > Is this and the below release paired with equivalent stuff in the dirty ring code?
>
> Yes, the acquire pairs with the smp_wmb() in kvm_dirty_ring_push()
> ahead of the flag store, and the release with the smp_load_acquire()
> in kvm_dirty_gfn_harvested(). dirty_log_test does the same.

I know it's test code, but general kernel practice is to add a comment saying
what memory barriers pair with so it's easier to track, could you add those for
these?

>
> [...]
> > > +        /* A read pre-fault can map a page read-only, or access-tracked on x86. */
> >
> > Hmm under what circumstances would that happen? Is there no way to ensure pages
> > are dirtied by prefaulting? I imagine not though!
>
> No, it's up to the architecture. arm64 maps a read fault read-only
> while the slot is logged with a bitmap, and x86 with EPT but no A/D
> bits maps it access-tracked. Neither pushes anything to the ring.

Right, OK, makes sense.

>
> > > +        if (!n && !harvested && !range.size) {
> >
> > Hmm, so !n && (harvested || range_size) will pass this, but then you assert
> > n immediately below, so it seems like n is an unconditional requirement.
> >
> > So I wonder whether this should just be !n in this conditional or maybe the
> > assert afterwards should be dropped?
>
> The skip is for when pre-faulting dirties nothing at all: the first
> call maps the whole range without pushing a single entry, so there's
> nothing to test. Any other empty harvest is a bug, which the assert
> reports, so I'd rather keep both as they are.

OK I see. Really the assert below is !n && (harvested || range.size), maybe
clearer to be explicit but it's also a bit noisy so not a big deal.

--
Cheers, Lorenzo