[PATCH V5 0/2] selftests: KVM: Add a test for eager page splitting

[PATCH V5 0/2] selftests: KVM: Add a test for eager page splitting

[Ben Gardon]

David Matlack recently added a feature known as eager page splitting
to x86 KVM. This feature improves vCPU performance during dirty
logging because the splitting operation is moved out of the page
fault path, avoiding EPT/NPT violations or allowing the vCPU threads
to resolve the violation in the fast path.

While this feature is a great performance improvement, it does not
have adequate testing in KVM selftests. Add a test to provide coverage
of eager page splitting.

Patch 1 is a quick refactor to be able to re-use some code from
dirty_log_perf_test.
Patch 2 adds the actual test.

V4->V5:
 - V4: https://lore.kernel.org/all/Y9lIfGtMEEsaw6cd@google.com/
 - Separated a helper function to just run vCPUs for an iteration and
   not also collect the stats. Suggested by Vipin.
 - Added a log message when the host does not support MANUAL_PROTECT,
   and so testing with that capability is skipped. Also suggested by
   Vipin.
 - Added RB from Vipin.

V3->V4:
 - Added the copyright notices back. Thanks Vipin for the right
   thing to do there.

V2->V3:
 - Removed copyright notice from the top of dirty_log_page_splitting.c
 - Adopted ASSERT_EQ for test assertions
 - Now skipping testing with MANUAL_PROTECT if unsupported

V1->V2:
 - Run test in multiple modes, as suggested by David and Ricardo
 - Cleanups from shameful copy-pasta, as suggested by David

Ben Gardon (2):
  selftests: KVM: Move dirty logging functions to memstress.(c|h)
  selftests: KVM: Add dirty logging page splitting test

 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/dirty_log_perf_test.c       |  84 +-----
 .../selftests/kvm/include/kvm_util_base.h     |   1 +
 .../testing/selftests/kvm/include/memstress.h |   8 +
 tools/testing/selftests/kvm/lib/kvm_util.c    |   5 +
 tools/testing/selftests/kvm/lib/memstress.c   |  72 +++++
 .../x86_64/dirty_log_page_splitting_test.c    | 260 ++++++++++++++++++
 7 files changed, 354 insertions(+), 77 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test.c

-- 
2.39.1.456.gfc5497dd1b-goog

[PATCH V5 1/2] selftests: KVM: Move dirty logging functions to memstress.(c|h)

[Ben Gardon]

Move some helper functions from dirty_log_perf_test.c to the memstress
library so that they can be used in a future commit which tests page
splitting during dirty logging.

Reviewed-by: Vipin Sharma <vipinsh@google.com>

Signed-off-by: Ben Gardon <bgardon@google.com>
---
 .../selftests/kvm/dirty_log_perf_test.c       | 84 ++-----------------
 .../testing/selftests/kvm/include/memstress.h |  8 ++
 tools/testing/selftests/kvm/lib/memstress.c   | 72 ++++++++++++++++
 3 files changed, 87 insertions(+), 77 deletions(-)

diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index e9d6d1aecf89c..416719e205183 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -136,77 +136,6 @@ struct test_params {
 	bool random_access;
 };
 
-static void toggle_dirty_logging(struct kvm_vm *vm, int slots, bool enable)
-{
-	int i;
-
-	for (i = 0; i < slots; i++) {
-		int slot = MEMSTRESS_MEM_SLOT_INDEX + i;
-		int flags = enable ? KVM_MEM_LOG_DIRTY_PAGES : 0;
-
-		vm_mem_region_set_flags(vm, slot, flags);
-	}
-}
-
-static inline void enable_dirty_logging(struct kvm_vm *vm, int slots)
-{
-	toggle_dirty_logging(vm, slots, true);
-}
-
-static inline void disable_dirty_logging(struct kvm_vm *vm, int slots)
-{
-	toggle_dirty_logging(vm, slots, false);
-}
-
-static void get_dirty_log(struct kvm_vm *vm, unsigned long *bitmaps[], int slots)
-{
-	int i;
-
-	for (i = 0; i < slots; i++) {
-		int slot = MEMSTRESS_MEM_SLOT_INDEX + i;
-
-		kvm_vm_get_dirty_log(vm, slot, bitmaps[i]);
-	}
-}
-
-static void clear_dirty_log(struct kvm_vm *vm, unsigned long *bitmaps[],
-			    int slots, uint64_t pages_per_slot)
-{
-	int i;
-
-	for (i = 0; i < slots; i++) {
-		int slot = MEMSTRESS_MEM_SLOT_INDEX + i;
-
-		kvm_vm_clear_dirty_log(vm, slot, bitmaps[i], 0, pages_per_slot);
-	}
-}
-
-static unsigned long **alloc_bitmaps(int slots, uint64_t pages_per_slot)
-{
-	unsigned long **bitmaps;
-	int i;
-
-	bitmaps = malloc(slots * sizeof(bitmaps[0]));
-	TEST_ASSERT(bitmaps, "Failed to allocate bitmaps array.");
-
-	for (i = 0; i < slots; i++) {
-		bitmaps[i] = bitmap_zalloc(pages_per_slot);
-		TEST_ASSERT(bitmaps[i], "Failed to allocate slot bitmap.");
-	}
-
-	return bitmaps;
-}
-
-static void free_bitmaps(unsigned long *bitmaps[], int slots)
-{
-	int i;
-
-	for (i = 0; i < slots; i++)
-		free(bitmaps[i]);
-
-	free(bitmaps);
-}
-
 static void run_test(enum vm_guest_mode mode, void *arg)
 {
 	struct test_params *p = arg;
@@ -236,7 +165,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	host_num_pages = vm_num_host_pages(mode, guest_num_pages);
 	pages_per_slot = host_num_pages / p->slots;
 
-	bitmaps = alloc_bitmaps(p->slots, pages_per_slot);
+	bitmaps = memstress_alloc_bitmaps(p->slots, pages_per_slot);
 
 	if (dirty_log_manual_caps)
 		vm_enable_cap(vm, KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2,
@@ -277,7 +206,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 
 	/* Enable dirty logging */
 	clock_gettime(CLOCK_MONOTONIC, &start);
-	enable_dirty_logging(vm, p->slots);
+	memstress_enable_dirty_logging(vm, p->slots);
 	ts_diff = timespec_elapsed(start);
 	pr_info("Enabling dirty logging time: %ld.%.9lds\n\n",
 		ts_diff.tv_sec, ts_diff.tv_nsec);
@@ -306,7 +235,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 			iteration, ts_diff.tv_sec, ts_diff.tv_nsec);
 
 		clock_gettime(CLOCK_MONOTONIC, &start);
-		get_dirty_log(vm, bitmaps, p->slots);
+		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);
@@ -315,7 +244,8 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 
 		if (dirty_log_manual_caps) {
 			clock_gettime(CLOCK_MONOTONIC, &start);
-			clear_dirty_log(vm, bitmaps, p->slots, pages_per_slot);
+			memstress_clear_dirty_log(vm, bitmaps, p->slots,
+						  pages_per_slot);
 			ts_diff = timespec_elapsed(start);
 			clear_dirty_log_total = timespec_add(clear_dirty_log_total,
 							     ts_diff);
@@ -334,7 +264,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 
 	/* Disable dirty logging */
 	clock_gettime(CLOCK_MONOTONIC, &start);
-	disable_dirty_logging(vm, p->slots);
+	memstress_disable_dirty_logging(vm, p->slots);
 	ts_diff = timespec_elapsed(start);
 	pr_info("Disabling dirty logging time: %ld.%.9lds\n",
 		ts_diff.tv_sec, ts_diff.tv_nsec);
@@ -359,7 +289,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 			clear_dirty_log_total.tv_nsec, avg.tv_sec, avg.tv_nsec);
 	}
 
-	free_bitmaps(bitmaps, p->slots);
+	memstress_free_bitmaps(bitmaps, p->slots);
 	arch_cleanup_vm(vm);
 	memstress_destroy_vm(vm);
 }
diff --git a/tools/testing/selftests/kvm/include/memstress.h b/tools/testing/selftests/kvm/include/memstress.h
index 72e3e358ef7bd..ce4e603050eaa 100644
--- a/tools/testing/selftests/kvm/include/memstress.h
+++ b/tools/testing/selftests/kvm/include/memstress.h
@@ -72,4 +72,12 @@ void memstress_guest_code(uint32_t vcpu_id);
 uint64_t memstress_nested_pages(int nr_vcpus);
 void memstress_setup_nested(struct kvm_vm *vm, int nr_vcpus, struct kvm_vcpu *vcpus[]);
 
+void memstress_enable_dirty_logging(struct kvm_vm *vm, int slots);
+void memstress_disable_dirty_logging(struct kvm_vm *vm, int slots);
+void memstress_get_dirty_log(struct kvm_vm *vm, unsigned long *bitmaps[], int slots);
+void memstress_clear_dirty_log(struct kvm_vm *vm, unsigned long *bitmaps[],
+			       int slots, uint64_t pages_per_slot);
+unsigned long **memstress_alloc_bitmaps(int slots, uint64_t pages_per_slot);
+void memstress_free_bitmaps(unsigned long *bitmaps[], int slots);
+
 #endif /* SELFTEST_KVM_MEMSTRESS_H */
diff --git a/tools/testing/selftests/kvm/lib/memstress.c b/tools/testing/selftests/kvm/lib/memstress.c
index 5f1d3173c238c..3632956c6bcf5 100644
--- a/tools/testing/selftests/kvm/lib/memstress.c
+++ b/tools/testing/selftests/kvm/lib/memstress.c
@@ -5,6 +5,7 @@
 #define _GNU_SOURCE
 
 #include <inttypes.h>
+#include <linux/bitmap.h>
 
 #include "kvm_util.h"
 #include "memstress.h"
@@ -320,3 +321,74 @@ void memstress_join_vcpu_threads(int nr_vcpus)
 	for (i = 0; i < nr_vcpus; i++)
 		pthread_join(vcpu_threads[i].thread, NULL);
 }
+
+static void toggle_dirty_logging(struct kvm_vm *vm, int slots, bool enable)
+{
+	int i;
+
+	for (i = 0; i < slots; i++) {
+		int slot = MEMSTRESS_MEM_SLOT_INDEX + i;
+		int flags = enable ? KVM_MEM_LOG_DIRTY_PAGES : 0;
+
+		vm_mem_region_set_flags(vm, slot, flags);
+	}
+}
+
+void memstress_enable_dirty_logging(struct kvm_vm *vm, int slots)
+{
+	toggle_dirty_logging(vm, slots, true);
+}
+
+void memstress_disable_dirty_logging(struct kvm_vm *vm, int slots)
+{
+	toggle_dirty_logging(vm, slots, false);
+}
+
+void memstress_get_dirty_log(struct kvm_vm *vm, unsigned long *bitmaps[], int slots)
+{
+	int i;
+
+	for (i = 0; i < slots; i++) {
+		int slot = MEMSTRESS_MEM_SLOT_INDEX + i;
+
+		kvm_vm_get_dirty_log(vm, slot, bitmaps[i]);
+	}
+}
+
+void memstress_clear_dirty_log(struct kvm_vm *vm, unsigned long *bitmaps[],
+			       int slots, uint64_t pages_per_slot)
+{
+	int i;
+
+	for (i = 0; i < slots; i++) {
+		int slot = MEMSTRESS_MEM_SLOT_INDEX + i;
+
+		kvm_vm_clear_dirty_log(vm, slot, bitmaps[i], 0, pages_per_slot);
+	}
+}
+
+unsigned long **memstress_alloc_bitmaps(int slots, uint64_t pages_per_slot)
+{
+	unsigned long **bitmaps;
+	int i;
+
+	bitmaps = malloc(slots * sizeof(bitmaps[0]));
+	TEST_ASSERT(bitmaps, "Failed to allocate bitmaps array.");
+
+	for (i = 0; i < slots; i++) {
+		bitmaps[i] = bitmap_zalloc(pages_per_slot);
+		TEST_ASSERT(bitmaps[i], "Failed to allocate slot bitmap.");
+	}
+
+	return bitmaps;
+}
+
+void memstress_free_bitmaps(unsigned long *bitmaps[], int slots)
+{
+	int i;
+
+	for (i = 0; i < slots; i++)
+		free(bitmaps[i]);
+
+	free(bitmaps);
+}
-- 
2.39.1.456.gfc5497dd1b-goog

[PATCH V5 2/2] selftests: KVM: Add dirty logging page splitting test

[Ben Gardon]

Add a test for page splitting during dirty logging and for hugepage
recovery after dirty logging.

Page splitting represents non-trivial behavior, which is complicated
by MANUAL_PROTECT mode, which causes pages to be split on the first
clear, instead of when dirty logging is enabled.

Add a test which makes assertions about page counts to help define the
expected behavior of page splitting and to provide needed coverage of the
behavior. This also helps ensure that a failure in eager page splitting
is not covered up by splitting in the vCPU path.

Tested by running the test on an Intel Haswell machine w/wo
MANUAL_PROTECT.

Reviewed-by: Vipin Sharma <vipinsh@google.com>

Signed-off-by: Ben Gardon <bgardon@google.com>
---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/include/kvm_util_base.h     |   1 +
 tools/testing/selftests/kvm/lib/kvm_util.c    |   5 +
 .../x86_64/dirty_log_page_splitting_test.c    | 260 ++++++++++++++++++
 4 files changed, 267 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 1750f91dd9362..9a8022918c3ea 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -61,6 +61,7 @@ TEST_PROGS_x86_64 += x86_64/nx_huge_pages_test.sh
 # Compiled test targets
 TEST_GEN_PROGS_x86_64 = x86_64/cpuid_test
 TEST_GEN_PROGS_x86_64 += x86_64/cr4_cpuid_sync_test
+TEST_GEN_PROGS_x86_64 += x86_64/dirty_log_page_splitting_test
 TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features
 TEST_GEN_PROGS_x86_64 += x86_64/exit_on_emulation_failure_test
 TEST_GEN_PROGS_x86_64 += x86_64/fix_hypercall_test
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index fbc2a79369b8b..a089c356f354e 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -213,6 +213,7 @@ extern const struct vm_guest_mode_params vm_guest_mode_params[];
 int open_path_or_exit(const char *path, int flags);
 int open_kvm_dev_path_or_exit(void);
 
+bool get_kvm_param_bool(const char *param);
 bool get_kvm_intel_param_bool(const char *param);
 bool get_kvm_amd_param_bool(const char *param);
 
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 56d5ea949cbbe..fa6d69f731990 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -80,6 +80,11 @@ static bool get_module_param_bool(const char *module_name, const char *param)
 	TEST_FAIL("Unrecognized value '%c' for boolean module param", value);
 }
 
+bool get_kvm_param_bool(const char *param)
+{
+	return get_module_param_bool("kvm", param);
+}
+
 bool get_kvm_intel_param_bool(const char *param)
 {
 	return get_module_param_bool("kvm_intel", param);
diff --git a/tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test.c b/tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test.c
new file mode 100644
index 0000000000000..ed08c7ac3b2e3
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test.c
@@ -0,0 +1,260 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KVM dirty logging page splitting test
+ *
+ * Based on dirty_log_perf.c
+ *
+ * Copyright (C) 2018, Red Hat, Inc.
+ * Copyright (C) 2023, Google, Inc.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <pthread.h>
+#include <linux/bitmap.h>
+
+#include "kvm_util.h"
+#include "test_util.h"
+#include "memstress.h"
+#include "guest_modes.h"
+
+#define VCPUS		2
+#define SLOTS		2
+#define ITERATIONS	2
+
+static uint64_t guest_percpu_mem_size = DEFAULT_PER_VCPU_MEM_SIZE;
+
+static enum vm_mem_backing_src_type backing_src = VM_MEM_SRC_ANONYMOUS_HUGETLB;
+
+static u64 dirty_log_manual_caps;
+static bool host_quit;
+static int iteration;
+static int vcpu_last_completed_iteration[KVM_MAX_VCPUS];
+
+struct kvm_page_stats {
+	uint64_t pages_4k;
+	uint64_t pages_2m;
+	uint64_t pages_1g;
+	uint64_t hugepages;
+};
+
+static void get_page_stats(struct kvm_vm *vm, struct kvm_page_stats *stats, const char *stage)
+{
+	stats->pages_4k = vm_get_stat(vm, "pages_4k");
+	stats->pages_2m = vm_get_stat(vm, "pages_2m");
+	stats->pages_1g = vm_get_stat(vm, "pages_1g");
+	stats->hugepages = stats->pages_2m + stats->pages_1g;
+
+	pr_debug("\nPage stats after %s: 4K: %ld 2M: %ld 1G: %ld huge: %ld\n",
+		 stage, stats->pages_4k, stats->pages_2m, stats->pages_1g,
+		 stats->hugepages);
+}
+
+static void run_vcpu_iteration(struct kvm_vm *vm)
+{
+	int i;
+
+	iteration++;
+	for (i = 0; i < VCPUS; i++) {
+		while (READ_ONCE(vcpu_last_completed_iteration[i]) !=
+		       iteration)
+			;
+	}
+}
+
+static void vcpu_worker(struct memstress_vcpu_args *vcpu_args)
+{
+	struct kvm_vcpu *vcpu = vcpu_args->vcpu;
+	int vcpu_idx = vcpu_args->vcpu_idx;
+
+	while (!READ_ONCE(host_quit)) {
+		int current_iteration = READ_ONCE(iteration);
+
+		vcpu_run(vcpu);
+
+		ASSERT_EQ(get_ucall(vcpu, NULL), UCALL_SYNC);
+
+		vcpu_last_completed_iteration[vcpu_idx] = current_iteration;
+
+		/* Wait for the start of the next iteration to be signaled. */
+		while (current_iteration == READ_ONCE(iteration) &&
+		       READ_ONCE(iteration) >= 0 &&
+		       !READ_ONCE(host_quit))
+			;
+	}
+}
+
+static void run_test(enum vm_guest_mode mode, void *unused)
+{
+	struct kvm_vm *vm;
+	unsigned long **bitmaps;
+	uint64_t guest_num_pages;
+	uint64_t host_num_pages;
+	uint64_t pages_per_slot;
+	int i;
+	uint64_t total_4k_pages;
+	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);
+
+	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;
+
+	bitmaps = memstress_alloc_bitmaps(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 = -1;
+	host_quit = false;
+
+	for (i = 0; i < VCPUS; i++)
+		vcpu_last_completed_iteration[i] = -1;
+
+	memstress_start_vcpu_threads(VCPUS, vcpu_worker);
+
+	run_vcpu_iteration(vm);
+	get_page_stats(vm, &stats_populated, "populating memory");
+
+	/* Enable dirty logging */
+	memstress_enable_dirty_logging(vm, SLOTS);
+
+	get_page_stats(vm, &stats_dirty_logging_enabled, "enabling dirty logging");
+
+	while (iteration < ITERATIONS) {
+		run_vcpu_iteration(vm);
+		get_page_stats(vm, &stats_dirty_pass[iteration - 1],
+			       "dirtying memory");
+
+		memstress_get_dirty_log(vm, bitmaps, SLOTS);
+
+		if (dirty_log_manual_caps) {
+			memstress_clear_dirty_log(vm, bitmaps, SLOTS, pages_per_slot);
+
+			get_page_stats(vm, &stats_clear_pass[iteration - 1], "clearing dirty log");
+		}
+	}
+
+	/* Disable dirty logging */
+	memstress_disable_dirty_logging(vm, SLOTS);
+
+	get_page_stats(vm, &stats_dirty_logging_disabled, "disabling dirty logging");
+
+	/* Run vCPUs again to fault pages back in. */
+	run_vcpu_iteration(vm);
+	get_page_stats(vm, &stats_repopulated, "repopulating memory");
+
+	/*
+	 * 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(VCPUS);
+
+	memstress_free_bitmaps(bitmaps, SLOTS);
+	memstress_destroy_vm(vm);
+
+	/* Make assertions about the page counts. */
+	total_4k_pages = stats_populated.pages_4k;
+	total_4k_pages += stats_populated.pages_2m * 512;
+	total_4k_pages += stats_populated.pages_1g * 512 * 512;
+
+	/*
+	 * Check that all huge pages were split. Since large pages can only
+	 * exist in the data slot, and the vCPUs should have dirtied all pages
+	 * in the data slot, there should be no huge pages left after splitting.
+	 * Splitting happens at dirty log enable time without
+	 * KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 and after the first clear pass
+	 * with that capability.
+	 */
+	if (dirty_log_manual_caps) {
+		ASSERT_EQ(stats_clear_pass[0].hugepages, 0);
+		ASSERT_EQ(stats_clear_pass[0].pages_4k, total_4k_pages);
+		ASSERT_EQ(stats_dirty_logging_enabled.hugepages, stats_populated.hugepages);
+	} else {
+		ASSERT_EQ(stats_dirty_logging_enabled.hugepages, 0);
+		ASSERT_EQ(stats_dirty_logging_enabled.pages_4k, total_4k_pages);
+	}
+
+	/*
+	 * Once dirty logging is disabled and the vCPUs have touched all their
+	 * memory again, the page counts should be the same as they were
+	 * right after initial population of memory.
+	 */
+	ASSERT_EQ(stats_populated.pages_4k, stats_repopulated.pages_4k);
+	ASSERT_EQ(stats_populated.pages_2m, stats_repopulated.pages_2m);
+	ASSERT_EQ(stats_populated.pages_1g, stats_repopulated.pages_1g);
+}
+
+static void help(char *name)
+{
+	puts("");
+	printf("usage: %s [-h] [-b vcpu bytes] [-s mem type]\n",
+	       name);
+	puts("");
+	printf(" -b: specify the size of the memory region which should be\n"
+	       "     dirtied by each vCPU. e.g. 10M or 3G.\n"
+	       "     (default: 1G)\n");
+	backing_src_help("-s");
+	puts("");
+}
+
+int main(int argc, char *argv[])
+{
+	int opt;
+
+	TEST_REQUIRE(get_kvm_param_bool("eager_page_split"));
+	TEST_REQUIRE(get_kvm_param_bool("tdp_mmu"));
+
+	while ((opt = getopt(argc, argv, "b:hs:")) != -1) {
+		switch (opt) {
+		case 'b':
+			guest_percpu_mem_size = parse_size(optarg);
+			break;
+		case 'h':
+			help(argv[0]);
+			exit(0);
+		case 's':
+			backing_src = parse_backing_src_type(optarg);
+			break;
+		default:
+			help(argv[0]);
+			exit(1);
+		}
+	}
+
+	if (!is_backing_src_hugetlb(backing_src)) {
+		pr_info("This test will only work reliably with HugeTLB memory. "
+			"It can work with THP, but that is best effort.");
+		return KSFT_SKIP;
+	}
+
+	guest_modes_append_default();
+
+	dirty_log_manual_caps = 0;
+	for_each_guest_mode(run_test, NULL);
+
+	dirty_log_manual_caps =
+		kvm_check_cap(KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2);
+
+	if (dirty_log_manual_caps) {
+		dirty_log_manual_caps &= (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE |
+					  KVM_DIRTY_LOG_INITIALLY_SET);
+		for_each_guest_mode(run_test, NULL);
+	} else {
+		pr_info("Skipping testing with MANUAL_PROTECT as it is not supported");
+	}
+
+	return 0;
+}
-- 
2.39.1.456.gfc5497dd1b-goog

Re: [PATCH V5 0/2] selftests: KVM: Add a test for eager page splitting

[Paolo Bonzini]

On Tue, Jan 31, 2023 at 7:18 PM Ben Gardon <bgardon@google.com> wrote:
>
> David Matlack recently added a feature known as eager page splitting
> to x86 KVM. This feature improves vCPU performance during dirty
> logging because the splitting operation is moved out of the page
> fault path, avoiding EPT/NPT violations or allowing the vCPU threads
> to resolve the violation in the fast path.
>
> While this feature is a great performance improvement, it does not
> have adequate testing in KVM selftests. Add a test to provide coverage
> of eager page splitting.
>
> Patch 1 is a quick refactor to be able to re-use some code from
> dirty_log_perf_test.
> Patch 2 adds the actual test.

I have finally queued it, but made a small change to allow running it
with non-hugetlbfs page types.

Also, see the "KVM: selftests: skip hugetlb tests if huge pages are
not available" patch I have just sent, which makes the test not fail
in a default kernel configuration.

Thanks,

Paolo

Re: [PATCH V5 0/2] selftests: KVM: Add a test for eager page splitting

[Paolo Bonzini]

On Tue, Mar 14, 2023 at 2:27 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> I have finally queued it, but made a small change to allow running it
> with non-hugetlbfs page types.

Oops, it fails on my AMD workstation:

$ tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test
Testing guest mode: PA-bits:ANY, VA-bits:48,  4K pages
guest physical test memory: [0x7fc7fe00000, 0x7fcffe00000)
==== Test Assertion Failure ====
  x86_64/dirty_log_page_splitting_test.c:195: __a == __b
  pid=1378203 tid=1378203 errno=0 - Success
     1    0x0000000000402d02: run_test at dirty_log_page_splitting_test.c:195
     2    0x000000000040367c: for_each_guest_mode at guest_modes.c:100
     3    0x00000000004024df: main at dirty_log_page_splitting_test.c:245
     4    0x00007f4227c3feaf: ?? ??:0
     5    0x00007f4227c3ff5f: ?? ??:0
     6    0x0000000000402594: _start at ??:?
  ASSERT_EQ(stats_populated.pages_4k, stats_repopulated.pages_4k) failed.
    stats_populated.pages_4k is 0x413
    stats_repopulated.pages_4k is 0x412

Haven't debugged it yet.

Paolo

Re: [PATCH V5 0/2] selftests: KVM: Add a test for eager page splitting

[David Matlack]

On Tue, Mar 14, 2023 at 7:23 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On Tue, Mar 14, 2023 at 2:27 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> > I have finally queued it, but made a small change to allow running it
> > with non-hugetlbfs page types.
>
> Oops, it fails on my AMD workstation:
>
> $ tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test
> Testing guest mode: PA-bits:ANY, VA-bits:48,  4K pages
> guest physical test memory: [0x7fc7fe00000, 0x7fcffe00000)
> ==== Test Assertion Failure ====
>   x86_64/dirty_log_page_splitting_test.c:195: __a == __b
>   pid=1378203 tid=1378203 errno=0 - Success
>      1    0x0000000000402d02: run_test at dirty_log_page_splitting_test.c:195
>      2    0x000000000040367c: for_each_guest_mode at guest_modes.c:100
>      3    0x00000000004024df: main at dirty_log_page_splitting_test.c:245
>      4    0x00007f4227c3feaf: ?? ??:0
>      5    0x00007f4227c3ff5f: ?? ??:0
>      6    0x0000000000402594: _start at ??:?
>   ASSERT_EQ(stats_populated.pages_4k, stats_repopulated.pages_4k) failed.
>     stats_populated.pages_4k is 0x413
>     stats_repopulated.pages_4k is 0x412
>
> Haven't debugged it yet.

I wonder if pages are getting swapped, especially if running on a
workstation. If so, mlock()ing all guest memory VMAs might be
necessary to be able to assert exact page counts.

>
> Paolo
>

Re: [PATCH V5 0/2] selftests: KVM: Add a test for eager page splitting

[Paolo Bonzini]

On Tue, Mar 14, 2023 at 5:00 PM David Matlack <dmatlack@google.com> wrote:
> On Tue, Mar 14, 2023 at 7:23 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> > $ tools/testing/selftests/kvm/x86_64/dirty_log_page_splitting_test
> > Testing guest mode: PA-bits:ANY, VA-bits:48,  4K pages
> > guest physical test memory: [0x7fc7fe00000, 0x7fcffe00000)
> > ==== Test Assertion Failure ====
> >   x86_64/dirty_log_page_splitting_test.c:195: __a == __b
> >   pid=1378203 tid=1378203 errno=0 - Success
> >      1    0x0000000000402d02: run_test at dirty_log_page_splitting_test.c:195
> >      2    0x000000000040367c: for_each_guest_mode at guest_modes.c:100
> >      3    0x00000000004024df: main at dirty_log_page_splitting_test.c:245
> >      4    0x00007f4227c3feaf: ?? ??:0
> >      5    0x00007f4227c3ff5f: ?? ??:0
> >      6    0x0000000000402594: _start at ??:?
> >   ASSERT_EQ(stats_populated.pages_4k, stats_repopulated.pages_4k) failed.
> >     stats_populated.pages_4k is 0x413
> >     stats_repopulated.pages_4k is 0x412
>
> I wonder if pages are getting swapped, especially if running on a
> workstation. If so, mlock()ing all guest memory VMAs might be
> necessary to be able to assert exact page counts.

I don't think so, it's 100% reproducible and the machine is idle and
only accessed via network. Also has 64 GB of RAM. :)

Paolo

Re: [PATCH V5 0/2] selftests: KVM: Add a test for eager page splitting

[Paolo Bonzini]

On 3/15/23 13:24, Paolo Bonzini wrote:
> On Tue, Mar 14, 2023 at 5:00 PM David Matlack <dmatlack@google.com> wrote:
>> I wonder if pages are getting swapped, especially if running on a
>> workstation. If so, mlock()ing all guest memory VMAs might be
>> necessary to be able to assert exact page counts.
> 
> I don't think so, it's 100% reproducible and the machine is idle and
> only accessed via network. Also has 64 GB of RAM. :)

It also reproduces on Intel with pml=0 and eptad=0; the reason is due
to the different semantics of dirty bits for page-table pages on AMD
and Intel.  Both AMD and eptad=0 Intel treat those as writes, therefore
more pages are dropped before the repopulation phase when dirty logging
is disabled.

The "missing" page had been included in the population phase because it
hosts the page tables for vcpu_args, but repopulation does not need it.

This fixes it:

-------------------- 8< ---------------
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH] selftests: KVM: perform the same memory accesses on every memstress iteration

Perform the same memory accesses including the initialization steps
that read from args and vcpu_args.  This ensures that the state of
KVM's page tables is the same after every iteration, including the
pages that host the guest page tables for args and vcpu_args.

This fixes a failure of dirty_log_page_splitting_test on AMD machines,
as well as on Intel if PML and EPT A/D bits are both disabled.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/tools/testing/selftests/kvm/lib/memstress.c b/tools/testing/selftests/kvm/lib/memstress.c
index 3632956c6bcf..8a429f4c86db 100644
--- a/tools/testing/selftests/kvm/lib/memstress.c
+++ b/tools/testing/selftests/kvm/lib/memstress.c
@@ -56,15 +56,15 @@ void memstress_guest_code(uint32_t vcpu_idx)
  	uint64_t page;
  	int i;
  
-	rand_state = new_guest_random_state(args->random_seed + vcpu_idx);
+	while (true) {
+		rand_state = new_guest_random_state(args->random_seed + vcpu_idx);
  
-	gva = vcpu_args->gva;
-	pages = vcpu_args->pages;
+		gva = vcpu_args->gva;
+		pages = vcpu_args->pages;
  
-	/* Make sure vCPU args data structure is not corrupt. */
-	GUEST_ASSERT(vcpu_args->vcpu_idx == vcpu_idx);
+		/* Make sure vCPU args data structure is not corrupt. */
+		GUEST_ASSERT(vcpu_args->vcpu_idx == vcpu_idx);
  
-	while (true) {
  		for (i = 0; i < pages; i++) {
  			if (args->random_access)
  				page = guest_random_u32(&rand_state) % pages;

Paolo

Re: [PATCH V5 0/2] selftests: KVM: Add a test for eager page splitting

[Sean Christopherson]

On Wed, Mar 15, 2023, Paolo Bonzini wrote:
> On 3/15/23 13:24, Paolo Bonzini wrote:
> > On Tue, Mar 14, 2023 at 5:00 PM David Matlack <dmatlack@google.com> wrote:
> > > I wonder if pages are getting swapped, especially if running on a
> > > workstation. If so, mlock()ing all guest memory VMAs might be
> > > necessary to be able to assert exact page counts.
> > 
> > I don't think so, it's 100% reproducible and the machine is idle and
> > only accessed via network. Also has 64 GB of RAM. :)
> 
> It also reproduces on Intel with pml=0 and eptad=0; the reason is due
> to the different semantics of dirty bits for page-table pages on AMD
> and Intel.  Both AMD and eptad=0 Intel treat those as writes, therefore
> more pages are dropped before the repopulation phase when dirty logging
> is disabled.
> 
> The "missing" page had been included in the population phase because it
> hosts the page tables for vcpu_args, but repopulation does not need it.
> 
> This fixes it:
> 
> -------------------- 8< ---------------
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH] selftests: KVM: perform the same memory accesses on every memstress iteration
> 
> Perform the same memory accesses including the initialization steps
> that read from args and vcpu_args.  This ensures that the state of
> KVM's page tables is the same after every iteration, including the
> pages that host the guest page tables for args and vcpu_args.
> 
> This fixes a failure of dirty_log_page_splitting_test on AMD machines,
> as well as on Intel if PML and EPT A/D bits are both disabled.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> diff --git a/tools/testing/selftests/kvm/lib/memstress.c b/tools/testing/selftests/kvm/lib/memstress.c
> index 3632956c6bcf..8a429f4c86db 100644
> --- a/tools/testing/selftests/kvm/lib/memstress.c
> +++ b/tools/testing/selftests/kvm/lib/memstress.c
> @@ -56,15 +56,15 @@ void memstress_guest_code(uint32_t vcpu_idx)
>  	uint64_t page;
>  	int i;
> -	rand_state = new_guest_random_state(args->random_seed + vcpu_idx);
> +	while (true) {
> +		rand_state = new_guest_random_state(args->random_seed + vcpu_idx);

Doesn't this partially defeat the randomization that some tests like want?  E.g.
a test that wants to heavily randomize state will get the same pRNG for every
iteration.  Seems like we should have a knob to control whether or not each
iteration needs to be identical.

Re: [PATCH V5 0/2] selftests: KVM: Add a test for eager page splitting

[Paolo Bonzini]

On 3/15/23 20:22, Sean Christopherson wrote:
> On Wed, Mar 15, 2023, Paolo Bonzini wrote:
>> On 3/15/23 13:24, Paolo Bonzini wrote:
>>> On Tue, Mar 14, 2023 at 5:00 PM David Matlack <dmatlack@google.com> wrote:
>>>> I wonder if pages are getting swapped, especially if running on a
>>>> workstation. If so, mlock()ing all guest memory VMAs might be
>>>> necessary to be able to assert exact page counts.
>>>
>>> I don't think so, it's 100% reproducible and the machine is idle and
>>> only accessed via network. Also has 64 GB of RAM. :)
>>
>> It also reproduces on Intel with pml=0 and eptad=0; the reason is due
>> to the different semantics of dirty bits for page-table pages on AMD
>> and Intel.  Both AMD and eptad=0 Intel treat those as writes, therefore
>> more pages are dropped before the repopulation phase when dirty logging
>> is disabled.
>>
>> The "missing" page had been included in the population phase because it
>> hosts the page tables for vcpu_args, but repopulation does not need it.
>>
>> This fixes it:
>>
>> -------------------- 8< ---------------
>> From: Paolo Bonzini <pbonzini@redhat.com>
>> Subject: [PATCH] selftests: KVM: perform the same memory accesses on every memstress iteration
>>
>> Perform the same memory accesses including the initialization steps
>> that read from args and vcpu_args.  This ensures that the state of
>> KVM's page tables is the same after every iteration, including the
>> pages that host the guest page tables for args and vcpu_args.
>>
>> This fixes a failure of dirty_log_page_splitting_test on AMD machines,
>> as well as on Intel if PML and EPT A/D bits are both disabled.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>
>> diff --git a/tools/testing/selftests/kvm/lib/memstress.c b/tools/testing/selftests/kvm/lib/memstress.c
>> index 3632956c6bcf..8a429f4c86db 100644
>> --- a/tools/testing/selftests/kvm/lib/memstress.c
>> +++ b/tools/testing/selftests/kvm/lib/memstress.c
>> @@ -56,15 +56,15 @@ void memstress_guest_code(uint32_t vcpu_idx)
>>   	uint64_t page;
>>   	int i;
>> -	rand_state = new_guest_random_state(args->random_seed + vcpu_idx);
>> +	while (true) {
>> +		rand_state = new_guest_random_state(args->random_seed + vcpu_idx);
> 
> Doesn't this partially defeat the randomization that some tests like want?  E.g.
> a test that wants to heavily randomize state will get the same pRNG for every
> iteration.  Seems like we should have a knob to control whether or not each
> iteration needs to be identical.

Yes, this wasn't really a full patch, just to prove what the bug is.

One possibility to avoid adding a new knob is to do something like:

unsigned iteration = 0;
rand_state = new_guest_random_state(args->random_seed
	+ vcpu_idx + iteration++);

Paolo

Re: [PATCH V5 0/2] selftests: KVM: Add a test for eager page splitting

[Sean Christopherson]

On Tue, 31 Jan 2023 18:18:18 +0000, Ben Gardon wrote:
> David Matlack recently added a feature known as eager page splitting
> to x86 KVM. This feature improves vCPU performance during dirty
> logging because the splitting operation is moved out of the page
> fault path, avoiding EPT/NPT violations or allowing the vCPU threads
> to resolve the violation in the fast path.
> 
> While this feature is a great performance improvement, it does not
> have adequate testing in KVM selftests. Add a test to provide coverage
> of eager page splitting.
> 
> [...]

Applied to kvm-x86 selftests, thanks!

[1/2] selftests: KVM: Move dirty logging functions to memstress.(c|h)
      https://github.com/kvm-x86/linux/commit/de10b798055d
[2/2] selftests: KVM: Add dirty logging page splitting test
      https://github.com/kvm-x86/linux/commit/dfa78a20cc87

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes