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@xxxxxxxxx>
> ---
>  .../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