[PATCH v4 00/10] Create a userfaultfd demand paging test

[PATCH v4 00/10] Create a userfaultfd demand paging test

[Ben Gardon]

When handling page faults for many vCPUs during demand paging, KVM's MMU
lock becomes highly contended. This series creates a test with a naive
userfaultfd based demand paging implementation to demonstrate that
contention. This test serves both as a functional test of userfaultfd
and a microbenchmark of demand paging performance with a variable number
of vCPUs and memory per vCPU.

The test creates N userfaultfd threads, N vCPUs, and a region of memory
with M pages per vCPU. The N userfaultfd polling threads are each set up
to serve faults on a region of memory corresponding to one of the vCPUs.
Each of the vCPUs is then started, and touches each page of its disjoint
memory region, sequentially. In response to faults, the userfaultfd
threads copy a static buffer into the guest's memory. This creates a
worst case for MMU lock contention as we have removed most of the
contention between the userfaultfd threads and there is no time required
to fetch the contents of guest memory.

This test was run successfully on Intel Haswell, Broadwell, and
Cascadelake hosts with a variety of vCPU counts and memory sizes.

This test was adapted from the dirty_log_test.

The series can also be viewed in Gerrit here:
https://linux-review.googlesource.com/c/virt/kvm/kvm/+/1464
(Thanks to Dmitry Vyukov <dvyukov@google.com> for setting up the Gerrit
instance)

v4 (Responding to feedback from Andrew Jones, Peter Xu, and Peter Shier):
- Tested this revision by running
  demand_paging_test
  at each commit in the series on an Intel Haswell machine. Ran
  demand_paging_test -u -v 8 -b 8M -d 10
  on the same machine at the last commit in the series.
- Readded partial aarch64 support, though aarch64 and s390 remain
  untested
- Implemented pipefd polling to reduce UFFD thread exit latency
- Added variable unit input for memory size so users can pass command
  line arguments of the form -b 24M instead of the raw number or bytes
- Moved a missing break from a patch later in the series to an earlier
  one
- Moved to syncing per-vCPU global variables to guest and looking up
  per-vcpu arguments based on a single CPU ID passed to each guest
  vCPU. This allows for future patches to pass more than the supported
  number of arguments for each arch to the vCPUs.
- Implemented vcpu_args_set for s390 and aarch64 [UNTESTED]
- Changed vm_create to always allocate memslot 0 at 4G instead of only
  when the number of pages required is large.
- Changed vcpu_wss to vcpu_memory_size for clarity.

Ben Gardon (10):
  KVM: selftests: Create a demand paging test
  KVM: selftests: Add demand paging content to the demand paging test
  KVM: selftests: Add configurable demand paging delay
  KVM: selftests: Add memory size parameter to the demand paging test
  KVM: selftests: Pass args to vCPU in global vCPU args struct
  KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x
  KVM: selftests: Support multiple vCPUs in demand paging test
  KVM: selftests: Time guest demand paging
  KVM: selftests: Stop memslot creation in KVM internal memslot region
  KVM: selftests: Move memslot 0 above KVM internal memslots

 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   5 +-
 .../selftests/kvm/demand_paging_test.c        | 680 ++++++++++++++++++
 .../testing/selftests/kvm/include/test_util.h |   2 +
 .../selftests/kvm/lib/aarch64/processor.c     |  33 +
 tools/testing/selftests/kvm/lib/kvm_util.c    |  27 +-
 .../selftests/kvm/lib/s390x/processor.c       |  35 +
 tools/testing/selftests/kvm/lib/test_util.c   |  61 ++
 8 files changed, 839 insertions(+), 5 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/demand_paging_test.c
 create mode 100644 tools/testing/selftests/kvm/lib/test_util.c

-- 
2.25.0.341.g760bfbb309-goog

[PATCH v4 01/10] KVM: selftests: Create a demand paging test

[Ben Gardon]

While userfaultfd, KVM's demand paging implementation, is not specific
to KVM, having a benchmark for its performance will be useful for
guiding performance improvements to KVM. As a first step towards creating
a userfaultfd demand paging test, create a simple memory access test,
based on dirty_log_test.

Reviewed-by: Oliver Upton <oupton@google.com>
Signed-off-by: Ben Gardon <bgardon@google.com>
---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   3 +
 .../selftests/kvm/demand_paging_test.c        | 286 ++++++++++++++++++
 3 files changed, 290 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/demand_paging_test.c

diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 30072c3f52fbe..9619d96e15c41 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -17,3 +17,4 @@
 /clear_dirty_log_test
 /dirty_log_test
 /kvm_create_max_vcpus
+/demand_paging_test
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 3138a916574a9..e2e1b92faee3b 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -28,15 +28,18 @@ TEST_GEN_PROGS_x86_64 += x86_64/vmx_tsc_adjust_test
 TEST_GEN_PROGS_x86_64 += x86_64/xss_msr_test
 TEST_GEN_PROGS_x86_64 += clear_dirty_log_test
 TEST_GEN_PROGS_x86_64 += dirty_log_test
+TEST_GEN_PROGS_x86_64 += demand_paging_test
 TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
 
 TEST_GEN_PROGS_aarch64 += clear_dirty_log_test
 TEST_GEN_PROGS_aarch64 += dirty_log_test
+TEST_GEN_PROGS_aarch64 += demand_paging_test
 TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
 
 TEST_GEN_PROGS_s390x = s390x/memop
 TEST_GEN_PROGS_s390x += s390x/sync_regs_test
 TEST_GEN_PROGS_s390x += dirty_log_test
+TEST_GEN_PROGS_s390x += demand_paging_test
 TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
 
 TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
new file mode 100644
index 0000000000000..5f214517ba1de
--- /dev/null
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KVM demand paging test
+ * Adapted from dirty_log_test.c
+ *
+ * Copyright (C) 2018, Red Hat, Inc.
+ * Copyright (C) 2019, Google, Inc.
+ */
+
+#define _GNU_SOURCE /* for program_invocation_name */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <time.h>
+#include <pthread.h>
+#include <linux/bitmap.h>
+#include <linux/bitops.h>
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+
+#define VCPU_ID				1
+
+/* The memory slot index demand page */
+#define TEST_MEM_SLOT_INDEX		1
+
+/* Default guest test virtual memory offset */
+#define DEFAULT_GUEST_TEST_MEM		0xc0000000
+
+/*
+ * Guest/Host shared variables. Ensure addr_gva2hva() and/or
+ * sync_global_to/from_guest() are used when accessing from
+ * the host. READ/WRITE_ONCE() should also be used with anything
+ * that may change.
+ */
+static uint64_t host_page_size;
+static uint64_t guest_page_size;
+static uint64_t guest_num_pages;
+
+/*
+ * 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;
+
+/*
+ * Continuously write to the first 8 bytes of each page in the demand paging
+ * memory region.
+ */
+static void guest_code(void)
+{
+	int i;
+
+	for (i = 0; i < guest_num_pages; i++) {
+		uint64_t addr = guest_test_virt_mem;
+
+		addr += i * guest_page_size;
+		addr &= ~(host_page_size - 1);
+		*(uint64_t *)addr = 0x0123456789ABCDEF;
+	}
+
+	GUEST_SYNC(1);
+}
+
+/* Points to the test VM memory region on which we are doing demand paging */
+static void *host_test_mem;
+static uint64_t host_num_pages;
+
+static void *vcpu_worker(void *data)
+{
+	int ret;
+	struct kvm_vm *vm = data;
+	struct kvm_run *run;
+
+	run = vcpu_state(vm, VCPU_ID);
+
+	/* Let the guest access its memory */
+	ret = _vcpu_run(vm, VCPU_ID);
+	TEST_ASSERT(ret == 0, "vcpu_run failed: %d\n", ret);
+	if (get_ucall(vm, VCPU_ID, NULL) != UCALL_SYNC) {
+		TEST_ASSERT(false,
+			    "Invalid guest sync status: exit_reason=%s\n",
+			    exit_reason_str(run->exit_reason));
+	}
+
+	return NULL;
+}
+
+static struct kvm_vm *create_vm(enum vm_guest_mode mode, uint32_t vcpuid,
+				uint64_t extra_mem_pages, void *guest_code)
+{
+	struct kvm_vm *vm;
+	uint64_t extra_pg_pages = extra_mem_pages / 512 * 2;
+
+	vm = _vm_create(mode, DEFAULT_GUEST_PHY_PAGES + extra_pg_pages, O_RDWR);
+	kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
+#ifdef __x86_64__
+	vm_create_irqchip(vm);
+#endif
+	vm_vcpu_add_default(vm, vcpuid, guest_code);
+	return vm;
+}
+
+#define GUEST_MEM_SHIFT 30 /* 1G */
+#define PAGE_SHIFT_4K  12
+
+static void run_test(enum vm_guest_mode mode)
+{
+	pthread_t vcpu_thread;
+	struct kvm_vm *vm;
+
+	/*
+	 * We reserve page table for 2 times of extra dirty mem which
+	 * will definitely cover the original (1G+) test range.  Here
+	 * we do the calculation with 4K page size which is the
+	 * smallest so the page number will be enough for all archs
+	 * (e.g., 64K page size guest will need even less memory for
+	 * page tables).
+	 */
+	vm = create_vm(mode, VCPU_ID,
+		       2ul << (GUEST_MEM_SHIFT - PAGE_SHIFT_4K),
+		       guest_code);
+
+	guest_page_size = vm_get_page_size(vm);
+	/*
+	 * A little more than 1G of guest page sized pages.  Cover the
+	 * case where the size is not aligned to 64 pages.
+	 */
+	guest_num_pages = (1ul << (GUEST_MEM_SHIFT -
+				   vm_get_page_shift(vm))) + 16;
+#ifdef __s390x__
+	/* Round up to multiple of 1M (segment size) */
+	guest_num_pages = (guest_num_pages + 0xff) & ~0xffUL;
+#endif
+
+	host_page_size = getpagesize();
+	host_num_pages = (guest_num_pages * guest_page_size) / host_page_size +
+			 !!((guest_num_pages * guest_page_size) %
+			    host_page_size);
+
+	guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) *
+			      guest_page_size;
+	guest_test_phys_mem &= ~(host_page_size - 1);
+
+#ifdef __s390x__
+	/* Align to 1M (segment size) */
+	guest_test_phys_mem &= ~((1 << 20) - 1);
+#endif
+
+	DEBUG("guest physical test memory offset: 0x%lx\n",
+	      guest_test_phys_mem);
+
+
+	/* Add an extra memory slot for testing demand paging */
+	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
+				    guest_test_phys_mem,
+				    TEST_MEM_SLOT_INDEX,
+				    guest_num_pages, 0);
+
+	/* Do mapping for the demand paging memory slot */
+	virt_map(vm, guest_test_virt_mem, guest_test_phys_mem,
+		 guest_num_pages * guest_page_size, 0);
+
+	/* Cache the HVA pointer of the region */
+	host_test_mem = addr_gpa2hva(vm, (vm_paddr_t)guest_test_phys_mem);
+
+#ifdef __x86_64__
+	vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
+#endif
+#ifdef __aarch64__
+	ucall_init(vm, NULL);
+#endif
+
+	/* Export the shared variables to the guest */
+	sync_global_to_guest(vm, host_page_size);
+	sync_global_to_guest(vm, guest_page_size);
+	sync_global_to_guest(vm, guest_test_virt_mem);
+	sync_global_to_guest(vm, guest_num_pages);
+
+	pthread_create(&vcpu_thread, NULL, vcpu_worker, vm);
+
+	/* Wait for the vcpu thread to quit */
+	pthread_join(vcpu_thread, NULL);
+
+	ucall_uninit(vm);
+	kvm_vm_free(vm);
+}
+
+struct vm_guest_mode_params {
+	bool supported;
+	bool enabled;
+};
+struct vm_guest_mode_params vm_guest_mode_params[NUM_VM_MODES];
+
+#define vm_guest_mode_params_init(mode, supported, enabled)		     \
+({									     \
+	vm_guest_mode_params[mode] =					     \
+			(struct vm_guest_mode_params){ supported, enabled }; \
+})
+
+static void help(char *name)
+{
+	int i;
+
+	puts("");
+	printf("usage: %s [-h] [-m mode]\n", name);
+	printf(" -m: specify the guest mode ID to test\n"
+	       "     (default: test all supported modes)\n"
+	       "     This option may be used multiple times.\n"
+	       "     Guest mode IDs:\n");
+	for (i = 0; i < NUM_VM_MODES; ++i) {
+		printf("         %d:    %s%s\n", i, vm_guest_mode_string(i),
+		       vm_guest_mode_params[i].supported ? " (supported)" : "");
+	}
+	puts("");
+	exit(0);
+}
+
+int main(int argc, char *argv[])
+{
+	bool mode_selected = false;
+	unsigned int mode;
+	int opt, i;
+#ifdef __aarch64__
+	unsigned int host_ipa_limit;
+#endif
+
+#ifdef __x86_64__
+	vm_guest_mode_params_init(VM_MODE_PXXV48_4K, true, true);
+#endif
+#ifdef __aarch64__
+	vm_guest_mode_params_init(VM_MODE_P40V48_4K, true, true);
+	vm_guest_mode_params_init(VM_MODE_P40V48_64K, true, true);
+
+	host_ipa_limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
+	if (host_ipa_limit >= 52)
+		vm_guest_mode_params_init(VM_MODE_P52V48_64K, true, true);
+	if (host_ipa_limit >= 48) {
+		vm_guest_mode_params_init(VM_MODE_P48V48_4K, true, true);
+		vm_guest_mode_params_init(VM_MODE_P48V48_64K, true, true);
+	}
+#endif
+#ifdef __s390x__
+	vm_guest_mode_params_init(VM_MODE_P40V48_4K, true, true);
+#endif
+
+	while ((opt = getopt(argc, argv, "hm:")) != -1) {
+		switch (opt) {
+		case 'm':
+			if (!mode_selected) {
+				for (i = 0; i < NUM_VM_MODES; ++i)
+					vm_guest_mode_params[i].enabled = false;
+				mode_selected = true;
+			}
+			mode = strtoul(optarg, NULL, 10);
+			TEST_ASSERT(mode < NUM_VM_MODES,
+				    "Guest mode ID %d too big", mode);
+			vm_guest_mode_params[mode].enabled = true;
+			break;
+		case 'h':
+		default:
+			help(argv[0]);
+			break;
+		}
+	}
+
+	for (i = 0; i < NUM_VM_MODES; ++i) {
+		if (!vm_guest_mode_params[i].enabled)
+			continue;
+		TEST_ASSERT(vm_guest_mode_params[i].supported,
+			    "Guest mode ID %d (%s) not supported.",
+			    i, vm_guest_mode_string(i));
+		run_test(i);
+	}
+
+	return 0;
+}
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v4 02/10] KVM: selftests: Add demand paging content to the demand paging test

[Ben Gardon]

The demand paging test is currently a simple page access test which, while
potentially useful, doesn't add much versus the existing dirty logging
test. To improve the demand paging test, add a basic userfaultfd demand
paging implementation.

Signed-off-by: Ben Gardon <bgardon@google.com>
---
 .../selftests/kvm/demand_paging_test.c        | 196 +++++++++++++++++-
 1 file changed, 192 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 5f214517ba1de..6fab4468f97f6 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -11,11 +11,14 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <sys/syscall.h>
 #include <unistd.h>
 #include <time.h>
+#include <poll.h>
 #include <pthread.h>
 #include <linux/bitmap.h>
 #include <linux/bitops.h>
+#include <linux/userfaultfd.h>
 
 #include "test_util.h"
 #include "kvm_util.h"
@@ -39,6 +42,8 @@ static uint64_t host_page_size;
 static uint64_t guest_page_size;
 static uint64_t guest_num_pages;
 
+static char *guest_data_prototype;
+
 /*
  * Guest physical memory offset of the testing memory slot.
  * This will be set to the topmost valid physical address minus
@@ -110,13 +115,169 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, uint32_t vcpuid,
 	return vm;
 }
 
+static int handle_uffd_page_request(int uffd, uint64_t addr)
+{
+	pid_t tid;
+	struct uffdio_copy copy;
+	int r;
+
+	tid = syscall(__NR_gettid);
+
+	copy.src = (uint64_t)guest_data_prototype;
+	copy.dst = addr;
+	copy.len = host_page_size;
+	copy.mode = 0;
+
+	r = ioctl(uffd, UFFDIO_COPY, &copy);
+	if (r == -1) {
+		DEBUG("Failed Paged in 0x%lx from thread %d with errno: %d\n",
+		      addr, tid, errno);
+		return r;
+	}
+
+	return 0;
+}
+
+bool quit_uffd_thread;
+
+struct uffd_handler_args {
+	int uffd;
+	int pipefd;
+};
+
+static void *uffd_handler_thread_fn(void *arg)
+{
+	struct uffd_handler_args *uffd_args = (struct uffd_handler_args *)arg;
+	int uffd = uffd_args->uffd;
+	int pipefd = uffd_args->pipefd;
+	int64_t pages = 0;
+
+	while (!quit_uffd_thread) {
+		struct uffd_msg msg;
+		struct pollfd pollfd[2];
+		char tmp_chr;
+		int r;
+		uint64_t addr;
+
+		pollfd[0].fd = uffd;
+		pollfd[0].events = POLLIN;
+		pollfd[1].fd = pipefd;
+		pollfd[1].events = POLLIN;
+
+		r = poll(pollfd, 2, -1);
+		switch (r) {
+		case -1:
+			DEBUG("poll err");
+			continue;
+		case 0:
+			continue;
+		case 1:
+			break;
+		default:
+			DEBUG("Polling uffd returned %d", r);
+			return NULL;
+		}
+
+		if (pollfd[0].revents & POLLERR) {
+			DEBUG("uffd revents has POLLERR");
+			return NULL;
+		}
+
+		if (pollfd[1].revents & POLLIN) {
+			r = read(pollfd[1].fd, &tmp_chr, 1);
+			TEST_ASSERT(r == 1,
+				    "Error reading pipefd in UFFD thread\n");
+			return NULL;
+		}
+
+		if (!pollfd[0].revents & POLLIN)
+			continue;
+
+		r = read(uffd, &msg, sizeof(msg));
+		if (r == -1) {
+			if (errno == EAGAIN)
+				continue;
+			DEBUG("Read of uffd gor errno %d", errno);
+			return NULL;
+		}
+
+		if (r != sizeof(msg)) {
+			DEBUG("Read on uffd returned unexpected size: %d bytes",
+			      r);
+			return NULL;
+		}
+
+		if (!(msg.event & UFFD_EVENT_PAGEFAULT))
+			continue;
+
+		addr =  msg.arg.pagefault.address;
+		r = handle_uffd_page_request(uffd, addr);
+		if (r < 0)
+			return NULL;
+		pages++;
+	}
+
+	return NULL;
+}
+
+static int setup_demand_paging(struct kvm_vm *vm,
+			       pthread_t *uffd_handler_thread, int pipefd)
+{
+	int uffd;
+	struct uffdio_api uffdio_api;
+	struct uffdio_register uffdio_register;
+	struct uffd_handler_args uffd_args;
+
+	guest_data_prototype = malloc(host_page_size);
+	TEST_ASSERT(guest_data_prototype,
+		    "Failed to allocate buffer for guest data pattern");
+	memset(guest_data_prototype, 0xAB, host_page_size);
+
+	uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
+	if (uffd == -1) {
+		DEBUG("uffd creation failed\n");
+		return -1;
+	}
+
+	uffdio_api.api = UFFD_API;
+	uffdio_api.features = 0;
+	if (ioctl(uffd, UFFDIO_API, &uffdio_api) == -1) {
+		DEBUG("ioctl uffdio_api failed\n");
+		return -1;
+	}
+
+	uffdio_register.range.start = (uint64_t)host_test_mem;
+	uffdio_register.range.len = host_num_pages * host_page_size;
+	uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING;
+	if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register) == -1) {
+		DEBUG("ioctl uffdio_register failed\n");
+		return -1;
+	}
+
+	if ((uffdio_register.ioctls & UFFD_API_RANGE_IOCTLS) !=
+			UFFD_API_RANGE_IOCTLS) {
+		DEBUG("unexpected userfaultfd ioctl set\n");
+		return -1;
+	}
+
+	uffd_args.uffd = uffd;
+	uffd_args.pipefd = pipefd;
+	pthread_create(uffd_handler_thread, NULL, uffd_handler_thread_fn,
+		       &uffd_args);
+
+	return 0;
+}
+
 #define GUEST_MEM_SHIFT 30 /* 1G */
 #define PAGE_SHIFT_4K  12
 
-static void run_test(enum vm_guest_mode mode)
+static void run_test(enum vm_guest_mode mode, bool use_uffd)
 {
 	pthread_t vcpu_thread;
+	pthread_t uffd_handler_thread;
+	int pipefd[2];
 	struct kvm_vm *vm;
+	int r;
 
 	/*
 	 * We reserve page table for 2 times of extra dirty mem which
@@ -173,6 +334,16 @@ static void run_test(enum vm_guest_mode mode)
 	/* Cache the HVA pointer of the region */
 	host_test_mem = addr_gpa2hva(vm, (vm_paddr_t)guest_test_phys_mem);
 
+	if (use_uffd) {
+		/* Set up user fault fd to handle demand paging requests. */
+		r = pipe2(pipefd, O_CLOEXEC | O_NONBLOCK);
+		TEST_ASSERT(!r, "Failed to set up pipefd");
+
+		r = setup_demand_paging(vm, &uffd_handler_thread, pipefd[0]);
+		if (r < 0)
+			exit(-r);
+	}
+
 #ifdef __x86_64__
 	vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
 #endif
@@ -191,8 +362,20 @@ static void run_test(enum vm_guest_mode mode)
 	/* Wait for the vcpu thread to quit */
 	pthread_join(vcpu_thread, NULL);
 
+	if (use_uffd) {
+		char c;
+
+		/* Tell the user fault fd handler thread to quit */
+		r = write(pipefd[1], &c, 1);
+		TEST_ASSERT(r == 1, "Unable to write to pipefd");
+
+		pthread_join(uffd_handler_thread, NULL);
+	}
+
 	ucall_uninit(vm);
 	kvm_vm_free(vm);
+
+	free(guest_data_prototype);
 }
 
 struct vm_guest_mode_params {
@@ -212,7 +395,7 @@ static void help(char *name)
 	int i;
 
 	puts("");
-	printf("usage: %s [-h] [-m mode]\n", name);
+	printf("usage: %s [-h] [-m mode] [-u]\n", name);
 	printf(" -m: specify the guest mode ID to test\n"
 	       "     (default: test all supported modes)\n"
 	       "     This option may be used multiple times.\n"
@@ -221,6 +404,7 @@ static void help(char *name)
 		printf("         %d:    %s%s\n", i, vm_guest_mode_string(i),
 		       vm_guest_mode_params[i].supported ? " (supported)" : "");
 	}
+	printf(" -u: Use User Fault FD to handle vCPU page faults.\n");
 	puts("");
 	exit(0);
 }
@@ -230,6 +414,7 @@ int main(int argc, char *argv[])
 	bool mode_selected = false;
 	unsigned int mode;
 	int opt, i;
+	bool use_uffd = false;
 #ifdef __aarch64__
 	unsigned int host_ipa_limit;
 #endif
@@ -253,7 +438,7 @@ int main(int argc, char *argv[])
 	vm_guest_mode_params_init(VM_MODE_P40V48_4K, true, true);
 #endif
 
-	while ((opt = getopt(argc, argv, "hm:")) != -1) {
+	while ((opt = getopt(argc, argv, "hm:u")) != -1) {
 		switch (opt) {
 		case 'm':
 			if (!mode_selected) {
@@ -266,6 +451,9 @@ int main(int argc, char *argv[])
 				    "Guest mode ID %d too big", mode);
 			vm_guest_mode_params[mode].enabled = true;
 			break;
+		case 'u':
+			use_uffd = true;
+			break;
 		case 'h':
 		default:
 			help(argv[0]);
@@ -279,7 +467,7 @@ int main(int argc, char *argv[])
 		TEST_ASSERT(vm_guest_mode_params[i].supported,
 			    "Guest mode ID %d (%s) not supported.",
 			    i, vm_guest_mode_string(i));
-		run_test(i);
+		run_test(i, use_uffd);
 	}
 
 	return 0;
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v4 03/10] KVM: selftests: Add configurable demand paging delay

[Ben Gardon]

When running the demand paging test with the -u option, the User Fault
FD handler essentially adds an arbitrary delay to page fault resolution.
To enable better simulation of a real demand paging scenario, add a
configurable delay to the UFFD handler.

Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Ben Gardon <bgardon@google.com>
---
 .../selftests/kvm/demand_paging_test.c        | 32 +++++++++++++++----
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 6fab4468f97f6..01d2c76ada55d 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -143,6 +143,7 @@ bool quit_uffd_thread;
 struct uffd_handler_args {
 	int uffd;
 	int pipefd;
+	useconds_t delay;
 };
 
 static void *uffd_handler_thread_fn(void *arg)
@@ -150,6 +151,7 @@ static void *uffd_handler_thread_fn(void *arg)
 	struct uffd_handler_args *uffd_args = (struct uffd_handler_args *)arg;
 	int uffd = uffd_args->uffd;
 	int pipefd = uffd_args->pipefd;
+	useconds_t delay = uffd_args->delay;
 	int64_t pages = 0;
 
 	while (!quit_uffd_thread) {
@@ -210,6 +212,8 @@ static void *uffd_handler_thread_fn(void *arg)
 		if (!(msg.event & UFFD_EVENT_PAGEFAULT))
 			continue;
 
+		if (delay)
+			usleep(delay);
 		addr =  msg.arg.pagefault.address;
 		r = handle_uffd_page_request(uffd, addr);
 		if (r < 0)
@@ -221,7 +225,8 @@ static void *uffd_handler_thread_fn(void *arg)
 }
 
 static int setup_demand_paging(struct kvm_vm *vm,
-			       pthread_t *uffd_handler_thread, int pipefd)
+			       pthread_t *uffd_handler_thread, int pipefd,
+			       useconds_t uffd_delay)
 {
 	int uffd;
 	struct uffdio_api uffdio_api;
@@ -262,6 +267,7 @@ static int setup_demand_paging(struct kvm_vm *vm,
 
 	uffd_args.uffd = uffd;
 	uffd_args.pipefd = pipefd;
+	uffd_args.delay = uffd_delay;
 	pthread_create(uffd_handler_thread, NULL, uffd_handler_thread_fn,
 		       &uffd_args);
 
@@ -271,7 +277,8 @@ static int setup_demand_paging(struct kvm_vm *vm,
 #define GUEST_MEM_SHIFT 30 /* 1G */
 #define PAGE_SHIFT_4K  12
 
-static void run_test(enum vm_guest_mode mode, bool use_uffd)
+static void run_test(enum vm_guest_mode mode, bool use_uffd,
+		     useconds_t uffd_delay)
 {
 	pthread_t vcpu_thread;
 	pthread_t uffd_handler_thread;
@@ -339,7 +346,8 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd)
 		r = pipe2(pipefd, O_CLOEXEC | O_NONBLOCK);
 		TEST_ASSERT(!r, "Failed to set up pipefd");
 
-		r = setup_demand_paging(vm, &uffd_handler_thread, pipefd[0]);
+		r = setup_demand_paging(vm, &uffd_handler_thread, pipefd[0],
+					uffd_delay);
 		if (r < 0)
 			exit(-r);
 	}
@@ -395,7 +403,7 @@ static void help(char *name)
 	int i;
 
 	puts("");
-	printf("usage: %s [-h] [-m mode] [-u]\n", name);
+	printf("usage: %s [-h] [-m mode] [-u] [-d uffd_delay_usec]\n", name);
 	printf(" -m: specify the guest mode ID to test\n"
 	       "     (default: test all supported modes)\n"
 	       "     This option may be used multiple times.\n"
@@ -404,7 +412,11 @@ static void help(char *name)
 		printf("         %d:    %s%s\n", i, vm_guest_mode_string(i),
 		       vm_guest_mode_params[i].supported ? " (supported)" : "");
 	}
-	printf(" -u: Use User Fault FD to handle vCPU page faults.\n");
+	printf(" -u: use User Fault FD to handle vCPU page\n"
+	       "     faults.\n");
+	printf(" -d: add a delay in usec to the User Fault\n"
+	       "     FD handler to simulate demand paging\n"
+	       "     overheads. Ignored without -u.\n");
 	puts("");
 	exit(0);
 }
@@ -415,6 +427,7 @@ int main(int argc, char *argv[])
 	unsigned int mode;
 	int opt, i;
 	bool use_uffd = false;
+	useconds_t uffd_delay = 0;
 #ifdef __aarch64__
 	unsigned int host_ipa_limit;
 #endif
@@ -438,7 +451,7 @@ int main(int argc, char *argv[])
 	vm_guest_mode_params_init(VM_MODE_P40V48_4K, true, true);
 #endif
 
-	while ((opt = getopt(argc, argv, "hm:u")) != -1) {
+	while ((opt = getopt(argc, argv, "hm:ud:")) != -1) {
 		switch (opt) {
 		case 'm':
 			if (!mode_selected) {
@@ -454,6 +467,11 @@ int main(int argc, char *argv[])
 		case 'u':
 			use_uffd = true;
 			break;
+		case 'd':
+			uffd_delay = strtoul(optarg, NULL, 0);
+			TEST_ASSERT(uffd_delay >= 0,
+				    "A negative UFFD delay is not supported.");
+			break;
 		case 'h':
 		default:
 			help(argv[0]);
@@ -467,7 +485,7 @@ int main(int argc, char *argv[])
 		TEST_ASSERT(vm_guest_mode_params[i].supported,
 			    "Guest mode ID %d (%s) not supported.",
 			    i, vm_guest_mode_string(i));
-		run_test(i, use_uffd);
+		run_test(i, use_uffd, uffd_delay);
 	}
 
 	return 0;
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v4 04/10] KVM: selftests: Add memory size parameter to the demand paging test

[Ben Gardon]

Add an argument to allow the demand paging test to work on larger and
smaller guest sizes.

Signed-off-by: Ben Gardon <bgardon@google.com>
---
 tools/testing/selftests/kvm/Makefile          |  2 +-
 .../selftests/kvm/demand_paging_test.c        | 57 ++++++++++-------
 .../testing/selftests/kvm/include/test_util.h |  2 +
 tools/testing/selftests/kvm/lib/test_util.c   | 61 +++++++++++++++++++
 4 files changed, 100 insertions(+), 22 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/lib/test_util.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index e2e1b92faee3b..89bf05d4c2f3e 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -7,7 +7,7 @@ top_srcdir = ../../../..
 KSFT_KHDR_INSTALL := 1
 UNAME_M := $(shell uname -m)
 
-LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c
+LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c lib/test_util.c
 LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/ucall.c
 LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
 LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 01d2c76ada55d..9d7514e96a639 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -32,6 +32,8 @@
 /* Default guest test virtual memory offset */
 #define DEFAULT_GUEST_TEST_MEM		0xc0000000
 
+#define DEFAULT_GUEST_TEST_MEM_SIZE (1 << 30) /* 1G */
+
 /*
  * Guest/Host shared variables. Ensure addr_gva2hva() and/or
  * sync_global_to/from_guest() are used when accessing from
@@ -274,11 +276,10 @@ static int setup_demand_paging(struct kvm_vm *vm,
 	return 0;
 }
 
-#define GUEST_MEM_SHIFT 30 /* 1G */
 #define PAGE_SHIFT_4K  12
 
 static void run_test(enum vm_guest_mode mode, bool use_uffd,
-		     useconds_t uffd_delay)
+		     useconds_t uffd_delay, uint64_t guest_memory_bytes)
 {
 	pthread_t vcpu_thread;
 	pthread_t uffd_handler_thread;
@@ -287,33 +288,40 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 	int r;
 
 	/*
-	 * We reserve page table for 2 times of extra dirty mem which
-	 * will definitely cover the original (1G+) test range.  Here
-	 * we do the calculation with 4K page size which is the
-	 * smallest so the page number will be enough for all archs
-	 * (e.g., 64K page size guest will need even less memory for
-	 * page tables).
+	 * We reserve page table for twice the ammount of memory we intend
+	 * to use in the test region for demand paging. Here we do the
+	 * calculation with 4K page size which is the smallest so the page
+	 * number will be enough for all archs. (e.g., 64K page size guest
+	 * will need even less memory for page tables).
 	 */
 	vm = create_vm(mode, VCPU_ID,
-		       2ul << (GUEST_MEM_SHIFT - PAGE_SHIFT_4K),
+		       (2 * guest_memory_bytes) >> PAGE_SHIFT_4K,
 		       guest_code);
 
 	guest_page_size = vm_get_page_size(vm);
-	/*
-	 * A little more than 1G of guest page sized pages.  Cover the
-	 * case where the size is not aligned to 64 pages.
-	 */
-	guest_num_pages = (1ul << (GUEST_MEM_SHIFT -
-				   vm_get_page_shift(vm))) + 16;
+
+	TEST_ASSERT(guest_memory_bytes % guest_page_size == 0,
+		    "Guest memory size is not guest page size aligned.");
+
+	guest_num_pages = guest_memory_bytes / guest_page_size;
+
 #ifdef __s390x__
 	/* Round up to multiple of 1M (segment size) */
 	guest_num_pages = (guest_num_pages + 0xff) & ~0xffUL;
 #endif
+	/*
+	 * If there should be more memory in the guest test region than there
+	 * can be pages in the guest, it will definitely cause problems.
+	 */
+	TEST_ASSERT(guest_num_pages < vm_get_max_gfn(vm),
+		    "Requested more guest memory than address space allows.\n"
+		    "    guest pages: %lx max gfn: %lx\n",
+		    guest_num_pages, vm_get_max_gfn(vm));
 
 	host_page_size = getpagesize();
-	host_num_pages = (guest_num_pages * guest_page_size) / host_page_size +
-			 !!((guest_num_pages * guest_page_size) %
-			    host_page_size);
+	TEST_ASSERT(guest_memory_bytes % host_page_size == 0,
+		    "Guest memory size is not host page size aligned.");
+	host_num_pages = guest_memory_bytes / host_page_size;
 
 	guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) *
 			      guest_page_size;
@@ -403,7 +411,8 @@ static void help(char *name)
 	int i;
 
 	puts("");
-	printf("usage: %s [-h] [-m mode] [-u] [-d uffd_delay_usec]\n", name);
+	printf("usage: %s [-h] [-m mode] [-u] [-d uffd_delay_usec]\n"
+	       "          [-b memory]\n", name);
 	printf(" -m: specify the guest mode ID to test\n"
 	       "     (default: test all supported modes)\n"
 	       "     This option may be used multiple times.\n"
@@ -417,6 +426,8 @@ static void help(char *name)
 	printf(" -d: add a delay in usec to the User Fault\n"
 	       "     FD handler to simulate demand paging\n"
 	       "     overheads. Ignored without -u.\n");
+	printf(" -b: specify the size of the memory region which should be\n"
+	       "     demand paged. e.g. 10M or 3G. Default: 1G\n");
 	puts("");
 	exit(0);
 }
@@ -424,6 +435,7 @@ static void help(char *name)
 int main(int argc, char *argv[])
 {
 	bool mode_selected = false;
+	uint64_t guest_memory_bytes = DEFAULT_GUEST_TEST_MEM_SIZE;
 	unsigned int mode;
 	int opt, i;
 	bool use_uffd = false;
@@ -451,7 +463,7 @@ int main(int argc, char *argv[])
 	vm_guest_mode_params_init(VM_MODE_P40V48_4K, true, true);
 #endif
 
-	while ((opt = getopt(argc, argv, "hm:ud:")) != -1) {
+	while ((opt = getopt(argc, argv, "hm:ud:b:")) != -1) {
 		switch (opt) {
 		case 'm':
 			if (!mode_selected) {
@@ -472,6 +484,9 @@ int main(int argc, char *argv[])
 			TEST_ASSERT(uffd_delay >= 0,
 				    "A negative UFFD delay is not supported.");
 			break;
+		case 'b':
+			guest_memory_bytes = parse_size(optarg);
+			break;
 		case 'h':
 		default:
 			help(argv[0]);
@@ -485,7 +500,7 @@ int main(int argc, char *argv[])
 		TEST_ASSERT(vm_guest_mode_params[i].supported,
 			    "Guest mode ID %d (%s) not supported.",
 			    i, vm_guest_mode_string(i));
-		run_test(i, use_uffd, uffd_delay);
+		run_test(i, use_uffd, uffd_delay, guest_memory_bytes);
 	}
 
 	return 0;
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index a41db6fb7e24b..e696c8219d69a 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -39,4 +39,6 @@ void test_assert(bool exp, const char *exp_str,
 		    #a, #b, #a, (unsigned long) __a, #b, (unsigned long) __b); \
 } while (0)
 
+size_t parse_size(const char *size);
+
 #endif /* SELFTEST_KVM_TEST_UTIL_H */
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
new file mode 100644
index 0000000000000..706e0f963a44b
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * tools/testing/selftests/kvm/lib/test_util.c
+ *
+ * Copyright (C) 2020, Google LLC.
+ */
+
+#include "test_util.h"
+
+#include <ctype.h>
+
+/*
+ * Parses "[0-9]+[kmgt]?".
+ */
+size_t parse_size(const char *size)
+{
+	size_t len = strlen(size);
+	size_t i;
+	size_t scale_shift = 0;
+	size_t base;
+
+	TEST_ASSERT(len > 0, "Need at least 1 digit in '%s'", size);
+
+	/* Find the first letter in the string, indicating scale. */
+	for (i = 0; i < len; i++) {
+		if (!isdigit(size[i])) {
+			TEST_ASSERT(i > 0, "Need at least 1 digit in '%s'",
+				    size);
+			TEST_ASSERT(i == len - 1,
+				    "Expected letter at the end in '%s'.",
+				    size);
+			switch (tolower(size[i])) {
+			case 't':
+				scale_shift = 40;
+				break;
+			case 'g':
+				scale_shift = 30;
+				break;
+			case 'm':
+				scale_shift = 20;
+				break;
+			case 'k':
+				scale_shift = 10;
+				break;
+			default:
+				TEST_ASSERT(false, "Unknown size letter %c",
+					    size[i]);
+			}
+		}
+	}
+
+	TEST_ASSERT(scale_shift < 8 * sizeof(size_t),
+		    "Overflow parsing scale!");
+
+	base = atoi(size);
+
+	TEST_ASSERT(!(base & ~((1 << (sizeof(size_t) - scale_shift)) - 1)),
+	       "Overflow parsing size!");
+
+	return base << scale_shift;
+}
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v4 05/10] KVM: selftests: Pass args to vCPU in global vCPU args struct

[Ben Gardon]

In preparation for supporting multiple vCPUs in the demand paging test,
pass arguments to the vCPU in a consolidated global struct instead of
syncing multiple globals.

Signed-off-by: Ben Gardon <bgardon@google.com>
---
 .../selftests/kvm/demand_paging_test.c        | 38 +++++++++++++------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 9d7514e96a639..9e2a5f7dfa140 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -42,7 +42,6 @@
  */
 static uint64_t host_page_size;
 static uint64_t guest_page_size;
-static uint64_t guest_num_pages;
 
 static char *guest_data_prototype;
 
@@ -59,18 +58,30 @@ static uint64_t guest_test_phys_mem;
  */
 static uint64_t guest_test_virt_mem = DEFAULT_GUEST_TEST_MEM;
 
+struct vcpu_args {
+	uint64_t gva;
+	uint64_t pages;
+
+	/* Only used by the host userspace part of the vCPU thread */
+	int vcpu_id;
+	struct kvm_vm *vm;
+};
+
+static struct vcpu_args vcpu_args;
+
 /*
  * Continuously write to the first 8 bytes of each page in the demand paging
  * memory region.
  */
 static void guest_code(void)
 {
+	uint64_t gva = vcpu_args.gva;
+	uint64_t pages = vcpu_args.pages;
 	int i;
 
-	for (i = 0; i < guest_num_pages; i++) {
-		uint64_t addr = guest_test_virt_mem;
+	for (i = 0; i < pages; i++) {
+		uint64_t addr = gva + (i * guest_page_size);
 
-		addr += i * guest_page_size;
 		addr &= ~(host_page_size - 1);
 		*(uint64_t *)addr = 0x0123456789ABCDEF;
 	}
@@ -85,15 +96,16 @@ static uint64_t host_num_pages;
 static void *vcpu_worker(void *data)
 {
 	int ret;
-	struct kvm_vm *vm = data;
+	struct kvm_vm *vm = vcpu_args.vm;
+	int vcpu_id = vcpu_args.vcpu_id;
 	struct kvm_run *run;
 
-	run = vcpu_state(vm, VCPU_ID);
+	run = vcpu_state(vm, vcpu_id);
 
 	/* Let the guest access its memory */
-	ret = _vcpu_run(vm, VCPU_ID);
+	ret = _vcpu_run(vm, vcpu_id);
 	TEST_ASSERT(ret == 0, "vcpu_run failed: %d\n", ret);
-	if (get_ucall(vm, VCPU_ID, NULL) != UCALL_SYNC) {
+	if (get_ucall(vm, vcpu_id, NULL) != UCALL_SYNC) {
 		TEST_ASSERT(false,
 			    "Invalid guest sync status: exit_reason=%s\n",
 			    exit_reason_str(run->exit_reason));
@@ -285,6 +297,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 	pthread_t uffd_handler_thread;
 	int pipefd[2];
 	struct kvm_vm *vm;
+	uint64_t guest_num_pages;
 	int r;
 
 	/*
@@ -370,10 +383,13 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 	/* Export the shared variables to the guest */
 	sync_global_to_guest(vm, host_page_size);
 	sync_global_to_guest(vm, guest_page_size);
-	sync_global_to_guest(vm, guest_test_virt_mem);
-	sync_global_to_guest(vm, guest_num_pages);
 
-	pthread_create(&vcpu_thread, NULL, vcpu_worker, vm);
+	vcpu_args.vm = vm;
+	vcpu_args.vcpu_id = VCPU_ID;
+	vcpu_args.gva = guest_test_virt_mem;
+	vcpu_args.pages = guest_num_pages;
+	sync_global_to_guest(vm, vcpu_args);
+	pthread_create(&vcpu_thread, NULL, vcpu_worker, &vcpu_args);
 
 	/* Wait for the vcpu thread to quit */
 	pthread_join(vcpu_thread, NULL);
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v4 06/10] KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x

[Ben Gardon]

Currently vcpu_args_set is only implemented for x86. This makes writing
tests with multiple vCPUs difficult as each guest vCPU must either a.)
do the same thing or b.) derive some kind of unique token from it's
registers or the architecture. To simplify the process of writing tests
with multiple vCPUs for s390 and aarch64, add set args functions for
those architectures.

Signed-off-by: Ben Gardon <bgardon@google.com>
---
 .../selftests/kvm/lib/aarch64/processor.c     | 33 +++++++++++++++++
 .../selftests/kvm/lib/s390x/processor.c       | 35 +++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index 86036a59a668e..a2ff90a75f326 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -333,3 +333,36 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
 {
 	aarch64_vcpu_add_default(vm, vcpuid, NULL, guest_code);
 }
+
+/* VM VCPU Args Set
+ *
+ * Input Args:
+ *   vm - Virtual Machine
+ *   vcpuid - VCPU ID
+ *   num - number of arguments
+ *   ... - arguments, each of type uint64_t
+ *
+ * Output Args: None
+ *
+ * Return: None
+ *
+ * Sets the first num function input arguments to the values
+ * given as variable args.  Each of the variable args is expected to
+ * be of type uint64_t. The registers set by this function are r0-r7.
+ */
+void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
+{
+	va_list ap;
+
+	TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
+		    "  num: %u\n",
+		    num);
+
+	va_start(ap, num);
+
+	for (i = 0; i < num; i++)
+		set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]),
+			va_arg(ap, uint64_t));
+
+	va_end(ap);
+}
diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c
index 32a02360b1eb0..680f37be9dbc9 100644
--- a/tools/testing/selftests/kvm/lib/s390x/processor.c
+++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
@@ -269,6 +269,41 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
 	run->psw_addr = (uintptr_t)guest_code;
 }
 
+/* VM VCPU Args Set
+ *
+ * Input Args:
+ *   vm - Virtual Machine
+ *   vcpuid - VCPU ID
+ *   num - number of arguments
+ *   ... - arguments, each of type uint64_t
+ *
+ * Output Args: None
+ *
+ * Return: None
+ *
+ * Sets the first num function input arguments to the values
+ * given as variable args.  Each of the variable args is expected to
+ * be of type uint64_t. The registers set by this function are r2-r6.
+ */
+void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
+{
+	va_list ap;
+	struct kvm_regs regs;
+
+	TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n"
+		    "  num: %u\n",
+		    num);
+
+	va_start(ap, num);
+	vcpu_regs_get(vm, vcpuid, &regs);
+
+	for (i = 0; i < num; i++)
+		regs.gprs[i + 2] = va_arg(ap, uint64_t);
+
+	vcpu_regs_set(vm, vcpuid, &regs);
+	va_end(ap);
+}
+
 void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent)
 {
 	struct vcpu *vcpu = vm->vcpu_head;
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v4 07/10] KVM: selftests: Support multiple vCPUs in demand paging test

[Ben Gardon]

Most VMs have multiple vCPUs, the concurrent execution of which has a
substantial impact on demand paging performance. Add an option to create
multiple vCPUs to each access disjoint regions of memory.

Signed-off-by: Ben Gardon <bgardon@google.com>
---
 .../selftests/kvm/demand_paging_test.c        | 255 ++++++++++++------
 1 file changed, 172 insertions(+), 83 deletions(-)

diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 9e2a5f7dfa140..2002032df32cc 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -24,8 +24,6 @@
 #include "kvm_util.h"
 #include "processor.h"
 
-#define VCPU_ID				1
-
 /* The memory slot index demand page */
 #define TEST_MEM_SLOT_INDEX		1
 
@@ -34,6 +32,14 @@
 
 #define DEFAULT_GUEST_TEST_MEM_SIZE (1 << 30) /* 1G */
 
+#ifdef PRINT_PER_VCPU_UPDATES
+#define PER_VCPU_DEBUG(...) DEBUG(__VA_ARGS__)
+#else
+#define PER_VCPU_DEBUG(...)
+#endif
+
+#define MAX_VCPUS 512
+
 /*
  * Guest/Host shared variables. Ensure addr_gva2hva() and/or
  * sync_global_to/from_guest() are used when accessing from
@@ -67,18 +73,25 @@ struct vcpu_args {
 	struct kvm_vm *vm;
 };
 
-static struct vcpu_args vcpu_args;
+static struct vcpu_args vcpu_args[MAX_VCPUS];
 
 /*
  * Continuously write to the first 8 bytes of each page in the demand paging
  * memory region.
  */
-static void guest_code(void)
+static void guest_code(uint32_t vcpu_id)
 {
-	uint64_t gva = vcpu_args.gva;
-	uint64_t pages = vcpu_args.pages;
+	uint64_t gva;
+	uint64_t pages;
 	int i;
 
+	/* Return to signal error if vCPU args data structure is courrupt. */
+	if (vcpu_args[vcpu_id].vcpu_id != vcpu_id)
+		return;
+
+	gva = vcpu_args[vcpu_id].gva;
+	pages = vcpu_args[vcpu_id].pages;
+
 	for (i = 0; i < pages; i++) {
 		uint64_t addr = gva + (i * guest_page_size);
 
@@ -89,17 +102,15 @@ static void guest_code(void)
 	GUEST_SYNC(1);
 }
 
-/* Points to the test VM memory region on which we are doing demand paging */
-static void *host_test_mem;
-static uint64_t host_num_pages;
-
 static void *vcpu_worker(void *data)
 {
 	int ret;
-	struct kvm_vm *vm = vcpu_args.vm;
-	int vcpu_id = vcpu_args.vcpu_id;
+	struct vcpu_args *args = (struct vcpu_args *)data;
+	struct kvm_vm *vm = args->vm;
+	int vcpu_id = args->vcpu_id;
 	struct kvm_run *run;
 
+	vcpu_args_set(vm, vcpu_id, 1, vcpu_id);
 	run = vcpu_state(vm, vcpu_id);
 
 	/* Let the guest access its memory */
@@ -114,18 +125,33 @@ static void *vcpu_worker(void *data)
 	return NULL;
 }
 
-static struct kvm_vm *create_vm(enum vm_guest_mode mode, uint32_t vcpuid,
-				uint64_t extra_mem_pages, void *guest_code)
+#define PAGE_SHIFT_4K  12
+#define PTES_PER_4K_PT 512
+
+static struct kvm_vm *create_vm(enum vm_guest_mode mode, int vcpus,
+				uint64_t vcpu_memory_bytes)
 {
 	struct kvm_vm *vm;
-	uint64_t extra_pg_pages = extra_mem_pages / 512 * 2;
+	uint64_t pages = DEFAULT_GUEST_PHY_PAGES;
+
+	/* Account for a few pages per-vCPU for stacks */
+	pages += DEFAULT_STACK_PGS * vcpus;
+
+	/*
+	 * Reserve twice the ammount of memory needed to map the test region and
+	 * the page table / stacks region, at 4k, for page tables. Do the
+	 * calculation with 4K page size: the smallest of all archs. (e.g., 64K
+	 * page size guest will need even less memory for page tables).
+	 */
+	pages += (2 * pages) / PTES_PER_4K_PT;
+	pages += ((2 * vcpus * vcpu_memory_bytes) >> PAGE_SHIFT_4K) /
+		 PTES_PER_4K_PT;
 
-	vm = _vm_create(mode, DEFAULT_GUEST_PHY_PAGES + extra_pg_pages, O_RDWR);
+	vm = _vm_create(mode, pages, O_RDWR);
 	kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
 #ifdef __x86_64__
 	vm_create_irqchip(vm);
 #endif
-	vm_vcpu_add_default(vm, vcpuid, guest_code);
 	return vm;
 }
 
@@ -240,17 +266,13 @@ static void *uffd_handler_thread_fn(void *arg)
 
 static int setup_demand_paging(struct kvm_vm *vm,
 			       pthread_t *uffd_handler_thread, int pipefd,
-			       useconds_t uffd_delay)
+			       useconds_t uffd_delay,
+			       struct uffd_handler_args *uffd_args,
+			       void *hva, uint64_t len)
 {
 	int uffd;
 	struct uffdio_api uffdio_api;
 	struct uffdio_register uffdio_register;
-	struct uffd_handler_args uffd_args;
-
-	guest_data_prototype = malloc(host_page_size);
-	TEST_ASSERT(guest_data_prototype,
-		    "Failed to allocate buffer for guest data pattern");
-	memset(guest_data_prototype, 0xAB, host_page_size);
 
 	uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
 	if (uffd == -1) {
@@ -265,8 +287,8 @@ static int setup_demand_paging(struct kvm_vm *vm,
 		return -1;
 	}
 
-	uffdio_register.range.start = (uint64_t)host_test_mem;
-	uffdio_register.range.len = host_num_pages * host_page_size;
+	uffdio_register.range.start = (uint64_t)hva;
+	uffdio_register.range.len = len;
 	uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING;
 	if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register) == -1) {
 		DEBUG("ioctl uffdio_register failed\n");
@@ -279,44 +301,39 @@ static int setup_demand_paging(struct kvm_vm *vm,
 		return -1;
 	}
 
-	uffd_args.uffd = uffd;
-	uffd_args.pipefd = pipefd;
-	uffd_args.delay = uffd_delay;
+	uffd_args->uffd = uffd;
+	uffd_args->pipefd = pipefd;
+	uffd_args->delay = uffd_delay;
 	pthread_create(uffd_handler_thread, NULL, uffd_handler_thread_fn,
-		       &uffd_args);
+		       uffd_args);
+
+	PER_VCPU_DEBUG("Created uffd thread for HVA range [%p, %p)\n",
+		       hva, hva + len);
 
 	return 0;
 }
 
-#define PAGE_SHIFT_4K  12
-
 static void run_test(enum vm_guest_mode mode, bool use_uffd,
-		     useconds_t uffd_delay, uint64_t guest_memory_bytes)
+		     useconds_t uffd_delay, int vcpus,
+		     uint64_t vcpu_memory_bytes)
 {
-	pthread_t vcpu_thread;
-	pthread_t uffd_handler_thread;
-	int pipefd[2];
+	pthread_t *vcpu_threads;
+	pthread_t *uffd_handler_threads = NULL;
+	struct uffd_handler_args *uffd_args = NULL;
+	int *pipefds;
 	struct kvm_vm *vm;
 	uint64_t guest_num_pages;
+	int vcpu_id;
 	int r;
 
-	/*
-	 * We reserve page table for twice the ammount of memory we intend
-	 * to use in the test region for demand paging. Here we do the
-	 * calculation with 4K page size which is the smallest so the page
-	 * number will be enough for all archs. (e.g., 64K page size guest
-	 * will need even less memory for page tables).
-	 */
-	vm = create_vm(mode, VCPU_ID,
-		       (2 * guest_memory_bytes) >> PAGE_SHIFT_4K,
-		       guest_code);
+	vm = create_vm(mode, vcpus, vcpu_memory_bytes);
 
 	guest_page_size = vm_get_page_size(vm);
 
-	TEST_ASSERT(guest_memory_bytes % guest_page_size == 0,
+	TEST_ASSERT(vcpu_memory_bytes % guest_page_size == 0,
 		    "Guest memory size is not guest page size aligned.");
 
-	guest_num_pages = guest_memory_bytes / guest_page_size;
+	guest_num_pages = (vcpus * vcpu_memory_bytes) / guest_page_size;
 
 #ifdef __s390x__
 	/* Round up to multiple of 1M (segment size) */
@@ -328,13 +345,13 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 	 */
 	TEST_ASSERT(guest_num_pages < vm_get_max_gfn(vm),
 		    "Requested more guest memory than address space allows.\n"
-		    "    guest pages: %lx max gfn: %lx\n",
-		    guest_num_pages, vm_get_max_gfn(vm));
+		    "    guest pages: %lx max gfn: %lx vcpus: %d wss: %lx]\n",
+		    guest_num_pages, vm_get_max_gfn(vm), vcpus,
+		    vcpu_memory_bytes);
 
 	host_page_size = getpagesize();
-	TEST_ASSERT(guest_memory_bytes % host_page_size == 0,
+	TEST_ASSERT(vcpu_memory_bytes % host_page_size == 0,
 		    "Guest memory size is not host page size aligned.");
-	host_num_pages = guest_memory_bytes / host_page_size;
 
 	guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) *
 			      guest_page_size;
@@ -359,55 +376,116 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 	virt_map(vm, guest_test_virt_mem, guest_test_phys_mem,
 		 guest_num_pages * guest_page_size, 0);
 
-	/* Cache the HVA pointer of the region */
-	host_test_mem = addr_gpa2hva(vm, (vm_paddr_t)guest_test_phys_mem);
+#ifdef __aarch64__
+	ucall_init(vm, NULL);
+#endif
+
+	guest_data_prototype = malloc(host_page_size);
+	TEST_ASSERT(guest_data_prototype,
+		    "Failed to allocate buffer for guest data pattern");
+	memset(guest_data_prototype, 0xAB, host_page_size);
+
+	vcpu_threads = malloc(vcpus * sizeof(*vcpu_threads));
+	TEST_ASSERT(vcpu_threads, "Memory allocation failed");
 
 	if (use_uffd) {
-		/* Set up user fault fd to handle demand paging requests. */
-		r = pipe2(pipefd, O_CLOEXEC | O_NONBLOCK);
-		TEST_ASSERT(!r, "Failed to set up pipefd");
+		uffd_handler_threads =
+			malloc(vcpus * sizeof(*uffd_handler_threads));
+		TEST_ASSERT(uffd_handler_threads, "Memory allocation failed");
 
-		r = setup_demand_paging(vm, &uffd_handler_thread, pipefd[0],
-					uffd_delay);
-		if (r < 0)
-			exit(-r);
+		uffd_args = malloc(vcpus * sizeof(*uffd_args));
+		TEST_ASSERT(uffd_args, "Memory allocation failed");
+
+		pipefds = malloc(sizeof(int) * vcpus * 2);
+		TEST_ASSERT(pipefds, "Unable to allocate memory for pipefd");
 	}
 
+	for (vcpu_id = 0; vcpu_id < vcpus; vcpu_id++) {
+		vm_paddr_t vcpu_gpa;
+		void *vcpu_hva;
+
+		vm_vcpu_add_default(vm, vcpu_id, guest_code);
+
+		vcpu_gpa = guest_test_phys_mem + (vcpu_id * vcpu_memory_bytes);
+		PER_VCPU_DEBUG("Added VCPU %d with test mem gpa [%lx, %lx)\n",
+			       vcpu_id, vcpu_gpa, vcpu_gpa + vcpu_memory_bytes);
+
+		/* Cache the HVA pointer of the region */
+		vcpu_hva = addr_gpa2hva(vm, vcpu_gpa);
+
+		if (use_uffd) {
+			/*
+			 * Set up user fault fd to handle demand paging
+			 * requests.
+			 */
+			r = pipe2(&pipefds[vcpu_id * 2],
+				  O_CLOEXEC | O_NONBLOCK);
+			TEST_ASSERT(!r, "Failed to set up pipefd");
+
+			r = setup_demand_paging(vm,
+						&uffd_handler_threads[vcpu_id],
+						pipefds[vcpu_id * 2],
+						uffd_delay, &uffd_args[vcpu_id],
+						vcpu_hva, vcpu_memory_bytes);
+			if (r < 0)
+				exit(-r);
+		}
+
 #ifdef __x86_64__
-	vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
-#endif
-#ifdef __aarch64__
-	ucall_init(vm, NULL);
+		vcpu_set_cpuid(vm, vcpu_id, kvm_get_supported_cpuid());
 #endif
 
+		vcpu_args[vcpu_id].vm = vm;
+		vcpu_args[vcpu_id].vcpu_id = vcpu_id;
+		vcpu_args[vcpu_id].gva = guest_test_virt_mem +
+					 (vcpu_id * vcpu_memory_bytes);
+		vcpu_args[vcpu_id].pages = vcpu_memory_bytes / guest_page_size;
+	}
+
 	/* Export the shared variables to the guest */
 	sync_global_to_guest(vm, host_page_size);
 	sync_global_to_guest(vm, guest_page_size);
-
-	vcpu_args.vm = vm;
-	vcpu_args.vcpu_id = VCPU_ID;
-	vcpu_args.gva = guest_test_virt_mem;
-	vcpu_args.pages = guest_num_pages;
 	sync_global_to_guest(vm, vcpu_args);
-	pthread_create(&vcpu_thread, NULL, vcpu_worker, &vcpu_args);
 
-	/* Wait for the vcpu thread to quit */
-	pthread_join(vcpu_thread, NULL);
+	DEBUG("Finished creating vCPUs and starting uffd threads\n");
+
+	for (vcpu_id = 0; vcpu_id < vcpus; vcpu_id++) {
+		pthread_create(&vcpu_threads[vcpu_id], NULL, vcpu_worker,
+			       &vcpu_args[vcpu_id]);
+	}
+
+	DEBUG("Started all vCPUs\n");
+
+	/* Wait for the vcpu threads to quit */
+	for (vcpu_id = 0; vcpu_id < vcpus; vcpu_id++) {
+		pthread_join(vcpu_threads[vcpu_id], NULL);
+		PER_VCPU_DEBUG("Joined thread for vCPU %d\n", vcpu_id);
+	}
+
+	DEBUG("All vCPU threads joined\n");
 
 	if (use_uffd) {
 		char c;
 
-		/* Tell the user fault fd handler thread to quit */
-		r = write(pipefd[1], &c, 1);
-		TEST_ASSERT(r == 1, "Unable to write to pipefd");
+		/* Tell the user fault fd handler threads to quit */
+		for (vcpu_id = 0; vcpu_id < vcpus; vcpu_id++) {
+			r = write(pipefds[vcpu_id * 2 + 1], &c, 1);
+			TEST_ASSERT(r == 1, "Unable to write to pipefd");
 
-		pthread_join(uffd_handler_thread, NULL);
+			pthread_join(uffd_handler_threads[vcpu_id], NULL);
+		}
 	}
 
 	ucall_uninit(vm);
 	kvm_vm_free(vm);
 
 	free(guest_data_prototype);
+	free(vcpu_threads);
+	if (use_uffd) {
+		free(uffd_handler_threads);
+		free(uffd_args);
+		free(pipefds);
+	}
 }
 
 struct vm_guest_mode_params {
@@ -428,7 +506,7 @@ static void help(char *name)
 
 	puts("");
 	printf("usage: %s [-h] [-m mode] [-u] [-d uffd_delay_usec]\n"
-	       "          [-b memory]\n", name);
+	       "          [-b memory] [-v vcpus]\n", name);
 	printf(" -m: specify the guest mode ID to test\n"
 	       "     (default: test all supported modes)\n"
 	       "     This option may be used multiple times.\n"
@@ -443,7 +521,9 @@ static void help(char *name)
 	       "     FD handler to simulate demand paging\n"
 	       "     overheads. Ignored without -u.\n");
 	printf(" -b: specify the size of the memory region which should be\n"
-	       "     demand paged. e.g. 10M or 3G. Default: 1G\n");
+	       "     demand paged by each vCPU. e.g. 10M or 3G.\n"
+	       "     Default: 1G\n");
+	printf(" -v: specify the number of vCPUs to run.\n");
 	puts("");
 	exit(0);
 }
@@ -451,7 +531,8 @@ static void help(char *name)
 int main(int argc, char *argv[])
 {
 	bool mode_selected = false;
-	uint64_t guest_memory_bytes = DEFAULT_GUEST_TEST_MEM_SIZE;
+	uint64_t vcpu_memory_bytes = DEFAULT_GUEST_TEST_MEM_SIZE;
+	int vcpus = 1;
 	unsigned int mode;
 	int opt, i;
 	bool use_uffd = false;
@@ -479,7 +560,7 @@ int main(int argc, char *argv[])
 	vm_guest_mode_params_init(VM_MODE_P40V48_4K, true, true);
 #endif
 
-	while ((opt = getopt(argc, argv, "hm:ud:b:")) != -1) {
+	while ((opt = getopt(argc, argv, "hm:ud:b:v:")) != -1) {
 		switch (opt) {
 		case 'm':
 			if (!mode_selected) {
@@ -501,7 +582,15 @@ int main(int argc, char *argv[])
 				    "A negative UFFD delay is not supported.");
 			break;
 		case 'b':
-			guest_memory_bytes = parse_size(optarg);
+			vcpu_memory_bytes = parse_size(optarg);
+			break;
+		case 'v':
+			vcpus = atoi(optarg);
+			TEST_ASSERT(vcpus > 0,
+				    "Must have a positive number of vCPUs");
+			TEST_ASSERT(vcpus <= MAX_VCPUS,
+				    "This test does not currently support\n"
+				    "more than %d vCPUs.", MAX_VCPUS);
 			break;
 		case 'h':
 		default:
@@ -516,7 +605,7 @@ int main(int argc, char *argv[])
 		TEST_ASSERT(vm_guest_mode_params[i].supported,
 			    "Guest mode ID %d (%s) not supported.",
 			    i, vm_guest_mode_string(i));
-		run_test(i, use_uffd, uffd_delay, guest_memory_bytes);
+		run_test(i, use_uffd, uffd_delay, vcpus, vcpu_memory_bytes);
 	}
 
 	return 0;
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v4 08/10] KVM: selftests: Time guest demand paging

[Ben Gardon]

In order to quantify demand paging performance, time guest execution
during demand paging.

Signed-off-by: Ben Gardon <bgardon@google.com>
---
 .../selftests/kvm/demand_paging_test.c        | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 2002032df32cc..0dc5d04718678 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -32,6 +32,12 @@
 
 #define DEFAULT_GUEST_TEST_MEM_SIZE (1 << 30) /* 1G */
 
+#ifdef PRINT_PER_PAGE_UPDATES
+#define PER_PAGE_DEBUG(...) DEBUG(__VA_ARGS__)
+#else
+#define PER_PAGE_DEBUG(...)
+#endif
+
 #ifdef PRINT_PER_VCPU_UPDATES
 #define PER_VCPU_DEBUG(...) DEBUG(__VA_ARGS__)
 #else
@@ -64,6 +70,26 @@ static uint64_t guest_test_phys_mem;
  */
 static uint64_t guest_test_virt_mem = DEFAULT_GUEST_TEST_MEM;
 
+int64_t to_ns(struct timespec ts)
+{
+	return (int64_t)ts.tv_nsec + 1000000000LL * (int64_t)ts.tv_sec;
+}
+
+struct timespec diff(struct timespec start, struct  timespec end)
+{
+	struct   timespec temp;
+
+	if ((end.tv_nsec-start.tv_nsec) < 0) {
+		temp.tv_sec = end.tv_sec - start.tv_sec - 1;
+		temp.tv_nsec = 1000000000 + end.tv_nsec - start.tv_nsec;
+	} else {
+		temp.tv_sec = end.tv_sec - start.tv_sec;
+		temp.tv_nsec = end.tv_nsec - start.tv_nsec;
+	}
+
+	return temp;
+}
+
 struct vcpu_args {
 	uint64_t gva;
 	uint64_t pages;
@@ -109,10 +135,14 @@ static void *vcpu_worker(void *data)
 	struct kvm_vm *vm = args->vm;
 	int vcpu_id = args->vcpu_id;
 	struct kvm_run *run;
+	struct timespec start;
+	struct timespec end;
 
 	vcpu_args_set(vm, vcpu_id, 1, vcpu_id);
 	run = vcpu_state(vm, vcpu_id);
 
+	clock_gettime(CLOCK_MONOTONIC, &start);
+
 	/* Let the guest access its memory */
 	ret = _vcpu_run(vm, vcpu_id);
 	TEST_ASSERT(ret == 0, "vcpu_run failed: %d\n", ret);
@@ -122,6 +152,11 @@ static void *vcpu_worker(void *data)
 			    exit_reason_str(run->exit_reason));
 	}
 
+	clock_gettime(CLOCK_MONOTONIC, &end);
+	PER_VCPU_DEBUG("vCPU %d execution time: %lld.%.9lds\n", vcpu_id,
+		       (long long)(diff(start, end).tv_sec),
+		       diff(start, end).tv_nsec);
+
 	return NULL;
 }
 
@@ -158,6 +193,8 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, int vcpus,
 static int handle_uffd_page_request(int uffd, uint64_t addr)
 {
 	pid_t tid;
+	struct timespec start;
+	struct timespec end;
 	struct uffdio_copy copy;
 	int r;
 
@@ -168,6 +205,8 @@ static int handle_uffd_page_request(int uffd, uint64_t addr)
 	copy.len = host_page_size;
 	copy.mode = 0;
 
+	clock_gettime(CLOCK_MONOTONIC, &start);
+
 	r = ioctl(uffd, UFFDIO_COPY, &copy);
 	if (r == -1) {
 		DEBUG("Failed Paged in 0x%lx from thread %d with errno: %d\n",
@@ -175,6 +214,13 @@ static int handle_uffd_page_request(int uffd, uint64_t addr)
 		return r;
 	}
 
+	clock_gettime(CLOCK_MONOTONIC, &end);
+
+	PER_PAGE_DEBUG("UFFDIO_COPY %d \t%lld ns\n", tid,
+		       (long long)to_ns(diff(start, end)));
+	PER_PAGE_DEBUG("Paged in %ld bytes at 0x%lx from thread %d\n",
+		       host_page_size, addr, tid);
+
 	return 0;
 }
 
@@ -193,7 +239,10 @@ static void *uffd_handler_thread_fn(void *arg)
 	int pipefd = uffd_args->pipefd;
 	useconds_t delay = uffd_args->delay;
 	int64_t pages = 0;
+	struct timespec start;
+	struct timespec end;
 
+	clock_gettime(CLOCK_MONOTONIC, &start);
 	while (!quit_uffd_thread) {
 		struct uffd_msg msg;
 		struct pollfd pollfd[2];
@@ -261,6 +310,13 @@ static void *uffd_handler_thread_fn(void *arg)
 		pages++;
 	}
 
+	clock_gettime(CLOCK_MONOTONIC, &end);
+	PER_VCPU_DEBUG("userfaulted %ld pages over %lld.%.9lds. (%f/sec)\n",
+		       pages, (long long)(diff(start, end).tv_sec),
+		       diff(start, end).tv_nsec, pages /
+		       ((double)diff(start, end).tv_sec +
+			(double)diff(start, end).tv_nsec / 100000000.0));
+
 	return NULL;
 }
 
@@ -325,6 +381,8 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 	uint64_t guest_num_pages;
 	int vcpu_id;
 	int r;
+	struct timespec start;
+	struct timespec end;
 
 	vm = create_vm(mode, vcpus, vcpu_memory_bytes);
 
@@ -449,6 +507,8 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 
 	DEBUG("Finished creating vCPUs and starting uffd threads\n");
 
+	clock_gettime(CLOCK_MONOTONIC, &start);
+
 	for (vcpu_id = 0; vcpu_id < vcpus; vcpu_id++) {
 		pthread_create(&vcpu_threads[vcpu_id], NULL, vcpu_worker,
 			       &vcpu_args[vcpu_id]);
@@ -464,6 +524,8 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 
 	DEBUG("All vCPU threads joined\n");
 
+	clock_gettime(CLOCK_MONOTONIC, &end);
+
 	if (use_uffd) {
 		char c;
 
@@ -476,6 +538,12 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 		}
 	}
 
+	DEBUG("Total guest execution time: %lld.%.9lds\n",
+	      (long long)(diff(start, end).tv_sec), diff(start, end).tv_nsec);
+	DEBUG("Overall demand paging rate: %f pgs/sec\n",
+	      guest_num_pages / ((double)diff(start, end).tv_sec +
+	      (double)diff(start, end).tv_nsec / 100000000.0));
+
 	ucall_uninit(vm);
 	kvm_vm_free(vm);
 
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v4 09/10] KVM: selftests: Stop memslot creation in KVM internal memslot region

[Ben Gardon]

KVM creates internal memslots covering the region between 3G and 4G in
the guest physical address space, when the first vCPU is created.
Mapping this region before creation of the first vCPU causes vCPU
creation to fail. Prohibit tests from creating such a memslot and fail
with a helpful warning when they try to.

Signed-off-by: Ben Gardon <bgardon@google.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 41cf45416060f..5b971c04f1643 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -113,6 +113,8 @@ const char * const vm_guest_mode_string[] = {
 _Static_assert(sizeof(vm_guest_mode_string)/sizeof(char *) == NUM_VM_MODES,
 	       "Missing new mode strings?");
 
+#define KVM_INTERNAL_MEMSLOTS_START_PADDR (3UL << 30)
+#define KVM_INTERNAL_MEMSLOTS_END_PADDR (4UL << 30)
 /*
  * VM Create
  *
@@ -593,6 +595,20 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
 		"  vm->max_gfn: 0x%lx vm->page_size: 0x%x",
 		guest_paddr, npages, vm->max_gfn, vm->page_size);
 
+	/*
+	 * Check that this region does not overlap with KVM internal memslots
+	 * which are created when the first vCPU is created.
+	 */
+	TEST_ASSERT(guest_paddr >= KVM_INTERNAL_MEMSLOTS_END_PADDR ||
+		    guest_paddr + npages < KVM_INTERNAL_MEMSLOTS_START_PADDR,
+		    "Memslot overlapps with region mapped by internal KVM\n"
+		    "memslots:\n"
+		    "  Requested paddr range:      [0x%lx, 0x%lx)\n"
+		    "  KVM internal memslot range: [0x%lx, 0x%lx)\n",
+		    guest_paddr, guest_paddr + npages,
+		    KVM_INTERNAL_MEMSLOTS_START_PADDR,
+		    KVM_INTERNAL_MEMSLOTS_END_PADDR);
+
 	/*
 	 * Confirm a mem region with an overlapping address doesn't
 	 * already exist.
-- 
2.25.0.341.g760bfbb309-goog

[PATCH v4 10/10] KVM: selftests: Move memslot 0 above KVM internal memslots

[Ben Gardon]

KVM creates internal memslots between 3 and 4 GiB paddrs on the first
vCPU creation. If memslot 0 is large enough it collides with these
memslots an causes vCPU creation to fail. Instead of creating memslot 0
at paddr 0, start it 4G into the guest physical address space.

Signed-off-by: Ben Gardon <bgardon@google.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 5b971c04f1643..427c88d32e988 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -130,9 +130,11 @@ _Static_assert(sizeof(vm_guest_mode_string)/sizeof(char *) == NUM_VM_MODES,
  *
  * Creates a VM with the mode specified by mode (e.g. VM_MODE_P52V48_4K).
  * When phy_pages is non-zero, a memory region of phy_pages physical pages
- * is created and mapped starting at guest physical address 0.  The file
- * descriptor to control the created VM is created with the permissions
- * given by perm (e.g. O_RDWR).
+ * is created, starting at 4G into the guest physical address space to avoid
+ * KVM internal memslots which map the region between 3G and 4G. If tests need
+ * to use the physical region between 0 and 3G, they can allocate another
+ * memslot for that region. The file descriptor to control the created VM is
+ * created with the permissions given by perm (e.g. O_RDWR).
  */
 struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
 {
@@ -231,7 +233,8 @@ struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
 	vm->vpages_mapped = sparsebit_alloc();
 	if (phy_pages != 0)
 		vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
-					    0, 0, phy_pages, 0);
+					    KVM_INTERNAL_MEMSLOTS_END_PADDR,
+					    0, phy_pages, 0);
 
 	return vm;
 }
-- 
2.25.0.341.g760bfbb309-goog

Re: [PATCH v4 09/10] KVM: selftests: Stop memslot creation in KVM internal memslot region

[Paolo Bonzini]

On 23/01/20 19:04, Ben Gardon wrote:
> KVM creates internal memslots covering the region between 3G and 4G in
> the guest physical address space, when the first vCPU is created.
> Mapping this region before creation of the first vCPU causes vCPU
> creation to fail. Prohibit tests from creating such a memslot and fail
> with a helpful warning when they try to.
> 
> Signed-off-by: Ben Gardon <bgardon@google.com>
> ---

The internal memslots are much higher than this (0xfffbc000 and
0xfee00000).  I'm changing the patch to block 0xfe0000000 and above,
otherwise it breaks vmx_dirty_log_test.

Paolo

Re: [PATCH v4 10/10] KVM: selftests: Move memslot 0 above KVM internal memslots

[Paolo Bonzini]

On 23/01/20 19:04, Ben Gardon wrote:
> KVM creates internal memslots between 3 and 4 GiB paddrs on the first
> vCPU creation. If memslot 0 is large enough it collides with these
> memslots an causes vCPU creation to fail. Instead of creating memslot 0
> at paddr 0, start it 4G into the guest physical address space.
> 
> Signed-off-by: Ben Gardon <bgardon@google.com>
> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

This breaks all tests for me:

   $ ./state_test
   Testing guest mode: PA-bits:ANY, VA-bits:48,  4K pages
   Guest physical address width detected: 46
   ==== Test Assertion Failure ====
  lib/x86_64/processor.c:580: false
  pid=4873 tid=4873 - Success
     1	0x0000000000409996: addr_gva2gpa at processor.c:579
     2	0x0000000000406a38: addr_gva2hva at kvm_util.c:1636
     3	0x000000000041036c: kvm_vm_elf_load at elf.c:192
     4	0x0000000000409ea9: vm_create_default at processor.c:829
     5	0x0000000000400f6f: main at state_test.c:132
     6	0x00007f21bdf90494: ?? ??:0
     7	0x0000000000401287: _start at ??:?
  No mapping for vm virtual address, gva: 0x400000

Memslot 0 should not be too large, so this patch should not be needed.

Paolo

Re: [PATCH v4 06/10] KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x

[Paolo Bonzini]

CCing Marc, Conny and Christian (plus Thomas and Drew who were already
in the list) for review.

Thanks,

Paolo

On 23/01/20 19:04, Ben Gardon wrote:
> Currently vcpu_args_set is only implemented for x86. This makes writing
> tests with multiple vCPUs difficult as each guest vCPU must either a.)
> do the same thing or b.) derive some kind of unique token from it's
> registers or the architecture. To simplify the process of writing tests
> with multiple vCPUs for s390 and aarch64, add set args functions for
> those architectures.
> 
> Signed-off-by: Ben Gardon <bgardon@google.com>
> ---
>  .../selftests/kvm/lib/aarch64/processor.c     | 33 +++++++++++++++++
>  .../selftests/kvm/lib/s390x/processor.c       | 35 +++++++++++++++++++
>  2 files changed, 68 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> index 86036a59a668e..a2ff90a75f326 100644
> --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> @@ -333,3 +333,36 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
>  {
>  	aarch64_vcpu_add_default(vm, vcpuid, NULL, guest_code);
>  }
> +
> +/* VM VCPU Args Set
> + *
> + * Input Args:
> + *   vm - Virtual Machine
> + *   vcpuid - VCPU ID
> + *   num - number of arguments
> + *   ... - arguments, each of type uint64_t
> + *
> + * Output Args: None
> + *
> + * Return: None
> + *
> + * Sets the first num function input arguments to the values
> + * given as variable args.  Each of the variable args is expected to
> + * be of type uint64_t. The registers set by this function are r0-r7.
> + */
> +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
> +{
> +	va_list ap;
> +
> +	TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
> +		    "  num: %u\n",
> +		    num);
> +
> +	va_start(ap, num);
> +
> +	for (i = 0; i < num; i++)
> +		set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]),
> +			va_arg(ap, uint64_t));
> +
> +	va_end(ap);
> +}
> diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c
> index 32a02360b1eb0..680f37be9dbc9 100644
> --- a/tools/testing/selftests/kvm/lib/s390x/processor.c
> +++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
> @@ -269,6 +269,41 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
>  	run->psw_addr = (uintptr_t)guest_code;
>  }
>  
> +/* VM VCPU Args Set
> + *
> + * Input Args:
> + *   vm - Virtual Machine
> + *   vcpuid - VCPU ID
> + *   num - number of arguments
> + *   ... - arguments, each of type uint64_t
> + *
> + * Output Args: None
> + *
> + * Return: None
> + *
> + * Sets the first num function input arguments to the values
> + * given as variable args.  Each of the variable args is expected to
> + * be of type uint64_t. The registers set by this function are r2-r6.
> + */
> +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
> +{
> +	va_list ap;
> +	struct kvm_regs regs;
> +
> +	TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n"
> +		    "  num: %u\n",
> +		    num);
> +
> +	va_start(ap, num);
> +	vcpu_regs_get(vm, vcpuid, &regs);
> +
> +	for (i = 0; i < num; i++)
> +		regs.gprs[i + 2] = va_arg(ap, uint64_t);
> +
> +	vcpu_regs_set(vm, vcpuid, &regs);
> +	va_end(ap);
> +}
> +
>  void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent)
>  {
>  	struct vcpu *vcpu = vm->vcpu_head;
> 

Re: [PATCH v4 00/10] Create a userfaultfd demand paging test

[Paolo Bonzini]

On 23/01/20 19:04, Ben Gardon wrote:
> When handling page faults for many vCPUs during demand paging, KVM's MMU
> lock becomes highly contended. This series creates a test with a naive
> userfaultfd based demand paging implementation to demonstrate that
> contention. This test serves both as a functional test of userfaultfd
> and a microbenchmark of demand paging performance with a variable number
> of vCPUs and memory per vCPU.
> 
> The test creates N userfaultfd threads, N vCPUs, and a region of memory
> with M pages per vCPU. The N userfaultfd polling threads are each set up
> to serve faults on a region of memory corresponding to one of the vCPUs.
> Each of the vCPUs is then started, and touches each page of its disjoint
> memory region, sequentially. In response to faults, the userfaultfd
> threads copy a static buffer into the guest's memory. This creates a
> worst case for MMU lock contention as we have removed most of the
> contention between the userfaultfd threads and there is no time required
> to fetch the contents of guest memory.
> 
> This test was run successfully on Intel Haswell, Broadwell, and
> Cascadelake hosts with a variety of vCPU counts and memory sizes.
> 
> This test was adapted from the dirty_log_test.
> 
> The series can also be viewed in Gerrit here:
> https://linux-review.googlesource.com/c/virt/kvm/kvm/+/1464
> (Thanks to Dmitry Vyukov <dvyukov@google.com> for setting up the Gerrit
> instance)
> 
> v4 (Responding to feedback from Andrew Jones, Peter Xu, and Peter Shier):
> - Tested this revision by running
>   demand_paging_test
>   at each commit in the series on an Intel Haswell machine. Ran
>   demand_paging_test -u -v 8 -b 8M -d 10
>   on the same machine at the last commit in the series.
> - Readded partial aarch64 support, though aarch64 and s390 remain
>   untested
> - Implemented pipefd polling to reduce UFFD thread exit latency
> - Added variable unit input for memory size so users can pass command
>   line arguments of the form -b 24M instead of the raw number or bytes
> - Moved a missing break from a patch later in the series to an earlier
>   one
> - Moved to syncing per-vCPU global variables to guest and looking up
>   per-vcpu arguments based on a single CPU ID passed to each guest
>   vCPU. This allows for future patches to pass more than the supported
>   number of arguments for each arch to the vCPUs.
> - Implemented vcpu_args_set for s390 and aarch64 [UNTESTED]
> - Changed vm_create to always allocate memslot 0 at 4G instead of only
>   when the number of pages required is large.
> - Changed vcpu_wss to vcpu_memory_size for clarity.
> 
> Ben Gardon (10):
>   KVM: selftests: Create a demand paging test
>   KVM: selftests: Add demand paging content to the demand paging test
>   KVM: selftests: Add configurable demand paging delay
>   KVM: selftests: Add memory size parameter to the demand paging test
>   KVM: selftests: Pass args to vCPU in global vCPU args struct
>   KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x
>   KVM: selftests: Support multiple vCPUs in demand paging test
>   KVM: selftests: Time guest demand paging
>   KVM: selftests: Stop memslot creation in KVM internal memslot region
>   KVM: selftests: Move memslot 0 above KVM internal memslots
> 
>  tools/testing/selftests/kvm/.gitignore        |   1 +
>  tools/testing/selftests/kvm/Makefile          |   5 +-
>  .../selftests/kvm/demand_paging_test.c        | 680 ++++++++++++++++++
>  .../testing/selftests/kvm/include/test_util.h |   2 +
>  .../selftests/kvm/lib/aarch64/processor.c     |  33 +
>  tools/testing/selftests/kvm/lib/kvm_util.c    |  27 +-
>  .../selftests/kvm/lib/s390x/processor.c       |  35 +
>  tools/testing/selftests/kvm/lib/test_util.c   |  61 ++
>  8 files changed, 839 insertions(+), 5 deletions(-)
>  create mode 100644 tools/testing/selftests/kvm/demand_paging_test.c
>  create mode 100644 tools/testing/selftests/kvm/lib/test_util.c
> 

Queued patches 1-9, thanks.

Paolo

Re: [PATCH v4 06/10] KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x

[Andrew Jones]

On Fri, Jan 24, 2020 at 10:03:08AM +0100, Paolo Bonzini wrote:
> CCing Marc, Conny and Christian (plus Thomas and Drew who were already
> in the list) for review.
> 
> Thanks,
> 
> Paolo
> 
> On 23/01/20 19:04, Ben Gardon wrote:
> > Currently vcpu_args_set is only implemented for x86. This makes writing
> > tests with multiple vCPUs difficult as each guest vCPU must either a.)
> > do the same thing or b.) derive some kind of unique token from it's
> > registers or the architecture. To simplify the process of writing tests
> > with multiple vCPUs for s390 and aarch64, add set args functions for
> > those architectures.

It'd be nice to keep the separate architecture changes in separate
patches. Otherwise I can't really give an r-b.

> > 
> > Signed-off-by: Ben Gardon <bgardon@google.com>
> > ---
> >  .../selftests/kvm/lib/aarch64/processor.c     | 33 +++++++++++++++++
> >  .../selftests/kvm/lib/s390x/processor.c       | 35 +++++++++++++++++++
> >  2 files changed, 68 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> > index 86036a59a668e..a2ff90a75f326 100644
> > --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> > +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> > @@ -333,3 +333,36 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
> >  {
> >  	aarch64_vcpu_add_default(vm, vcpuid, NULL, guest_code);
> >  }
> > +
> > +/* VM VCPU Args Set
> > + *
> > + * Input Args:
> > + *   vm - Virtual Machine
> > + *   vcpuid - VCPU ID
> > + *   num - number of arguments
> > + *   ... - arguments, each of type uint64_t
> > + *
> > + * Output Args: None
> > + *
> > + * Return: None
> > + *
> > + * Sets the first num function input arguments to the values
> > + * given as variable args.  Each of the variable args is expected to
> > + * be of type uint64_t. The registers set by this function are r0-r7.
> > + */

lib/aarch64/processor.c so far doesn't have big function headers like
this. Also, since this function is common for all architectures [now],
I feel like the documentation should be in common code - so in the header
file.

> > +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
> > +{
> > +	va_list ap;
> > +
> > +	TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
> > +		    "  num: %u\n",
> > +		    num);

Weird line breaking. I see it came from the x86 implementation, but it's
weird there too... Personally I'd just put it all on one line, because
my vt100 died two decades ago.

> > +
> > +	va_start(ap, num);
> > +
> > +	for (i = 0; i < num; i++)
> > +		set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]),
                                                             ^^ should be 'i'

> > +			va_arg(ap, uint64_t));

nit: I'd use {} because of the line break. Or just not break the line and
bust the 80 char "limit" (RIP vt100).

Thanks,
drew

> > +
> > +	va_end(ap);
> > +}
> > diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c
> > index 32a02360b1eb0..680f37be9dbc9 100644
> > --- a/tools/testing/selftests/kvm/lib/s390x/processor.c
> > +++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
> > @@ -269,6 +269,41 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
> >  	run->psw_addr = (uintptr_t)guest_code;
> >  }
> >  
> > +/* VM VCPU Args Set
> > + *
> > + * Input Args:
> > + *   vm - Virtual Machine
> > + *   vcpuid - VCPU ID
> > + *   num - number of arguments
> > + *   ... - arguments, each of type uint64_t
> > + *
> > + * Output Args: None
> > + *
> > + * Return: None
> > + *
> > + * Sets the first num function input arguments to the values
> > + * given as variable args.  Each of the variable args is expected to
> > + * be of type uint64_t. The registers set by this function are r2-r6.
> > + */
> > +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
> > +{
> > +	va_list ap;
> > +	struct kvm_regs regs;
> > +
> > +	TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n"
> > +		    "  num: %u\n",
> > +		    num);
> > +
> > +	va_start(ap, num);
> > +	vcpu_regs_get(vm, vcpuid, &regs);
> > +
> > +	for (i = 0; i < num; i++)
> > +		regs.gprs[i + 2] = va_arg(ap, uint64_t);
> > +
> > +	vcpu_regs_set(vm, vcpuid, &regs);
> > +	va_end(ap);
> > +}
> > +
> >  void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent)
> >  {
> >  	struct vcpu *vcpu = vm->vcpu_head;
> > 
> 

Re: [PATCH v4 06/10] KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x

[Paolo Bonzini]

On 24/01/20 10:35, Andrew Jones wrote:
> On Fri, Jan 24, 2020 at 10:03:08AM +0100, Paolo Bonzini wrote:
>> CCing Marc, Conny and Christian (plus Thomas and Drew who were already
>> in the list) for review.
>>
>> Thanks,
>>
>> Paolo
>>
>> On 23/01/20 19:04, Ben Gardon wrote:
>>> Currently vcpu_args_set is only implemented for x86. This makes writing
>>> tests with multiple vCPUs difficult as each guest vCPU must either a.)
>>> do the same thing or b.) derive some kind of unique token from it's
>>> registers or the architecture. To simplify the process of writing tests
>>> with multiple vCPUs for s390 and aarch64, add set args functions for
>>> those architectures.
> 
> It'd be nice to keep the separate architecture changes in separate
> patches. Otherwise I can't really give an r-b.

You can say:

For ARM parts:

Reviewed-by: ...

Paolo

>>>
>>> Signed-off-by: Ben Gardon <bgardon@google.com>
>>> ---
>>>  .../selftests/kvm/lib/aarch64/processor.c     | 33 +++++++++++++++++
>>>  .../selftests/kvm/lib/s390x/processor.c       | 35 +++++++++++++++++++
>>>  2 files changed, 68 insertions(+)
>>>
>>> diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
>>> index 86036a59a668e..a2ff90a75f326 100644
>>> --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
>>> +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
>>> @@ -333,3 +333,36 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
>>>  {
>>>  	aarch64_vcpu_add_default(vm, vcpuid, NULL, guest_code);
>>>  }
>>> +
>>> +/* VM VCPU Args Set
>>> + *
>>> + * Input Args:
>>> + *   vm - Virtual Machine
>>> + *   vcpuid - VCPU ID
>>> + *   num - number of arguments
>>> + *   ... - arguments, each of type uint64_t
>>> + *
>>> + * Output Args: None
>>> + *
>>> + * Return: None
>>> + *
>>> + * Sets the first num function input arguments to the values
>>> + * given as variable args.  Each of the variable args is expected to
>>> + * be of type uint64_t. The registers set by this function are r0-r7.
>>> + */
> 
> lib/aarch64/processor.c so far doesn't have big function headers like
> this. Also, since this function is common for all architectures [now],
> I feel like the documentation should be in common code - so in the header
> file.
> 
>>> +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
>>> +{
>>> +	va_list ap;
>>> +
>>> +	TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
>>> +		    "  num: %u\n",
>>> +		    num);
> 
> Weird line breaking. I see it came from the x86 implementation, but it's
> weird there too... Personally I'd just put it all on one line, because
> my vt100 died two decades ago.
> 
>>> +
>>> +	va_start(ap, num);
>>> +
>>> +	for (i = 0; i < num; i++)
>>> +		set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]),
>                                                              ^^ should be 'i'
> 
>>> +			va_arg(ap, uint64_t));
> 
> nit: I'd use {} because of the line break. Or just not break the line and
> bust the 80 char "limit" (RIP vt100).
> 
> Thanks,
> drew
> 
>>> +
>>> +	va_end(ap);
>>> +}
>>> diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c
>>> index 32a02360b1eb0..680f37be9dbc9 100644
>>> --- a/tools/testing/selftests/kvm/lib/s390x/processor.c
>>> +++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
>>> @@ -269,6 +269,41 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
>>>  	run->psw_addr = (uintptr_t)guest_code;
>>>  }
>>>  
>>> +/* VM VCPU Args Set
>>> + *
>>> + * Input Args:
>>> + *   vm - Virtual Machine
>>> + *   vcpuid - VCPU ID
>>> + *   num - number of arguments
>>> + *   ... - arguments, each of type uint64_t
>>> + *
>>> + * Output Args: None
>>> + *
>>> + * Return: None
>>> + *
>>> + * Sets the first num function input arguments to the values
>>> + * given as variable args.  Each of the variable args is expected to
>>> + * be of type uint64_t. The registers set by this function are r2-r6.
>>> + */
>>> +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
>>> +{
>>> +	va_list ap;
>>> +	struct kvm_regs regs;
>>> +
>>> +	TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n"
>>> +		    "  num: %u\n",
>>> +		    num);
>>> +
>>> +	va_start(ap, num);
>>> +	vcpu_regs_get(vm, vcpuid, &regs);
>>> +
>>> +	for (i = 0; i < num; i++)
>>> +		regs.gprs[i + 2] = va_arg(ap, uint64_t);
>>> +
>>> +	vcpu_regs_set(vm, vcpuid, &regs);
>>> +	va_end(ap);
>>> +}
>>> +
>>>  void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent)
>>>  {
>>>  	struct vcpu *vcpu = vm->vcpu_head;
>>>
>>
> 

Re: [PATCH v4 01/10] KVM: selftests: Create a demand paging test

[Andrew Jones]

On Thu, Jan 23, 2020 at 10:04:27AM -0800, Ben Gardon wrote:
...
> +#ifdef __x86_64__
> +	vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
> +#endif
> +#ifdef __aarch64__
> +	ucall_init(vm, NULL);
> +#endif

We don't need the #ifdef around ucall_init anymore, now that architectures
that don't need it got stubs. I guess dirty_log_test.c could be cleaned up
too. I'll send a patch.

Thanks,
drew

Re: [PATCH v4 04/10] KVM: selftests: Add memory size parameter to the demand paging test

[Andrew Jones]

> diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> new file mode 100644
> index 0000000000000..706e0f963a44b
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/lib/test_util.c
> @@ -0,0 +1,61 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * tools/testing/selftests/kvm/lib/test_util.c
> + *
> + * Copyright (C) 2020, Google LLC.
> + */
> +
> +#include "test_util.h"
> +
> +#include <ctype.h>
> +
> +/*
> + * Parses "[0-9]+[kmgt]?".
> + */
> +size_t parse_size(const char *size)
> +{
> +	size_t len = strlen(size);
> +	size_t i;
> +	size_t scale_shift = 0;
> +	size_t base;
> +
> +	TEST_ASSERT(len > 0, "Need at least 1 digit in '%s'", size);
> +
> +	/* Find the first letter in the string, indicating scale. */
> +	for (i = 0; i < len; i++) {
> +		if (!isdigit(size[i])) {
> +			TEST_ASSERT(i > 0, "Need at least 1 digit in '%s'",
> +				    size);
> +			TEST_ASSERT(i == len - 1,
> +				    "Expected letter at the end in '%s'.",
> +				    size);
> +			switch (tolower(size[i])) {
> +			case 't':
> +				scale_shift = 40;
> +				break;
> +			case 'g':
> +				scale_shift = 30;
> +				break;
> +			case 'm':
> +				scale_shift = 20;
> +				break;
> +			case 'k':
> +				scale_shift = 10;
> +				break;
> +			default:
> +				TEST_ASSERT(false, "Unknown size letter %c",
> +					    size[i]);
> +			}
> +		}
> +	}
> +
> +	TEST_ASSERT(scale_shift < 8 * sizeof(size_t),
> +		    "Overflow parsing scale!");
> +
> +	base = atoi(size);

I'd use strtoull(size, NULL, 0), allowing the user to input full 0x...
sizes too. And, if the strtoull is done before the scale parsing, then
you could supply a non-null endptr and avoid the need for the for loop.

Thanks,
drew

Re: [PATCH v4 07/10] KVM: selftests: Support multiple vCPUs in demand paging test

[Andrew Jones]

On Thu, Jan 23, 2020 at 10:04:33AM -0800, Ben Gardon wrote:
> Most VMs have multiple vCPUs, the concurrent execution of which has a
> substantial impact on demand paging performance. Add an option to create
> multiple vCPUs to each access disjoint regions of memory.
> 
> Signed-off-by: Ben Gardon <bgardon@google.com>
> ---
>  .../selftests/kvm/demand_paging_test.c        | 255 ++++++++++++------
>  1 file changed, 172 insertions(+), 83 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
> index 9e2a5f7dfa140..2002032df32cc 100644
> --- a/tools/testing/selftests/kvm/demand_paging_test.c
> +++ b/tools/testing/selftests/kvm/demand_paging_test.c
> @@ -24,8 +24,6 @@
>  #include "kvm_util.h"
>  #include "processor.h"
>  
> -#define VCPU_ID				1
> -
>  /* The memory slot index demand page */
>  #define TEST_MEM_SLOT_INDEX		1
>  
> @@ -34,6 +32,14 @@
>  
>  #define DEFAULT_GUEST_TEST_MEM_SIZE (1 << 30) /* 1G */
>  
> +#ifdef PRINT_PER_VCPU_UPDATES
> +#define PER_VCPU_DEBUG(...) DEBUG(__VA_ARGS__)
> +#else
> +#define PER_VCPU_DEBUG(...)
> +#endif
> +
> +#define MAX_VCPUS 512
> +
>  /*
>   * Guest/Host shared variables. Ensure addr_gva2hva() and/or
>   * sync_global_to/from_guest() are used when accessing from
> @@ -67,18 +73,25 @@ struct vcpu_args {
>  	struct kvm_vm *vm;
>  };
>  
> -static struct vcpu_args vcpu_args;
> +static struct vcpu_args vcpu_args[MAX_VCPUS];
>  
>  /*
>   * Continuously write to the first 8 bytes of each page in the demand paging
>   * memory region.
>   */
> -static void guest_code(void)
> +static void guest_code(uint32_t vcpu_id)
>  {
> -	uint64_t gva = vcpu_args.gva;
> -	uint64_t pages = vcpu_args.pages;
> +	uint64_t gva;
> +	uint64_t pages;
>  	int i;
>  
> +	/* Return to signal error if vCPU args data structure is courrupt. */
> +	if (vcpu_args[vcpu_id].vcpu_id != vcpu_id)
> +		return;

This should be GUEST_ASSERT(vcpu_args[vcpu_id].vcpu_id == vcpu_id), which
will do a UCALL_ABORT when it fails. Otherwise we're returning to where?
Likely we'll get an exception of some sort when we return to nothing,
but that's not a very clean way to die.

Thanks,
drew

Re: [PATCH v4 08/10] KVM: selftests: Time guest demand paging

[Andrew Jones]

On Thu, Jan 23, 2020 at 10:04:34AM -0800, Ben Gardon wrote:
> In order to quantify demand paging performance, time guest execution
> during demand paging.
> 
> Signed-off-by: Ben Gardon <bgardon@google.com>
> ---
>  .../selftests/kvm/demand_paging_test.c        | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
> index 2002032df32cc..0dc5d04718678 100644
> --- a/tools/testing/selftests/kvm/demand_paging_test.c
> +++ b/tools/testing/selftests/kvm/demand_paging_test.c
> @@ -32,6 +32,12 @@
>  
>  #define DEFAULT_GUEST_TEST_MEM_SIZE (1 << 30) /* 1G */
>  
> +#ifdef PRINT_PER_PAGE_UPDATES
> +#define PER_PAGE_DEBUG(...) DEBUG(__VA_ARGS__)
> +#else
> +#define PER_PAGE_DEBUG(...)
> +#endif
> +
>  #ifdef PRINT_PER_VCPU_UPDATES
>  #define PER_VCPU_DEBUG(...) DEBUG(__VA_ARGS__)
>  #else
> @@ -64,6 +70,26 @@ static uint64_t guest_test_phys_mem;
>   */
>  static uint64_t guest_test_virt_mem = DEFAULT_GUEST_TEST_MEM;
>  
> +int64_t to_ns(struct timespec ts)
> +{
> +	return (int64_t)ts.tv_nsec + 1000000000LL * (int64_t)ts.tv_sec;
> +}
> +
> +struct timespec diff(struct timespec start, struct  timespec end)
> +{
> +	struct   timespec temp;
> +
> +	if ((end.tv_nsec-start.tv_nsec) < 0) {

spaces around the '-' would be nice

> +		temp.tv_sec = end.tv_sec - start.tv_sec - 1;
> +		temp.tv_nsec = 1000000000 + end.tv_nsec - start.tv_nsec;
> +	} else {
> +		temp.tv_sec = end.tv_sec - start.tv_sec;
> +		temp.tv_nsec = end.tv_nsec - start.tv_nsec;
> +	}
> +
> +	return temp;
> +}

Could probably add these new time utilities to the new test_util.c

Thanks,
drew

Re: [PATCH v4 06/10] KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x

[Andrew Jones]

On Fri, Jan 24, 2020 at 10:35:43AM +0100, Andrew Jones wrote:
> > > +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
> > > +{
> > > +	va_list ap;
> > > +
> > > +	TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
> > > +		    "  num: %u\n",
> > > +		    num);
> 
> Weird line breaking. I see it came from the x86 implementation, but it's
> weird there too... Personally I'd just put it all on one line, because
> my vt100 died two decades ago.
> 
> > > +
> > > +	va_start(ap, num);
> > > +
> > > +	for (i = 0; i < num; i++)
> > > +		set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]),
>                                                              ^^ should be 'i'

The declaration of 'i' is also missing.

> 
> > > +			va_arg(ap, uint64_t));
> 
> nit: I'd use {} because of the line break. Or just not break the line and
> bust the 80 char "limit" (RIP vt100).
>

Thanks,
drew 

Re: [PATCH v4 07/10] KVM: selftests: Support multiple vCPUs in demand paging test

[Andrew Jones]

On Thu, Jan 23, 2020 at 10:04:33AM -0800, Ben Gardon wrote:
...
> -static struct kvm_vm *create_vm(enum vm_guest_mode mode, uint32_t vcpuid,
> -				uint64_t extra_mem_pages, void *guest_code)
> +#define PAGE_SHIFT_4K  12
> +#define PTES_PER_4K_PT 512
> +
> +static struct kvm_vm *create_vm(enum vm_guest_mode mode, int vcpus,
> +				uint64_t vcpu_memory_bytes)
>  {
>  	struct kvm_vm *vm;
> -	uint64_t extra_pg_pages = extra_mem_pages / 512 * 2;
> +	uint64_t pages = DEFAULT_GUEST_PHY_PAGES;
> +
> +	/* Account for a few pages per-vCPU for stacks */
> +	pages += DEFAULT_STACK_PGS * vcpus;
> +
> +	/*
> +	 * Reserve twice the ammount of memory needed to map the test region and
> +	 * the page table / stacks region, at 4k, for page tables. Do the
> +	 * calculation with 4K page size: the smallest of all archs. (e.g., 64K
> +	 * page size guest will need even less memory for page tables).
> +	 */
> +	pages += (2 * pages) / PTES_PER_4K_PT;
> +	pages += ((2 * vcpus * vcpu_memory_bytes) >> PAGE_SHIFT_4K) /
> +		 PTES_PER_4K_PT;

pages needs to be rounded up to the next multiple of 16 in order for this
to work on aarch64 machines with 64k pages.

Thanks,
drew

Re: [PATCH v4 06/10] KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x

[Christian Borntraeger]

On 24.01.20 10:03, Paolo Bonzini wrote:
> CCing Marc, Conny and Christian (plus Thomas and Drew who were already
> in the list) for review.

CC Thomas Huth,

> 
> Thanks,
> 
> Paolo
> 
> On 23/01/20 19:04, Ben Gardon wrote:
>> Currently vcpu_args_set is only implemented for x86. This makes writing
>> tests with multiple vCPUs difficult as each guest vCPU must either a.)
>> do the same thing or b.) derive some kind of unique token from it's
>> registers or the architecture. To simplify the process of writing tests
>> with multiple vCPUs for s390 and aarch64, add set args functions for
>> those architectures.
[...]
>>  .../selftests/kvm/lib/s390x/processor.c       | 35 +++++++++++++++++++
[...]
>> --- a/tools/testing/selftests/kvm/lib/s390x/processor.c
>> +++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
>> @@ -269,6 +269,41 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
>>  	run->psw_addr = (uintptr_t)guest_code;
>>  }
>>  
>> +/* VM VCPU Args Set
>> + *
>> + * Input Args:
>> + *   vm - Virtual Machine
>> + *   vcpuid - VCPU ID
>> + *   num - number of arguments
>> + *   ... - arguments, each of type uint64_t
>> + *
>> + * Output Args: None
>> + *
>> + * Return: None
>> + *
>> + * Sets the first num function input arguments to the values
>> + * given as variable args.  Each of the variable args is expected to
>> + * be of type uint64_t. The registers set by this function are r2-r6.
>> + */
>> +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
>> +{
>> +	va_list ap;
>> +	struct kvm_regs regs;
>> +
>> +	TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n"
>> +		    "  num: %u\n",
>> +		    num);
>> +
>> +	va_start(ap, num);
>> +	vcpu_regs_get(vm, vcpuid, &regs);
>> +
>> +	for (i = 0; i < num; i++)
>> +		regs.gprs[i + 2] = va_arg(ap, uint64_t);
>> +
>> +	vcpu_regs_set(vm, vcpuid, &regs);
>> +	va_end(ap);
>> +}
>> +
>>  void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent)
>>  {
>>  	struct vcpu *vcpu = vm->vcpu_head;
>>
> 

Untested but the logic looks sane according to the ELF ABI. 

Re: [PATCH v4 09/10] KVM: selftests: Stop memslot creation in KVM internal memslot region

[Ben Gardon]

On Fri, Jan 24, 2020 at 12:58 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 23/01/20 19:04, Ben Gardon wrote:
> > KVM creates internal memslots covering the region between 3G and 4G in
> > the guest physical address space, when the first vCPU is created.
> > Mapping this region before creation of the first vCPU causes vCPU
> > creation to fail. Prohibit tests from creating such a memslot and fail
> > with a helpful warning when they try to.
> >
> > Signed-off-by: Ben Gardon <bgardon@google.com>
> > ---
>
> The internal memslots are much higher than this (0xfffbc000 and
> 0xfee00000).  I'm changing the patch to block 0xfe0000000 and above,
> otherwise it breaks vmx_dirty_log_test.

Perhaps we're working in different units, but I believe paddrs
0xfffbc000 and 0xfee00000 are between 3GiB and 4GiB.
"Proof by Python":
>>> B=1
>>> KB=1024*B
>>> MB=1024*KB
>>> GB=1024*MB
>>> hex(3*GB)
'0xc0000000'
>>> hex(4*GB)
'0x100000000'
>>> 3*GB == 3<<30
True
>>> 0xfffbc000 > 3*GB
True
>>> 0xfffbc000 < 4*GB
True
>>> 0xfee00000 > 3*GB
True
>>> 0xfee00000 < 4*GB
True

Am I missing something?

I don't think blocking 0xfe0000000 and above is useful, as there's
nothing mapped in that region and AFAIK it's perfectly valid to create
memslots there.


>
> Paolo
>

Re: [PATCH v4 10/10] KVM: selftests: Move memslot 0 above KVM internal memslots

[Ben Gardon]

On Fri, Jan 24, 2020 at 1:01 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 23/01/20 19:04, Ben Gardon wrote:
> > KVM creates internal memslots between 3 and 4 GiB paddrs on the first
> > vCPU creation. If memslot 0 is large enough it collides with these
> > memslots an causes vCPU creation to fail. Instead of creating memslot 0
> > at paddr 0, start it 4G into the guest physical address space.
> >
> > Signed-off-by: Ben Gardon <bgardon@google.com>
> > ---
> >  tools/testing/selftests/kvm/lib/kvm_util.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
>
> This breaks all tests for me:
>
>    $ ./state_test
>    Testing guest mode: PA-bits:ANY, VA-bits:48,  4K pages
>    Guest physical address width detected: 46
>    ==== Test Assertion Failure ====
>   lib/x86_64/processor.c:580: false
>   pid=4873 tid=4873 - Success
>      1  0x0000000000409996: addr_gva2gpa at processor.c:579
>      2  0x0000000000406a38: addr_gva2hva at kvm_util.c:1636
>      3  0x000000000041036c: kvm_vm_elf_load at elf.c:192
>      4  0x0000000000409ea9: vm_create_default at processor.c:829
>      5  0x0000000000400f6f: main at state_test.c:132
>      6  0x00007f21bdf90494: ?? ??:0
>      7  0x0000000000401287: _start at ??:?
>   No mapping for vm virtual address, gva: 0x400000

Uh oh, I obviously did not test this patch adequately. My apologies.
I'll send another version of this patch after I've had time to test it
better. The memslots between 3G and 4G are also somewhat x86 specific,
so maybe this code should be elsewhere.

>
> Memslot 0 should not be too large, so this patch should not be needed.

I found that 3GB was not sufficient for memslot zero in my testing
because it needs to contain both the stack for every vCPU and the page
tables for the VM. When I ran with 416 vCPUs and of 1.6TB of total
ram, memslot zero needed to be substantially larger than 3G. Just the
4K guest PTEs required to map 4G per-vCPU for 416 vCPUs require (((416
* (4<<30)) / 4096) * 8) / (1<<30) = 3.25GB of memory.
I suppose another slot could be used for the page tables, but that
would complicate the implementation of any tests that want to run
large VMs substantially.

>
> Paolo
>

Re: [PATCH v4 06/10] KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x

[Paolo Bonzini]

On 23/01/20 19:04, Ben Gardon wrote:
> Currently vcpu_args_set is only implemented for x86. This makes writing
> tests with multiple vCPUs difficult as each guest vCPU must either a.)
> do the same thing or b.) derive some kind of unique token from it's
> registers or the architecture. To simplify the process of writing tests
> with multiple vCPUs for s390 and aarch64, add set args functions for
> those architectures.
> 
> Signed-off-by: Ben Gardon <bgardon@google.com>
> ---
>  .../selftests/kvm/lib/aarch64/processor.c     | 33 +++++++++++++++++
>  .../selftests/kvm/lib/s390x/processor.c       | 35 +++++++++++++++++++
>  2 files changed, 68 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> index 86036a59a668e..a2ff90a75f326 100644
> --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> @@ -333,3 +333,36 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
>  {
>  	aarch64_vcpu_add_default(vm, vcpuid, NULL, guest_code);
>  }
> +
> +/* VM VCPU Args Set
> + *
> + * Input Args:
> + *   vm - Virtual Machine
> + *   vcpuid - VCPU ID
> + *   num - number of arguments
> + *   ... - arguments, each of type uint64_t
> + *
> + * Output Args: None
> + *
> + * Return: None
> + *
> + * Sets the first num function input arguments to the values
> + * given as variable args.  Each of the variable args is expected to
> + * be of type uint64_t. The registers set by this function are r0-r7.
> + */
> +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
> +{
> +	va_list ap;
> +
> +	TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
> +		    "  num: %u\n",
> +		    num);
> +
> +	va_start(ap, num);
> +
> +	for (i = 0; i < num; i++)
> +		set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]),
> +			va_arg(ap, uint64_t));
> +
> +	va_end(ap);
> +}
> diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c
> index 32a02360b1eb0..680f37be9dbc9 100644
> --- a/tools/testing/selftests/kvm/lib/s390x/processor.c
> +++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
> @@ -269,6 +269,41 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
>  	run->psw_addr = (uintptr_t)guest_code;
>  }
>  
> +/* VM VCPU Args Set
> + *
> + * Input Args:
> + *   vm - Virtual Machine
> + *   vcpuid - VCPU ID
> + *   num - number of arguments
> + *   ... - arguments, each of type uint64_t
> + *
> + * Output Args: None
> + *
> + * Return: None
> + *
> + * Sets the first num function input arguments to the values
> + * given as variable args.  Each of the variable args is expected to
> + * be of type uint64_t. The registers set by this function are r2-r6.
> + */
> +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
> +{
> +	va_list ap;
> +	struct kvm_regs regs;
> +
> +	TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n"
> +		    "  num: %u\n",
> +		    num);
> +
> +	va_start(ap, num);
> +	vcpu_regs_get(vm, vcpuid, &regs);
> +
> +	for (i = 0; i < num; i++)
> +		regs.gprs[i + 2] = va_arg(ap, uint64_t);
> +
> +	vcpu_regs_set(vm, vcpuid, &regs);
> +	va_end(ap);
> +}
> +
>  void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent)
>  {
>  	struct vcpu *vcpu = vm->vcpu_head;
> 

Squashing this:

diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index a2ff90a75f32..839a76c96f01 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -353,6 +353,7 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
 void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
 {
 	va_list ap;
+	int i;
 
 	TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
 		    "  num: %u\n",
diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c
index 680f37be9dbc..a0b84235c848 100644
--- a/tools/testing/selftests/kvm/lib/s390x/processor.c
+++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
@@ -289,6 +289,7 @@ void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
 {
 	va_list ap;
 	struct kvm_regs regs;
+	int i;
 
 	TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n"
 		    "  num: %u\n",

Paolo

Re: [PATCH v4 09/10] KVM: selftests: Stop memslot creation in KVM internal memslot region

[Paolo Bonzini]

On 24/01/20 19:41, Ben Gardon wrote:
> On Fri, Jan 24, 2020 at 12:58 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 23/01/20 19:04, Ben Gardon wrote:
>>> KVM creates internal memslots covering the region between 3G and 4G in
>>> the guest physical address space, when the first vCPU is created.
>>> Mapping this region before creation of the first vCPU causes vCPU
>>> creation to fail. Prohibit tests from creating such a memslot and fail
>>> with a helpful warning when they try to.
>>>
>>> Signed-off-by: Ben Gardon <bgardon@google.com>
>>> ---
>>
>> The internal memslots are much higher than this (0xfffbc000 and
>> 0xfee00000).  I'm changing the patch to block 0xfe0000000 and above,
>> otherwise it breaks vmx_dirty_log_test.
> 
> Perhaps we're working in different units, but I believe paddrs
> 0xfffbc000 and 0xfee00000 are between 3GiB and 4GiB.
> "Proof by Python":

I invoke the "not a native speaker" card.  Rephrasing: there is a large
part at the beginning of the area between 3GiB and 4GiB that isn't used
by internal memslot (but is used by vmx_dirty_log_test).

Though I have no excuse for the extra zero, the range to block is
0xfe000000 to 0x100000000.

Paolo

>>>> B=1
>>>> KB=1024*B
>>>> MB=1024*KB
>>>> GB=1024*MB
>>>> hex(3*GB)
> '0xc0000000'
>>>> hex(4*GB)
> '0x100000000'
>>>> 3*GB == 3<<30
> True
>>>> 0xfffbc000 > 3*GB
> True
>>>> 0xfffbc000 < 4*GB
> True
>>>> 0xfee00000 > 3*GB
> True
>>>> 0xfee00000 < 4*GB
> True
> 
> Am I missing something?
> 
> I don't think blocking 0xfe0000000 and above is useful, as there's
> nothing mapped in that region and AFAIK it's perfectly valid to create
> memslots there.
> 
> 
>>
>> Paolo
>>
> 

Re: [PATCH v4 07/10] KVM: selftests: Support multiple vCPUs in demand paging test

[Paolo Bonzini]

On 24/01/20 11:49, Andrew Jones wrote:
>> +
>> +	/*
>> +	 * Reserve twice the ammount of memory needed to map the test region and
>> +	 * the page table / stacks region, at 4k, for page tables. Do the
>> +	 * calculation with 4K page size: the smallest of all archs. (e.g., 64K
>> +	 * page size guest will need even less memory for page tables).
>> +	 */
>> +	pages += (2 * pages) / PTES_PER_4K_PT;
>> +	pages += ((2 * vcpus * vcpu_memory_bytes) >> PAGE_SHIFT_4K) /
>> +		 PTES_PER_4K_PT;
> pages needs to be rounded up to the next multiple of 16 in order for this
> to work on aarch64 machines with 64k pages.

I think this is best done with a generic function that does the rounding
and an arch-specific function that returns the page size.  Can you send
a patch to implement this?

Thanks,

Paolo

Re: [PATCH v4 06/10] KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x

[Andrew Jones]

On Sat, Jan 25, 2020 at 10:34:16AM +0100, Paolo Bonzini wrote:
> On 23/01/20 19:04, Ben Gardon wrote:
> > Currently vcpu_args_set is only implemented for x86. This makes writing
> > tests with multiple vCPUs difficult as each guest vCPU must either a.)
> > do the same thing or b.) derive some kind of unique token from it's
> > registers or the architecture. To simplify the process of writing tests
> > with multiple vCPUs for s390 and aarch64, add set args functions for
> > those architectures.
> > 
> > Signed-off-by: Ben Gardon <bgardon@google.com>
> > ---
> >  .../selftests/kvm/lib/aarch64/processor.c     | 33 +++++++++++++++++
> >  .../selftests/kvm/lib/s390x/processor.c       | 35 +++++++++++++++++++
> >  2 files changed, 68 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> > index 86036a59a668e..a2ff90a75f326 100644
> > --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> > +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> > @@ -333,3 +333,36 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
> >  {
> >  	aarch64_vcpu_add_default(vm, vcpuid, NULL, guest_code);
> >  }
> > +
> > +/* VM VCPU Args Set
> > + *
> > + * Input Args:
> > + *   vm - Virtual Machine
> > + *   vcpuid - VCPU ID
> > + *   num - number of arguments
> > + *   ... - arguments, each of type uint64_t
> > + *
> > + * Output Args: None
> > + *
> > + * Return: None
> > + *
> > + * Sets the first num function input arguments to the values
> > + * given as variable args.  Each of the variable args is expected to
> > + * be of type uint64_t. The registers set by this function are r0-r7.
> > + */
> > +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
> > +{
> > +	va_list ap;
> > +
> > +	TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
> > +		    "  num: %u\n",
> > +		    num);
> > +
> > +	va_start(ap, num);
> > +
> > +	for (i = 0; i < num; i++)
> > +		set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]),

For AArch64 you also need to squash s/num/i/ for this line.

(Plus I'm still not that pleased with adding this big header to this
file for this function, or the weird line breaks in the assert.)

> > +			va_arg(ap, uint64_t));
> > +
> > +	va_end(ap);
> > +}
> > diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c
> > index 32a02360b1eb0..680f37be9dbc9 100644
> > --- a/tools/testing/selftests/kvm/lib/s390x/processor.c
> > +++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
> > @@ -269,6 +269,41 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
> >  	run->psw_addr = (uintptr_t)guest_code;
> >  }
> >  
> > +/* VM VCPU Args Set
> > + *
> > + * Input Args:
> > + *   vm - Virtual Machine
> > + *   vcpuid - VCPU ID
> > + *   num - number of arguments
> > + *   ... - arguments, each of type uint64_t
> > + *
> > + * Output Args: None
> > + *
> > + * Return: None
> > + *
> > + * Sets the first num function input arguments to the values
> > + * given as variable args.  Each of the variable args is expected to
> > + * be of type uint64_t. The registers set by this function are r2-r6.
> > + */
> > +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
> > +{
> > +	va_list ap;
> > +	struct kvm_regs regs;
> > +
> > +	TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n"
> > +		    "  num: %u\n",
> > +		    num);
> > +
> > +	va_start(ap, num);
> > +	vcpu_regs_get(vm, vcpuid, &regs);
> > +
> > +	for (i = 0; i < num; i++)
> > +		regs.gprs[i + 2] = va_arg(ap, uint64_t);
> > +
> > +	vcpu_regs_set(vm, vcpuid, &regs);
> > +	va_end(ap);
> > +}
> > +
> >  void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent)
> >  {
> >  	struct vcpu *vcpu = vm->vcpu_head;
> > 
> 
> Squashing this:
> 
> diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> index a2ff90a75f32..839a76c96f01 100644
> --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> @@ -353,6 +353,7 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
>  void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
>  {
>  	va_list ap;
> +	int i;
>  
>  	TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
>  		    "  num: %u\n",
> diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c
> index 680f37be9dbc..a0b84235c848 100644
> --- a/tools/testing/selftests/kvm/lib/s390x/processor.c
> +++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
> @@ -289,6 +289,7 @@ void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
>  {
>  	va_list ap;
>  	struct kvm_regs regs;
> +	int i;
>  
>  	TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n"
>  		    "  num: %u\n",
> 
> Paolo
> 

Re: [PATCH v4 07/10] KVM: selftests: Support multiple vCPUs in demand paging test

[Andrew Jones]

On Sat, Jan 25, 2020 at 10:39:20AM +0100, Paolo Bonzini wrote:
> On 24/01/20 11:49, Andrew Jones wrote:
> >> +
> >> +	/*
> >> +	 * Reserve twice the ammount of memory needed to map the test region and
> >> +	 * the page table / stacks region, at 4k, for page tables. Do the
> >> +	 * calculation with 4K page size: the smallest of all archs. (e.g., 64K
> >> +	 * page size guest will need even less memory for page tables).
> >> +	 */
> >> +	pages += (2 * pages) / PTES_PER_4K_PT;
> >> +	pages += ((2 * vcpus * vcpu_memory_bytes) >> PAGE_SHIFT_4K) /
> >> +		 PTES_PER_4K_PT;
> > pages needs to be rounded up to the next multiple of 16 in order for this
> > to work on aarch64 machines with 64k pages.
> 
> I think this is best done with a generic function that does the rounding
> and an arch-specific function that returns the page size.  Can you send
> a patch to implement this?
>

Sure. Will do.

Thanks,
drew 

Re: [PATCH v4 06/10] KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x

[Thomas Huth]

On 23/01/2020 19.04, Ben Gardon wrote:
> Currently vcpu_args_set is only implemented for x86. This makes writing
> tests with multiple vCPUs difficult as each guest vCPU must either a.)
> do the same thing or b.) derive some kind of unique token from it's
> registers or the architecture. To simplify the process of writing tests
> with multiple vCPUs for s390 and aarch64, add set args functions for
> those architectures.
> 
> Signed-off-by: Ben Gardon <bgardon@google.com>
> ---
>  .../selftests/kvm/lib/aarch64/processor.c     | 33 +++++++++++++++++
>  .../selftests/kvm/lib/s390x/processor.c       | 35 +++++++++++++++++++
>  2 files changed, 68 insertions(+)
[...]
> diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c
> index 32a02360b1eb0..680f37be9dbc9 100644
> --- a/tools/testing/selftests/kvm/lib/s390x/processor.c
> +++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
> @@ -269,6 +269,41 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
>  	run->psw_addr = (uintptr_t)guest_code;
>  }
>  
> +/* VM VCPU Args Set
> + *
> + * Input Args:
> + *   vm - Virtual Machine
> + *   vcpuid - VCPU ID
> + *   num - number of arguments
> + *   ... - arguments, each of type uint64_t
> + *
> + * Output Args: None
> + *
> + * Return: None
> + *
> + * Sets the first num function input arguments to the values
> + * given as variable args.  Each of the variable args is expected to
> + * be of type uint64_t. The registers set by this function are r2-r6.
> + */
> +void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
> +{
> +	va_list ap;
> +	struct kvm_regs regs;

You need an "int i;" here to make it compile.

> +	TEST_ASSERT(num >= 1 && num <= 5, "Unsupported number of args,\n"

why the "\n" right in the middle of the string? Could you please make it
one-line only?

> +		    "  num: %u\n",
> +		    num);
> +
> +	va_start(ap, num);
> +	vcpu_regs_get(vm, vcpuid, &regs);
> +
> +	for (i = 0; i < num; i++)
> +		regs.gprs[i + 2] = va_arg(ap, uint64_t);
> +
> +	vcpu_regs_set(vm, vcpuid, &regs);
> +	va_end(ap);
> +}

... but apart from the nits, this looks basically sane to me.

 Thomas

Re: [PATCH v4 01/10] KVM: selftests: Create a demand paging test

[Thomas Huth]

On 23/01/2020 19.04, Ben Gardon wrote:
> While userfaultfd, KVM's demand paging implementation, is not specific
> to KVM, having a benchmark for its performance will be useful for
> guiding performance improvements to KVM. As a first step towards creating
> a userfaultfd demand paging test, create a simple memory access test,
> based on dirty_log_test.
> 
> Reviewed-by: Oliver Upton <oupton@google.com>
> Signed-off-by: Ben Gardon <bgardon@google.com>
> ---
>  tools/testing/selftests/kvm/.gitignore        |   1 +
>  tools/testing/selftests/kvm/Makefile          |   3 +
>  .../selftests/kvm/demand_paging_test.c        | 286 ++++++++++++++++++
>  3 files changed, 290 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/demand_paging_test.c
> 
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index 30072c3f52fbe..9619d96e15c41 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -17,3 +17,4 @@
>  /clear_dirty_log_test
>  /dirty_log_test
>  /kvm_create_max_vcpus
> +/demand_paging_test
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 3138a916574a9..e2e1b92faee3b 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -28,15 +28,18 @@ TEST_GEN_PROGS_x86_64 += x86_64/vmx_tsc_adjust_test
>  TEST_GEN_PROGS_x86_64 += x86_64/xss_msr_test
>  TEST_GEN_PROGS_x86_64 += clear_dirty_log_test
>  TEST_GEN_PROGS_x86_64 += dirty_log_test
> +TEST_GEN_PROGS_x86_64 += demand_paging_test
>  TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
>  
>  TEST_GEN_PROGS_aarch64 += clear_dirty_log_test
>  TEST_GEN_PROGS_aarch64 += dirty_log_test
> +TEST_GEN_PROGS_aarch64 += demand_paging_test
>  TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
>  
>  TEST_GEN_PROGS_s390x = s390x/memop
>  TEST_GEN_PROGS_s390x += s390x/sync_regs_test
>  TEST_GEN_PROGS_s390x += dirty_log_test
> +TEST_GEN_PROGS_s390x += demand_paging_test
>  TEST_GEN_PROGS_s390x += kvm_create_max_vcpus

I gave your series a quick try on s390x (without patch 10/10 since that
is causing more trouble), but the test does not work there yet:

# selftests: kvm: demand_paging_test
# ==== Test Assertion Failure ====
#   lib/kvm_util.c:700: ret == 0
#   pid=247240 tid=247240 - Invalid argument
#      1	0x0000000001004085: vm_userspace_mem_region_add at kvm_util.c:695
#      2	0x00000000010042dd: _vm_create at kvm_util.c:233
#      3	0x0000000001001b07: create_vm at demand_paging_test.c:185
#      4	 (inlined by) run_test at demand_paging_test.c:387
#      5	 (inlined by) main at demand_paging_test.c:676
#      6	0x000003ffb5323461: ?? ??:0
#      7	0x000000000100259d: .annobin_init.c.hot at crt1.o:?
#      8	0xffffffffffffffff: ?? ??:0
#   KVM_SET_USER_MEMORY_REGION IOCTL failed,
#   rc: -1 errno: 22
#   slot: 0 flags: 0x0
#   guest_phys_addr: 0x0 size: 0x607000
# Testing guest mode: PA-bits:40,  VA-bits:48,  4K pages
not ok 4 selftests: kvm: demand_paging_test # exit=254

I'd suggest to leave it disabled on s390x until the issue has been debugged.

 Thomas

Re: [PATCH v4 10/10] KVM: selftests: Move memslot 0 above KVM internal memslots

[Thomas Huth]

On 23/01/2020 19.04, Ben Gardon wrote:
> KVM creates internal memslots between 3 and 4 GiB paddrs on the first
> vCPU creation. If memslot 0 is large enough it collides with these
> memslots an causes vCPU creation to fail. Instead of creating memslot 0
> at paddr 0, start it 4G into the guest physical address space.
> 
> Signed-off-by: Ben Gardon <bgardon@google.com>
> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 5b971c04f1643..427c88d32e988 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -130,9 +130,11 @@ _Static_assert(sizeof(vm_guest_mode_string)/sizeof(char *) == NUM_VM_MODES,
>   *
>   * Creates a VM with the mode specified by mode (e.g. VM_MODE_P52V48_4K).
>   * When phy_pages is non-zero, a memory region of phy_pages physical pages
> - * is created and mapped starting at guest physical address 0.  The file
> - * descriptor to control the created VM is created with the permissions
> - * given by perm (e.g. O_RDWR).
> + * is created, starting at 4G into the guest physical address space to avoid
> + * KVM internal memslots which map the region between 3G and 4G. If tests need
> + * to use the physical region between 0 and 3G, they can allocate another
> + * memslot for that region. The file descriptor to control the created VM is
> + * created with the permissions given by perm (e.g. O_RDWR).
>   */
>  struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
>  {
> @@ -231,7 +233,8 @@ struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
>  	vm->vpages_mapped = sparsebit_alloc();
>  	if (phy_pages != 0)
>  		vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
> -					    0, 0, phy_pages, 0);
> +					    KVM_INTERNAL_MEMSLOTS_END_PADDR,
> +					    0, phy_pages, 0);
>  
>  	return vm;
>  }

This patch causes *all* tests on s390x to fail like this:

# selftests: kvm: sync_regs_test
# Testing guest mode: PA-bits:52,  VA-bits:48,  4K pages
# ==== Test Assertion Failure ====
#   lib/kvm_util.c:1059: false
#   pid=248244 tid=248244 - Success
#      1	0x0000000001002f3d: addr_gpa2hva at kvm_util.c:1059
#      2	 (inlined by) addr_gpa2hva at kvm_util.c:1047
#      3	0x0000000001006edf: addr_gva2gpa at processor.c:144
#      4	0x0000000001004345: addr_gva2hva at kvm_util.c:1636
#      5	0x00000000010077c1: kvm_vm_elf_load at elf.c:192
#      6	0x00000000010070c3: vm_create_default at processor.c:228
#      7	0x0000000001001347: main at sync_regs_test.c:87
#      8	0x000003ffba7a3461: ?? ??:0
#      9	0x0000000001001965: .annobin_init.c.hot at crt1.o:?
#     10	0xffffffffffffffff: ?? ??:0
#   No vm physical memory at 0x0
not ok 2 selftests: kvm: sync_regs_test # exit=254

AFAIK the ELF binaries on s390x are linked to addresses below 4G, so
generally removing the memslot here seems to be a bad idea on s390x.

 Thomas

Re: [PATCH v4 09/10] KVM: selftests: Stop memslot creation in KVM internal memslot region

[Ben Gardon]

On Sat, Jan 25, 2020 at 1:37 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 24/01/20 19:41, Ben Gardon wrote:
> > On Fri, Jan 24, 2020 at 12:58 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>
> >> On 23/01/20 19:04, Ben Gardon wrote:
> >>> KVM creates internal memslots covering the region between 3G and 4G in
> >>> the guest physical address space, when the first vCPU is created.
> >>> Mapping this region before creation of the first vCPU causes vCPU
> >>> creation to fail. Prohibit tests from creating such a memslot and fail
> >>> with a helpful warning when they try to.
> >>>
> >>> Signed-off-by: Ben Gardon <bgardon@google.com>
> >>> ---
> >>
> >> The internal memslots are much higher than this (0xfffbc000 and
> >> 0xfee00000).  I'm changing the patch to block 0xfe0000000 and above,
> >> otherwise it breaks vmx_dirty_log_test.
> >
> > Perhaps we're working in different units, but I believe paddrs
> > 0xfffbc000 and 0xfee00000 are between 3GiB and 4GiB.
> > "Proof by Python":
>
> I invoke the "not a native speaker" card.  Rephrasing: there is a large
> part at the beginning of the area between 3GiB and 4GiB that isn't used
> by internal memslot (but is used by vmx_dirty_log_test).

Ah, that makes perfect sense, thank you for clarifying. I think the
3G-4G in my head may have come from the x86 PCI hole or similar. In
any case, reducing the prohibited range to just the range covered by
internal memslots feels like a good change.

> Though I have no excuse for the extra zero, the range to block is
> 0xfe000000 to 0x100000000.
>
> Paolo
>
> >>>> B=1
> >>>> KB=1024*B
> >>>> MB=1024*KB
> >>>> GB=1024*MB
> >>>> hex(3*GB)
> > '0xc0000000'
> >>>> hex(4*GB)
> > '0x100000000'
> >>>> 3*GB == 3<<30
> > True
> >>>> 0xfffbc000 > 3*GB
> > True
> >>>> 0xfffbc000 < 4*GB
> > True
> >>>> 0xfee00000 > 3*GB
> > True
> >>>> 0xfee00000 < 4*GB
> > True
> >
> > Am I missing something?
> >
> > I don't think blocking 0xfe0000000 and above is useful, as there's
> > nothing mapped in that region and AFAIK it's perfectly valid to create
> > memslots there.
> >
> >
> >>
> >> Paolo
> >>
> >
>