All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] KVM: selftests: Cleanups, take 2
@ 2020-11-10 20:47 Andrew Jones
  2020-11-10 20:47 ` [PATCH 1/8] KVM: selftests: Update .gitignore Andrew Jones
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Andrew Jones @ 2020-11-10 20:47 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, borntraeger, frankja, bgardon, peterx

This series attempts to clean up demand_paging_test, dirty_log_perf_test,
and dirty_log_test by factoring out common code, creating some new API
along the way.  It also splits include/perf_test_util.h into a more
conventional header and source pair. There's still some stuff I don't
like, for example the unbalanced ucall_uninit we now get for tests using
the perf_test API, but I'll maybe revisit that stuff again some other day.

I've tested on x86 and AArch64 (one config each), but not s390x.

Thanks,
drew


Andrew Jones (8):
  KVM: selftests: Update .gitignore
  KVM: selftests: Remove deadcode
  KVM: selftests: Factor out guest mode code
  KVM: selftests: Make vm_create_default common
  KVM: selftests: Introduce vm_create_[default_]_with_vcpus
  KVM: selftests: dirty_log_test: Remove create_vm
  KVM: selftests: Use vm_create_with_vcpus in create_vm
  KVM: selftests: Implement perf_test_util more conventionally

 tools/testing/selftests/kvm/.gitignore        |   2 +-
 tools/testing/selftests/kvm/Makefile          |   2 +-
 .../selftests/kvm/demand_paging_test.c        | 115 +++--------
 .../selftests/kvm/dirty_log_perf_test.c       | 174 ++++-------------
 tools/testing/selftests/kvm/dirty_log_test.c  | 179 +++++-------------
 .../selftests/kvm/include/guest_modes.h       |  21 ++
 .../testing/selftests/kvm/include/kvm_util.h  |  42 +++-
 .../selftests/kvm/include/perf_test_util.h    | 170 +----------------
 .../selftests/kvm/lib/aarch64/processor.c     |  17 --
 tools/testing/selftests/kvm/lib/guest_modes.c |  70 +++++++
 tools/testing/selftests/kvm/lib/kvm_util.c    |  60 +++++-
 .../selftests/kvm/lib/perf_test_util.c        | 134 +++++++++++++
 .../selftests/kvm/lib/s390x/processor.c       |  22 ---
 .../selftests/kvm/lib/x86_64/processor.c      |  32 ----
 14 files changed, 451 insertions(+), 589 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/include/guest_modes.h
 create mode 100644 tools/testing/selftests/kvm/lib/guest_modes.c
 create mode 100644 tools/testing/selftests/kvm/lib/perf_test_util.c

-- 
2.26.2


^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH 1/8] KVM: selftests: Update .gitignore
  2020-11-10 20:47 [PATCH 0/8] KVM: selftests: Cleanups, take 2 Andrew Jones
@ 2020-11-10 20:47 ` Andrew Jones
  2020-11-10 21:59   ` Ben Gardon
  2020-11-10 20:47 ` [PATCH 2/8] KVM: selftests: Remove deadcode Andrew Jones
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Andrew Jones @ 2020-11-10 20:47 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, borntraeger, frankja, bgardon, peterx

Add x86_64/tsc_msrs_test and remove clear_dirty_log_test.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 tools/testing/selftests/kvm/.gitignore | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 7a2c242b7152..ceff9f4c1781 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -18,13 +18,13 @@
 /x86_64/vmx_preemption_timer_test
 /x86_64/svm_vmcall_test
 /x86_64/sync_regs_test
+/x86_64/tsc_msrs_test
 /x86_64/vmx_apic_access_test
 /x86_64/vmx_close_while_nested_test
 /x86_64/vmx_dirty_log_test
 /x86_64/vmx_set_nested_state_test
 /x86_64/vmx_tsc_adjust_test
 /x86_64/xss_msr_test
-/clear_dirty_log_test
 /demand_paging_test
 /dirty_log_test
 /dirty_log_perf_test
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 2/8] KVM: selftests: Remove deadcode
  2020-11-10 20:47 [PATCH 0/8] KVM: selftests: Cleanups, take 2 Andrew Jones
  2020-11-10 20:47 ` [PATCH 1/8] KVM: selftests: Update .gitignore Andrew Jones
@ 2020-11-10 20:47 ` Andrew Jones
  2020-11-10 21:47   ` Ben Gardon
  2020-11-10 20:47 ` [PATCH 3/8] KVM: selftests: Factor out guest mode code Andrew Jones
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Andrew Jones @ 2020-11-10 20:47 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, borntraeger, frankja, bgardon, peterx

Nothing sets USE_CLEAR_DIRTY_LOG anymore, so anything it surrounds
is dead code.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 .../selftests/kvm/dirty_log_perf_test.c       | 44 -------------------
 1 file changed, 44 deletions(-)

diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 85c9b8f73142..b9115e8ef0ed 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -88,10 +88,6 @@ static void *vcpu_worker(void *data)
 	return NULL;
 }
 
-#ifdef USE_CLEAR_DIRTY_LOG
-static u64 dirty_log_manual_caps;
-#endif
-
 static void run_test(enum vm_guest_mode mode, unsigned long iterations,
 		     uint64_t phys_offset, int wr_fract)
 {
@@ -106,10 +102,6 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
 	struct timespec get_dirty_log_total = (struct timespec){0};
 	struct timespec vcpu_dirty_total = (struct timespec){0};
 	struct timespec avg;
-#ifdef USE_CLEAR_DIRTY_LOG
-	struct kvm_enable_cap cap = {};
-	struct timespec clear_dirty_log_total = (struct timespec){0};
-#endif
 
 	vm = create_vm(mode, nr_vcpus, guest_percpu_mem_size);
 
@@ -120,12 +112,6 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
 	host_num_pages = vm_num_host_pages(mode, guest_num_pages);
 	bmap = bitmap_alloc(host_num_pages);
 
-#ifdef USE_CLEAR_DIRTY_LOG
-	cap.cap = KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2;
-	cap.args[0] = dirty_log_manual_caps;
-	vm_enable_cap(vm, &cap);
-#endif
-
 	vcpu_threads = malloc(nr_vcpus * sizeof(*vcpu_threads));
 	TEST_ASSERT(vcpu_threads, "Memory allocation failed");
 
@@ -189,18 +175,6 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
 						   ts_diff);
 		pr_info("Iteration %lu get dirty log time: %ld.%.9lds\n",
 			iteration, ts_diff.tv_sec, ts_diff.tv_nsec);
-
-#ifdef USE_CLEAR_DIRTY_LOG
-		clock_gettime(CLOCK_MONOTONIC, &start);
-		kvm_vm_clear_dirty_log(vm, TEST_MEM_SLOT_INDEX, bmap, 0,
-				       host_num_pages);
-
-		ts_diff = timespec_diff_now(start);
-		clear_dirty_log_total = timespec_add(clear_dirty_log_total,
-						     ts_diff);
-		pr_info("Iteration %lu clear dirty log time: %ld.%.9lds\n",
-			iteration, ts_diff.tv_sec, ts_diff.tv_nsec);
-#endif
 	}
 
 	/* Tell the vcpu thread to quit */
@@ -220,13 +194,6 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
 		iterations, get_dirty_log_total.tv_sec,
 		get_dirty_log_total.tv_nsec, avg.tv_sec, avg.tv_nsec);
 
-#ifdef USE_CLEAR_DIRTY_LOG
-	avg = timespec_div(clear_dirty_log_total, iterations);
-	pr_info("Clear dirty log over %lu iterations took %ld.%.9lds. (Avg %ld.%.9lds/iteration)\n",
-		iterations, clear_dirty_log_total.tv_sec,
-		clear_dirty_log_total.tv_nsec, avg.tv_sec, avg.tv_nsec);
-#endif
-
 	free(bmap);
 	free(vcpu_threads);
 	ucall_uninit(vm);
@@ -284,17 +251,6 @@ int main(int argc, char *argv[])
 	int opt, i;
 	int wr_fract = 1;
 
-#ifdef USE_CLEAR_DIRTY_LOG
-	dirty_log_manual_caps =
-		kvm_check_cap(KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2);
-	if (!dirty_log_manual_caps) {
-		print_skip("KVM_CLEAR_DIRTY_LOG not available");
-		exit(KSFT_SKIP);
-	}
-	dirty_log_manual_caps &= (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE |
-				  KVM_DIRTY_LOG_INITIALLY_SET);
-#endif
-
 #ifdef __x86_64__
 	guest_mode_init(VM_MODE_PXXV48_4K, true, true);
 #endif
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 3/8] KVM: selftests: Factor out guest mode code
  2020-11-10 20:47 [PATCH 0/8] KVM: selftests: Cleanups, take 2 Andrew Jones
  2020-11-10 20:47 ` [PATCH 1/8] KVM: selftests: Update .gitignore Andrew Jones
  2020-11-10 20:47 ` [PATCH 2/8] KVM: selftests: Remove deadcode Andrew Jones
@ 2020-11-10 20:47 ` Andrew Jones
  2020-11-10 21:52   ` Ben Gardon
  2020-11-10 20:47 ` [PATCH 4/8] KVM: selftests: Make vm_create_default common Andrew Jones
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Andrew Jones @ 2020-11-10 20:47 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, borntraeger, frankja, bgardon, peterx

demand_paging_test, dirty_log_test, and dirty_log_perf_test have
redundant guest mode code. Factor it out.

Also, while adding a new include, remove the ones we don't need.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 tools/testing/selftests/kvm/Makefile          |   2 +-
 .../selftests/kvm/demand_paging_test.c        | 107 ++++-----------
 .../selftests/kvm/dirty_log_perf_test.c       | 119 +++++------------
 tools/testing/selftests/kvm/dirty_log_test.c  | 125 ++++++------------
 .../selftests/kvm/include/guest_modes.h       |  21 +++
 tools/testing/selftests/kvm/lib/guest_modes.c |  70 ++++++++++
 6 files changed, 188 insertions(+), 256 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/include/guest_modes.h
 create mode 100644 tools/testing/selftests/kvm/lib/guest_modes.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 3d14ef77755e..ca6b64d9ab64 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -33,7 +33,7 @@ ifeq ($(ARCH),s390)
 	UNAME_M := s390x
 endif
 
-LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c lib/test_util.c
+LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c
 LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S
 LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
 LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 3d96a7bfaff3..946161a9ce2d 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -7,23 +7,20 @@
  * Copyright (C) 2019, Google, Inc.
  */
 
-#define _GNU_SOURCE /* for program_invocation_name */
+#define _GNU_SOURCE /* for program_invocation_name and pipe2 */
 
 #include <stdio.h>
 #include <stdlib.h>
-#include <sys/syscall.h>
-#include <unistd.h>
-#include <asm/unistd.h>
 #include <time.h>
 #include <poll.h>
 #include <pthread.h>
-#include <linux/bitmap.h>
-#include <linux/bitops.h>
 #include <linux/userfaultfd.h>
+#include <sys/syscall.h>
 
-#include "perf_test_util.h"
-#include "processor.h"
+#include "kvm_util.h"
 #include "test_util.h"
+#include "perf_test_util.h"
+#include "guest_modes.h"
 
 #ifdef __NR_userfaultfd
 
@@ -248,9 +245,14 @@ static int setup_demand_paging(struct kvm_vm *vm,
 	return 0;
 }
 
-static void run_test(enum vm_guest_mode mode, bool use_uffd,
-		     useconds_t uffd_delay)
+struct test_params {
+	bool use_uffd;
+	useconds_t uffd_delay;
+};
+
+static void run_test(enum vm_guest_mode mode, void *arg)
 {
+	struct test_params *p = arg;
 	pthread_t *vcpu_threads;
 	pthread_t *uffd_handler_threads = NULL;
 	struct uffd_handler_args *uffd_args = NULL;
@@ -275,7 +277,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 
 	add_vcpus(vm, nr_vcpus, guest_percpu_mem_size);
 
-	if (use_uffd) {
+	if (p->use_uffd) {
 		uffd_handler_threads =
 			malloc(nr_vcpus * sizeof(*uffd_handler_threads));
 		TEST_ASSERT(uffd_handler_threads, "Memory allocation failed");
@@ -308,7 +310,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 			r = setup_demand_paging(vm,
 						&uffd_handler_threads[vcpu_id],
 						pipefds[vcpu_id * 2],
-						uffd_delay, &uffd_args[vcpu_id],
+						p->uffd_delay, &uffd_args[vcpu_id],
 						vcpu_hva, guest_percpu_mem_size);
 			if (r < 0)
 				exit(-r);
@@ -339,7 +341,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 
 	pr_info("All vCPU threads joined\n");
 
-	if (use_uffd) {
+	if (p->use_uffd) {
 		char c;
 
 		/* Tell the user fault fd handler threads to quit */
@@ -362,38 +364,19 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 
 	free(guest_data_prototype);
 	free(vcpu_threads);
-	if (use_uffd) {
+	if (p->use_uffd) {
 		free(uffd_handler_threads);
 		free(uffd_args);
 		free(pipefds);
 	}
 }
 
-struct guest_mode {
-	bool supported;
-	bool enabled;
-};
-static struct guest_mode guest_modes[NUM_VM_MODES];
-
-#define guest_mode_init(mode, supported, enabled) ({ \
-	guest_modes[mode] = (struct guest_mode){ supported, enabled }; \
-})
-
 static void help(char *name)
 {
-	int i;
-
 	puts("");
 	printf("usage: %s [-h] [-m mode] [-u] [-d uffd_delay_usec]\n"
 	       "          [-b memory] [-v vcpus]\n", name);
-	printf(" -m: specify the guest mode ID to test\n"
-	       "     (default: test all supported modes)\n"
-	       "     This option may be used multiple times.\n"
-	       "     Guest mode IDs:\n");
-	for (i = 0; i < NUM_VM_MODES; ++i) {
-		printf("         %d:    %s%s\n", i, vm_guest_mode_string(i),
-		       guest_modes[i].supported ? " (supported)" : "");
-	}
+	guest_modes_help();
 	printf(" -u: use User Fault FD to handle vCPU page\n"
 	       "     faults.\n");
 	printf(" -d: add a delay in usec to the User Fault\n"
@@ -410,53 +393,22 @@ static void help(char *name)
 int main(int argc, char *argv[])
 {
 	int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
-	bool mode_selected = false;
-	unsigned int mode;
-	int opt, i;
-	bool use_uffd = false;
-	useconds_t uffd_delay = 0;
-
-#ifdef __x86_64__
-	guest_mode_init(VM_MODE_PXXV48_4K, true, true);
-#endif
-#ifdef __aarch64__
-	guest_mode_init(VM_MODE_P40V48_4K, true, true);
-	guest_mode_init(VM_MODE_P40V48_64K, true, true);
-	{
-		unsigned int limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
-
-		if (limit >= 52)
-			guest_mode_init(VM_MODE_P52V48_64K, true, true);
-		if (limit >= 48) {
-			guest_mode_init(VM_MODE_P48V48_4K, true, true);
-			guest_mode_init(VM_MODE_P48V48_64K, true, true);
-		}
-	}
-#endif
-#ifdef __s390x__
-	guest_mode_init(VM_MODE_P40V48_4K, true, true);
-#endif
+	struct test_params p = {};
+	int opt;
+
+	guest_modes_append_default();
 
 	while ((opt = getopt(argc, argv, "hm:ud:b:v:")) != -1) {
 		switch (opt) {
 		case 'm':
-			if (!mode_selected) {
-				for (i = 0; i < NUM_VM_MODES; ++i)
-					guest_modes[i].enabled = false;
-				mode_selected = true;
-			}
-			mode = strtoul(optarg, NULL, 10);
-			TEST_ASSERT(mode < NUM_VM_MODES,
-				    "Guest mode ID %d too big", mode);
-			guest_modes[mode].enabled = true;
+			guest_modes_cmdline(optarg);
 			break;
 		case 'u':
-			use_uffd = true;
+			p.use_uffd = true;
 			break;
 		case 'd':
-			uffd_delay = strtoul(optarg, NULL, 0);
-			TEST_ASSERT(uffd_delay >= 0,
-				    "A negative UFFD delay is not supported.");
+			p.uffd_delay = strtoul(optarg, NULL, 0);
+			TEST_ASSERT(p.uffd_delay >= 0, "A negative UFFD delay is not supported.");
 			break;
 		case 'b':
 			guest_percpu_mem_size = parse_size(optarg);
@@ -473,14 +425,7 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	for (i = 0; i < NUM_VM_MODES; ++i) {
-		if (!guest_modes[i].enabled)
-			continue;
-		TEST_ASSERT(guest_modes[i].supported,
-			    "Guest mode ID %d (%s) not supported.",
-			    i, vm_guest_mode_string(i));
-		run_test(i, use_uffd, uffd_delay);
-	}
+	for_each_guest_mode(run_test, &p);
 
 	return 0;
 }
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index b9115e8ef0ed..b448c17bd7aa 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -12,16 +12,14 @@
 
 #include <stdio.h>
 #include <stdlib.h>
-#include <unistd.h>
 #include <time.h>
 #include <pthread.h>
 #include <linux/bitmap.h>
-#include <linux/bitops.h>
 
 #include "kvm_util.h"
-#include "perf_test_util.h"
-#include "processor.h"
 #include "test_util.h"
+#include "perf_test_util.h"
+#include "guest_modes.h"
 
 /* How many host loops to run by default (one KVM_GET_DIRTY_LOG for each loop)*/
 #define TEST_HOST_LOOP_N		2UL
@@ -88,9 +86,15 @@ static void *vcpu_worker(void *data)
 	return NULL;
 }
 
-static void run_test(enum vm_guest_mode mode, unsigned long iterations,
-		     uint64_t phys_offset, int wr_fract)
+struct test_params {
+	unsigned long iterations;
+	uint64_t phys_offset;
+	int wr_fract;
+};
+
+static void run_test(enum vm_guest_mode mode, void *arg)
 {
+	struct test_params *p = arg;
 	pthread_t *vcpu_threads;
 	struct kvm_vm *vm;
 	unsigned long *bmap;
@@ -105,7 +109,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
 
 	vm = create_vm(mode, nr_vcpus, guest_percpu_mem_size);
 
-	perf_test_args.wr_fract = wr_fract;
+	perf_test_args.wr_fract = p->wr_fract;
 
 	guest_num_pages = (nr_vcpus * guest_percpu_mem_size) >> vm_get_page_shift(vm);
 	guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
@@ -147,7 +151,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
 	pr_info("Enabling dirty logging time: %ld.%.9lds\n\n",
 		ts_diff.tv_sec, ts_diff.tv_nsec);
 
-	while (iteration < iterations) {
+	while (iteration < p->iterations) {
 		/*
 		 * Incrementing the iteration number will start the vCPUs
 		 * dirtying memory again.
@@ -189,9 +193,9 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
 	pr_info("Disabling dirty logging time: %ld.%.9lds\n",
 		ts_diff.tv_sec, ts_diff.tv_nsec);
 
-	avg = timespec_div(get_dirty_log_total, iterations);
+	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",
-		iterations, get_dirty_log_total.tv_sec,
+		p->iterations, get_dirty_log_total.tv_sec,
 		get_dirty_log_total.tv_nsec, avg.tv_sec, avg.tv_nsec);
 
 	free(bmap);
@@ -200,20 +204,8 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
 	kvm_vm_free(vm);
 }
 
-struct guest_mode {
-	bool supported;
-	bool enabled;
-};
-static struct guest_mode guest_modes[NUM_VM_MODES];
-
-#define guest_mode_init(mode, supported, enabled) ({ \
-	guest_modes[mode] = (struct guest_mode){ supported, enabled }; \
-})
-
 static void help(char *name)
 {
-	int i;
-
 	puts("");
 	printf("usage: %s [-h] [-i iterations] [-p offset] "
 	       "[-m mode] [-b vcpu bytes] [-v vcpus]\n", name);
@@ -222,14 +214,7 @@ static void help(char *name)
 	       TEST_HOST_LOOP_N);
 	printf(" -p: specify guest physical test memory offset\n"
 	       "     Warning: a low offset can conflict with the loaded test code.\n");
-	printf(" -m: specify the guest mode ID to test "
-	       "(default: test all supported modes)\n"
-	       "     This option may be used multiple times.\n"
-	       "     Guest mode IDs:\n");
-	for (i = 0; i < NUM_VM_MODES; ++i) {
-		printf("         %d:    %s%s\n", i, vm_guest_mode_string(i),
-		       guest_modes[i].supported ? " (supported)" : "");
-	}
+	guest_modes_help();
 	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");
@@ -244,69 +229,38 @@ static void help(char *name)
 
 int main(int argc, char *argv[])
 {
-	unsigned long iterations = TEST_HOST_LOOP_N;
-	bool mode_selected = false;
-	uint64_t phys_offset = 0;
-	unsigned int mode;
-	int opt, i;
-	int wr_fract = 1;
-
-#ifdef __x86_64__
-	guest_mode_init(VM_MODE_PXXV48_4K, true, true);
-#endif
-#ifdef __aarch64__
-	guest_mode_init(VM_MODE_P40V48_4K, true, true);
-	guest_mode_init(VM_MODE_P40V48_64K, true, true);
-
-	{
-		unsigned int limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
-
-		if (limit >= 52)
-			guest_mode_init(VM_MODE_P52V48_64K, true, true);
-		if (limit >= 48) {
-			guest_mode_init(VM_MODE_P48V48_4K, true, true);
-			guest_mode_init(VM_MODE_P48V48_64K, true, true);
-		}
-	}
-#endif
-#ifdef __s390x__
-	guest_mode_init(VM_MODE_P40V48_4K, true, true);
-#endif
+	int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
+	struct test_params p = {
+		.iterations = TEST_HOST_LOOP_N,
+		.wr_fract = 1,
+	};
+	int opt;
+
+	guest_modes_append_default();
 
 	while ((opt = getopt(argc, argv, "hi:p:m:b:f:v:")) != -1) {
 		switch (opt) {
 		case 'i':
-			iterations = strtol(optarg, NULL, 10);
+			p.iterations = strtol(optarg, NULL, 10);
 			break;
 		case 'p':
-			phys_offset = strtoull(optarg, NULL, 0);
+			p.phys_offset = strtoull(optarg, NULL, 0);
 			break;
 		case 'm':
-			if (!mode_selected) {
-				for (i = 0; i < NUM_VM_MODES; ++i)
-					guest_modes[i].enabled = false;
-				mode_selected = true;
-			}
-			mode = strtoul(optarg, NULL, 10);
-			TEST_ASSERT(mode < NUM_VM_MODES,
-				    "Guest mode ID %d too big", mode);
-			guest_modes[mode].enabled = true;
+			guest_modes_cmdline(optarg);
 			break;
 		case 'b':
 			guest_percpu_mem_size = parse_size(optarg);
 			break;
 		case 'f':
-			wr_fract = atoi(optarg);
-			TEST_ASSERT(wr_fract >= 1,
+			p.wr_fract = atoi(optarg);
+			TEST_ASSERT(p.wr_fract >= 1,
 				    "Write fraction cannot be less than one");
 			break;
 		case 'v':
 			nr_vcpus = atoi(optarg);
-			TEST_ASSERT(nr_vcpus > 0,
-				    "Must have a positive number of vCPUs");
-			TEST_ASSERT(nr_vcpus <= MAX_VCPUS,
-				    "This test does not currently support\n"
-				    "more than %d vCPUs.", MAX_VCPUS);
+			TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
+				    "Invalid number of vcpus, must be between 1 and %d", max_vcpus);
 			break;
 		case 'h':
 		default:
@@ -315,18 +269,11 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	TEST_ASSERT(iterations >= 2, "The test should have at least two iterations");
+	TEST_ASSERT(p.iterations >= 2, "The test should have at least two iterations");
 
-	pr_info("Test iterations: %"PRIu64"\n",	iterations);
+	pr_info("Test iterations: %"PRIu64"\n",	p.iterations);
 
-	for (i = 0; i < NUM_VM_MODES; ++i) {
-		if (!guest_modes[i].enabled)
-			continue;
-		TEST_ASSERT(guest_modes[i].supported,
-			    "Guest mode ID %d (%s) not supported.",
-			    i, vm_guest_mode_string(i));
-		run_test(i, iterations, phys_offset, wr_fract);
-	}
+	for_each_guest_mode(run_test, &p);
 
 	return 0;
 }
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 54da9cc20db4..1b7375d2acea 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -9,14 +9,13 @@
 
 #include <stdio.h>
 #include <stdlib.h>
-#include <unistd.h>
-#include <time.h>
 #include <pthread.h>
 #include <linux/bitmap.h>
 #include <linux/bitops.h>
 
-#include "test_util.h"
 #include "kvm_util.h"
+#include "test_util.h"
+#include "guest_modes.h"
 #include "processor.h"
 
 #define VCPU_ID				1
@@ -375,9 +374,15 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, uint32_t vcpuid,
 #define DIRTY_MEM_BITS 30 /* 1G */
 #define PAGE_SHIFT_4K  12
 
-static void run_test(enum vm_guest_mode mode, unsigned long iterations,
-		     unsigned long interval, uint64_t phys_offset)
+struct test_params {
+	unsigned long iterations;
+	unsigned long interval;
+	uint64_t phys_offset;
+};
+
+static void run_test(enum vm_guest_mode mode, void *arg)
 {
+	struct test_params *p = arg;
 	pthread_t vcpu_thread;
 	struct kvm_vm *vm;
 	unsigned long *bmap;
@@ -412,12 +417,12 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
 	host_page_size = getpagesize();
 	host_num_pages = vm_num_host_pages(mode, guest_num_pages);
 
-	if (!phys_offset) {
+	if (!p->phys_offset) {
 		guest_test_phys_mem = (vm_get_max_gfn(vm) -
 				       guest_num_pages) * guest_page_size;
 		guest_test_phys_mem &= ~(host_page_size - 1);
 	} else {
-		guest_test_phys_mem = phys_offset;
+		guest_test_phys_mem = p->phys_offset;
 	}
 
 #ifdef __s390x__
@@ -464,9 +469,9 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
 
 	pthread_create(&vcpu_thread, NULL, vcpu_worker, vm);
 
-	while (iteration < iterations) {
+	while (iteration < p->iterations) {
 		/* Give the vcpu thread some time to dirty some pages */
-		usleep(interval * 1000);
+		usleep(p->interval * 1000);
 		log_mode_collect_dirty_pages(vm, TEST_MEM_SLOT_INDEX,
 					     bmap, host_num_pages);
 		vm_dirty_log_verify(mode, bmap);
@@ -488,20 +493,8 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
 	kvm_vm_free(vm);
 }
 
-struct guest_mode {
-	bool supported;
-	bool enabled;
-};
-static struct guest_mode guest_modes[NUM_VM_MODES];
-
-#define guest_mode_init(mode, supported, enabled) ({ \
-	guest_modes[mode] = (struct guest_mode){ supported, enabled }; \
-})
-
 static void help(char *name)
 {
-	int i;
-
 	puts("");
 	printf("usage: %s [-h] [-i iterations] [-I interval] "
 	       "[-p offset] [-m mode]\n", name);
@@ -515,70 +508,34 @@ static void help(char *name)
 	printf(" -M: specify the host logging mode "
 	       "(default: run all log modes).  Supported modes: \n\t");
 	log_modes_dump();
-	printf(" -m: specify the guest mode ID to test "
-	       "(default: test all supported modes)\n"
-	       "     This option may be used multiple times.\n"
-	       "     Guest mode IDs:\n");
-	for (i = 0; i < NUM_VM_MODES; ++i) {
-		printf("         %d:    %s%s\n", i, vm_guest_mode_string(i),
-		       guest_modes[i].supported ? " (supported)" : "");
-	}
+	guest_modes_help();
 	puts("");
 	exit(0);
 }
 
 int main(int argc, char *argv[])
 {
-	unsigned long iterations = TEST_HOST_LOOP_N;
-	unsigned long interval = TEST_HOST_LOOP_INTERVAL;
-	bool mode_selected = false;
-	uint64_t phys_offset = 0;
-	unsigned int mode;
-	int opt, i, j;
-
-#ifdef __x86_64__
-	guest_mode_init(VM_MODE_PXXV48_4K, true, true);
-#endif
-#ifdef __aarch64__
-	guest_mode_init(VM_MODE_P40V48_4K, true, true);
-	guest_mode_init(VM_MODE_P40V48_64K, true, true);
-
-	{
-		unsigned int limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
+	struct test_params p = {
+		.iterations = TEST_HOST_LOOP_N,
+		.interval = TEST_HOST_LOOP_INTERVAL,
+	};
+	int opt, i;
 
-		if (limit >= 52)
-			guest_mode_init(VM_MODE_P52V48_64K, true, true);
-		if (limit >= 48) {
-			guest_mode_init(VM_MODE_P48V48_4K, true, true);
-			guest_mode_init(VM_MODE_P48V48_64K, true, true);
-		}
-	}
-#endif
-#ifdef __s390x__
-	guest_mode_init(VM_MODE_P40V48_4K, true, true);
-#endif
+	guest_modes_append_default();
 
 	while ((opt = getopt(argc, argv, "hi:I:p:m:M:")) != -1) {
 		switch (opt) {
 		case 'i':
-			iterations = strtol(optarg, NULL, 10);
+			p.iterations = strtol(optarg, NULL, 10);
 			break;
 		case 'I':
-			interval = strtol(optarg, NULL, 10);
+			p.interval = strtol(optarg, NULL, 10);
 			break;
 		case 'p':
-			phys_offset = strtoull(optarg, NULL, 0);
+			p.phys_offset = strtoull(optarg, NULL, 0);
 			break;
 		case 'm':
-			if (!mode_selected) {
-				for (i = 0; i < NUM_VM_MODES; ++i)
-					guest_modes[i].enabled = false;
-				mode_selected = true;
-			}
-			mode = strtoul(optarg, NULL, 10);
-			TEST_ASSERT(mode < NUM_VM_MODES,
-				    "Guest mode ID %d too big", mode);
-			guest_modes[mode].enabled = true;
+			guest_modes_cmdline(optarg);
 			break;
 		case 'M':
 			if (!strcmp(optarg, "all")) {
@@ -607,32 +564,24 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	TEST_ASSERT(iterations > 2, "Iterations must be greater than two");
-	TEST_ASSERT(interval > 0, "Interval must be greater than zero");
+	TEST_ASSERT(p.iterations > 2, "Iterations must be greater than two");
+	TEST_ASSERT(p.interval > 0, "Interval must be greater than zero");
 
 	pr_info("Test iterations: %"PRIu64", interval: %"PRIu64" (ms)\n",
-		iterations, interval);
+		p.iterations, p.interval);
 
 	srandom(time(0));
 
-	for (i = 0; i < NUM_VM_MODES; ++i) {
-		if (!guest_modes[i].enabled)
-			continue;
-		TEST_ASSERT(guest_modes[i].supported,
-			    "Guest mode ID %d (%s) not supported.",
-			    i, vm_guest_mode_string(i));
-		if (host_log_mode_option == LOG_MODE_ALL) {
-			/* Run each log mode */
-			for (j = 0; j < LOG_MODE_NUM; j++) {
-				pr_info("Testing Log Mode '%s'\n",
-					log_modes[j].name);
-				host_log_mode = j;
-				run_test(i, iterations, interval, phys_offset);
-			}
-		} else {
-			host_log_mode = host_log_mode_option;
-			run_test(i, iterations, interval, phys_offset);
+	if (host_log_mode_option == LOG_MODE_ALL) {
+		/* Run each log mode */
+		for (i = 0; i < LOG_MODE_NUM; i++) {
+			pr_info("Testing Log Mode '%s'\n", log_modes[i].name);
+			host_log_mode = i;
+			for_each_guest_mode(run_test, &p);
 		}
+	} else {
+		host_log_mode = host_log_mode_option;
+		for_each_guest_mode(run_test, &p);
 	}
 
 	return 0;
diff --git a/tools/testing/selftests/kvm/include/guest_modes.h b/tools/testing/selftests/kvm/include/guest_modes.h
new file mode 100644
index 000000000000..b691df33e64e
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/guest_modes.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020, Red Hat, Inc.
+ */
+#include "kvm_util.h"
+
+struct guest_mode {
+	bool supported;
+	bool enabled;
+};
+
+extern struct guest_mode guest_modes[NUM_VM_MODES];
+
+#define guest_mode_append(mode, supported, enabled) ({ \
+	guest_modes[mode] = (struct guest_mode){ supported, enabled }; \
+})
+
+void guest_modes_append_default(void);
+void for_each_guest_mode(void (*func)(enum vm_guest_mode, void *), void *arg);
+void guest_modes_help(void);
+void guest_modes_cmdline(const char *arg);
diff --git a/tools/testing/selftests/kvm/lib/guest_modes.c b/tools/testing/selftests/kvm/lib/guest_modes.c
new file mode 100644
index 000000000000..25bff307c71f
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/guest_modes.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020, Red Hat, Inc.
+ */
+#include "guest_modes.h"
+
+struct guest_mode guest_modes[NUM_VM_MODES];
+
+void guest_modes_append_default(void)
+{
+	guest_mode_append(VM_MODE_DEFAULT, true, true);
+
+#ifdef __aarch64__
+	guest_mode_append(VM_MODE_P40V48_64K, true, true);
+	{
+		unsigned int limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
+		if (limit >= 52)
+			guest_mode_append(VM_MODE_P52V48_64K, true, true);
+		if (limit >= 48) {
+			guest_mode_append(VM_MODE_P48V48_4K, true, true);
+			guest_mode_append(VM_MODE_P48V48_64K, true, true);
+		}
+	}
+#endif
+}
+
+void for_each_guest_mode(void (*func)(enum vm_guest_mode, void *), void *arg)
+{
+	int i;
+
+	for (i = 0; i < NUM_VM_MODES; ++i) {
+		if (!guest_modes[i].enabled)
+			continue;
+		TEST_ASSERT(guest_modes[i].supported,
+			    "Guest mode ID %d (%s) not supported.",
+			    i, vm_guest_mode_string(i));
+		func(i, arg);
+	}
+}
+
+void guest_modes_help(void)
+{
+	int i;
+
+	printf(" -m: specify the guest mode ID to test\n"
+	       "     (default: test all supported modes)\n"
+	       "     This option may be used multiple times.\n"
+	       "     Guest mode IDs:\n");
+	for (i = 0; i < NUM_VM_MODES; ++i) {
+		printf("         %d:    %s%s\n", i, vm_guest_mode_string(i),
+		       guest_modes[i].supported ? " (supported)" : "");
+	}
+}
+
+void guest_modes_cmdline(const char *arg)
+{
+	static bool mode_selected;
+	unsigned int mode;
+	int i;
+
+	if (!mode_selected) {
+		for (i = 0; i < NUM_VM_MODES; ++i)
+			guest_modes[i].enabled = false;
+		mode_selected = true;
+	}
+
+	mode = strtoul(optarg, NULL, 10);
+	TEST_ASSERT(mode < NUM_VM_MODES, "Guest mode ID %d too big", mode);
+	guest_modes[mode].enabled = true;
+}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 4/8] KVM: selftests: Make vm_create_default common
  2020-11-10 20:47 [PATCH 0/8] KVM: selftests: Cleanups, take 2 Andrew Jones
                   ` (2 preceding siblings ...)
  2020-11-10 20:47 ` [PATCH 3/8] KVM: selftests: Factor out guest mode code Andrew Jones
@ 2020-11-10 20:47 ` Andrew Jones
  2020-11-10 20:47 ` [PATCH 5/8] KVM: selftests: Introduce vm_create_[default_]_with_vcpus Andrew Jones
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2020-11-10 20:47 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, borntraeger, frankja, bgardon, peterx

The code is almost 100% the same anyway. Just move it to common
and add a few arch-specific macros.

Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Ben Gardon <bgardon@google.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 .../testing/selftests/kvm/include/kvm_util.h  | 23 ++++++++++---
 .../selftests/kvm/lib/aarch64/processor.c     | 17 ----------
 tools/testing/selftests/kvm/lib/kvm_util.c    | 26 +++++++++++++++
 .../selftests/kvm/lib/s390x/processor.c       | 22 -------------
 .../selftests/kvm/lib/x86_64/processor.c      | 32 -------------------
 5 files changed, 45 insertions(+), 75 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 7d29aa786959..48b48a0014e2 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -45,13 +45,28 @@ enum vm_guest_mode {
 };
 
 #if defined(__aarch64__)
-#define VM_MODE_DEFAULT VM_MODE_P40V48_4K
+
+#define VM_MODE_DEFAULT			VM_MODE_P40V48_4K
+#define MIN_PAGE_SHIFT			12U
+#define ptes_per_page(page_size)	((page_size) / 8)
+
 #elif defined(__x86_64__)
-#define VM_MODE_DEFAULT VM_MODE_PXXV48_4K
-#else
-#define VM_MODE_DEFAULT VM_MODE_P52V48_4K
+
+#define VM_MODE_DEFAULT			VM_MODE_PXXV48_4K
+#define MIN_PAGE_SHIFT			12U
+#define ptes_per_page(page_size)	((page_size) / 8)
+
+#elif defined(__s390x__)
+
+#define VM_MODE_DEFAULT			VM_MODE_P52V48_4K
+#define MIN_PAGE_SHIFT			12U
+#define ptes_per_page(page_size)	((page_size) / 16)
+
 #endif
 
+#define MIN_PAGE_SIZE		(1U << MIN_PAGE_SHIFT)
+#define PTES_PER_MIN_PAGE	ptes_per_page(MIN_PAGE_SIZE)
+
 #define vm_guest_mode_string(m) vm_guest_mode_string[m]
 extern const char * const vm_guest_mode_string[];
 
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index d6c32c328e9a..cee92d477dc0 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -5,8 +5,6 @@
  * Copyright (C) 2018, Red Hat, Inc.
  */
 
-#define _GNU_SOURCE /* for program_invocation_name */
-
 #include <linux/compiler.h>
 
 #include "kvm_util.h"
@@ -219,21 +217,6 @@ void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
 	}
 }
 
-struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
-				 void *guest_code)
-{
-	uint64_t ptrs_per_4k_pte = 512;
-	uint64_t extra_pg_pages = (extra_mem_pages / ptrs_per_4k_pte) * 2;
-	struct kvm_vm *vm;
-
-	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES + extra_pg_pages, O_RDWR);
-
-	kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
-	vm_vcpu_add_default(vm, vcpuid, guest_code);
-
-	return vm;
-}
-
 void aarch64_vcpu_setup(struct kvm_vm *vm, int vcpuid, struct kvm_vcpu_init *init)
 {
 	struct kvm_vcpu_init default_init = { .target = -1, };
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 126c6727a6b0..a7e28e33fc3b 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2018, Google LLC.
  */
 
+#define _GNU_SOURCE /* for program_invocation_name */
 #include "test_util.h"
 #include "kvm_util.h"
 #include "kvm_util_internal.h"
@@ -271,6 +272,31 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
 	return vm;
 }
 
+struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
+				 void *guest_code)
+{
+	/* The maximum page table size for a memory region will be when the
+	 * smallest pages are used. Considering each page contains x page
+	 * table descriptors, the total extra size for page tables (for extra
+	 * N pages) will be: N/x+N/x^2+N/x^3+... which is definitely smaller
+	 * than N/x*2.
+	 */
+	uint64_t extra_pg_pages = (extra_mem_pages / PTES_PER_MIN_PAGE) * 2;
+	struct kvm_vm *vm;
+
+	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES + extra_pg_pages, O_RDWR);
+
+	kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
+
+#ifdef __x86_64__
+	vm_create_irqchip(vm);
+#endif
+
+	vm_vcpu_add_default(vm, vcpuid, guest_code);
+
+	return vm;
+}
+
 /*
  * VM Restart
  *
diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c
index 7349bb2e1a24..0152f356c099 100644
--- a/tools/testing/selftests/kvm/lib/s390x/processor.c
+++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
@@ -5,8 +5,6 @@
  * Copyright (C) 2019, Red Hat, Inc.
  */
 
-#define _GNU_SOURCE /* for program_invocation_name */
-
 #include "processor.h"
 #include "kvm_util.h"
 #include "../kvm_util_internal.h"
@@ -160,26 +158,6 @@ void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
 	virt_dump_region(stream, vm, indent, vm->pgd);
 }
 
-struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
-				 void *guest_code)
-{
-	/*
-	 * The additional amount of pages required for the page tables is:
-	 * 1 * n / 256 + 4 * (n / 256) / 2048 + 4 * (n / 256) / 2048^2 + ...
-	 * which is definitely smaller than (n / 256) * 2.
-	 */
-	uint64_t extra_pg_pages = extra_mem_pages / 256 * 2;
-	struct kvm_vm *vm;
-
-	vm = vm_create(VM_MODE_DEFAULT,
-		       DEFAULT_GUEST_PHY_PAGES + extra_pg_pages, O_RDWR);
-
-	kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
-	vm_vcpu_add_default(vm, vcpuid, guest_code);
-
-	return vm;
-}
-
 void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
 {
 	size_t stack_size =  DEFAULT_STACK_PGS * getpagesize();
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index d10c5c05bdf0..95e1a757c629 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -5,8 +5,6 @@
  * Copyright (C) 2018, Google LLC.
  */
 
-#define _GNU_SOURCE /* for program_invocation_name */
-
 #include "test_util.h"
 #include "kvm_util.h"
 #include "../kvm_util_internal.h"
@@ -731,36 +729,6 @@ void vcpu_set_cpuid(struct kvm_vm *vm,
 
 }
 
-struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
-				 void *guest_code)
-{
-	struct kvm_vm *vm;
-	/*
-	 * For x86 the maximum page table size for a memory region
-	 * will be when only 4K pages are used.  In that case the
-	 * total extra size for page tables (for extra N pages) will
-	 * be: N/512+N/512^2+N/512^3+... which is definitely smaller
-	 * than N/512*2.
-	 */
-	uint64_t extra_pg_pages = extra_mem_pages / 512 * 2;
-
-	/* Create VM */
-	vm = vm_create(VM_MODE_DEFAULT,
-		       DEFAULT_GUEST_PHY_PAGES + extra_pg_pages,
-		       O_RDWR);
-
-	/* Setup guest code */
-	kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
-
-	/* Setup IRQ Chip */
-	vm_create_irqchip(vm);
-
-	/* Add the first vCPU. */
-	vm_vcpu_add_default(vm, vcpuid, guest_code);
-
-	return vm;
-}
-
 /*
  * VCPU Get MSR
  *
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 5/8] KVM: selftests: Introduce vm_create_[default_]_with_vcpus
  2020-11-10 20:47 [PATCH 0/8] KVM: selftests: Cleanups, take 2 Andrew Jones
                   ` (3 preceding siblings ...)
  2020-11-10 20:47 ` [PATCH 4/8] KVM: selftests: Make vm_create_default common Andrew Jones
@ 2020-11-10 20:47 ` Andrew Jones
  2020-11-10 22:13   ` Ben Gardon
  2020-11-10 20:48 ` [PATCH 6/8] KVM: selftests: dirty_log_test: Remove create_vm Andrew Jones
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Andrew Jones @ 2020-11-10 20:47 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, borntraeger, frankja, bgardon, peterx

Introduce new vm_create variants that also takes a number of vcpus,
an amount of per-vcpu pages, and optionally a list of vcpuids. These
variants will create default VMs with enough additional pages to
cover the vcpu stacks, per-vcpu pages, and pagetable pages for all.
The new 'default' variant uses VM_MODE_DEFAULT, whereas the other
new variant accepts the mode as a parameter.

Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Ben Gardon <bgardon@google.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 .../testing/selftests/kvm/include/kvm_util.h  | 10 ++++++
 tools/testing/selftests/kvm/lib/kvm_util.c    | 35 ++++++++++++++++---
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 48b48a0014e2..bc8db80309f5 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -261,6 +261,16 @@ vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
 struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
 				 void *guest_code);
 
+/* Same as vm_create_default, but can be used for more than one vcpu */
+struct kvm_vm *vm_create_default_with_vcpus(uint32_t nr_vcpus, uint64_t extra_mem_pages,
+					    uint32_t num_percpu_pages, void *guest_code,
+					    uint32_t vcpuids[]);
+
+/* Like vm_create_default_with_vcpus, but accepts mode as a parameter */
+struct kvm_vm *vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus,
+				    uint64_t extra_mem_pages, uint32_t num_percpu_pages,
+				    void *guest_code, uint32_t vcpuids[]);
+
 /*
  * Adds a vCPU with reasonable defaults (e.g. a stack)
  *
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index a7e28e33fc3b..b31a4e988a5d 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -272,8 +272,9 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
 	return vm;
 }
 
-struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
-				 void *guest_code)
+struct kvm_vm *vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus,
+				    uint64_t extra_mem_pages, uint32_t num_percpu_pages,
+				    void *guest_code, uint32_t vcpuids[])
 {
 	/* The maximum page table size for a memory region will be when the
 	 * smallest pages are used. Considering each page contains x page
@@ -281,10 +282,18 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
 	 * N pages) will be: N/x+N/x^2+N/x^3+... which is definitely smaller
 	 * than N/x*2.
 	 */
-	uint64_t extra_pg_pages = (extra_mem_pages / PTES_PER_MIN_PAGE) * 2;
+	uint64_t vcpu_pages = (DEFAULT_STACK_PGS + num_percpu_pages) * nr_vcpus;
+	uint64_t extra_pg_pages = (extra_mem_pages + vcpu_pages) / PTES_PER_MIN_PAGE * 2;
+	uint64_t pages = DEFAULT_GUEST_PHY_PAGES + vcpu_pages + extra_pg_pages;
 	struct kvm_vm *vm;
+	int i;
+
+	TEST_ASSERT(nr_vcpus <= kvm_check_cap(KVM_CAP_MAX_VCPUS),
+		    "nr_vcpus = %d too large for host, max-vcpus = %d",
+		    nr_vcpus, kvm_check_cap(KVM_CAP_MAX_VCPUS));
 
-	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES + extra_pg_pages, O_RDWR);
+	pages = vm_adjust_num_guest_pages(mode, pages);
+	vm = vm_create(mode, pages, O_RDWR);
 
 	kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
 
@@ -292,11 +301,27 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
 	vm_create_irqchip(vm);
 #endif
 
-	vm_vcpu_add_default(vm, vcpuid, guest_code);
+	for (i = 0; i < nr_vcpus; ++i)
+		vm_vcpu_add_default(vm, vcpuids ? vcpuids[i] : i, guest_code);
 
 	return vm;
 }
 
+struct kvm_vm *vm_create_default_with_vcpus(uint32_t nr_vcpus, uint64_t extra_mem_pages,
+					    uint32_t num_percpu_pages, void *guest_code,
+					    uint32_t vcpuids[])
+{
+	return vm_create_with_vcpus(VM_MODE_DEFAULT, nr_vcpus, extra_mem_pages,
+				    num_percpu_pages, guest_code, vcpuids);
+}
+
+struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
+				 void *guest_code)
+{
+	return vm_create_default_with_vcpus(1, extra_mem_pages, 0, guest_code,
+					    (uint32_t []){ vcpuid });
+}
+
 /*
  * VM Restart
  *
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 6/8] KVM: selftests: dirty_log_test: Remove create_vm
  2020-11-10 20:47 [PATCH 0/8] KVM: selftests: Cleanups, take 2 Andrew Jones
                   ` (4 preceding siblings ...)
  2020-11-10 20:47 ` [PATCH 5/8] KVM: selftests: Introduce vm_create_[default_]_with_vcpus Andrew Jones
@ 2020-11-10 20:48 ` Andrew Jones
  2020-11-10 20:48 ` [PATCH 7/8] KVM: selftests: Use vm_create_with_vcpus in create_vm Andrew Jones
  2020-11-10 20:48 ` [PATCH 8/8] KVM: selftests: Implement perf_test_util more conventionally Andrew Jones
  7 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2020-11-10 20:48 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, borntraeger, frankja, bgardon, peterx

Use vm_create_with_vcpus instead of create_vm and do
some minor cleanups around it.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 tools/testing/selftests/kvm/dirty_log_test.c | 56 ++++++--------------
 1 file changed, 16 insertions(+), 40 deletions(-)

diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 1b7375d2acea..2e0dcd453ef0 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -5,8 +5,6 @@
  * Copyright (C) 2018, Red Hat, Inc.
  */
 
-#define _GNU_SOURCE /* for program_invocation_name */
-
 #include <stdio.h>
 #include <stdlib.h>
 #include <pthread.h>
@@ -20,6 +18,9 @@
 
 #define VCPU_ID				1
 
+#define DIRTY_MEM_BITS			30 /* 1G */
+#define DIRTY_MEM_SIZE			(1UL << 30)
+
 /* The memory slot index to track dirty pages */
 #define TEST_MEM_SLOT_INDEX		1
 
@@ -353,27 +354,6 @@ static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap)
 	}
 }
 
-static struct kvm_vm *create_vm(enum vm_guest_mode mode, uint32_t vcpuid,
-				uint64_t extra_mem_pages, void *guest_code)
-{
-	struct kvm_vm *vm;
-	uint64_t extra_pg_pages = extra_mem_pages / 512 * 2;
-
-	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
-
-	vm = vm_create(mode, DEFAULT_GUEST_PHY_PAGES + extra_pg_pages, O_RDWR);
-	kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
-#ifdef __x86_64__
-	vm_create_irqchip(vm);
-#endif
-	log_mode_create_vm_done(vm);
-	vm_vcpu_add_default(vm, vcpuid, guest_code);
-	return vm;
-}
-
-#define DIRTY_MEM_BITS 30 /* 1G */
-#define PAGE_SHIFT_4K  12
-
 struct test_params {
 	unsigned long iterations;
 	unsigned long interval;
@@ -393,43 +373,39 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 		return;
 	}
 
+	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
+
 	/*
 	 * We reserve page table for 2 times of extra dirty mem which
-	 * will definitely cover the original (1G+) test range.  Here
-	 * we do the calculation with 4K page size which is the
-	 * smallest so the page number will be enough for all archs
-	 * (e.g., 64K page size guest will need even less memory for
-	 * page tables).
+	 * will definitely cover the original (1G+) test range.
 	 */
-	vm = create_vm(mode, VCPU_ID,
-		       2ul << (DIRTY_MEM_BITS - PAGE_SHIFT_4K),
-		       guest_code);
+	vm = vm_create_with_vcpus(mode, 1,
+			vm_calc_num_guest_pages(mode, DIRTY_MEM_SIZE * 2),
+			0, guest_code, (uint32_t []){ VCPU_ID });
+
+	log_mode_create_vm_done(vm);
 
 	guest_page_size = vm_get_page_size(vm);
+	host_page_size = getpagesize();
+
 	/*
 	 * A little more than 1G of guest page sized pages.  Cover the
 	 * case where the size is not aligned to 64 pages.
 	 */
-	guest_num_pages = (1ul << (DIRTY_MEM_BITS -
-				   vm_get_page_shift(vm))) + 3;
-	guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
-
-	host_page_size = getpagesize();
+	guest_num_pages = vm_adjust_num_guest_pages(mode,
+				(1ul << (DIRTY_MEM_BITS - vm_get_page_shift(vm))) + 3);
 	host_num_pages = vm_num_host_pages(mode, guest_num_pages);
 
 	if (!p->phys_offset) {
-		guest_test_phys_mem = (vm_get_max_gfn(vm) -
-				       guest_num_pages) * guest_page_size;
+		guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) * guest_page_size;
 		guest_test_phys_mem &= ~(host_page_size - 1);
 	} else {
 		guest_test_phys_mem = p->phys_offset;
 	}
-
 #ifdef __s390x__
 	/* Align to 1M (segment size) */
 	guest_test_phys_mem &= ~((1 << 20) - 1);
 #endif
-
 	pr_info("guest physical test memory offset: 0x%lx\n", guest_test_phys_mem);
 
 	bmap = bitmap_alloc(host_num_pages);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 7/8] KVM: selftests: Use vm_create_with_vcpus in create_vm
  2020-11-10 20:47 [PATCH 0/8] KVM: selftests: Cleanups, take 2 Andrew Jones
                   ` (5 preceding siblings ...)
  2020-11-10 20:48 ` [PATCH 6/8] KVM: selftests: dirty_log_test: Remove create_vm Andrew Jones
@ 2020-11-10 20:48 ` Andrew Jones
  2020-11-10 20:48 ` [PATCH 8/8] KVM: selftests: Implement perf_test_util more conventionally Andrew Jones
  7 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2020-11-10 20:48 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, borntraeger, frankja, bgardon, peterx

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 .../testing/selftests/kvm/include/kvm_util.h  |  8 ++++
 .../selftests/kvm/include/perf_test_util.h    | 47 +++++--------------
 tools/testing/selftests/kvm/lib/kvm_util.c    |  9 +---
 3 files changed, 20 insertions(+), 44 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index bc8db80309f5..011e8c6b4600 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -70,6 +70,14 @@ enum vm_guest_mode {
 #define vm_guest_mode_string(m) vm_guest_mode_string[m]
 extern const char * const vm_guest_mode_string[];
 
+struct vm_guest_mode_params {
+	unsigned int pa_bits;
+	unsigned int va_bits;
+	unsigned int page_size;
+	unsigned int page_shift;
+};
+extern const struct vm_guest_mode_params vm_guest_mode_params[];
+
 enum vm_mem_backing_src_type {
 	VM_MEM_SRC_ANONYMOUS,
 	VM_MEM_SRC_ANONYMOUS_THP,
diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
index 2618052057b1..0bd4407fb662 100644
--- a/tools/testing/selftests/kvm/include/perf_test_util.h
+++ b/tools/testing/selftests/kvm/include/perf_test_util.h
@@ -13,9 +13,6 @@
 
 #define MAX_VCPUS 512
 
-#define PAGE_SHIFT_4K  12
-#define PTES_PER_4K_PT 512
-
 #define TEST_MEM_SLOT_INDEX		1
 
 /* Default guest test virtual memory offset */
@@ -94,41 +91,26 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, int vcpus,
 				uint64_t vcpu_memory_bytes)
 {
 	struct kvm_vm *vm;
-	uint64_t pages = DEFAULT_GUEST_PHY_PAGES;
 	uint64_t guest_num_pages;
 
-	/* Account for a few pages per-vCPU for stacks */
-	pages += DEFAULT_STACK_PGS * vcpus;
-
-	/*
-	 * Reserve twice the ammount of memory needed to map the test region and
-	 * the page table / stacks region, at 4k, for page tables. Do the
-	 * calculation with 4K page size: the smallest of all archs. (e.g., 64K
-	 * page size guest will need even less memory for page tables).
-	 */
-	pages += (2 * pages) / PTES_PER_4K_PT;
-	pages += ((2 * vcpus * vcpu_memory_bytes) >> PAGE_SHIFT_4K) /
-		 PTES_PER_4K_PT;
-	pages = vm_adjust_num_guest_pages(mode, pages);
-
 	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
 
-	vm = vm_create(mode, pages, O_RDWR);
-	kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
-#ifdef __x86_64__
-	vm_create_irqchip(vm);
-#endif
-
-	perf_test_args.vm = vm;
-	perf_test_args.guest_page_size = vm_get_page_size(vm);
 	perf_test_args.host_page_size = getpagesize();
+	perf_test_args.guest_page_size = vm_guest_mode_params[mode].page_size;
+
+	guest_num_pages = vm_adjust_num_guest_pages(mode,
+				(vcpus * vcpu_memory_bytes) / perf_test_args.guest_page_size);
 
+	TEST_ASSERT(vcpu_memory_bytes % perf_test_args.host_page_size == 0,
+		    "Guest memory size is not host page size aligned.");
 	TEST_ASSERT(vcpu_memory_bytes % perf_test_args.guest_page_size == 0,
 		    "Guest memory size is not guest page size aligned.");
 
-	guest_num_pages = (vcpus * vcpu_memory_bytes) /
-			  perf_test_args.guest_page_size;
-	guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
+	vm = vm_create_with_vcpus(mode, vcpus, 0,
+				  vcpu_memory_bytes / perf_test_args.guest_page_size,
+				  guest_code, NULL);
+
+	perf_test_args.vm = vm;
 
 	/*
 	 * If there should be more memory in the guest test region than there
@@ -140,18 +122,13 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, int vcpus,
 		    guest_num_pages, vm_get_max_gfn(vm), vcpus,
 		    vcpu_memory_bytes);
 
-	TEST_ASSERT(vcpu_memory_bytes % perf_test_args.host_page_size == 0,
-		    "Guest memory size is not host page size aligned.");
-
 	guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) *
 			      perf_test_args.guest_page_size;
 	guest_test_phys_mem &= ~(perf_test_args.host_page_size - 1);
-
 #ifdef __s390x__
 	/* Align to 1M (segment size) */
 	guest_test_phys_mem &= ~((1 << 20) - 1);
 #endif
-
 	pr_info("guest physical test memory offset: 0x%lx\n", guest_test_phys_mem);
 
 	/* Add an extra memory slot for testing */
@@ -177,8 +154,6 @@ static void add_vcpus(struct kvm_vm *vm, int vcpus, uint64_t vcpu_memory_bytes)
 	for (vcpu_id = 0; vcpu_id < vcpus; vcpu_id++) {
 		vcpu_args = &perf_test_args.vcpu_args[vcpu_id];
 
-		vm_vcpu_add_default(vm, vcpu_id, guest_code);
-
 #ifdef __x86_64__
 		vcpu_set_cpuid(vm, vcpu_id, kvm_get_supported_cpuid());
 #endif
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index b31a4e988a5d..ff4a0310c420 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -143,14 +143,7 @@ const char * const vm_guest_mode_string[] = {
 _Static_assert(sizeof(vm_guest_mode_string)/sizeof(char *) == NUM_VM_MODES,
 	       "Missing new mode strings?");
 
-struct vm_guest_mode_params {
-	unsigned int pa_bits;
-	unsigned int va_bits;
-	unsigned int page_size;
-	unsigned int page_shift;
-};
-
-static const struct vm_guest_mode_params vm_guest_mode_params[] = {
+const struct vm_guest_mode_params vm_guest_mode_params[] = {
 	{ 52, 48,  0x1000, 12 },
 	{ 52, 48, 0x10000, 16 },
 	{ 48, 48,  0x1000, 12 },
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH 8/8] KVM: selftests: Implement perf_test_util more conventionally
  2020-11-10 20:47 [PATCH 0/8] KVM: selftests: Cleanups, take 2 Andrew Jones
                   ` (6 preceding siblings ...)
  2020-11-10 20:48 ` [PATCH 7/8] KVM: selftests: Use vm_create_with_vcpus in create_vm Andrew Jones
@ 2020-11-10 20:48 ` Andrew Jones
  7 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2020-11-10 20:48 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, borntraeger, frankja, bgardon, peterx

It's not conventional C to put non-inline functions in header
files. Create a source file for the functions instead. Also
reduce the amount of globals and rename the functions to
something less generic.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 tools/testing/selftests/kvm/Makefile          |   2 +-
 .../selftests/kvm/demand_paging_test.c        |   8 +-
 .../selftests/kvm/dirty_log_perf_test.c       |  13 +-
 .../testing/selftests/kvm/include/kvm_util.h  |   1 +
 .../selftests/kvm/include/perf_test_util.h    | 145 ++----------------
 .../selftests/kvm/lib/perf_test_util.c        | 134 ++++++++++++++++
 6 files changed, 160 insertions(+), 143 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/lib/perf_test_util.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index ca6b64d9ab64..120b02bf3f1e 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -33,7 +33,7 @@ ifeq ($(ARCH),s390)
 	UNAME_M := s390x
 endif
 
-LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c
+LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c lib/perf_test_util.c
 LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S
 LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
 LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 946161a9ce2d..b3c994aa4965 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -36,12 +36,14 @@
 #define PER_VCPU_DEBUG(...) _no_printf(__VA_ARGS__)
 #endif
 
+static int nr_vcpus = 1;
+static uint64_t guest_percpu_mem_size = DEFAULT_PER_VCPU_MEM_SIZE;
 static char *guest_data_prototype;
 
 static void *vcpu_worker(void *data)
 {
 	int ret;
-	struct vcpu_args *vcpu_args = (struct vcpu_args *)data;
+	struct perf_test_vcpu_args *vcpu_args = (struct perf_test_vcpu_args *)data;
 	int vcpu_id = vcpu_args->vcpu_id;
 	struct kvm_vm *vm = perf_test_args.vm;
 	struct kvm_run *run;
@@ -263,7 +265,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	int vcpu_id;
 	int r;
 
-	vm = create_vm(mode, nr_vcpus, guest_percpu_mem_size);
+	vm = perf_test_create_vm(mode, nr_vcpus, guest_percpu_mem_size);
 
 	perf_test_args.wr_fract = 1;
 
@@ -275,7 +277,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	vcpu_threads = malloc(nr_vcpus * sizeof(*vcpu_threads));
 	TEST_ASSERT(vcpu_threads, "Memory allocation failed");
 
-	add_vcpus(vm, nr_vcpus, guest_percpu_mem_size);
+	perf_test_add_vcpus(vm, nr_vcpus, guest_percpu_mem_size);
 
 	if (p->use_uffd) {
 		uffd_handler_threads =
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index b448c17bd7aa..df1e0b144c34 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -24,10 +24,15 @@
 /* How many host loops to run by default (one KVM_GET_DIRTY_LOG for each loop)*/
 #define TEST_HOST_LOOP_N		2UL
 
+#define TEST_MEM_SLOT_INDEX		1
+
+static int nr_vcpus = 1;
+static uint64_t guest_percpu_mem_size = DEFAULT_PER_VCPU_MEM_SIZE;
+
 /* Host variables */
 static bool host_quit;
 static uint64_t iteration;
-static uint64_t vcpu_last_completed_iteration[MAX_VCPUS];
+static uint64_t vcpu_last_completed_iteration[KVM_MAX_VCPUS];
 
 static void *vcpu_worker(void *data)
 {
@@ -39,7 +44,7 @@ static void *vcpu_worker(void *data)
 	struct timespec ts_diff;
 	struct timespec total = (struct timespec){0};
 	struct timespec avg;
-	struct vcpu_args *vcpu_args = (struct vcpu_args *)data;
+	struct perf_test_vcpu_args *vcpu_args = (struct perf_test_vcpu_args *)data;
 	int vcpu_id = vcpu_args->vcpu_id;
 
 	vcpu_args_set(vm, vcpu_id, 1, vcpu_id);
@@ -107,7 +112,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	struct timespec vcpu_dirty_total = (struct timespec){0};
 	struct timespec avg;
 
-	vm = create_vm(mode, nr_vcpus, guest_percpu_mem_size);
+	vm = perf_test_create_vm(mode, nr_vcpus, guest_percpu_mem_size);
 
 	perf_test_args.wr_fract = p->wr_fract;
 
@@ -119,7 +124,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	vcpu_threads = malloc(nr_vcpus * sizeof(*vcpu_threads));
 	TEST_ASSERT(vcpu_threads, "Memory allocation failed");
 
-	add_vcpus(vm, nr_vcpus, guest_percpu_mem_size);
+	perf_test_add_vcpus(vm, nr_vcpus, guest_percpu_mem_size);
 
 	sync_global_to_guest(vm, perf_test_args);
 
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 011e8c6b4600..99fa84ed2db9 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -16,6 +16,7 @@
 
 #include "sparsebit.h"
 
+#define KVM_MAX_VCPUS 512
 
 /*
  * Callers of kvm_util only have an incomplete/opaque description of the
diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
index 0bd4407fb662..65e8abaf75f7 100644
--- a/tools/testing/selftests/kvm/include/perf_test_util.h
+++ b/tools/testing/selftests/kvm/include/perf_test_util.h
@@ -9,35 +9,13 @@
 #define SELFTEST_KVM_PERF_TEST_UTIL_H
 
 #include "kvm_util.h"
-#include "processor.h"
-
-#define MAX_VCPUS 512
-
-#define TEST_MEM_SLOT_INDEX		1
 
 /* Default guest test virtual memory offset */
 #define DEFAULT_GUEST_TEST_MEM		0xc0000000
 
 #define DEFAULT_PER_VCPU_MEM_SIZE	(1 << 30) /* 1G */
 
-/*
- * Guest physical memory offset of the testing memory slot.
- * This will be set to the topmost valid physical address minus
- * the test memory size.
- */
-static uint64_t guest_test_phys_mem;
-
-/*
- * Guest virtual memory offset of the testing memory slot.
- * Must not conflict with identity mapped test code.
- */
-static uint64_t guest_test_virt_mem = DEFAULT_GUEST_TEST_MEM;
-static uint64_t guest_percpu_mem_size = DEFAULT_PER_VCPU_MEM_SIZE;
-
-/* Number of VCPUs for the test */
-static int nr_vcpus = 1;
-
-struct vcpu_args {
+struct perf_test_vcpu_args {
 	uint64_t gva;
 	uint64_t pages;
 
@@ -51,123 +29,20 @@ struct perf_test_args {
 	uint64_t guest_page_size;
 	int wr_fract;
 
-	struct vcpu_args vcpu_args[MAX_VCPUS];
+	struct perf_test_vcpu_args vcpu_args[KVM_MAX_VCPUS];
 };
 
-static struct perf_test_args perf_test_args;
+extern struct perf_test_args perf_test_args;
 
 /*
- * Continuously write to the first 8 bytes of each page in the
- * specified region.
+ * Guest physical memory offset of the testing memory slot.
+ * This will be set to the topmost valid physical address minus
+ * the test memory size.
  */
-static void guest_code(uint32_t vcpu_id)
-{
-	struct vcpu_args *vcpu_args = &perf_test_args.vcpu_args[vcpu_id];
-	uint64_t gva;
-	uint64_t pages;
-	int i;
-
-	/* Make sure vCPU args data structure is not corrupt. */
-	GUEST_ASSERT(vcpu_args->vcpu_id == vcpu_id);
-
-	gva = vcpu_args->gva;
-	pages = vcpu_args->pages;
-
-	while (true) {
-		for (i = 0; i < pages; i++) {
-			uint64_t addr = gva + (i * perf_test_args.guest_page_size);
-
-			if (i % perf_test_args.wr_fract == 0)
-				*(uint64_t *)addr = 0x0123456789ABCDEF;
-			else
-				READ_ONCE(*(uint64_t *)addr);
-		}
-
-		GUEST_SYNC(1);
-	}
-}
-
-static struct kvm_vm *create_vm(enum vm_guest_mode mode, int vcpus,
-				uint64_t vcpu_memory_bytes)
-{
-	struct kvm_vm *vm;
-	uint64_t guest_num_pages;
-
-	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
-
-	perf_test_args.host_page_size = getpagesize();
-	perf_test_args.guest_page_size = vm_guest_mode_params[mode].page_size;
-
-	guest_num_pages = vm_adjust_num_guest_pages(mode,
-				(vcpus * vcpu_memory_bytes) / perf_test_args.guest_page_size);
-
-	TEST_ASSERT(vcpu_memory_bytes % perf_test_args.host_page_size == 0,
-		    "Guest memory size is not host page size aligned.");
-	TEST_ASSERT(vcpu_memory_bytes % perf_test_args.guest_page_size == 0,
-		    "Guest memory size is not guest page size aligned.");
-
-	vm = vm_create_with_vcpus(mode, vcpus, 0,
-				  vcpu_memory_bytes / perf_test_args.guest_page_size,
-				  guest_code, NULL);
-
-	perf_test_args.vm = vm;
-
-	/*
-	 * If there should be more memory in the guest test region than there
-	 * can be pages in the guest, it will definitely cause problems.
-	 */
-	TEST_ASSERT(guest_num_pages < vm_get_max_gfn(vm),
-		    "Requested more guest memory than address space allows.\n"
-		    "    guest pages: %lx max gfn: %x vcpus: %d wss: %lx]\n",
-		    guest_num_pages, vm_get_max_gfn(vm), vcpus,
-		    vcpu_memory_bytes);
-
-	guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) *
-			      perf_test_args.guest_page_size;
-	guest_test_phys_mem &= ~(perf_test_args.host_page_size - 1);
-#ifdef __s390x__
-	/* Align to 1M (segment size) */
-	guest_test_phys_mem &= ~((1 << 20) - 1);
-#endif
-	pr_info("guest physical test memory offset: 0x%lx\n", guest_test_phys_mem);
-
-	/* Add an extra memory slot for testing */
-	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
-				    guest_test_phys_mem,
-				    TEST_MEM_SLOT_INDEX,
-				    guest_num_pages, 0);
-
-	/* Do mapping for the demand paging memory slot */
-	virt_map(vm, guest_test_virt_mem, guest_test_phys_mem, guest_num_pages, 0);
-
-	ucall_init(vm, NULL);
-
-	return vm;
-}
-
-static void add_vcpus(struct kvm_vm *vm, int vcpus, uint64_t vcpu_memory_bytes)
-{
-	vm_paddr_t vcpu_gpa;
-	struct vcpu_args *vcpu_args;
-	int vcpu_id;
-
-	for (vcpu_id = 0; vcpu_id < vcpus; vcpu_id++) {
-		vcpu_args = &perf_test_args.vcpu_args[vcpu_id];
-
-#ifdef __x86_64__
-		vcpu_set_cpuid(vm, vcpu_id, kvm_get_supported_cpuid());
-#endif
-
-		vcpu_args->vcpu_id = vcpu_id;
-		vcpu_args->gva = guest_test_virt_mem +
-				 (vcpu_id * vcpu_memory_bytes);
-		vcpu_args->pages = vcpu_memory_bytes /
-				   perf_test_args.guest_page_size;
+extern uint64_t guest_test_phys_mem;
 
-		vcpu_gpa = guest_test_phys_mem + (vcpu_id * vcpu_memory_bytes);
-		pr_debug("Added VCPU %d with test mem gpa [%lx, %lx)\n",
-			 vcpu_id, vcpu_gpa, vcpu_gpa + vcpu_memory_bytes);
-	}
-}
+struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
+				uint64_t vcpu_memory_bytes);
+void perf_test_add_vcpus(struct kvm_vm *vm, int vcpus, uint64_t vcpu_memory_bytes);
 
 #endif /* SELFTEST_KVM_PERF_TEST_UTIL_H */
diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
new file mode 100644
index 000000000000..e89d0c5a6917
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020, Google LLC.
+ */
+
+#include "kvm_util.h"
+#include "perf_test_util.h"
+#include "processor.h"
+
+struct perf_test_args perf_test_args;
+
+uint64_t guest_test_phys_mem;
+
+/*
+ * Guest virtual memory offset of the testing memory slot.
+ * Must not conflict with identity mapped test code.
+ */
+static uint64_t guest_test_virt_mem = DEFAULT_GUEST_TEST_MEM;
+
+/*
+ * Continuously write to the first 8 bytes of each page in the
+ * specified region.
+ */
+static void guest_code(uint32_t vcpu_id)
+{
+	struct perf_test_vcpu_args *vcpu_args = &perf_test_args.vcpu_args[vcpu_id];
+	uint64_t gva;
+	uint64_t pages;
+	int i;
+
+	/* Make sure vCPU args data structure is not corrupt. */
+	GUEST_ASSERT(vcpu_args->vcpu_id == vcpu_id);
+
+	gva = vcpu_args->gva;
+	pages = vcpu_args->pages;
+
+	while (true) {
+		for (i = 0; i < pages; i++) {
+			uint64_t addr = gva + (i * perf_test_args.guest_page_size);
+
+			if (i % perf_test_args.wr_fract == 0)
+				*(uint64_t *)addr = 0x0123456789ABCDEF;
+			else
+				READ_ONCE(*(uint64_t *)addr);
+		}
+
+		GUEST_SYNC(1);
+	}
+}
+
+#define TEST_MEM_SLOT_INDEX 1
+
+struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
+				   uint64_t vcpu_memory_bytes)
+{
+	struct kvm_vm *vm;
+	uint64_t guest_num_pages;
+
+	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
+
+	perf_test_args.host_page_size = getpagesize();
+	perf_test_args.guest_page_size = vm_guest_mode_params[mode].page_size;
+
+	guest_num_pages = vm_adjust_num_guest_pages(mode,
+				(vcpus * vcpu_memory_bytes) / perf_test_args.guest_page_size);
+
+	TEST_ASSERT(vcpu_memory_bytes % perf_test_args.host_page_size == 0,
+		    "Guest memory size is not host page size aligned.");
+	TEST_ASSERT(vcpu_memory_bytes % perf_test_args.guest_page_size == 0,
+		    "Guest memory size is not guest page size aligned.");
+
+	vm = vm_create_with_vcpus(mode, vcpus, 0,
+				  vcpu_memory_bytes / perf_test_args.guest_page_size,
+				  guest_code, NULL);
+
+	perf_test_args.vm = vm;
+
+	/*
+	 * If there should be more memory in the guest test region than there
+	 * can be pages in the guest, it will definitely cause problems.
+	 */
+	TEST_ASSERT(guest_num_pages < vm_get_max_gfn(vm),
+		    "Requested more guest memory than address space allows.\n"
+		    "    guest pages: %lx max gfn: %x vcpus: %d wss: %lx]\n",
+		    guest_num_pages, vm_get_max_gfn(vm), vcpus,
+		    vcpu_memory_bytes);
+
+	guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) *
+			      perf_test_args.guest_page_size;
+	guest_test_phys_mem &= ~(perf_test_args.host_page_size - 1);
+#ifdef __s390x__
+	/* Align to 1M (segment size) */
+	guest_test_phys_mem &= ~((1 << 20) - 1);
+#endif
+	pr_info("guest physical test memory offset: 0x%lx\n", guest_test_phys_mem);
+
+	/* Add an extra memory slot for testing */
+	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
+				    guest_test_phys_mem,
+				    TEST_MEM_SLOT_INDEX,
+				    guest_num_pages, 0);
+
+	/* Do mapping for the demand paging memory slot */
+	virt_map(vm, guest_test_virt_mem, guest_test_phys_mem, guest_num_pages, 0);
+
+	ucall_init(vm, NULL);
+
+	return vm;
+}
+
+void perf_test_add_vcpus(struct kvm_vm *vm, int vcpus, uint64_t vcpu_memory_bytes)
+{
+	vm_paddr_t vcpu_gpa;
+	struct perf_test_vcpu_args *vcpu_args;
+	int vcpu_id;
+
+	for (vcpu_id = 0; vcpu_id < vcpus; vcpu_id++) {
+		vcpu_args = &perf_test_args.vcpu_args[vcpu_id];
+
+#ifdef __x86_64__
+		vcpu_set_cpuid(vm, vcpu_id, kvm_get_supported_cpuid());
+#endif
+
+		vcpu_args->vcpu_id = vcpu_id;
+		vcpu_args->gva = guest_test_virt_mem +
+				 (vcpu_id * vcpu_memory_bytes);
+		vcpu_args->pages = vcpu_memory_bytes /
+				   perf_test_args.guest_page_size;
+
+		vcpu_gpa = guest_test_phys_mem + (vcpu_id * vcpu_memory_bytes);
+		pr_debug("Added VCPU %d with test mem gpa [%lx, %lx)\n",
+			 vcpu_id, vcpu_gpa, vcpu_gpa + vcpu_memory_bytes);
+	}
+}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH 2/8] KVM: selftests: Remove deadcode
  2020-11-10 20:47 ` [PATCH 2/8] KVM: selftests: Remove deadcode Andrew Jones
@ 2020-11-10 21:47   ` Ben Gardon
  0 siblings, 0 replies; 15+ messages in thread
From: Ben Gardon @ 2020-11-10 21:47 UTC (permalink / raw)
  To: Andrew Jones
  Cc: kvm, Paolo Bonzini, Christian Borntraeger, Janosch Frank, Peter Xu

On Tue, Nov 10, 2020 at 12:48 PM Andrew Jones <drjones@redhat.com> wrote:
>
> Nothing sets USE_CLEAR_DIRTY_LOG anymore, so anything it surrounds
> is dead code.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Reviewed-by: Ben Gardon <bgardon@google.com>

> ---
>  .../selftests/kvm/dirty_log_perf_test.c       | 44 -------------------
>  1 file changed, 44 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index 85c9b8f73142..b9115e8ef0ed 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -88,10 +88,6 @@ static void *vcpu_worker(void *data)
>         return NULL;
>  }
>
> -#ifdef USE_CLEAR_DIRTY_LOG
> -static u64 dirty_log_manual_caps;
> -#endif
> -
>  static void run_test(enum vm_guest_mode mode, unsigned long iterations,
>                      uint64_t phys_offset, int wr_fract)
>  {
> @@ -106,10 +102,6 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
>         struct timespec get_dirty_log_total = (struct timespec){0};
>         struct timespec vcpu_dirty_total = (struct timespec){0};
>         struct timespec avg;
> -#ifdef USE_CLEAR_DIRTY_LOG
> -       struct kvm_enable_cap cap = {};
> -       struct timespec clear_dirty_log_total = (struct timespec){0};
> -#endif
>
>         vm = create_vm(mode, nr_vcpus, guest_percpu_mem_size);
>
> @@ -120,12 +112,6 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
>         host_num_pages = vm_num_host_pages(mode, guest_num_pages);
>         bmap = bitmap_alloc(host_num_pages);
>
> -#ifdef USE_CLEAR_DIRTY_LOG
> -       cap.cap = KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2;
> -       cap.args[0] = dirty_log_manual_caps;
> -       vm_enable_cap(vm, &cap);
> -#endif
> -
>         vcpu_threads = malloc(nr_vcpus * sizeof(*vcpu_threads));
>         TEST_ASSERT(vcpu_threads, "Memory allocation failed");
>
> @@ -189,18 +175,6 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
>                                                    ts_diff);
>                 pr_info("Iteration %lu get dirty log time: %ld.%.9lds\n",
>                         iteration, ts_diff.tv_sec, ts_diff.tv_nsec);
> -
> -#ifdef USE_CLEAR_DIRTY_LOG
> -               clock_gettime(CLOCK_MONOTONIC, &start);
> -               kvm_vm_clear_dirty_log(vm, TEST_MEM_SLOT_INDEX, bmap, 0,
> -                                      host_num_pages);
> -
> -               ts_diff = timespec_diff_now(start);
> -               clear_dirty_log_total = timespec_add(clear_dirty_log_total,
> -                                                    ts_diff);
> -               pr_info("Iteration %lu clear dirty log time: %ld.%.9lds\n",
> -                       iteration, ts_diff.tv_sec, ts_diff.tv_nsec);
> -#endif
>         }
>
>         /* Tell the vcpu thread to quit */
> @@ -220,13 +194,6 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
>                 iterations, get_dirty_log_total.tv_sec,
>                 get_dirty_log_total.tv_nsec, avg.tv_sec, avg.tv_nsec);
>
> -#ifdef USE_CLEAR_DIRTY_LOG
> -       avg = timespec_div(clear_dirty_log_total, iterations);
> -       pr_info("Clear dirty log over %lu iterations took %ld.%.9lds. (Avg %ld.%.9lds/iteration)\n",
> -               iterations, clear_dirty_log_total.tv_sec,
> -               clear_dirty_log_total.tv_nsec, avg.tv_sec, avg.tv_nsec);
> -#endif
> -
>         free(bmap);
>         free(vcpu_threads);
>         ucall_uninit(vm);
> @@ -284,17 +251,6 @@ int main(int argc, char *argv[])
>         int opt, i;
>         int wr_fract = 1;
>
> -#ifdef USE_CLEAR_DIRTY_LOG
> -       dirty_log_manual_caps =
> -               kvm_check_cap(KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2);
> -       if (!dirty_log_manual_caps) {
> -               print_skip("KVM_CLEAR_DIRTY_LOG not available");
> -               exit(KSFT_SKIP);
> -       }
> -       dirty_log_manual_caps &= (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE |
> -                                 KVM_DIRTY_LOG_INITIALLY_SET);
> -#endif
> -
>  #ifdef __x86_64__
>         guest_mode_init(VM_MODE_PXXV48_4K, true, true);
>  #endif
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/8] KVM: selftests: Factor out guest mode code
  2020-11-10 20:47 ` [PATCH 3/8] KVM: selftests: Factor out guest mode code Andrew Jones
@ 2020-11-10 21:52   ` Ben Gardon
  2020-11-11  8:13     ` Andrew Jones
  0 siblings, 1 reply; 15+ messages in thread
From: Ben Gardon @ 2020-11-10 21:52 UTC (permalink / raw)
  To: Andrew Jones
  Cc: kvm, Paolo Bonzini, Christian Borntraeger, Janosch Frank, Peter Xu

On Tue, Nov 10, 2020 at 12:48 PM Andrew Jones <drjones@redhat.com> wrote:
>
> demand_paging_test, dirty_log_test, and dirty_log_perf_test have
> redundant guest mode code. Factor it out.
>
> Also, while adding a new include, remove the ones we don't need.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  tools/testing/selftests/kvm/Makefile          |   2 +-
>  .../selftests/kvm/demand_paging_test.c        | 107 ++++-----------
>  .../selftests/kvm/dirty_log_perf_test.c       | 119 +++++------------
>  tools/testing/selftests/kvm/dirty_log_test.c  | 125 ++++++------------
>  .../selftests/kvm/include/guest_modes.h       |  21 +++
>  tools/testing/selftests/kvm/lib/guest_modes.c |  70 ++++++++++
>  6 files changed, 188 insertions(+), 256 deletions(-)
>  create mode 100644 tools/testing/selftests/kvm/include/guest_modes.h
>  create mode 100644 tools/testing/selftests/kvm/lib/guest_modes.c
>
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 3d14ef77755e..ca6b64d9ab64 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -33,7 +33,7 @@ ifeq ($(ARCH),s390)
>         UNAME_M := s390x
>  endif
>
> -LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c lib/test_util.c
> +LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c
>  LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S
>  LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
>  LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c
> diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
> index 3d96a7bfaff3..946161a9ce2d 100644
> --- a/tools/testing/selftests/kvm/demand_paging_test.c
> +++ b/tools/testing/selftests/kvm/demand_paging_test.c
> @@ -7,23 +7,20 @@
>   * Copyright (C) 2019, Google, Inc.
>   */
>
> -#define _GNU_SOURCE /* for program_invocation_name */
> +#define _GNU_SOURCE /* for program_invocation_name and pipe2 */

What is the purpose of pipe2 in this patch / why add it to this
comment but not the comments in the other files modified here?

>
>  #include <stdio.h>
>  #include <stdlib.h>
> -#include <sys/syscall.h>
> -#include <unistd.h>
> -#include <asm/unistd.h>
>  #include <time.h>
>  #include <poll.h>
>  #include <pthread.h>
> -#include <linux/bitmap.h>
> -#include <linux/bitops.h>
>  #include <linux/userfaultfd.h>
> +#include <sys/syscall.h>
>
> -#include "perf_test_util.h"
> -#include "processor.h"
> +#include "kvm_util.h"
>  #include "test_util.h"
> +#include "perf_test_util.h"
> +#include "guest_modes.h"
>
>  #ifdef __NR_userfaultfd
>
> @@ -248,9 +245,14 @@ static int setup_demand_paging(struct kvm_vm *vm,
>         return 0;
>  }
>
> -static void run_test(enum vm_guest_mode mode, bool use_uffd,
> -                    useconds_t uffd_delay)
> +struct test_params {
> +       bool use_uffd;
> +       useconds_t uffd_delay;
> +};
> +
> +static void run_test(enum vm_guest_mode mode, void *arg)
>  {
> +       struct test_params *p = arg;
>         pthread_t *vcpu_threads;
>         pthread_t *uffd_handler_threads = NULL;
>         struct uffd_handler_args *uffd_args = NULL;
> @@ -275,7 +277,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
>
>         add_vcpus(vm, nr_vcpus, guest_percpu_mem_size);
>
> -       if (use_uffd) {
> +       if (p->use_uffd) {
>                 uffd_handler_threads =
>                         malloc(nr_vcpus * sizeof(*uffd_handler_threads));
>                 TEST_ASSERT(uffd_handler_threads, "Memory allocation failed");
> @@ -308,7 +310,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
>                         r = setup_demand_paging(vm,
>                                                 &uffd_handler_threads[vcpu_id],
>                                                 pipefds[vcpu_id * 2],
> -                                               uffd_delay, &uffd_args[vcpu_id],
> +                                               p->uffd_delay, &uffd_args[vcpu_id],
>                                                 vcpu_hva, guest_percpu_mem_size);
>                         if (r < 0)
>                                 exit(-r);
> @@ -339,7 +341,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
>
>         pr_info("All vCPU threads joined\n");
>
> -       if (use_uffd) {
> +       if (p->use_uffd) {
>                 char c;
>
>                 /* Tell the user fault fd handler threads to quit */
> @@ -362,38 +364,19 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
>
>         free(guest_data_prototype);
>         free(vcpu_threads);
> -       if (use_uffd) {
> +       if (p->use_uffd) {
>                 free(uffd_handler_threads);
>                 free(uffd_args);
>                 free(pipefds);
>         }
>  }
>
> -struct guest_mode {
> -       bool supported;
> -       bool enabled;
> -};
> -static struct guest_mode guest_modes[NUM_VM_MODES];
> -
> -#define guest_mode_init(mode, supported, enabled) ({ \
> -       guest_modes[mode] = (struct guest_mode){ supported, enabled }; \
> -})
> -
>  static void help(char *name)
>  {
> -       int i;
> -
>         puts("");
>         printf("usage: %s [-h] [-m mode] [-u] [-d uffd_delay_usec]\n"
>                "          [-b memory] [-v vcpus]\n", name);
> -       printf(" -m: specify the guest mode ID to test\n"
> -              "     (default: test all supported modes)\n"
> -              "     This option may be used multiple times.\n"
> -              "     Guest mode IDs:\n");
> -       for (i = 0; i < NUM_VM_MODES; ++i) {
> -               printf("         %d:    %s%s\n", i, vm_guest_mode_string(i),
> -                      guest_modes[i].supported ? " (supported)" : "");
> -       }
> +       guest_modes_help();
>         printf(" -u: use User Fault FD to handle vCPU page\n"
>                "     faults.\n");
>         printf(" -d: add a delay in usec to the User Fault\n"
> @@ -410,53 +393,22 @@ static void help(char *name)
>  int main(int argc, char *argv[])
>  {
>         int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
> -       bool mode_selected = false;
> -       unsigned int mode;
> -       int opt, i;
> -       bool use_uffd = false;
> -       useconds_t uffd_delay = 0;
> -
> -#ifdef __x86_64__
> -       guest_mode_init(VM_MODE_PXXV48_4K, true, true);
> -#endif
> -#ifdef __aarch64__
> -       guest_mode_init(VM_MODE_P40V48_4K, true, true);
> -       guest_mode_init(VM_MODE_P40V48_64K, true, true);
> -       {
> -               unsigned int limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
> -
> -               if (limit >= 52)
> -                       guest_mode_init(VM_MODE_P52V48_64K, true, true);
> -               if (limit >= 48) {
> -                       guest_mode_init(VM_MODE_P48V48_4K, true, true);
> -                       guest_mode_init(VM_MODE_P48V48_64K, true, true);
> -               }
> -       }
> -#endif
> -#ifdef __s390x__
> -       guest_mode_init(VM_MODE_P40V48_4K, true, true);
> -#endif
> +       struct test_params p = {};
> +       int opt;
> +
> +       guest_modes_append_default();
>
>         while ((opt = getopt(argc, argv, "hm:ud:b:v:")) != -1) {
>                 switch (opt) {
>                 case 'm':
> -                       if (!mode_selected) {
> -                               for (i = 0; i < NUM_VM_MODES; ++i)
> -                                       guest_modes[i].enabled = false;
> -                               mode_selected = true;
> -                       }
> -                       mode = strtoul(optarg, NULL, 10);
> -                       TEST_ASSERT(mode < NUM_VM_MODES,
> -                                   "Guest mode ID %d too big", mode);
> -                       guest_modes[mode].enabled = true;
> +                       guest_modes_cmdline(optarg);
>                         break;
>                 case 'u':
> -                       use_uffd = true;
> +                       p.use_uffd = true;
>                         break;
>                 case 'd':
> -                       uffd_delay = strtoul(optarg, NULL, 0);
> -                       TEST_ASSERT(uffd_delay >= 0,
> -                                   "A negative UFFD delay is not supported.");
> +                       p.uffd_delay = strtoul(optarg, NULL, 0);
> +                       TEST_ASSERT(p.uffd_delay >= 0, "A negative UFFD delay is not supported.");
>                         break;
>                 case 'b':
>                         guest_percpu_mem_size = parse_size(optarg);
> @@ -473,14 +425,7 @@ int main(int argc, char *argv[])
>                 }
>         }
>
> -       for (i = 0; i < NUM_VM_MODES; ++i) {
> -               if (!guest_modes[i].enabled)
> -                       continue;
> -               TEST_ASSERT(guest_modes[i].supported,
> -                           "Guest mode ID %d (%s) not supported.",
> -                           i, vm_guest_mode_string(i));
> -               run_test(i, use_uffd, uffd_delay);
> -       }
> +       for_each_guest_mode(run_test, &p);
>
>         return 0;
>  }
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index b9115e8ef0ed..b448c17bd7aa 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -12,16 +12,14 @@
>
>  #include <stdio.h>
>  #include <stdlib.h>
> -#include <unistd.h>
>  #include <time.h>
>  #include <pthread.h>
>  #include <linux/bitmap.h>
> -#include <linux/bitops.h>
>
>  #include "kvm_util.h"
> -#include "perf_test_util.h"
> -#include "processor.h"
>  #include "test_util.h"
> +#include "perf_test_util.h"
> +#include "guest_modes.h"
>
>  /* How many host loops to run by default (one KVM_GET_DIRTY_LOG for each loop)*/
>  #define TEST_HOST_LOOP_N               2UL
> @@ -88,9 +86,15 @@ static void *vcpu_worker(void *data)
>         return NULL;
>  }
>
> -static void run_test(enum vm_guest_mode mode, unsigned long iterations,
> -                    uint64_t phys_offset, int wr_fract)
> +struct test_params {
> +       unsigned long iterations;
> +       uint64_t phys_offset;
> +       int wr_fract;
> +};
> +
> +static void run_test(enum vm_guest_mode mode, void *arg)
>  {
> +       struct test_params *p = arg;
>         pthread_t *vcpu_threads;
>         struct kvm_vm *vm;
>         unsigned long *bmap;
> @@ -105,7 +109,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
>
>         vm = create_vm(mode, nr_vcpus, guest_percpu_mem_size);
>
> -       perf_test_args.wr_fract = wr_fract;
> +       perf_test_args.wr_fract = p->wr_fract;
>
>         guest_num_pages = (nr_vcpus * guest_percpu_mem_size) >> vm_get_page_shift(vm);
>         guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
> @@ -147,7 +151,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
>         pr_info("Enabling dirty logging time: %ld.%.9lds\n\n",
>                 ts_diff.tv_sec, ts_diff.tv_nsec);
>
> -       while (iteration < iterations) {
> +       while (iteration < p->iterations) {
>                 /*
>                  * Incrementing the iteration number will start the vCPUs
>                  * dirtying memory again.
> @@ -189,9 +193,9 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
>         pr_info("Disabling dirty logging time: %ld.%.9lds\n",
>                 ts_diff.tv_sec, ts_diff.tv_nsec);
>
> -       avg = timespec_div(get_dirty_log_total, iterations);
> +       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",
> -               iterations, get_dirty_log_total.tv_sec,
> +               p->iterations, get_dirty_log_total.tv_sec,
>                 get_dirty_log_total.tv_nsec, avg.tv_sec, avg.tv_nsec);
>
>         free(bmap);
> @@ -200,20 +204,8 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
>         kvm_vm_free(vm);
>  }
>
> -struct guest_mode {
> -       bool supported;
> -       bool enabled;
> -};
> -static struct guest_mode guest_modes[NUM_VM_MODES];
> -
> -#define guest_mode_init(mode, supported, enabled) ({ \
> -       guest_modes[mode] = (struct guest_mode){ supported, enabled }; \
> -})
> -
>  static void help(char *name)
>  {
> -       int i;
> -
>         puts("");
>         printf("usage: %s [-h] [-i iterations] [-p offset] "
>                "[-m mode] [-b vcpu bytes] [-v vcpus]\n", name);
> @@ -222,14 +214,7 @@ static void help(char *name)
>                TEST_HOST_LOOP_N);
>         printf(" -p: specify guest physical test memory offset\n"
>                "     Warning: a low offset can conflict with the loaded test code.\n");
> -       printf(" -m: specify the guest mode ID to test "
> -              "(default: test all supported modes)\n"
> -              "     This option may be used multiple times.\n"
> -              "     Guest mode IDs:\n");
> -       for (i = 0; i < NUM_VM_MODES; ++i) {
> -               printf("         %d:    %s%s\n", i, vm_guest_mode_string(i),
> -                      guest_modes[i].supported ? " (supported)" : "");
> -       }
> +       guest_modes_help();
>         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");
> @@ -244,69 +229,38 @@ static void help(char *name)
>
>  int main(int argc, char *argv[])
>  {
> -       unsigned long iterations = TEST_HOST_LOOP_N;
> -       bool mode_selected = false;
> -       uint64_t phys_offset = 0;
> -       unsigned int mode;
> -       int opt, i;
> -       int wr_fract = 1;
> -
> -#ifdef __x86_64__
> -       guest_mode_init(VM_MODE_PXXV48_4K, true, true);
> -#endif
> -#ifdef __aarch64__
> -       guest_mode_init(VM_MODE_P40V48_4K, true, true);
> -       guest_mode_init(VM_MODE_P40V48_64K, true, true);
> -
> -       {
> -               unsigned int limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
> -
> -               if (limit >= 52)
> -                       guest_mode_init(VM_MODE_P52V48_64K, true, true);
> -               if (limit >= 48) {
> -                       guest_mode_init(VM_MODE_P48V48_4K, true, true);
> -                       guest_mode_init(VM_MODE_P48V48_64K, true, true);
> -               }
> -       }
> -#endif
> -#ifdef __s390x__
> -       guest_mode_init(VM_MODE_P40V48_4K, true, true);
> -#endif
> +       int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
> +       struct test_params p = {
> +               .iterations = TEST_HOST_LOOP_N,
> +               .wr_fract = 1,
> +       };
> +       int opt;
> +
> +       guest_modes_append_default();
>
>         while ((opt = getopt(argc, argv, "hi:p:m:b:f:v:")) != -1) {
>                 switch (opt) {
>                 case 'i':
> -                       iterations = strtol(optarg, NULL, 10);
> +                       p.iterations = strtol(optarg, NULL, 10);
>                         break;
>                 case 'p':
> -                       phys_offset = strtoull(optarg, NULL, 0);
> +                       p.phys_offset = strtoull(optarg, NULL, 0);
>                         break;
>                 case 'm':
> -                       if (!mode_selected) {
> -                               for (i = 0; i < NUM_VM_MODES; ++i)
> -                                       guest_modes[i].enabled = false;
> -                               mode_selected = true;
> -                       }
> -                       mode = strtoul(optarg, NULL, 10);
> -                       TEST_ASSERT(mode < NUM_VM_MODES,
> -                                   "Guest mode ID %d too big", mode);
> -                       guest_modes[mode].enabled = true;
> +                       guest_modes_cmdline(optarg);
>                         break;
>                 case 'b':
>                         guest_percpu_mem_size = parse_size(optarg);
>                         break;
>                 case 'f':
> -                       wr_fract = atoi(optarg);
> -                       TEST_ASSERT(wr_fract >= 1,
> +                       p.wr_fract = atoi(optarg);
> +                       TEST_ASSERT(p.wr_fract >= 1,
>                                     "Write fraction cannot be less than one");
>                         break;
>                 case 'v':
>                         nr_vcpus = atoi(optarg);
> -                       TEST_ASSERT(nr_vcpus > 0,
> -                                   "Must have a positive number of vCPUs");
> -                       TEST_ASSERT(nr_vcpus <= MAX_VCPUS,
> -                                   "This test does not currently support\n"
> -                                   "more than %d vCPUs.", MAX_VCPUS);
> +                       TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
> +                                   "Invalid number of vcpus, must be between 1 and %d", max_vcpus);
>                         break;
>                 case 'h':
>                 default:
> @@ -315,18 +269,11 @@ int main(int argc, char *argv[])
>                 }
>         }
>
> -       TEST_ASSERT(iterations >= 2, "The test should have at least two iterations");
> +       TEST_ASSERT(p.iterations >= 2, "The test should have at least two iterations");
>
> -       pr_info("Test iterations: %"PRIu64"\n", iterations);
> +       pr_info("Test iterations: %"PRIu64"\n", p.iterations);
>
> -       for (i = 0; i < NUM_VM_MODES; ++i) {
> -               if (!guest_modes[i].enabled)
> -                       continue;
> -               TEST_ASSERT(guest_modes[i].supported,
> -                           "Guest mode ID %d (%s) not supported.",
> -                           i, vm_guest_mode_string(i));
> -               run_test(i, iterations, phys_offset, wr_fract);
> -       }
> +       for_each_guest_mode(run_test, &p);
>
>         return 0;
>  }
> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> index 54da9cc20db4..1b7375d2acea 100644
> --- a/tools/testing/selftests/kvm/dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> @@ -9,14 +9,13 @@
>
>  #include <stdio.h>
>  #include <stdlib.h>
> -#include <unistd.h>
> -#include <time.h>
>  #include <pthread.h>
>  #include <linux/bitmap.h>
>  #include <linux/bitops.h>
>
> -#include "test_util.h"
>  #include "kvm_util.h"
> +#include "test_util.h"
> +#include "guest_modes.h"
>  #include "processor.h"
>
>  #define VCPU_ID                                1
> @@ -375,9 +374,15 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, uint32_t vcpuid,
>  #define DIRTY_MEM_BITS 30 /* 1G */
>  #define PAGE_SHIFT_4K  12
>
> -static void run_test(enum vm_guest_mode mode, unsigned long iterations,
> -                    unsigned long interval, uint64_t phys_offset)
> +struct test_params {
> +       unsigned long iterations;
> +       unsigned long interval;
> +       uint64_t phys_offset;
> +};
> +
> +static void run_test(enum vm_guest_mode mode, void *arg)
>  {
> +       struct test_params *p = arg;
>         pthread_t vcpu_thread;
>         struct kvm_vm *vm;
>         unsigned long *bmap;
> @@ -412,12 +417,12 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
>         host_page_size = getpagesize();
>         host_num_pages = vm_num_host_pages(mode, guest_num_pages);
>
> -       if (!phys_offset) {
> +       if (!p->phys_offset) {
>                 guest_test_phys_mem = (vm_get_max_gfn(vm) -
>                                        guest_num_pages) * guest_page_size;
>                 guest_test_phys_mem &= ~(host_page_size - 1);
>         } else {
> -               guest_test_phys_mem = phys_offset;
> +               guest_test_phys_mem = p->phys_offset;
>         }
>
>  #ifdef __s390x__
> @@ -464,9 +469,9 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
>
>         pthread_create(&vcpu_thread, NULL, vcpu_worker, vm);
>
> -       while (iteration < iterations) {
> +       while (iteration < p->iterations) {
>                 /* Give the vcpu thread some time to dirty some pages */
> -               usleep(interval * 1000);
> +               usleep(p->interval * 1000);
>                 log_mode_collect_dirty_pages(vm, TEST_MEM_SLOT_INDEX,
>                                              bmap, host_num_pages);
>                 vm_dirty_log_verify(mode, bmap);
> @@ -488,20 +493,8 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
>         kvm_vm_free(vm);
>  }
>
> -struct guest_mode {
> -       bool supported;
> -       bool enabled;
> -};
> -static struct guest_mode guest_modes[NUM_VM_MODES];
> -
> -#define guest_mode_init(mode, supported, enabled) ({ \
> -       guest_modes[mode] = (struct guest_mode){ supported, enabled }; \
> -})
> -
>  static void help(char *name)
>  {
> -       int i;
> -
>         puts("");
>         printf("usage: %s [-h] [-i iterations] [-I interval] "
>                "[-p offset] [-m mode]\n", name);
> @@ -515,70 +508,34 @@ static void help(char *name)
>         printf(" -M: specify the host logging mode "
>                "(default: run all log modes).  Supported modes: \n\t");
>         log_modes_dump();
> -       printf(" -m: specify the guest mode ID to test "
> -              "(default: test all supported modes)\n"
> -              "     This option may be used multiple times.\n"
> -              "     Guest mode IDs:\n");
> -       for (i = 0; i < NUM_VM_MODES; ++i) {
> -               printf("         %d:    %s%s\n", i, vm_guest_mode_string(i),
> -                      guest_modes[i].supported ? " (supported)" : "");
> -       }
> +       guest_modes_help();
>         puts("");
>         exit(0);
>  }
>
>  int main(int argc, char *argv[])
>  {
> -       unsigned long iterations = TEST_HOST_LOOP_N;
> -       unsigned long interval = TEST_HOST_LOOP_INTERVAL;
> -       bool mode_selected = false;
> -       uint64_t phys_offset = 0;
> -       unsigned int mode;
> -       int opt, i, j;
> -
> -#ifdef __x86_64__
> -       guest_mode_init(VM_MODE_PXXV48_4K, true, true);
> -#endif
> -#ifdef __aarch64__
> -       guest_mode_init(VM_MODE_P40V48_4K, true, true);
> -       guest_mode_init(VM_MODE_P40V48_64K, true, true);
> -
> -       {
> -               unsigned int limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
> +       struct test_params p = {
> +               .iterations = TEST_HOST_LOOP_N,
> +               .interval = TEST_HOST_LOOP_INTERVAL,
> +       };
> +       int opt, i;
>
> -               if (limit >= 52)
> -                       guest_mode_init(VM_MODE_P52V48_64K, true, true);
> -               if (limit >= 48) {
> -                       guest_mode_init(VM_MODE_P48V48_4K, true, true);
> -                       guest_mode_init(VM_MODE_P48V48_64K, true, true);
> -               }
> -       }
> -#endif
> -#ifdef __s390x__
> -       guest_mode_init(VM_MODE_P40V48_4K, true, true);
> -#endif
> +       guest_modes_append_default();
>
>         while ((opt = getopt(argc, argv, "hi:I:p:m:M:")) != -1) {
>                 switch (opt) {
>                 case 'i':
> -                       iterations = strtol(optarg, NULL, 10);
> +                       p.iterations = strtol(optarg, NULL, 10);
>                         break;
>                 case 'I':
> -                       interval = strtol(optarg, NULL, 10);
> +                       p.interval = strtol(optarg, NULL, 10);
>                         break;
>                 case 'p':
> -                       phys_offset = strtoull(optarg, NULL, 0);
> +                       p.phys_offset = strtoull(optarg, NULL, 0);
>                         break;
>                 case 'm':
> -                       if (!mode_selected) {
> -                               for (i = 0; i < NUM_VM_MODES; ++i)
> -                                       guest_modes[i].enabled = false;
> -                               mode_selected = true;
> -                       }
> -                       mode = strtoul(optarg, NULL, 10);
> -                       TEST_ASSERT(mode < NUM_VM_MODES,
> -                                   "Guest mode ID %d too big", mode);
> -                       guest_modes[mode].enabled = true;
> +                       guest_modes_cmdline(optarg);
>                         break;
>                 case 'M':
>                         if (!strcmp(optarg, "all")) {
> @@ -607,32 +564,24 @@ int main(int argc, char *argv[])
>                 }
>         }
>
> -       TEST_ASSERT(iterations > 2, "Iterations must be greater than two");
> -       TEST_ASSERT(interval > 0, "Interval must be greater than zero");
> +       TEST_ASSERT(p.iterations > 2, "Iterations must be greater than two");
> +       TEST_ASSERT(p.interval > 0, "Interval must be greater than zero");
>
>         pr_info("Test iterations: %"PRIu64", interval: %"PRIu64" (ms)\n",
> -               iterations, interval);
> +               p.iterations, p.interval);
>
>         srandom(time(0));
>
> -       for (i = 0; i < NUM_VM_MODES; ++i) {
> -               if (!guest_modes[i].enabled)
> -                       continue;
> -               TEST_ASSERT(guest_modes[i].supported,
> -                           "Guest mode ID %d (%s) not supported.",
> -                           i, vm_guest_mode_string(i));
> -               if (host_log_mode_option == LOG_MODE_ALL) {
> -                       /* Run each log mode */
> -                       for (j = 0; j < LOG_MODE_NUM; j++) {
> -                               pr_info("Testing Log Mode '%s'\n",
> -                                       log_modes[j].name);
> -                               host_log_mode = j;
> -                               run_test(i, iterations, interval, phys_offset);
> -                       }
> -               } else {
> -                       host_log_mode = host_log_mode_option;
> -                       run_test(i, iterations, interval, phys_offset);
> +       if (host_log_mode_option == LOG_MODE_ALL) {
> +               /* Run each log mode */
> +               for (i = 0; i < LOG_MODE_NUM; i++) {
> +                       pr_info("Testing Log Mode '%s'\n", log_modes[i].name);
> +                       host_log_mode = i;
> +                       for_each_guest_mode(run_test, &p);
>                 }
> +       } else {
> +               host_log_mode = host_log_mode_option;
> +               for_each_guest_mode(run_test, &p);
>         }
>
>         return 0;
> diff --git a/tools/testing/selftests/kvm/include/guest_modes.h b/tools/testing/selftests/kvm/include/guest_modes.h
> new file mode 100644
> index 000000000000..b691df33e64e
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/include/guest_modes.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2020, Red Hat, Inc.
> + */
> +#include "kvm_util.h"
> +
> +struct guest_mode {
> +       bool supported;
> +       bool enabled;
> +};
> +
> +extern struct guest_mode guest_modes[NUM_VM_MODES];
> +
> +#define guest_mode_append(mode, supported, enabled) ({ \
> +       guest_modes[mode] = (struct guest_mode){ supported, enabled }; \
> +})
> +
> +void guest_modes_append_default(void);
> +void for_each_guest_mode(void (*func)(enum vm_guest_mode, void *), void *arg);
> +void guest_modes_help(void);
> +void guest_modes_cmdline(const char *arg);
> diff --git a/tools/testing/selftests/kvm/lib/guest_modes.c b/tools/testing/selftests/kvm/lib/guest_modes.c
> new file mode 100644
> index 000000000000..25bff307c71f
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/lib/guest_modes.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020, Red Hat, Inc.
> + */
> +#include "guest_modes.h"
> +
> +struct guest_mode guest_modes[NUM_VM_MODES];
> +
> +void guest_modes_append_default(void)
> +{
> +       guest_mode_append(VM_MODE_DEFAULT, true, true);
> +
> +#ifdef __aarch64__
> +       guest_mode_append(VM_MODE_P40V48_64K, true, true);
> +       {
> +               unsigned int limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
> +               if (limit >= 52)
> +                       guest_mode_append(VM_MODE_P52V48_64K, true, true);
> +               if (limit >= 48) {
> +                       guest_mode_append(VM_MODE_P48V48_4K, true, true);
> +                       guest_mode_append(VM_MODE_P48V48_64K, true, true);
> +               }
> +       }
> +#endif
> +}
> +
> +void for_each_guest_mode(void (*func)(enum vm_guest_mode, void *), void *arg)
> +{
> +       int i;
> +
> +       for (i = 0; i < NUM_VM_MODES; ++i) {
> +               if (!guest_modes[i].enabled)
> +                       continue;
> +               TEST_ASSERT(guest_modes[i].supported,
> +                           "Guest mode ID %d (%s) not supported.",
> +                           i, vm_guest_mode_string(i));
> +               func(i, arg);
> +       }
> +}
> +
> +void guest_modes_help(void)
> +{
> +       int i;
> +
> +       printf(" -m: specify the guest mode ID to test\n"
> +              "     (default: test all supported modes)\n"
> +              "     This option may be used multiple times.\n"
> +              "     Guest mode IDs:\n");
> +       for (i = 0; i < NUM_VM_MODES; ++i) {
> +               printf("         %d:    %s%s\n", i, vm_guest_mode_string(i),
> +                      guest_modes[i].supported ? " (supported)" : "");
> +       }
> +}
> +
> +void guest_modes_cmdline(const char *arg)
> +{
> +       static bool mode_selected;
> +       unsigned int mode;
> +       int i;
> +
> +       if (!mode_selected) {
> +               for (i = 0; i < NUM_VM_MODES; ++i)
> +                       guest_modes[i].enabled = false;
> +               mode_selected = true;
> +       }
> +
> +       mode = strtoul(optarg, NULL, 10);
> +       TEST_ASSERT(mode < NUM_VM_MODES, "Guest mode ID %d too big", mode);
> +       guest_modes[mode].enabled = true;
> +}
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 1/8] KVM: selftests: Update .gitignore
  2020-11-10 20:47 ` [PATCH 1/8] KVM: selftests: Update .gitignore Andrew Jones
@ 2020-11-10 21:59   ` Ben Gardon
  0 siblings, 0 replies; 15+ messages in thread
From: Ben Gardon @ 2020-11-10 21:59 UTC (permalink / raw)
  To: Andrew Jones
  Cc: kvm, Paolo Bonzini, Christian Borntraeger, Janosch Frank, Peter Xu

On Tue, Nov 10, 2020 at 12:48 PM Andrew Jones <drjones@redhat.com> wrote:
>
> Add x86_64/tsc_msrs_test and remove clear_dirty_log_test.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Reviewed-by: Ben Gardon <bgardon@google.com>

> ---
>  tools/testing/selftests/kvm/.gitignore | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index 7a2c242b7152..ceff9f4c1781 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -18,13 +18,13 @@
>  /x86_64/vmx_preemption_timer_test
>  /x86_64/svm_vmcall_test
>  /x86_64/sync_regs_test
> +/x86_64/tsc_msrs_test
>  /x86_64/vmx_apic_access_test
>  /x86_64/vmx_close_while_nested_test
>  /x86_64/vmx_dirty_log_test
>  /x86_64/vmx_set_nested_state_test
>  /x86_64/vmx_tsc_adjust_test
>  /x86_64/xss_msr_test
> -/clear_dirty_log_test
>  /demand_paging_test
>  /dirty_log_test
>  /dirty_log_perf_test
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 5/8] KVM: selftests: Introduce vm_create_[default_]_with_vcpus
  2020-11-10 20:47 ` [PATCH 5/8] KVM: selftests: Introduce vm_create_[default_]_with_vcpus Andrew Jones
@ 2020-11-10 22:13   ` Ben Gardon
  2020-11-11  8:29     ` Andrew Jones
  0 siblings, 1 reply; 15+ messages in thread
From: Ben Gardon @ 2020-11-10 22:13 UTC (permalink / raw)
  To: Andrew Jones
  Cc: kvm, Paolo Bonzini, Christian Borntraeger, Janosch Frank, Peter Xu

On Tue, Nov 10, 2020 at 12:48 PM Andrew Jones <drjones@redhat.com> wrote:
>
> Introduce new vm_create variants that also takes a number of vcpus,
> an amount of per-vcpu pages, and optionally a list of vcpuids. These
> variants will create default VMs with enough additional pages to
> cover the vcpu stacks, per-vcpu pages, and pagetable pages for all.
> The new 'default' variant uses VM_MODE_DEFAULT, whereas the other
> new variant accepts the mode as a parameter.
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Reviewed-by: Ben Gardon <bgardon@google.com>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  .../testing/selftests/kvm/include/kvm_util.h  | 10 ++++++
>  tools/testing/selftests/kvm/lib/kvm_util.c    | 35 ++++++++++++++++---
>  2 files changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 48b48a0014e2..bc8db80309f5 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -261,6 +261,16 @@ vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
>  struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
>                                  void *guest_code);
>
> +/* Same as vm_create_default, but can be used for more than one vcpu */
> +struct kvm_vm *vm_create_default_with_vcpus(uint32_t nr_vcpus, uint64_t extra_mem_pages,
> +                                           uint32_t num_percpu_pages, void *guest_code,
> +                                           uint32_t vcpuids[]);
> +
> +/* Like vm_create_default_with_vcpus, but accepts mode as a parameter */
> +struct kvm_vm *vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus,
> +                                   uint64_t extra_mem_pages, uint32_t num_percpu_pages,
> +                                   void *guest_code, uint32_t vcpuids[]);
> +
>  /*
>   * Adds a vCPU with reasonable defaults (e.g. a stack)
>   *
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index a7e28e33fc3b..b31a4e988a5d 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -272,8 +272,9 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
>         return vm;
>  }
>
> -struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
> -                                void *guest_code)
> +struct kvm_vm *vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus,
> +                                   uint64_t extra_mem_pages, uint32_t num_percpu_pages,
> +                                   void *guest_code, uint32_t vcpuids[])
>  {
>         /* The maximum page table size for a memory region will be when the
>          * smallest pages are used. Considering each page contains x page
> @@ -281,10 +282,18 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
>          * N pages) will be: N/x+N/x^2+N/x^3+... which is definitely smaller
>          * than N/x*2.
>          */
> -       uint64_t extra_pg_pages = (extra_mem_pages / PTES_PER_MIN_PAGE) * 2;
> +       uint64_t vcpu_pages = (DEFAULT_STACK_PGS + num_percpu_pages) * nr_vcpus;
> +       uint64_t extra_pg_pages = (extra_mem_pages + vcpu_pages) / PTES_PER_MIN_PAGE * 2;
> +       uint64_t pages = DEFAULT_GUEST_PHY_PAGES + vcpu_pages + extra_pg_pages;
>         struct kvm_vm *vm;
> +       int i;
> +
> +       TEST_ASSERT(nr_vcpus <= kvm_check_cap(KVM_CAP_MAX_VCPUS),
> +                   "nr_vcpus = %d too large for host, max-vcpus = %d",
> +                   nr_vcpus, kvm_check_cap(KVM_CAP_MAX_VCPUS));
>
> -       vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES + extra_pg_pages, O_RDWR);
> +       pages = vm_adjust_num_guest_pages(mode, pages);
> +       vm = vm_create(mode, pages, O_RDWR);

I think this will substantially change the behavior of this function
to create a much larger memslot 0. In the existing code, the memslot
created in vm_create is just sized large enough for the stacks and
page tables. Another memslot is then created for the memory under
test.

I think separating the memslots is a good arrangement because it
limits the extent to which kernel bugs could screw up the test and
makes it easier to debug if you're testing something like dirty
logging. It's also useful if you wanted to back the memslot under test
with a different kind of memory from memslot 0. e.g. memslot 0 could
use anonymous pages and the slot(s) under test could use hugetlbfs.
You might also want multiple memslots to assign them to different NUMA
nodes.

Is that change intentional? I would suggest not adding vcpu_pages to
the calculation for pages above, similar to what it was before:
uint64_t pages = DEFAULT_GUEST_PHY_PAGES + extra_pg_pages;

>
>         kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
>
> @@ -292,11 +301,27 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
>         vm_create_irqchip(vm);
>  #endif
>
> -       vm_vcpu_add_default(vm, vcpuid, guest_code);
> +       for (i = 0; i < nr_vcpus; ++i)
> +               vm_vcpu_add_default(vm, vcpuids ? vcpuids[i] : i, guest_code);
>
>         return vm;
>  }
>
> +struct kvm_vm *vm_create_default_with_vcpus(uint32_t nr_vcpus, uint64_t extra_mem_pages,
> +                                           uint32_t num_percpu_pages, void *guest_code,
> +                                           uint32_t vcpuids[])
> +{
> +       return vm_create_with_vcpus(VM_MODE_DEFAULT, nr_vcpus, extra_mem_pages,
> +                                   num_percpu_pages, guest_code, vcpuids);
> +}
> +
> +struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
> +                                void *guest_code)
> +{
> +       return vm_create_default_with_vcpus(1, extra_mem_pages, 0, guest_code,
> +                                           (uint32_t []){ vcpuid });
> +}
> +
>  /*
>   * VM Restart
>   *
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 3/8] KVM: selftests: Factor out guest mode code
  2020-11-10 21:52   ` Ben Gardon
@ 2020-11-11  8:13     ` Andrew Jones
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2020-11-11  8:13 UTC (permalink / raw)
  To: Ben Gardon
  Cc: kvm, Paolo Bonzini, Christian Borntraeger, Janosch Frank, Peter Xu

On Tue, Nov 10, 2020 at 01:52:56PM -0800, Ben Gardon wrote:
> On Tue, Nov 10, 2020 at 12:48 PM Andrew Jones <drjones@redhat.com> wrote:
> >
> > demand_paging_test, dirty_log_test, and dirty_log_perf_test have
> > redundant guest mode code. Factor it out.
> >
> > Also, while adding a new include, remove the ones we don't need.
> >
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  tools/testing/selftests/kvm/Makefile          |   2 +-
> >  .../selftests/kvm/demand_paging_test.c        | 107 ++++-----------
> >  .../selftests/kvm/dirty_log_perf_test.c       | 119 +++++------------
> >  tools/testing/selftests/kvm/dirty_log_test.c  | 125 ++++++------------
> >  .../selftests/kvm/include/guest_modes.h       |  21 +++
> >  tools/testing/selftests/kvm/lib/guest_modes.c |  70 ++++++++++
> >  6 files changed, 188 insertions(+), 256 deletions(-)
> >  create mode 100644 tools/testing/selftests/kvm/include/guest_modes.h
> >  create mode 100644 tools/testing/selftests/kvm/lib/guest_modes.c
> >
> > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> > index 3d14ef77755e..ca6b64d9ab64 100644
> > --- a/tools/testing/selftests/kvm/Makefile
> > +++ b/tools/testing/selftests/kvm/Makefile
> > @@ -33,7 +33,7 @@ ifeq ($(ARCH),s390)
> >         UNAME_M := s390x
> >  endif
> >
> > -LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c lib/test_util.c
> > +LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c
> >  LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S
> >  LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
> >  LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c
> > diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
> > index 3d96a7bfaff3..946161a9ce2d 100644
> > --- a/tools/testing/selftests/kvm/demand_paging_test.c
> > +++ b/tools/testing/selftests/kvm/demand_paging_test.c
> > @@ -7,23 +7,20 @@
> >   * Copyright (C) 2019, Google, Inc.
> >   */
> >
> > -#define _GNU_SOURCE /* for program_invocation_name */
> > +#define _GNU_SOURCE /* for program_invocation_name and pipe2 */
> 
> What is the purpose of pipe2 in this patch / why add it to this
> comment but not the comments in the other files modified here?

Only this file uses pipe2. If we do a later cleanup removing the
program_invocation_name usage from this file, then I want to point out
that we need to keep _GNU_SOURCE defined for pipe2. Actually, the only
reason we still need program_invocation_name at this point in the
series is because program_invocation_name is used in perf_test_util.h
in a function we include in this file. I should have removed
program_invocation_name from the comment with the "KVM: selftests: Use
vm_create_with_vcpus in create_vm" patch, but I forgot.

Thanks,
drew


^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH 5/8] KVM: selftests: Introduce vm_create_[default_]_with_vcpus
  2020-11-10 22:13   ` Ben Gardon
@ 2020-11-11  8:29     ` Andrew Jones
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2020-11-11  8:29 UTC (permalink / raw)
  To: Ben Gardon
  Cc: kvm, Paolo Bonzini, Christian Borntraeger, Janosch Frank, Peter Xu

On Tue, Nov 10, 2020 at 02:13:14PM -0800, Ben Gardon wrote:
> On Tue, Nov 10, 2020 at 12:48 PM Andrew Jones <drjones@redhat.com> wrote:
> >
> > Introduce new vm_create variants that also takes a number of vcpus,
> > an amount of per-vcpu pages, and optionally a list of vcpuids. These
> > variants will create default VMs with enough additional pages to
> > cover the vcpu stacks, per-vcpu pages, and pagetable pages for all.
> > The new 'default' variant uses VM_MODE_DEFAULT, whereas the other
> > new variant accepts the mode as a parameter.
> >
> > Reviewed-by: Peter Xu <peterx@redhat.com>
> > Reviewed-by: Ben Gardon <bgardon@google.com>
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  .../testing/selftests/kvm/include/kvm_util.h  | 10 ++++++
> >  tools/testing/selftests/kvm/lib/kvm_util.c    | 35 ++++++++++++++++---
> >  2 files changed, 40 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > index 48b48a0014e2..bc8db80309f5 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > @@ -261,6 +261,16 @@ vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
> >  struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
> >                                  void *guest_code);
> >
> > +/* Same as vm_create_default, but can be used for more than one vcpu */
> > +struct kvm_vm *vm_create_default_with_vcpus(uint32_t nr_vcpus, uint64_t extra_mem_pages,
> > +                                           uint32_t num_percpu_pages, void *guest_code,
> > +                                           uint32_t vcpuids[]);
> > +
> > +/* Like vm_create_default_with_vcpus, but accepts mode as a parameter */
> > +struct kvm_vm *vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus,
> > +                                   uint64_t extra_mem_pages, uint32_t num_percpu_pages,
> > +                                   void *guest_code, uint32_t vcpuids[]);
> > +
> >  /*
> >   * Adds a vCPU with reasonable defaults (e.g. a stack)
> >   *
> > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> > index a7e28e33fc3b..b31a4e988a5d 100644
> > --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> > @@ -272,8 +272,9 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
> >         return vm;
> >  }
> >
> > -struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
> > -                                void *guest_code)
> > +struct kvm_vm *vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus,
> > +                                   uint64_t extra_mem_pages, uint32_t num_percpu_pages,
> > +                                   void *guest_code, uint32_t vcpuids[])
> >  {
> >         /* The maximum page table size for a memory region will be when the
> >          * smallest pages are used. Considering each page contains x page
> > @@ -281,10 +282,18 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
> >          * N pages) will be: N/x+N/x^2+N/x^3+... which is definitely smaller
> >          * than N/x*2.
> >          */
> > -       uint64_t extra_pg_pages = (extra_mem_pages / PTES_PER_MIN_PAGE) * 2;
> > +       uint64_t vcpu_pages = (DEFAULT_STACK_PGS + num_percpu_pages) * nr_vcpus;
> > +       uint64_t extra_pg_pages = (extra_mem_pages + vcpu_pages) / PTES_PER_MIN_PAGE * 2;
> > +       uint64_t pages = DEFAULT_GUEST_PHY_PAGES + vcpu_pages + extra_pg_pages;
> >         struct kvm_vm *vm;
> > +       int i;
> > +
> > +       TEST_ASSERT(nr_vcpus <= kvm_check_cap(KVM_CAP_MAX_VCPUS),
> > +                   "nr_vcpus = %d too large for host, max-vcpus = %d",
> > +                   nr_vcpus, kvm_check_cap(KVM_CAP_MAX_VCPUS));
> >
> > -       vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES + extra_pg_pages, O_RDWR);
> > +       pages = vm_adjust_num_guest_pages(mode, pages);
> > +       vm = vm_create(mode, pages, O_RDWR);
> 
> I think this will substantially change the behavior of this function
> to create a much larger memslot 0. In the existing code, the memslot
> created in vm_create is just sized large enough for the stacks and
> page tables. Another memslot is then created for the memory under
> test.
> 
> I think separating the memslots is a good arrangement because it
> limits the extent to which kernel bugs could screw up the test and
> makes it easier to debug if you're testing something like dirty
> logging. It's also useful if you wanted to back the memslot under test
> with a different kind of memory from memslot 0. e.g. memslot 0 could
> use anonymous pages and the slot(s) under test could use hugetlbfs.
> You might also want multiple memslots to assign them to different NUMA
> nodes.
> 
> Is that change intentional? I would suggest not adding vcpu_pages to
> the calculation for pages above, similar to what it was before:
> uint64_t pages = DEFAULT_GUEST_PHY_PAGES + extra_pg_pages;

It was definitely intentional to create API that allows us to allocate
per-vcpu pages for the VM created with vcpus. But, to do what you want,
we just need to change the

 vm = vm_create_with_vcpus(mode, vcpus, 0,
                           vcpu_memory_bytes / perf_test_args.guest_page_size,
                           guest_code, NULL);

line in perf_test_util.h in the "KVM: selftests: Use vm_create_with_vcpus
in create_vm" patch to


 vm = vm_create_with_vcpus(mode, vcpus,
                           (vcpus * vcpu_memory_bytes) / perf_test_args.guest_page_size,
                           0, guest_code, NULL);


Notice how that moves the per-vcpu pages to the extra_mem_pages, which
only get used to calculate page table pages.

I'll do that in v2.

Thanks,
drew


^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-11-11  8:29 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10 20:47 [PATCH 0/8] KVM: selftests: Cleanups, take 2 Andrew Jones
2020-11-10 20:47 ` [PATCH 1/8] KVM: selftests: Update .gitignore Andrew Jones
2020-11-10 21:59   ` Ben Gardon
2020-11-10 20:47 ` [PATCH 2/8] KVM: selftests: Remove deadcode Andrew Jones
2020-11-10 21:47   ` Ben Gardon
2020-11-10 20:47 ` [PATCH 3/8] KVM: selftests: Factor out guest mode code Andrew Jones
2020-11-10 21:52   ` Ben Gardon
2020-11-11  8:13     ` Andrew Jones
2020-11-10 20:47 ` [PATCH 4/8] KVM: selftests: Make vm_create_default common Andrew Jones
2020-11-10 20:47 ` [PATCH 5/8] KVM: selftests: Introduce vm_create_[default_]_with_vcpus Andrew Jones
2020-11-10 22:13   ` Ben Gardon
2020-11-11  8:29     ` Andrew Jones
2020-11-10 20:48 ` [PATCH 6/8] KVM: selftests: dirty_log_test: Remove create_vm Andrew Jones
2020-11-10 20:48 ` [PATCH 7/8] KVM: selftests: Use vm_create_with_vcpus in create_vm Andrew Jones
2020-11-10 20:48 ` [PATCH 8/8] KVM: selftests: Implement perf_test_util more conventionally Andrew Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.