[RFC PATCH v5 00/10] KVM: selftests: some improvement and a new test for kvm page table

[RFC PATCH v5 00/10] KVM: selftests: some improvement and a new test for kvm page table

[Yanan Wang]

Hi,
This v5 series can mainly include two parts.
Based on kvm queue branch: https://git.kernel.org/pub/scm/virt/kvm/kvm.git/log/?h=queue

In the first part, all the known hugetlb backing src types specified
with different hugepage sizes are listed, so that we can specify use
of hugetlb source of the exact granularity that we want, instead of
the system default ones. And as all the known hugetlb page sizes are
listed, it's appropriate for all architectures. Besides, a helper that
can get granularity of different backing src types(anonumous/thp/hugetlb)
is added, so that we can use the accurate backing src granularity for
kinds of alignment or guest memory accessing of vcpus.

In the second part, a new test is added:
This test is added to serve as a performance tester and a bug reproducer
for kvm page table code (GPA->HPA mappings), it gives guidance for the
people trying to make some improvement for kvm. And the following explains
what we can exactly do through this test.

The function guest_code() can cover the conditions where a single vcpu or
multiple vcpus access guest pages within the same memory region, in three
VM stages(before dirty logging, during dirty logging, after dirty logging).
Besides, the backing src memory type(ANONYMOUS/THP/HUGETLB) of the tested
memory region can be specified by users, which means normal page mappings
or block mappings can be chosen by users to be created in the test.

If ANONYMOUS memory is specified, kvm will create normal page mappings
for the tested memory region before dirty logging, and update attributes
of the page mappings from RO to RW during dirty logging. If THP/HUGETLB
memory is specified, kvm will create block mappings for the tested memory
region before dirty logging, and split the blcok mappings into normal page
mappings during dirty logging, and coalesce the page mappings back into
block mappings after dirty logging is stopped.

So in summary, as a performance tester, this test can present the
performance of kvm creating/updating normal page mappings, or the
performance of kvm creating/splitting/recovering block mappings,
through execution time.

When we need to coalesce the page mappings back to block mappings after
dirty logging is stopped, we have to firstly invalidate *all* the TLB
entries for the page mappings right before installation of the block entry,
because a TLB conflict abort error could occur if we can't invalidate the
TLB entries fully. We have hit this TLB conflict twice on aarch64 software
implementation and fixed it. As this test can imulate process from dirty
logging enabled to dirty logging stopped of a VM with block mappings,
so it can also reproduce this TLB conflict abort due to inadequate TLB
invalidation when coalescing tables.

Links about the TLB conflict abort:
https://lore.kernel.org/lkml/20201201201034.116760-3-wangyanan55@huawei.com/

---

Change logs:

v4->v5:
- Use synchronization(sem_wait) for time measurement
- Add a new patch about TEST_ASSERT(patch 4)
- Address Andrew Jones's comments for v4 series
- Add Andrew Jones's R-b tags in some patches
- v4: https://lore.kernel.org/lkml/20210302125751.19080-1-wangyanan55@huawei.com/

v3->v4:
- Add a helper to get system default hugetlb page size
- Add tags of Reviewed-by of Ben in the patches
- v3: https://lore.kernel.org/lkml/20210301065916.11484-1-wangyanan55@huawei.com/

v2->v3:
- Add tags of Suggested-by, Reviewed-by in the patches
- Add a generic micro to get hugetlb page sizes
- Some changes for suggestions about v2 series
- v2: https://lore.kernel.org/lkml/20210225055940.18748-1-wangyanan55@huawei.com/

v1->v2:
- Add a patch to sync header files
- Add helpers to get granularity of different backing src types
- Some changes for suggestions about v1 series
- v1: https://lore.kernel.org/lkml/20210208090841.333724-1-wangyanan55@huawei.com/

---

Yanan Wang (10):
  tools headers: sync headers of asm-generic/hugetlb_encode.h
  tools headers: Add a macro to get HUGETLB page sizes for mmap
  KVM: selftests: Use flag CLOCK_MONOTONIC_RAW for timing
  KVM: selftests: Print the errno besides error-string in TEST_ASSERT
  KVM: selftests: Make a generic helper to get vm guest mode strings
  KVM: selftests: Add a helper to get system configured THP page size
  KVM: selftests: Add a helper to get system default hugetlb page size
  KVM: selftests: List all hugetlb src types specified with page sizes
  KVM: selftests: Adapt vm_userspace_mem_region_add to new helpers
  KVM: selftests: Add a test for kvm page table code

 include/uapi/linux/mman.h                     |   2 +
 tools/include/asm-generic/hugetlb_encode.h    |   3 +
 tools/include/uapi/linux/mman.h               |   2 +
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   3 +
 .../selftests/kvm/demand_paging_test.c        |   8 +-
 .../selftests/kvm/dirty_log_perf_test.c       |  14 +-
 .../testing/selftests/kvm/include/kvm_util.h  |   4 +-
 .../testing/selftests/kvm/include/test_util.h |  21 +-
 .../selftests/kvm/kvm_page_table_test.c       | 512 ++++++++++++++++++
 tools/testing/selftests/kvm/lib/assert.c      |   4 +-
 tools/testing/selftests/kvm/lib/kvm_util.c    |  59 +-
 tools/testing/selftests/kvm/lib/test_util.c   | 163 +++++-
 tools/testing/selftests/kvm/steal_time.c      |   4 +-
 14 files changed, 739 insertions(+), 61 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/kvm_page_table_test.c

-- 
2.23.0

[RFC PATCH v5 01/10] tools headers: sync headers of asm-generic/hugetlb_encode.h

[Yanan Wang]

This patch syncs contents of tools/include/asm-generic/hugetlb_encode.h
and include/uapi/asm-generic/hugetlb_encode.h. Arch powerpc supports 16KB
hugepages and ARM64 supports 32MB/512MB hugepages. The corresponding mmap
flags have already been added in include/uapi/asm-generic/hugetlb_encode.h,
but not tools/include/asm-generic/hugetlb_encode.h.

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Ben Gardon <bgardon@google.com>
---
 tools/include/asm-generic/hugetlb_encode.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/include/asm-generic/hugetlb_encode.h b/tools/include/asm-generic/hugetlb_encode.h
index e4732d3c2998..4f3d5aaa11f5 100644
--- a/tools/include/asm-generic/hugetlb_encode.h
+++ b/tools/include/asm-generic/hugetlb_encode.h
@@ -20,13 +20,16 @@
 #define HUGETLB_FLAG_ENCODE_SHIFT	26
 #define HUGETLB_FLAG_ENCODE_MASK	0x3f
 
+#define HUGETLB_FLAG_ENCODE_16KB	(14 << HUGETLB_FLAG_ENCODE_SHIFT)
 #define HUGETLB_FLAG_ENCODE_64KB	(16 << HUGETLB_FLAG_ENCODE_SHIFT)
 #define HUGETLB_FLAG_ENCODE_512KB	(19 << HUGETLB_FLAG_ENCODE_SHIFT)
 #define HUGETLB_FLAG_ENCODE_1MB		(20 << HUGETLB_FLAG_ENCODE_SHIFT)
 #define HUGETLB_FLAG_ENCODE_2MB		(21 << HUGETLB_FLAG_ENCODE_SHIFT)
 #define HUGETLB_FLAG_ENCODE_8MB		(23 << HUGETLB_FLAG_ENCODE_SHIFT)
 #define HUGETLB_FLAG_ENCODE_16MB	(24 << HUGETLB_FLAG_ENCODE_SHIFT)
+#define HUGETLB_FLAG_ENCODE_32MB	(25 << HUGETLB_FLAG_ENCODE_SHIFT)
 #define HUGETLB_FLAG_ENCODE_256MB	(28 << HUGETLB_FLAG_ENCODE_SHIFT)
+#define HUGETLB_FLAG_ENCODE_512MB	(29 << HUGETLB_FLAG_ENCODE_SHIFT)
 #define HUGETLB_FLAG_ENCODE_1GB		(30 << HUGETLB_FLAG_ENCODE_SHIFT)
 #define HUGETLB_FLAG_ENCODE_2GB		(31 << HUGETLB_FLAG_ENCODE_SHIFT)
 #define HUGETLB_FLAG_ENCODE_16GB	(34 << HUGETLB_FLAG_ENCODE_SHIFT)
-- 
2.19.1

[RFC PATCH v5 02/10] tools headers: Add a macro to get HUGETLB page sizes for mmap

[Yanan Wang]

We know that if a system supports multiple hugetlb page sizes,
the desired hugetlb page size can be specified in bits [26:31]
of the flag arguments. The value in these 6 bits will be the
shift of each hugetlb page size.

So add a macro to get the page size shift and then calculate the
corresponding hugetlb page size, using flag x.

Cc: Ben Gardon <bgardon@google.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Suggested-by: Ben Gardon <bgardon@google.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Ben Gardon <bgardon@google.com>
---
 include/uapi/linux/mman.h       | 2 ++
 tools/include/uapi/linux/mman.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/include/uapi/linux/mman.h b/include/uapi/linux/mman.h
index f55bc680b5b0..d72df73b182d 100644
--- a/include/uapi/linux/mman.h
+++ b/include/uapi/linux/mman.h
@@ -41,4 +41,6 @@
 #define MAP_HUGE_2GB	HUGETLB_FLAG_ENCODE_2GB
 #define MAP_HUGE_16GB	HUGETLB_FLAG_ENCODE_16GB
 
+#define MAP_HUGE_PAGE_SIZE(x) (1ULL << ((x >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK))
+
 #endif /* _UAPI_LINUX_MMAN_H */
diff --git a/tools/include/uapi/linux/mman.h b/tools/include/uapi/linux/mman.h
index f55bc680b5b0..d72df73b182d 100644
--- a/tools/include/uapi/linux/mman.h
+++ b/tools/include/uapi/linux/mman.h
@@ -41,4 +41,6 @@
 #define MAP_HUGE_2GB	HUGETLB_FLAG_ENCODE_2GB
 #define MAP_HUGE_16GB	HUGETLB_FLAG_ENCODE_16GB
 
+#define MAP_HUGE_PAGE_SIZE(x) (1ULL << ((x >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK))
+
 #endif /* _UAPI_LINUX_MMAN_H */
-- 
2.19.1

[RFC PATCH v5 03/10] KVM: selftests: Use flag CLOCK_MONOTONIC_RAW for timing

[Yanan Wang]

In addition to function of CLOCK_MONOTONIC, flag CLOCK_MONOTONIC_RAW can
also shield possiable impact of NTP, which can provide more robustness.

Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Ben Gardon <bgardon@google.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
 tools/testing/selftests/kvm/demand_paging_test.c  |  8 ++++----
 tools/testing/selftests/kvm/dirty_log_perf_test.c | 14 +++++++-------
 tools/testing/selftests/kvm/lib/test_util.c       |  2 +-
 tools/testing/selftests/kvm/steal_time.c          |  4 ++--
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 5f7a229c3af1..efbf0c1e9130 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -53,7 +53,7 @@ static void *vcpu_worker(void *data)
 	vcpu_args_set(vm, vcpu_id, 1, vcpu_id);
 	run = vcpu_state(vm, vcpu_id);
 
-	clock_gettime(CLOCK_MONOTONIC, &start);
+	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
 
 	/* Let the guest access its memory */
 	ret = _vcpu_run(vm, vcpu_id);
@@ -86,7 +86,7 @@ static int handle_uffd_page_request(int uffd, uint64_t addr)
 	copy.len = perf_test_args.host_page_size;
 	copy.mode = 0;
 
-	clock_gettime(CLOCK_MONOTONIC, &start);
+	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
 
 	r = ioctl(uffd, UFFDIO_COPY, &copy);
 	if (r == -1) {
@@ -123,7 +123,7 @@ static void *uffd_handler_thread_fn(void *arg)
 	struct timespec start;
 	struct timespec ts_diff;
 
-	clock_gettime(CLOCK_MONOTONIC, &start);
+	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
 	while (!quit_uffd_thread) {
 		struct uffd_msg msg;
 		struct pollfd pollfd[2];
@@ -336,7 +336,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 
 	pr_info("Finished creating vCPUs and starting uffd threads\n");
 
-	clock_gettime(CLOCK_MONOTONIC, &start);
+	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
 
 	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++) {
 		pthread_create(&vcpu_threads[vcpu_id], NULL, vcpu_worker,
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 04a2641261be..6cff4ccf9525 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -50,7 +50,7 @@ static void *vcpu_worker(void *data)
 	while (!READ_ONCE(host_quit)) {
 		int current_iteration = READ_ONCE(iteration);
 
-		clock_gettime(CLOCK_MONOTONIC, &start);
+		clock_gettime(CLOCK_MONOTONIC_RAW, &start);
 		ret = _vcpu_run(vm, vcpu_id);
 		ts_diff = timespec_elapsed(start);
 
@@ -141,7 +141,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	iteration = 0;
 	host_quit = false;
 
-	clock_gettime(CLOCK_MONOTONIC, &start);
+	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
 	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++) {
 		vcpu_last_completed_iteration[vcpu_id] = -1;
 
@@ -162,7 +162,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 		ts_diff.tv_sec, ts_diff.tv_nsec);
 
 	/* Enable dirty logging */
-	clock_gettime(CLOCK_MONOTONIC, &start);
+	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
 	vm_mem_region_set_flags(vm, PERF_TEST_MEM_SLOT_INDEX,
 				KVM_MEM_LOG_DIRTY_PAGES);
 	ts_diff = timespec_elapsed(start);
@@ -174,7 +174,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 		 * Incrementing the iteration number will start the vCPUs
 		 * dirtying memory again.
 		 */
-		clock_gettime(CLOCK_MONOTONIC, &start);
+		clock_gettime(CLOCK_MONOTONIC_RAW, &start);
 		iteration++;
 
 		pr_debug("Starting iteration %d\n", iteration);
@@ -189,7 +189,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 		pr_info("Iteration %d dirty memory time: %ld.%.9lds\n",
 			iteration, ts_diff.tv_sec, ts_diff.tv_nsec);
 
-		clock_gettime(CLOCK_MONOTONIC, &start);
+		clock_gettime(CLOCK_MONOTONIC_RAW, &start);
 		kvm_vm_get_dirty_log(vm, PERF_TEST_MEM_SLOT_INDEX, bmap);
 
 		ts_diff = timespec_elapsed(start);
@@ -199,7 +199,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 			iteration, ts_diff.tv_sec, ts_diff.tv_nsec);
 
 		if (dirty_log_manual_caps) {
-			clock_gettime(CLOCK_MONOTONIC, &start);
+			clock_gettime(CLOCK_MONOTONIC_RAW, &start);
 			kvm_vm_clear_dirty_log(vm, PERF_TEST_MEM_SLOT_INDEX, bmap, 0,
 					       host_num_pages);
 
@@ -212,7 +212,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	}
 
 	/* Disable dirty logging */
-	clock_gettime(CLOCK_MONOTONIC, &start);
+	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
 	vm_mem_region_set_flags(vm, PERF_TEST_MEM_SLOT_INDEX, 0);
 	ts_diff = timespec_elapsed(start);
 	pr_info("Disabling dirty logging time: %ld.%.9lds\n",
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index 906c955384e2..c7c0627c6842 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -89,7 +89,7 @@ struct timespec timespec_elapsed(struct timespec start)
 {
 	struct timespec end;
 
-	clock_gettime(CLOCK_MONOTONIC, &end);
+	clock_gettime(CLOCK_MONOTONIC_RAW, &end);
 	return timespec_sub(end, start);
 }
 
diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
index fcc840088c91..5bc582d3f2a2 100644
--- a/tools/testing/selftests/kvm/steal_time.c
+++ b/tools/testing/selftests/kvm/steal_time.c
@@ -237,11 +237,11 @@ static void *do_steal_time(void *arg)
 {
 	struct timespec ts, stop;
 
-	clock_gettime(CLOCK_MONOTONIC, &ts);
+	clock_gettime(CLOCK_MONOTONIC_RAW, &ts);
 	stop = timespec_add_ns(ts, MIN_RUN_DELAY_NS);
 
 	while (1) {
-		clock_gettime(CLOCK_MONOTONIC, &ts);
+		clock_gettime(CLOCK_MONOTONIC_RAW, &ts);
 		if (timespec_to_ns(timespec_sub(ts, stop)) >= 0)
 			break;
 	}
-- 
2.19.1

[RFC PATCH v5 04/10] KVM: selftests: Print the errno besides error-string in TEST_ASSERT

[Yanan Wang]

Print the errno besides error-string in TEST_ASSERT in the format of
"errno=%d - %s" will explicitly indicate that the string is an error
information. Besides, the errno is easier to be used for debugging
than the error-string.

Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 tools/testing/selftests/kvm/lib/assert.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c
index 5ebbd0d6b472..71ade6100fd3 100644
--- a/tools/testing/selftests/kvm/lib/assert.c
+++ b/tools/testing/selftests/kvm/lib/assert.c
@@ -71,9 +71,9 @@ test_assert(bool exp, const char *exp_str,
 
 		fprintf(stderr, "==== Test Assertion Failure ====\n"
 			"  %s:%u: %s\n"
-			"  pid=%d tid=%d - %s\n",
+			"  pid=%d tid=%d errno=%d - %s\n",
 			file, line, exp_str, getpid(), _gettid(),
-			strerror(errno));
+			errno, strerror(errno));
 		test_dump_stack();
 		if (fmt) {
 			fputs("  ", stderr);
-- 
2.19.1

[RFC PATCH v5 05/10] KVM: selftests: Make a generic helper to get vm guest mode strings

[Yanan Wang]

For generality and conciseness, make an API which can be used in all
kvm libs and selftests to get vm guest mode strings. And the index i
is checked in the API in case of possiable faults.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Ben Gardon <bgardon@google.com>
---
 .../testing/selftests/kvm/include/kvm_util.h  |  4 +--
 tools/testing/selftests/kvm/lib/kvm_util.c    | 29 ++++++++++++-------
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 2d7eb6989e83..f52a7492f47f 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -68,9 +68,6 @@ enum vm_guest_mode {
 #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[];
-
 struct vm_guest_mode_params {
 	unsigned int pa_bits;
 	unsigned int va_bits;
@@ -84,6 +81,7 @@ int vm_enable_cap(struct kvm_vm *vm, struct kvm_enable_cap *cap);
 int vcpu_enable_cap(struct kvm_vm *vm, uint32_t vcpu_id,
 		    struct kvm_enable_cap *cap);
 void vm_enable_dirty_ring(struct kvm_vm *vm, uint32_t ring_size);
+const char *vm_guest_mode_string(uint32_t i);
 
 struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm);
 void kvm_vm_free(struct kvm_vm *vmp);
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index e5fbf16f725b..2ea837fe03af 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -143,17 +143,24 @@ static void vm_open(struct kvm_vm *vm, int perm)
 		"rc: %i errno: %i", vm->fd, errno);
 }
 
-const char * const vm_guest_mode_string[] = {
-	"PA-bits:52,  VA-bits:48,  4K pages",
-	"PA-bits:52,  VA-bits:48, 64K pages",
-	"PA-bits:48,  VA-bits:48,  4K pages",
-	"PA-bits:48,  VA-bits:48, 64K pages",
-	"PA-bits:40,  VA-bits:48,  4K pages",
-	"PA-bits:40,  VA-bits:48, 64K pages",
-	"PA-bits:ANY, VA-bits:48,  4K pages",
-};
-_Static_assert(sizeof(vm_guest_mode_string)/sizeof(char *) == NUM_VM_MODES,
-	       "Missing new mode strings?");
+const char *vm_guest_mode_string(uint32_t i)
+{
+	static const char * const strings[] = {
+		[VM_MODE_P52V48_4K]	= "PA-bits:52,  VA-bits:48,  4K pages",
+		[VM_MODE_P52V48_64K]	= "PA-bits:52,  VA-bits:48, 64K pages",
+		[VM_MODE_P48V48_4K]	= "PA-bits:48,  VA-bits:48,  4K pages",
+		[VM_MODE_P48V48_64K]	= "PA-bits:48,  VA-bits:48, 64K pages",
+		[VM_MODE_P40V48_4K]	= "PA-bits:40,  VA-bits:48,  4K pages",
+		[VM_MODE_P40V48_64K]	= "PA-bits:40,  VA-bits:48, 64K pages",
+		[VM_MODE_PXXV48_4K]	= "PA-bits:ANY, VA-bits:48,  4K pages",
+	};
+	_Static_assert(sizeof(strings)/sizeof(char *) == NUM_VM_MODES,
+		       "Missing new mode strings?");
+
+	TEST_ASSERT(i < NUM_VM_MODES, "Guest mode ID %d too big", i);
+
+	return strings[i];
+}
 
 const struct vm_guest_mode_params vm_guest_mode_params[] = {
 	{ 52, 48,  0x1000, 12 },
-- 
2.19.1

[RFC PATCH v5 06/10] KVM: selftests: Add a helper to get system configured THP page size

[Yanan Wang]

If we want to have some tests about transparent hugepages, the system
configured THP hugepage size should better be known by the tests, which
can be used for kinds of alignment or guest memory accessing of vcpus...
So it makes sense to add a helper to get the transparent hugepage size.

With VM_MEM_SRC_ANONYMOUS_THP specified in vm_userspace_mem_region_add(),
we now stat /sys/kernel/mm/transparent_hugepage to check whether THP is
configured in the host kernel before madvise(). Based on this, we can also
read file /sys/kernel/mm/transparent_hugepage/hpage_pmd_size to get THP
hugepage size.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Ben Gardon <bgardon@google.com>
---
 .../testing/selftests/kvm/include/test_util.h |  2 ++
 tools/testing/selftests/kvm/lib/test_util.c   | 29 +++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index b7f41399f22c..ef24c76ba89a 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -78,6 +78,8 @@ struct vm_mem_backing_src_alias {
 	enum vm_mem_backing_src_type type;
 };
 
+bool thp_configured(void);
+size_t get_trans_hugepagesz(void);
 void backing_src_help(void);
 enum vm_mem_backing_src_type parse_backing_src_type(const char *type_name);
 
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index c7c0627c6842..efc1a7782de0 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -10,6 +10,7 @@
 #include <limits.h>
 #include <stdlib.h>
 #include <time.h>
+#include <sys/stat.h>
 #include "linux/kernel.h"
 
 #include "test_util.h"
@@ -117,6 +118,34 @@ const struct vm_mem_backing_src_alias backing_src_aliases[] = {
 	{"anonymous_hugetlb", VM_MEM_SRC_ANONYMOUS_HUGETLB,},
 };
 
+bool thp_configured(void)
+{
+	int ret;
+	struct stat statbuf;
+
+	ret = stat("/sys/kernel/mm/transparent_hugepage", &statbuf);
+	TEST_ASSERT(ret == 0 || (ret == -1 && errno == ENOENT),
+		    "Error in stating /sys/kernel/mm/transparent_hugepage");
+
+	return ret == 0;
+}
+
+size_t get_trans_hugepagesz(void)
+{
+	size_t size;
+	FILE *f;
+
+	TEST_ASSERT(thp_configured(), "THP is not configured in host kernel");
+
+	f = fopen("/sys/kernel/mm/transparent_hugepage/hpage_pmd_size", "r");
+	TEST_ASSERT(f != NULL, "Error in opening transparent_hugepage/hpage_pmd_size");
+
+	fscanf(f, "%ld", &size);
+	fclose(f);
+
+	return size;
+}
+
 void backing_src_help(void)
 {
 	int i;
-- 
2.19.1

[RFC PATCH v5 07/10] KVM: selftests: Add a helper to get system default hugetlb page size

[Yanan Wang]

If HUGETLB is configured in the host kernel, then we can know the system
default hugetlb page size through *cat /proc/meminfo*. Otherwise, we will
not see the information of hugetlb pages in file /proc/meminfo if it's not
configured. So add a helper to determine whether HUGETLB is configured and
then get the default page size by reading /proc/meminfo.

This helper can be useful when a program wants to use the default hugetlb
pages of the system and doesn't know the default page size.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
 .../testing/selftests/kvm/include/test_util.h |  1 +
 tools/testing/selftests/kvm/lib/test_util.c   | 25 +++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index ef24c76ba89a..e087174eefe5 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -80,6 +80,7 @@ struct vm_mem_backing_src_alias {
 
 bool thp_configured(void);
 size_t get_trans_hugepagesz(void);
+size_t get_def_hugetlb_pagesz(void);
 void backing_src_help(void);
 enum vm_mem_backing_src_type parse_backing_src_type(const char *type_name);
 
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index efc1a7782de0..665724ccab97 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -146,6 +146,31 @@ size_t get_trans_hugepagesz(void)
 	return size;
 }
 
+size_t get_def_hugetlb_pagesz(void)
+{
+	char buf[64];
+	const char *tag = "Hugepagesize:";
+	FILE *f;
+
+	f = fopen("/proc/meminfo", "r");
+	TEST_ASSERT(f != NULL, "Error in opening /proc/meminfo");
+
+	while (fgets(buf, sizeof(buf), f) != NULL) {
+		if (strstr(buf, tag) == buf) {
+			fclose(f);
+			return strtoull(buf + strlen(tag), NULL, 10) << 10;
+		}
+	}
+
+	if (feof(f))
+		TEST_FAIL("HUGETLB is not configured in host kernel");
+	else
+		TEST_FAIL("Error in reading /proc/meminfo");
+
+	fclose(f);
+	return 0;
+}
+
 void backing_src_help(void)
 {
 	int i;
-- 
2.19.1

[RFC PATCH v5 08/10] KVM: selftests: List all hugetlb src types specified with page sizes

[Yanan Wang]

With VM_MEM_SRC_ANONYMOUS_HUGETLB, we currently can only use system
default hugetlb pages to back the testing guest memory. In order to
add flexibility, now list all the known hugetlb backing src types with
different page sizes, so that we can specify use of hugetlb pages of the
exact granularity that we want. And as all the known hugetlb page sizes
are listed, it's appropriate for all architectures.

Besides, the helper get_backing_src_pagesz() is added to get the
granularity of different backing src types(anonumous, thp, hugetlb).

Suggested-by: Ben Gardon <bgardon@google.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
 .../testing/selftests/kvm/include/test_util.h |  18 ++-
 tools/testing/selftests/kvm/lib/kvm_util.c    |   2 +-
 tools/testing/selftests/kvm/lib/test_util.c   | 109 ++++++++++++++++--
 3 files changed, 116 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index e087174eefe5..fade3130eb01 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -71,16 +71,32 @@ enum vm_mem_backing_src_type {
 	VM_MEM_SRC_ANONYMOUS,
 	VM_MEM_SRC_ANONYMOUS_THP,
 	VM_MEM_SRC_ANONYMOUS_HUGETLB,
+	VM_MEM_SRC_ANONYMOUS_HUGETLB_16KB,
+	VM_MEM_SRC_ANONYMOUS_HUGETLB_64KB,
+	VM_MEM_SRC_ANONYMOUS_HUGETLB_512KB,
+	VM_MEM_SRC_ANONYMOUS_HUGETLB_1MB,
+	VM_MEM_SRC_ANONYMOUS_HUGETLB_2MB,
+	VM_MEM_SRC_ANONYMOUS_HUGETLB_8MB,
+	VM_MEM_SRC_ANONYMOUS_HUGETLB_16MB,
+	VM_MEM_SRC_ANONYMOUS_HUGETLB_32MB,
+	VM_MEM_SRC_ANONYMOUS_HUGETLB_256MB,
+	VM_MEM_SRC_ANONYMOUS_HUGETLB_512MB,
+	VM_MEM_SRC_ANONYMOUS_HUGETLB_1GB,
+	VM_MEM_SRC_ANONYMOUS_HUGETLB_2GB,
+	VM_MEM_SRC_ANONYMOUS_HUGETLB_16GB,
+	NUM_SRC_TYPES,
 };
 
 struct vm_mem_backing_src_alias {
 	const char *name;
-	enum vm_mem_backing_src_type type;
+	uint32_t flag;
 };
 
 bool thp_configured(void);
 size_t get_trans_hugepagesz(void);
 size_t get_def_hugetlb_pagesz(void);
+const struct vm_mem_backing_src_alias *vm_mem_backing_src_alias(uint32_t i);
+size_t get_backing_src_pagesz(uint32_t i);
 void backing_src_help(void);
 enum vm_mem_backing_src_type parse_backing_src_type(const char *type_name);
 
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 2ea837fe03af..3506174c2053 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -759,7 +759,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
 	region->mmap_start = mmap(NULL, region->mmap_size,
 				  PROT_READ | PROT_WRITE,
 				  MAP_PRIVATE | MAP_ANONYMOUS
-				  | (src_type == VM_MEM_SRC_ANONYMOUS_HUGETLB ? MAP_HUGETLB : 0),
+				  | vm_mem_backing_src_alias(src_type)->flag,
 				  -1, 0);
 	TEST_ASSERT(region->mmap_start != MAP_FAILED,
 		    "test_malloc failed, mmap_start: %p errno: %i",
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index 665724ccab97..205408bffa38 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -11,6 +11,7 @@
 #include <stdlib.h>
 #include <time.h>
 #include <sys/stat.h>
+#include <linux/mman.h>
 #include "linux/kernel.h"
 
 #include "test_util.h"
@@ -112,12 +113,6 @@ void print_skip(const char *fmt, ...)
 	puts(", skipping test");
 }
 
-const struct vm_mem_backing_src_alias backing_src_aliases[] = {
-	{"anonymous", VM_MEM_SRC_ANONYMOUS,},
-	{"anonymous_thp", VM_MEM_SRC_ANONYMOUS_THP,},
-	{"anonymous_hugetlb", VM_MEM_SRC_ANONYMOUS_HUGETLB,},
-};
-
 bool thp_configured(void)
 {
 	int ret;
@@ -171,22 +166,114 @@ size_t get_def_hugetlb_pagesz(void)
 	return 0;
 }
 
+const struct vm_mem_backing_src_alias *vm_mem_backing_src_alias(uint32_t i)
+{
+	static const struct vm_mem_backing_src_alias aliases[] = {
+		[VM_MEM_SRC_ANONYMOUS] = {
+			.name = "anonymous",
+			.flag = 0,
+		},
+		[VM_MEM_SRC_ANONYMOUS_THP] = {
+			.name = "anonymous_thp",
+			.flag = 0,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB] = {
+			.name = "anonymous_hugetlb",
+			.flag = MAP_HUGETLB,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB_16KB] = {
+			.name = "anonymous_hugetlb_16kb",
+			.flag = MAP_HUGETLB | MAP_HUGE_16KB,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB_64KB] = {
+			.name = "anonymous_hugetlb_64kb",
+			.flag = MAP_HUGETLB | MAP_HUGE_64KB,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB_512KB] = {
+			.name = "anonymous_hugetlb_512kb",
+			.flag = MAP_HUGETLB | MAP_HUGE_512KB,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB_1MB] = {
+			.name = "anonymous_hugetlb_1mb",
+			.flag = MAP_HUGETLB | MAP_HUGE_1MB,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB_2MB] = {
+			.name = "anonymous_hugetlb_2mb",
+			.flag = MAP_HUGETLB | MAP_HUGE_2MB,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB_8MB] = {
+			.name = "anonymous_hugetlb_8mb",
+			.flag = MAP_HUGETLB | MAP_HUGE_8MB,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB_16MB] = {
+			.name = "anonymous_hugetlb_16mb",
+			.flag = MAP_HUGETLB | MAP_HUGE_16MB,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB_32MB] = {
+			.name = "anonymous_hugetlb_32mb",
+			.flag = MAP_HUGETLB | MAP_HUGE_32MB,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB_256MB] = {
+			.name = "anonymous_hugetlb_256mb",
+			.flag = MAP_HUGETLB | MAP_HUGE_256MB,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB_512MB] = {
+			.name = "anonymous_hugetlb_512mb",
+			.flag = MAP_HUGETLB | MAP_HUGE_512MB,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB_1GB] = {
+			.name = "anonymous_hugetlb_1gb",
+			.flag = MAP_HUGETLB | MAP_HUGE_1GB,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB_2GB] = {
+			.name = "anonymous_hugetlb_2gb",
+			.flag = MAP_HUGETLB | MAP_HUGE_2GB,
+		},
+		[VM_MEM_SRC_ANONYMOUS_HUGETLB_16GB] = {
+			.name = "anonymous_hugetlb_16gb",
+			.flag = MAP_HUGETLB | MAP_HUGE_16GB,
+		},
+	};
+	_Static_assert(ARRAY_SIZE(aliases) == NUM_SRC_TYPES,
+		       "Missing new backing src types?");
+
+	TEST_ASSERT(i < NUM_SRC_TYPES, "Backing src type ID %d too big", i);
+
+	return &aliases[i];
+}
+
+size_t get_backing_src_pagesz(uint32_t i)
+{
+	uint32_t flag = vm_mem_backing_src_alias(i)->flag;
+
+	switch (i) {
+	case VM_MEM_SRC_ANONYMOUS:
+		return getpagesize();
+	case VM_MEM_SRC_ANONYMOUS_THP:
+		return get_trans_hugepagesz();
+	case VM_MEM_SRC_ANONYMOUS_HUGETLB:
+		return get_def_hugetlb_pagesz();
+	default:
+		return MAP_HUGE_PAGE_SIZE(flag);
+	}
+}
+
 void backing_src_help(void)
 {
 	int i;
 
 	printf("Available backing src types:\n");
-	for (i = 0; i < ARRAY_SIZE(backing_src_aliases); i++)
-		printf("\t%s\n", backing_src_aliases[i].name);
+	for (i = 0; i < NUM_SRC_TYPES; i++)
+		printf("\t%s\n", vm_mem_backing_src_alias(i)->name);
 }
 
 enum vm_mem_backing_src_type parse_backing_src_type(const char *type_name)
 {
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(backing_src_aliases); i++)
-		if (!strcmp(type_name, backing_src_aliases[i].name))
-			return backing_src_aliases[i].type;
+	for (i = 0; i < NUM_SRC_TYPES; i++)
+		if (!strcmp(type_name, vm_mem_backing_src_alias(i)->name))
+			return i;
 
 	backing_src_help();
 	TEST_FAIL("Unknown backing src type: %s", type_name);
-- 
2.19.1

[RFC PATCH v5 09/10] KVM: selftests: Adapt vm_userspace_mem_region_add to new helpers

[Yanan Wang]

With VM_MEM_SRC_ANONYMOUS_THP specified in vm_userspace_mem_region_add(),
we have to get the transparent hugepage size for HVA alignment. With the
new helpers, we can use get_backing_src_pagesz() to check whether THP is
configured and then get the exact configured hugepage size.

As different architectures may have different THP page sizes configured,
this can get the accurate THP page sizes on any platform.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Ben Gardon <bgardon@google.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 28 +++++++---------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 3506174c2053..c7a2228deaf3 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -18,7 +18,6 @@
 #include <unistd.h>
 #include <linux/kernel.h>
 
-#define KVM_UTIL_PGS_PER_HUGEPG 512
 #define KVM_UTIL_MIN_PFN	2
 
 static int vcpu_mmap_sz(void);
@@ -688,7 +687,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
 {
 	int ret;
 	struct userspace_mem_region *region;
-	size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size;
+	size_t backing_src_pagesz = get_backing_src_pagesz(src_type);
 	size_t alignment;
 
 	TEST_ASSERT(vm_adjust_num_guest_pages(vm->mode, npages) == npages,
@@ -750,7 +749,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
 #endif
 
 	if (src_type == VM_MEM_SRC_ANONYMOUS_THP)
-		alignment = max(huge_page_size, alignment);
+		alignment = max(backing_src_pagesz, alignment);
 
 	/* Add enough memory to align up if necessary */
 	if (alignment > 1)
@@ -769,22 +768,13 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
 	region->host_mem = align(region->mmap_start, alignment);
 
 	/* As needed perform madvise */
-	if (src_type == VM_MEM_SRC_ANONYMOUS || src_type == VM_MEM_SRC_ANONYMOUS_THP) {
-		struct stat statbuf;
-
-		ret = stat("/sys/kernel/mm/transparent_hugepage", &statbuf);
-		TEST_ASSERT(ret == 0 || (ret == -1 && errno == ENOENT),
-			    "stat /sys/kernel/mm/transparent_hugepage");
-
-		TEST_ASSERT(ret == 0 || src_type != VM_MEM_SRC_ANONYMOUS_THP,
-			    "VM_MEM_SRC_ANONYMOUS_THP requires THP to be configured in the host kernel");
-
-		if (ret == 0) {
-			ret = madvise(region->host_mem, npages * vm->page_size,
-				      src_type == VM_MEM_SRC_ANONYMOUS ? MADV_NOHUGEPAGE : MADV_HUGEPAGE);
-			TEST_ASSERT(ret == 0, "madvise failed, addr: %p length: 0x%lx src_type: %x",
-				    region->host_mem, npages * vm->page_size, src_type);
-		}
+	if ((src_type == VM_MEM_SRC_ANONYMOUS ||
+	     src_type == VM_MEM_SRC_ANONYMOUS_THP) && thp_configured()) {
+		ret = madvise(region->host_mem, npages * vm->page_size,
+			      src_type == VM_MEM_SRC_ANONYMOUS ? MADV_NOHUGEPAGE : MADV_HUGEPAGE);
+		TEST_ASSERT(ret == 0, "madvise failed, addr: %p length: 0x%lx src_type: %s",
+			    region->host_mem, npages * vm->page_size,
+			    vm_mem_backing_src_alias(src_type)->name);
 	}
 
 	region->unused_phy_pages = sparsebit_alloc();
-- 
2.19.1

[RFC PATCH v5 10/10] KVM: selftests: Add a test for kvm page table code

[Yanan Wang]

This test serves as a performance tester and a bug reproducer for
kvm page table code (GPA->HPA mappings), so it gives guidance for
people trying to make some improvement for kvm.

The function guest_code() can cover the conditions where a single vcpu or
multiple vcpus access guest pages within the same memory region, in three
VM stages(before dirty logging, during dirty logging, after dirty logging).
Besides, the backing src memory type(ANONYMOUS/THP/HUGETLB) of the tested
memory region can be specified by users, which means normal page mappings
or block mappings can be chosen by users to be created in the test.

If ANONYMOUS memory is specified, kvm will create normal page mappings
for the tested memory region before dirty logging, and update attributes
of the page mappings from RO to RW during dirty logging. If THP/HUGETLB
memory is specified, kvm will create block mappings for the tested memory
region before dirty logging, and split the blcok mappings into normal page
mappings during dirty logging, and coalesce the page mappings back into
block mappings after dirty logging is stopped.

So in summary, as a performance tester, this test can present the
performance of kvm creating/updating normal page mappings, or the
performance of kvm creating/splitting/recovering block mappings,
through execution time.

When we need to coalesce the page mappings back to block mappings after
dirty logging is stopped, we have to firstly invalidate *all* the TLB
entries for the page mappings right before installation of the block entry,
because a TLB conflict abort error could occur if we can't invalidate the
TLB entries fully. We have hit this TLB conflict twice on aarch64 software
implementation and fixed it. As this test can imulate process from dirty
logging enabled to dirty logging stopped of a VM with block mappings,
so it can also reproduce this TLB conflict abort due to inadequate TLB
invalidation when coalescing tables.

Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Ben Gardon <bgardon@google.com>
---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   3 +
 .../selftests/kvm/kvm_page_table_test.c       | 512 ++++++++++++++++++
 3 files changed, 516 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/kvm_page_table_test.c

diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 32b87cc77c8e..137ab7273be6 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -35,6 +35,7 @@
 /dirty_log_perf_test
 /hardware_disable_test
 /kvm_create_max_vcpus
+/kvm_page_table_test
 /memslot_modification_stress_test
 /set_memory_region_test
 /steal_time
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index a6d61f451f88..75dc57db36b4 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -69,6 +69,7 @@ TEST_GEN_PROGS_x86_64 += dirty_log_test
 TEST_GEN_PROGS_x86_64 += dirty_log_perf_test
 TEST_GEN_PROGS_x86_64 += hardware_disable_test
 TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
+TEST_GEN_PROGS_x86_64 += kvm_page_table_test
 TEST_GEN_PROGS_x86_64 += memslot_modification_stress_test
 TEST_GEN_PROGS_x86_64 += set_memory_region_test
 TEST_GEN_PROGS_x86_64 += steal_time
@@ -79,6 +80,7 @@ TEST_GEN_PROGS_aarch64 += demand_paging_test
 TEST_GEN_PROGS_aarch64 += dirty_log_test
 TEST_GEN_PROGS_aarch64 += dirty_log_perf_test
 TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
+TEST_GEN_PROGS_aarch64 += kvm_page_table_test
 TEST_GEN_PROGS_aarch64 += set_memory_region_test
 TEST_GEN_PROGS_aarch64 += steal_time
 
@@ -88,6 +90,7 @@ TEST_GEN_PROGS_s390x += s390x/sync_regs_test
 TEST_GEN_PROGS_s390x += demand_paging_test
 TEST_GEN_PROGS_s390x += dirty_log_test
 TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
+TEST_GEN_PROGS_s390x += kvm_page_table_test
 TEST_GEN_PROGS_s390x += set_memory_region_test
 
 TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
diff --git a/tools/testing/selftests/kvm/kvm_page_table_test.c b/tools/testing/selftests/kvm/kvm_page_table_test.c
new file mode 100644
index 000000000000..bbd5c489d61f
--- /dev/null
+++ b/tools/testing/selftests/kvm/kvm_page_table_test.c
@@ -0,0 +1,512 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KVM page table test
+ *
+ * Copyright (C) 2021, Huawei, Inc.
+ *
+ * Make sure that THP has been enabled or enough HUGETLB pages with specific
+ * page size have been pre-allocated on your system, if you are planning to
+ * use hugepages to back the guest memory for testing.
+ */
+
+#define _GNU_SOURCE /* for program_invocation_name */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <time.h>
+#include <pthread.h>
+#include <semaphore.h>
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+#include "guest_modes.h"
+
+#define TEST_MEM_SLOT_INDEX             1
+
+/* Default size(1GB) of the memory for testing */
+#define DEFAULT_TEST_MEM_SIZE		(1 << 30)
+
+/* Default guest test virtual memory offset */
+#define DEFAULT_GUEST_TEST_MEM		0xc0000000
+
+/*
+ * In our test, we use thread synchronization functions (e.g. sem_wait)
+ * for time measurement and they can't fail at all, since a failure will
+ * impact the time accuracy and vcpus will not run as what we expect.
+ * So we will use safer versions of the functions.
+ */
+#define sem_init_s(sem_ptr, ps, val) \
+	TEST_ASSERT(sem_init(sem_ptr, ps, val) == 0, "Error in sem_init")
+#define sem_destroy_s(sem_ptr) \
+	TEST_ASSERT(sem_destroy(sem_ptr) == 0, "Error in sem_destroy")
+#define sem_wait_s(sem_ptr) \
+	TEST_ASSERT(sem_wait(sem_ptr) == 0, "Error in sem_wait")
+#define sem_post_s(sem_ptr) \
+	TEST_ASSERT(sem_post(sem_ptr) == 0, "Error in sem_post")
+
+/* Different guest memory accessing stages */
+enum test_stage {
+	KVM_BEFORE_MAPPINGS,
+	KVM_CREATE_MAPPINGS,
+	KVM_UPDATE_MAPPINGS,
+	KVM_ADJUST_MAPPINGS,
+	NUM_TEST_STAGES,
+};
+
+static const char * const test_stage_string[] = {
+	"KVM_BEFORE_MAPPINGS",
+	"KVM_CREATE_MAPPINGS",
+	"KVM_UPDATE_MAPPINGS",
+	"KVM_ADJUST_MAPPINGS",
+};
+
+struct perf_test_vcpu_args {
+	int vcpu_id;
+	bool vcpu_write;
+};
+
+struct perf_test_args {
+	struct kvm_vm *vm;
+	uint64_t guest_test_virt_mem;
+	uint64_t host_page_size;
+	uint64_t host_num_pages;
+	uint64_t large_page_size;
+	uint64_t large_num_pages;
+	uint64_t host_pages_per_lpage;
+	enum vm_mem_backing_src_type src_type;
+	struct perf_test_vcpu_args vcpu_args[KVM_MAX_VCPUS];
+};
+
+/*
+ * Guest variables. Use addr_gva2hva() if these variables need
+ * to be changed in host.
+ */
+static enum test_stage guest_test_stage;
+
+/* Host variables */
+static uint32_t nr_vcpus = 1;
+static struct perf_test_args perf_test_args;
+static enum test_stage *current_stage;
+static bool host_quit;
+
+/* Whether the test stage is updated, or completed */
+static sem_t test_stage_updated;
+static sem_t test_stage_completed;
+
+/*
+ * 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 void guest_code(int vcpu_id)
+{
+	struct perf_test_args *p = &perf_test_args;
+	struct perf_test_vcpu_args *vcpu_args = &p->vcpu_args[vcpu_id];
+	enum vm_mem_backing_src_type src_type = p->src_type;
+	uint64_t host_page_size = p->host_page_size;
+	uint64_t host_num_pages = p->host_num_pages;
+	uint64_t large_page_size = p->large_page_size;
+	uint64_t large_num_pages = p->large_num_pages;
+	uint64_t host_pages_per_lpage = p->host_pages_per_lpage;
+	uint64_t half = host_pages_per_lpage / 2;
+	bool vcpu_write;
+	enum test_stage stage;
+	uint64_t addr;
+	int i, j;
+
+	/* Make sure vCPU args data structure is not corrupt */
+	GUEST_ASSERT(vcpu_args->vcpu_id == vcpu_id);
+	vcpu_write = vcpu_args->vcpu_write;
+
+	while (true) {
+		stage = READ_ONCE(guest_test_stage);
+		addr = perf_test_args.guest_test_virt_mem;
+
+		switch (stage) {
+		/*
+		 * Before dirty logging, vCPUs concurrently access the first
+		 * 8 bytes of each page (host page/large page) within the same
+		 * memory region with different accessing types (read/write).
+		 * Then KVM will create normal page mappings or huge block
+		 * mappings for them.
+		 */
+		case KVM_CREATE_MAPPINGS:
+			for (i = 0; i < large_num_pages; i++) {
+				if (vcpu_write)
+					*(uint64_t *)addr = 0x0123456789ABCDEF;
+				else
+					READ_ONCE(*(uint64_t *)addr);
+
+				addr += large_page_size;
+			}
+			break;
+
+		/*
+		 * During dirty logging, KVM will only update attributes of the
+		 * normal page mappings from RO to RW if memory backing src type
+		 * is anonymous. In other cases, KVM will split the huge block
+		 * mappings into normal page mappings if memory backing src type
+		 * is THP or HUGETLB.
+		 */
+		case KVM_UPDATE_MAPPINGS:
+			if (src_type == VM_MEM_SRC_ANONYMOUS) {
+				for (i = 0; i < host_num_pages; i++) {
+					*(uint64_t *)addr = 0x0123456789ABCDEF;
+					addr += host_page_size;
+				}
+				break;
+			}
+
+			for (i = 0; i < large_num_pages; i++) {
+				/*
+				 * Write to the first host page in each large
+				 * page region, and triger break of large pages.
+				 */
+				*(uint64_t *)addr = 0x0123456789ABCDEF;
+
+				/*
+				 * Access the middle host pages in each large
+				 * page region. Since dirty logging is enabled,
+				 * this will create new mappings at the smallest
+				 * granularity.
+				 */
+				addr += host_page_size * half;
+				for (j = half; j < host_pages_per_lpage; j++) {
+					READ_ONCE(*(uint64_t *)addr);
+					addr += host_page_size;
+				}
+			}
+			break;
+
+		/*
+		 * After dirty logging is stopped, vCPUs concurrently read
+		 * from every single host page. Then KVM will coalesce the
+		 * split page mappings back to block mappings. And a TLB
+		 * conflict abort could occur here if TLB entries of the
+		 * page mappings are not fully invalidated.
+		 */
+		case KVM_ADJUST_MAPPINGS:
+			for (i = 0; i < host_num_pages; i++) {
+				READ_ONCE(*(uint64_t *)addr);
+				addr += host_page_size;
+			}
+			break;
+
+		default:
+			break;
+		}
+
+		GUEST_SYNC(1);
+	}
+}
+
+static void *vcpu_worker(void *data)
+{
+	int ret;
+	struct perf_test_vcpu_args *vcpu_args = data;
+	struct kvm_vm *vm = perf_test_args.vm;
+	int vcpu_id = vcpu_args->vcpu_id;
+	struct kvm_run *run;
+	struct timespec start;
+	struct timespec ts_diff;
+	enum test_stage stage;
+
+	vcpu_args_set(vm, vcpu_id, 1, vcpu_id);
+	run = vcpu_state(vm, vcpu_id);
+
+	while (!READ_ONCE(host_quit)) {
+		clock_gettime(CLOCK_MONOTONIC_RAW, &start);
+		ret = _vcpu_run(vm, vcpu_id);
+		ts_diff = timespec_elapsed(start);
+
+		TEST_ASSERT(ret == 0, "vcpu_run failed: %d\n", ret);
+		TEST_ASSERT(get_ucall(vm, vcpu_id, NULL) == UCALL_SYNC,
+			    "Invalid guest sync status: exit_reason=%s\n",
+			    exit_reason_str(run->exit_reason));
+
+		pr_debug("Got sync event from vCPU %d\n", vcpu_id);
+		stage = READ_ONCE(*current_stage);
+
+		/*
+		 * Here we can know the execution time of every
+		 * single vcpu running in different test stages.
+		 */
+		pr_debug("vCPU %d has completed stage %s\n"
+			 "execution time is: %ld.%.9lds\n\n",
+			 vcpu_id, test_stage_string[stage],
+			 ts_diff.tv_sec, ts_diff.tv_nsec);
+
+		sem_post_s(&test_stage_completed);
+		sem_wait_s(&test_stage_updated);
+	}
+
+	return NULL;
+}
+
+struct test_params {
+	uint64_t phys_offset;
+	uint64_t test_mem_size;
+	enum vm_mem_backing_src_type src_type;
+};
+
+static struct kvm_vm *pre_init_before_test(enum vm_guest_mode mode, void *arg)
+{
+	struct test_params *p = arg;
+	struct perf_test_vcpu_args *vcpu_args;
+	enum vm_mem_backing_src_type src_type = p->src_type;
+	uint64_t large_page_size = get_backing_src_pagesz(src_type);
+	uint64_t test_mem_size = p->test_mem_size, guest_num_pages;
+	uint64_t guest_page_size = vm_guest_mode_params[mode].page_size;
+	uint64_t host_page_size = getpagesize();
+	uint64_t alignment;
+	void *host_test_mem;
+	struct kvm_vm *vm;
+	int vcpu_id;
+
+	/* Align up the test memory size */
+	alignment = max(large_page_size, guest_page_size);
+	test_mem_size = (test_mem_size + alignment - 1) & ~(alignment - 1);
+
+	/* Create a VM with enough guest pages */
+	guest_num_pages = test_mem_size / guest_page_size;
+	vm = vm_create_with_vcpus(mode, nr_vcpus,
+				  guest_num_pages, 0, guest_code, NULL);
+
+	/* Align down GPA of the testing memslot */
+	if (!p->phys_offset)
+		guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) *
+				       guest_page_size;
+	else
+		guest_test_phys_mem = p->phys_offset;
+#ifdef __s390x__
+	alignment = max(0x100000, alignment);
+#endif
+	guest_test_phys_mem &= ~(alignment - 1);
+
+	/* Set up the shared data structure perf_test_args */
+	perf_test_args.vm = vm;
+	perf_test_args.guest_test_virt_mem = guest_test_virt_mem;
+	perf_test_args.host_page_size = host_page_size;
+	perf_test_args.host_num_pages = test_mem_size / host_page_size;
+	perf_test_args.large_page_size = large_page_size;
+	perf_test_args.large_num_pages = test_mem_size / large_page_size;
+	perf_test_args.host_pages_per_lpage = large_page_size / host_page_size;
+	perf_test_args.src_type = src_type;
+
+	for (vcpu_id = 0; vcpu_id < KVM_MAX_VCPUS; vcpu_id++) {
+		vcpu_args = &perf_test_args.vcpu_args[vcpu_id];
+		vcpu_args->vcpu_id = vcpu_id;
+		vcpu_args->vcpu_write = !(vcpu_id % 2);
+	}
+
+	/* Add an extra memory slot with specified backing src type */
+	vm_userspace_mem_region_add(vm, src_type, guest_test_phys_mem,
+				    TEST_MEM_SLOT_INDEX, guest_num_pages, 0);
+
+	/* Do mapping(GVA->GPA) for the testing memory slot */
+	virt_map(vm, guest_test_virt_mem, guest_test_phys_mem, guest_num_pages, 0);
+
+	/* Cache the HVA pointer of the region */
+	host_test_mem = addr_gpa2hva(vm, (vm_paddr_t)guest_test_phys_mem);
+
+	/* Export shared structure perf_test_args to guest */
+	ucall_init(vm, NULL);
+	sync_global_to_guest(vm, perf_test_args);
+
+	sem_init_s(&test_stage_updated, 0, 0);
+	sem_init_s(&test_stage_completed, 0, 0);
+
+	current_stage = addr_gva2hva(vm, (vm_vaddr_t)(&guest_test_stage));
+	*current_stage = NUM_TEST_STAGES;
+
+	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
+	pr_info("Testing memory backing src type: %s\n",
+		vm_mem_backing_src_alias(src_type)->name);
+	pr_info("Testing memory backing src granularity: 0x%lx\n",
+		large_page_size);
+	pr_info("Testing memory size(aligned): 0x%lx\n", test_mem_size);
+	pr_info("Guest physical test memory offset: 0x%lx\n",
+		guest_test_phys_mem);
+	pr_info("Host  virtual  test memory offset: 0x%lx\n",
+		(uint64_t)host_test_mem);
+	pr_info("Number of testing vCPUs: %d\n", nr_vcpus);
+
+	return vm;
+}
+
+/* Wake up all the vcpus to run new test stage */
+static void vcpus_start_new_stage(void)
+{
+	int vcpus;
+
+	for (vcpus = 1; vcpus <= nr_vcpus; vcpus++)
+		sem_post_s(&test_stage_updated);
+
+	pr_debug("All vcpus have been notified to continue\n");
+}
+
+/* Wait for all the vcpus to complete new test stage */
+static void vcpus_complete_new_stage(enum test_stage stage)
+{
+	int vcpus;
+
+	for (vcpus = 1; vcpus <= nr_vcpus; vcpus++) {
+		sem_wait_s(&test_stage_completed);
+		pr_debug("%d vcpus have completed stage %s\n",
+			 vcpus, test_stage_string[stage]);
+	}
+
+	pr_debug("All vcpus have completed stage %s\n",
+		 test_stage_string[stage]);
+}
+
+static void run_test(enum vm_guest_mode mode, void *arg)
+{
+	pthread_t *vcpu_threads;
+	struct kvm_vm *vm;
+	int vcpu_id;
+	struct timespec start;
+	struct timespec ts_diff;
+
+	/* Create VM with vCPUs and make some pre-initialization */
+	vm = pre_init_before_test(mode, arg);
+
+	vcpu_threads = malloc(nr_vcpus * sizeof(*vcpu_threads));
+	TEST_ASSERT(vcpu_threads, "Memory allocation failed");
+
+	host_quit = false;
+	*current_stage = KVM_BEFORE_MAPPINGS;
+
+	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++) {
+		pthread_create(&vcpu_threads[vcpu_id], NULL, vcpu_worker,
+			       &perf_test_args.vcpu_args[vcpu_id]);
+	}
+
+	pr_info("Started all vCPUs successfully\n");
+
+	vcpus_complete_new_stage(*current_stage);
+
+	/* Test the stage of KVM creating mappings */
+	*current_stage = KVM_CREATE_MAPPINGS;
+
+	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
+	vcpus_start_new_stage();
+	vcpus_complete_new_stage(*current_stage);
+	ts_diff = timespec_elapsed(start);
+
+	pr_info("KVM_CREATE_MAPPINGS: total execution time: %ld.%.9lds\n\n",
+		ts_diff.tv_sec, ts_diff.tv_nsec);
+
+	/* Test the stage of KVM updating mappings */
+	vm_mem_region_set_flags(vm, TEST_MEM_SLOT_INDEX,
+				KVM_MEM_LOG_DIRTY_PAGES);
+
+	*current_stage = KVM_UPDATE_MAPPINGS;
+
+	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
+	vcpus_start_new_stage();
+	vcpus_complete_new_stage(*current_stage);
+	ts_diff = timespec_elapsed(start);
+
+	pr_info("KVM_UPDATE_MAPPINGS: total execution time: %ld.%.9lds\n\n",
+		ts_diff.tv_sec, ts_diff.tv_nsec);
+
+	/* Test the stage of KVM adjusting mappings */
+	vm_mem_region_set_flags(vm, TEST_MEM_SLOT_INDEX, 0);
+
+	*current_stage = KVM_ADJUST_MAPPINGS;
+
+	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
+	vcpus_start_new_stage();
+	vcpus_complete_new_stage(*current_stage);
+	ts_diff = timespec_elapsed(start);
+
+	pr_info("KVM_ADJUST_MAPPINGS: total execution time: %ld.%.9lds\n\n",
+		ts_diff.tv_sec, ts_diff.tv_nsec);
+
+	/* Tell the vcpu thread to quit */
+	host_quit = true;
+	vcpus_start_new_stage();
+
+	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++)
+		pthread_join(vcpu_threads[vcpu_id], NULL);
+
+	sem_destroy_s(&test_stage_updated);
+	sem_destroy_s(&test_stage_completed);
+
+	free(vcpu_threads);
+	ucall_uninit(vm);
+	kvm_vm_free(vm);
+}
+
+static void help(char *name)
+{
+	puts("");
+	printf("usage: %s [-h] [-p offset] [-m mode] "
+	       "[-b mem-size] [-v vcpus] [-s mem-type]\n", name);
+	puts("");
+	printf(" -p: specify guest physical test memory offset\n"
+	       "     Warning: a low offset can conflict with the loaded test code.\n");
+	guest_modes_help();
+	printf(" -b: specify size of the memory region for testing. e.g. 10M or 3G.\n"
+	       "     (default: 1G)\n");
+	printf(" -v: specify the number of vCPUs to run\n"
+	       "     (default: 1)\n");
+	printf(" -s: specify the type of memory that should be used to\n"
+	       "     back the guest data region.\n"
+	       "     (default: anonymous)\n\n");
+	backing_src_help();
+	puts("");
+	exit(0);
+}
+
+int main(int argc, char *argv[])
+{
+	int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
+	struct test_params p = {
+		.test_mem_size = DEFAULT_TEST_MEM_SIZE,
+		.src_type = VM_MEM_SRC_ANONYMOUS,
+	};
+	int opt;
+
+	guest_modes_append_default();
+
+	while ((opt = getopt(argc, argv, "hp:m:b:v:s:")) != -1) {
+		switch (opt) {
+		case 'p':
+			p.phys_offset = strtoull(optarg, NULL, 0);
+			break;
+		case 'm':
+			guest_modes_cmdline(optarg);
+			break;
+		case 'b':
+			p.test_mem_size = parse_size(optarg);
+			break;
+		case 'v':
+			nr_vcpus = atoi(optarg);
+			TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
+				    "Invalid number of vcpus, must be between 1 and %d", max_vcpus);
+			break;
+		case 's':
+			p.src_type = parse_backing_src_type(optarg);
+			break;
+		case 'h':
+		default:
+			help(argv[0]);
+			break;
+		}
+	}
+
+	for_each_guest_mode(run_test, &p);
+
+	return 0;
+}
-- 
2.19.1

Re: [RFC PATCH v5 02/10] tools headers: Add a macro to get HUGETLB page sizes for mmap

[Andrew Jones]

$SUBJECT says "tools headers", but this is actually changing
a UAPI header and then copying the change to tools.

Thanks,
drew

On Tue, Mar 23, 2021 at 09:52:23PM +0800, Yanan Wang wrote:
> We know that if a system supports multiple hugetlb page sizes,
> the desired hugetlb page size can be specified in bits [26:31]
> of the flag arguments. The value in these 6 bits will be the
> shift of each hugetlb page size.
> 
> So add a macro to get the page size shift and then calculate the
> corresponding hugetlb page size, using flag x.
> 
> Cc: Ben Gardon <bgardon@google.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Suggested-by: Ben Gardon <bgardon@google.com>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> Reviewed-by: Ben Gardon <bgardon@google.com>
> ---
>  include/uapi/linux/mman.h       | 2 ++
>  tools/include/uapi/linux/mman.h | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/include/uapi/linux/mman.h b/include/uapi/linux/mman.h
> index f55bc680b5b0..d72df73b182d 100644
> --- a/include/uapi/linux/mman.h
> +++ b/include/uapi/linux/mman.h
> @@ -41,4 +41,6 @@
>  #define MAP_HUGE_2GB	HUGETLB_FLAG_ENCODE_2GB
>  #define MAP_HUGE_16GB	HUGETLB_FLAG_ENCODE_16GB
>  
> +#define MAP_HUGE_PAGE_SIZE(x) (1ULL << ((x >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK))
> +
>  #endif /* _UAPI_LINUX_MMAN_H */
> diff --git a/tools/include/uapi/linux/mman.h b/tools/include/uapi/linux/mman.h
> index f55bc680b5b0..d72df73b182d 100644
> --- a/tools/include/uapi/linux/mman.h
> +++ b/tools/include/uapi/linux/mman.h
> @@ -41,4 +41,6 @@
>  #define MAP_HUGE_2GB	HUGETLB_FLAG_ENCODE_2GB
>  #define MAP_HUGE_16GB	HUGETLB_FLAG_ENCODE_16GB
>  
> +#define MAP_HUGE_PAGE_SIZE(x) (1ULL << ((x >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK))
> +
>  #endif /* _UAPI_LINUX_MMAN_H */
> -- 
> 2.19.1
> 

Re: [RFC PATCH v5 04/10] KVM: selftests: Print the errno besides error-string in TEST_ASSERT

[Andrew Jones]

On Tue, Mar 23, 2021 at 09:52:25PM +0800, Yanan Wang wrote:
> Print the errno besides error-string in TEST_ASSERT in the format of
> "errno=%d - %s" will explicitly indicate that the string is an error
> information. Besides, the errno is easier to be used for debugging
> than the error-string.
> 
> Suggested-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  tools/testing/selftests/kvm/lib/assert.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c
> index 5ebbd0d6b472..71ade6100fd3 100644
> --- a/tools/testing/selftests/kvm/lib/assert.c
> +++ b/tools/testing/selftests/kvm/lib/assert.c
> @@ -71,9 +71,9 @@ test_assert(bool exp, const char *exp_str,
>  
>  		fprintf(stderr, "==== Test Assertion Failure ====\n"
>  			"  %s:%u: %s\n"
> -			"  pid=%d tid=%d - %s\n",
> +			"  pid=%d tid=%d errno=%d - %s\n",
>  			file, line, exp_str, getpid(), _gettid(),
> -			strerror(errno));
> +			errno, strerror(errno));
>  		test_dump_stack();
>  		if (fmt) {
>  			fputs("  ", stderr);
> -- 
> 2.19.1
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [RFC PATCH v5 06/10] KVM: selftests: Add a helper to get system configured THP page size

[Andrew Jones]

On Tue, Mar 23, 2021 at 09:52:27PM +0800, Yanan Wang wrote:
> If we want to have some tests about transparent hugepages, the system
> configured THP hugepage size should better be known by the tests, which
> can be used for kinds of alignment or guest memory accessing of vcpus...
> So it makes sense to add a helper to get the transparent hugepage size.
> 
> With VM_MEM_SRC_ANONYMOUS_THP specified in vm_userspace_mem_region_add(),
> we now stat /sys/kernel/mm/transparent_hugepage to check whether THP is
> configured in the host kernel before madvise(). Based on this, we can also
> read file /sys/kernel/mm/transparent_hugepage/hpage_pmd_size to get THP
> hugepage size.
> 
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> Reviewed-by: Ben Gardon <bgardon@google.com>
> ---
>  .../testing/selftests/kvm/include/test_util.h |  2 ++
>  tools/testing/selftests/kvm/lib/test_util.c   | 29 +++++++++++++++++++
>  2 files changed, 31 insertions(+)


Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [RFC PATCH v5 08/10] KVM: selftests: List all hugetlb src types specified with page sizes

[Andrew Jones]

On Tue, Mar 23, 2021 at 09:52:29PM +0800, Yanan Wang wrote:
> With VM_MEM_SRC_ANONYMOUS_HUGETLB, we currently can only use system
> default hugetlb pages to back the testing guest memory. In order to
> add flexibility, now list all the known hugetlb backing src types with
> different page sizes, so that we can specify use of hugetlb pages of the
> exact granularity that we want. And as all the known hugetlb page sizes
> are listed, it's appropriate for all architectures.
> 
> Besides, the helper get_backing_src_pagesz() is added to get the
> granularity of different backing src types(anonumous, thp, hugetlb).
> 
> Suggested-by: Ben Gardon <bgardon@google.com>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  .../testing/selftests/kvm/include/test_util.h |  18 ++-
>  tools/testing/selftests/kvm/lib/kvm_util.c    |   2 +-
>  tools/testing/selftests/kvm/lib/test_util.c   | 109 ++++++++++++++++--
>  3 files changed, 116 insertions(+), 13 deletions(-)
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [RFC PATCH v5 00/10] KVM: selftests: some improvement and a new test for kvm page table

[Sean Christopherson]

On Tue, Mar 23, 2021, Yanan Wang wrote:
> Hi,
> This v5 series can mainly include two parts.
> Based on kvm queue branch: https://git.kernel.org/pub/scm/virt/kvm/kvm.git/log/?h=queue

Given the number of Reviewed-by tags, I'm pretty sure you can drop the "RFC" :-)

Re: [RFC PATCH v5 02/10] tools headers: Add a macro to get HUGETLB page sizes for mmap

[wangyanan (Y)]

On 2021/3/23 22:03, Andrew Jones wrote:
> $SUBJECT says "tools headers", but this is actually changing
> a UAPI header and then copying the change to tools.
Indeed. I think head of the subject should be "mm/hugetlb".
I will fix it.

Thanks,
Yanan
> Thanks,
> drew
>
> On Tue, Mar 23, 2021 at 09:52:23PM +0800, Yanan Wang wrote:
>> We know that if a system supports multiple hugetlb page sizes,
>> the desired hugetlb page size can be specified in bits [26:31]
>> of the flag arguments. The value in these 6 bits will be the
>> shift of each hugetlb page size.
>>
>> So add a macro to get the page size shift and then calculate the
>> corresponding hugetlb page size, using flag x.
>>
>> Cc: Ben Gardon <bgardon@google.com>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Cc: Adrian Hunter <adrian.hunter@intel.com>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Michael Kerrisk <mtk.manpages@gmail.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Suggested-by: Ben Gardon <bgardon@google.com>
>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> Reviewed-by: Ben Gardon <bgardon@google.com>
>> ---
>>   include/uapi/linux/mman.h       | 2 ++
>>   tools/include/uapi/linux/mman.h | 2 ++
>>   2 files changed, 4 insertions(+)
>>
>> diff --git a/include/uapi/linux/mman.h b/include/uapi/linux/mman.h
>> index f55bc680b5b0..d72df73b182d 100644
>> --- a/include/uapi/linux/mman.h
>> +++ b/include/uapi/linux/mman.h
>> @@ -41,4 +41,6 @@
>>   #define MAP_HUGE_2GB	HUGETLB_FLAG_ENCODE_2GB
>>   #define MAP_HUGE_16GB	HUGETLB_FLAG_ENCODE_16GB
>>   
>> +#define MAP_HUGE_PAGE_SIZE(x) (1ULL << ((x >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK))
>> +
>>   #endif /* _UAPI_LINUX_MMAN_H */
>> diff --git a/tools/include/uapi/linux/mman.h b/tools/include/uapi/linux/mman.h
>> index f55bc680b5b0..d72df73b182d 100644
>> --- a/tools/include/uapi/linux/mman.h
>> +++ b/tools/include/uapi/linux/mman.h
>> @@ -41,4 +41,6 @@
>>   #define MAP_HUGE_2GB	HUGETLB_FLAG_ENCODE_2GB
>>   #define MAP_HUGE_16GB	HUGETLB_FLAG_ENCODE_16GB
>>   
>> +#define MAP_HUGE_PAGE_SIZE(x) (1ULL << ((x >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK))
>> +
>>   #endif /* _UAPI_LINUX_MMAN_H */
>> -- 
>> 2.19.1
>>
> .

Re: [RFC PATCH v5 00/10] KVM: selftests: some improvement and a new test for kvm page table

[wangyanan (Y)]

On 2021/3/23 23:58, Sean Christopherson wrote:
> On Tue, Mar 23, 2021, Yanan Wang wrote:
>> Hi,
>> This v5 series can mainly include two parts.
>> Based on kvm queue branch: https://git.kernel.org/pub/scm/virt/kvm/kvm.git/log/?h=queue
> Given the number of Reviewed-by tags, I'm pretty sure you can drop the "RFC" :-)
Ah, yes! Will drop it in v6 where I hope everything would be fine. :)

Thanks,
Yanan
> .

Re: [RFC PATCH v5 10/10] KVM: selftests: Add a test for kvm page table code

[wangyanan (Y)]

Hi Drew,

BTW, any thoughts about the change in this patch? :)

Thanks,
Yanan
On 2021/3/23 21:52, Yanan Wang wrote:
> This test serves as a performance tester and a bug reproducer for
> kvm page table code (GPA->HPA mappings), so it gives guidance for
> people trying to make some improvement for kvm.
>
> The function guest_code() can cover the conditions where a single vcpu or
> multiple vcpus access guest pages within the same memory region, in three
> VM stages(before dirty logging, during dirty logging, after dirty logging).
> Besides, the backing src memory type(ANONYMOUS/THP/HUGETLB) of the tested
> memory region can be specified by users, which means normal page mappings
> or block mappings can be chosen by users to be created in the test.
>
> If ANONYMOUS memory is specified, kvm will create normal page mappings
> for the tested memory region before dirty logging, and update attributes
> of the page mappings from RO to RW during dirty logging. If THP/HUGETLB
> memory is specified, kvm will create block mappings for the tested memory
> region before dirty logging, and split the blcok mappings into normal page
> mappings during dirty logging, and coalesce the page mappings back into
> block mappings after dirty logging is stopped.
>
> So in summary, as a performance tester, this test can present the
> performance of kvm creating/updating normal page mappings, or the
> performance of kvm creating/splitting/recovering block mappings,
> through execution time.
>
> When we need to coalesce the page mappings back to block mappings after
> dirty logging is stopped, we have to firstly invalidate *all* the TLB
> entries for the page mappings right before installation of the block entry,
> because a TLB conflict abort error could occur if we can't invalidate the
> TLB entries fully. We have hit this TLB conflict twice on aarch64 software
> implementation and fixed it. As this test can imulate process from dirty
> logging enabled to dirty logging stopped of a VM with block mappings,
> so it can also reproduce this TLB conflict abort due to inadequate TLB
> invalidation when coalescing tables.
>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> Reviewed-by: Ben Gardon <bgardon@google.com>
> ---
>   tools/testing/selftests/kvm/.gitignore        |   1 +
>   tools/testing/selftests/kvm/Makefile          |   3 +
>   .../selftests/kvm/kvm_page_table_test.c       | 512 ++++++++++++++++++
>   3 files changed, 516 insertions(+)
>   create mode 100644 tools/testing/selftests/kvm/kvm_page_table_test.c
>
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index 32b87cc77c8e..137ab7273be6 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -35,6 +35,7 @@
>   /dirty_log_perf_test
>   /hardware_disable_test
>   /kvm_create_max_vcpus
> +/kvm_page_table_test
>   /memslot_modification_stress_test
>   /set_memory_region_test
>   /steal_time
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index a6d61f451f88..75dc57db36b4 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -69,6 +69,7 @@ TEST_GEN_PROGS_x86_64 += dirty_log_test
>   TEST_GEN_PROGS_x86_64 += dirty_log_perf_test
>   TEST_GEN_PROGS_x86_64 += hardware_disable_test
>   TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
> +TEST_GEN_PROGS_x86_64 += kvm_page_table_test
>   TEST_GEN_PROGS_x86_64 += memslot_modification_stress_test
>   TEST_GEN_PROGS_x86_64 += set_memory_region_test
>   TEST_GEN_PROGS_x86_64 += steal_time
> @@ -79,6 +80,7 @@ TEST_GEN_PROGS_aarch64 += demand_paging_test
>   TEST_GEN_PROGS_aarch64 += dirty_log_test
>   TEST_GEN_PROGS_aarch64 += dirty_log_perf_test
>   TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
> +TEST_GEN_PROGS_aarch64 += kvm_page_table_test
>   TEST_GEN_PROGS_aarch64 += set_memory_region_test
>   TEST_GEN_PROGS_aarch64 += steal_time
>   
> @@ -88,6 +90,7 @@ TEST_GEN_PROGS_s390x += s390x/sync_regs_test
>   TEST_GEN_PROGS_s390x += demand_paging_test
>   TEST_GEN_PROGS_s390x += dirty_log_test
>   TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
> +TEST_GEN_PROGS_s390x += kvm_page_table_test
>   TEST_GEN_PROGS_s390x += set_memory_region_test
>   
>   TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
> diff --git a/tools/testing/selftests/kvm/kvm_page_table_test.c b/tools/testing/selftests/kvm/kvm_page_table_test.c
> new file mode 100644
> index 000000000000..bbd5c489d61f
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/kvm_page_table_test.c
> @@ -0,0 +1,512 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KVM page table test
> + *
> + * Copyright (C) 2021, Huawei, Inc.
> + *
> + * Make sure that THP has been enabled or enough HUGETLB pages with specific
> + * page size have been pre-allocated on your system, if you are planning to
> + * use hugepages to back the guest memory for testing.
> + */
> +
> +#define _GNU_SOURCE /* for program_invocation_name */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <time.h>
> +#include <pthread.h>
> +#include <semaphore.h>
> +
> +#include "test_util.h"
> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "guest_modes.h"
> +
> +#define TEST_MEM_SLOT_INDEX             1
> +
> +/* Default size(1GB) of the memory for testing */
> +#define DEFAULT_TEST_MEM_SIZE		(1 << 30)
> +
> +/* Default guest test virtual memory offset */
> +#define DEFAULT_GUEST_TEST_MEM		0xc0000000
> +
> +/*
> + * In our test, we use thread synchronization functions (e.g. sem_wait)
> + * for time measurement and they can't fail at all, since a failure will
> + * impact the time accuracy and vcpus will not run as what we expect.
> + * So we will use safer versions of the functions.
> + */
> +#define sem_init_s(sem_ptr, ps, val) \
> +	TEST_ASSERT(sem_init(sem_ptr, ps, val) == 0, "Error in sem_init")
> +#define sem_destroy_s(sem_ptr) \
> +	TEST_ASSERT(sem_destroy(sem_ptr) == 0, "Error in sem_destroy")
> +#define sem_wait_s(sem_ptr) \
> +	TEST_ASSERT(sem_wait(sem_ptr) == 0, "Error in sem_wait")
> +#define sem_post_s(sem_ptr) \
> +	TEST_ASSERT(sem_post(sem_ptr) == 0, "Error in sem_post")
> +
> +/* Different guest memory accessing stages */
> +enum test_stage {
> +	KVM_BEFORE_MAPPINGS,
> +	KVM_CREATE_MAPPINGS,
> +	KVM_UPDATE_MAPPINGS,
> +	KVM_ADJUST_MAPPINGS,
> +	NUM_TEST_STAGES,
> +};
> +
> +static const char * const test_stage_string[] = {
> +	"KVM_BEFORE_MAPPINGS",
> +	"KVM_CREATE_MAPPINGS",
> +	"KVM_UPDATE_MAPPINGS",
> +	"KVM_ADJUST_MAPPINGS",
> +};
> +
> +struct perf_test_vcpu_args {
> +	int vcpu_id;
> +	bool vcpu_write;
> +};
> +
> +struct perf_test_args {
> +	struct kvm_vm *vm;
> +	uint64_t guest_test_virt_mem;
> +	uint64_t host_page_size;
> +	uint64_t host_num_pages;
> +	uint64_t large_page_size;
> +	uint64_t large_num_pages;
> +	uint64_t host_pages_per_lpage;
> +	enum vm_mem_backing_src_type src_type;
> +	struct perf_test_vcpu_args vcpu_args[KVM_MAX_VCPUS];
> +};
> +
> +/*
> + * Guest variables. Use addr_gva2hva() if these variables need
> + * to be changed in host.
> + */
> +static enum test_stage guest_test_stage;
> +
> +/* Host variables */
> +static uint32_t nr_vcpus = 1;
> +static struct perf_test_args perf_test_args;
> +static enum test_stage *current_stage;
> +static bool host_quit;
> +
> +/* Whether the test stage is updated, or completed */
> +static sem_t test_stage_updated;
> +static sem_t test_stage_completed;
> +
> +/*
> + * 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 void guest_code(int vcpu_id)
> +{
> +	struct perf_test_args *p = &perf_test_args;
> +	struct perf_test_vcpu_args *vcpu_args = &p->vcpu_args[vcpu_id];
> +	enum vm_mem_backing_src_type src_type = p->src_type;
> +	uint64_t host_page_size = p->host_page_size;
> +	uint64_t host_num_pages = p->host_num_pages;
> +	uint64_t large_page_size = p->large_page_size;
> +	uint64_t large_num_pages = p->large_num_pages;
> +	uint64_t host_pages_per_lpage = p->host_pages_per_lpage;
> +	uint64_t half = host_pages_per_lpage / 2;
> +	bool vcpu_write;
> +	enum test_stage stage;
> +	uint64_t addr;
> +	int i, j;
> +
> +	/* Make sure vCPU args data structure is not corrupt */
> +	GUEST_ASSERT(vcpu_args->vcpu_id == vcpu_id);
> +	vcpu_write = vcpu_args->vcpu_write;
> +
> +	while (true) {
> +		stage = READ_ONCE(guest_test_stage);
> +		addr = perf_test_args.guest_test_virt_mem;
> +
> +		switch (stage) {
> +		/*
> +		 * Before dirty logging, vCPUs concurrently access the first
> +		 * 8 bytes of each page (host page/large page) within the same
> +		 * memory region with different accessing types (read/write).
> +		 * Then KVM will create normal page mappings or huge block
> +		 * mappings for them.
> +		 */
> +		case KVM_CREATE_MAPPINGS:
> +			for (i = 0; i < large_num_pages; i++) {
> +				if (vcpu_write)
> +					*(uint64_t *)addr = 0x0123456789ABCDEF;
> +				else
> +					READ_ONCE(*(uint64_t *)addr);
> +
> +				addr += large_page_size;
> +			}
> +			break;
> +
> +		/*
> +		 * During dirty logging, KVM will only update attributes of the
> +		 * normal page mappings from RO to RW if memory backing src type
> +		 * is anonymous. In other cases, KVM will split the huge block
> +		 * mappings into normal page mappings if memory backing src type
> +		 * is THP or HUGETLB.
> +		 */
> +		case KVM_UPDATE_MAPPINGS:
> +			if (src_type == VM_MEM_SRC_ANONYMOUS) {
> +				for (i = 0; i < host_num_pages; i++) {
> +					*(uint64_t *)addr = 0x0123456789ABCDEF;
> +					addr += host_page_size;
> +				}
> +				break;
> +			}
> +
> +			for (i = 0; i < large_num_pages; i++) {
> +				/*
> +				 * Write to the first host page in each large
> +				 * page region, and triger break of large pages.
> +				 */
> +				*(uint64_t *)addr = 0x0123456789ABCDEF;
> +
> +				/*
> +				 * Access the middle host pages in each large
> +				 * page region. Since dirty logging is enabled,
> +				 * this will create new mappings at the smallest
> +				 * granularity.
> +				 */
> +				addr += host_page_size * half;
> +				for (j = half; j < host_pages_per_lpage; j++) {
> +					READ_ONCE(*(uint64_t *)addr);
> +					addr += host_page_size;
> +				}
> +			}
> +			break;
> +
> +		/*
> +		 * After dirty logging is stopped, vCPUs concurrently read
> +		 * from every single host page. Then KVM will coalesce the
> +		 * split page mappings back to block mappings. And a TLB
> +		 * conflict abort could occur here if TLB entries of the
> +		 * page mappings are not fully invalidated.
> +		 */
> +		case KVM_ADJUST_MAPPINGS:
> +			for (i = 0; i < host_num_pages; i++) {
> +				READ_ONCE(*(uint64_t *)addr);
> +				addr += host_page_size;
> +			}
> +			break;
> +
> +		default:
> +			break;
> +		}
> +
> +		GUEST_SYNC(1);
> +	}
> +}
> +
> +static void *vcpu_worker(void *data)
> +{
> +	int ret;
> +	struct perf_test_vcpu_args *vcpu_args = data;
> +	struct kvm_vm *vm = perf_test_args.vm;
> +	int vcpu_id = vcpu_args->vcpu_id;
> +	struct kvm_run *run;
> +	struct timespec start;
> +	struct timespec ts_diff;
> +	enum test_stage stage;
> +
> +	vcpu_args_set(vm, vcpu_id, 1, vcpu_id);
> +	run = vcpu_state(vm, vcpu_id);
> +
> +	while (!READ_ONCE(host_quit)) {
> +		clock_gettime(CLOCK_MONOTONIC_RAW, &start);
> +		ret = _vcpu_run(vm, vcpu_id);
> +		ts_diff = timespec_elapsed(start);
> +
> +		TEST_ASSERT(ret == 0, "vcpu_run failed: %d\n", ret);
> +		TEST_ASSERT(get_ucall(vm, vcpu_id, NULL) == UCALL_SYNC,
> +			    "Invalid guest sync status: exit_reason=%s\n",
> +			    exit_reason_str(run->exit_reason));
> +
> +		pr_debug("Got sync event from vCPU %d\n", vcpu_id);
> +		stage = READ_ONCE(*current_stage);
> +
> +		/*
> +		 * Here we can know the execution time of every
> +		 * single vcpu running in different test stages.
> +		 */
> +		pr_debug("vCPU %d has completed stage %s\n"
> +			 "execution time is: %ld.%.9lds\n\n",
> +			 vcpu_id, test_stage_string[stage],
> +			 ts_diff.tv_sec, ts_diff.tv_nsec);
> +
> +		sem_post_s(&test_stage_completed);
> +		sem_wait_s(&test_stage_updated);
> +	}
> +
> +	return NULL;
> +}
> +
> +struct test_params {
> +	uint64_t phys_offset;
> +	uint64_t test_mem_size;
> +	enum vm_mem_backing_src_type src_type;
> +};
> +
> +static struct kvm_vm *pre_init_before_test(enum vm_guest_mode mode, void *arg)
> +{
> +	struct test_params *p = arg;
> +	struct perf_test_vcpu_args *vcpu_args;
> +	enum vm_mem_backing_src_type src_type = p->src_type;
> +	uint64_t large_page_size = get_backing_src_pagesz(src_type);
> +	uint64_t test_mem_size = p->test_mem_size, guest_num_pages;
> +	uint64_t guest_page_size = vm_guest_mode_params[mode].page_size;
> +	uint64_t host_page_size = getpagesize();
> +	uint64_t alignment;
> +	void *host_test_mem;
> +	struct kvm_vm *vm;
> +	int vcpu_id;
> +
> +	/* Align up the test memory size */
> +	alignment = max(large_page_size, guest_page_size);
> +	test_mem_size = (test_mem_size + alignment - 1) & ~(alignment - 1);
> +
> +	/* Create a VM with enough guest pages */
> +	guest_num_pages = test_mem_size / guest_page_size;
> +	vm = vm_create_with_vcpus(mode, nr_vcpus,
> +				  guest_num_pages, 0, guest_code, NULL);
> +
> +	/* Align down GPA of the testing memslot */
> +	if (!p->phys_offset)
> +		guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) *
> +				       guest_page_size;
> +	else
> +		guest_test_phys_mem = p->phys_offset;
> +#ifdef __s390x__
> +	alignment = max(0x100000, alignment);
> +#endif
> +	guest_test_phys_mem &= ~(alignment - 1);
> +
> +	/* Set up the shared data structure perf_test_args */
> +	perf_test_args.vm = vm;
> +	perf_test_args.guest_test_virt_mem = guest_test_virt_mem;
> +	perf_test_args.host_page_size = host_page_size;
> +	perf_test_args.host_num_pages = test_mem_size / host_page_size;
> +	perf_test_args.large_page_size = large_page_size;
> +	perf_test_args.large_num_pages = test_mem_size / large_page_size;
> +	perf_test_args.host_pages_per_lpage = large_page_size / host_page_size;
> +	perf_test_args.src_type = src_type;
> +
> +	for (vcpu_id = 0; vcpu_id < KVM_MAX_VCPUS; vcpu_id++) {
> +		vcpu_args = &perf_test_args.vcpu_args[vcpu_id];
> +		vcpu_args->vcpu_id = vcpu_id;
> +		vcpu_args->vcpu_write = !(vcpu_id % 2);
> +	}
> +
> +	/* Add an extra memory slot with specified backing src type */
> +	vm_userspace_mem_region_add(vm, src_type, guest_test_phys_mem,
> +				    TEST_MEM_SLOT_INDEX, guest_num_pages, 0);
> +
> +	/* Do mapping(GVA->GPA) for the testing memory slot */
> +	virt_map(vm, guest_test_virt_mem, guest_test_phys_mem, guest_num_pages, 0);
> +
> +	/* Cache the HVA pointer of the region */
> +	host_test_mem = addr_gpa2hva(vm, (vm_paddr_t)guest_test_phys_mem);
> +
> +	/* Export shared structure perf_test_args to guest */
> +	ucall_init(vm, NULL);
> +	sync_global_to_guest(vm, perf_test_args);
> +
> +	sem_init_s(&test_stage_updated, 0, 0);
> +	sem_init_s(&test_stage_completed, 0, 0);
> +
> +	current_stage = addr_gva2hva(vm, (vm_vaddr_t)(&guest_test_stage));
> +	*current_stage = NUM_TEST_STAGES;
> +
> +	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
> +	pr_info("Testing memory backing src type: %s\n",
> +		vm_mem_backing_src_alias(src_type)->name);
> +	pr_info("Testing memory backing src granularity: 0x%lx\n",
> +		large_page_size);
> +	pr_info("Testing memory size(aligned): 0x%lx\n", test_mem_size);
> +	pr_info("Guest physical test memory offset: 0x%lx\n",
> +		guest_test_phys_mem);
> +	pr_info("Host  virtual  test memory offset: 0x%lx\n",
> +		(uint64_t)host_test_mem);
> +	pr_info("Number of testing vCPUs: %d\n", nr_vcpus);
> +
> +	return vm;
> +}
> +
> +/* Wake up all the vcpus to run new test stage */
> +static void vcpus_start_new_stage(void)
> +{
> +	int vcpus;
> +
> +	for (vcpus = 1; vcpus <= nr_vcpus; vcpus++)
> +		sem_post_s(&test_stage_updated);
> +
> +	pr_debug("All vcpus have been notified to continue\n");
> +}
> +
> +/* Wait for all the vcpus to complete new test stage */
> +static void vcpus_complete_new_stage(enum test_stage stage)
> +{
> +	int vcpus;
> +
> +	for (vcpus = 1; vcpus <= nr_vcpus; vcpus++) {
> +		sem_wait_s(&test_stage_completed);
> +		pr_debug("%d vcpus have completed stage %s\n",
> +			 vcpus, test_stage_string[stage]);
> +	}
> +
> +	pr_debug("All vcpus have completed stage %s\n",
> +		 test_stage_string[stage]);
> +}
> +
> +static void run_test(enum vm_guest_mode mode, void *arg)
> +{
> +	pthread_t *vcpu_threads;
> +	struct kvm_vm *vm;
> +	int vcpu_id;
> +	struct timespec start;
> +	struct timespec ts_diff;
> +
> +	/* Create VM with vCPUs and make some pre-initialization */
> +	vm = pre_init_before_test(mode, arg);
> +
> +	vcpu_threads = malloc(nr_vcpus * sizeof(*vcpu_threads));
> +	TEST_ASSERT(vcpu_threads, "Memory allocation failed");
> +
> +	host_quit = false;
> +	*current_stage = KVM_BEFORE_MAPPINGS;
> +
> +	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++) {
> +		pthread_create(&vcpu_threads[vcpu_id], NULL, vcpu_worker,
> +			       &perf_test_args.vcpu_args[vcpu_id]);
> +	}
> +
> +	pr_info("Started all vCPUs successfully\n");
> +
> +	vcpus_complete_new_stage(*current_stage);
> +
> +	/* Test the stage of KVM creating mappings */
> +	*current_stage = KVM_CREATE_MAPPINGS;
> +
> +	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
> +	vcpus_start_new_stage();
> +	vcpus_complete_new_stage(*current_stage);
> +	ts_diff = timespec_elapsed(start);
> +
> +	pr_info("KVM_CREATE_MAPPINGS: total execution time: %ld.%.9lds\n\n",
> +		ts_diff.tv_sec, ts_diff.tv_nsec);
> +
> +	/* Test the stage of KVM updating mappings */
> +	vm_mem_region_set_flags(vm, TEST_MEM_SLOT_INDEX,
> +				KVM_MEM_LOG_DIRTY_PAGES);
> +
> +	*current_stage = KVM_UPDATE_MAPPINGS;
> +
> +	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
> +	vcpus_start_new_stage();
> +	vcpus_complete_new_stage(*current_stage);
> +	ts_diff = timespec_elapsed(start);
> +
> +	pr_info("KVM_UPDATE_MAPPINGS: total execution time: %ld.%.9lds\n\n",
> +		ts_diff.tv_sec, ts_diff.tv_nsec);
> +
> +	/* Test the stage of KVM adjusting mappings */
> +	vm_mem_region_set_flags(vm, TEST_MEM_SLOT_INDEX, 0);
> +
> +	*current_stage = KVM_ADJUST_MAPPINGS;
> +
> +	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
> +	vcpus_start_new_stage();
> +	vcpus_complete_new_stage(*current_stage);
> +	ts_diff = timespec_elapsed(start);
> +
> +	pr_info("KVM_ADJUST_MAPPINGS: total execution time: %ld.%.9lds\n\n",
> +		ts_diff.tv_sec, ts_diff.tv_nsec);
> +
> +	/* Tell the vcpu thread to quit */
> +	host_quit = true;
> +	vcpus_start_new_stage();
> +
> +	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++)
> +		pthread_join(vcpu_threads[vcpu_id], NULL);
> +
> +	sem_destroy_s(&test_stage_updated);
> +	sem_destroy_s(&test_stage_completed);
> +
> +	free(vcpu_threads);
> +	ucall_uninit(vm);
> +	kvm_vm_free(vm);
> +}
> +
> +static void help(char *name)
> +{
> +	puts("");
> +	printf("usage: %s [-h] [-p offset] [-m mode] "
> +	       "[-b mem-size] [-v vcpus] [-s mem-type]\n", name);
> +	puts("");
> +	printf(" -p: specify guest physical test memory offset\n"
> +	       "     Warning: a low offset can conflict with the loaded test code.\n");
> +	guest_modes_help();
> +	printf(" -b: specify size of the memory region for testing. e.g. 10M or 3G.\n"
> +	       "     (default: 1G)\n");
> +	printf(" -v: specify the number of vCPUs to run\n"
> +	       "     (default: 1)\n");
> +	printf(" -s: specify the type of memory that should be used to\n"
> +	       "     back the guest data region.\n"
> +	       "     (default: anonymous)\n\n");
> +	backing_src_help();
> +	puts("");
> +	exit(0);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
> +	struct test_params p = {
> +		.test_mem_size = DEFAULT_TEST_MEM_SIZE,
> +		.src_type = VM_MEM_SRC_ANONYMOUS,
> +	};
> +	int opt;
> +
> +	guest_modes_append_default();
> +
> +	while ((opt = getopt(argc, argv, "hp:m:b:v:s:")) != -1) {
> +		switch (opt) {
> +		case 'p':
> +			p.phys_offset = strtoull(optarg, NULL, 0);
> +			break;
> +		case 'm':
> +			guest_modes_cmdline(optarg);
> +			break;
> +		case 'b':
> +			p.test_mem_size = parse_size(optarg);
> +			break;
> +		case 'v':
> +			nr_vcpus = atoi(optarg);
> +			TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
> +				    "Invalid number of vcpus, must be between 1 and %d", max_vcpus);
> +			break;
> +		case 's':
> +			p.src_type = parse_backing_src_type(optarg);
> +			break;
> +		case 'h':
> +		default:
> +			help(argv[0]);
> +			break;
> +		}
> +	}
> +
> +	for_each_guest_mode(run_test, &p);
> +
> +	return 0;
> +}

Re: [RFC PATCH v5 10/10] KVM: selftests: Add a test for kvm page table code

[Andrew Jones]

On Tue, Mar 23, 2021 at 09:52:31PM +0800, Yanan Wang wrote:
> This test serves as a performance tester and a bug reproducer for
> kvm page table code (GPA->HPA mappings), so it gives guidance for
> people trying to make some improvement for kvm.
> 
> The function guest_code() can cover the conditions where a single vcpu or
> multiple vcpus access guest pages within the same memory region, in three
> VM stages(before dirty logging, during dirty logging, after dirty logging).
> Besides, the backing src memory type(ANONYMOUS/THP/HUGETLB) of the tested
> memory region can be specified by users, which means normal page mappings
> or block mappings can be chosen by users to be created in the test.
> 
> If ANONYMOUS memory is specified, kvm will create normal page mappings
> for the tested memory region before dirty logging, and update attributes
> of the page mappings from RO to RW during dirty logging. If THP/HUGETLB
> memory is specified, kvm will create block mappings for the tested memory
> region before dirty logging, and split the blcok mappings into normal page
> mappings during dirty logging, and coalesce the page mappings back into
> block mappings after dirty logging is stopped.
> 
> So in summary, as a performance tester, this test can present the
> performance of kvm creating/updating normal page mappings, or the
> performance of kvm creating/splitting/recovering block mappings,
> through execution time.
> 
> When we need to coalesce the page mappings back to block mappings after
> dirty logging is stopped, we have to firstly invalidate *all* the TLB
> entries for the page mappings right before installation of the block entry,
> because a TLB conflict abort error could occur if we can't invalidate the
> TLB entries fully. We have hit this TLB conflict twice on aarch64 software
> implementation and fixed it. As this test can imulate process from dirty
> logging enabled to dirty logging stopped of a VM with block mappings,
> so it can also reproduce this TLB conflict abort due to inadequate TLB
> invalidation when coalescing tables.
> 
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> Reviewed-by: Ben Gardon <bgardon@google.com>
> ---
>  tools/testing/selftests/kvm/.gitignore        |   1 +
>  tools/testing/selftests/kvm/Makefile          |   3 +
>  .../selftests/kvm/kvm_page_table_test.c       | 512 ++++++++++++++++++
>  3 files changed, 516 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/kvm_page_table_test.c
> 
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index 32b87cc77c8e..137ab7273be6 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -35,6 +35,7 @@
>  /dirty_log_perf_test
>  /hardware_disable_test
>  /kvm_create_max_vcpus
> +/kvm_page_table_test
>  /memslot_modification_stress_test
>  /set_memory_region_test
>  /steal_time
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index a6d61f451f88..75dc57db36b4 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -69,6 +69,7 @@ TEST_GEN_PROGS_x86_64 += dirty_log_test
>  TEST_GEN_PROGS_x86_64 += dirty_log_perf_test
>  TEST_GEN_PROGS_x86_64 += hardware_disable_test
>  TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
> +TEST_GEN_PROGS_x86_64 += kvm_page_table_test
>  TEST_GEN_PROGS_x86_64 += memslot_modification_stress_test
>  TEST_GEN_PROGS_x86_64 += set_memory_region_test
>  TEST_GEN_PROGS_x86_64 += steal_time
> @@ -79,6 +80,7 @@ TEST_GEN_PROGS_aarch64 += demand_paging_test
>  TEST_GEN_PROGS_aarch64 += dirty_log_test
>  TEST_GEN_PROGS_aarch64 += dirty_log_perf_test
>  TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
> +TEST_GEN_PROGS_aarch64 += kvm_page_table_test
>  TEST_GEN_PROGS_aarch64 += set_memory_region_test
>  TEST_GEN_PROGS_aarch64 += steal_time
>  
> @@ -88,6 +90,7 @@ TEST_GEN_PROGS_s390x += s390x/sync_regs_test
>  TEST_GEN_PROGS_s390x += demand_paging_test
>  TEST_GEN_PROGS_s390x += dirty_log_test
>  TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
> +TEST_GEN_PROGS_s390x += kvm_page_table_test
>  TEST_GEN_PROGS_s390x += set_memory_region_test
>  
>  TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
> diff --git a/tools/testing/selftests/kvm/kvm_page_table_test.c b/tools/testing/selftests/kvm/kvm_page_table_test.c
> new file mode 100644
> index 000000000000..bbd5c489d61f
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/kvm_page_table_test.c
> @@ -0,0 +1,512 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KVM page table test
> + *
> + * Copyright (C) 2021, Huawei, Inc.
> + *
> + * Make sure that THP has been enabled or enough HUGETLB pages with specific
> + * page size have been pre-allocated on your system, if you are planning to
> + * use hugepages to back the guest memory for testing.
> + */
> +
> +#define _GNU_SOURCE /* for program_invocation_name */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <time.h>
> +#include <pthread.h>
> +#include <semaphore.h>
> +
> +#include "test_util.h"
> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "guest_modes.h"
> +
> +#define TEST_MEM_SLOT_INDEX             1
> +
> +/* Default size(1GB) of the memory for testing */
> +#define DEFAULT_TEST_MEM_SIZE		(1 << 30)
> +
> +/* Default guest test virtual memory offset */
> +#define DEFAULT_GUEST_TEST_MEM		0xc0000000
> +
> +/*
> + * In our test, we use thread synchronization functions (e.g. sem_wait)
> + * for time measurement and they can't fail at all, since a failure will
> + * impact the time accuracy and vcpus will not run as what we expect.
> + * So we will use safer versions of the functions.
> + */
> +#define sem_init_s(sem_ptr, ps, val) \
> +	TEST_ASSERT(sem_init(sem_ptr, ps, val) == 0, "Error in sem_init")
> +#define sem_destroy_s(sem_ptr) \
> +	TEST_ASSERT(sem_destroy(sem_ptr) == 0, "Error in sem_destroy")
> +#define sem_wait_s(sem_ptr) \
> +	TEST_ASSERT(sem_wait(sem_ptr) == 0, "Error in sem_wait")
> +#define sem_post_s(sem_ptr) \
> +	TEST_ASSERT(sem_post(sem_ptr) == 0, "Error in sem_post")

I'd rather not do this. I'd prefer to see

 ret = sem_*(...);
 TEST_ASSERT(ret == 0, ...);

at the callsites.

> +
> +/* Different guest memory accessing stages */
> +enum test_stage {
> +	KVM_BEFORE_MAPPINGS,
> +	KVM_CREATE_MAPPINGS,
> +	KVM_UPDATE_MAPPINGS,
> +	KVM_ADJUST_MAPPINGS,
> +	NUM_TEST_STAGES,
> +};
> +
> +static const char * const test_stage_string[] = {
> +	"KVM_BEFORE_MAPPINGS",
> +	"KVM_CREATE_MAPPINGS",
> +	"KVM_UPDATE_MAPPINGS",
> +	"KVM_ADJUST_MAPPINGS",
> +};
> +
> +struct perf_test_vcpu_args {
> +	int vcpu_id;
> +	bool vcpu_write;
> +};
> +
> +struct perf_test_args {
> +	struct kvm_vm *vm;
> +	uint64_t guest_test_virt_mem;
> +	uint64_t host_page_size;
> +	uint64_t host_num_pages;
> +	uint64_t large_page_size;
> +	uint64_t large_num_pages;
> +	uint64_t host_pages_per_lpage;
> +	enum vm_mem_backing_src_type src_type;
> +	struct perf_test_vcpu_args vcpu_args[KVM_MAX_VCPUS];
> +};

The above two structure names already have declarations in
include/perf_test_util.h. Using those names here is a bit confusing. I
suggest new names or extending the ones in perf_test_util.h, if the
extensions make sense for other perf tests. If extending the structures
makes sense in general, but these specific extensions don't, then you
can consider adding 'void *data' pointers allowing them to be extended
arbitrarily.

> +
> +/*
> + * Guest variables. Use addr_gva2hva() if these variables need
> + * to be changed in host.
> + */
> +static enum test_stage guest_test_stage;
> +
> +/* Host variables */
> +static uint32_t nr_vcpus = 1;
> +static struct perf_test_args perf_test_args;
> +static enum test_stage *current_stage;
> +static bool host_quit;
> +
> +/* Whether the test stage is updated, or completed */
> +static sem_t test_stage_updated;
> +static sem_t test_stage_completed;
> +
> +/*
> + * 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 void guest_code(int vcpu_id)
> +{
> +	struct perf_test_args *p = &perf_test_args;
> +	struct perf_test_vcpu_args *vcpu_args = &p->vcpu_args[vcpu_id];
> +	enum vm_mem_backing_src_type src_type = p->src_type;
> +	uint64_t host_page_size = p->host_page_size;
> +	uint64_t host_num_pages = p->host_num_pages;
> +	uint64_t large_page_size = p->large_page_size;
> +	uint64_t large_num_pages = p->large_num_pages;
> +	uint64_t host_pages_per_lpage = p->host_pages_per_lpage;

My suggestion to create the 'p' alias was to avoid creating all
these local variables. E.g. instead of creating host_page_size,
just use p->host_page_size wherever it's needed.

> +	uint64_t half = host_pages_per_lpage / 2;
> +	bool vcpu_write;
> +	enum test_stage stage;
> +	uint64_t addr;
> +	int i, j;
> +
> +	/* Make sure vCPU args data structure is not corrupt */
> +	GUEST_ASSERT(vcpu_args->vcpu_id == vcpu_id);

I'm OK with this sanity check, but I don't see how the args could be
corrupt. Maybe they could be poorly initialized or there could be a
missing sync_global_to_guest() though.

> +	vcpu_write = vcpu_args->vcpu_write;

Another unnecessary local variable.

> +
> +	while (true) {
> +		stage = READ_ONCE(guest_test_stage);

Another unnecessary local variable. I'd just put the READ_ONCE(...)
in the switch(). Also, before this loop I'd do 

 current_stage = &guest_test_stage;

allowing the switch to use READ_ONCE(*current_stage), which makes
it easier to understand how it relates to the host code.

> +		addr = perf_test_args.guest_test_virt_mem;
> +
> +		switch (stage) {
> +		/*
> +		 * Before dirty logging, vCPUs concurrently access the first
> +		 * 8 bytes of each page (host page/large page) within the same
> +		 * memory region with different accessing types (read/write).
> +		 * Then KVM will create normal page mappings or huge block
> +		 * mappings for them.
> +		 */
> +		case KVM_CREATE_MAPPINGS:
> +			for (i = 0; i < large_num_pages; i++) {
> +				if (vcpu_write)
> +					*(uint64_t *)addr = 0x0123456789ABCDEF;
> +				else
> +					READ_ONCE(*(uint64_t *)addr);
> +
> +				addr += large_page_size;
> +			}
> +			break;
> +
> +		/*
> +		 * During dirty logging, KVM will only update attributes of the
> +		 * normal page mappings from RO to RW if memory backing src type
> +		 * is anonymous. In other cases, KVM will split the huge block
> +		 * mappings into normal page mappings if memory backing src type
> +		 * is THP or HUGETLB.
> +		 */
> +		case KVM_UPDATE_MAPPINGS:
> +			if (src_type == VM_MEM_SRC_ANONYMOUS) {
> +				for (i = 0; i < host_num_pages; i++) {
> +					*(uint64_t *)addr = 0x0123456789ABCDEF;
> +					addr += host_page_size;
> +				}
> +				break;
> +			}
> +
> +			for (i = 0; i < large_num_pages; i++) {
> +				/*
> +				 * Write to the first host page in each large
> +				 * page region, and triger break of large pages.
> +				 */
> +				*(uint64_t *)addr = 0x0123456789ABCDEF;
> +
> +				/*
> +				 * Access the middle host pages in each large
> +				 * page region. Since dirty logging is enabled,
> +				 * this will create new mappings at the smallest
> +				 * granularity.
> +				 */
> +				addr += host_page_size * half;
> +				for (j = half; j < host_pages_per_lpage; j++) {
> +					READ_ONCE(*(uint64_t *)addr);
> +					addr += host_page_size;
> +				}
> +			}
> +			break;
> +
> +		/*
> +		 * After dirty logging is stopped, vCPUs concurrently read
> +		 * from every single host page. Then KVM will coalesce the
> +		 * split page mappings back to block mappings. And a TLB
> +		 * conflict abort could occur here if TLB entries of the
> +		 * page mappings are not fully invalidated.
> +		 */
> +		case KVM_ADJUST_MAPPINGS:
> +			for (i = 0; i < host_num_pages; i++) {
> +				READ_ONCE(*(uint64_t *)addr);
> +				addr += host_page_size;
> +			}
> +			break;
> +
> +		default:

How about this do nothing break be applied only to KVM_BEFORE_MAPPINGS
and the default case be a GUEST_ASSERT? Or does there also need to be
a QUIT?

> +			break;
> +		}
> +
> +		GUEST_SYNC(1);
> +	}
> +}
> +
> +static void *vcpu_worker(void *data)
> +{
> +	int ret;
> +	struct perf_test_vcpu_args *vcpu_args = data;
> +	struct kvm_vm *vm = perf_test_args.vm;
> +	int vcpu_id = vcpu_args->vcpu_id;
> +	struct kvm_run *run;
> +	struct timespec start;
> +	struct timespec ts_diff;
> +	enum test_stage stage;
> +
> +	vcpu_args_set(vm, vcpu_id, 1, vcpu_id);
> +	run = vcpu_state(vm, vcpu_id);
> +
> +	while (!READ_ONCE(host_quit)) {
> +		clock_gettime(CLOCK_MONOTONIC_RAW, &start);
> +		ret = _vcpu_run(vm, vcpu_id);
> +		ts_diff = timespec_elapsed(start);
> +
> +		TEST_ASSERT(ret == 0, "vcpu_run failed: %d\n", ret);
> +		TEST_ASSERT(get_ucall(vm, vcpu_id, NULL) == UCALL_SYNC,
> +			    "Invalid guest sync status: exit_reason=%s\n",
> +			    exit_reason_str(run->exit_reason));
> +
> +		pr_debug("Got sync event from vCPU %d\n", vcpu_id);
> +		stage = READ_ONCE(*current_stage);
> +
> +		/*
> +		 * Here we can know the execution time of every
> +		 * single vcpu running in different test stages.
> +		 */
> +		pr_debug("vCPU %d has completed stage %s\n"
> +			 "execution time is: %ld.%.9lds\n\n",
> +			 vcpu_id, test_stage_string[stage],
> +			 ts_diff.tv_sec, ts_diff.tv_nsec);
> +
> +		sem_post_s(&test_stage_completed);
> +		sem_wait_s(&test_stage_updated);

Shouldn't this wait be at the top of the loop?

> +	}
> +
> +	return NULL;
> +}
> +
> +struct test_params {
> +	uint64_t phys_offset;
> +	uint64_t test_mem_size;
> +	enum vm_mem_backing_src_type src_type;
> +};
> +
> +static struct kvm_vm *pre_init_before_test(enum vm_guest_mode mode, void *arg)
> +{
> +	struct test_params *p = arg;
> +	struct perf_test_vcpu_args *vcpu_args;
> +	enum vm_mem_backing_src_type src_type = p->src_type;
> +	uint64_t large_page_size = get_backing_src_pagesz(src_type);
> +	uint64_t test_mem_size = p->test_mem_size, guest_num_pages;
> +	uint64_t guest_page_size = vm_guest_mode_params[mode].page_size;
> +	uint64_t host_page_size = getpagesize();
> +	uint64_t alignment;
> +	void *host_test_mem;
> +	struct kvm_vm *vm;
> +	int vcpu_id;
> +
> +	/* Align up the test memory size */
> +	alignment = max(large_page_size, guest_page_size);
> +	test_mem_size = (test_mem_size + alignment - 1) & ~(alignment - 1);

We have align() in lib/kvm_util.c. I see it's static though. We should
probably expose that by making it a static inline in test_util.h

> +
> +	/* Create a VM with enough guest pages */
> +	guest_num_pages = test_mem_size / guest_page_size;
> +	vm = vm_create_with_vcpus(mode, nr_vcpus,
> +				  guest_num_pages, 0, guest_code, NULL);
> +
> +	/* Align down GPA of the testing memslot */
> +	if (!p->phys_offset)
> +		guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) *
> +				       guest_page_size;
> +	else
> +		guest_test_phys_mem = p->phys_offset;
> +#ifdef __s390x__
> +	alignment = max(0x100000, alignment);
> +#endif
> +	guest_test_phys_mem &= ~(alignment - 1);
> +
> +	/* Set up the shared data structure perf_test_args */
> +	perf_test_args.vm = vm;
> +	perf_test_args.guest_test_virt_mem = guest_test_virt_mem;
> +	perf_test_args.host_page_size = host_page_size;
> +	perf_test_args.host_num_pages = test_mem_size / host_page_size;
> +	perf_test_args.large_page_size = large_page_size;
> +	perf_test_args.large_num_pages = test_mem_size / large_page_size;
> +	perf_test_args.host_pages_per_lpage = large_page_size / host_page_size;
> +	perf_test_args.src_type = src_type;
> +
> +	for (vcpu_id = 0; vcpu_id < KVM_MAX_VCPUS; vcpu_id++) {
> +		vcpu_args = &perf_test_args.vcpu_args[vcpu_id];
> +		vcpu_args->vcpu_id = vcpu_id;
> +		vcpu_args->vcpu_write = !(vcpu_id % 2);
> +	}
> +
> +	/* Add an extra memory slot with specified backing src type */
> +	vm_userspace_mem_region_add(vm, src_type, guest_test_phys_mem,
> +				    TEST_MEM_SLOT_INDEX, guest_num_pages, 0);
> +
> +	/* Do mapping(GVA->GPA) for the testing memory slot */
> +	virt_map(vm, guest_test_virt_mem, guest_test_phys_mem, guest_num_pages, 0);
> +
> +	/* Cache the HVA pointer of the region */
> +	host_test_mem = addr_gpa2hva(vm, (vm_paddr_t)guest_test_phys_mem);
> +
> +	/* Export shared structure perf_test_args to guest */
> +	ucall_init(vm, NULL);
> +	sync_global_to_guest(vm, perf_test_args);
> +
> +	sem_init_s(&test_stage_updated, 0, 0);
> +	sem_init_s(&test_stage_completed, 0, 0);
> +
> +	current_stage = addr_gva2hva(vm, (vm_vaddr_t)(&guest_test_stage));
> +	*current_stage = NUM_TEST_STAGES;
> +
> +	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
> +	pr_info("Testing memory backing src type: %s\n",
> +		vm_mem_backing_src_alias(src_type)->name);
> +	pr_info("Testing memory backing src granularity: 0x%lx\n",
> +		large_page_size);
> +	pr_info("Testing memory size(aligned): 0x%lx\n", test_mem_size);
> +	pr_info("Guest physical test memory offset: 0x%lx\n",
> +		guest_test_phys_mem);
> +	pr_info("Host  virtual  test memory offset: 0x%lx\n",
> +		(uint64_t)host_test_mem);
> +	pr_info("Number of testing vCPUs: %d\n", nr_vcpus);
> +
> +	return vm;
> +}
> +
> +/* Wake up all the vcpus to run new test stage */
> +static void vcpus_start_new_stage(void)
> +{
> +	int vcpus;
> +
> +	for (vcpus = 1; vcpus <= nr_vcpus; vcpus++)

nit: vcpus = 0; vcpus < nr_vcpus; is more traditional.

> +		sem_post_s(&test_stage_updated);
> +
> +	pr_debug("All vcpus have been notified to continue\n");
> +}
> +
> +/* Wait for all the vcpus to complete new test stage */
> +static void vcpus_complete_new_stage(enum test_stage stage)
> +{
> +	int vcpus;
> +
> +	for (vcpus = 1; vcpus <= nr_vcpus; vcpus++) {
> +		sem_wait_s(&test_stage_completed);
> +		pr_debug("%d vcpus have completed stage %s\n",
> +			 vcpus, test_stage_string[stage]);
> +	}
> +
> +	pr_debug("All vcpus have completed stage %s\n",
> +		 test_stage_string[stage]);
> +}
> +
> +static void run_test(enum vm_guest_mode mode, void *arg)
> +{
> +	pthread_t *vcpu_threads;
> +	struct kvm_vm *vm;
> +	int vcpu_id;
> +	struct timespec start;
> +	struct timespec ts_diff;
> +
> +	/* Create VM with vCPUs and make some pre-initialization */
> +	vm = pre_init_before_test(mode, arg);
> +
> +	vcpu_threads = malloc(nr_vcpus * sizeof(*vcpu_threads));
> +	TEST_ASSERT(vcpu_threads, "Memory allocation failed");
> +
> +	host_quit = false;
> +	*current_stage = KVM_BEFORE_MAPPINGS;
> +
> +	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++) {
> +		pthread_create(&vcpu_threads[vcpu_id], NULL, vcpu_worker,
> +			       &perf_test_args.vcpu_args[vcpu_id]);
> +	}
> +
> +	pr_info("Started all vCPUs successfully\n");
> +
> +	vcpus_complete_new_stage(*current_stage);

With the sem_wait in vcpu_working moved to the top of the loop, I'd
write the last two lines as

 vcpus_start_new_stage();
 vcpus_complete_new_stage(*current_stage);
 pr_info("Started all vCPUs successfully\n");

> +
> +	/* Test the stage of KVM creating mappings */
> +	*current_stage = KVM_CREATE_MAPPINGS;
> +
> +	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
> +	vcpus_start_new_stage();
> +	vcpus_complete_new_stage(*current_stage);
> +	ts_diff = timespec_elapsed(start);
> +
> +	pr_info("KVM_CREATE_MAPPINGS: total execution time: %ld.%.9lds\n\n",
> +		ts_diff.tv_sec, ts_diff.tv_nsec);
> +
> +	/* Test the stage of KVM updating mappings */
> +	vm_mem_region_set_flags(vm, TEST_MEM_SLOT_INDEX,
> +				KVM_MEM_LOG_DIRTY_PAGES);
> +
> +	*current_stage = KVM_UPDATE_MAPPINGS;
> +
> +	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
> +	vcpus_start_new_stage();
> +	vcpus_complete_new_stage(*current_stage);
> +	ts_diff = timespec_elapsed(start);
> +
> +	pr_info("KVM_UPDATE_MAPPINGS: total execution time: %ld.%.9lds\n\n",
> +		ts_diff.tv_sec, ts_diff.tv_nsec);
> +
> +	/* Test the stage of KVM adjusting mappings */
> +	vm_mem_region_set_flags(vm, TEST_MEM_SLOT_INDEX, 0);
> +
> +	*current_stage = KVM_ADJUST_MAPPINGS;
> +
> +	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
> +	vcpus_start_new_stage();
> +	vcpus_complete_new_stage(*current_stage);
> +	ts_diff = timespec_elapsed(start);
> +
> +	pr_info("KVM_ADJUST_MAPPINGS: total execution time: %ld.%.9lds\n\n",
> +		ts_diff.tv_sec, ts_diff.tv_nsec);
> +
> +	/* Tell the vcpu thread to quit */
> +	host_quit = true;
> +	vcpus_start_new_stage();

Looks like besides this one the vcpus_start_new_stage and
vcpus_complete_new_stage calls always come together. Maybe
they could be merged into one function and this one could be handled
differently.

> +
> +	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++)
> +		pthread_join(vcpu_threads[vcpu_id], NULL);
> +
> +	sem_destroy_s(&test_stage_updated);
> +	sem_destroy_s(&test_stage_completed);
> +
> +	free(vcpu_threads);
> +	ucall_uninit(vm);
> +	kvm_vm_free(vm);
> +}
> +
> +static void help(char *name)
> +{
> +	puts("");
> +	printf("usage: %s [-h] [-p offset] [-m mode] "
> +	       "[-b mem-size] [-v vcpus] [-s mem-type]\n", name);
> +	puts("");
> +	printf(" -p: specify guest physical test memory offset\n"
> +	       "     Warning: a low offset can conflict with the loaded test code.\n");
> +	guest_modes_help();
> +	printf(" -b: specify size of the memory region for testing. e.g. 10M or 3G.\n"
> +	       "     (default: 1G)\n");
> +	printf(" -v: specify the number of vCPUs to run\n"
> +	       "     (default: 1)\n");
> +	printf(" -s: specify the type of memory that should be used to\n"
> +	       "     back the guest data region.\n"
> +	       "     (default: anonymous)\n\n");
                                           ^ is this extra \n needed?
> +	backing_src_help();
> +	puts("");
> +	exit(0);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
> +	struct test_params p = {
> +		.test_mem_size = DEFAULT_TEST_MEM_SIZE,
> +		.src_type = VM_MEM_SRC_ANONYMOUS,
> +	};
> +	int opt;
> +
> +	guest_modes_append_default();
> +
> +	while ((opt = getopt(argc, argv, "hp:m:b:v:s:")) != -1) {
> +		switch (opt) {
> +		case 'p':
> +			p.phys_offset = strtoull(optarg, NULL, 0);
> +			break;
> +		case 'm':
> +			guest_modes_cmdline(optarg);
> +			break;
> +		case 'b':
> +			p.test_mem_size = parse_size(optarg);
> +			break;
> +		case 'v':
> +			nr_vcpus = atoi(optarg);
> +			TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
> +				    "Invalid number of vcpus, must be between 1 and %d", max_vcpus);
> +			break;
> +		case 's':
> +			p.src_type = parse_backing_src_type(optarg);
> +			break;
> +		case 'h':
> +		default:
> +			help(argv[0]);
> +			break;

nit: I'd replace this break with an exit() and not exit from help().

> +		}
> +	}
> +
> +	for_each_guest_mode(run_test, &p);
> +
> +	return 0;
> +}
> -- 
> 2.19.1
>

My comments are mainly just a bunch of nits, so

Reviewed-by: Andrew Jones <drjones@redhat.com>

Thanks,
drew 

Re: [RFC PATCH v5 10/10] KVM: selftests: Add a test for kvm page table code

[wangyanan (Y)]

Hi Drew,
Thanks for having a look.
On 2021/3/29 19:38, Andrew Jones wrote:
> On Tue, Mar 23, 2021 at 09:52:31PM +0800, Yanan Wang wrote:
>> This test serves as a performance tester and a bug reproducer for
>> kvm page table code (GPA->HPA mappings), so it gives guidance for
>> people trying to make some improvement for kvm.
>>
>> The function guest_code() can cover the conditions where a single vcpu or
>> multiple vcpus access guest pages within the same memory region, in three
>> VM stages(before dirty logging, during dirty logging, after dirty logging).
>> Besides, the backing src memory type(ANONYMOUS/THP/HUGETLB) of the tested
>> memory region can be specified by users, which means normal page mappings
>> or block mappings can be chosen by users to be created in the test.
>>
>> If ANONYMOUS memory is specified, kvm will create normal page mappings
>> for the tested memory region before dirty logging, and update attributes
>> of the page mappings from RO to RW during dirty logging. If THP/HUGETLB
>> memory is specified, kvm will create block mappings for the tested memory
>> region before dirty logging, and split the blcok mappings into normal page
>> mappings during dirty logging, and coalesce the page mappings back into
>> block mappings after dirty logging is stopped.
>>
>> So in summary, as a performance tester, this test can present the
>> performance of kvm creating/updating normal page mappings, or the
>> performance of kvm creating/splitting/recovering block mappings,
>> through execution time.
>>
>> When we need to coalesce the page mappings back to block mappings after
>> dirty logging is stopped, we have to firstly invalidate *all* the TLB
>> entries for the page mappings right before installation of the block entry,
>> because a TLB conflict abort error could occur if we can't invalidate the
>> TLB entries fully. We have hit this TLB conflict twice on aarch64 software
>> implementation and fixed it. As this test can imulate process from dirty
>> logging enabled to dirty logging stopped of a VM with block mappings,
>> so it can also reproduce this TLB conflict abort due to inadequate TLB
>> invalidation when coalescing tables.
>>
>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> Reviewed-by: Ben Gardon <bgardon@google.com>
>> ---
>>   tools/testing/selftests/kvm/.gitignore        |   1 +
>>   tools/testing/selftests/kvm/Makefile          |   3 +
>>   .../selftests/kvm/kvm_page_table_test.c       | 512 ++++++++++++++++++
>>   3 files changed, 516 insertions(+)
>>   create mode 100644 tools/testing/selftests/kvm/kvm_page_table_test.c
>>
>> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
>> index 32b87cc77c8e..137ab7273be6 100644
>> --- a/tools/testing/selftests/kvm/.gitignore
>> +++ b/tools/testing/selftests/kvm/.gitignore
>> @@ -35,6 +35,7 @@
>>   /dirty_log_perf_test
>>   /hardware_disable_test
>>   /kvm_create_max_vcpus
>> +/kvm_page_table_test
>>   /memslot_modification_stress_test
>>   /set_memory_region_test
>>   /steal_time
>> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
>> index a6d61f451f88..75dc57db36b4 100644
>> --- a/tools/testing/selftests/kvm/Makefile
>> +++ b/tools/testing/selftests/kvm/Makefile
>> @@ -69,6 +69,7 @@ TEST_GEN_PROGS_x86_64 += dirty_log_test
>>   TEST_GEN_PROGS_x86_64 += dirty_log_perf_test
>>   TEST_GEN_PROGS_x86_64 += hardware_disable_test
>>   TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
>> +TEST_GEN_PROGS_x86_64 += kvm_page_table_test
>>   TEST_GEN_PROGS_x86_64 += memslot_modification_stress_test
>>   TEST_GEN_PROGS_x86_64 += set_memory_region_test
>>   TEST_GEN_PROGS_x86_64 += steal_time
>> @@ -79,6 +80,7 @@ TEST_GEN_PROGS_aarch64 += demand_paging_test
>>   TEST_GEN_PROGS_aarch64 += dirty_log_test
>>   TEST_GEN_PROGS_aarch64 += dirty_log_perf_test
>>   TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
>> +TEST_GEN_PROGS_aarch64 += kvm_page_table_test
>>   TEST_GEN_PROGS_aarch64 += set_memory_region_test
>>   TEST_GEN_PROGS_aarch64 += steal_time
>>   
>> @@ -88,6 +90,7 @@ TEST_GEN_PROGS_s390x += s390x/sync_regs_test
>>   TEST_GEN_PROGS_s390x += demand_paging_test
>>   TEST_GEN_PROGS_s390x += dirty_log_test
>>   TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
>> +TEST_GEN_PROGS_s390x += kvm_page_table_test
>>   TEST_GEN_PROGS_s390x += set_memory_region_test
>>   
>>   TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
>> diff --git a/tools/testing/selftests/kvm/kvm_page_table_test.c b/tools/testing/selftests/kvm/kvm_page_table_test.c
>> new file mode 100644
>> index 000000000000..bbd5c489d61f
>> --- /dev/null
>> +++ b/tools/testing/selftests/kvm/kvm_page_table_test.c
>> @@ -0,0 +1,512 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * KVM page table test
>> + *
>> + * Copyright (C) 2021, Huawei, Inc.
>> + *
>> + * Make sure that THP has been enabled or enough HUGETLB pages with specific
>> + * page size have been pre-allocated on your system, if you are planning to
>> + * use hugepages to back the guest memory for testing.
>> + */
>> +
>> +#define _GNU_SOURCE /* for program_invocation_name */
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <time.h>
>> +#include <pthread.h>
>> +#include <semaphore.h>
>> +
>> +#include "test_util.h"
>> +#include "kvm_util.h"
>> +#include "processor.h"
>> +#include "guest_modes.h"
>> +
>> +#define TEST_MEM_SLOT_INDEX             1
>> +
>> +/* Default size(1GB) of the memory for testing */
>> +#define DEFAULT_TEST_MEM_SIZE		(1 << 30)
>> +
>> +/* Default guest test virtual memory offset */
>> +#define DEFAULT_GUEST_TEST_MEM		0xc0000000
>> +
>> +/*
>> + * In our test, we use thread synchronization functions (e.g. sem_wait)
>> + * for time measurement and they can't fail at all, since a failure will
>> + * impact the time accuracy and vcpus will not run as what we expect.
>> + * So we will use safer versions of the functions.
>> + */
>> +#define sem_init_s(sem_ptr, ps, val) \
>> +	TEST_ASSERT(sem_init(sem_ptr, ps, val) == 0, "Error in sem_init")
>> +#define sem_destroy_s(sem_ptr) \
>> +	TEST_ASSERT(sem_destroy(sem_ptr) == 0, "Error in sem_destroy")
>> +#define sem_wait_s(sem_ptr) \
>> +	TEST_ASSERT(sem_wait(sem_ptr) == 0, "Error in sem_wait")
>> +#define sem_post_s(sem_ptr) \
>> +	TEST_ASSERT(sem_post(sem_ptr) == 0, "Error in sem_post")
> I'd rather not do this. I'd prefer to see
>
>   ret = sem_*(...);
>   TEST_ASSERT(ret == 0, ...);
>
> at the callsites.
Ok, I will make some adjustment.
>> +
>> +/* Different guest memory accessing stages */
>> +enum test_stage {
>> +	KVM_BEFORE_MAPPINGS,
>> +	KVM_CREATE_MAPPINGS,
>> +	KVM_UPDATE_MAPPINGS,
>> +	KVM_ADJUST_MAPPINGS,
>> +	NUM_TEST_STAGES,
>> +};
>> +
>> +static const char * const test_stage_string[] = {
>> +	"KVM_BEFORE_MAPPINGS",
>> +	"KVM_CREATE_MAPPINGS",
>> +	"KVM_UPDATE_MAPPINGS",
>> +	"KVM_ADJUST_MAPPINGS",
>> +};
>> +
>> +struct perf_test_vcpu_args {
>> +	int vcpu_id;
>> +	bool vcpu_write;
>> +};
>> +
>> +struct perf_test_args {
>> +	struct kvm_vm *vm;
>> +	uint64_t guest_test_virt_mem;
>> +	uint64_t host_page_size;
>> +	uint64_t host_num_pages;
>> +	uint64_t large_page_size;
>> +	uint64_t large_num_pages;
>> +	uint64_t host_pages_per_lpage;
>> +	enum vm_mem_backing_src_type src_type;
>> +	struct perf_test_vcpu_args vcpu_args[KVM_MAX_VCPUS];
>> +};
> The above two structure names already have declarations in
> include/perf_test_util.h. Using those names here is a bit confusing. I
> suggest new names or extending the ones in perf_test_util.h, if the
> extensions make sense for other perf tests. If extending the structures
> makes sense in general, but these specific extensions don't, then you
> can consider adding 'void *data' pointers allowing them to be extended
> arbitrarily.
I think I prefer using other new names for these two structures in this 
test,
because most of the structure members are specific for this test and are
quite different from declarations in include/perf_test_util.h so that the
extensions don't really make sense for other perf tests.
>> +
>> +/*
>> + * Guest variables. Use addr_gva2hva() if these variables need
>> + * to be changed in host.
>> + */
>> +static enum test_stage guest_test_stage;
>> +
>> +/* Host variables */
>> +static uint32_t nr_vcpus = 1;
>> +static struct perf_test_args perf_test_args;
>> +static enum test_stage *current_stage;
>> +static bool host_quit;
>> +
>> +/* Whether the test stage is updated, or completed */
>> +static sem_t test_stage_updated;
>> +static sem_t test_stage_completed;
>> +
>> +/*
>> + * 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 void guest_code(int vcpu_id)
>> +{
>> +	struct perf_test_args *p = &perf_test_args;
>> +	struct perf_test_vcpu_args *vcpu_args = &p->vcpu_args[vcpu_id];
>> +	enum vm_mem_backing_src_type src_type = p->src_type;
>> +	uint64_t host_page_size = p->host_page_size;
>> +	uint64_t host_num_pages = p->host_num_pages;
>> +	uint64_t large_page_size = p->large_page_size;
>> +	uint64_t large_num_pages = p->large_num_pages;
>> +	uint64_t host_pages_per_lpage = p->host_pages_per_lpage;
> My suggestion to create the 'p' alias was to avoid creating all
> these local variables. E.g. instead of creating host_page_size,
> just use p->host_page_size wherever it's needed.
>
>> +	uint64_t half = host_pages_per_lpage / 2;
>> +	bool vcpu_write;
>> +	enum test_stage stage;
>> +	uint64_t addr;
>> +	int i, j;
>> +
>> +	/* Make sure vCPU args data structure is not corrupt */
>> +	GUEST_ASSERT(vcpu_args->vcpu_id == vcpu_id);
> I'm OK with this sanity check, but I don't see how the args could be
> corrupt. Maybe they could be poorly initialized or there could be a
> missing sync_global_to_guest() though.
>
>> +	vcpu_write = vcpu_args->vcpu_write;
> Another unnecessary local variable.
>
>> +
>> +	while (true) {
>> +		stage = READ_ONCE(guest_test_stage);
> Another unnecessary local variable. I'd just put the READ_ONCE(...)
> in the switch(). Also, before this loop I'd do
>
>   current_stage = &guest_test_stage;
>
> allowing the switch to use READ_ONCE(*current_stage), which makes
> it easier to understand how it relates to the host code.
Thanks for above suggestions for guest_code(). I agree and will fix them.
>> +		addr = perf_test_args.guest_test_virt_mem;
>> +
>> +		switch (stage) {
>> +		/*
>> +		 * Before dirty logging, vCPUs concurrently access the first
>> +		 * 8 bytes of each page (host page/large page) within the same
>> +		 * memory region with different accessing types (read/write).
>> +		 * Then KVM will create normal page mappings or huge block
>> +		 * mappings for them.
>> +		 */
>> +		case KVM_CREATE_MAPPINGS:
>> +			for (i = 0; i < large_num_pages; i++) {
>> +				if (vcpu_write)
>> +					*(uint64_t *)addr = 0x0123456789ABCDEF;
>> +				else
>> +					READ_ONCE(*(uint64_t *)addr);
>> +
>> +				addr += large_page_size;
>> +			}
>> +			break;
>> +
>> +		/*
>> +		 * During dirty logging, KVM will only update attributes of the
>> +		 * normal page mappings from RO to RW if memory backing src type
>> +		 * is anonymous. In other cases, KVM will split the huge block
>> +		 * mappings into normal page mappings if memory backing src type
>> +		 * is THP or HUGETLB.
>> +		 */
>> +		case KVM_UPDATE_MAPPINGS:
>> +			if (src_type == VM_MEM_SRC_ANONYMOUS) {
>> +				for (i = 0; i < host_num_pages; i++) {
>> +					*(uint64_t *)addr = 0x0123456789ABCDEF;
>> +					addr += host_page_size;
>> +				}
>> +				break;
>> +			}
>> +
>> +			for (i = 0; i < large_num_pages; i++) {
>> +				/*
>> +				 * Write to the first host page in each large
>> +				 * page region, and triger break of large pages.
>> +				 */
>> +				*(uint64_t *)addr = 0x0123456789ABCDEF;
>> +
>> +				/*
>> +				 * Access the middle host pages in each large
>> +				 * page region. Since dirty logging is enabled,
>> +				 * this will create new mappings at the smallest
>> +				 * granularity.
>> +				 */
>> +				addr += host_page_size * half;
>> +				for (j = half; j < host_pages_per_lpage; j++) {
>> +					READ_ONCE(*(uint64_t *)addr);
>> +					addr += host_page_size;
>> +				}
>> +			}
>> +			break;
>> +
>> +		/*
>> +		 * After dirty logging is stopped, vCPUs concurrently read
>> +		 * from every single host page. Then KVM will coalesce the
>> +		 * split page mappings back to block mappings. And a TLB
>> +		 * conflict abort could occur here if TLB entries of the
>> +		 * page mappings are not fully invalidated.
>> +		 */
>> +		case KVM_ADJUST_MAPPINGS:
>> +			for (i = 0; i < host_num_pages; i++) {
>> +				READ_ONCE(*(uint64_t *)addr);
>> +				addr += host_page_size;
>> +			}
>> +			break;
>> +
>> +		default:
> How about this do nothing break be applied only to KVM_BEFORE_MAPPINGS
> and the default case be a GUEST_ASSERT?
Nice idea. It will also indicate that there totally are four stages
in accord to structure test_stage.
> Or does there also need to be
> a QUIT?
>
>> +			break;
>> +		}
>> +
>> +		GUEST_SYNC(1);
>> +	}
>> +}
>> +
>> +static void *vcpu_worker(void *data)
>> +{
>> +	int ret;
>> +	struct perf_test_vcpu_args *vcpu_args = data;
>> +	struct kvm_vm *vm = perf_test_args.vm;
>> +	int vcpu_id = vcpu_args->vcpu_id;
>> +	struct kvm_run *run;
>> +	struct timespec start;
>> +	struct timespec ts_diff;
>> +	enum test_stage stage;
>> +
>> +	vcpu_args_set(vm, vcpu_id, 1, vcpu_id);
>> +	run = vcpu_state(vm, vcpu_id);
>> +
>> +	while (!READ_ONCE(host_quit)) {
>> +		clock_gettime(CLOCK_MONOTONIC_RAW, &start);
>> +		ret = _vcpu_run(vm, vcpu_id);
>> +		ts_diff = timespec_elapsed(start);
>> +
>> +		TEST_ASSERT(ret == 0, "vcpu_run failed: %d\n", ret);
>> +		TEST_ASSERT(get_ucall(vm, vcpu_id, NULL) == UCALL_SYNC,
>> +			    "Invalid guest sync status: exit_reason=%s\n",
>> +			    exit_reason_str(run->exit_reason));
>> +
>> +		pr_debug("Got sync event from vCPU %d\n", vcpu_id);
>> +		stage = READ_ONCE(*current_stage);
>> +
>> +		/*
>> +		 * Here we can know the execution time of every
>> +		 * single vcpu running in different test stages.
>> +		 */
>> +		pr_debug("vCPU %d has completed stage %s\n"
>> +			 "execution time is: %ld.%.9lds\n\n",
>> +			 vcpu_id, test_stage_string[stage],
>> +			 ts_diff.tv_sec, ts_diff.tv_nsec);
>> +
>> +		sem_post_s(&test_stage_completed);
>> +		sem_wait_s(&test_stage_updated);
> Shouldn't this wait be at the top of the loop?
>
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>> +struct test_params {
>> +	uint64_t phys_offset;
>> +	uint64_t test_mem_size;
>> +	enum vm_mem_backing_src_type src_type;
>> +};
>> +
>> +static struct kvm_vm *pre_init_before_test(enum vm_guest_mode mode, void *arg)
>> +{
>> +	struct test_params *p = arg;
>> +	struct perf_test_vcpu_args *vcpu_args;
>> +	enum vm_mem_backing_src_type src_type = p->src_type;
>> +	uint64_t large_page_size = get_backing_src_pagesz(src_type);
>> +	uint64_t test_mem_size = p->test_mem_size, guest_num_pages;
>> +	uint64_t guest_page_size = vm_guest_mode_params[mode].page_size;
>> +	uint64_t host_page_size = getpagesize();
>> +	uint64_t alignment;
>> +	void *host_test_mem;
>> +	struct kvm_vm *vm;
>> +	int vcpu_id;
>> +
>> +	/* Align up the test memory size */
>> +	alignment = max(large_page_size, guest_page_size);
>> +	test_mem_size = (test_mem_size + alignment - 1) & ~(alignment - 1);
> We have align() in lib/kvm_util.c. I see it's static though. We should
> probably expose that by making it a static inline in test_util.h
Yes, I know the align() function. But exposure of the function and the
corresponding fix of the tests is already what Sean's previous patch did.
Maybe I should leave it for Sean to fix uniformly when his v2 will be
posted after this series is queued.
>> +
>> +	/* Create a VM with enough guest pages */
>> +	guest_num_pages = test_mem_size / guest_page_size;
>> +	vm = vm_create_with_vcpus(mode, nr_vcpus,
>> +				  guest_num_pages, 0, guest_code, NULL);
>> +
>> +	/* Align down GPA of the testing memslot */
>> +	if (!p->phys_offset)
>> +		guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) *
>> +				       guest_page_size;
>> +	else
>> +		guest_test_phys_mem = p->phys_offset;
>> +#ifdef __s390x__
>> +	alignment = max(0x100000, alignment);
>> +#endif
>> +	guest_test_phys_mem &= ~(alignment - 1);
>> +
>> +	/* Set up the shared data structure perf_test_args */
>> +	perf_test_args.vm = vm;
>> +	perf_test_args.guest_test_virt_mem = guest_test_virt_mem;
>> +	perf_test_args.host_page_size = host_page_size;
>> +	perf_test_args.host_num_pages = test_mem_size / host_page_size;
>> +	perf_test_args.large_page_size = large_page_size;
>> +	perf_test_args.large_num_pages = test_mem_size / large_page_size;
>> +	perf_test_args.host_pages_per_lpage = large_page_size / host_page_size;
>> +	perf_test_args.src_type = src_type;
>> +
>> +	for (vcpu_id = 0; vcpu_id < KVM_MAX_VCPUS; vcpu_id++) {
>> +		vcpu_args = &perf_test_args.vcpu_args[vcpu_id];
>> +		vcpu_args->vcpu_id = vcpu_id;
>> +		vcpu_args->vcpu_write = !(vcpu_id % 2);
>> +	}
>> +
>> +	/* Add an extra memory slot with specified backing src type */
>> +	vm_userspace_mem_region_add(vm, src_type, guest_test_phys_mem,
>> +				    TEST_MEM_SLOT_INDEX, guest_num_pages, 0);
>> +
>> +	/* Do mapping(GVA->GPA) for the testing memory slot */
>> +	virt_map(vm, guest_test_virt_mem, guest_test_phys_mem, guest_num_pages, 0);
>> +
>> +	/* Cache the HVA pointer of the region */
>> +	host_test_mem = addr_gpa2hva(vm, (vm_paddr_t)guest_test_phys_mem);
>> +
>> +	/* Export shared structure perf_test_args to guest */
>> +	ucall_init(vm, NULL);
>> +	sync_global_to_guest(vm, perf_test_args);
>> +
>> +	sem_init_s(&test_stage_updated, 0, 0);
>> +	sem_init_s(&test_stage_completed, 0, 0);
>> +
>> +	current_stage = addr_gva2hva(vm, (vm_vaddr_t)(&guest_test_stage));
>> +	*current_stage = NUM_TEST_STAGES;
>> +
>> +	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
>> +	pr_info("Testing memory backing src type: %s\n",
>> +		vm_mem_backing_src_alias(src_type)->name);
>> +	pr_info("Testing memory backing src granularity: 0x%lx\n",
>> +		large_page_size);
>> +	pr_info("Testing memory size(aligned): 0x%lx\n", test_mem_size);
>> +	pr_info("Guest physical test memory offset: 0x%lx\n",
>> +		guest_test_phys_mem);
>> +	pr_info("Host  virtual  test memory offset: 0x%lx\n",
>> +		(uint64_t)host_test_mem);
>> +	pr_info("Number of testing vCPUs: %d\n", nr_vcpus);
>> +
>> +	return vm;
>> +}
>> +
>> +/* Wake up all the vcpus to run new test stage */
>> +static void vcpus_start_new_stage(void)
>> +{
>> +	int vcpus;
>> +
>> +	for (vcpus = 1; vcpus <= nr_vcpus; vcpus++)
> nit: vcpus = 0; vcpus < nr_vcpus; is more traditional.
Ok, will fix.
>> +		sem_post_s(&test_stage_updated);
>> +
>> +	pr_debug("All vcpus have been notified to continue\n");
>> +}
>> +
>> +/* Wait for all the vcpus to complete new test stage */
>> +static void vcpus_complete_new_stage(enum test_stage stage)
>> +{
>> +	int vcpus;
>> +
>> +	for (vcpus = 1; vcpus <= nr_vcpus; vcpus++) {
>> +		sem_wait_s(&test_stage_completed);
>> +		pr_debug("%d vcpus have completed stage %s\n",
>> +			 vcpus, test_stage_string[stage]);
>> +	}
>> +
>> +	pr_debug("All vcpus have completed stage %s\n",
>> +		 test_stage_string[stage]);
>> +}
>> +
>> +static void run_test(enum vm_guest_mode mode, void *arg)
>> +{
>> +	pthread_t *vcpu_threads;
>> +	struct kvm_vm *vm;
>> +	int vcpu_id;
>> +	struct timespec start;
>> +	struct timespec ts_diff;
>> +
>> +	/* Create VM with vCPUs and make some pre-initialization */
>> +	vm = pre_init_before_test(mode, arg);
>> +
>> +	vcpu_threads = malloc(nr_vcpus * sizeof(*vcpu_threads));
>> +	TEST_ASSERT(vcpu_threads, "Memory allocation failed");
>> +
>> +	host_quit = false;
>> +	*current_stage = KVM_BEFORE_MAPPINGS;
>> +
>> +	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++) {
>> +		pthread_create(&vcpu_threads[vcpu_id], NULL, vcpu_worker,
>> +			       &perf_test_args.vcpu_args[vcpu_id]);
>> +	}
>> +
>> +	pr_info("Started all vCPUs successfully\n");
>> +
>> +	vcpus_complete_new_stage(*current_stage);
> With the sem_wait in vcpu_working moved to the top of the loop, I'd
> write the last two lines as
>
>   vcpus_start_new_stage();
>   vcpus_complete_new_stage(*current_stage);
>   pr_info("Started all vCPUs successfully\n");
Yes, it will look better.
>> +
>> +	/* Test the stage of KVM creating mappings */
>> +	*current_stage = KVM_CREATE_MAPPINGS;
>> +
>> +	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
>> +	vcpus_start_new_stage();
>> +	vcpus_complete_new_stage(*current_stage);
>> +	ts_diff = timespec_elapsed(start);
>> +
>> +	pr_info("KVM_CREATE_MAPPINGS: total execution time: %ld.%.9lds\n\n",
>> +		ts_diff.tv_sec, ts_diff.tv_nsec);
>> +
>> +	/* Test the stage of KVM updating mappings */
>> +	vm_mem_region_set_flags(vm, TEST_MEM_SLOT_INDEX,
>> +				KVM_MEM_LOG_DIRTY_PAGES);
>> +
>> +	*current_stage = KVM_UPDATE_MAPPINGS;
>> +
>> +	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
>> +	vcpus_start_new_stage();
>> +	vcpus_complete_new_stage(*current_stage);
>> +	ts_diff = timespec_elapsed(start);
>> +
>> +	pr_info("KVM_UPDATE_MAPPINGS: total execution time: %ld.%.9lds\n\n",
>> +		ts_diff.tv_sec, ts_diff.tv_nsec);
>> +
>> +	/* Test the stage of KVM adjusting mappings */
>> +	vm_mem_region_set_flags(vm, TEST_MEM_SLOT_INDEX, 0);
>> +
>> +	*current_stage = KVM_ADJUST_MAPPINGS;
>> +
>> +	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
>> +	vcpus_start_new_stage();
>> +	vcpus_complete_new_stage(*current_stage);
>> +	ts_diff = timespec_elapsed(start);
>> +
>> +	pr_info("KVM_ADJUST_MAPPINGS: total execution time: %ld.%.9lds\n\n",
>> +		ts_diff.tv_sec, ts_diff.tv_nsec);
>> +
>> +	/* Tell the vcpu thread to quit */
>> +	host_quit = true;
>> +	vcpus_start_new_stage();
> Looks like besides this one the vcpus_start_new_stage and
> vcpus_complete_new_stage calls always come together. Maybe
> they could be merged into one function and this one could be handled
> differently.
Makes sense.
>
>> +
>> +	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++)
>> +		pthread_join(vcpu_threads[vcpu_id], NULL);
>> +
>> +	sem_destroy_s(&test_stage_updated);
>> +	sem_destroy_s(&test_stage_completed);
>> +
>> +	free(vcpu_threads);
>> +	ucall_uninit(vm);
>> +	kvm_vm_free(vm);
>> +}
>> +
>> +static void help(char *name)
>> +{
>> +	puts("");
>> +	printf("usage: %s [-h] [-p offset] [-m mode] "
>> +	       "[-b mem-size] [-v vcpus] [-s mem-type]\n", name);
>> +	puts("");
>> +	printf(" -p: specify guest physical test memory offset\n"
>> +	       "     Warning: a low offset can conflict with the loaded test code.\n");
>> +	guest_modes_help();
>> +	printf(" -b: specify size of the memory region for testing. e.g. 10M or 3G.\n"
>> +	       "     (default: 1G)\n");
>> +	printf(" -v: specify the number of vCPUs to run\n"
>> +	       "     (default: 1)\n");
>> +	printf(" -s: specify the type of memory that should be used to\n"
>> +	       "     back the guest data region.\n"
>> +	       "     (default: anonymous)\n\n");
>                                             ^ is this extra \n needed?
The extra \n adds an empty line before followed list of different backing
source types, which possibly makes it easier to be read for users.
No strong feelings about either way actually.
>> +	backing_src_help();
>> +	puts("");
>> +	exit(0);
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +	int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
>> +	struct test_params p = {
>> +		.test_mem_size = DEFAULT_TEST_MEM_SIZE,
>> +		.src_type = VM_MEM_SRC_ANONYMOUS,
>> +	};
>> +	int opt;
>> +
>> +	guest_modes_append_default();
>> +
>> +	while ((opt = getopt(argc, argv, "hp:m:b:v:s:")) != -1) {
>> +		switch (opt) {
>> +		case 'p':
>> +			p.phys_offset = strtoull(optarg, NULL, 0);
>> +			break;
>> +		case 'm':
>> +			guest_modes_cmdline(optarg);
>> +			break;
>> +		case 'b':
>> +			p.test_mem_size = parse_size(optarg);
>> +			break;
>> +		case 'v':
>> +			nr_vcpus = atoi(optarg);
>> +			TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
>> +				    "Invalid number of vcpus, must be between 1 and %d", max_vcpus);
>> +			break;
>> +		case 's':
>> +			p.src_type = parse_backing_src_type(optarg);
>> +			break;
>> +		case 'h':
>> +		default:
>> +			help(argv[0]);
>> +			break;
> nit: I'd replace this break with an exit() and not exit from help().
Makes sense.
>> +		}
>> +	}
>> +
>> +	for_each_guest_mode(run_test, &p);
>> +
>> +	return 0;
>> +}
>> -- 
>> 2.19.1
>>
> My comments are mainly just a bunch of nits, so
>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
Thanks again,
Yanan
>
> Thanks,
> drew
>
> .