mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v1 0/3] KVM: selftests: Add support for dirty-ring on dirty_log_perf_test
@ 2026-09-29 11:37 Leonardo Bras
  2026-09-29 11:37 ` [PATCH v1 1/3] KVM: selftests: memstress: Add option to enable dirty-ring on VM creation Leonardo Bras
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Leonardo Bras @ 2026-09-29 11:37 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson, Shuah Khan, David Matlack,
	Leonardo Bras, Ackerley Tng, Marc Zyngier, Josh Hilke,
	Oliver Upton, Wu Fei, Steffen Eiden, Claudio Imbrenda
  Cc: kvm, linux-kselftest, linux-kernel

Add support to dirty-ring on dirty_log_perf_test, so we can measure
performance differences when we change / improve the mechanism.

One big change in this set is that dirty-ring needs to be enabled 
after the VM was created, but before any cpu is created, so for that,
I made a few changes on memstress so it can actually take dirty-ring
size as a parameter for VM creation. (Patch #1)
I honestly don't think it's pretty, but I could not think on a
better way of doing this without messing too much in the code.

Also added some checks in dirty-ring enable, so an error message on
ring size is more informative.

Please provide feedback :)

Thanks!
Leo

Changes since RFCv4:
- Series title 
- Rebase on v7.3-rc5
Link: https://lore.kernel.org/all/20260729161519.3778839-2-leo.bras@arm.com/

- Using acquire/release barriers to make sure there is no reorder between
  incrementing iteration and summing the time spent on cleaning.
- Improved ring-size checks, although not perfect due to reserved entries
  affecting minimum size.
- Do not allocate bitmaps for dirty-ring tests, as they are unused.
- Improve help text
- Make sure ret==0 before going back to vcpu_run in dirty-ring case
Link: https://lore.kernel.org/all/20260708152232.2568907-1-leo.bras@arm.com/

Changes since RFCv2:
- Added mutex to make sure only a single vcpu is 
  collecting/cleaning at a time. This gets us the ability to
  test return values, and not account for kvm.slot_lock waiting time.
- Stuff reported by Sashiko
  - Return value always positive, so assert was pointless
  - Concurrency inflating the time for dirty-ring cleaning
Link: https://lore.kernel.org/all/20260629105950.1790259-1-leo.bras@arm.com/

Changes since RFCv1:
- Stuff reported by Sashiko
  - Fixed elements / byte size wrapping issue
  - Removed element count due to concurrency
  - Testing exit_reason instead of ioctl return value
  - Some nits
Link: https://lore.kernel.org/all/20260624171656.1737580-1-leo.bras@arm.com/

Leonardo Bras (3):
  KVM: selftests: memstress: Add option to enable dirty-ring on VM
    creation
  KVM: selftests: Check dirty-ring size before enabling
  KVM: selftests: dirty_log_perf_test: Add dirty-ring support

 .../testing/selftests/kvm/include/memstress.h |   3 +-
 .../selftests/kvm/access_tracking_perf_test.c |   2 +-
 .../selftests/kvm/demand_paging_test.c        |   2 +-
 .../selftests/kvm/dirty_log_perf_test.c       | 128 ++++++++++++++++--
 tools/testing/selftests/kvm/lib/kvm_util.c    |  21 ++-
 tools/testing/selftests/kvm/lib/memstress.c   |  34 ++++-
 .../kvm/memslot_modification_stress_test.c    |   2 +-
 .../kvm/x86/dirty_log_page_splitting_test.c   |   2 +-
 8 files changed, 167 insertions(+), 27 deletions(-)


base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
-- 
2.55.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v1 1/3] KVM: selftests: memstress: Add option to enable dirty-ring on VM creation
  2026-09-29 11:37 [PATCH v1 0/3] KVM: selftests: Add support for dirty-ring on dirty_log_perf_test Leonardo Bras
@ 2026-09-29 11:37 ` Leonardo Bras
  2026-09-30 18:21   ` Sean Christopherson
  2026-09-29 11:37 ` [PATCH v1 2/3] KVM: selftests: Check dirty-ring size before enabling Leonardo Bras
  2026-09-29 11:37 ` [PATCH v1 3/3] KVM: selftests: dirty_log_perf_test: Add dirty-ring support Leonardo Bras
  2 siblings, 1 reply; 8+ messages in thread
From: Leonardo Bras @ 2026-09-29 11:37 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson, Shuah Khan, David Matlack,
	Leonardo Bras, Ackerley Tng, Marc Zyngier, Josh Hilke,
	Oliver Upton, Wu Fei, Steffen Eiden, Claudio Imbrenda
  Cc: kvm, linux-kselftest, linux-kernel

Dirty-ring should be enabled after the VM is created, but before the
creation of any cpu. To to so, add an option on memstress_create_vm() that
takes dirty-ring size, and enables it at the correct moment.

This required a new function memstress_vm_create_with_vcpus() to be
created based on the previously used __vm_create_with_vcpus();

Signed-off-by: Leonardo Bras <leo.bras@arm.com>
---
 .../testing/selftests/kvm/include/memstress.h |  3 +-
 .../selftests/kvm/access_tracking_perf_test.c |  2 +-
 .../selftests/kvm/demand_paging_test.c        |  2 +-
 .../selftests/kvm/dirty_log_perf_test.c       |  2 +-
 tools/testing/selftests/kvm/lib/memstress.c   | 34 ++++++++++++++++---
 .../kvm/memslot_modification_stress_test.c    |  2 +-
 .../kvm/x86/dirty_log_page_splitting_test.c   |  2 +-
 7 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/memstress.h b/tools/testing/selftests/kvm/include/memstress.h
index 0d1d6230cc05..e17f344978af 100644
--- a/tools/testing/selftests/kvm/include/memstress.h
+++ b/tools/testing/selftests/kvm/include/memstress.h
@@ -51,21 +51,22 @@ struct memstress_args {
  	bool stop_vcpus;
 
 	struct memstress_vcpu_args vcpu_args[KVM_MAX_VCPUS];
 };
 
 extern struct memstress_args memstress_args;
 
 struct kvm_vm *memstress_create_vm(enum vm_guest_mode mode, int nr_vcpus,
 				   u64 vcpu_memory_bytes, int slots,
 				   enum vm_mem_backing_src_type backing_src,
-				   bool partition_vcpu_memory_access);
+				   bool partition_vcpu_memory_access,
+				   u32 dirty_ring_size);
 void memstress_destroy_vm(struct kvm_vm *vm);
 
 void memstress_set_write_percent(struct kvm_vm *vm, u32 write_percent);
 void memstress_set_random_access(struct kvm_vm *vm, bool random_access);
 
 void memstress_start_vcpu_threads(int vcpus, void (*vcpu_fn)(struct memstress_vcpu_args *));
 void memstress_join_vcpu_threads(int vcpus);
 void memstress_guest_code(u32 vcpu_id);
 
 u64 memstress_nested_pages(int nr_vcpus);
diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
index 4415c94b2866..f78d56265d89 100644
--- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
+++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
@@ -402,21 +402,21 @@ static void mark_memory_idle(struct kvm_vm *vm, int nr_vcpus)
 	run_iteration(vm, nr_vcpus, "Mark memory idle (page_idle)");
 }
 
 static void run_test(enum vm_guest_mode mode, void *arg)
 {
 	struct test_params *params = arg;
 	struct kvm_vm *vm;
 	int nr_vcpus = params->nr_vcpus;
 
 	vm = memstress_create_vm(mode, nr_vcpus, params->vcpu_memory_bytes, 1,
-				 params->backing_src, !overlap_memory_access);
+				 params->backing_src, !overlap_memory_access, 0);
 
 	/*
 	 * If guest_page_size is larger than the host's page size, the
 	 * guest (memstress) will only fault in a subset of the host's pages.
 	 */
 	test_pages = params->nr_vcpus * params->vcpu_memory_bytes /
 		      max(memstress_args.guest_page_size,
 			  (u64)getpagesize());
 
 	memstress_start_vcpu_threads(nr_vcpus, vcpu_thread_main);
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index f8b3d0b68830..3ec00de73553 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -155,21 +155,21 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	struct test_params *p = arg;
 	struct uffd_desc **uffd_descs = NULL;
 	u64 uffd_region_size;
 	struct timespec start;
 	struct timespec ts_diff;
 	double vcpu_paging_rate;
 	struct kvm_vm *vm;
 	int i, num_uffds = 0;
 
 	vm = memstress_create_vm(mode, nr_vcpus, guest_percpu_mem_size, 1,
-				 p->src_type, p->partition_vcpu_memory_access);
+				 p->src_type, p->partition_vcpu_memory_access, 0);
 
 	demand_paging_size = get_backing_src_pagesz(p->src_type);
 
 	guest_data_prototype = malloc(demand_paging_size);
 	TEST_ASSERT(guest_data_prototype,
 		    "Failed to allocate buffer for guest data pattern");
 	memset(guest_data_prototype, 0xAB, demand_paging_size);
 
 	if (p->uffd_mode == UFFDIO_REGISTER_MODE_MINOR) {
 		num_uffds = p->single_uffd ? 1 : nr_vcpus;
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 7c5abe1ae9e0..8f791ad7b86a 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -112,21 +112,21 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	struct timespec start;
 	struct timespec ts_diff;
 	struct timespec get_dirty_log_total = (struct timespec){0};
 	struct timespec vcpu_dirty_total = (struct timespec){0};
 	struct timespec avg;
 	struct timespec clear_dirty_log_total = (struct timespec){0};
 	int i;
 
 	vm = memstress_create_vm(mode, nr_vcpus, guest_percpu_mem_size,
 				 p->slots, p->backing_src,
-				 p->partition_vcpu_memory_access);
+				 p->partition_vcpu_memory_access, 0);
 
 	memstress_set_write_percent(vm, p->write_percent);
 
 	guest_num_pages = (nr_vcpus * guest_percpu_mem_size) >> vm->page_shift;
 	guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
 	host_num_pages = vm_num_host_pages(mode, guest_num_pages);
 	pages_per_slot = host_num_pages / p->slots;
 
 	bitmaps = memstress_alloc_bitmaps(p->slots, pages_per_slot);
 
diff --git a/tools/testing/selftests/kvm/lib/memstress.c b/tools/testing/selftests/kvm/lib/memstress.c
index 2864562f57b4..4632449a5875 100644
--- a/tools/testing/selftests/kvm/lib/memstress.c
+++ b/tools/testing/selftests/kvm/lib/memstress.c
@@ -114,24 +114,49 @@ void memstress_setup_vcpus(struct kvm_vm *vm, int nr_vcpus,
 		}
 
 		vcpu_args_set(vcpus[i], 1, i);
 
 		pr_debug("Added VCPU %d with test mem gpa [%lx, %lx)\n",
 			 i, vcpu_args->gpa, vcpu_args->gpa +
 			 (vcpu_args->pages * args->guest_page_size));
 	}
 }
 
+static struct kvm_vm *memstress_vm_create_with_vcpus(struct vm_shape shape,
+						     u32 nr_vcpus,
+						     u64 extra_mem_pages,
+						     void *guest_code,
+						     struct kvm_vcpu *vcpus[],
+						     u32 dirty_ring_size)
+{
+	struct kvm_vm *vm;
+	int i;
+
+	TEST_ASSERT(!nr_vcpus || vcpus, "Must provide vCPU array");
+
+	vm = __vm_create(shape, nr_vcpus, extra_mem_pages);
+
+	if (dirty_ring_size)
+		vm_enable_dirty_ring(vm, dirty_ring_size);
+
+	for (i = 0; i < nr_vcpus; ++i)
+		vcpus[i] = vm_vcpu_add(vm, i, guest_code);
+
+	kvm_arch_vm_finalize_vcpus(vm);
+	return vm;
+}
+
 struct kvm_vm *memstress_create_vm(enum vm_guest_mode mode, int nr_vcpus,
 				   u64 vcpu_memory_bytes, int slots,
 				   enum vm_mem_backing_src_type backing_src,
-				   bool partition_vcpu_memory_access)
+				   bool partition_vcpu_memory_access,
+				   u32 dirty_ring_size)
 {
 	struct memstress_args *args = &memstress_args;
 	struct kvm_vm *vm;
 	u64 guest_num_pages, slot0_pages = 0;
 	u64 backing_src_pagesz = get_backing_src_pagesz(backing_src);
 	u64 region_end_gfn;
 	int i;
 
 	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
 
@@ -160,23 +185,24 @@ struct kvm_vm *memstress_create_vm(enum vm_guest_mode mode, int nr_vcpus,
 	 * in-memory data structures.
 	 */
 	if (args->nested)
 		slot0_pages += memstress_nested_pages(nr_vcpus);
 
 	/*
 	 * Pass guest_num_pages to populate the page tables for test memory.
 	 * The memory is also added to memslot 0, but that's a benign side
 	 * effect as KVM allows aliasing HVAs in meslots.
 	 */
-	vm = __vm_create_with_vcpus(VM_SHAPE(mode), nr_vcpus,
-				    slot0_pages + guest_num_pages,
-				    memstress_guest_code, vcpus);
+	vm = memstress_vm_create_with_vcpus(VM_SHAPE(mode), nr_vcpus,
+					    slot0_pages + guest_num_pages,
+					    memstress_guest_code, vcpus,
+					    dirty_ring_size);
 
 	args->vm = vm;
 
 	/* Put the test region at the top guest physical memory. */
 	region_end_gfn = vm->max_gfn + 1;
 
 #ifdef __x86_64__
 	/*
 	 * When running vCPUs in L2, restrict the test region to 48 bits to
 	 * avoid needing 5-level page tables to identity map L2.
diff --git a/tools/testing/selftests/kvm/memslot_modification_stress_test.c b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
index 9c7578a098c3..3a1be9b75e01 100644
--- a/tools/testing/selftests/kvm/memslot_modification_stress_test.c
+++ b/tools/testing/selftests/kvm/memslot_modification_stress_test.c
@@ -83,21 +83,21 @@ struct test_params {
 	bool disable_slot_zap_quirk;
 };
 
 static void run_test(enum vm_guest_mode mode, void *arg)
 {
 	struct test_params *p = arg;
 	struct kvm_vm *vm;
 
 	vm = memstress_create_vm(mode, nr_vcpus, guest_percpu_mem_size, 1,
 				 VM_MEM_SRC_ANONYMOUS,
-				 p->partition_vcpu_memory_access);
+				 p->partition_vcpu_memory_access, 0);
 #ifdef __x86_64__
 	if (p->disable_slot_zap_quirk)
 		vm_enable_cap(vm, KVM_CAP_DISABLE_QUIRKS2, KVM_X86_QUIRK_SLOT_ZAP_ALL);
 
 	pr_info("Memslot zap quirk %s\n", p->disable_slot_zap_quirk ?
 		"disabled" : "enabled");
 #endif
 
 	pr_info("Finished creating vCPUs\n");
 
diff --git a/tools/testing/selftests/kvm/x86/dirty_log_page_splitting_test.c b/tools/testing/selftests/kvm/x86/dirty_log_page_splitting_test.c
index 388ba4101f97..661e9abfb439 100644
--- a/tools/testing/selftests/kvm/x86/dirty_log_page_splitting_test.c
+++ b/tools/testing/selftests/kvm/x86/dirty_log_page_splitting_test.c
@@ -94,21 +94,21 @@ static void run_test(enum vm_guest_mode mode, void *unused)
 	u64 pages_per_slot;
 	int i;
 	struct kvm_page_stats stats_populated;
 	struct kvm_page_stats stats_dirty_logging_enabled;
 	struct kvm_page_stats stats_dirty_pass[ITERATIONS];
 	struct kvm_page_stats stats_clear_pass[ITERATIONS];
 	struct kvm_page_stats stats_dirty_logging_disabled;
 	struct kvm_page_stats stats_repopulated;
 
 	vm = memstress_create_vm(mode, VCPUS, guest_percpu_mem_size,
-				 SLOTS, backing_src, false);
+				 SLOTS, backing_src, false, 0);
 
 	guest_num_pages = (VCPUS * guest_percpu_mem_size) >> vm->page_shift;
 	guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
 	host_num_pages = vm_num_host_pages(mode, guest_num_pages);
 	pages_per_slot = host_num_pages / SLOTS;
 	TEST_ASSERT_EQ(host_num_pages, pages_per_slot * SLOTS);
 	TEST_ASSERT(!(host_num_pages % 512),
 		    "Number of pages, '%lu' not a multiple of 2MiB", host_num_pages);
 
 	bitmaps = memstress_alloc_bitmaps(SLOTS, pages_per_slot);
-- 
2.55.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v1 2/3] KVM: selftests: Check dirty-ring size before enabling
  2026-09-29 11:37 [PATCH v1 0/3] KVM: selftests: Add support for dirty-ring on dirty_log_perf_test Leonardo Bras
  2026-09-29 11:37 ` [PATCH v1 1/3] KVM: selftests: memstress: Add option to enable dirty-ring on VM creation Leonardo Bras
@ 2026-09-29 11:37 ` Leonardo Bras
  2026-09-30 20:31   ` Sean Christopherson
  2026-09-29 11:37 ` [PATCH v1 3/3] KVM: selftests: dirty_log_perf_test: Add dirty-ring support Leonardo Bras
  2 siblings, 1 reply; 8+ messages in thread
From: Leonardo Bras @ 2026-09-29 11:37 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson, Shuah Khan, David Matlack,
	Leonardo Bras, Ackerley Tng, Marc Zyngier, Josh Hilke,
	Oliver Upton, Wu Fei, Steffen Eiden, Claudio Imbrenda
  Cc: kvm, linux-kselftest, linux-kernel

As of today, trying to enable dirty-ring with a size bigger than the
maximum will return an "argument list too long" error.

Change vm_enable_dirty_ring() to get the maximum size, then compare it to
the desired size before enabling. If the value is invalid, print a more
precise error message.

Signed-off-by: Leonardo Bras <leo.bras@arm.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 9ddc047d5c27..15a671b71553 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -168,24 +168,37 @@ unsigned int kvm_check_cap(long cap)
 	ret = __kvm_ioctl(kvm_fd, KVM_CHECK_EXTENSION, (void *)cap);
 	TEST_ASSERT(ret >= 0, KVM_IOCTL_ERROR(KVM_CHECK_EXTENSION, ret));
 
 	kvm_free_fd(kvm_fd);
 
 	return (unsigned int)ret;
 }
 
 void vm_enable_dirty_ring(struct kvm_vm *vm, u32 ring_size)
 {
-	if (vm_check_cap(vm, KVM_CAP_DIRTY_LOG_RING_ACQ_REL))
-		vm_enable_cap(vm, KVM_CAP_DIRTY_LOG_RING_ACQ_REL, ring_size);
-	else
-		vm_enable_cap(vm, KVM_CAP_DIRTY_LOG_RING, ring_size);
+	long cap = KVM_CAP_DIRTY_LOG_RING_ACQ_REL;
+	int max_size = vm_check_cap(vm, cap);
+
+	if (!max_size) {
+		cap = KVM_CAP_DIRTY_LOG_RING;
+		max_size = vm_check_cap(vm, cap);
+	}
+
+	TEST_ASSERT(max_size > 0, "Dirty-ring not supported in this kernel\n");
+	TEST_ASSERT(ring_size <= max_size && is_power_of_2(ring_size) &&
+		    ring_size >= getpagesize(),
+		    "Invalid dirty-ring size: Should be a power of two "
+		    "between %lu and %lu entries\n",
+		    getpagesize() / sizeof(struct kvm_dirty_gfn),
+		    max_size / sizeof(struct kvm_dirty_gfn));
+
+	vm_enable_cap(vm, cap, ring_size);
 	vm->dirty_ring_size = ring_size;
 }
 
 static void vm_open(struct kvm_vm *vm)
 {
 	vm->kvm_fd = _open_kvm_dev_path_or_exit(O_RDWR);
 
 	TEST_REQUIRE(kvm_has_cap(KVM_CAP_IMMEDIATE_EXIT));
 
 	vm->fd = __kvm_ioctl(vm->kvm_fd, KVM_CREATE_VM, (void *)vm->type);
-- 
2.55.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v1 3/3] KVM: selftests: dirty_log_perf_test: Add dirty-ring support
  2026-09-29 11:37 [PATCH v1 0/3] KVM: selftests: Add support for dirty-ring on dirty_log_perf_test Leonardo Bras
  2026-09-29 11:37 ` [PATCH v1 1/3] KVM: selftests: memstress: Add option to enable dirty-ring on VM creation Leonardo Bras
  2026-09-29 11:37 ` [PATCH v1 2/3] KVM: selftests: Check dirty-ring size before enabling Leonardo Bras
@ 2026-09-29 11:37 ` Leonardo Bras
  2 siblings, 0 replies; 8+ messages in thread
From: Leonardo Bras @ 2026-09-29 11:37 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson, Shuah Khan, David Matlack,
	Leonardo Bras, Ackerley Tng, Marc Zyngier, Josh Hilke,
	Oliver Upton, Wu Fei, Steffen Eiden, Claudio Imbrenda
  Cc: kvm, linux-kselftest, linux-kernel

dirty_log_test supports both dirty-bitmap and dirty-ring as dirty-page
tracking mechanisms, while dirty_log_perf_test only supports dirty-bitmap.

Add support to dirty-ring on dirty_log_perf_test so it can be used to
compare performance between changes in the mechanism.

vcpu_last_completed_iteration now needs smp_load_acquire/smp_store_relase()
for iterations >0 (after populating) as dirty-ring will save per-cpu time
spent on cleaning, and we can't have that reordered, as it may break the
summing-up.

smp_load_acquire(&vcpu_last_completed_iteration) is not needed on iteration
zero (populating) as dirty-logging should not be enabled, and we do not
access it from the run_test thread.

Signed-off-by: Leonardo Bras <leo.bras@arm.com>
---
 .../selftests/kvm/dirty_log_perf_test.c       | 128 ++++++++++++++++--
 1 file changed, 114 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 8f791ad7b86a..af0ab654d46e 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -6,68 +6,134 @@
  *
  * Copyright (C) 2018, Red Hat, Inc.
  * Copyright (C) 2020, Google, Inc.
  */
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <time.h>
 #include <pthread.h>
 #include <linux/bitmap.h>
+#include <asm/barrier.h>
 
 #include "kvm_util.h"
 #include "test_util.h"
 #include "memstress.h"
 #include "guest_modes.h"
 #include "ucall_common.h"
 
 /* How many host loops to run by default (one KVM_GET_DIRTY_LOG for each loop)*/
 #define TEST_HOST_LOOP_N		2UL
 
 static int nr_vcpus = 1;
 static u64 guest_percpu_mem_size = DEFAULT_PER_VCPU_MEM_SIZE;
 static bool run_vcpus_while_disabling_dirty_logging;
 
 /* Host variables */
 static u64 dirty_log_manual_caps;
+static u32 dirty_ring_size;
 static bool host_quit;
 static int iteration;
 static int vcpu_last_completed_iteration[KVM_MAX_VCPUS];
+static struct timespec vcpu_dirty_ring_collect[KVM_MAX_VCPUS];
+
+static void dirty_ring_collect(struct kvm_vcpu *vcpu, u32 *ring_idx,
+				struct timespec *ts)
+{
+	static pthread_mutex_t collect = PTHREAD_MUTEX_INITIALIZER;
+	struct timespec start;
+	struct kvm_dirty_gfn *dirty_gfns = vcpu_map_dirty_ring(vcpu);
+	u32 idx = *ring_idx;
+	u32 ring_size = vcpu->vm->dirty_ring_size / sizeof(struct kvm_dirty_gfn);
+	int cleared, count;
+
+	pthread_mutex_lock(&collect);
+
+	clock_gettime(CLOCK_MONOTONIC, &start);
+
+	while (true) {
+		struct kvm_dirty_gfn *cur;
+
+		cur = &dirty_gfns[idx % ring_size];
+		if (smp_load_acquire(&cur->flags) != KVM_DIRTY_GFN_F_DIRTY)
+			break;
+
+		smp_store_release(&cur->flags, KVM_DIRTY_GFN_F_RESET);
+		idx++;
+	}
+
+	count = idx - *ring_idx;
+	*ring_idx = idx;
+
+	cleared = kvm_vm_reset_dirty_ring(vcpu->vm);
+
+	/* Cleared pages should be the same as collected, as KVM is supposed to
+	 * clear only the entries that have been harvested, and a single vcpu will
+	 * harvest at time.
+	 */
+	TEST_ASSERT(cleared == count, "Reset dirty pages (%u) mismatch "
+		    "with collected (%u)", cleared, count);
+
+	*ts = timespec_add(*ts, timespec_elapsed(start));
+
+	pthread_mutex_unlock(&collect);
+}
 
 static void vcpu_worker(struct memstress_vcpu_args *vcpu_args)
 {
 	struct kvm_vcpu *vcpu = vcpu_args->vcpu;
 	int vcpu_idx = vcpu_args->vcpu_idx;
 	u64 pages_count = 0;
 	struct kvm_run *run;
 	struct timespec start;
 	struct timespec ts_diff;
 	struct timespec total = (struct timespec){0};
 	struct timespec avg;
+	bool use_dirty_ring = !!vcpu->vm->dirty_ring_size;
+	u32 ring_idx = 0;
 	int ret;
 
 	run = vcpu->run;
 
 	while (!READ_ONCE(host_quit)) {
 		int current_iteration = READ_ONCE(iteration);
+		struct timespec collect = (struct timespec){0};
 
 		clock_gettime(CLOCK_MONOTONIC, &start);
-		ret = _vcpu_run(vcpu);
+
+		do {
+			ret = _vcpu_run(vcpu);
+			if (!use_dirty_ring)
+				break;
+
+			dirty_ring_collect(vcpu, &ring_idx, &collect);
+		} while (!ret && run->exit_reason == KVM_EXIT_DIRTY_RING_FULL);
+
 		ts_diff = timespec_elapsed(start);
 
+		if (use_dirty_ring) {
+			ts_diff = timespec_sub(ts_diff, collect);
+			vcpu_dirty_ring_collect[vcpu_idx] = collect;
+		}
+
 		TEST_ASSERT(ret == 0, "vcpu_run failed: %d", ret);
 		TEST_ASSERT(get_ucall(vcpu, NULL) == UCALL_SYNC,
 			    "Invalid guest sync status: exit_reason=%s",
 			    exit_reason_str(run->exit_reason));
 
 		pr_debug("Got sync event from vCPU %d\n", vcpu_idx);
-		vcpu_last_completed_iteration[vcpu_idx] = current_iteration;
+		/*
+		 * Make sure vcpu_last_completed_iteration write happens before
+		 * updating current iteration.  Pairs with run_test, after populating..
+		 */
+		smp_store_release(&vcpu_last_completed_iteration[vcpu_idx],
+				  current_iteration);
 		pr_debug("vCPU %d updated last completed iteration to %d\n",
 			 vcpu_idx, vcpu_last_completed_iteration[vcpu_idx]);
 
 		if (current_iteration) {
 			pages_count += vcpu_args->pages;
 			total = timespec_add(total, ts_diff);
 			pr_debug("vCPU %d iteration %d dirty memory time: %ld.%.9lds\n",
 				vcpu_idx, current_iteration, ts_diff.tv_sec,
 				ts_diff.tv_nsec);
 		} else {
@@ -112,42 +178,46 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	struct timespec start;
 	struct timespec ts_diff;
 	struct timespec get_dirty_log_total = (struct timespec){0};
 	struct timespec vcpu_dirty_total = (struct timespec){0};
 	struct timespec avg;
 	struct timespec clear_dirty_log_total = (struct timespec){0};
 	int i;
 
 	vm = memstress_create_vm(mode, nr_vcpus, guest_percpu_mem_size,
 				 p->slots, p->backing_src,
-				 p->partition_vcpu_memory_access, 0);
+				 p->partition_vcpu_memory_access,
+				 dirty_ring_size);
 
 	memstress_set_write_percent(vm, p->write_percent);
 
 	guest_num_pages = (nr_vcpus * guest_percpu_mem_size) >> vm->page_shift;
 	guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
 	host_num_pages = vm_num_host_pages(mode, guest_num_pages);
 	pages_per_slot = host_num_pages / p->slots;
 
-	bitmaps = memstress_alloc_bitmaps(p->slots, pages_per_slot);
+	if (!dirty_ring_size)
+		bitmaps = memstress_alloc_bitmaps(p->slots, pages_per_slot);
 
 	if (dirty_log_manual_caps)
 		vm_enable_cap(vm, KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2,
 			      dirty_log_manual_caps);
 
 	/* Start the iterations */
 	iteration = 0;
 	host_quit = false;
 
 	clock_gettime(CLOCK_MONOTONIC, &start);
-	for (i = 0; i < nr_vcpus; i++)
+	for (i = 0; i < nr_vcpus; i++) {
 		vcpu_last_completed_iteration[i] = -1;
+		vcpu_dirty_ring_collect[i] = (struct timespec){0};
+	}
 
 	/*
 	 * Use 100% writes during the population phase to ensure all
 	 * memory is actually populated and not just mapped to the zero
 	 * page. The prevents expensive copy-on-write faults from
 	 * occurring during the dirty memory iterations below, which
 	 * would pollute the performance results.
 	 */
 	memstress_set_write_percent(vm, 100);
 	memstress_set_random_access(vm, false);
@@ -178,30 +248,45 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	while (iteration < p->iterations) {
 		/*
 		 * Incrementing the iteration number will start the vCPUs
 		 * dirtying memory again.
 		 */
 		clock_gettime(CLOCK_MONOTONIC, &start);
 		iteration++;
 
 		pr_debug("Starting iteration %d\n", iteration);
 		for (i = 0; i < nr_vcpus; i++) {
-			while (READ_ONCE(vcpu_last_completed_iteration[i])
+			while (smp_load_acquire(&vcpu_last_completed_iteration[i])
 			       != iteration)
 				;
 		}
 
 		ts_diff = timespec_elapsed(start);
 		vcpu_dirty_total = timespec_add(vcpu_dirty_total, ts_diff);
 		pr_info("Iteration %d dirty memory time: %ld.%.9lds\n",
 			iteration, ts_diff.tv_sec, ts_diff.tv_nsec);
 
+		if (dirty_ring_size) {
+			struct timespec iteration_sum = (struct timespec){0};
+
+			for (i = 0; i < nr_vcpus; i++)
+				iteration_sum = timespec_add(iteration_sum,
+							     vcpu_dirty_ring_collect[i]);
+
+			pr_info("Iteration %d clear dirty ring time: %ld.%.9lds\n",
+				iteration, iteration_sum.tv_sec, iteration_sum.tv_nsec);
+
+			clear_dirty_log_total = timespec_add(clear_dirty_log_total,
+							     iteration_sum);
+			continue;
+		}
+
 		clock_gettime(CLOCK_MONOTONIC, &start);
 		memstress_get_dirty_log(vm, bitmaps, p->slots);
 		ts_diff = timespec_elapsed(start);
 		get_dirty_log_total = timespec_add(get_dirty_log_total,
 						   ts_diff);
 		pr_info("Iteration %d get dirty log time: %ld.%.9lds\n",
 			iteration, ts_diff.tv_sec, ts_diff.tv_nsec);
 
 		if (dirty_log_manual_caps) {
 			clock_gettime(CLOCK_MONOTONIC, &start);
@@ -231,46 +316,53 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 		ts_diff.tv_sec, ts_diff.tv_nsec);
 
 	/*
 	 * Tell the vCPU threads to quit.  No need to manually check that vCPUs
 	 * have stopped running after disabling dirty logging, the join will
 	 * wait for them to exit.
 	 */
 	host_quit = true;
 	memstress_join_vcpu_threads(nr_vcpus);
 
-	avg = timespec_div(get_dirty_log_total, p->iterations);
-	pr_info("Get dirty log over %lu iterations took %ld.%.9lds. (Avg %ld.%.9lds/iteration)\n",
-		p->iterations, get_dirty_log_total.tv_sec,
-		get_dirty_log_total.tv_nsec, avg.tv_sec, avg.tv_nsec);
+	if (!dirty_ring_size) {
+		avg = timespec_div(get_dirty_log_total, p->iterations);
+		pr_info("Get dirty log over %lu iterations took %ld.%.9lds. (Avg %ld.%.9lds/iteration)\n",
+			p->iterations, get_dirty_log_total.tv_sec,
+			get_dirty_log_total.tv_nsec, avg.tv_sec, avg.tv_nsec);
+	}
 
-	if (dirty_log_manual_caps) {
+	if (dirty_log_manual_caps || dirty_ring_size) {
 		avg = timespec_div(clear_dirty_log_total, p->iterations);
 		pr_info("Clear dirty log over %lu iterations took %ld.%.9lds. (Avg %ld.%.9lds/iteration)\n",
 			p->iterations, clear_dirty_log_total.tv_sec,
 			clear_dirty_log_total.tv_nsec, avg.tv_sec, avg.tv_nsec);
 	}
 
-	memstress_free_bitmaps(bitmaps, p->slots);
+	if (!dirty_ring_size)
+		memstress_free_bitmaps(bitmaps, p->slots);
 	memstress_destroy_vm(vm);
 }
 
 static void help(char *name)
 {
 	puts("");
 	printf("usage: %s [-h] [-a] [-i iterations] [-p offset] [-g] "
 	       "[-m mode] [-n] [-b vcpu bytes] [-v vcpus] [-o] [-r random seed ] [-s mem type]"
-	       "[-x memslots] [-w percentage] [-c physical cpus to run test on]\n", name);
+	       "[-x memslots] [-w percentage] [-c physical cpus to run test on] \n"
+	       "[-d dirty-ring entries]\n", name);
 	puts("");
 	printf(" -a: access memory randomly rather than in order.\n");
 	printf(" -i: specify iteration counts (default: %"PRIu64")\n",
 	       TEST_HOST_LOOP_N);
+	printf(" -d: enable dirty-ring for tracking dirty pages, with given entries count.\n"
+	       "     If non-zero, will cause dirty-ring to be used instead of\n"
+	       "     dirty-bitmap. Must be a power of two.\n");
 	printf(" -g: Do not enable KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2. This\n"
 	       "     makes KVM_GET_DIRTY_LOG clear the dirty log (i.e.\n"
 	       "     KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE is not enabled)\n"
 	       "     and writes will be tracked as soon as dirty logging is\n"
 	       "     enabled on the memslot (i.e. KVM_DIRTY_LOG_INITIALLY_SET\n"
 	       "     is not enabled).\n");
 	printf(" -p: specify guest physical test memory offset\n"
 	       "     Warning: a low offset can conflict with the loaded test code.\n");
 	guest_modes_help();
 	printf(" -n: Run the vCPUs in nested mode (L2)\n");
@@ -302,42 +394,50 @@ int main(int argc, char *argv[])
 	int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
 	const char *pcpu_list = NULL;
 	struct test_params p = {
 		.iterations = TEST_HOST_LOOP_N,
 		.partition_vcpu_memory_access = true,
 		.backing_src = DEFAULT_VM_MEM_SRC,
 		.slots = 1,
 		.write_percent = 100,
 	};
 	int opt;
+	u64 tmp;
 
 	/* Override the seed to be deterministic by default. */
 	kvm_random_seed = 1;
 
 	dirty_log_manual_caps =
 		kvm_check_cap(KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2);
 	dirty_log_manual_caps &= (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE |
 				  KVM_DIRTY_LOG_INITIALLY_SET);
 
 	guest_modes_append_default();
 
-	while ((opt = getopt(argc, argv, "ab:c:eghi:m:nop:r:s:v:x:w:")) != -1) {
+	while ((opt = getopt(argc, argv, "ab:c:d:eghi:m:nop:r:s:v:x:w:")) != -1) {
 		switch (opt) {
 		case 'a':
 			p.random_access = true;
 			break;
 		case 'b':
 			guest_percpu_mem_size = parse_size(optarg);
 			break;
 		case 'c':
 			pcpu_list = optarg;
 			break;
+		case 'd':
+			tmp = parse_size(optarg);
+			TEST_ASSERT(tmp, "Dirty-ring size should be > 0");
+			TEST_ASSERT(tmp <= UINT32_MAX / sizeof(struct kvm_dirty_gfn),
+				    "Dirty-ring size (%lu) is too large.", tmp);
+			dirty_ring_size = tmp * sizeof(struct kvm_dirty_gfn);
+			break;
 		case 'e':
 			/* 'e' is for evil. */
 			run_vcpus_while_disabling_dirty_logging = true;
 			break;
 		case 'g':
 			dirty_log_manual_caps = 0;
 			break;
 		case 'h':
 			help(argv[0]);
 			break;
-- 
2.55.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 1/3] KVM: selftests: memstress: Add option to enable dirty-ring on VM creation
  2026-09-29 11:37 ` [PATCH v1 1/3] KVM: selftests: memstress: Add option to enable dirty-ring on VM creation Leonardo Bras
@ 2026-09-30 18:21   ` Sean Christopherson
  2026-10-01 14:40     ` Leonardo Bras
  0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2026-09-30 18:21 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: Paolo Bonzini, Shuah Khan, David Matlack, Ackerley Tng,
	Marc Zyngier, Josh Hilke, Oliver Upton, Wu Fei, Steffen Eiden,
	Claudio Imbrenda, kvm, linux-kselftest, linux-kernel

On Tue, Sep 29, 2026, Leonardo Bras wrote:
> Dirty-ring should be enabled after the VM is created, but before the
> creation of any cpu. To to so, add an option on memstress_create_vm() that
> takes dirty-ring size, and enables it at the correct moment.
> 
> This required a new function memstress_vm_create_with_vcpus() to be
> created based on the previously used __vm_create_with_vcpus();

Rather than pile on more params, if I ask really nicely, would you rework
memstress_create_vm() and friends APIs to take a "struct memstress_params"?
Except for dirty_log_page_splitting_test.c, all of the memstress tests already
use a struct, i.e. while tedious, it should be straightforward and won't put
undue burden on tests (which is why the core VM creation APIs don't take a struct
of params).

Then it will be easier to extend memstress in the future, and I suspect the
existing code will come out cleaner as well.  E.g. dirty_log_perf_test.c stores
nr_vcpus as a global, despite it being fully contained by main() => run_test(),
right alongside the test params...

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 2/3] KVM: selftests: Check dirty-ring size before enabling
  2026-09-29 11:37 ` [PATCH v1 2/3] KVM: selftests: Check dirty-ring size before enabling Leonardo Bras
@ 2026-09-30 20:31   ` Sean Christopherson
  2026-10-01 14:46     ` Leonardo Bras
  0 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2026-09-30 20:31 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: Paolo Bonzini, Shuah Khan, David Matlack, Ackerley Tng,
	Marc Zyngier, Josh Hilke, Oliver Upton, Wu Fei, Steffen Eiden,
	Claudio Imbrenda, kvm, linux-kselftest, linux-kernel

On Tue, Sep 29, 2026, Leonardo Bras wrote:
> As of today, trying to enable dirty-ring with a size bigger than the
> maximum will return an "argument list too long" error.
> 
> Change vm_enable_dirty_ring() to get the maximum size, then compare it to
> the desired size before enabling. If the value is invalid, print a more
> precise error message.
> 
> Signed-off-by: Leonardo Bras <leo.bras@arm.com>
> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 9ddc047d5c27..15a671b71553 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -168,24 +168,37 @@ unsigned int kvm_check_cap(long cap)
>  	ret = __kvm_ioctl(kvm_fd, KVM_CHECK_EXTENSION, (void *)cap);
>  	TEST_ASSERT(ret >= 0, KVM_IOCTL_ERROR(KVM_CHECK_EXTENSION, ret));
>  
>  	kvm_free_fd(kvm_fd);
>  
>  	return (unsigned int)ret;
>  }
>  
>  void vm_enable_dirty_ring(struct kvm_vm *vm, u32 ring_size)
>  {
> -	if (vm_check_cap(vm, KVM_CAP_DIRTY_LOG_RING_ACQ_REL))
> -		vm_enable_cap(vm, KVM_CAP_DIRTY_LOG_RING_ACQ_REL, ring_size);
> -	else
> -		vm_enable_cap(vm, KVM_CAP_DIRTY_LOG_RING, ring_size);
> +	long cap = KVM_CAP_DIRTY_LOG_RING_ACQ_REL;
> +	int max_size = vm_check_cap(vm, cap);
> +
> +	if (!max_size) {
> +		cap = KVM_CAP_DIRTY_LOG_RING;
> +		max_size = vm_check_cap(vm, cap);
> +	}
> +
> +	TEST_ASSERT(max_size > 0,

Rather than open code this, which is kinda sorta going to show up in multiple
places, what if we do this as a prep patch?  Then vm_enable_dirty_ring() can use
kvm_get_dirty_ring_cap() (completely untested).

diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index af5eb0334a74..558a71d631da 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -291,8 +291,7 @@ static void default_after_vcpu_run(struct kvm_vcpu *vcpu)
 
 static bool dirty_ring_supported(void)
 {
-	return (kvm_has_cap(KVM_CAP_DIRTY_LOG_RING) ||
-		kvm_has_cap(KVM_CAP_DIRTY_LOG_RING_ACQ_REL));
+	return kvm_get_dirty_ring_cap();
 }
 
 static void dirty_ring_create_vm_done(struct kvm_vm *vm)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index e3b122719262..c08d1581221e 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -339,6 +339,17 @@ static inline bool kvm_has_cap(long cap)
 	return kvm_check_cap(cap);
 }
 
+static inline long kvm_get_dirty_ring_cap(void)
+{
+	if (kvm_has_cap(KVM_CAP_DIRTY_LOG_RING_ACQ_REL))
+		return KVM_CAP_DIRTY_LOG_RING_ACQ_REL;
+
+	if (kvm_has_cap(KVM_CAP_DIRTY_LOG_RING))
+		return KVM_CAP_DIRTY_LOG_RING;
+
+	return 0;	
+}
+
 /*
  * Use the "inner", double-underscore macro when reporting errors from within
  * other macros so that the name of ioctl() and not its literal numeric value


> "Dirty-ring not supported in this kernel\n");

"this kernel" could be misleading, some architectures simply don't support the
dirty ring.

> +	TEST_ASSERT(ring_size <= max_size && is_power_of_2(ring_size) &&
> +		    ring_size >= getpagesize(),
> +		    "Invalid dirty-ring size: Should be a power of two "
> +		    "between %lu and %lu entries\n",

Don't wrap strings.  The "Invalid dirty-ring size:" part is redudant with the
expressions, just drop that to shorten things.  This should also spit out the
requested ring_size.  Stating the range as a number of entries is also confusing;
as a debugger, I don't want to have to go look at the size of kvm_dirty_gfn to
understand why ring_size is invalid.

And maybe split up the asserts?  If the goal is to make it easier for developers
to know when they messed up, might as well make it as easy as possible.  E.g.
leaning on the above diff, something like this?

	long cap = kvm_get_dirty_ring_cap();

	TEST_ASSERT(cap, "Dirty-ring not supported");

	TEST_ASSERT(is_power_of_2(ring_size),
		    "Dirty-ring size '0x%x' must be a power-of-2", ring_size);
	TEST_ASSERT(ring_size >= getpagesize(),
		    "Dirty-ring size '0x%x' must be at least one (host) page", ring_size);
	TEST_ASSERT(ring_size >= vm_check_cap(vm, cap),
		    "Dirty-ring size '0x%x' is larger than KVM's limit of '0x%x'",
		    ring_size, vm_check_cap(vm, cap));

	vm_enable_cap(vm, cap, ring_size);
	vm->dirty_ring_size = ring_size;

> +		    getpagesize() / sizeof(struct kvm_dirty_gfn),
> +		    max_size / sizeof(struct kvm_dirty_gfn));
> +
> +	vm_enable_cap(vm, cap, ring_size);
>  	vm->dirty_ring_size = ring_size;
>  }
>  
>  static void vm_open(struct kvm_vm *vm)
>  {
>  	vm->kvm_fd = _open_kvm_dev_path_or_exit(O_RDWR);
>  
>  	TEST_REQUIRE(kvm_has_cap(KVM_CAP_IMMEDIATE_EXIT));
>  
>  	vm->fd = __kvm_ioctl(vm->kvm_fd, KVM_CREATE_VM, (void *)vm->type);
> -- 
> 2.55.0
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 1/3] KVM: selftests: memstress: Add option to enable dirty-ring on VM creation
  2026-09-30 18:21   ` Sean Christopherson
@ 2026-10-01 14:40     ` Leonardo Bras
  0 siblings, 0 replies; 8+ messages in thread
From: Leonardo Bras @ 2026-10-01 14:40 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Leonardo Bras, Paolo Bonzini, Shuah Khan, David Matlack,
	Ackerley Tng, Marc Zyngier, Josh Hilke, Oliver Upton, Wu Fei,
	Steffen Eiden, Claudio Imbrenda, kvm, linux-kselftest,
	linux-kernel

On Wed, Sep 30, 2026 at 11:21:22AM -0700, Sean Christopherson wrote:
> On Tue, Sep 29, 2026, Leonardo Bras wrote:
> > Dirty-ring should be enabled after the VM is created, but before the
> > creation of any cpu. To to so, add an option on memstress_create_vm() that
> > takes dirty-ring size, and enables it at the correct moment.
> > 
> > This required a new function memstress_vm_create_with_vcpus() to be
> > created based on the previously used __vm_create_with_vcpus();
> 
> Rather than pile on more params, if I ask really nicely, would you rework
> memstress_create_vm() and friends APIs to take a "struct memstress_params"?
> Except for dirty_log_page_splitting_test.c, all of the memstress tests already
> use a struct, i.e. while tedious, it should be straightforward and won't put
> undue burden on tests (which is why the core VM creation APIs don't take a struct
> of params).
> 
> Then it will be easier to extend memstress in the future, and I suspect the
> existing code will come out cleaner as well.  E.g. dirty_log_perf_test.c stores
> nr_vcpus as a global, despite it being fully contained by main() => run_test(),
> right alongside the test params...


Hi Sean, thanks for reviewing!

Sure, will convert it to use a struct as well :)

Thanks!
Leo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 2/3] KVM: selftests: Check dirty-ring size before enabling
  2026-09-30 20:31   ` Sean Christopherson
@ 2026-10-01 14:46     ` Leonardo Bras
  0 siblings, 0 replies; 8+ messages in thread
From: Leonardo Bras @ 2026-10-01 14:46 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Leonardo Bras, Paolo Bonzini, Shuah Khan, David Matlack,
	Ackerley Tng, Marc Zyngier, Josh Hilke, Oliver Upton, Wu Fei,
	Steffen Eiden, Claudio Imbrenda, kvm, linux-kselftest,
	linux-kernel

On Wed, Sep 30, 2026 at 01:31:25PM -0700, Sean Christopherson wrote:
> On Tue, Sep 29, 2026, Leonardo Bras wrote:
> > As of today, trying to enable dirty-ring with a size bigger than the
> > maximum will return an "argument list too long" error.
> > 
> > Change vm_enable_dirty_ring() to get the maximum size, then compare it to
> > the desired size before enabling. If the value is invalid, print a more
> > precise error message.
> > 
> > Signed-off-by: Leonardo Bras <leo.bras@arm.com>
> > ---
> >  tools/testing/selftests/kvm/lib/kvm_util.c | 21 +++++++++++++++++----
> >  1 file changed, 17 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> > index 9ddc047d5c27..15a671b71553 100644
> > --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> > @@ -168,24 +168,37 @@ unsigned int kvm_check_cap(long cap)
> >  	ret = __kvm_ioctl(kvm_fd, KVM_CHECK_EXTENSION, (void *)cap);
> >  	TEST_ASSERT(ret >= 0, KVM_IOCTL_ERROR(KVM_CHECK_EXTENSION, ret));
> >  
> >  	kvm_free_fd(kvm_fd);
> >  
> >  	return (unsigned int)ret;
> >  }
> >  
> >  void vm_enable_dirty_ring(struct kvm_vm *vm, u32 ring_size)
> >  {
> > -	if (vm_check_cap(vm, KVM_CAP_DIRTY_LOG_RING_ACQ_REL))
> > -		vm_enable_cap(vm, KVM_CAP_DIRTY_LOG_RING_ACQ_REL, ring_size);
> > -	else
> > -		vm_enable_cap(vm, KVM_CAP_DIRTY_LOG_RING, ring_size);
> > +	long cap = KVM_CAP_DIRTY_LOG_RING_ACQ_REL;
> > +	int max_size = vm_check_cap(vm, cap);
> > +
> > +	if (!max_size) {
> > +		cap = KVM_CAP_DIRTY_LOG_RING;
> > +		max_size = vm_check_cap(vm, cap);
> > +	}
> > +
> > +	TEST_ASSERT(max_size > 0,
> 
> Rather than open code this, which is kinda sorta going to show up in multiple
> places, what if we do this as a prep patch?  Then vm_enable_dirty_ring() can use
> kvm_get_dirty_ring_cap() (completely untested).

Sure, if you think it's useful :)

> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> index af5eb0334a74..558a71d631da 100644
> --- a/tools/testing/selftests/kvm/dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> @@ -291,8 +291,7 @@ static void default_after_vcpu_run(struct kvm_vcpu *vcpu)
>  
>  static bool dirty_ring_supported(void)
>  {
> -	return (kvm_has_cap(KVM_CAP_DIRTY_LOG_RING) ||
> -		kvm_has_cap(KVM_CAP_DIRTY_LOG_RING_ACQ_REL));
> +	return kvm_get_dirty_ring_cap();
>  }
>  
>  static void dirty_ring_create_vm_done(struct kvm_vm *vm)
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index e3b122719262..c08d1581221e 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -339,6 +339,17 @@ static inline bool kvm_has_cap(long cap)
>  	return kvm_check_cap(cap);
>  }
>  
> +static inline long kvm_get_dirty_ring_cap(void)
> +{
> +	if (kvm_has_cap(KVM_CAP_DIRTY_LOG_RING_ACQ_REL))
> +		return KVM_CAP_DIRTY_LOG_RING_ACQ_REL;
> +
> +	if (kvm_has_cap(KVM_CAP_DIRTY_LOG_RING))
> +		return KVM_CAP_DIRTY_LOG_RING;
> +
> +	return 0;	
> +}
> +
>  /*
>   * Use the "inner", double-underscore macro when reporting errors from within
>   * other macros so that the name of ioctl() and not its literal numeric value
> 
> 
> > "Dirty-ring not supported in this kernel\n");
> 
> "this kernel" could be misleading, some architectures simply don't support the
> dirty ring.

So, do you suggest keeping it simple by printing "Dirty-ring not supported"?


> 
> > +	TEST_ASSERT(ring_size <= max_size && is_power_of_2(ring_size) &&
> > +		    ring_size >= getpagesize(),
> > +		    "Invalid dirty-ring size: Should be a power of two "
> > +		    "between %lu and %lu entries\n",
> 
> Don't wrap strings.  The "Invalid dirty-ring size:" part is redudant with the
> expressions, just drop that to shorten things.  This should also spit out the
> requested ring_size.  Stating the range as a number of entries is also confusing;
> as a debugger, I don't want to have to go look at the size of kvm_dirty_gfn to
> understand why ring_size is invalid.
> 
> And maybe split up the asserts?  If the goal is to make it easier for developers
> to know when they messed up, might as well make it as easy as possible.  E.g.
> leaning on the above diff, something like this?
> 
> 	long cap = kvm_get_dirty_ring_cap();
> 
> 	TEST_ASSERT(cap, "Dirty-ring not supported");
> 
> 	TEST_ASSERT(is_power_of_2(ring_size),
> 		    "Dirty-ring size '0x%x' must be a power-of-2", ring_size);
> 	TEST_ASSERT(ring_size >= getpagesize(),
> 		    "Dirty-ring size '0x%x' must be at least one (host) page", ring_size);
> 	TEST_ASSERT(ring_size >= vm_check_cap(vm, cap),
> 		    "Dirty-ring size '0x%x' is larger than KVM's limit of '0x%x'",
> 		    ring_size, vm_check_cap(vm, cap));
> 
> 	vm_enable_cap(vm, cap, ring_size);
> 	vm->dirty_ring_size = ring_size;
> 

Humm, I see the point. 
Will do as suggested then, thanks!

Leo

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-10-01 14:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-29 11:37 [PATCH v1 0/3] KVM: selftests: Add support for dirty-ring on dirty_log_perf_test Leonardo Bras
2026-09-29 11:37 ` [PATCH v1 1/3] KVM: selftests: memstress: Add option to enable dirty-ring on VM creation Leonardo Bras
2026-09-30 18:21   ` Sean Christopherson
2026-10-01 14:40     ` Leonardo Bras
2026-09-29 11:37 ` [PATCH v1 2/3] KVM: selftests: Check dirty-ring size before enabling Leonardo Bras
2026-09-30 20:31   ` Sean Christopherson
2026-10-01 14:46     ` Leonardo Bras
2026-09-29 11:37 ` [PATCH v1 3/3] KVM: selftests: dirty_log_perf_test: Add dirty-ring support Leonardo Bras

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®