All of lore.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests
@ 2017-04-21  0:49 David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 01/32] x86: add test filter to vmx.flat David Matlack
                   ` (33 more replies)
  0 siblings, 34 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: David Matlack

Patches 1-6 split out the VMX tests into their own unit tests.

Patches 7-12 add new VMX unit tests.

Patches 13-17 implement a new VMX test framework ("v2"). Instead of
registering a main function and an exit handler, entering and exiting
guest mode is accomplished by calling a function, enter_guest().

Patches 18-22 add some assert and printf enhancements to lib/.

Patches 23-32 add EPT access tests, using the v2 framework.

4 of the EPT access tests reveal KVM bugs (4.11-rc6, on a Haswell CPU):
	vmx_ept_access_test_execute_only
	vmx_ept_access_test_paddr_not_present_ad_disabled
	vmx_ept_access_test_paddr_read_only_ad_disabled
	vmx_ept_access_test_paddr_read_execute_ad_disabled

The rest of the new tests in this series pass.

David Matlack (8):
  x86: move basic vmx tests into separate test cases
  x86: add missing vmx test cases to unittests.cfg
  x86: vmx: filter exit_monitor_from_l2_test from full vmx test
  x86: basic vmwrite/vmread test
  x86: vmcs lifecycle test
  x86: test VMCS in memory after VMCLEAR
  x86: test VMPTRLD does not drop VMWRITEs
  lib: add report_pass

GanShun (1):
  x86: Adding gtest to check correct instruction error is returned

Peter Feiner (23):
  x86: add test filter to vmx.flat.
  x86: add config for each vmx unit test case
  lib: better test name filtering
  x86: test exit while vmcs02 is loaded
  x86: don't special case vmx null test
  x86: factor out vmx_enter_guest
  x86: binstr utility function
  x86: vmx exit reason descriptions
  x86: v2 vmx test framework
  lib: provide stdio.h
  lib: added assert_msg macro
  lib: fix *printf return value
  lib: printf-style report prefixes
  lib: add newline to assert_msg
  lib: x86: store free pages in ascending order
  lib: x86: multi-page allocator
  x86: ept assertions
  x86: setup_ept code motion
  x86: 2GiB RAM for vmx tests
  x86: ept capability utilities
  lib: x86: page table utilities
  x86: ept access tests
  x86: force ept 2m test

 lib/libcflat.h     |  21 ++
 lib/printf.c       |  31 +-
 lib/report.c       |  45 ++-
 lib/stdio.h        |   4 +
 lib/string.c       |  41 +++
 lib/x86/asm/page.h |   2 +
 lib/x86/vm.c       | 183 ++++++++--
 lib/x86/vm.h       |  28 ++
 x86/unittests.cfg  | 291 +++++++++++++++-
 x86/vmx.c          | 833 +++++++++++++++++++++++++++++++++++++++-----
 x86/vmx.h          | 151 +++++++-
 x86/vmx_tests.c    | 989 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 12 files changed, 2493 insertions(+), 126 deletions(-)
 create mode 100644 lib/stdio.h

-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 01/32] x86: add test filter to vmx.flat.
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 02/32] x86: add config for each vmx unit test case David Matlack
                   ` (32 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

The VMX nesting test has lots of test cases. Now a specific test case
can be run by appending its name to the multiboot command line (i.e.,
under Qemu's: -append <name>)

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/vmx.c | 39 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index b0b69602bdae..202cfaaa822e 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1158,9 +1158,37 @@ out:
 
 extern struct vmx_test vmx_tests[];
 
-int main(void)
+/* Match name with wanted allowing underscores in place of spaces. */
+static bool test_name_wanted(const char *name, const char *wanted)
+{
+	const char *n;
+	const char *w;
+
+	for (n = name, w = wanted; *n != '\0' && *w != '\0'; n++, w++) {
+		if (*n != *w && !(*n == ' ' && *w == '_'))
+			return false;
+	}
+	return *n == '\0' && *w == '\0';
+}
+
+static bool test_wanted(struct vmx_test *test, char *wanted[], int nwanted)
+{
+	int i;
+
+	if (!nwanted)
+		return true;
+
+	for (i = 0; i < nwanted; ++i) {
+		if (test_name_wanted(test->name, wanted[i]))
+			return true;
+	}
+	return false;
+}
+
+int main(int argc, char *argv[])
 {
 	int i = 0;
+	int matched = 0;
 
 	setup_vm();
 	setup_idt();
@@ -1188,9 +1216,16 @@ int main(void)
 	test_vmxoff();
 	test_vmx_caps();
 
-	while (vmx_tests[++i].name != NULL)
+	while (vmx_tests[++i].name != NULL) {
+		if (!test_wanted(&vmx_tests[i], argv + 1, argc - 1))
+			continue;
+		matched++;
 		if (test_run(&vmx_tests[i]))
 			goto exit;
+	}
+
+	if (!matched)
+		report("command line didn't match any tests!", matched);
 
 exit:
 	return report_summary();
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 02/32] x86: add config for each vmx unit test case
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 01/32] x86: add test filter to vmx.flat David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 03/32] x86: move basic vmx tests into separate test cases David Matlack
                   ` (31 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/unittests.cfg | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 x86/vmx_tests.c   |  4 +--
 2 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 54138388fd5d..a267638733aa 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -198,6 +198,91 @@ arch = x86_64
 file = vmx.flat
 extra_params = -cpu host,+vmx
 arch = x86_64
+groups = vmx
+
+[vmx_vmenter]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append vmenter
+arch = x86_64
+groups = vmx
+
+[vmx_preemption_timer]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append preemption_timer
+arch = x86_64
+groups = vmx
+
+[vmx_control_field_PAT]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append control_field_PAT
+arch = x86_64
+groups = vmx
+
+[vmx_control_field_EFER]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append control_field_EFER
+arch = x86_64
+groups = vmx
+
+[vmx_CR_shadowing]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append CR_shadowing
+arch = x86_64
+groups = vmx
+
+[vmx_IO_bitmap]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append I/O_bitmap
+arch = x86_64
+groups = vmx
+
+[vmx_instruction_intercept]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append instruction_intercept
+arch = x86_64
+groups = vmx
+
+[vmx_EPT_AD_enabled]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append EPT_A/D_enabled
+arch = x86_64
+groups = vmx
+
+[vmx_EPT_AD_disabled]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append EPT_A/D_disabled
+arch = x86_64
+groups = vmx
+
+[vmx_VPID]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append VPID
+arch = x86_64
+groups = vmx
+
+[vmx_interrupt]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append interrupt
+arch = x86_64
+groups = vmx
+
+[vmx_debug_controls]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append debug_controls
+arch = x86_64
+groups = vmx
+
+[vmx_MSR_switch]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append MSR_switch
+arch = x86_64
+groups = vmx
+
+[vmx_vmmcall]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append vmmcall
+arch = x86_64
+groups = vmx
 
 [debug]
 file = debug.flat
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index b9115637915c..cd18b133a3a3 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1900,8 +1900,8 @@ struct vmx_test vmx_tests[] = {
 		NULL, {0} },
 	{ "instruction intercept", insn_intercept_init, insn_intercept_main,
 		insn_intercept_exit_handler, NULL, {0} },
-	{ "EPT, A/D disabled", ept_init, ept_main, ept_exit_handler, NULL, {0} },
-	{ "EPT, A/D enabled", eptad_init, eptad_main, eptad_exit_handler, NULL, {0} },
+	{ "EPT A/D disabled", ept_init, ept_main, ept_exit_handler, NULL, {0} },
+	{ "EPT A/D enabled", eptad_init, eptad_main, eptad_exit_handler, NULL, {0} },
 	{ "VPID", vpid_init, vpid_main, vpid_exit_handler, NULL, {0} },
 	{ "interrupt", interrupt_init, interrupt_main,
 		interrupt_exit_handler, NULL, {0} },
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 03/32] x86: move basic vmx tests into separate test cases
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 01/32] x86: add test filter to vmx.flat David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 02/32] x86: add config for each vmx unit test case David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 04/32] x86: add missing vmx test cases to unittests.cfg David Matlack
                   ` (30 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: David Matlack

By default every vmx kvm-unit-test runs a series of tests at startup.
This patch moves those startup tests into test cases which are only run
when requested.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/unittests.cfg | 36 +++++++++++++++++++++++++++++++
 x86/vmx.c         | 64 +++++++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 84 insertions(+), 16 deletions(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index a267638733aa..c942adad6325 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -200,6 +200,42 @@ extra_params = -cpu host,+vmx
 arch = x86_64
 groups = vmx
 
+[vmx_test_vmx_feature_control]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append test_vmx_feature_control
+arch = x86_64
+groups = vmx
+
+[vmx_test_vmxon]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append test_vmxon
+arch = x86_64
+groups = vmx
+
+[vmx_test_vmptrld]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append test_vmptrld
+arch = x86_64
+groups = vmx
+
+[vmx_test_vmclear]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append test_vmclear
+arch = x86_64
+groups = vmx
+
+[vmx_test_vmptrst]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append test_vmptrst
+arch = x86_64
+groups = vmx
+
+[vmx_test_vmx_caps]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append test_vmx_caps
+arch = x86_64
+groups = vmx
+
 [vmx_vmenter]
 file = vmx.flat
 extra_params = -cpu host,+vmx -append vmenter
diff --git a/x86/vmx.c b/x86/vmx.c
index 202cfaaa822e..0590b69ec1d2 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -45,6 +45,7 @@ struct regs regs;
 struct vmx_test *current;
 u64 hypercall_field;
 bool launched;
+static int matched;
 
 union vmx_basic basic;
 union vmx_ctrl_msr ctrl_pin_rev;
@@ -1171,55 +1172,86 @@ static bool test_name_wanted(const char *name, const char *wanted)
 	return *n == '\0' && *w == '\0';
 }
 
-static bool test_wanted(struct vmx_test *test, char *wanted[], int nwanted)
+static bool test_wanted(const char *name, char *wanted[], int nwanted)
 {
+	bool is_wanted = true;
 	int i;
 
 	if (!nwanted)
-		return true;
+		goto out;
 
 	for (i = 0; i < nwanted; ++i) {
-		if (test_name_wanted(test->name, wanted[i]))
-			return true;
+		if (test_name_wanted(name, wanted[i]))
+			goto out;
 	}
-	return false;
+
+	is_wanted = false;
+
+out:
+	if (is_wanted)
+		matched++;
+	return is_wanted;
 }
 
 int main(int argc, char *argv[])
 {
 	int i = 0;
-	int matched = 0;
 
 	setup_vm();
 	setup_idt();
 	hypercall_field = 0;
 
+	argv++;
+	argc--;
+
 	if (!(cpuid(1).c & (1 << 5))) {
 		printf("WARNING: vmx not supported, add '-cpu host'\n");
 		goto exit;
 	}
 	init_vmx();
-	if (test_vmx_feature_control() != 0)
-		goto exit;
+	if (test_wanted("test_vmx_feature_control", argv, argc)) {
+		/* Sets MSR_IA32_FEATURE_CONTROL to 0x5 */
+		if (test_vmx_feature_control() != 0)
+			goto exit;
+	} else {
+		if ((rdmsr(MSR_IA32_FEATURE_CONTROL) & 0x5) != 0x5)
+			wrmsr(MSR_IA32_FEATURE_CONTROL, 0x5);
+	}
+
 	/* Set basic test ctxt the same as "null" */
 	current = &vmx_tests[0];
-	if (test_vmxon() != 0)
-		goto exit;
-	test_vmptrld();
-	test_vmclear();
-	test_vmptrst();
+
+	if (test_wanted("test_vmxon", argv, argc)) {
+		/* Enables VMX */
+		if (test_vmxon() != 0)
+			goto exit;
+	} else {
+		if (vmx_on()) {
+			report("vmxon", 0);
+			goto exit;
+		}
+	}
+
+	if (test_wanted("test_vmptrld", argv, argc))
+		test_vmptrld();
+	if (test_wanted("test_vmclear", argv, argc))
+		test_vmclear();
+	if (test_wanted("test_vmptrst", argv, argc))
+		test_vmptrst();
 	init_vmcs(&vmcs_root);
 	if (vmx_run()) {
 		report("test vmlaunch", 0);
 		goto exit;
 	}
+
 	test_vmxoff();
-	test_vmx_caps();
+
+	if (test_wanted("test_vmx_caps", argv, argc))
+		test_vmx_caps();
 
 	while (vmx_tests[++i].name != NULL) {
-		if (!test_wanted(&vmx_tests[i], argv + 1, argc - 1))
+		if (!test_wanted(vmx_tests[i].name, argv, argc))
 			continue;
-		matched++;
 		if (test_run(&vmx_tests[i]))
 			goto exit;
 	}
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 04/32] x86: add missing vmx test cases to unittests.cfg
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (2 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 03/32] x86: move basic vmx tests into separate test cases David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 05/32] lib: better test name filtering David Matlack
                   ` (29 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: David Matlack

Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/unittests.cfg | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index c942adad6325..02dd6b235739 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -320,6 +320,30 @@ extra_params = -cpu host,+vmx -append vmmcall
 arch = x86_64
 groups = vmx
 
+[vmx_disable_RDTSCP]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append disable_RDTSCP
+arch = x86_64
+groups = vmx
+
+[vmx_int3]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append int3
+arch = x86_64
+groups = vmx
+
+[vmx_into]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append into
+arch = x86_64
+groups = vmx
+
+[vmx_exit_monitor_from_l2_test]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append exit_monitor_from_l2_test
+arch = x86_64
+groups = vmx
+
 [debug]
 file = debug.flat
 arch = x86_64
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 05/32] lib: better test name filtering
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (3 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 04/32] x86: add missing vmx test cases to unittests.cfg David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 06/32] x86: vmx: filter exit_monitor_from_l2_test from full vmx test David Matlack
                   ` (28 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Limited glob matching with leading or trailing '*'.

Tests (and globs of tests) can be excluded by prepending with a '-'.

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 lib/libcflat.h |  2 ++
 lib/string.c   | 41 +++++++++++++++++++++++++++++++++++++++++
 x86/vmx.c      | 52 +++++++++++++++++++++++++++-------------------------
 3 files changed, 70 insertions(+), 25 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 96a37926f302..248fd023626e 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -105,6 +105,8 @@ extern void report_skip(const char *msg_fmt, ...);
 extern void report_info(const char *msg_fmt, ...);
 extern int report_summary(void);
 
+bool simple_glob(const char *text, const char *pattern);
+
 extern void dump_stack(void);
 extern void dump_frame_stack(const void *instruction, const void *frame);
 
diff --git a/lib/string.c b/lib/string.c
index 833f22be48c5..bc52d5c2695f 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -173,3 +173,44 @@ char *getenv(const char *name)
     }
     return NULL;
 }
+
+/* Very simple glob matching. Allows '*' at beginning and end of pattern. */
+bool simple_glob(const char *text, const char *pattern)
+{
+	bool star_start = false;
+	bool star_end = false;
+	size_t n = strlen(pattern);
+	char copy[n + 1];
+
+	if (pattern[0] == '*') {
+		pattern += 1;
+		n -= 1;
+		star_start = true;
+	}
+
+	strcpy(copy, pattern);
+
+	if (n > 0 && pattern[n - 1] == '*') {
+		n -= 1;
+		copy[n] = '\0';
+		star_end = true;
+	}
+
+	if (star_start && star_end)
+		return strstr(text, copy);
+
+	if (star_end)
+		return strstr(text, copy) == text;
+
+	if (star_start) {
+		size_t text_len = strlen(text);
+		const char *suffix;
+
+		if (n > text_len)
+			return false;
+		suffix = text + text_len - n;
+		return !strcmp(suffix, copy);
+	}
+
+	return !strcmp(text, copy);
+}
diff --git a/x86/vmx.c b/x86/vmx.c
index 0590b69ec1d2..651807a76a1e 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1159,41 +1159,43 @@ out:
 
 extern struct vmx_test vmx_tests[];
 
-/* Match name with wanted allowing underscores in place of spaces. */
-static bool test_name_wanted(const char *name, const char *wanted)
+static bool
+test_wanted(const char *name, const char *filters[], int filter_count)
 {
+	int i;
+	bool positive = false;
+	bool match = false;
+	char clean_name[strlen(name) + 1];
+	char *c;
 	const char *n;
-	const char *w;
 
-	for (n = name, w = wanted; *n != '\0' && *w != '\0'; n++, w++) {
-		if (*n != *w && !(*n == ' ' && *w == '_'))
-			return false;
-	}
-	return *n == '\0' && *w == '\0';
-}
+	/* Replace spaces with underscores. */
+	n = name;
+	c = &clean_name[0];
+	do *c++ = (*n == ' ') ? '_' : *n;
+	while (*n++);
 
-static bool test_wanted(const char *name, char *wanted[], int nwanted)
-{
-	bool is_wanted = true;
-	int i;
-
-	if (!nwanted)
-		goto out;
+	for (i = 0; i < filter_count; i++) {
+		const char *filter = filters[i];
 
-	for (i = 0; i < nwanted; ++i) {
-		if (test_name_wanted(name, wanted[i]))
-			goto out;
+		if (filter[0] == '-') {
+			if (simple_glob(clean_name, filter + 1))
+				return false;
+		} else {
+			positive = true;
+			match |= simple_glob(clean_name, filter);
+		}
 	}
 
-	is_wanted = false;
-
-out:
-	if (is_wanted)
+	if (!positive || match) {
 		matched++;
-	return is_wanted;
+		return true;
+	} else {
+		return false;
+	}
 }
 
-int main(int argc, char *argv[])
+int main(int argc, const char *argv[])
 {
 	int i = 0;
 
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 06/32] x86: vmx: filter exit_monitor_from_l2_test from full vmx test
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (4 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 05/32] lib: better test name filtering David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  8:55   ` Paolo Bonzini
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 07/32] x86: test exit while vmcs02 is loaded David Matlack
                   ` (27 subsequent siblings)
  33 siblings, 1 reply; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: David Matlack

Filter out exit_monitor_from_l2_test when running the full vmx test
suite (i.e. the "[vmx]" test suite in unittest.cfg). Since
exit_monitor_from_l2_test causes the VM to shutdown, many of the tests
in vmx_tests.c end up getting skipped.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/unittests.cfg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 02dd6b235739..8011429d2307 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -196,7 +196,7 @@ arch = x86_64
 
 [vmx]
 file = vmx.flat
-extra_params = -cpu host,+vmx
+extra_params = -cpu host,+vmx -append -exit_monitor_from_l2_test
 arch = x86_64
 groups = vmx
 
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 07/32] x86: test exit while vmcs02 is loaded
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (5 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 06/32] x86: vmx: filter exit_monitor_from_l2_test from full vmx test David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 08/32] x86: Adding gtest to check correct instruction error is returned David Matlack
                   ` (26 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Before applying "KVM: nVMX: fix lifetime issues for vmcs0", exiting while
vmcs02 was loaded would trigger a VM_BUG_ON.

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/vmx_tests.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index cd18b133a3a3..35af67744971 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1884,6 +1884,18 @@ int into_exit_handler()
 	return VMX_TEST_VMEXIT;
 }
 
+static void exit_monitor_from_l2_main(void)
+{
+	printf("Calling exit(0) from l2...\n");
+	exit(0);
+}
+
+static int exit_monitor_from_l2_handler(void)
+{
+	report("The guest should have killed the VMM", false);
+	return VMX_TEST_EXIT;
+}
+
 /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
 struct vmx_test vmx_tests[] = {
 	{ "null", NULL, basic_guest_main, basic_exit_handler, NULL, {0} },
@@ -1914,5 +1926,7 @@ struct vmx_test vmx_tests[] = {
 		disable_rdtscp_exit_handler, NULL, {0} },
 	{ "int3", int3_init, int3_guest_main, int3_exit_handler, NULL, {0} },
 	{ "into", into_init, into_guest_main, into_exit_handler, NULL, {0} },
+	{ "exit_monitor_from_l2_test", NULL, exit_monitor_from_l2_main,
+		exit_monitor_from_l2_handler, NULL, {0} },
 	{ NULL, NULL, NULL, NULL, NULL, {0} },
 };
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 08/32] x86: Adding gtest to check correct instruction error is returned
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (6 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 07/32] x86: test exit while vmcs02 is loaded David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 09/32] x86: basic vmwrite/vmread test David Matlack
                   ` (25 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: GanShun, David Matlack

From: GanShun <ganshun@google.com>

Added a check in test_vmptrld to verfiy that VMXERR_VMPTRLD_VMXON_POINTER
is returned instead of VMXERR_VMCLEAR_VMXON_POINTER.

Signed-off-by: GanShun <ganshun@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/vmx.c |  3 +++
 x86/vmx.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/x86/vmx.c b/x86/vmx.c
index 651807a76a1e..39a891da4635 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -899,9 +899,12 @@ static void test_vmptrld(void)
 	       make_vmcs_current(tmp_root) == 1);
 
 	/* Pass VMXON region */
+	make_vmcs_current(vmcs);
 	tmp_root = (struct vmcs *)vmxon_region;
 	report("test vmptrld with vmxon region",
 	       make_vmcs_current(tmp_root) == 1);
+	report("test vmptrld with vmxon region vm-instruction error",
+	       vmcs_read(VMX_INST_ERROR) == VMXERR_VMPTRLD_VMXON_POINTER);
 
 	report("test vmptrld with valid vmcs region", make_vmcs_current(vmcs) == 0);
 }
diff --git a/x86/vmx.h b/x86/vmx.h
index 290e6bd72b19..52ece1aa53c8 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -396,6 +396,37 @@ enum Intr_type {
 
 #define INTR_INFO_INTR_TYPE_SHIFT       8
 
+/*
+ * VM-instruction error numbers
+ */
+enum vm_instruction_error_number {
+	VMXERR_VMCALL_IN_VMX_ROOT_OPERATION = 1,
+	VMXERR_VMCLEAR_INVALID_ADDRESS = 2,
+	VMXERR_VMCLEAR_VMXON_POINTER = 3,
+	VMXERR_VMLAUNCH_NONCLEAR_VMCS = 4,
+	VMXERR_VMRESUME_NONLAUNCHED_VMCS = 5,
+	VMXERR_VMRESUME_AFTER_VMXOFF = 6,
+	VMXERR_ENTRY_INVALID_CONTROL_FIELD = 7,
+	VMXERR_ENTRY_INVALID_HOST_STATE_FIELD = 8,
+	VMXERR_VMPTRLD_INVALID_ADDRESS = 9,
+	VMXERR_VMPTRLD_VMXON_POINTER = 10,
+	VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID = 11,
+	VMXERR_UNSUPPORTED_VMCS_COMPONENT = 12,
+	VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT = 13,
+	VMXERR_VMXON_IN_VMX_ROOT_OPERATION = 15,
+	VMXERR_ENTRY_INVALID_EXECUTIVE_VMCS_POINTER = 16,
+	VMXERR_ENTRY_NONLAUNCHED_EXECUTIVE_VMCS = 17,
+	VMXERR_ENTRY_EXECUTIVE_VMCS_POINTER_NOT_VMXON_POINTER = 18,
+	VMXERR_VMCALL_NONCLEAR_VMCS = 19,
+	VMXERR_VMCALL_INVALID_VM_EXIT_CONTROL_FIELDS = 20,
+	VMXERR_VMCALL_INCORRECT_MSEG_REVISION_ID = 22,
+	VMXERR_VMXOFF_UNDER_DUAL_MONITOR_TREATMENT_OF_SMIS_AND_SMM = 23,
+	VMXERR_VMCALL_INVALID_SMM_MONITOR_FEATURES = 24,
+	VMXERR_ENTRY_INVALID_VM_EXECUTION_CONTROL_FIELDS_IN_EXECUTIVE_VMCS = 25,
+	VMXERR_ENTRY_EVENTS_BLOCKED_BY_MOV_SS = 26,
+	VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID = 28,
+};
+
 #define SAVE_GPR				\
 	"xchg %rax, regs\n\t"			\
 	"xchg %rbx, regs+0x8\n\t"		\
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 09/32] x86: basic vmwrite/vmread test
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (7 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 08/32] x86: Adding gtest to check correct instruction error is returned David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 10/32] x86: vmcs lifecycle test David Matlack
                   ` (24 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: David Matlack

Issues VMWRITE to every VMCS field and the checks that VMREAD returns
the expected result. Some tricky cases: read-only fields (skipped),
not-yet-implemented fields (skipped, VMREAD fails with VMfailValid),
guest segment access rights fields (reserved bits are zeroed by the
CPU, so only check non-reserved bits).

Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/unittests.cfg |   6 ++
 x86/vmx.c         | 250 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 x86/vmx.h         |  29 +++++++
 3 files changed, 275 insertions(+), 10 deletions(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 8011429d2307..7973f2f62d26 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -230,6 +230,12 @@ extra_params = -cpu host,+vmx -append test_vmptrst
 arch = x86_64
 groups = vmx
 
+[vmx_test_vmwrite_vmread]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append test_vmwrite_vmread
+arch = x86_64
+groups = vmx
+
 [vmx_test_vmx_caps]
 file = vmx.flat
 extra_params = -cpu host,+vmx -append test_vmx_caps
diff --git a/x86/vmx.c b/x86/vmx.c
index 39a891da4635..47404fbbd782 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -63,6 +63,243 @@ extern void *guest_entry;
 
 static volatile u32 stage;
 
+struct vmcs_field {
+	u64 mask;
+	u64 encoding;
+};
+
+#define MASK(_bits) GENMASK_ULL((_bits) - 1, 0)
+#define MASK_NATURAL MASK(sizeof(unsigned long) * 8)
+
+static struct vmcs_field vmcs_fields[] = {
+	{ MASK(16), VPID },
+	{ MASK(16), PINV },
+	{ MASK(16), EPTP_IDX },
+
+	{ MASK(16), GUEST_SEL_ES },
+	{ MASK(16), GUEST_SEL_CS },
+	{ MASK(16), GUEST_SEL_SS },
+	{ MASK(16), GUEST_SEL_DS },
+	{ MASK(16), GUEST_SEL_FS },
+	{ MASK(16), GUEST_SEL_GS },
+	{ MASK(16), GUEST_SEL_LDTR },
+	{ MASK(16), GUEST_SEL_TR },
+	{ MASK(16), GUEST_INT_STATUS },
+
+	{ MASK(16), HOST_SEL_ES },
+	{ MASK(16), HOST_SEL_CS },
+	{ MASK(16), HOST_SEL_SS },
+	{ MASK(16), HOST_SEL_DS },
+	{ MASK(16), HOST_SEL_FS },
+	{ MASK(16), HOST_SEL_GS },
+	{ MASK(16), HOST_SEL_TR },
+
+	{ MASK(64), IO_BITMAP_A },
+	{ MASK(64), IO_BITMAP_B },
+	{ MASK(64), MSR_BITMAP },
+	{ MASK(64), EXIT_MSR_ST_ADDR },
+	{ MASK(64), EXIT_MSR_LD_ADDR },
+	{ MASK(64), ENTER_MSR_LD_ADDR },
+	{ MASK(64), VMCS_EXEC_PTR },
+	{ MASK(64), TSC_OFFSET },
+	{ MASK(64), APIC_VIRT_ADDR },
+	{ MASK(64), APIC_ACCS_ADDR },
+	{ MASK(64), EPTP },
+
+	{ 0 /* read-only */, INFO_PHYS_ADDR },
+
+	{ MASK(64), VMCS_LINK_PTR },
+	{ MASK(64), GUEST_DEBUGCTL },
+	{ MASK(64), GUEST_EFER },
+	{ MASK(64), GUEST_PAT },
+	{ MASK(64), GUEST_PERF_GLOBAL_CTRL },
+	{ MASK(64), GUEST_PDPTE },
+
+	{ MASK(64), HOST_PAT },
+	{ MASK(64), HOST_EFER },
+	{ MASK(64), HOST_PERF_GLOBAL_CTRL },
+
+	{ MASK(32), PIN_CONTROLS },
+	{ MASK(32), CPU_EXEC_CTRL0 },
+	{ MASK(32), EXC_BITMAP },
+	{ MASK(32), PF_ERROR_MASK },
+	{ MASK(32), PF_ERROR_MATCH },
+	{ MASK(32), CR3_TARGET_COUNT },
+	{ MASK(32), EXI_CONTROLS },
+	{ MASK(32), EXI_MSR_ST_CNT },
+	{ MASK(32), EXI_MSR_LD_CNT },
+	{ MASK(32), ENT_CONTROLS },
+	{ MASK(32), ENT_MSR_LD_CNT },
+	{ MASK(32), ENT_INTR_INFO },
+	{ MASK(32), ENT_INTR_ERROR },
+	{ MASK(32), ENT_INST_LEN },
+	{ MASK(32), TPR_THRESHOLD },
+	{ MASK(32), CPU_EXEC_CTRL1 },
+
+	{ 0 /* read-only */, VMX_INST_ERROR },
+	{ 0 /* read-only */, EXI_REASON },
+	{ 0 /* read-only */, EXI_INTR_INFO },
+	{ 0 /* read-only */, EXI_INTR_ERROR },
+	{ 0 /* read-only */, IDT_VECT_INFO },
+	{ 0 /* read-only */, IDT_VECT_ERROR },
+	{ 0 /* read-only */, EXI_INST_LEN },
+	{ 0 /* read-only */, EXI_INST_INFO },
+
+	{ MASK(32), GUEST_LIMIT_ES },
+	{ MASK(32), GUEST_LIMIT_CS },
+	{ MASK(32), GUEST_LIMIT_SS },
+	{ MASK(32), GUEST_LIMIT_DS },
+	{ MASK(32), GUEST_LIMIT_FS },
+	{ MASK(32), GUEST_LIMIT_GS },
+	{ MASK(32), GUEST_LIMIT_LDTR },
+	{ MASK(32), GUEST_LIMIT_TR },
+	{ MASK(32), GUEST_LIMIT_GDTR },
+	{ MASK(32), GUEST_LIMIT_IDTR },
+	{ 0x1d0ff, GUEST_AR_ES },
+	{ 0x1f0ff, GUEST_AR_CS },
+	{ 0x1d0ff, GUEST_AR_SS },
+	{ 0x1d0ff, GUEST_AR_DS },
+	{ 0x1d0ff, GUEST_AR_FS },
+	{ 0x1d0ff, GUEST_AR_GS },
+	{ 0x1d0ff, GUEST_AR_LDTR },
+	{ 0x1d0ff, GUEST_AR_TR },
+	{ MASK(32), GUEST_INTR_STATE },
+	{ MASK(32), GUEST_ACTV_STATE },
+	{ MASK(32), GUEST_SMBASE },
+	{ MASK(32), GUEST_SYSENTER_CS },
+	{ MASK(32), PREEMPT_TIMER_VALUE },
+
+	{ MASK(32), HOST_SYSENTER_CS },
+
+	{ MASK_NATURAL, CR0_MASK },
+	{ MASK_NATURAL, CR4_MASK },
+	{ MASK_NATURAL, CR0_READ_SHADOW },
+	{ MASK_NATURAL, CR4_READ_SHADOW },
+	{ MASK_NATURAL, CR3_TARGET_0 },
+	{ MASK_NATURAL, CR3_TARGET_1 },
+	{ MASK_NATURAL, CR3_TARGET_2 },
+	{ MASK_NATURAL, CR3_TARGET_3 },
+
+	{ 0 /* read-only */, EXI_QUALIFICATION },
+	{ 0 /* read-only */, IO_RCX },
+	{ 0 /* read-only */, IO_RSI },
+	{ 0 /* read-only */, IO_RDI },
+	{ 0 /* read-only */, IO_RIP },
+	{ 0 /* read-only */, GUEST_LINEAR_ADDRESS },
+
+	{ MASK_NATURAL, GUEST_CR0 },
+	{ MASK_NATURAL, GUEST_CR3 },
+	{ MASK_NATURAL, GUEST_CR4 },
+	{ MASK_NATURAL, GUEST_BASE_ES },
+	{ MASK_NATURAL, GUEST_BASE_CS },
+	{ MASK_NATURAL, GUEST_BASE_SS },
+	{ MASK_NATURAL, GUEST_BASE_DS },
+	{ MASK_NATURAL, GUEST_BASE_FS },
+	{ MASK_NATURAL, GUEST_BASE_GS },
+	{ MASK_NATURAL, GUEST_BASE_LDTR },
+	{ MASK_NATURAL, GUEST_BASE_TR },
+	{ MASK_NATURAL, GUEST_BASE_GDTR },
+	{ MASK_NATURAL, GUEST_BASE_IDTR },
+	{ MASK_NATURAL, GUEST_DR7 },
+	{ MASK_NATURAL, GUEST_RSP },
+	{ MASK_NATURAL, GUEST_RIP },
+	{ MASK_NATURAL, GUEST_RFLAGS },
+	{ MASK_NATURAL, GUEST_PENDING_DEBUG },
+	{ MASK_NATURAL, GUEST_SYSENTER_ESP },
+	{ MASK_NATURAL, GUEST_SYSENTER_EIP },
+
+	{ MASK_NATURAL, HOST_CR0 },
+	{ MASK_NATURAL, HOST_CR3 },
+	{ MASK_NATURAL, HOST_CR4 },
+	{ MASK_NATURAL, HOST_BASE_FS },
+	{ MASK_NATURAL, HOST_BASE_GS },
+	{ MASK_NATURAL, HOST_BASE_TR },
+	{ MASK_NATURAL, HOST_BASE_GDTR },
+	{ MASK_NATURAL, HOST_BASE_IDTR },
+	{ MASK_NATURAL, HOST_SYSENTER_ESP },
+	{ MASK_NATURAL, HOST_SYSENTER_EIP },
+	{ MASK_NATURAL, HOST_RSP },
+	{ MASK_NATURAL, HOST_RIP },
+};
+
+static inline u64 vmcs_field_value(struct vmcs_field *f, u8 cookie)
+{
+	u64 value;
+
+	/* Incorporate the cookie and the field encoding into the value. */
+	value = cookie;
+	value |= (f->encoding << 8);
+	value |= 0xdeadbeefull << 32;
+
+	return value & f->mask;
+}
+
+static void set_vmcs_field(struct vmcs_field *f, u8 cookie)
+{
+	vmcs_write(f->encoding, vmcs_field_value(f, cookie));
+}
+
+static bool check_vmcs_field(struct vmcs_field *f, u8 cookie)
+{
+	u64 expected;
+	u64 actual;
+	int ret;
+
+	ret = vmcs_read_checking(f->encoding, &actual);
+	assert(!(ret & X86_EFLAGS_CF));
+	/* Skip VMCS fields that aren't recognized by the CPU */
+	if (ret & X86_EFLAGS_ZF)
+		return true;
+
+	expected = vmcs_field_value(f, cookie);
+	actual &= f->mask;
+
+	if (expected == actual)
+		return true;
+
+	printf("FAIL: VMWRITE/VMREAD %lx (expected: %lx, actual: %lx)",
+	       f->encoding, (unsigned long) expected, (unsigned long) actual);
+
+	return false;
+}
+
+static void set_all_vmcs_fields(u8 cookie)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(vmcs_fields); i++)
+		set_vmcs_field(&vmcs_fields[i], cookie);
+}
+
+static bool check_all_vmcs_fields(u8 cookie)
+{
+	bool pass = true;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(vmcs_fields); i++) {
+		if (!check_vmcs_field(&vmcs_fields[i], cookie))
+			pass = false;
+	}
+
+	return pass;
+}
+
+void test_vmwrite_vmread(void)
+{
+	struct vmcs *vmcs = alloc_page();
+
+	memset(vmcs, 0, PAGE_SIZE);
+	vmcs->revision_id = basic.revision;
+	assert(!vmcs_clear(vmcs));
+	assert(!make_vmcs_current(vmcs));
+
+	set_all_vmcs_fields(0x42);
+	report("VMWRITE/VMREAD", check_all_vmcs_fields(0x42));
+
+	assert(!vmcs_clear(vmcs));
+	free_page(vmcs);
+}
+
 void vmx_set_test_stage(u32 s)
 {
 	barrier();
@@ -87,16 +324,6 @@ void vmx_inc_test_stage(void)
 	barrier();
 }
 
-static int make_vmcs_current(struct vmcs *vmcs)
-{
-	bool ret;
-	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
-
-	asm volatile ("push %1; popf; vmptrld %2; setbe %0"
-		      : "=q" (ret) : "q" (rflags), "m" (vmcs) : "cc");
-	return ret;
-}
-
 /* entry_sysenter */
 asm(
 	".align	4, 0x90\n\t"
@@ -1243,6 +1470,9 @@ int main(int argc, const char *argv[])
 		test_vmclear();
 	if (test_wanted("test_vmptrst", argv, argc))
 		test_vmptrst();
+	if (test_wanted("test_vmwrite_vmread", argv, argc))
+		test_vmwrite_vmread();
+
 	init_vmcs(&vmcs_root);
 	if (vmx_run()) {
 		report("test vmlaunch", 0);
diff --git a/x86/vmx.h b/x86/vmx.h
index 52ece1aa53c8..2328f0eee05d 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -567,6 +567,16 @@ void vmx_set_test_stage(u32 s);
 u32 vmx_get_test_stage(void);
 void vmx_inc_test_stage(void);
 
+static inline int make_vmcs_current(struct vmcs *vmcs)
+{
+	bool ret;
+	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
+
+	asm volatile ("push %1; popf; vmptrld %2; setbe %0"
+		      : "=q" (ret) : "q" (rflags), "m" (vmcs) : "cc");
+	return ret;
+}
+
 static inline int vmcs_clear(struct vmcs *vmcs)
 {
 	bool ret;
@@ -584,6 +594,25 @@ static inline u64 vmcs_read(enum Encoding enc)
 	return val;
 }
 
+static inline int vmcs_read_checking(enum Encoding enc, u64 *value)
+{
+	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
+	u64 encoding = enc;
+	u64 val;
+
+	asm volatile ("shl $8, %%rax;"
+		      "sahf;"
+		      "vmread %[encoding], %[val];"
+		      "lahf;"
+		      "shr $8, %%rax"
+		      : /* output */ [val]"=rm"(val), "+a"(rflags)
+		      : /* input */ [encoding]"r"(encoding)
+		      : /* clobber */ "cc");
+
+	*value = val;
+	return rflags & (X86_EFLAGS_CF | X86_EFLAGS_ZF);
+}
+
 static inline int vmcs_write(enum Encoding enc, u64 val)
 {
 	bool ret;
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 10/32] x86: vmcs lifecycle test
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (8 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 09/32] x86: basic vmwrite/vmread test David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 11/32] x86: test VMCS in memory after VMCLEAR David Matlack
                   ` (23 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: David Matlack

This test VMCLEARs and VMPTRLDs two VMCS's to put each VMCS into different
states of active/not-active and current/not-current and then verifies
that VMREAD/VMWRITE to the current VMCS work correctly.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/unittests.cfg |  6 ++++++
 x86/vmx.c         | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 7973f2f62d26..0f2c94fb548d 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -236,6 +236,12 @@ extra_params = -cpu host,+vmx -append test_vmwrite_vmread
 arch = x86_64
 groups = vmx
 
+[vmx_test_vmcs_lifecycle]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append test_vmcs_lifecycle
+arch = x86_64
+groups = vmx
+
 [vmx_test_vmx_caps]
 file = vmx.flat
 extra_params = -cpu host,+vmx -append test_vmx_caps
diff --git a/x86/vmx.c b/x86/vmx.c
index 47404fbbd782..7daa1d110c82 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -300,6 +300,64 @@ void test_vmwrite_vmread(void)
 	free_page(vmcs);
 }
 
+void test_vmcs_lifecycle(void)
+{
+	struct vmcs *vmcs[2] = {};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(vmcs); i++) {
+		vmcs[i] = alloc_page();
+		memset(vmcs[i], 0, PAGE_SIZE);
+		vmcs[i]->revision_id = basic.revision;
+	}
+
+#define VMPTRLD(_i) do { \
+	assert(_i < ARRAY_SIZE(vmcs)); \
+	assert(!make_vmcs_current(vmcs[_i])); \
+	printf("VMPTRLD VMCS%d\n", (_i)); \
+} while (0)
+
+#define VMCLEAR(_i) do { \
+	assert(_i < ARRAY_SIZE(vmcs)); \
+	assert(!vmcs_clear(vmcs[_i])); \
+	printf("VMCLEAR VMCS%d\n", (_i)); \
+} while (0)
+
+	VMCLEAR(0);
+	VMPTRLD(0);
+	set_all_vmcs_fields(0);
+	report("current:VMCS0 active:[VMCS0]", check_all_vmcs_fields(0));
+
+	VMCLEAR(0);
+	VMPTRLD(0);
+	report("current:VMCS0 active:[VMCS0]", check_all_vmcs_fields(0));
+
+	VMCLEAR(1);
+	report("current:VMCS0 active:[VMCS0]", check_all_vmcs_fields(0));
+
+	VMPTRLD(1);
+	set_all_vmcs_fields(1);
+	report("current:VMCS1 active:[VMCS0,VCMS1]", check_all_vmcs_fields(1));
+
+	VMPTRLD(0);
+	report("current:VMCS0 active:[VMCS0,VCMS1]", check_all_vmcs_fields(0));
+	VMPTRLD(1);
+	report("current:VMCS1 active:[VMCS0,VCMS1]", check_all_vmcs_fields(1));
+	VMPTRLD(1);
+	report("current:VMCS1 active:[VMCS0,VCMS1]", check_all_vmcs_fields(1));
+
+	VMCLEAR(0);
+	report("current:VMCS1 active:[VCMS1]", check_all_vmcs_fields(1));
+
+	for (i = 0; i < ARRAY_SIZE(vmcs); i++) {
+		VMCLEAR(i);
+		free_page(vmcs[i]);
+	}
+
+#undef VMPTRLD
+#undef VMCLEAR
+}
+
 void vmx_set_test_stage(u32 s)
 {
 	barrier();
@@ -1472,6 +1530,8 @@ int main(int argc, const char *argv[])
 		test_vmptrst();
 	if (test_wanted("test_vmwrite_vmread", argv, argc))
 		test_vmwrite_vmread();
+	if (test_wanted("test_vmcs_lifecycle", argv, argc))
+		test_vmcs_lifecycle();
 
 	init_vmcs(&vmcs_root);
 	if (vmx_run()) {
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 11/32] x86: test VMCS in memory after VMCLEAR
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (9 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 10/32] x86: vmcs lifecycle test David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 12/32] x86: test VMPTRLD does not drop VMWRITEs David Matlack
                   ` (22 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: David Matlack

The Intel SDM states that following a VMCLEAR instruction, the VMCS
given as an operand to VMCLEAR should be resident in memory. If the
CPU caches VMCS data outside of memory, VMCLEAR must flush this cache
to memory.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/vmx.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/x86/vmx.c b/x86/vmx.c
index 7daa1d110c82..fd1aa3bc6c2e 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -487,6 +487,42 @@ print_vmentry_failure_info(struct vmentry_failure *failure) {
 	}
 }
 
+/*
+ * VMCLEAR should ensures all VMCS state is flushed to the VMCS
+ * region in memory.
+ */
+static void test_vmclear_flushing(void)
+{
+	struct vmcs *vmcs[3] = {};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(vmcs); i++) {
+		vmcs[i] = alloc_page();
+		memset(vmcs[i], 0, PAGE_SIZE);
+	}
+
+	vmcs[0]->revision_id = basic.revision;
+	assert(!vmcs_clear(vmcs[0]));
+	assert(!make_vmcs_current(vmcs[0]));
+	set_all_vmcs_fields(0x86);
+
+	assert(!vmcs_clear(vmcs[0]));
+	memcpy(vmcs[1], vmcs[0], basic.size);
+	assert(!make_vmcs_current(vmcs[1]));
+	report("test vmclear flush (current VMCS)", check_all_vmcs_fields(0x86));
+
+	set_all_vmcs_fields(0x87);
+	assert(!make_vmcs_current(vmcs[0]));
+	assert(!vmcs_clear(vmcs[1]));
+	memcpy(vmcs[2], vmcs[1], basic.size);
+	assert(!make_vmcs_current(vmcs[2]));
+	report("test vmclear flush (!current VMCS)", check_all_vmcs_fields(0x87));
+
+	for (i = 0; i < ARRAY_SIZE(vmcs); i++) {
+		assert(!vmcs_clear(vmcs[i]));
+		free_page(vmcs[i]);
+	}
+}
 
 static void test_vmclear(void)
 {
@@ -519,6 +555,7 @@ static void test_vmclear(void)
 	/* Valid VMCS */
 	report("test vmclear with valid vmcs region", vmcs_clear(vmcs_root) == 0);
 
+	test_vmclear_flushing();
 }
 
 static void test_vmxoff(void)
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 12/32] x86: test VMPTRLD does not drop VMWRITEs
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (10 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 11/32] x86: test VMCS in memory after VMCLEAR David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 13/32] x86: don't special case vmx null test David Matlack
                   ` (21 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: David Matlack

Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/vmx.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index fd1aa3bc6c2e..9ca37f63b636 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -257,7 +257,7 @@ static bool check_vmcs_field(struct vmcs_field *f, u8 cookie)
 	if (expected == actual)
 		return true;
 
-	printf("FAIL: VMWRITE/VMREAD %lx (expected: %lx, actual: %lx)",
+	printf("FAIL: VMWRITE/VMREAD %lx (expected: %lx, actual: %lx)\n",
 	       f->encoding, (unsigned long) expected, (unsigned long) actual);
 
 	return false;
@@ -349,6 +349,11 @@ void test_vmcs_lifecycle(void)
 	VMCLEAR(0);
 	report("current:VMCS1 active:[VCMS1]", check_all_vmcs_fields(1));
 
+	/* VMPTRLD should not erase VMWRITEs to the current VMCS */
+	set_all_vmcs_fields(2);
+	VMPTRLD(1);
+	report("current:VMCS1 active:[VCMS1]", check_all_vmcs_fields(2));
+
 	for (i = 0; i < ARRAY_SIZE(vmcs); i++) {
 		VMCLEAR(i);
 		free_page(vmcs[i]);
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 13/32] x86: don't special case vmx null test
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (11 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 12/32] x86: test VMPTRLD does not drop VMWRITEs David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 14/32] x86: factor out vmx_enter_guest David Matlack
                   ` (20 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/unittests.cfg |  6 ++++++
 x86/vmx.c         | 25 ++++---------------------
 x86/vmx_tests.c   |  1 +
 3 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 0f2c94fb548d..8a89eb0ef0ad 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -200,6 +200,12 @@ extra_params = -cpu host,+vmx -append -exit_monitor_from_l2_test
 arch = x86_64
 groups = vmx
 
+[vmx_null]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append null
+arch = x86_64
+groups = vmx
+
 [vmx_test_vmx_feature_control]
 file = vmx.flat
 extra_params = -cpu host,+vmx -append test_vmx_feature_control
diff --git a/x86/vmx.c b/x86/vmx.c
index 9ca37f63b636..15ed94af56fd 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -563,14 +563,6 @@ static void test_vmclear(void)
 	test_vmclear_flushing();
 }
 
-static void test_vmxoff(void)
-{
-	int ret;
-
-	ret = vmx_off();
-	report("test vmxoff", !ret);
-}
-
 static void __attribute__((__used__)) guest_main(void)
 {
 	current->guest_main();
@@ -1550,9 +1542,6 @@ int main(int argc, const char *argv[])
 			wrmsr(MSR_IA32_FEATURE_CONTROL, 0x5);
 	}
 
-	/* Set basic test ctxt the same as "null" */
-	current = &vmx_tests[0];
-
 	if (test_wanted("test_vmxon", argv, argc)) {
 		/* Enables VMX */
 		if (test_vmxon() != 0)
@@ -1574,19 +1563,13 @@ int main(int argc, const char *argv[])
 		test_vmwrite_vmread();
 	if (test_wanted("test_vmcs_lifecycle", argv, argc))
 		test_vmcs_lifecycle();
-
-	init_vmcs(&vmcs_root);
-	if (vmx_run()) {
-		report("test vmlaunch", 0);
-		goto exit;
-	}
-
-	test_vmxoff();
-
 	if (test_wanted("test_vmx_caps", argv, argc))
 		test_vmx_caps();
 
-	while (vmx_tests[++i].name != NULL) {
+	/* Balance vmxon from test_vmxon. */
+	vmx_off();
+
+	for (; vmx_tests[i].name != NULL; i++) {
 		if (!test_wanted(vmx_tests[i].name, argv, argc))
 			continue;
 		if (test_run(&vmx_tests[i]))
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 35af67744971..918cade2a3e8 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -29,6 +29,7 @@ static inline void vmcall()
 
 void basic_guest_main()
 {
+	report("Basic VMX test", 1);
 }
 
 int basic_exit_handler()
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 14/32] x86: factor out vmx_enter_guest
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (12 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 13/32] x86: don't special case vmx null test David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 15/32] x86: binstr utility function David Matlack
                   ` (19 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/vmx.c | 75 ++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 41 insertions(+), 34 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 15ed94af56fd..e6c11b013d94 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1376,43 +1376,54 @@ entry_failure_handler(struct vmentry_failure *failure)
 		return VMX_TEST_EXIT;
 }
 
-static int vmx_run()
+/*
+ * Tries to enter the guest. Returns true iff entry succeeded. Otherwise,
+ * populates @failure.
+ */
+static bool vmx_enter_guest(struct vmentry_failure *failure)
 {
-	unsigned long host_rflags;
+	failure->early = 0;
+
+	asm volatile (
+		"mov %[HOST_RSP], %%rdi\n\t"
+		"vmwrite %%rsp, %%rdi\n\t"
+		LOAD_GPR_C
+		"cmpb $0, %[launched]\n\t"
+		"jne 1f\n\t"
+		"vmlaunch\n\t"
+		"jmp 2f\n\t"
+		"1: "
+		"vmresume\n\t"
+		"2: "
+		SAVE_GPR_C
+		"pushf\n\t"
+		"pop %%rdi\n\t"
+		"mov %%rdi, %[failure_flags]\n\t"
+		"movl $1, %[failure_flags]\n\t"
+		"jmp 3f\n\t"
+		"vmx_return:\n\t"
+		SAVE_GPR_C
+		"3: \n\t"
+		: [failure_early]"+m"(failure->early),
+		  [failure_flags]"=m"(failure->flags)
+		: [launched]"m"(launched), [HOST_RSP]"i"(HOST_RSP)
+		: "rdi", "memory", "cc"
+	);
+
+	failure->vmlaunch = !launched;
+	failure->instr = launched ? "vmresume" : "vmlaunch";
+
+	return !failure->early && !(vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE);
+}
 
+static int vmx_run()
+{
 	while (1) {
 		u32 ret;
-		u32 fail = 0;
 		bool entered;
 		struct vmentry_failure failure;
 
-		asm volatile (
-			"mov %[HOST_RSP], %%rdi\n\t"
-			"vmwrite %%rsp, %%rdi\n\t"
-			LOAD_GPR_C
-			"cmpb $0, %[launched]\n\t"
-			"jne 1f\n\t"
-			"vmlaunch\n\t"
-			"jmp 2f\n\t"
-			"1: "
-			"vmresume\n\t"
-			"2: "
-			SAVE_GPR_C
-			"pushf\n\t"
-			"pop %%rdi\n\t"
-			"mov %%rdi, %[host_rflags]\n\t"
-			"movl $1, %[fail]\n\t"
-			"jmp 3f\n\t"
-			"vmx_return:\n\t"
-			SAVE_GPR_C
-			"3: \n\t"
-			: [fail]"+m"(fail), [host_rflags]"=m"(host_rflags)
-			: [launched]"m"(launched), [HOST_RSP]"i"(HOST_RSP)
-			: "rdi", "memory", "cc"
-
-		);
-
-		entered = !fail && !(vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE);
+		entered = vmx_enter_guest(&failure);
 
 		if (entered) {
 			/*
@@ -1422,10 +1433,6 @@ static int vmx_run()
 			launched = 1;
 			ret = exit_handler();
 		} else {
-			failure.flags = host_rflags;
-			failure.vmlaunch = !launched;
-			failure.instr = launched ? "vmresume" : "vmlaunch";
-			failure.early = fail;
 			ret = entry_failure_handler(&failure);
 		}
 
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 15/32] x86: binstr utility function
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (13 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 14/32] x86: factor out vmx_enter_guest David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 16/32] x86: vmx exit reason descriptions David Matlack
                   ` (18 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 lib/libcflat.h |  7 +++++++
 lib/printf.c   | 30 ++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 248fd023626e..1d2eba980e1a 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -131,4 +131,11 @@ static inline bool is_power_of_2(unsigned long n)
 	return n && !(n & (n - 1));
 }
 
+/*
+ * One byte per bit, a ' between each group of 4 bits, and a null terminator.
+ */
+#define BINSTR_SZ (sizeof(unsigned long) * 8 + sizeof(unsigned long) * 2)
+void binstr(unsigned long x, char out[BINSTR_SZ]);
+void print_binstr(unsigned long x);
+
 #endif
diff --git a/lib/printf.c b/lib/printf.c
index 2aec59aa6d6d..cecbeadc4440 100644
--- a/lib/printf.c
+++ b/lib/printf.c
@@ -259,3 +259,33 @@ int printf(const char *fmt, ...)
     puts(buf);
     return r;
 }
+
+void binstr(unsigned long x, char out[BINSTR_SZ])
+{
+	int i;
+	char *c;
+	int n;
+
+	n = sizeof(unsigned long) * 8;
+	i = 0;
+	c = &out[0];
+	for (;;) {
+		*c++ = (x & (1ul << (n - i - 1))) ? '1' : '0';
+		i++;
+
+		if (i == n) {
+			*c = '\0';
+			break;
+		}
+		if (i % 4 == 0)
+			*c++ = '\'';
+	}
+	assert(c + 1 - &out[0] == BINSTR_SZ);
+}
+
+void print_binstr(unsigned long x)
+{
+	char out[BINSTR_SZ];
+	binstr(x, out);
+	printf("%s", out);
+}
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 16/32] x86: vmx exit reason descriptions
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (14 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 15/32] x86: binstr utility function David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 17/32] x86: v2 vmx test framework David Matlack
                   ` (17 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/vmx.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 x86/vmx.h | 11 +++++++++-
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index e6c11b013d94..cd4cb1219040 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -425,6 +425,75 @@ static inline int vmx_off()
 	return ret;
 }
 
+static const char * const exit_reason_descriptions[] = {
+	[VMX_EXC_NMI]		= "VMX_EXC_NMI",
+	[VMX_EXTINT]		= "VMX_EXTINT",
+	[VMX_TRIPLE_FAULT]	= "VMX_TRIPLE_FAULT",
+	[VMX_INIT]		= "VMX_INIT",
+	[VMX_SIPI]		= "VMX_SIPI",
+	[VMX_SMI_IO]		= "VMX_SMI_IO",
+	[VMX_SMI_OTHER]		= "VMX_SMI_OTHER",
+	[VMX_INTR_WINDOW]	= "VMX_INTR_WINDOW",
+	[VMX_NMI_WINDOW]	= "VMX_NMI_WINDOW",
+	[VMX_TASK_SWITCH]	= "VMX_TASK_SWITCH",
+	[VMX_CPUID]		= "VMX_CPUID",
+	[VMX_GETSEC]		= "VMX_GETSEC",
+	[VMX_HLT]		= "VMX_HLT",
+	[VMX_INVD]		= "VMX_INVD",
+	[VMX_INVLPG]		= "VMX_INVLPG",
+	[VMX_RDPMC]		= "VMX_RDPMC",
+	[VMX_RDTSC]		= "VMX_RDTSC",
+	[VMX_RSM]		= "VMX_RSM",
+	[VMX_VMCALL]		= "VMX_VMCALL",
+	[VMX_VMCLEAR]		= "VMX_VMCLEAR",
+	[VMX_VMLAUNCH]		= "VMX_VMLAUNCH",
+	[VMX_VMPTRLD]		= "VMX_VMPTRLD",
+	[VMX_VMPTRST]		= "VMX_VMPTRST",
+	[VMX_VMREAD]		= "VMX_VMREAD",
+	[VMX_VMRESUME]		= "VMX_VMRESUME",
+	[VMX_VMWRITE]		= "VMX_VMWRITE",
+	[VMX_VMXOFF]		= "VMX_VMXOFF",
+	[VMX_VMXON]		= "VMX_VMXON",
+	[VMX_CR]		= "VMX_CR",
+	[VMX_DR]		= "VMX_DR",
+	[VMX_IO]		= "VMX_IO",
+	[VMX_RDMSR]		= "VMX_RDMSR",
+	[VMX_WRMSR]		= "VMX_WRMSR",
+	[VMX_FAIL_STATE]	= "VMX_FAIL_STATE",
+	[VMX_FAIL_MSR]		= "VMX_FAIL_MSR",
+	[VMX_MWAIT]		= "VMX_MWAIT",
+	[VMX_MTF]		= "VMX_MTF",
+	[VMX_MONITOR]		= "VMX_MONITOR",
+	[VMX_PAUSE]		= "VMX_PAUSE",
+	[VMX_FAIL_MCHECK]	= "VMX_FAIL_MCHECK",
+	[VMX_TPR_THRESHOLD]	= "VMX_TPR_THRESHOLD",
+	[VMX_APIC_ACCESS]	= "VMX_APIC_ACCESS",
+	[VMX_GDTR_IDTR]		= "VMX_GDTR_IDTR",
+	[VMX_LDTR_TR]		= "VMX_LDTR_TR",
+	[VMX_EPT_VIOLATION]	= "VMX_EPT_VIOLATION",
+	[VMX_EPT_MISCONFIG]	= "VMX_EPT_MISCONFIG",
+	[VMX_INVEPT]		= "VMX_INVEPT",
+	[VMX_PREEMPT]		= "VMX_PREEMPT",
+	[VMX_INVVPID]		= "VMX_INVVPID",
+	[VMX_WBINVD]		= "VMX_WBINVD",
+	[VMX_XSETBV]		= "VMX_XSETBV",
+	[VMX_APIC_WRITE]	= "VMX_APIC_WRITE",
+	[VMX_RDRAND]		= "VMX_RDRAND",
+	[VMX_INVPCID]		= "VMX_INVPCID",
+	[VMX_VMFUNC]		= "VMX_VMFUNC",
+	[VMX_RDSEED]		= "VMX_RDSEED",
+	[VMX_PML_FULL]		= "VMX_PML_FULL",
+	[VMX_XSAVES]		= "VMX_XSAVES",
+	[VMX_XRSTORS]		= "VMX_XRSTORS",
+};
+
+const char *exit_reason_description(u64 reason)
+{
+	if (reason >= ARRAY_SIZE(exit_reason_descriptions))
+		return "(unknown)";
+	return exit_reason_descriptions[reason] ? : "(unused)";
+}
+
 void print_vmexit_info()
 {
 	u64 guest_rip, guest_rsp;
diff --git a/x86/vmx.h b/x86/vmx.h
index 2328f0eee05d..3e4015660797 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -317,7 +317,15 @@ enum Reason {
 	VMX_PREEMPT		= 52,
 	VMX_INVVPID		= 53,
 	VMX_WBINVD		= 54,
-	VMX_XSETBV		= 55
+	VMX_XSETBV		= 55,
+	VMX_APIC_WRITE		= 56,
+	VMX_RDRAND		= 57,
+	VMX_INVPCID		= 58,
+	VMX_VMFUNC		= 59,
+	VMX_RDSEED		= 61,
+	VMX_PML_FULL		= 62,
+	VMX_XSAVES		= 63,
+	VMX_XRSTORS		= 64,
 };
 
 enum Ctrl_exi {
@@ -659,6 +667,7 @@ static inline bool invvpid(unsigned long type, u16 vpid, u64 gva)
 	return ret;
 }
 
+const char *exit_reason_description(u64 reason);
 void print_vmexit_info();
 void print_vmentry_failure_info(struct vmentry_failure *failure);
 void ept_sync(int type, u64 eptp);
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 17/32] x86: v2 vmx test framework
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (15 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 16/32] x86: vmx exit reason descriptions David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 18/32] lib: provide stdio.h David Matlack
                   ` (16 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/unittests.cfg |   6 ++
 x86/vmx.c         | 179 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 x86/vmx.h         |  66 ++++++++++++++++++++
 x86/vmx_tests.c   |  94 ++++++++++++++++++++++++++++
 4 files changed, 343 insertions(+), 2 deletions(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 8a89eb0ef0ad..f79688af93f8 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -362,6 +362,12 @@ extra_params = -cpu host,+vmx -append exit_monitor_from_l2_test
 arch = x86_64
 groups = vmx
 
+[vmx_v2]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append "v2_null_test v2_multiple_entries_test fixture_test_case1 fixture_test_case2"
+arch = x86_64
+groups = vmx
+
 [debug]
 file = debug.flat
 arch = x86_64
diff --git a/x86/vmx.c b/x86/vmx.c
index cd4cb1219040..cae650ae2e68 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -42,10 +42,26 @@ u32 vpid_cnt;
 void *guest_stack, *guest_syscall_stack;
 u32 ctrl_pin, ctrl_enter, ctrl_exit, ctrl_cpu[2];
 struct regs regs;
+
 struct vmx_test *current;
+
+#define MAX_TEST_TEARDOWN_STEPS 10
+
+struct test_teardown_step {
+	test_teardown_func func;
+	void *data;
+};
+
+static int teardown_count;
+static struct test_teardown_step teardown_steps[MAX_TEST_TEARDOWN_STEPS];
+
+static test_guest_func v2_guest_main;
+
 u64 hypercall_field;
 bool launched;
 static int matched;
+static int guest_finished;
+static int in_guest;
 
 union vmx_basic basic;
 union vmx_ctrl_msr ctrl_pin_rev;
@@ -63,6 +79,8 @@ extern void *guest_entry;
 
 static volatile u32 stage;
 
+static jmp_buf abort_target;
+
 struct vmcs_field {
 	u64 mask;
 	u64 encoding;
@@ -634,7 +652,10 @@ static void test_vmclear(void)
 
 static void __attribute__((__used__)) guest_main(void)
 {
-	current->guest_main();
+	if (current->v2)
+		v2_guest_main();
+	else
+		current->guest_main();
 }
 
 /* guest_entry */
@@ -1409,12 +1430,51 @@ static int handle_hypercall()
 	switch (hypercall_no) {
 	case HYPERCALL_VMEXIT:
 		return VMX_TEST_VMEXIT;
+	case HYPERCALL_VMABORT:
+		return VMX_TEST_VMABORT;
+	case HYPERCALL_VMSKIP:
+		return VMX_TEST_VMSKIP;
 	default:
 		printf("ERROR : Invalid hypercall number : %ld\n", hypercall_no);
 	}
 	return VMX_TEST_EXIT;
 }
 
+static void continue_abort(void)
+{
+	assert(!in_guest);
+	printf("Host was here when guest aborted:\n");
+	dump_stack();
+	longjmp(abort_target, 1);
+	abort();
+}
+
+void __abort_test(void)
+{
+	if (in_guest)
+		hypercall(HYPERCALL_VMABORT);
+	else
+		longjmp(abort_target, 1);
+	abort();
+}
+
+static void continue_skip(void)
+{
+	assert(!in_guest);
+	longjmp(abort_target, 1);
+	abort();
+}
+
+void test_skip(const char *msg)
+{
+	printf("%s skipping test: %s\n", in_guest ? "Guest" : "Host", msg);
+	if (in_guest)
+		hypercall(HYPERCALL_VMABORT);
+	else
+		longjmp(abort_target, 1);
+	abort();
+}
+
 static int exit_handler()
 {
 	int ret;
@@ -1453,6 +1513,7 @@ static bool vmx_enter_guest(struct vmentry_failure *failure)
 {
 	failure->early = 0;
 
+	in_guest = 1;
 	asm volatile (
 		"mov %[HOST_RSP], %%rdi\n\t"
 		"vmwrite %%rsp, %%rdi\n\t"
@@ -1478,6 +1539,7 @@ static bool vmx_enter_guest(struct vmentry_failure *failure)
 		: [launched]"m"(launched), [HOST_RSP]"i"(HOST_RSP)
 		: "rdi", "memory", "cc"
 	);
+	in_guest = 0;
 
 	failure->vmlaunch = !launched;
 	failure->instr = launched ? "vmresume" : "vmlaunch";
@@ -1509,6 +1571,7 @@ static int vmx_run()
 		case VMX_TEST_RESUME:
 			continue;
 		case VMX_TEST_VMEXIT:
+			guest_finished = 1;
 			return 0;
 		case VMX_TEST_EXIT:
 			break;
@@ -1527,26 +1590,67 @@ static int vmx_run()
 	}
 }
 
+static void run_teardown_step(struct test_teardown_step *step)
+{
+	step->func(step->data);
+}
+
 static int test_run(struct vmx_test *test)
 {
+	int r;
+
+	/* Validate V2 interface. */
+	if (test->v2) {
+		int ret = 0;
+		if (test->init || test->guest_main || test->exit_handler ||
+		    test->syscall_handler) {
+			report("V2 test cannot specify V1 callbacks.", 0);
+			ret = 1;
+		}
+		if (ret)
+			return ret;
+	}
+
 	if (test->name == NULL)
 		test->name = "(no name)";
 	if (vmx_on()) {
 		printf("%s : vmxon failed.\n", __func__);
 		return 1;
 	}
+
 	init_vmcs(&(test->vmcs));
 	/* Directly call test->init is ok here, init_vmcs has done
 	   vmcs init, vmclear and vmptrld*/
 	if (test->init && test->init(test->vmcs) != VMX_TEST_START)
 		goto out;
+	teardown_count = 0;
+	v2_guest_main = NULL;
 	test->exits = 0;
 	current = test;
 	regs = test->guest_regs;
 	vmcs_write(GUEST_RFLAGS, regs.rflags | 0x2);
 	launched = 0;
+	guest_finished = 0;
 	printf("\nTest suite: %s\n", test->name);
-	vmx_run();
+
+	r = setjmp(abort_target);
+	if (r) {
+		assert(!in_guest);
+		goto out;
+	}
+
+
+	if (test->v2)
+		test->v2();
+	else
+		vmx_run();
+
+	while (teardown_count > 0)
+		run_teardown_step(&teardown_steps[--teardown_count]);
+
+	if (launched && !guest_finished)
+		report("Guest didn't run to completion.", 0);
+
 out:
 	if (vmx_off()) {
 		printf("%s : vmxoff failed.\n", __func__);
@@ -1555,6 +1659,77 @@ out:
 	return 0;
 }
 
+/*
+ * Add a teardown step. Executed after the test's main function returns.
+ * Teardown steps executed in reverse order.
+ */
+void test_add_teardown(test_teardown_func func, void *data)
+{
+	struct test_teardown_step *step;
+
+	TEST_ASSERT_MSG(teardown_count < MAX_TEST_TEARDOWN_STEPS,
+			"There are already %d teardown steps.",
+			teardown_count);
+	step = &teardown_steps[teardown_count++];
+	step->func = func;
+	step->data = data;
+}
+
+/*
+ * Set the target of the first enter_guest call. Can only be called once per
+ * test. Must be called before first enter_guest call.
+ */
+void test_set_guest(test_guest_func func)
+{
+	assert(current->v2);
+	TEST_ASSERT_MSG(!v2_guest_main, "Already set guest func.");
+	v2_guest_main = func;
+}
+
+/*
+ * Enters the guest (or launches it for the first time). Error to call once the
+ * guest has returned (i.e., run past the end of its guest() function). Also
+ * aborts if guest entry fails.
+ */
+void enter_guest(void)
+{
+	struct vmentry_failure failure;
+
+	TEST_ASSERT_MSG(v2_guest_main,
+			"Never called test_set_guest_func!");
+
+	TEST_ASSERT_MSG(!guest_finished,
+			"Called enter_guest() after guest returned.");
+
+	if (!vmx_enter_guest(&failure)) {
+		print_vmentry_failure_info(&failure);
+		abort();
+	}
+
+	launched = 1;
+
+	if (is_hypercall()) {
+		int ret;
+
+		ret = handle_hypercall();
+		switch (ret) {
+		case VMX_TEST_VMEXIT:
+			guest_finished = 1;
+			break;
+		case VMX_TEST_VMABORT:
+			continue_abort();
+			break;
+		case VMX_TEST_VMSKIP:
+			continue_skip();
+			break;
+		default:
+			printf("ERROR : Invalid handle_hypercall return %d.\n",
+			       ret);
+			abort();
+		}
+	}
+}
+
 extern struct vmx_test vmx_tests[];
 
 static bool
diff --git a/x86/vmx.h b/x86/vmx.h
index 3e4015660797..966fff653561 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -54,6 +54,8 @@ struct vmx_test {
 	int (*entry_failure_handler)(struct vmentry_failure *failure);
 	struct vmcs *vmcs;
 	int exits;
+	/* Alternative test interface. */
+	void (*v2)(void);
 };
 
 union vmx_basic {
@@ -490,10 +492,14 @@ enum vm_instruction_error_number {
 #define VMX_TEST_VMEXIT		1
 #define VMX_TEST_EXIT		2
 #define VMX_TEST_RESUME		3
+#define VMX_TEST_VMABORT	4
+#define VMX_TEST_VMSKIP		5
 
 #define HYPERCALL_BIT		(1ul << 12)
 #define HYPERCALL_MASK		0xFFF
 #define HYPERCALL_VMEXIT	0x1
+#define HYPERCALL_VMABORT	0x2
+#define HYPERCALL_VMSKIP	0x3
 
 #define EPTP_PG_WALK_LEN_SHIFT	3ul
 #define EPTP_AD_FLAG		(1ul << 6)
@@ -693,4 +699,64 @@ void check_ept_ad(unsigned long *pml4, u64 guest_cr3,
 void clear_ept_ad(unsigned long *pml4, u64 guest_cr3,
 		  unsigned long guest_addr);
 
+void enter_guest(void);
+
+typedef void (*test_guest_func)(void);
+typedef void (*test_teardown_func)(void *data);
+void test_set_guest(test_guest_func func);
+void test_add_teardown(test_teardown_func func, void *data);
+void test_skip(const char *msg);
+
+void __abort_test(void);
+
+#define TEST_ASSERT(cond) \
+do { \
+	if (!(cond)) { \
+		report("%s:%d: Assertion failed: %s", 0, \
+		       __FILE__, __LINE__, #cond); \
+		dump_stack(); \
+		__abort_test(); \
+	} \
+} while (0)
+
+#define TEST_ASSERT_MSG(cond, fmt, args...) \
+do { \
+	if (!(cond)) { \
+		report("%s:%d: Assertion failed: %s\n" fmt, 0, \
+		       __FILE__, __LINE__, #cond, ##args); \
+		dump_stack(); \
+		__abort_test(); \
+	} \
+} while (0)
+
+#define __TEST_EQ(a, b, a_str, b_str, assertion, fmt, args...) \
+do { \
+	typeof(a) _a = a; \
+	typeof(b) _b = b; \
+	if (_a != _b) { \
+		char _bin_a[BINSTR_SZ]; \
+		char _bin_b[BINSTR_SZ]; \
+		binstr(_a, _bin_a); \
+		binstr(_b, _bin_b); \
+		report("%s:%d: %s failed: (%s) == (%s)\n" \
+		       "\tLHS: 0x%016lx - %s - %lu\n" \
+		       "\tRHS: 0x%016lx - %s - %lu%s" fmt, 0, \
+		       __FILE__, __LINE__, \
+		       assertion ? "Assertion" : "Expectation", a_str, b_str, \
+		       (unsigned long) _a, _bin_a, (unsigned long) _a, \
+		       (unsigned long) _b, _bin_b, (unsigned long) _b, \
+		       fmt[0] == '\0' ? "" : "\n", ## args); \
+		dump_stack(); \
+		if (assertion) \
+			__abort_test(); \
+	} \
+} while (0)
+
+#define TEST_ASSERT_EQ(a, b) __TEST_EQ(a, b, #a, #b, 1, "")
+#define TEST_ASSERT_EQ_MSG(a, b, fmt, args...) \
+	__TEST_EQ(a, b, #a, #b, 1, fmt, ## args)
+#define TEST_EXPECT_EQ(a, b) __TEST_EQ(a, b, #a, #b, 0, "")
+#define TEST_EXPECT_EQ_MSG(a, b, fmt, args...) \
+	__TEST_EQ(a, b, #a, #b, 0, fmt, ## args)
+
 #endif
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 918cade2a3e8..5758b6becfa6 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1897,6 +1897,95 @@ static int exit_monitor_from_l2_handler(void)
 	return VMX_TEST_EXIT;
 }
 
+static void assert_exit_reason(u64 expected)
+{
+	u64 actual = vmcs_read(EXI_REASON);
+
+	TEST_ASSERT_EQ_MSG(expected, actual, "Expected %s, got %s.",
+			   exit_reason_description(expected),
+			   exit_reason_description(actual));
+}
+
+static void skip_exit_vmcall()
+{
+	u64 guest_rip = vmcs_read(GUEST_RIP);
+	u32 insn_len = vmcs_read(EXI_INST_LEN);
+
+	assert_exit_reason(VMX_VMCALL);
+	vmcs_write(GUEST_RIP, guest_rip + insn_len);
+}
+
+static void v2_null_test_guest(void)
+{
+}
+
+static void v2_null_test(void)
+{
+	test_set_guest(v2_null_test_guest);
+	enter_guest();
+	report(__func__, 1);
+}
+
+static void v2_multiple_entries_test_guest(void)
+{
+	vmx_set_test_stage(1);
+	vmcall();
+	vmx_set_test_stage(2);
+}
+
+static void v2_multiple_entries_test(void)
+{
+	test_set_guest(v2_multiple_entries_test_guest);
+	enter_guest();
+	TEST_ASSERT_EQ(vmx_get_test_stage(), 1);
+	skip_exit_vmcall();
+	enter_guest();
+	TEST_ASSERT_EQ(vmx_get_test_stage(), 2);
+	report(__func__, 1);
+}
+
+static int fixture_test_data = 1;
+
+static void fixture_test_teardown(void *data)
+{
+	*((int *) data) = 1;
+}
+
+static void fixture_test_guest(void)
+{
+	fixture_test_data++;
+}
+
+
+static void fixture_test_setup(void)
+{
+	TEST_ASSERT_EQ_MSG(1, fixture_test_data,
+			   "fixture_test_teardown didn't run?!");
+	fixture_test_data = 2;
+	test_add_teardown(fixture_test_teardown, &fixture_test_data);
+	test_set_guest(fixture_test_guest);
+}
+
+static void fixture_test_case1(void)
+{
+	fixture_test_setup();
+	TEST_ASSERT_EQ(2, fixture_test_data);
+	enter_guest();
+	TEST_ASSERT_EQ(3, fixture_test_data);
+	report(__func__, 1);
+}
+
+static void fixture_test_case2(void)
+{
+	fixture_test_setup();
+	TEST_ASSERT_EQ(2, fixture_test_data);
+	enter_guest();
+	TEST_ASSERT_EQ(3, fixture_test_data);
+	report(__func__, 1);
+}
+
+#define TEST(name) { #name, .v2 = name }
+
 /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
 struct vmx_test vmx_tests[] = {
 	{ "null", NULL, basic_guest_main, basic_exit_handler, NULL, {0} },
@@ -1929,5 +2018,10 @@ struct vmx_test vmx_tests[] = {
 	{ "into", into_init, into_guest_main, into_exit_handler, NULL, {0} },
 	{ "exit_monitor_from_l2_test", NULL, exit_monitor_from_l2_main,
 		exit_monitor_from_l2_handler, NULL, {0} },
+	/* Basic V2 tests. */
+	TEST(v2_null_test),
+	TEST(v2_multiple_entries_test),
+	TEST(fixture_test_case1),
+	TEST(fixture_test_case2),
 	{ NULL, NULL, NULL, NULL, NULL, {0} },
 };
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 18/32] lib: provide stdio.h
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (16 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 17/32] x86: v2 vmx test framework David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  7:16   ` Andrew Jones
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 19/32] lib: added assert_msg macro David Matlack
                   ` (15 subsequent siblings)
  33 siblings, 1 reply; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

kvm-unit-tests builds with the odd combination of system include files
but not system libraries. That is, -nostdlib is specified but
-nostdinc isn't! Problems arise when the include files don't match the
libraries, e.g., printf() in some versions of glibc linking in a
__printf_chk symbol that libcflat doesn't have. This patch fixes the
problem with printf by pointing '#include "stdio.h"' to
lib/libcflat.h, where a printf declaration that libcflat actually
implements exists, rather than letting /usr/include/stdio.h get
included. This patch does not solve the general problem of potential
mismatches between libcflat and the compiler's notion standard include
files; doing so would require rewriting stdarg.h, stdint.h, and
several others in libcflat.

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 lib/stdio.h | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 lib/stdio.h

diff --git a/lib/stdio.h b/lib/stdio.h
new file mode 100644
index 000000000000..7b30db93f7f1
--- /dev/null
+++ b/lib/stdio.h
@@ -0,0 +1,4 @@
+#ifndef LIBCFLAT_STDIO_H
+#define LIBCFLAT_STDIO_H
+#include "libcflat.h"
+#endif
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 19/32] lib: added assert_msg macro
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (17 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 18/32] lib: provide stdio.h David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  7:18   ` Andrew Jones
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 20/32] lib: fix *printf return value David Matlack
                   ` (14 subsequent siblings)
  33 siblings, 1 reply; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 lib/libcflat.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 1d2eba980e1a..5d356df75f1f 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -126,6 +126,16 @@ do {									\
 	}								\
 } while (0)
 
+#define assert_msg(cond, fmt, args...)					\
+do {									\
+	if (!(cond)) {							\
+		printf("%s:%d: assert failed: %s: " fmt,		\
+		       __FILE__, __LINE__, #cond, ## args);		\
+		dump_stack();						\
+		abort();						\
+	}								\
+} while (0)
+
 static inline bool is_power_of_2(unsigned long n)
 {
 	return n && !(n & (n - 1));
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 20/32] lib: fix *printf return value
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (18 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 19/32] lib: added assert_msg macro David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  7:21   ` Andrew Jones
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 21/32] lib: printf-style report prefixes David Matlack
                   ` (13 subsequent siblings)
  33 siblings, 1 reply; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Was returning the number of characters printed plus, incorrectly, one
for the null terminator.

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 lib/printf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/printf.c b/lib/printf.c
index cecbeadc4440..aca2920887c1 100644
--- a/lib/printf.c
+++ b/lib/printf.c
@@ -221,7 +221,6 @@ int vsnprintf(char *buf, int size, const char *fmt, va_list va)
 	}
     }
     *s.buffer = 0;
-    ++s.added;
     return s.added;
 }
 
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 21/32] lib: printf-style report prefixes
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (19 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 20/32] lib: fix *printf return value David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  7:42   ` Andrew Jones
  2017-05-12 10:51   ` Thomas Huth
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 22/32] lib: add newline to assert_msg David Matlack
                   ` (12 subsequent siblings)
  33 siblings, 2 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 lib/libcflat.h |  1 +
 lib/report.c   | 38 +++++++++++++++++++++++++++++++++-----
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 5d356df75f1f..05c18543dd72 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -96,6 +96,7 @@ extern int vsnprintf(char *buf, int size, const char *fmt, va_list va)
 extern int vprintf(const char *fmt, va_list va)
 					__attribute__((format(printf, 1, 0)));
 
+void report_prefix_pushf(const char *prefix_fmt, ...);
 extern void report_prefix_push(const char *prefix);
 extern void report_prefix_pop(void);
 extern void report(const char *msg_fmt, bool pass, ...);
diff --git a/lib/report.c b/lib/report.c
index e24e81382f9e..1033f1e44e99 100644
--- a/lib/report.c
+++ b/lib/report.c
@@ -17,14 +17,42 @@ static unsigned int tests, failures, xfailures, skipped;
 static char prefixes[256];
 static struct spinlock lock;
 
-void report_prefix_push(const char *prefix)
+#define PREFIX_DELIMITER ": "
+
+void report_prefix_pushf(const char *prefix_fmt, ...)
 {
+	va_list va;
+	int len;
+	int start;
+
 	spin_lock(&lock);
-	strcat(prefixes, prefix);
-	strcat(prefixes, ": ");
+
+	len = strlen(prefixes);
+	assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
+	start = len;
+
+	va_start(va, prefix_fmt);
+	len += vsnprintf(&prefixes[len], sizeof(prefixes) - len, prefix_fmt,
+			 va);
+	va_end(va);
+	assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
+
+	assert_msg(!strstr(&prefixes[start], PREFIX_DELIMITER),
+		   "Prefix \"%s\" contains delimiter \"" PREFIX_DELIMITER "\"",
+		   &prefixes[start]);
+
+	len += snprintf(&prefixes[len], sizeof(prefixes) - len,
+			PREFIX_DELIMITER);
+	assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
+
 	spin_unlock(&lock);
 }
 
+void report_prefix_push(const char *prefix)
+{
+	report_prefix_pushf("%s", prefix);
+}
+
 void report_prefix_pop(void)
 {
 	char *p, *q;
@@ -34,9 +62,9 @@ void report_prefix_pop(void)
 	if (!*prefixes)
 		return;
 
-	for (p = prefixes, q = strstr(p, ": ") + 2;
+	for (p = prefixes, q = strstr(p, PREFIX_DELIMITER) + 2;
 			*q;
-			p = q, q = strstr(p, ": ") + 2)
+			p = q, q = strstr(p, PREFIX_DELIMITER) + 2)
 		;
 	*p = '\0';
 
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 22/32] lib: add newline to assert_msg
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (20 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 21/32] lib: printf-style report prefixes David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  7:44   ` Andrew Jones
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 23/32] lib: x86: store free pages in ascending order David Matlack
                   ` (11 subsequent siblings)
  33 siblings, 1 reply; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 lib/libcflat.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 05c18543dd72..7e4bce1dd335 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -130,7 +130,7 @@ do {									\
 #define assert_msg(cond, fmt, args...)					\
 do {									\
 	if (!(cond)) {							\
-		printf("%s:%d: assert failed: %s: " fmt,		\
+		printf("%s:%d: assert failed: %s: " fmt "\n",		\
 		       __FILE__, __LINE__, #cond, ## args);		\
 		dump_stack();						\
 		abort();						\
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 23/32] lib: x86: store free pages in ascending order
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (21 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 22/32] lib: add newline to assert_msg David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 24/32] lib: x86: multi-page allocator David Matlack
                   ` (10 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Makes allocation of contiguous physical pages simpler.

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 lib/x86/vm.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/lib/x86/vm.c b/lib/x86/vm.c
index cda4c5f46da3..8bd0c88e141e 100644
--- a/lib/x86/vm.c
+++ b/lib/x86/vm.c
@@ -7,12 +7,29 @@ static void *vfree_top = 0;
 
 static void free_memory(void *mem, unsigned long size)
 {
-    while (size >= PAGE_SIZE) {
-	*(void **)mem = free;
+	void *end;
+
+	assert_msg((unsigned long) mem % PAGE_SIZE == 0,
+		   "mem not page aligned: %p", mem);
+
+	assert_msg(size % PAGE_SIZE == 0, "size not page aligned: 0x%lx", size);
+
+	assert_msg(size == 0 || mem + size > mem,
+		   "mem + size overflow: %p + 0x%lx", mem, size);
+
+	if (size == 0) {
+		free = NULL;
+		return;
+	}
+
 	free = mem;
-	mem += PAGE_SIZE;
-	size -= PAGE_SIZE;
-    }
+	end = mem + size;
+	while (mem + PAGE_SIZE != end) {
+		*(void **)mem = (mem + PAGE_SIZE);
+		mem += PAGE_SIZE;
+	}
+
+	*(void **)mem = NULL;
 }
 
 void *alloc_page()
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 24/32] lib: x86: multi-page allocator
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (22 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 23/32] lib: x86: store free pages in ascending order David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 25/32] x86: ept assertions David Matlack
                   ` (9 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Need this for testing huge pages.

Change-Id: Iecf4f916a3d3d24844957dcd4e81c8aa10b1cdda
Signed-off-by: David Matlack <dmatlack@google.com>
---
 lib/x86/vm.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/x86/vm.h |  1 +
 2 files changed, 58 insertions(+)

diff --git a/lib/x86/vm.c b/lib/x86/vm.c
index 8bd0c88e141e..438ab9c70ffa 100644
--- a/lib/x86/vm.c
+++ b/lib/x86/vm.c
@@ -45,6 +45,63 @@ void *alloc_page()
     return p;
 }
 
+/*
+ * Allocates (1 << order) physically contiguous and naturally aligned pages.
+ * Returns NULL if there's no memory left.
+ */
+void *alloc_pages(unsigned long order)
+{
+	/* Generic list traversal. */
+	void *prev;
+	void *curr = NULL;
+	void *next = free;
+
+	/* Looking for a run of length (1 << order). */
+	unsigned long run = 0;
+	const unsigned long n = 1ul << order;
+	const unsigned long align_mask = (n << PAGE_SHIFT) - 1;
+	void *run_start = NULL;
+	void *run_prev = NULL;
+	unsigned long run_next_pa = 0;
+	unsigned long pa;
+
+	assert(order < sizeof(unsigned long) * 8);
+
+	for (;;) {
+		prev = curr;
+		curr = next;
+		next = curr ? *((void **) curr) : NULL;
+
+		if (!curr)
+			return 0;
+
+		pa = virt_to_phys(curr);
+
+		if (run == 0) {
+			if (!(pa & align_mask)) {
+				run_start = curr;
+				run_prev = prev;
+				run_next_pa = pa + PAGE_SIZE;
+				run = 1;
+			}
+		} else if (pa == run_next_pa) {
+			run_next_pa += PAGE_SIZE;
+			run += 1;
+		} else {
+			run = 0;
+		}
+
+		if (run == n) {
+			if (run_prev)
+				*((void **) run_prev) = next;
+			else
+				free = next;
+			return run_start;
+		}
+	}
+}
+
+
 void free_page(void *page)
 {
     *(void **)page = free;
diff --git a/lib/x86/vm.h b/lib/x86/vm.h
index 6a4384f5a48d..64e4b566333f 100644
--- a/lib/x86/vm.h
+++ b/lib/x86/vm.h
@@ -22,6 +22,7 @@ unsigned long *install_pte(unsigned long *cr3,
                            unsigned long *pt_page);
 
 void *alloc_page();
+void *alloc_pages(unsigned long order);
 void free_page(void *page);
 
 unsigned long *install_large_page(unsigned long *cr3,unsigned long phys,
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 25/32] x86: ept assertions
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (23 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 24/32] lib: x86: multi-page allocator David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  9:57   ` Paolo Bonzini
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 26/32] x86: setup_ept code motion David Matlack
                   ` (8 subsequent siblings)
  33 siblings, 1 reply; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

The upper bounds on level in set_ept_pte and install_ept_entry were
just wrong. There's nothing wrong with setting an EPT PTE at the
highest (512GiB) level.

Got rid of unused error return values and replaced them with
assertions. Nobody was checking the return values, so these functions
were silently failing.

Change-Id: I4db1e1c2a0c050f5f69cce28df460b4c8ce10d1c
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/vmx.c | 23 ++++++++++++-----------
 x86/vmx.h |  2 +-
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index cae650ae2e68..c003d5d11e63 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -724,6 +724,9 @@ void install_ept_entry(unsigned long *pml4,
 	unsigned long *pt = pml4;
 	unsigned offset;
 
+	/* EPT only uses 48 bits of GPA. */
+	assert(guest_addr < (1ul << 48));
+
 	for (level = EPT_PAGE_LEVEL; level > pte_level; --level) {
 		offset = (guest_addr >> EPT_LEVEL_SHIFT(level))
 				& EPT_PGDIR_MASK;
@@ -813,17 +816,17 @@ unsigned long get_ept_pte(unsigned long *pml4,
 	unsigned long *pt = pml4, pte;
 	unsigned offset;
 
-	if (level < 1 || level > 3)
-		return -1;
+	assert(level >= 1 && level <= 4);
+
 	for (l = EPT_PAGE_LEVEL; ; --l) {
 		offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 		pte = pt[offset];
 		if (!(pte & (EPT_PRESENT)))
-			return 0;
+			return -1;
 		if (l == level)
 			break;
 		if (l < 4 && (pte & EPT_LARGE_PAGE))
-			return pte;
+			return -1;
 		pt = (unsigned long *)(pte & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
@@ -950,26 +953,24 @@ void ept_sync(int type, u64 eptp)
 	}
 }
 
-int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
-		int level, u64 pte_val)
+void set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
+		 int level, u64 pte_val)
 {
 	int l;
 	unsigned long *pt = pml4;
 	unsigned offset;
 
-	if (level < 1 || level > 3)
-		return -1;
+	assert(level >= 1 && level <= 4);
+
 	for (l = EPT_PAGE_LEVEL; ; --l) {
 		offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 		if (l == level)
 			break;
-		if (!(pt[offset] & (EPT_PRESENT)))
-			return -1;
+		assert(pt[offset] & EPT_PRESENT);
 		pt = (unsigned long *)(pt[offset] & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 	pt[offset] = pte_val;
-	return 0;
 }
 
 void vpid_sync(int type, u16 vpid)
diff --git a/x86/vmx.h b/x86/vmx.h
index 966fff653561..4b899aa69145 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -691,7 +691,7 @@ void setup_ept_range(unsigned long *pml4, unsigned long start,
 		     unsigned long len, int map_1g, int map_2m, u64 perm);
 unsigned long get_ept_pte(unsigned long *pml4,
 		unsigned long guest_addr, int level);
-int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
+void set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 		int level, u64 pte_val);
 void check_ept_ad(unsigned long *pml4, u64 guest_cr3,
 		  unsigned long guest_addr, int expected_gpa_ad,
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 26/32] x86: setup_ept code motion
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (24 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 25/32] x86: ept assertions David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 27/32] x86: 2GiB RAM for vmx tests David Matlack
                   ` (7 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/vmx_tests.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 5758b6becfa6..be01709d3057 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -940,12 +940,20 @@ static int insn_intercept_exit_handler()
 }
 
 
+/* Enables EPT and sets up the identity map. */
 static int setup_ept(bool enable_ad)
 {
 	int support_2m;
 	unsigned long end_of_memory;
 	u32 ctrl_cpu[2];
 
+	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) ||
+	    !(ctrl_cpu_rev[1].clr & CPU_EPT)) {
+		printf("\tEPT is not supported");
+		return 1;
+	}
+
+
 	if (!(ept_vpid.val & EPT_CAP_UC) &&
 			!(ept_vpid.val & EPT_CAP_WB)) {
 		printf("\tEPT paging-structure memory type "
@@ -992,12 +1000,6 @@ static int apic_version;
 
 static int ept_init_common(bool have_ad)
 {
-	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) ||
-	    !(ctrl_cpu_rev[1].clr & CPU_EPT)) {
-		printf("\tEPT is not supported");
-		return VMX_TEST_EXIT;
-	}
-
 	if (setup_ept(have_ad))
 		return VMX_TEST_EXIT;
 	data_page1 = alloc_page();
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 27/32] x86: 2GiB RAM for vmx tests
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (25 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 26/32] x86: setup_ept code motion David Matlack
@ 2017-04-21  0:49 ` David Matlack
  2017-04-21  8:54   ` Paolo Bonzini
  2017-04-21  0:50 ` [kvm-unit-tests PATCH 28/32] x86: ept capability utilities David Matlack
                   ` (6 subsequent siblings)
  33 siblings, 1 reply; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:49 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Upcoming test allocates a 1GiB huge page. The VM needs more memory and
the default container limit of 2 GiB isn't enough.

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/unittests.cfg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index f79688af93f8..587bf40e8bc9 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -196,7 +196,7 @@ arch = x86_64
 
 [vmx]
 file = vmx.flat
-extra_params = -cpu host,+vmx -append -exit_monitor_from_l2_test
+extra_params = -cpu host,+vmx -m 2048 -append -exit_monitor_from_l2_test
 arch = x86_64
 groups = vmx
 
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 28/32] x86: ept capability utilities
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (26 preceding siblings ...)
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 27/32] x86: 2GiB RAM for vmx tests David Matlack
@ 2017-04-21  0:50 ` David Matlack
  2017-04-21  0:50 ` [kvm-unit-tests PATCH 29/32] lib: x86: page table utilities David Matlack
                   ` (5 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:50 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/vmx.c       | 30 ++++++++++++++++++++++++++++++
 x86/vmx.h       |  6 ++++++
 x86/vmx_tests.c |  6 ++----
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index c003d5d11e63..f7a34d20ab6a 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -973,6 +973,36 @@ void set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 	pt[offset] = pte_val;
 }
 
+bool ept_2m_supported(void)
+{
+	return ept_vpid.val & EPT_CAP_2M_PAGE;
+}
+
+bool ept_1g_supported(void)
+{
+	return ept_vpid.val & EPT_CAP_1G_PAGE;
+}
+
+bool ept_huge_pages_supported(int level)
+{
+	if (level == 2)
+		return ept_2m_supported();
+	else if (level == 3)
+		return ept_1g_supported();
+	else
+		return false;
+}
+
+bool ept_execute_only_supported(void)
+{
+	return ept_vpid.val & EPT_CAP_WT;
+}
+
+bool ept_ad_bits_supported(void)
+{
+	return ept_vpid.val & EPT_CAP_AD_FLAG;
+}
+
 void vpid_sync(int type, u16 vpid)
 {
 	switch(type) {
diff --git a/x86/vmx.h b/x86/vmx.h
index 4b899aa69145..890cdcc9e67f 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -699,6 +699,12 @@ void check_ept_ad(unsigned long *pml4, u64 guest_cr3,
 void clear_ept_ad(unsigned long *pml4, u64 guest_cr3,
 		  unsigned long guest_addr);
 
+bool ept_2m_supported(void);
+bool ept_1g_supported(void);
+bool ept_huge_pages_supported(int level);
+bool ept_execute_only_supported(void);
+bool ept_ad_bits_supported(void);
+
 void enter_guest(void);
 
 typedef void (*test_guest_func)(void);
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index be01709d3057..bf45bd2566ca 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -943,7 +943,6 @@ static int insn_intercept_exit_handler()
 /* Enables EPT and sets up the identity map. */
 static int setup_ept(bool enable_ad)
 {
-	int support_2m;
 	unsigned long end_of_memory;
 	u32 ctrl_cpu[2];
 
@@ -983,15 +982,14 @@ static int setup_ept(bool enable_ad)
 	if (enable_ad)
 		eptp |= EPTP_AD_FLAG;
 	vmcs_write(EPTP, eptp);
-	support_2m = !!(ept_vpid.val & EPT_CAP_2M_PAGE);
 	end_of_memory = fwcfg_get_u64(FW_CFG_RAM_SIZE);
 	if (end_of_memory < (1ul << 32))
 		end_of_memory = (1ul << 32);
 	/* Cannot use large EPT pages if we need to track EPT
 	 * accessed/dirty bits at 4K granularity.
 	 */
-	setup_ept_range(pml4, 0, end_of_memory,
-			0, !enable_ad && support_2m,
+	setup_ept_range(pml4, 0, end_of_memory, 0,
+			!enable_ad && ept_2m_supported(),
 			EPT_WA | EPT_RA | EPT_EA);
 	return 0;
 }
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 29/32] lib: x86: page table utilities
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (27 preceding siblings ...)
  2017-04-21  0:50 ` [kvm-unit-tests PATCH 28/32] x86: ept capability utilities David Matlack
@ 2017-04-21  0:50 ` David Matlack
  2017-04-21  0:50 ` [kvm-unit-tests PATCH 30/32] x86: ept access tests David Matlack
                   ` (4 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:50 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Convenience functions for manipulating and querying page tables.

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 lib/x86/vm.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++-----------
 lib/x86/vm.h | 27 +++++++++++++++++
 2 files changed, 108 insertions(+), 18 deletions(-)

diff --git a/lib/x86/vm.c b/lib/x86/vm.c
index 438ab9c70ffa..d52bb53fcf5e 100644
--- a/lib/x86/vm.c
+++ b/lib/x86/vm.c
@@ -139,23 +139,62 @@ unsigned long *install_pte(unsigned long *cr3,
     return &pt[offset];
 }
 
+/*
+ * Finds last PTE in the mapping of @virt that's at or above @lowest_level. The
+ * returned PTE isn't necessarily present, but its parent is.
+ */
+struct pte_search find_pte_level(unsigned long *cr3, void *virt,
+				 int lowest_level)
+{
+	unsigned long *pt = cr3, pte;
+	unsigned offset;
+	unsigned long shift;
+	struct pte_search r;
+
+	assert(lowest_level >= 1 && lowest_level <= PAGE_LEVEL);
+
+	for (r.level = PAGE_LEVEL;; --r.level) {
+		shift = (r.level - 1) * PGDIR_WIDTH + 12;
+		offset = ((unsigned long)virt >> shift) & PGDIR_MASK;
+		r.pte = &pt[offset];
+		pte = *r.pte;
+
+		if (!(pte & PT_PRESENT_MASK))
+			return r;
+
+		if ((r.level == 2 || r.level == 3) && (pte & PT_PAGE_SIZE_MASK))
+			return r;
+
+		if (r.level == lowest_level)
+			return r;
+
+		pt = phys_to_virt(pte & 0xffffffffff000ull);
+	}
+}
+
+/*
+ * Returns the leaf PTE in the mapping of @virt (i.e., 4K PTE or a present huge
+ * PTE). Returns NULL if no leaf PTE exists.
+ */
 unsigned long *get_pte(unsigned long *cr3, void *virt)
 {
-    int level;
-    unsigned long *pt = cr3, pte;
-    unsigned offset;
+	struct pte_search search;
 
-    for (level = PAGE_LEVEL; level > 1; --level) {
-	offset = ((unsigned long)virt >> (((level-1) * PGDIR_WIDTH) + 12)) & PGDIR_MASK;
-	pte = pt[offset];
-	if (!(pte & PT_PRESENT_MASK))
-	    return NULL;
-	if (level == 2 && (pte & PT_PAGE_SIZE_MASK))
-	    return &pt[offset];
-	pt = phys_to_virt(pte & PT_ADDR_MASK);
-    }
-    offset = ((unsigned long)virt >> (((level-1) * PGDIR_WIDTH) + 12)) & PGDIR_MASK;
-    return &pt[offset];
+	search = find_pte_level(cr3, virt, 1);
+	return found_leaf_pte(search) ? search.pte : NULL;
+}
+
+/*
+ * Returns the PTE in the mapping of @virt at the given level @pte_level.
+ * Returns NULL if the PT at @pte_level isn't present (i.e., the mapping at
+ * @pte_level - 1 isn't present).
+ */
+unsigned long *get_pte_level(unsigned long *cr3, void *virt, int pte_level)
+{
+	struct pte_search search;
+
+	search = find_pte_level(cr3, virt, pte_level);
+	return search.level == pte_level ? search.pte : NULL;
 }
 
 unsigned long *install_large_page(unsigned long *cr3,
@@ -173,6 +212,33 @@ unsigned long *install_page(unsigned long *cr3,
     return install_pte(cr3, 1, virt, phys | PT_PRESENT_MASK | PT_WRITABLE_MASK | PT_USER_MASK, 0);
 }
 
+void install_pages(unsigned long *cr3, unsigned long phys, unsigned long len,
+		   void *virt)
+{
+	unsigned long max = (u64)len + (u64)phys;
+	assert(phys % PAGE_SIZE == 0);
+	assert((unsigned long) virt % PAGE_SIZE == 0);
+	assert(len % PAGE_SIZE == 0);
+
+	while (phys + PAGE_SIZE <= max) {
+		install_page(cr3, phys, virt);
+		phys += PAGE_SIZE;
+		virt = (char *) virt + PAGE_SIZE;
+	}
+}
+
+bool any_present_pages(unsigned long *cr3, void *virt, unsigned long len)
+{
+	unsigned long max = (unsigned long) virt + len;
+	unsigned long curr;
+
+	for (curr = (unsigned long) virt; curr < max; curr += PAGE_SIZE) {
+		unsigned long *ptep = get_pte(cr3, (void *) curr);
+		if (ptep && (*ptep & PT_PRESENT_MASK))
+			return true;
+	}
+	return false;
+}
 
 static void setup_mmu_range(unsigned long *cr3, unsigned long start,
 			    unsigned long len)
@@ -184,10 +250,7 @@ static void setup_mmu_range(unsigned long *cr3, unsigned long start,
 		install_large_page(cr3, phys, (void *)(ulong)phys);
 		phys += LARGE_PAGE_SIZE;
 	}
-	while (phys + PAGE_SIZE <= max) {
-		install_page(cr3, phys, (void *)(ulong)phys);
-		phys += PAGE_SIZE;
-	}
+	install_pages(cr3, phys, max - phys, (void *)(ulong)phys);
 }
 
 static void setup_mmu(unsigned long len)
diff --git a/lib/x86/vm.h b/lib/x86/vm.h
index 64e4b566333f..3522ba8b99b0 100644
--- a/lib/x86/vm.h
+++ b/lib/x86/vm.h
@@ -14,7 +14,27 @@ void *alloc_vpage(void);
 void *alloc_vpages(ulong nr);
 uint64_t virt_to_phys_cr3(void *mem);
 
+struct pte_search {
+	int level;
+	unsigned long *pte;
+};
+
+static inline bool found_huge_pte(struct pte_search search)
+{
+	return (search.level == 2 || search.level == 3) &&
+	       (*search.pte & PT_PRESENT_MASK) &&
+	       (*search.pte & PT_PAGE_SIZE_MASK);
+}
+
+static inline bool found_leaf_pte(struct pte_search search)
+{
+	return search.level == 1 || found_huge_pte(search);
+}
+
+struct pte_search find_pte_level(unsigned long *cr3, void *virt,
+				 int lowest_level);
 unsigned long *get_pte(unsigned long *cr3, void *virt);
+unsigned long *get_pte_level(unsigned long *cr3, void *virt, int pte_level);
 unsigned long *install_pte(unsigned long *cr3,
                            int pte_level,
                            void *virt,
@@ -28,5 +48,12 @@ void free_page(void *page);
 unsigned long *install_large_page(unsigned long *cr3,unsigned long phys,
                                   void *virt);
 unsigned long *install_page(unsigned long *cr3, unsigned long phys, void *virt);
+void install_pages(unsigned long *cr3, unsigned long phys, unsigned long len,
+		   void *virt);
+bool any_present_pages(unsigned long *cr3, void *virt, unsigned long len);
 
+static inline void *current_page_table(void)
+{
+	return phys_to_virt(read_cr3());
+}
 #endif
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 30/32] x86: ept access tests
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (28 preceding siblings ...)
  2017-04-21  0:50 ` [kvm-unit-tests PATCH 29/32] lib: x86: page table utilities David Matlack
@ 2017-04-21  0:50 ` David Matlack
  2017-04-21  0:50 ` [kvm-unit-tests PATCH 31/32] x86: force ept 2m test David Matlack
                   ` (3 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:50 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 lib/x86/asm/page.h |   2 +
 x86/unittests.cfg  | 114 ++++++++
 x86/vmx.h          |   3 +
 x86/vmx_tests.c    | 843 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 962 insertions(+)

diff --git a/lib/x86/asm/page.h b/lib/x86/asm/page.h
index c43bab28ca2e..562594df2450 100644
--- a/lib/x86/asm/page.h
+++ b/lib/x86/asm/page.h
@@ -31,6 +31,8 @@
 #define PT64_NX_MASK		(1ull << 63)
 #define PT_ADDR_MASK		GENMASK_ULL(51, 12)
 
+#define PT_AD_MASK              (PT_ACCESSED_MASK | PT_DIRTY_MASK)
+
 #ifdef __x86_64__
 #define	PAGE_LEVEL	4
 #define	PGDIR_WIDTH	9
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 587bf40e8bc9..bc245651ce80 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -368,6 +368,120 @@ extra_params = -cpu host,+vmx -append "v2_null_test v2_multiple_entries_test fix
 arch = x86_64
 groups = vmx
 
+[vmx_ept_access_test_not_present]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_not_present
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_read_only]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_read_only
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_write_only]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_write_only
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_read_write]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_read_write
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_execute_only]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_execute_only
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_read_execute]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_read_execute
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_write_execute]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_write_execute
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_read_write_execute]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_read_write_execute
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_reserved_bits]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_reserved_bits
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_ignored_bits]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_ignored_bits
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_paddr_not_present_ad_disabled]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_paddr_not_present_ad_disabled
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_paddr_not_present_ad_enabled]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_paddr_not_present_ad_enabled
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_paddr_read_only_ad_disabled]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_paddr_read_only_ad_disabled
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_paddr_read_only_ad_enabled]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_paddr_read_only_ad_enabled
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_paddr_read_write]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_paddr_read_write
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_paddr_read_write_execute]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_paddr_read_write_execute
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_paddr_read_execute_ad_disabled]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_paddr_read_execute_ad_disabled
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_paddr_read_execute_ad_enabled]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_paddr_read_execute_ad_enabled
+arch = x86_64
+groups = vmx
+
+[vmx_ept_access_test_paddr_not_present_page_fault]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_paddr_not_present_page_fault
+arch = x86_64
+groups = vmx
+
 [debug]
 file = debug.flat
 arch = x86_64
diff --git a/x86/vmx.h b/x86/vmx.h
index 890cdcc9e67f..2390c4a99901 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -551,12 +551,15 @@ enum vm_instruction_error_number {
 #define EPT_VLT_PERM_RD		(1 << 3)
 #define EPT_VLT_PERM_WR		(1 << 4)
 #define EPT_VLT_PERM_EX		(1 << 5)
+#define EPT_VLT_PERMS		(EPT_VLT_PERM_RD | EPT_VLT_PERM_WR | \
+				 EPT_VLT_PERM_EX)
 #define EPT_VLT_LADDR_VLD	(1 << 7)
 #define EPT_VLT_PADDR		(1 << 8)
 
 #define MAGIC_VAL_1		0x12345678ul
 #define MAGIC_VAL_2		0x87654321ul
 #define MAGIC_VAL_3		0xfffffffful
+#define MAGIC_VAL_4		0xdeadbeeful
 
 #define INVEPT_SINGLE		1
 #define INVEPT_GLOBAL		2
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index bf45bd2566ca..91bcef99d1d9 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -994,6 +994,25 @@ static int setup_ept(bool enable_ad)
 	return 0;
 }
 
+static void ept_enable_ad_bits(void)
+{
+	eptp |= EPTP_AD_FLAG;
+	vmcs_write(EPTP, eptp);
+}
+
+static void ept_disable_ad_bits(void)
+{
+	eptp &= ~EPTP_AD_FLAG;
+	vmcs_write(EPTP, eptp);
+}
+
+static void ept_enable_ad_bits_or_skip_test(void)
+{
+	if (!ept_ad_bits_supported())
+		test_skip("EPT AD bits not supported.");
+	ept_enable_ad_bits();
+}
+
 static int apic_version;
 
 static int ept_init_common(bool have_ad)
@@ -1984,6 +2003,810 @@ static void fixture_test_case2(void)
 	report(__func__, 1);
 }
 
+enum ept_access_op {
+	OP_READ,
+	OP_WRITE,
+	OP_EXEC,
+	OP_FLUSH_TLB,
+	OP_EXIT,
+};
+
+static struct ept_access_test_data {
+	unsigned long gpa;
+	unsigned long *gva;
+	unsigned long hpa;
+	unsigned long *hva;
+	enum ept_access_op op;
+} ept_access_test_data;
+
+extern unsigned char ret42_start;
+extern unsigned char ret42_end;
+
+/* Returns 42. */
+asm(
+	".align 64\n"
+	"ret42_start:\n"
+	"mov $42, %eax\n"
+	"ret\n"
+	"ret42_end:\n"
+);
+
+static void
+diagnose_ept_violation_qual(u64 expected, u64 actual)
+{
+
+#define DIAGNOSE(flag)							\
+do {									\
+	if ((expected & flag) != (actual & flag))			\
+		printf(#flag " %sexpected\n",				\
+		       (expected & flag) ? "" : "un");			\
+} while (0)
+
+	DIAGNOSE(EPT_VLT_RD);
+	DIAGNOSE(EPT_VLT_WR);
+	DIAGNOSE(EPT_VLT_FETCH);
+	DIAGNOSE(EPT_VLT_PERM_RD);
+	DIAGNOSE(EPT_VLT_PERM_WR);
+	DIAGNOSE(EPT_VLT_PERM_EX);
+	DIAGNOSE(EPT_VLT_LADDR_VLD);
+	DIAGNOSE(EPT_VLT_PADDR);
+
+#undef DIAGNOSE
+}
+
+static void do_ept_access_op(enum ept_access_op op)
+{
+	ept_access_test_data.op = op;
+	enter_guest();
+}
+
+/*
+ * Force the guest to flush its TLB (i.e., flush gva -> gpa mappings). Only
+ * needed by tests that modify guest PTEs.
+ */
+static void ept_access_test_guest_flush_tlb(void)
+{
+	do_ept_access_op(OP_FLUSH_TLB);
+	skip_exit_vmcall();
+}
+
+/*
+ * Modifies the EPT entry at @level in the mapping of @gpa. First clears the
+ * bits in @clear then sets the bits in @set. @mkhuge transforms the entry into
+ * a huge page.
+ */
+static unsigned long ept_twiddle(unsigned long gpa, bool mkhuge, int level,
+				 unsigned long clear, unsigned long set)
+{
+	struct ept_access_test_data *data = &ept_access_test_data;
+	unsigned long orig_pte;
+	unsigned long pte;
+
+	/* Screw with the mapping at the requested level. */
+	orig_pte = get_ept_pte(pml4, gpa, level);
+	TEST_ASSERT(orig_pte != -1);
+	pte = orig_pte;
+	if (mkhuge)
+		pte = (orig_pte & ~EPT_ADDR_MASK) | data->hpa | EPT_LARGE_PAGE;
+	else
+		pte = orig_pte;
+	pte = (pte & ~clear) | set;
+	set_ept_pte(pml4, gpa, level, pte);
+	ept_sync(INVEPT_SINGLE, eptp);
+
+	return orig_pte;
+}
+
+static void ept_untwiddle(unsigned long gpa, int level, unsigned long orig_pte)
+{
+	set_ept_pte(pml4, gpa, level, orig_pte);
+}
+
+static void do_ept_violation(bool leaf, enum ept_access_op op,
+			     u64 expected_qual, u64 expected_paddr)
+{
+	u64 qual;
+
+	/* Try the access and observe the violation. */
+	do_ept_access_op(op);
+
+	assert_exit_reason(VMX_EPT_VIOLATION);
+
+	qual = vmcs_read(EXI_QUALIFICATION);
+
+	/* Hack now so important tests can pass. */
+	if (!leaf && (expected_qual & EPT_VLT_PERM_RD)
+	    && !(expected_qual & EPT_VLT_PERM_EX))
+		expected_qual = (expected_qual & ~EPT_VLT_PERM_RD) |
+				EPT_VLT_PERM_EX;
+
+	diagnose_ept_violation_qual(expected_qual, qual);
+	TEST_EXPECT_EQ(expected_qual, qual);
+
+	#if 0
+	/* Disable for now otherwise every test will fail */
+	TEST_EXPECT_EQ(vmcs_read(GUEST_LINEAR_ADDRESS),
+		       (unsigned long) (
+			       op == OP_EXEC ? data->gva + 1 : data->gva));
+	#endif
+	/*
+	 * TODO: tests that probe expected_paddr in pages other than the one at
+	 * the beginning of the 1g region.
+	 */
+	TEST_EXPECT_EQ(vmcs_read(INFO_PHYS_ADDR), expected_paddr);
+}
+
+static void
+ept_violation_at_level_mkhuge(bool mkhuge, int level, unsigned long clear,
+			      unsigned long set, enum ept_access_op op,
+			      u64 expected_qual)
+{
+	struct ept_access_test_data *data = &ept_access_test_data;
+	unsigned long orig_pte;
+
+	orig_pte = ept_twiddle(data->gpa, mkhuge, level, clear, set);
+
+	do_ept_violation(level == 1 || mkhuge, op, expected_qual,
+			 op == OP_EXEC ? data->gpa + sizeof(unsigned long) :
+					 data->gpa);
+
+	/* Fix the violation and resume the op loop. */
+	ept_untwiddle(data->gpa, level, orig_pte);
+	enter_guest();
+	skip_exit_vmcall();
+}
+
+static void
+ept_violation_at_level(int level, unsigned long clear, unsigned long set,
+		       enum ept_access_op op, u64 expected_qual)
+{
+	ept_violation_at_level_mkhuge(false, level, clear, set, op,
+				      expected_qual);
+	if (ept_huge_pages_supported(level))
+		ept_violation_at_level_mkhuge(true, level, clear, set, op,
+					      expected_qual);
+}
+
+static void ept_violation(unsigned long clear, unsigned long set,
+			  enum ept_access_op op, u64 expected_qual)
+{
+	ept_violation_at_level(1, clear, set, op, expected_qual);
+	ept_violation_at_level(2, clear, set, op, expected_qual);
+	ept_violation_at_level(3, clear, set, op, expected_qual);
+	ept_violation_at_level(4, clear, set, op, expected_qual);
+}
+
+static void ept_access_violation(unsigned long access, enum ept_access_op op,
+				       u64 expected_qual)
+{
+	ept_violation(EPT_PRESENT, access, op,
+		      expected_qual | EPT_VLT_LADDR_VLD | EPT_VLT_PADDR);
+}
+
+/*
+ * For translations that don't involve a GVA, that is physical address (paddr)
+ * accesses, EPT violations don't set the flag EPT_VLT_PADDR.  For a typical
+ * guest memory access, the hardware does GVA -> GPA -> HPA.  However, certain
+ * translations don't involve GVAs, such as when the hardware does the guest
+ * page table walk. For example, in translating GVA_1 -> GPA_1, the guest MMU
+ * might try to set an A bit on a guest PTE. If the GPA_2 that the PTE resides
+ * on isn't present in the EPT, then the EPT violation will be for GPA_2 and
+ * the EPT_VLT_PADDR bit will be clear in the exit qualification.
+ *
+ * Note that paddr violations can also be triggered by loading PAE page tables
+ * with wonky addresses. We don't test that yet.
+ *
+ * This function modifies the EPT entry that maps the GPA that the guest page
+ * table entry mapping ept_access_data.gva resides on.
+ *
+ *	@ept_access	EPT permissions to set. Other permissions are cleared.
+ *
+ *	@pte_ad		Set the A/D bits on the guest PTE accordingly.
+ *
+ *	@op		Guest operation to perform with ept_access_data.gva.
+ *
+ *	@expect_violation
+ *			Is a violation expected during the paddr access?
+ *
+ *	@expected_qual	Expected qualification for the EPT violation.
+ *			EPT_VLT_PADDR should be clear.
+ */
+static void ept_access_paddr(unsigned long ept_access, unsigned long pte_ad,
+			     enum ept_access_op op, bool expect_violation,
+			     u64 expected_qual)
+{
+	struct ept_access_test_data *data = &ept_access_test_data;
+	unsigned long *ptep;
+	unsigned long gpa;
+	unsigned long orig_epte;
+
+	/* Modify the guest PTE mapping data->gva according to @pte_ad.  */
+	ptep = get_pte_level(current_page_table(), data->gva, /*level=*/1);
+	TEST_ASSERT(ptep);
+	TEST_ASSERT_EQ(*ptep & PT_ADDR_MASK, data->gpa);
+	*ptep = (*ptep & ~PT_AD_MASK) | pte_ad;
+	ept_access_test_guest_flush_tlb();
+
+	/*
+	 * Now modify the access bits on the EPT entry for the GPA that the
+	 * guest PTE resides on. Note that by modifying a single EPT entry,
+	 * we're potentially affecting 512 guest PTEs. However, we've carefully
+	 * constructed our test such that those other 511 PTEs aren't used by
+	 * the guest: data->gva is at the beginning of a 1G huge page, thus the
+	 * PTE we're modifying is at the beginning of a 4K page and the
+	 * following 511 entires are also under our control (and not touched by
+	 * the guest).
+	 */
+	gpa = virt_to_phys(ptep);
+	TEST_ASSERT_EQ(gpa & ~PAGE_MASK, 0);
+	/*
+	 * Make sure the guest page table page is mapped with a 4K EPT entry,
+	 * otherwise our level=1 twiddling below will fail. We use the
+	 * identity map (gpa = gpa) since page tables are shared with the host.
+	 */
+	install_ept(pml4, gpa, gpa, EPT_PRESENT);
+	orig_epte = ept_twiddle(gpa, /*mkhuge=*/0, /*level=*/1,
+				/*clear=*/EPT_PRESENT, /*set=*/ept_access);
+
+	if (expect_violation)
+		do_ept_violation(/*leaf=*/true, op,
+				 expected_qual | EPT_VLT_LADDR_VLD, gpa);
+
+	ept_untwiddle(gpa, /*level=*/1, orig_epte);
+	do_ept_access_op(op);
+
+	TEST_ASSERT(*ptep & PT_ACCESSED_MASK);
+	if ((pte_ad & PT_DIRTY_MASK) || op == OP_WRITE)
+		TEST_ASSERT(*ptep & PT_DIRTY_MASK);
+
+	skip_exit_vmcall();
+}
+
+static void ept_access_allowed_paddr(unsigned long ept_access,
+				     unsigned long pte_ad,
+				     enum ept_access_op op)
+{
+	ept_access_paddr(ept_access, pte_ad, op, /*expect_violation=*/false,
+			 /*expected_qual=*/-1);
+}
+
+static void ept_access_violation_paddr(unsigned long ept_access,
+				       unsigned long pte_ad,
+				       enum ept_access_op op,
+				       u64 expected_qual)
+{
+	ept_access_paddr(ept_access, pte_ad, op, /*expect_violation=*/true,
+			 expected_qual);
+}
+
+
+static void ept_allowed_at_level_mkhuge(bool mkhuge, int level,
+					unsigned long clear,
+					unsigned long set,
+					enum ept_access_op op)
+{
+	struct ept_access_test_data *data = &ept_access_test_data;
+	unsigned long orig_pte;
+
+	orig_pte = ept_twiddle(data->gpa, mkhuge, level, clear, set);
+
+	/* No violation. Should proceed to vmcall. */
+	do_ept_access_op(op);
+	skip_exit_vmcall();
+
+	ept_untwiddle(data->gpa, level, orig_pte);
+}
+
+static void ept_allowed_at_level(int level, unsigned long clear,
+				 unsigned long set, enum ept_access_op op)
+{
+	ept_allowed_at_level_mkhuge(false, level, clear, set, op);
+	if (ept_huge_pages_supported(level))
+		ept_allowed_at_level_mkhuge(true, level, clear, set, op);
+}
+
+static void ept_allowed(unsigned long clear, unsigned long set,
+			enum ept_access_op op)
+{
+	ept_allowed_at_level(1, clear, set, op);
+	ept_allowed_at_level(2, clear, set, op);
+	ept_allowed_at_level(3, clear, set, op);
+	ept_allowed_at_level(4, clear, set, op);
+}
+
+static void ept_ignored_bit(int bit)
+{
+	/* Set the bit. */
+	ept_allowed(0, 1ul << bit, OP_READ);
+	ept_allowed(0, 1ul << bit, OP_WRITE);
+	ept_allowed(0, 1ul << bit, OP_EXEC);
+
+	/* Clear the bit. */
+	ept_allowed(1ul << bit, 0, OP_READ);
+	ept_allowed(1ul << bit, 0, OP_WRITE);
+	ept_allowed(1ul << bit, 0, OP_EXEC);
+}
+
+static void ept_access_allowed(unsigned long access, enum ept_access_op op)
+{
+	ept_allowed(EPT_PRESENT, access, op);
+}
+
+
+static void ept_misconfig_at_level_mkhuge_op(bool mkhuge, int level,
+					     unsigned long clear,
+					     unsigned long set,
+					     enum ept_access_op op)
+{
+	struct ept_access_test_data *data = &ept_access_test_data;
+	unsigned long orig_pte;
+
+	orig_pte = ept_twiddle(data->gpa, mkhuge, level, clear, set);
+
+	do_ept_access_op(op);
+	assert_exit_reason(VMX_EPT_MISCONFIG);
+
+	/* Intel 27.2.1, "For all other VM exits, this field is cleared." */
+	#if 0
+	/* broken: */
+	TEST_EXPECT_EQ_MSG(vmcs_read(EXI_QUALIFICATION), 0);
+	#endif
+	#if 0
+	/*
+	 * broken:
+	 * According to description of exit qual for EPT violation,
+	 * EPT_VLT_LADDR_VLD indicates if GUEST_LINEAR_ADDRESS is valid.
+	 * However, I can't find anything that says GUEST_LINEAR_ADDRESS ought
+	 * to be set for msiconfig.
+	 */
+	TEST_EXPECT_EQ(vmcs_read(GUEST_LINEAR_ADDRESS),
+		       (unsigned long) (
+			       op == OP_EXEC ? data->gva + 1 : data->gva));
+	#endif
+
+	/* Fix the violation and resume the op loop. */
+	ept_untwiddle(data->gpa, level, orig_pte);
+	enter_guest();
+	skip_exit_vmcall();
+}
+
+static void ept_misconfig_at_level_mkhuge(bool mkhuge, int level,
+					  unsigned long clear,
+					  unsigned long set)
+{
+	/* The op shouldn't matter (read, write, exec), so try them all! */
+	ept_misconfig_at_level_mkhuge_op(mkhuge, level, clear, set, OP_READ);
+	ept_misconfig_at_level_mkhuge_op(mkhuge, level, clear, set, OP_WRITE);
+	ept_misconfig_at_level_mkhuge_op(mkhuge, level, clear, set, OP_EXEC);
+}
+
+static void ept_misconfig_at_level(int level, unsigned long clear,
+				   unsigned long set)
+{
+	ept_misconfig_at_level_mkhuge(false, level, clear, set);
+	if (ept_huge_pages_supported(level))
+		ept_misconfig_at_level_mkhuge(true, level, clear, set);
+}
+
+static void ept_misconfig(unsigned long clear, unsigned long set)
+{
+	ept_misconfig_at_level(1, clear, set);
+	ept_misconfig_at_level(2, clear, set);
+	ept_misconfig_at_level(3, clear, set);
+	ept_misconfig_at_level(4, clear, set);
+}
+
+static void ept_access_misconfig(unsigned long access)
+{
+	ept_misconfig(EPT_PRESENT, access);
+}
+
+static void ept_reserved_bit_at_level_nohuge(int level, int bit)
+{
+	/* Setting the bit causes a misconfig. */
+	ept_misconfig_at_level_mkhuge(false, level, 0, 1ul << bit);
+
+	/* Making the entry non-present turns reserved bits into ignored. */
+	ept_violation_at_level(level, EPT_PRESENT, 1ul << bit, OP_READ,
+			       EPT_VLT_RD | EPT_VLT_LADDR_VLD | EPT_VLT_PADDR);
+}
+
+static void ept_reserved_bit_at_level_huge(int level, int bit)
+{
+	/* Setting the bit causes a misconfig. */
+	ept_misconfig_at_level_mkhuge(true, level, 0, 1ul << bit);
+
+	/* Making the entry non-present turns reserved bits into ignored. */
+	ept_violation_at_level(level, EPT_PRESENT, 1ul << bit, OP_READ,
+			       EPT_VLT_RD | EPT_VLT_LADDR_VLD | EPT_VLT_PADDR);
+}
+
+static void ept_reserved_bit_at_level(int level, int bit)
+{
+	/* Setting the bit causes a misconfig. */
+	ept_misconfig_at_level(level, 0, 1ul << bit);
+
+	/* Making the entry non-present turns reserved bits into ignored. */
+	ept_violation_at_level(level, EPT_PRESENT, 1ul << bit, OP_READ,
+			       EPT_VLT_RD | EPT_VLT_LADDR_VLD | EPT_VLT_PADDR);
+}
+
+static void ept_reserved_bit(int bit)
+{
+	ept_reserved_bit_at_level(1, bit);
+	ept_reserved_bit_at_level(2, bit);
+	ept_reserved_bit_at_level(3, bit);
+	ept_reserved_bit_at_level(4, bit);
+}
+
+#define PAGE_2M_ORDER 9
+#define PAGE_1G_ORDER 18
+
+static void *get_1g_page(void)
+{
+	static void *alloc;
+
+	if (!alloc)
+		alloc = alloc_pages(PAGE_1G_ORDER);
+	return alloc;
+}
+
+static void ept_access_test_teardown(void *unused)
+{
+	/* Exit the guest cleanly. */
+	do_ept_access_op(OP_EXIT);
+}
+
+static void ept_access_test_guest(void)
+{
+	struct ept_access_test_data *data = &ept_access_test_data;
+	int (*code)(void) = (int (*)(void)) &data->gva[1];
+
+	while (true) {
+		switch (data->op) {
+		case OP_READ:
+			TEST_ASSERT_EQ(*data->gva, MAGIC_VAL_1);
+			break;
+		case OP_WRITE:
+			*data->gva = MAGIC_VAL_2;
+			TEST_ASSERT_EQ(*data->gva, MAGIC_VAL_2);
+			*data->gva = MAGIC_VAL_1;
+			break;
+		case OP_EXEC:
+			TEST_ASSERT_EQ(42, code());
+			break;
+		case OP_FLUSH_TLB:
+			write_cr3(read_cr3());
+			break;
+		case OP_EXIT:
+			return;
+		default:
+			TEST_ASSERT_MSG(false, "Unknown op %d", data->op);
+		}
+		vmcall();
+	}
+}
+
+static void ept_access_test_setup(void)
+{
+	struct ept_access_test_data *data = &ept_access_test_data;
+	unsigned long npages = 1ul << PAGE_1G_ORDER;
+	unsigned long size = npages * PAGE_SIZE;
+	unsigned long *page_table = current_page_table();
+
+	if (setup_ept(false))
+		test_skip("EPT not supported");
+
+	test_set_guest(ept_access_test_guest);
+	test_add_teardown(ept_access_test_teardown, NULL);
+
+	data->hva = get_1g_page();
+	TEST_ASSERT(data->hva);
+	data->hpa = virt_to_phys(data->hva);
+
+	data->gpa = 1ul << 40;
+	data->gva = (void *) ALIGN((unsigned long) alloc_vpages(npages * 2),
+				   size);
+	TEST_ASSERT(!any_present_pages(page_table, data->gva, size));
+	install_pages(page_table, data->gpa, size, data->gva);
+
+	/*
+	 * Make sure nothing's mapped here so the tests that screw with the
+	 * pml4 entry don't inadvertently break something.
+	 */
+	TEST_ASSERT_EQ(get_ept_pte(pml4, data->gpa, 4), -1);
+	TEST_ASSERT_EQ(get_ept_pte(pml4, data->gpa + size - 1, 4), -1);
+	install_ept(pml4, data->hpa, data->gpa, EPT_PRESENT);
+
+	data->hva[0] = MAGIC_VAL_1;
+	memcpy(&data->hva[1], &ret42_start, &ret42_end - &ret42_start);
+}
+
+static void ept_access_test_not_present(void)
+{
+	ept_access_test_setup();
+	/* --- */
+	ept_access_violation(0, OP_READ, EPT_VLT_RD);
+	ept_access_violation(0, OP_WRITE, EPT_VLT_WR);
+	ept_access_violation(0, OP_EXEC, EPT_VLT_FETCH);
+}
+
+static void ept_access_test_read_only(void)
+{
+	ept_access_test_setup();
+
+	/* r-- */
+	ept_access_allowed(EPT_RA, OP_READ);
+	ept_access_violation(EPT_RA, OP_WRITE, EPT_VLT_WR | EPT_VLT_PERM_RD);
+	ept_access_violation(EPT_RA, OP_EXEC, EPT_VLT_FETCH | EPT_VLT_PERM_RD);
+}
+
+static void ept_access_test_write_only(void)
+{
+	ept_access_test_setup();
+	/* -w- */
+	ept_access_misconfig(EPT_WA);
+}
+
+static void ept_access_test_read_write(void)
+{
+	ept_access_test_setup();
+	/* rw- */
+	ept_access_allowed(EPT_RA | EPT_WA, OP_READ);
+	ept_access_allowed(EPT_RA | EPT_WA, OP_WRITE);
+	ept_access_violation(EPT_RA | EPT_WA, OP_EXEC,
+			   EPT_VLT_FETCH | EPT_VLT_PERM_RD | EPT_VLT_PERM_WR);
+}
+
+
+static void ept_access_test_execute_only(void)
+{
+	ept_access_test_setup();
+	/* --x */
+	if (ept_execute_only_supported()) {
+		ept_access_violation(EPT_EA, OP_READ,
+				     EPT_VLT_RD | EPT_VLT_PERM_EX);
+		ept_access_violation(EPT_EA, OP_WRITE,
+				     EPT_VLT_WR | EPT_VLT_PERM_EX);
+		ept_access_allowed(EPT_EA, OP_EXEC);
+	} else {
+		ept_access_misconfig(EPT_EA);
+	}
+}
+
+static void ept_access_test_read_execute(void)
+{
+	ept_access_test_setup();
+	/* r-x */
+	ept_access_allowed(EPT_RA | EPT_EA, OP_READ);
+	ept_access_violation(EPT_RA | EPT_EA, OP_WRITE,
+			   EPT_VLT_WR | EPT_VLT_PERM_RD | EPT_VLT_PERM_EX);
+	ept_access_allowed(EPT_RA | EPT_EA, OP_EXEC);
+}
+
+static void ept_access_test_write_execute(void)
+{
+	ept_access_test_setup();
+	/* -wx */
+	ept_access_misconfig(EPT_WA | EPT_EA);
+}
+
+static void ept_access_test_read_write_execute(void)
+{
+	ept_access_test_setup();
+	/* rwx */
+	ept_access_allowed(EPT_RA | EPT_WA | EPT_EA, OP_READ);
+	ept_access_allowed(EPT_RA | EPT_WA | EPT_EA, OP_WRITE);
+	ept_access_allowed(EPT_RA | EPT_WA | EPT_EA, OP_EXEC);
+}
+
+static void ept_access_test_reserved_bits(void)
+{
+	int i;
+	int maxphyaddr;
+
+	ept_access_test_setup();
+
+	/* Reserved bits above maxphyaddr. */
+	maxphyaddr = cpuid_maxphyaddr();
+	for (i = maxphyaddr; i <= 51; i++) {
+		report_prefix_pushf("reserved_bit=%d", i);
+		ept_reserved_bit(i);
+		report_prefix_pop();
+	}
+
+	/* Level-specific reserved bits. */
+	ept_reserved_bit_at_level_nohuge(2, 3);
+	ept_reserved_bit_at_level_nohuge(2, 4);
+	ept_reserved_bit_at_level_nohuge(2, 5);
+	ept_reserved_bit_at_level_nohuge(2, 6);
+	/* 2M alignment. */
+	for (i = 12; i < 20; i++) {
+		report_prefix_pushf("reserved_bit=%d", i);
+		ept_reserved_bit_at_level_huge(2, i);
+		report_prefix_pop();
+	}
+	ept_reserved_bit_at_level_nohuge(3, 3);
+	ept_reserved_bit_at_level_nohuge(3, 4);
+	ept_reserved_bit_at_level_nohuge(3, 5);
+	ept_reserved_bit_at_level_nohuge(3, 6);
+	/* 1G alignment. */
+	for (i = 12; i < 29; i++) {
+		report_prefix_pushf("reserved_bit=%d", i);
+		ept_reserved_bit_at_level_huge(3, i);
+		report_prefix_pop();
+	}
+	ept_reserved_bit_at_level(4, 3);
+	ept_reserved_bit_at_level(4, 4);
+	ept_reserved_bit_at_level(4, 5);
+	ept_reserved_bit_at_level(4, 6);
+	ept_reserved_bit_at_level(4, 7);
+}
+
+static void ept_access_test_ignored_bits(void)
+{
+	ept_access_test_setup();
+	/*
+	 * Bits ignored at every level. Bits 8 and 9 (A and D) are ignored as
+	 * far as translation is concerned even if AD bits are enabled in the
+	 * EPTP. Bit 63 is ignored because "EPT-violation #VE" VM-execution
+	 * control is 0.
+	 */
+	ept_ignored_bit(8);
+	ept_ignored_bit(9);
+	ept_ignored_bit(10);
+	ept_ignored_bit(11);
+	ept_ignored_bit(52);
+	ept_ignored_bit(53);
+	ept_ignored_bit(54);
+	ept_ignored_bit(55);
+	ept_ignored_bit(56);
+	ept_ignored_bit(57);
+	ept_ignored_bit(58);
+	ept_ignored_bit(59);
+	ept_ignored_bit(60);
+	ept_ignored_bit(61);
+	ept_ignored_bit(62);
+	ept_ignored_bit(63);
+}
+
+static void ept_access_test_paddr_not_present_ad_disabled(void)
+{
+	ept_access_test_setup();
+	ept_disable_ad_bits();
+
+	ept_access_violation_paddr(0, PT_AD_MASK, OP_READ, EPT_VLT_RD);
+	ept_access_violation_paddr(0, PT_AD_MASK, OP_WRITE, EPT_VLT_RD);
+	ept_access_violation_paddr(0, PT_AD_MASK, OP_EXEC, EPT_VLT_RD);
+}
+
+static void ept_access_test_paddr_not_present_ad_enabled(void)
+{
+	ept_access_test_setup();
+	ept_enable_ad_bits_or_skip_test();
+
+	ept_access_violation_paddr(0, PT_AD_MASK, OP_READ, EPT_VLT_WR);
+	ept_access_violation_paddr(0, PT_AD_MASK, OP_WRITE, EPT_VLT_WR);
+	ept_access_violation_paddr(0, PT_AD_MASK, OP_EXEC, EPT_VLT_WR);
+}
+
+static void ept_access_test_paddr_read_only_ad_disabled(void)
+{
+	u64 qual = EPT_VLT_WR | EPT_VLT_PERM_RD;
+
+	ept_access_test_setup();
+	ept_disable_ad_bits();
+
+	/* Can't update A bit, so all accesses fail. */
+	ept_access_violation_paddr(EPT_RA, 0, OP_READ, qual);
+	ept_access_violation_paddr(EPT_RA, 0, OP_WRITE, qual);
+	ept_access_violation_paddr(EPT_RA, 0, OP_EXEC, qual);
+	/* AD bits disabled, so only writes try to update the D bit. */
+	ept_access_allowed_paddr(EPT_RA, PT_ACCESSED_MASK, OP_READ);
+	ept_access_violation_paddr(EPT_RA, PT_ACCESSED_MASK, OP_WRITE, qual);
+	ept_access_allowed_paddr(EPT_RA, PT_ACCESSED_MASK, OP_EXEC);
+	/* Both A and D already set, so read-only is OK. */
+	ept_access_allowed_paddr(EPT_RA, PT_AD_MASK, OP_READ);
+	ept_access_allowed_paddr(EPT_RA, PT_AD_MASK, OP_WRITE);
+	ept_access_allowed_paddr(EPT_RA, PT_AD_MASK, OP_EXEC);
+}
+
+static void ept_access_test_paddr_read_only_ad_enabled(void)
+{
+	/*
+	 * When EPT AD bits are enabled, all accesses to guest paging
+	 * structures are considered writes as far as EPT translation
+	 * is concerned.
+	 */
+	u64 qual = EPT_VLT_WR | EPT_VLT_PERM_RD;
+
+	ept_access_test_setup();
+	ept_enable_ad_bits_or_skip_test();
+
+	ept_access_violation_paddr(EPT_RA, 0, OP_READ, qual);
+	ept_access_violation_paddr(EPT_RA, 0, OP_WRITE, qual);
+	ept_access_violation_paddr(EPT_RA, 0, OP_EXEC, qual);
+	ept_access_violation_paddr(EPT_RA, PT_ACCESSED_MASK, OP_READ, qual);
+	ept_access_violation_paddr(EPT_RA, PT_ACCESSED_MASK, OP_WRITE, qual);
+	ept_access_violation_paddr(EPT_RA, PT_ACCESSED_MASK, OP_EXEC, qual);
+	ept_access_violation_paddr(EPT_RA, PT_AD_MASK, OP_READ, qual);
+	ept_access_violation_paddr(EPT_RA, PT_AD_MASK, OP_WRITE, qual);
+	ept_access_violation_paddr(EPT_RA, PT_AD_MASK, OP_EXEC, qual);
+}
+
+static void ept_access_test_paddr_read_write(void)
+{
+	ept_access_test_setup();
+	/* Read-write access to paging structure. */
+	ept_access_allowed_paddr(EPT_RA | EPT_WA, 0, OP_READ);
+	ept_access_allowed_paddr(EPT_RA | EPT_WA, 0, OP_WRITE);
+	ept_access_allowed_paddr(EPT_RA | EPT_WA, 0, OP_EXEC);
+}
+
+static void ept_access_test_paddr_read_write_execute(void)
+{
+	ept_access_test_setup();
+	/* RWX access to paging structure. */
+	ept_access_allowed_paddr(EPT_PRESENT, 0, OP_READ);
+	ept_access_allowed_paddr(EPT_PRESENT, 0, OP_WRITE);
+	ept_access_allowed_paddr(EPT_PRESENT, 0, OP_EXEC);
+}
+
+static void ept_access_test_paddr_read_execute_ad_disabled(void)
+{
+	u64 qual = EPT_VLT_WR | EPT_VLT_PERM_RD | EPT_VLT_PERM_EX;
+
+	ept_access_test_setup();
+	ept_disable_ad_bits();
+
+	/* Can't update A bit, so all accesses fail. */
+	ept_access_violation_paddr(EPT_RA | EPT_EA, 0, OP_READ, qual);
+	ept_access_violation_paddr(EPT_RA | EPT_EA, 0, OP_WRITE, qual);
+	ept_access_violation_paddr(EPT_RA | EPT_EA, 0, OP_EXEC, qual);
+	/* AD bits disabled, so only writes try to update the D bit. */
+	ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_READ);
+	ept_access_violation_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_WRITE, qual);
+	ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_EXEC);
+	/* Both A and D already set, so read-only is OK. */
+	ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_READ);
+	ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_WRITE);
+	ept_access_allowed_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_EXEC);
+}
+
+static void ept_access_test_paddr_read_execute_ad_enabled(void)
+{
+	/*
+	 * When EPT AD bits are enabled, all accesses to guest paging
+	 * structures are considered writes as far as EPT translation
+	 * is concerned.
+	 */
+	u64 qual = EPT_VLT_WR | EPT_VLT_PERM_RD | EPT_VLT_PERM_EX;
+
+	ept_access_test_setup();
+	ept_enable_ad_bits_or_skip_test();
+
+	ept_access_violation_paddr(EPT_RA | EPT_EA, 0, OP_READ, qual);
+	ept_access_violation_paddr(EPT_RA | EPT_EA, 0, OP_WRITE, qual);
+	ept_access_violation_paddr(EPT_RA | EPT_EA, 0, OP_EXEC, qual);
+	ept_access_violation_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_READ, qual);
+	ept_access_violation_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_WRITE, qual);
+	ept_access_violation_paddr(EPT_RA | EPT_EA, PT_ACCESSED_MASK, OP_EXEC, qual);
+	ept_access_violation_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_READ, qual);
+	ept_access_violation_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_WRITE, qual);
+	ept_access_violation_paddr(EPT_RA | EPT_EA, PT_AD_MASK, OP_EXEC, qual);
+}
+
+static void ept_access_test_paddr_not_present_page_fault(void)
+{
+	ept_access_test_setup();
+	/*
+	 * TODO: test no EPT violation as long as guest PF occurs. e.g., GPA is
+	 * page is read-only in EPT but GVA is also mapped read only in PT.
+	 * Thus guest page fault before host takes EPT violation for trying to
+	 * update A bit.
+	 */
+}
+
 #define TEST(name) { #name, .v2 = name }
 
 /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
@@ -2023,5 +2846,25 @@ struct vmx_test vmx_tests[] = {
 	TEST(v2_multiple_entries_test),
 	TEST(fixture_test_case1),
 	TEST(fixture_test_case2),
+	/* EPT access tests. */
+	TEST(ept_access_test_not_present),
+	TEST(ept_access_test_read_only),
+	TEST(ept_access_test_write_only),
+	TEST(ept_access_test_read_write),
+	TEST(ept_access_test_execute_only),
+	TEST(ept_access_test_read_execute),
+	TEST(ept_access_test_write_execute),
+	TEST(ept_access_test_read_write_execute),
+	TEST(ept_access_test_reserved_bits),
+	TEST(ept_access_test_ignored_bits),
+	TEST(ept_access_test_paddr_not_present_ad_disabled),
+	TEST(ept_access_test_paddr_not_present_ad_enabled),
+	TEST(ept_access_test_paddr_read_only_ad_disabled),
+	TEST(ept_access_test_paddr_read_only_ad_enabled),
+	TEST(ept_access_test_paddr_read_write),
+	TEST(ept_access_test_paddr_read_write_execute),
+	TEST(ept_access_test_paddr_read_execute_ad_disabled),
+	TEST(ept_access_test_paddr_read_execute_ad_enabled),
+	TEST(ept_access_test_paddr_not_present_page_fault),
 	{ NULL, NULL, NULL, NULL, NULL, {0} },
 };
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 31/32] x86: force ept 2m test
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (29 preceding siblings ...)
  2017-04-21  0:50 ` [kvm-unit-tests PATCH 30/32] x86: ept access tests David Matlack
@ 2017-04-21  0:50 ` David Matlack
  2017-04-21  0:50 ` [kvm-unit-tests PATCH 32/32] lib: add report_pass David Matlack
                   ` (2 subsequent siblings)
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:50 UTC (permalink / raw)
  To: kvm; +Cc: Peter Feiner, David Matlack

From: Peter Feiner <pfeiner@google.com>

EPT tests don't exercise 2m support unless it's advertised properly.
This test asserts that we actually advertise it and that some basic 2m
stuff works.

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/unittests.cfg |  6 ++++++
 x86/vmx_tests.c   | 13 +++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index bc245651ce80..09b2252ac2af 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -482,6 +482,12 @@ extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_paddr_not_present_
 arch = x86_64
 groups = vmx
 
+[vmx_ept_access_test_force_2m_page]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_force_2m_page
+arch = x86_64
+groups = vmx
+
 [debug]
 file = debug.flat
 arch = x86_64
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index c4cd3f96001d..d59ca0baf48a 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -2807,6 +2807,18 @@ static void ept_access_test_paddr_not_present_page_fault(void)
 	 */
 }
 
+static void ept_access_test_force_2m_page(void)
+{
+	ept_access_test_setup();
+
+	TEST_ASSERT_EQ(ept_2m_supported(), true);
+	ept_allowed_at_level_mkhuge(true, 2, 0, 0, OP_READ);
+	ept_violation_at_level_mkhuge(true, 2, EPT_PRESENT, EPT_RA, OP_WRITE,
+				      EPT_VLT_WR | EPT_VLT_PERM_RD |
+				      EPT_VLT_LADDR_VLD | EPT_VLT_PADDR);
+	ept_misconfig_at_level_mkhuge(true, 2, EPT_PRESENT, EPT_WA);
+}
+
 #define TEST(name) { #name, .v2 = name }
 
 /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
@@ -2866,5 +2878,6 @@ struct vmx_test vmx_tests[] = {
 	TEST(ept_access_test_paddr_read_execute_ad_disabled),
 	TEST(ept_access_test_paddr_read_execute_ad_enabled),
 	TEST(ept_access_test_paddr_not_present_page_fault),
+	TEST(ept_access_test_force_2m_page),
 	{ NULL, NULL, NULL, NULL, NULL, {0} },
 };
-- 
2.12.2.816.g2cccc81164-goog

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

* [kvm-unit-tests PATCH 32/32] lib: add report_pass
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (30 preceding siblings ...)
  2017-04-21  0:50 ` [kvm-unit-tests PATCH 31/32] x86: force ept 2m test David Matlack
@ 2017-04-21  0:50 ` David Matlack
  2017-04-21  1:01 ` [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests Wanpeng Li
  2017-05-02 14:00 ` Paolo Bonzini
  33 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21  0:50 UTC (permalink / raw)
  To: kvm; +Cc: David Matlack

The assertion macros in the vmx tests only call report() if the test
fails. This avoids a lot of logging (e.g. vmx_ept_access_test_reserved_bits
checks 3460 assertions). However the test runner interprets passing runs
as "skipped" because it looks like no testcases were run. Add a function
called report_pass(), which lets tests report that a test passed without
printing anything to the console.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 lib/libcflat.h | 1 +
 lib/report.c   | 7 +++++++
 x86/vmx.h      | 3 +++
 3 files changed, 11 insertions(+)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 7e4bce1dd335..b1ea5e607033 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -104,6 +104,7 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
 extern void report_abort(const char *msg_fmt, ...);
 extern void report_skip(const char *msg_fmt, ...);
 extern void report_info(const char *msg_fmt, ...);
+extern void report_pass(void);
 extern int report_summary(void);
 
 bool simple_glob(const char *text, const char *pattern);
diff --git a/lib/report.c b/lib/report.c
index 1033f1e44e99..aa23b6582512 100644
--- a/lib/report.c
+++ b/lib/report.c
@@ -19,6 +19,13 @@ static struct spinlock lock;
 
 #define PREFIX_DELIMITER ": "
 
+void report_pass(void)
+{
+	spin_lock(&lock);
+	tests++;
+	spin_unlock(&lock);
+}
+
 void report_prefix_pushf(const char *prefix_fmt, ...)
 {
 	va_list va;
diff --git a/x86/vmx.h b/x86/vmx.h
index 2390c4a99901..4194429e07b8 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -726,6 +726,7 @@ do { \
 		dump_stack(); \
 		__abort_test(); \
 	} \
+	report_pass(); \
 } while (0)
 
 #define TEST_ASSERT_MSG(cond, fmt, args...) \
@@ -736,6 +737,7 @@ do { \
 		dump_stack(); \
 		__abort_test(); \
 	} \
+	report_pass(); \
 } while (0)
 
 #define __TEST_EQ(a, b, a_str, b_str, assertion, fmt, args...) \
@@ -759,6 +761,7 @@ do { \
 		if (assertion) \
 			__abort_test(); \
 	} \
+	report_pass(); \
 } while (0)
 
 #define TEST_ASSERT_EQ(a, b) __TEST_EQ(a, b, #a, #b, 1, "")
-- 
2.12.2.816.g2cccc81164-goog

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

* Re: [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (31 preceding siblings ...)
  2017-04-21  0:50 ` [kvm-unit-tests PATCH 32/32] lib: add report_pass David Matlack
@ 2017-04-21  1:01 ` Wanpeng Li
  2017-05-02 14:00 ` Paolo Bonzini
  33 siblings, 0 replies; 52+ messages in thread
From: Wanpeng Li @ 2017-04-21  1:01 UTC (permalink / raw)
  To: David Matlack; +Cc: kvm

2017-04-21 8:49 GMT+08:00 David Matlack <dmatlack@google.com>:
> Patches 1-6 split out the VMX tests into their own unit tests.
>
> Patches 7-12 add new VMX unit tests.
>
> Patches 13-17 implement a new VMX test framework ("v2"). Instead of
> registering a main function and an exit handler, entering and exiting
> guest mode is accomplished by calling a function, enter_guest().
>
> Patches 18-22 add some assert and printf enhancements to lib/.
>
> Patches 23-32 add EPT access tests, using the v2 framework.
>
> 4 of the EPT access tests reveal KVM bugs (4.11-rc6, on a Haswell CPU):
>         vmx_ept_access_test_execute_only
>         vmx_ept_access_test_paddr_not_present_ad_disabled
>         vmx_ept_access_test_paddr_read_only_ad_disabled
>         vmx_ept_access_test_paddr_read_execute_ad_disabled

If the bugs are still present after apply Paolo's nested EPT A/D bit?
I can look into it if they are still present.

Regards,
Wanpeng Li

>
> The rest of the new tests in this series pass.
>
> David Matlack (8):
>   x86: move basic vmx tests into separate test cases
>   x86: add missing vmx test cases to unittests.cfg
>   x86: vmx: filter exit_monitor_from_l2_test from full vmx test
>   x86: basic vmwrite/vmread test
>   x86: vmcs lifecycle test
>   x86: test VMCS in memory after VMCLEAR
>   x86: test VMPTRLD does not drop VMWRITEs
>   lib: add report_pass
>
> GanShun (1):
>   x86: Adding gtest to check correct instruction error is returned
>
> Peter Feiner (23):
>   x86: add test filter to vmx.flat.
>   x86: add config for each vmx unit test case
>   lib: better test name filtering
>   x86: test exit while vmcs02 is loaded
>   x86: don't special case vmx null test
>   x86: factor out vmx_enter_guest
>   x86: binstr utility function
>   x86: vmx exit reason descriptions
>   x86: v2 vmx test framework
>   lib: provide stdio.h
>   lib: added assert_msg macro
>   lib: fix *printf return value
>   lib: printf-style report prefixes
>   lib: add newline to assert_msg
>   lib: x86: store free pages in ascending order
>   lib: x86: multi-page allocator
>   x86: ept assertions
>   x86: setup_ept code motion
>   x86: 2GiB RAM for vmx tests
>   x86: ept capability utilities
>   lib: x86: page table utilities
>   x86: ept access tests
>   x86: force ept 2m test
>
>  lib/libcflat.h     |  21 ++
>  lib/printf.c       |  31 +-
>  lib/report.c       |  45 ++-
>  lib/stdio.h        |   4 +
>  lib/string.c       |  41 +++
>  lib/x86/asm/page.h |   2 +
>  lib/x86/vm.c       | 183 ++++++++--
>  lib/x86/vm.h       |  28 ++
>  x86/unittests.cfg  | 291 +++++++++++++++-
>  x86/vmx.c          | 833 +++++++++++++++++++++++++++++++++++++++-----
>  x86/vmx.h          | 151 +++++++-
>  x86/vmx_tests.c    | 989 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  12 files changed, 2493 insertions(+), 126 deletions(-)
>  create mode 100644 lib/stdio.h
>
> --
> 2.12.2.816.g2cccc81164-goog
>

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

* Re: [kvm-unit-tests PATCH 18/32] lib: provide stdio.h
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 18/32] lib: provide stdio.h David Matlack
@ 2017-04-21  7:16   ` Andrew Jones
  2017-04-21  8:24     ` Paolo Bonzini
  2017-04-21 19:40     ` David Matlack
  0 siblings, 2 replies; 52+ messages in thread
From: Andrew Jones @ 2017-04-21  7:16 UTC (permalink / raw)
  To: David Matlack; +Cc: kvm, Peter Feiner

On Thu, Apr 20, 2017 at 05:49:50PM -0700, David Matlack wrote:
> From: Peter Feiner <pfeiner@google.com>
> 
> kvm-unit-tests builds with the odd combination of system include files
> but not system libraries. That is, -nostdlib is specified but
> -nostdinc isn't! Problems arise when the include files don't match the
> libraries, e.g., printf() in some versions of glibc linking in a
> __printf_chk symbol that libcflat doesn't have. This patch fixes the
> problem with printf by pointing '#include "stdio.h"' to
> lib/libcflat.h, where a printf declaration that libcflat actually
> implements exists, rather than letting /usr/include/stdio.h get
> included. This patch does not solve the general problem of potential
> mismatches between libcflat and the compiler's notion standard include
> files; doing so would require rewriting stdarg.h, stdint.h, and
> several others in libcflat.

Yeah, I'm not sure rewriting those would be worth being able to add
-nostdinc. When we cross compile ARM and POWER those few headers are
some of the ones provided by the cross compiler.

> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>  lib/stdio.h | 4 ++++
>  1 file changed, 4 insertions(+)
>  create mode 100644 lib/stdio.h
> 
> diff --git a/lib/stdio.h b/lib/stdio.h
> new file mode 100644
> index 000000000000..7b30db93f7f1
> --- /dev/null
> +++ b/lib/stdio.h
> @@ -0,0 +1,4 @@
> +#ifndef LIBCFLAT_STDIO_H
> +#define LIBCFLAT_STDIO_H
> +#include "libcflat.h"
> +#endif
> -- 
> 2.12.2.816.g2cccc81164-goog
>

I'd rather we move the stdio functions (just printf?) into stdio.h rather
than redirect to libcflat. We can include stdio.h from libcflat.h to
avoid updating all the unit tests for now though.

Thanks,
drew

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

* Re: [kvm-unit-tests PATCH 19/32] lib: added assert_msg macro
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 19/32] lib: added assert_msg macro David Matlack
@ 2017-04-21  7:18   ` Andrew Jones
  0 siblings, 0 replies; 52+ messages in thread
From: Andrew Jones @ 2017-04-21  7:18 UTC (permalink / raw)
  To: David Matlack; +Cc: kvm, Peter Feiner

On Thu, Apr 20, 2017 at 05:49:51PM -0700, David Matlack wrote:
> From: Peter Feiner <pfeiner@google.com>
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>  lib/libcflat.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)

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

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

* Re: [kvm-unit-tests PATCH 20/32] lib: fix *printf return value
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 20/32] lib: fix *printf return value David Matlack
@ 2017-04-21  7:21   ` Andrew Jones
  0 siblings, 0 replies; 52+ messages in thread
From: Andrew Jones @ 2017-04-21  7:21 UTC (permalink / raw)
  To: David Matlack; +Cc: kvm, Peter Feiner

On Thu, Apr 20, 2017 at 05:49:52PM -0700, David Matlack wrote:
> From: Peter Feiner <pfeiner@google.com>
> 
> Was returning the number of characters printed plus, incorrectly, one
> for the null terminator.
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>  lib/printf.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/lib/printf.c b/lib/printf.c
> index cecbeadc4440..aca2920887c1 100644
> --- a/lib/printf.c
> +++ b/lib/printf.c
> @@ -221,7 +221,6 @@ int vsnprintf(char *buf, int size, const char *fmt, va_list va)
>  	}
>      }
>      *s.buffer = 0;
> -    ++s.added;
>      return s.added;
>  }
>  
> -- 
> 2.12.2.816.g2cccc81164-goog
>

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

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

* Re: [kvm-unit-tests PATCH 21/32] lib: printf-style report prefixes
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 21/32] lib: printf-style report prefixes David Matlack
@ 2017-04-21  7:42   ` Andrew Jones
  2017-05-12 10:51   ` Thomas Huth
  1 sibling, 0 replies; 52+ messages in thread
From: Andrew Jones @ 2017-04-21  7:42 UTC (permalink / raw)
  To: David Matlack; +Cc: kvm, Peter Feiner

On Thu, Apr 20, 2017 at 05:49:53PM -0700, David Matlack wrote:
> From: Peter Feiner <pfeiner@google.com>
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>  lib/libcflat.h |  1 +
>  lib/report.c   | 38 +++++++++++++++++++++++++++++++++-----
>  2 files changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 5d356df75f1f..05c18543dd72 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -96,6 +96,7 @@ extern int vsnprintf(char *buf, int size, const char *fmt, va_list va)
>  extern int vprintf(const char *fmt, va_list va)
>  					__attribute__((format(printf, 1, 0)));
>  
> +void report_prefix_pushf(const char *prefix_fmt, ...);
>  extern void report_prefix_push(const char *prefix);
>  extern void report_prefix_pop(void);
>  extern void report(const char *msg_fmt, bool pass, ...);
> diff --git a/lib/report.c b/lib/report.c
> index e24e81382f9e..1033f1e44e99 100644
> --- a/lib/report.c
> +++ b/lib/report.c
> @@ -17,14 +17,42 @@ static unsigned int tests, failures, xfailures, skipped;
>  static char prefixes[256];
>  static struct spinlock lock;
>  
> -void report_prefix_push(const char *prefix)
> +#define PREFIX_DELIMITER ": "
> +
> +void report_prefix_pushf(const char *prefix_fmt, ...)
>  {
> +	va_list va;
> +	int len;
> +	int start;
> +
>  	spin_lock(&lock);
> -	strcat(prefixes, prefix);
> -	strcat(prefixes, ": ");
> +
> +	len = strlen(prefixes);
> +	assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
> +	start = len;
> +
> +	va_start(va, prefix_fmt);
> +	len += vsnprintf(&prefixes[len], sizeof(prefixes) - len, prefix_fmt,
> +			 va);
> +	va_end(va);
> +	assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
> +
> +	assert_msg(!strstr(&prefixes[start], PREFIX_DELIMITER),
> +		   "Prefix \"%s\" contains delimiter \"" PREFIX_DELIMITER "\"",
> +		   &prefixes[start]);
> +
> +	len += snprintf(&prefixes[len], sizeof(prefixes) - len,
> +			PREFIX_DELIMITER);
> +	assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));

I don't think this last assert is necessary.

> +
>  	spin_unlock(&lock);
>  }
>  
> +void report_prefix_push(const char *prefix)
> +{
> +	report_prefix_pushf("%s", prefix);
> +}
> +
>  void report_prefix_pop(void)
>  {
>  	char *p, *q;
> @@ -34,9 +62,9 @@ void report_prefix_pop(void)
>  	if (!*prefixes)
>  		return;
>  
> -	for (p = prefixes, q = strstr(p, ": ") + 2;
> +	for (p = prefixes, q = strstr(p, PREFIX_DELIMITER) + 2;
>  			*q;
> -			p = q, q = strstr(p, ": ") + 2)
> +			p = q, q = strstr(p, PREFIX_DELIMITER) + 2)
>  		;
>  	*p = '\0';
>  
> -- 
> 2.12.2.816.g2cccc81164-goog
>

Otherwise

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

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

* Re: [kvm-unit-tests PATCH 22/32] lib: add newline to assert_msg
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 22/32] lib: add newline to assert_msg David Matlack
@ 2017-04-21  7:44   ` Andrew Jones
  2017-04-21  8:25     ` Paolo Bonzini
  0 siblings, 1 reply; 52+ messages in thread
From: Andrew Jones @ 2017-04-21  7:44 UTC (permalink / raw)
  To: David Matlack; +Cc: kvm, Peter Feiner

On Thu, Apr 20, 2017 at 05:49:54PM -0700, David Matlack wrote:
> From: Peter Feiner <pfeiner@google.com>
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>  lib/libcflat.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 05c18543dd72..7e4bce1dd335 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -130,7 +130,7 @@ do {									\
>  #define assert_msg(cond, fmt, args...)					\
>  do {									\
>  	if (!(cond)) {							\
> -		printf("%s:%d: assert failed: %s: " fmt,		\
> +		printf("%s:%d: assert failed: %s: " fmt "\n",		\
>  		       __FILE__, __LINE__, #cond, ## args);		\
>  		dump_stack();						\
>  		abort();						\
> -- 
> 2.12.2.816.g2cccc81164-goog
>

Should be squashed in with the introduction of assert_msg 

Thanks,
drew

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

* Re: [kvm-unit-tests PATCH 18/32] lib: provide stdio.h
  2017-04-21  7:16   ` Andrew Jones
@ 2017-04-21  8:24     ` Paolo Bonzini
  2017-04-21 19:40     ` David Matlack
  1 sibling, 0 replies; 52+ messages in thread
From: Paolo Bonzini @ 2017-04-21  8:24 UTC (permalink / raw)
  To: Andrew Jones, David Matlack; +Cc: kvm, Peter Feiner



On 21/04/2017 09:16, Andrew Jones wrote:
>>
>> kvm-unit-tests builds with the odd combination of system include files
>> but not system libraries. That is, -nostdlib is specified but
>> -nostdinc isn't! Problems arise when the include files don't match the
>> libraries, e.g., printf() in some versions of glibc linking in a
>> __printf_chk symbol that libcflat doesn't have. This patch fixes the
>> problem with printf by pointing '#include "stdio.h"' to
>> lib/libcflat.h, where a printf declaration that libcflat actually
>> implements exists, rather than letting /usr/include/stdio.h get
>> included. This patch does not solve the general problem of potential
>> mismatches between libcflat and the compiler's notion standard include
>> files; doing so would require rewriting stdarg.h, stdint.h, and
>> several others in libcflat.
> 
> Yeah, I'm not sure rewriting those would be worth being able to add
> -nostdinc. When we cross compile ARM and POWER those few headers are
> some of the ones provided by the cross compiler.

I think -sysroot=`pwd` would keep the usual compiler include path, and
remove /usr/include.

Paolo

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

* Re: [kvm-unit-tests PATCH 22/32] lib: add newline to assert_msg
  2017-04-21  7:44   ` Andrew Jones
@ 2017-04-21  8:25     ` Paolo Bonzini
  0 siblings, 0 replies; 52+ messages in thread
From: Paolo Bonzini @ 2017-04-21  8:25 UTC (permalink / raw)
  To: Andrew Jones, David Matlack; +Cc: kvm, Peter Feiner



On 21/04/2017 09:44, Andrew Jones wrote:
> On Thu, Apr 20, 2017 at 05:49:54PM -0700, David Matlack wrote:
>> From: Peter Feiner <pfeiner@google.com>
>>
>> Signed-off-by: Peter Feiner <pfeiner@google.com>
>> Signed-off-by: David Matlack <dmatlack@google.com>
>> ---
>>  lib/libcflat.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/libcflat.h b/lib/libcflat.h
>> index 05c18543dd72..7e4bce1dd335 100644
>> --- a/lib/libcflat.h
>> +++ b/lib/libcflat.h
>> @@ -130,7 +130,7 @@ do {									\
>>  #define assert_msg(cond, fmt, args...)					\
>>  do {									\
>>  	if (!(cond)) {							\
>> -		printf("%s:%d: assert failed: %s: " fmt,		\
>> +		printf("%s:%d: assert failed: %s: " fmt "\n",		\
>>  		       __FILE__, __LINE__, #cond, ## args);		\
>>  		dump_stack();						\
>>  		abort();						\
>> -- 
>> 2.12.2.816.g2cccc81164-goog
>>
> 
> Should be squashed in with the introduction of assert_msg 

Indeed, can do when applying too.

Paolo

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

* Re: [kvm-unit-tests PATCH 27/32] x86: 2GiB RAM for vmx tests
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 27/32] x86: 2GiB RAM for vmx tests David Matlack
@ 2017-04-21  8:54   ` Paolo Bonzini
  2017-04-21 19:33     ` David Matlack
  0 siblings, 1 reply; 52+ messages in thread
From: Paolo Bonzini @ 2017-04-21  8:54 UTC (permalink / raw)
  To: David Matlack, kvm; +Cc: Peter Feiner



On 21/04/2017 02:49, David Matlack wrote:
> From: Peter Feiner <pfeiner@google.com>
> 
> Upcoming test allocates a 1GiB huge page. The VM needs more memory and
> the default container limit of 2 GiB isn't enough.

Google-y commit message? :)

Paolo

> Signed-off-by: Peter Feiner <pfeiner@google.com>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>  x86/unittests.cfg | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> index f79688af93f8..587bf40e8bc9 100644
> --- a/x86/unittests.cfg
> +++ b/x86/unittests.cfg
> @@ -196,7 +196,7 @@ arch = x86_64
>  
>  [vmx]
>  file = vmx.flat
> -extra_params = -cpu host,+vmx -append -exit_monitor_from_l2_test
> +extra_params = -cpu host,+vmx -m 2048 -append -exit_monitor_from_l2_test

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

* Re: [kvm-unit-tests PATCH 06/32] x86: vmx: filter exit_monitor_from_l2_test from full vmx test
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 06/32] x86: vmx: filter exit_monitor_from_l2_test from full vmx test David Matlack
@ 2017-04-21  8:55   ` Paolo Bonzini
  0 siblings, 0 replies; 52+ messages in thread
From: Paolo Bonzini @ 2017-04-21  8:55 UTC (permalink / raw)
  To: David Matlack, kvm

On 21/04/2017 02:49, David Matlack wrote:
> Filter out exit_monitor_from_l2_test when running the full vmx test
> suite (i.e. the "[vmx]" test suite in unittest.cfg). Since
> exit_monitor_from_l2_test causes the VM to shutdown, many of the tests
> in vmx_tests.c end up getting skipped.
> 
> Signed-off-by: David Matlack <dmatlack@google.com>

This can be squashed into patch 7, since the test doesn't exist yet.

Paolo

> ---
>  x86/unittests.cfg | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> index 02dd6b235739..8011429d2307 100644
> --- a/x86/unittests.cfg
> +++ b/x86/unittests.cfg
> @@ -196,7 +196,7 @@ arch = x86_64
>  
>  [vmx]
>  file = vmx.flat
> -extra_params = -cpu host,+vmx
> +extra_params = -cpu host,+vmx -append -exit_monitor_from_l2_test
>  arch = x86_64
>  groups = vmx
>  
> 

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

* Re: [kvm-unit-tests PATCH 25/32] x86: ept assertions
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 25/32] x86: ept assertions David Matlack
@ 2017-04-21  9:57   ` Paolo Bonzini
  0 siblings, 0 replies; 52+ messages in thread
From: Paolo Bonzini @ 2017-04-21  9:57 UTC (permalink / raw)
  To: David Matlack, kvm; +Cc: Peter Feiner



On 21/04/2017 02:49, David Matlack wrote:
> From: Peter Feiner <pfeiner@google.com>
> 
> The upper bounds on level in set_ept_pte and install_ept_entry were
> just wrong. There's nothing wrong with setting an EPT PTE at the
> highest (512GiB) level.

Right, I think the idea was that the biggest supported page size is 1GB,
not 512, but setting up "super-huge" EPT pages can even be desirable for
testing purposes.

Paolo

> Got rid of unused error return values and replaced them with
> assertions. Nobody was checking the return values, so these functions
> were silently failing.
> 
> Change-Id: I4db1e1c2a0c050f5f69cce28df460b4c8ce10d1c
> Signed-off-by: David Matlack <dmatlack@google.com>

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

* Re: [kvm-unit-tests PATCH 27/32] x86: 2GiB RAM for vmx tests
  2017-04-21  8:54   ` Paolo Bonzini
@ 2017-04-21 19:33     ` David Matlack
  0 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21 19:33 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm list, Peter Feiner

On Fri, Apr 21, 2017 at 1:54 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 21/04/2017 02:49, David Matlack wrote:
>> From: Peter Feiner <pfeiner@google.com>
>>
>> Upcoming test allocates a 1GiB huge page. The VM needs more memory and
>> the default container limit of 2 GiB isn't enough.
>
> Google-y commit message? :)

Whoops, yes it is. Feel free to drop that sentence when applying the patch.

>
> Paolo
>
>> Signed-off-by: Peter Feiner <pfeiner@google.com>
>> Signed-off-by: David Matlack <dmatlack@google.com>
>> ---
>>  x86/unittests.cfg | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
>> index f79688af93f8..587bf40e8bc9 100644
>> --- a/x86/unittests.cfg
>> +++ b/x86/unittests.cfg
>> @@ -196,7 +196,7 @@ arch = x86_64
>>
>>  [vmx]
>>  file = vmx.flat
>> -extra_params = -cpu host,+vmx -append -exit_monitor_from_l2_test
>> +extra_params = -cpu host,+vmx -m 2048 -append -exit_monitor_from_l2_test

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

* Re: [kvm-unit-tests PATCH 18/32] lib: provide stdio.h
  2017-04-21  7:16   ` Andrew Jones
  2017-04-21  8:24     ` Paolo Bonzini
@ 2017-04-21 19:40     ` David Matlack
  1 sibling, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-04-21 19:40 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm list, Peter Feiner

On Fri, Apr 21, 2017 at 12:16 AM, Andrew Jones <drjones@redhat.com> wrote:
> On Thu, Apr 20, 2017 at 05:49:50PM -0700, David Matlack wrote:
>> From: Peter Feiner <pfeiner@google.com>
>>
>> kvm-unit-tests builds with the odd combination of system include files
>> but not system libraries. That is, -nostdlib is specified but
>> -nostdinc isn't! Problems arise when the include files don't match the
>> libraries, e.g., printf() in some versions of glibc linking in a
>> __printf_chk symbol that libcflat doesn't have. This patch fixes the
>> problem with printf by pointing '#include "stdio.h"' to
>> lib/libcflat.h, where a printf declaration that libcflat actually
>> implements exists, rather than letting /usr/include/stdio.h get
>> included. This patch does not solve the general problem of potential
>> mismatches between libcflat and the compiler's notion standard include
>> files; doing so would require rewriting stdarg.h, stdint.h, and
>> several others in libcflat.
>
> Yeah, I'm not sure rewriting those would be worth being able to add
> -nostdinc. When we cross compile ARM and POWER those few headers are
> some of the ones provided by the cross compiler.
>
>>
>> Signed-off-by: Peter Feiner <pfeiner@google.com>
>> Signed-off-by: David Matlack <dmatlack@google.com>
>> ---
>>  lib/stdio.h | 4 ++++
>>  1 file changed, 4 insertions(+)
>>  create mode 100644 lib/stdio.h
>>
>> diff --git a/lib/stdio.h b/lib/stdio.h
>> new file mode 100644
>> index 000000000000..7b30db93f7f1
>> --- /dev/null
>> +++ b/lib/stdio.h
>> @@ -0,0 +1,4 @@
>> +#ifndef LIBCFLAT_STDIO_H
>> +#define LIBCFLAT_STDIO_H
>> +#include "libcflat.h"
>> +#endif
>> --
>> 2.12.2.816.g2cccc81164-goog
>>
>
> I'd rather we move the stdio functions (just printf?) into stdio.h rather
> than redirect to libcflat. We can include stdio.h from libcflat.h to
> avoid updating all the unit tests for now though.

That sounds reasonable to me. Feel free to leave this patch out when
applying if you'd like. None of the other patches in this series
actually depend on it.

>
> Thanks,
> drew

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

* Re: [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests
  2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
                   ` (32 preceding siblings ...)
  2017-04-21  1:01 ` [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests Wanpeng Li
@ 2017-05-02 14:00 ` Paolo Bonzini
  2017-05-05 19:35   ` David Matlack
  33 siblings, 1 reply; 52+ messages in thread
From: Paolo Bonzini @ 2017-05-02 14:00 UTC (permalink / raw)
  To: David Matlack, kvm



On 21/04/2017 02:49, David Matlack wrote:
> 
> 4 of the EPT access tests reveal KVM bugs (4.11-rc6, on a Haswell CPU):
> 	vmx_ept_access_test_execute_only
> 	vmx_ept_access_test_paddr_not_present_ad_disabled
> 	vmx_ept_access_test_paddr_read_only_ad_disabled
> 	vmx_ept_access_test_paddr_read_execute_ad_disabled

These are the failure I get:

FAIL vmx_ept_access_test_execute_only (164 tests, 6 unexpected failures)
FAIL vmx_ept_access_test_paddr_not_present_ad_enabled (65 tests, 3 unexpected failures)
FAIL vmx_ept_access_test_paddr_read_only_ad_disabled (timeout; duration=90s)
FAIL vmx_ept_access_test_paddr_read_only_ad_enabled (175 tests, 9 unexpected failures)
FAIL vmx_ept_access_test_paddr_read_execute_ad_disabled (timeout; duration=90s)
FAIL vmx_ept_access_test_paddr_read_execute_ad_enabled (175 tests, 9 unexpected failures)

The problem is that there is the tests have no comment explaining what is being tested.
For example in ept_access_test_paddr_read_only_ad_enabled they are all like this:

EPT_VLT_RD unexpected
FAIL: x86/vmx_tests.c:2124: Expectation failed: (expected_qual) == (qual)
	LHS: 0x000000000000008a - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'1000'1010 - 138
	RHS: 0x000000000000008b - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'1000'1011 - 139
	STACK: 4072ad 407a00 407acc 407be6 401a9c 403653 4002d2

and it's not obvious what is the difference between them without looking at
the source.  The problem is that, when only one of them is broken, it's not
clear which one it is.  Please make sure that prefix push/pop is used
consistently around each subtest; it's okay to do it on top of these patches,
so I've placed them in the next branch of kvm-unit-tests.git.

In this case, in addition, the exit qualification comes straight from the
processor, so I think the failures are a testcase bug: "If such an access
causes an EPT violation, the processor sets both bit 0 and bit 1 of the
exit qualification", footnote 1 of table 27-7 "Exit Qualification for EPT
Violations".

Paolo

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

* Re: [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests
  2017-05-02 14:00 ` Paolo Bonzini
@ 2017-05-05 19:35   ` David Matlack
  2017-05-06  8:05     ` Paolo Bonzini
  0 siblings, 1 reply; 52+ messages in thread
From: David Matlack @ 2017-05-05 19:35 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm list

On Tue, May 2, 2017 at 7:00 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 21/04/2017 02:49, David Matlack wrote:
>>
>> 4 of the EPT access tests reveal KVM bugs (4.11-rc6, on a Haswell CPU):
>>       vmx_ept_access_test_execute_only
>>       vmx_ept_access_test_paddr_not_present_ad_disabled
>>       vmx_ept_access_test_paddr_read_only_ad_disabled
>>       vmx_ept_access_test_paddr_read_execute_ad_disabled
>
> These are the failure I get:
>
> FAIL vmx_ept_access_test_execute_only (164 tests, 6 unexpected failures)
> FAIL vmx_ept_access_test_paddr_not_present_ad_enabled (65 tests, 3 unexpected failures)
> FAIL vmx_ept_access_test_paddr_read_only_ad_disabled (timeout; duration=90s)
> FAIL vmx_ept_access_test_paddr_read_only_ad_enabled (175 tests, 9 unexpected failures)
> FAIL vmx_ept_access_test_paddr_read_execute_ad_disabled (timeout; duration=90s)
> FAIL vmx_ept_access_test_paddr_read_execute_ad_enabled (175 tests, 9 unexpected failures)

I get the same set of failures when testing a kernel sync'd to
kvm/next (at commit bd17117bb2af).

>
> The problem is that there is the tests have no comment explaining what is being tested.
> For example in ept_access_test_paddr_read_only_ad_enabled they are all like this:
>
> EPT_VLT_RD unexpected
> FAIL: x86/vmx_tests.c:2124: Expectation failed: (expected_qual) == (qual)
>         LHS: 0x000000000000008a - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'1000'1010 - 138
>         RHS: 0x000000000000008b - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'1000'1011 - 139
>         STACK: 4072ad 407a00 407acc 407be6 401a9c 403653 4002d2
>
> and it's not obvious what is the difference between them without looking at
> the source.  The problem is that, when only one of them is broken, it's not
> clear which one it is.  Please make sure that prefix push/pop is used
> consistently around each subtest; it's okay to do it on top of these patches,
> so I've placed them in the next branch of kvm-unit-tests.git.

The stack trace identifies which test case failed. For example, in
logs/vmx_ept_access_test_paddr_not_present_ad_enabled.log I see:

EPT_VLT_RD unexpected
FAIL: x86/vmx_tests.c:2124: Expectation failed: (expected_qual) == (qual)
        LHS: 0x0000000000000082 -
0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'1000'0010
- 130
        RHS: 0x0000000000000083 -
0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'1000'0011
- 131
        STACK: 4072ad 407a00 407acc 407d09 401a9c 403653 4002d2
0x00000000004072ac: do_ept_violation at x86/vmx_tests.c:2124 (discriminator 1)
                diagnose_ept_violation_qual(expected_qual, qual);
      >         TEST_EXPECT_EQ(expected_qual, qual);

0x00000000004079ff: ept_access_paddr at x86/vmx_tests.c:2252
                if (expect_violation)
      >                 do_ept_violation(/*leaf=*/true, op,
                                         expected_qual |
EPT_VLT_LADDR_VLD, gpa);
0x0000000000407acb: ept_access_violation_paddr at x86/vmx_tests.c:2278
        {
      >         ept_access_paddr(ept_access, pte_ad, op,
/*expect_violation=*/true,
                                 expected_qual);
0x0000000000407d08: ept_access_test_paddr_not_present_ad_enabled at
x86/vmx_tests.c:2690
                ept_access_violation_paddr(0, PT_AD_MASK, OP_WRITE, EPT_VLT_WR);
      >         ept_access_violation_paddr(0, PT_AD_MASK, OP_EXEC, EPT_VLT_WR);
        }
0x0000000000401a9b: test_run at x86/vmx.c:1675
                if (test->v2)
      >                 test->v2();
                else
0x0000000000403652: main at x86/vmx.c:1857
                                continue;
      >                 if (test_run(&vmx_tests[i]))
                                goto exit;
0x00000000004002d1: start64 at x86/cstart64.S:210
                lea __environ(%rip), %rdx
      >         call main
                mov %eax, %edi

Does this work for you?

>
> In this case, in addition, the exit qualification comes straight from the
> processor, so I think the failures are a testcase bug: "If such an access
> causes an EPT violation, the processor sets both bit 0 and bit 1 of the
> exit qualification", footnote 1 of table 27-7 "Exit Qualification for EPT
> Violations".

Agreed, good catch. I will send a patch to fix the tests.

>
> Paolo

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

* Re: [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests
  2017-05-05 19:35   ` David Matlack
@ 2017-05-06  8:05     ` Paolo Bonzini
  0 siblings, 0 replies; 52+ messages in thread
From: Paolo Bonzini @ 2017-05-06  8:05 UTC (permalink / raw)
  To: David Matlack, KVM list



On 05/05/2017 21:35, David Matlack wrote:
>> The problem is that there is the tests have no comment explaining what is being tested.
>> For example in ept_access_test_paddr_read_only_ad_enabled they are all like this:
>>
>> EPT_VLT_RD unexpected
>> FAIL: x86/vmx_tests.c:2124: Expectation failed: (expected_qual) == (qual)
>>         LHS: 0x000000000000008a - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'1000'1010 - 138
>>         RHS: 0x000000000000008b - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'1000'1011 - 139
>>         STACK: 4072ad 407a00 407acc 407be6 401a9c 403653 4002d2
>>
>> and it's not obvious what is the difference between them without looking at
>> the source.  The problem is that, when only one of them is broken, it's not
>> clear which one it is.  Please make sure that prefix push/pop is used
>> consistently around each subtest; it's okay to do it on top of these patches,
>> so I've placed them in the next branch of kvm-unit-tests.git.
> 
> The stack trace identifies which test case failed. For example, in
> logs/vmx_ept_access_test_paddr_not_present_ad_enabled.log I see:
> 
> EPT_VLT_RD unexpected
> FAIL: x86/vmx_tests.c:2124: Expectation failed: (expected_qual) == (qual)
>         LHS: 0x0000000000000082 -
> 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'1000'0010
> - 130
>         RHS: 0x0000000000000083 -
> 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'1000'0011
> - 131
>         STACK: 4072ad 407a00 407acc 407d09 401a9c 403653 4002d2
> 0x00000000004072ac: do_ept_violation at x86/vmx_tests.c:2124 (discriminator 1)
>                 diagnose_ept_violation_qual(expected_qual, qual);
>       >         TEST_EXPECT_EQ(expected_qual, qual);
> 
> 0x00000000004079ff: ept_access_paddr at x86/vmx_tests.c:2252
>                 if (expect_violation)
>       >                 do_ept_violation(/*leaf=*/true, op,
>                                          expected_qual |
> EPT_VLT_LADDR_VLD, gpa);
> 0x0000000000407acb: ept_access_violation_paddr at x86/vmx_tests.c:2278
>         {
>       >         ept_access_paddr(ept_access, pte_ad, op,
> /*expect_violation=*/true,
>                                  expected_qual);
> 0x0000000000407d08: ept_access_test_paddr_not_present_ad_enabled at
> x86/vmx_tests.c:2690
>                 ept_access_violation_paddr(0, PT_AD_MASK, OP_WRITE, EPT_VLT_WR);
>       >         ept_access_violation_paddr(0, PT_AD_MASK, OP_EXEC, EPT_VLT_WR);
>         }
> 0x0000000000401a9b: test_run at x86/vmx.c:1675
>                 if (test->v2)
>       >                 test->v2();
>                 else
> 0x0000000000403652: main at x86/vmx.c:1857
>                                 continue;
>       >                 if (test_run(&vmx_tests[i]))
>                                 goto exit;
> 0x00000000004002d1: start64 at x86/cstart64.S:210
>                 lea __environ(%rip), %rdx
>       >         call main
>                 mov %eax, %edi
> 
> Does this work for you?

Certainly much better. :)

Paolo

>> In this case, in addition, the exit qualification comes straight from the
>> processor, so I think the failures are a testcase bug: "If such an access
>> causes an EPT violation, the processor sets both bit 0 and bit 1 of the
>> exit qualification", footnote 1 of table 27-7 "Exit Qualification for EPT
>> Violations".
> 
> Agreed, good catch. I will send a patch to fix the tests.
> 
>>
>> Paolo
> 

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

* Re: [kvm-unit-tests PATCH 21/32] lib: printf-style report prefixes
  2017-04-21  0:49 ` [kvm-unit-tests PATCH 21/32] lib: printf-style report prefixes David Matlack
  2017-04-21  7:42   ` Andrew Jones
@ 2017-05-12 10:51   ` Thomas Huth
  2017-05-12 11:22     ` David Hildenbrand
  1 sibling, 1 reply; 52+ messages in thread
From: Thomas Huth @ 2017-05-12 10:51 UTC (permalink / raw)
  To: David Matlack, kvm, Peter Feiner
  Cc: Paolo Bonzini, Radim Krčmář, Laurent Vivier

On 21.04.2017 02:49, David Matlack wrote:
> From: Peter Feiner <pfeiner@google.com>
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>  lib/libcflat.h |  1 +
>  lib/report.c   | 38 +++++++++++++++++++++++++++++++++-----
>  2 files changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 5d356df75f1f..05c18543dd72 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -96,6 +96,7 @@ extern int vsnprintf(char *buf, int size, const char *fmt, va_list va)
>  extern int vprintf(const char *fmt, va_list va)
>  					__attribute__((format(printf, 1, 0)));
>  
> +void report_prefix_pushf(const char *prefix_fmt, ...);
>  extern void report_prefix_push(const char *prefix);
>  extern void report_prefix_pop(void);
>  extern void report(const char *msg_fmt, bool pass, ...);
> diff --git a/lib/report.c b/lib/report.c
> index e24e81382f9e..1033f1e44e99 100644
> --- a/lib/report.c
> +++ b/lib/report.c
> @@ -17,14 +17,42 @@ static unsigned int tests, failures, xfailures, skipped;
>  static char prefixes[256];
>  static struct spinlock lock;
>  
> -void report_prefix_push(const char *prefix)
> +#define PREFIX_DELIMITER ": "
> +
> +void report_prefix_pushf(const char *prefix_fmt, ...)
>  {
> +	va_list va;
> +	int len;
> +	int start;
> +
>  	spin_lock(&lock);
> -	strcat(prefixes, prefix);
> -	strcat(prefixes, ": ");
> +
> +	len = strlen(prefixes);
> +	assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
> +	start = len;
> +
> +	va_start(va, prefix_fmt);
> +	len += vsnprintf(&prefixes[len], sizeof(prefixes) - len, prefix_fmt,
> +			 va);
> +	va_end(va);
> +	assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
> +
> +	assert_msg(!strstr(&prefixes[start], PREFIX_DELIMITER),
> +		   "Prefix \"%s\" contains delimiter \"" PREFIX_DELIMITER "\"",
> +		   &prefixes[start]);
> +
> +	len += snprintf(&prefixes[len], sizeof(prefixes) - len,
> +			PREFIX_DELIMITER);
> +	assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
> +
>  	spin_unlock(&lock);
>  }

I can not compile the current master of kvm-unit-tests anymore... I
think it is due to the above patch. I get now these error messages:

powerpc64-linux-gnu-gcc  -std=gnu99 -ffreestanding -Wextra -O2 -I lib -I lib/libfdt -Wa,-mregnames -g -MMD -MF lib/.report.d -Wall -Werror  -fomit-frame-pointer    -Wno-frame-address   -fno-pic   -mbig-endian   -c -o lib/report.o lib/report.c
In file included from lib/report.c:13:0:
lib/report.c: In function ‘report_prefix_pushf’:
lib/report.c:38:17: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
  assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
                 ^
lib/libcflat.h:133:8: note: in definition of macro ‘assert_msg’
  if (!(cond)) {       \
        ^
lib/report.c:45:17: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
  assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
                 ^
lib/libcflat.h:133:8: note: in definition of macro ‘assert_msg’
  if (!(cond)) {       \
        ^
lib/report.c:53:17: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
  assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
                 ^
lib/libcflat.h:133:8: note: in definition of macro ‘assert_msg’
  if (!(cond)) {       \
        ^
 Thomas

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

* Re: [kvm-unit-tests PATCH 21/32] lib: printf-style report prefixes
  2017-05-12 10:51   ` Thomas Huth
@ 2017-05-12 11:22     ` David Hildenbrand
  2017-05-12 16:53       ` David Matlack
  0 siblings, 1 reply; 52+ messages in thread
From: David Hildenbrand @ 2017-05-12 11:22 UTC (permalink / raw)
  To: Thomas Huth, David Matlack, kvm, Peter Feiner
  Cc: Paolo Bonzini, Radim Krčmář, Laurent Vivier

On 12.05.2017 12:51, Thomas Huth wrote:
> On 21.04.2017 02:49, David Matlack wrote:
>> From: Peter Feiner <pfeiner@google.com>
>>
>> Signed-off-by: Peter Feiner <pfeiner@google.com>
>> Signed-off-by: David Matlack <dmatlack@google.com>
>> ---
>>  lib/libcflat.h |  1 +
>>  lib/report.c   | 38 +++++++++++++++++++++++++++++++++-----
>>  2 files changed, 34 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/libcflat.h b/lib/libcflat.h
>> index 5d356df75f1f..05c18543dd72 100644
>> --- a/lib/libcflat.h
>> +++ b/lib/libcflat.h
>> @@ -96,6 +96,7 @@ extern int vsnprintf(char *buf, int size, const char *fmt, va_list va)
>>  extern int vprintf(const char *fmt, va_list va)
>>  					__attribute__((format(printf, 1, 0)));
>>  
>> +void report_prefix_pushf(const char *prefix_fmt, ...);
>>  extern void report_prefix_push(const char *prefix);
>>  extern void report_prefix_pop(void);
>>  extern void report(const char *msg_fmt, bool pass, ...);
>> diff --git a/lib/report.c b/lib/report.c
>> index e24e81382f9e..1033f1e44e99 100644
>> --- a/lib/report.c
>> +++ b/lib/report.c
>> @@ -17,14 +17,42 @@ static unsigned int tests, failures, xfailures, skipped;
>>  static char prefixes[256];
>>  static struct spinlock lock;
>>  
>> -void report_prefix_push(const char *prefix)
>> +#define PREFIX_DELIMITER ": "
>> +
>> +void report_prefix_pushf(const char *prefix_fmt, ...)
>>  {
>> +	va_list va;
>> +	int len;
>> +	int start;
>> +
>>  	spin_lock(&lock);
>> -	strcat(prefixes, prefix);
>> -	strcat(prefixes, ": ");
>> +
>> +	len = strlen(prefixes);
>> +	assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
>> +	start = len;
>> +
>> +	va_start(va, prefix_fmt);
>> +	len += vsnprintf(&prefixes[len], sizeof(prefixes) - len, prefix_fmt,
>> +			 va);
>> +	va_end(va);
>> +	assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
>> +
>> +	assert_msg(!strstr(&prefixes[start], PREFIX_DELIMITER),
>> +		   "Prefix \"%s\" contains delimiter \"" PREFIX_DELIMITER "\"",
>> +		   &prefixes[start]);
>> +
>> +	len += snprintf(&prefixes[len], sizeof(prefixes) - len,
>> +			PREFIX_DELIMITER);
>> +	assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
>> +
>>  	spin_unlock(&lock);
>>  }
> 
> I can not compile the current master of kvm-unit-tests anymore... I
> think it is due to the above patch. I get now these error messages:
> 
> powerpc64-linux-gnu-gcc  -std=gnu99 -ffreestanding -Wextra -O2 -I lib -I lib/libfdt -Wa,-mregnames -g -MMD -MF lib/.report.d -Wall -Werror  -fomit-frame-pointer    -Wno-frame-address   -fno-pic   -mbig-endian   -c -o lib/report.o lib/report.c
> In file included from lib/report.c:13:0:
> lib/report.c: In function ‘report_prefix_pushf’:
> lib/report.c:38:17: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
>   assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
>                  ^
> lib/libcflat.h:133:8: note: in definition of macro ‘assert_msg’
>   if (!(cond)) {       \
>         ^
> lib/report.c:45:17: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
>   assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
>                  ^
> lib/libcflat.h:133:8: note: in definition of macro ‘assert_msg’
>   if (!(cond)) {       \
>         ^
> lib/report.c:53:17: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
>   assert_msg(len < sizeof(prefixes), "%d >= %lu", len, sizeof(prefixes));
>                  ^
> lib/libcflat.h:133:8: note: in definition of macro ‘assert_msg’
>   if (!(cond)) {       \
>         ^
>  Thomas
> 

Also found that while preparing the s390x patch series. Already sent

[kvm-unit-tests PATCH v1] lib: fix compilation warning

-- 

Thanks,

David

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

* Re: [kvm-unit-tests PATCH 21/32] lib: printf-style report prefixes
  2017-05-12 11:22     ` David Hildenbrand
@ 2017-05-12 16:53       ` David Matlack
  0 siblings, 0 replies; 52+ messages in thread
From: David Matlack @ 2017-05-12 16:53 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Thomas Huth, kvm list, Peter Feiner, Paolo Bonzini,
	Radim Krčmář,
	Laurent Vivier

On Fri, May 12, 2017 at 4:22 AM, David Hildenbrand <david@redhat.com> wrote:
>
> Also found that while preparing the s390x patch series. Already sent
>
> [kvm-unit-tests PATCH v1] lib: fix compilation warning

Sorry about that. Thanks for the fix!

>
> --
>
> Thanks,
>
> David

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

end of thread, other threads:[~2017-05-12 16:54 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-21  0:49 [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 01/32] x86: add test filter to vmx.flat David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 02/32] x86: add config for each vmx unit test case David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 03/32] x86: move basic vmx tests into separate test cases David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 04/32] x86: add missing vmx test cases to unittests.cfg David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 05/32] lib: better test name filtering David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 06/32] x86: vmx: filter exit_monitor_from_l2_test from full vmx test David Matlack
2017-04-21  8:55   ` Paolo Bonzini
2017-04-21  0:49 ` [kvm-unit-tests PATCH 07/32] x86: test exit while vmcs02 is loaded David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 08/32] x86: Adding gtest to check correct instruction error is returned David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 09/32] x86: basic vmwrite/vmread test David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 10/32] x86: vmcs lifecycle test David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 11/32] x86: test VMCS in memory after VMCLEAR David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 12/32] x86: test VMPTRLD does not drop VMWRITEs David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 13/32] x86: don't special case vmx null test David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 14/32] x86: factor out vmx_enter_guest David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 15/32] x86: binstr utility function David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 16/32] x86: vmx exit reason descriptions David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 17/32] x86: v2 vmx test framework David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 18/32] lib: provide stdio.h David Matlack
2017-04-21  7:16   ` Andrew Jones
2017-04-21  8:24     ` Paolo Bonzini
2017-04-21 19:40     ` David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 19/32] lib: added assert_msg macro David Matlack
2017-04-21  7:18   ` Andrew Jones
2017-04-21  0:49 ` [kvm-unit-tests PATCH 20/32] lib: fix *printf return value David Matlack
2017-04-21  7:21   ` Andrew Jones
2017-04-21  0:49 ` [kvm-unit-tests PATCH 21/32] lib: printf-style report prefixes David Matlack
2017-04-21  7:42   ` Andrew Jones
2017-05-12 10:51   ` Thomas Huth
2017-05-12 11:22     ` David Hildenbrand
2017-05-12 16:53       ` David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 22/32] lib: add newline to assert_msg David Matlack
2017-04-21  7:44   ` Andrew Jones
2017-04-21  8:25     ` Paolo Bonzini
2017-04-21  0:49 ` [kvm-unit-tests PATCH 23/32] lib: x86: store free pages in ascending order David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 24/32] lib: x86: multi-page allocator David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 25/32] x86: ept assertions David Matlack
2017-04-21  9:57   ` Paolo Bonzini
2017-04-21  0:49 ` [kvm-unit-tests PATCH 26/32] x86: setup_ept code motion David Matlack
2017-04-21  0:49 ` [kvm-unit-tests PATCH 27/32] x86: 2GiB RAM for vmx tests David Matlack
2017-04-21  8:54   ` Paolo Bonzini
2017-04-21 19:33     ` David Matlack
2017-04-21  0:50 ` [kvm-unit-tests PATCH 28/32] x86: ept capability utilities David Matlack
2017-04-21  0:50 ` [kvm-unit-tests PATCH 29/32] lib: x86: page table utilities David Matlack
2017-04-21  0:50 ` [kvm-unit-tests PATCH 30/32] x86: ept access tests David Matlack
2017-04-21  0:50 ` [kvm-unit-tests PATCH 31/32] x86: force ept 2m test David Matlack
2017-04-21  0:50 ` [kvm-unit-tests PATCH 32/32] lib: add report_pass David Matlack
2017-04-21  1:01 ` [kvm-unit-tests PATCH 00/32] VMX framework enhancements and new tests Wanpeng Li
2017-05-02 14:00 ` Paolo Bonzini
2017-05-05 19:35   ` David Matlack
2017-05-06  8:05     ` Paolo Bonzini

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