All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] kvm-unit-tests: more instr. interceptions, debug control migration
@ 2014-06-15 14:24 Jan Kiszka
  2014-06-15 14:24 ` [PATCH 1/5] VMX: Add tests for CR3 and CR8 interception Jan Kiszka
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Jan Kiszka @ 2014-06-15 14:24 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

The tests corresponding to (and going beyond) the issues fixed in
http://thread.gmane.org/gmane.comp.emulators.kvm.devel/123282

Jan Kiszka (5):
  VMX: Add tests for CR3 and CR8 interception
  VMX: Only use get_stage accessor
  VMX: Test both interception and execution of instructions
  VMX: Validate capability MSRs
  VMX: Test behavior on set and cleared save/load debug controls

 x86/vmx.c       |  73 ++++++++++++-
 x86/vmx.h       |   9 +-
 x86/vmx_tests.c | 327 +++++++++++++++++++++++++++++++++++++++++---------------
 3 files changed, 322 insertions(+), 87 deletions(-)

-- 
1.8.1.1.298.ge7eed54


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

* [PATCH 1/5] VMX: Add tests for CR3 and CR8 interception
  2014-06-15 14:24 [PATCH 0/5] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
@ 2014-06-15 14:24 ` Jan Kiszka
  2014-06-16 10:53   ` Paolo Bonzini
  2014-06-15 14:24 ` [PATCH 2/5] VMX: Only use get_stage accessor Jan Kiszka
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2014-06-15 14:24 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

From: Jan Kiszka <jan.kiszka@siemens.com>

Need to fix FIELD_* constants for this to make the exit qualification
check work.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 x86/vmx.h       |  2 ++
 x86/vmx_tests.c | 32 +++++++++++++++++++++++++++++---
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/x86/vmx.h b/x86/vmx.h
index 26dd161..69a5385 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -357,6 +357,8 @@ enum Ctrl0 {
 	CPU_RDTSC		= 1ul << 12,
 	CPU_CR3_LOAD		= 1ul << 15,
 	CPU_CR3_STORE		= 1ul << 16,
+	CPU_CR8_LOAD		= 1ul << 19,
+	CPU_CR8_STORE		= 1ul << 20,
 	CPU_TPR_SHADOW		= 1ul << 21,
 	CPU_NMI_WINDOW		= 1ul << 22,
 	CPU_IO			= 1ul << 24,
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index a40cb18..d0ce365 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -820,8 +820,8 @@ static int iobmp_exit_handler()
 #define INSN_ALWAYS_TRAP	2
 #define INSN_NEVER_TRAP		3
 
-#define FIELD_EXIT_QUAL		0
-#define FIELD_INSN_INFO		1
+#define FIELD_EXIT_QUAL		0x1
+#define FIELD_INSN_INFO		0x2
 
 asm(
 	"insn_hlt: hlt;ret\n\t"
@@ -829,6 +829,12 @@ asm(
 	"insn_mwait: mwait;ret\n\t"
 	"insn_rdpmc: rdpmc;ret\n\t"
 	"insn_rdtsc: rdtsc;ret\n\t"
+	"insn_cr3_load: mov %rax,%cr3;ret\n\t"
+	"insn_cr3_store: mov %cr3,%rax;ret\n\t"
+#ifdef __x86_64__
+	"insn_cr8_load: mov %rax,%cr8;ret\n\t"
+	"insn_cr8_store: mov %cr8,%rax;ret\n\t"
+#endif
 	"insn_monitor: monitor;ret\n\t"
 	"insn_pause: pause;ret\n\t"
 	"insn_wbinvd: wbinvd;ret\n\t"
@@ -840,6 +846,12 @@ extern void insn_invlpg();
 extern void insn_mwait();
 extern void insn_rdpmc();
 extern void insn_rdtsc();
+extern void insn_cr3_load();
+extern void insn_cr3_store();
+#ifdef __x86_64__
+extern void insn_cr8_load();
+extern void insn_cr8_store();
+#endif
 extern void insn_monitor();
 extern void insn_pause();
 extern void insn_wbinvd();
@@ -856,7 +868,7 @@ struct insn_table {
 	u32 reason;
 	ulong exit_qual;
 	u32 insn_info;
-	// Use FIELD_EXIT_QUAL and FIELD_INSN_INFO to efines
+	// Use FIELD_EXIT_QUAL and FIELD_INSN_INFO to define
 	// which field need to be tested, reason is always tested
 	u32 test_field;
 };
@@ -877,6 +889,16 @@ static struct insn_table insn_table[] = {
 	{"MWAIT", CPU_MWAIT, insn_mwait, INSN_CPU0, 36, 0, 0, 0},
 	{"RDPMC", CPU_RDPMC, insn_rdpmc, INSN_CPU0, 15, 0, 0, 0},
 	{"RDTSC", CPU_RDTSC, insn_rdtsc, INSN_CPU0, 16, 0, 0, 0},
+	{"CR3 load", CPU_CR3_LOAD, insn_cr3_load, INSN_CPU0, 28, 0x3, 0,
+		FIELD_EXIT_QUAL},
+	{"CR3 store", CPU_CR3_STORE, insn_cr3_store, INSN_CPU0, 28, 0x13, 0,
+		FIELD_EXIT_QUAL},
+#ifdef __x86_64__
+	{"CR8 load", CPU_CR8_LOAD, insn_cr8_load, INSN_CPU0, 28, 0x8, 0,
+		FIELD_EXIT_QUAL},
+	{"CR8 store", CPU_CR8_STORE, insn_cr8_store, INSN_CPU0, 28, 0x18, 0,
+		FIELD_EXIT_QUAL},
+#endif
 	{"MONITOR", CPU_MONITOR, insn_monitor, INSN_CPU0, 39, 0, 0, 0},
 	{"PAUSE", CPU_PAUSE, insn_pause, INSN_CPU0, 40, 0, 0, 0},
 	// Flags for Secondary Processor-Based VM-Execution Controls
@@ -894,6 +916,10 @@ static int insn_intercept_init()
 
 	ctrl_cpu[0] = vmcs_read(CPU_EXEC_CTRL0);
 	ctrl_cpu[0] |= CPU_HLT | CPU_INVLPG | CPU_MWAIT | CPU_RDPMC | CPU_RDTSC |
+		CPU_CR3_LOAD | CPU_CR3_STORE |
+#ifdef __x86_64__
+		CPU_CR8_LOAD | CPU_CR8_STORE |
+#endif
 		CPU_MONITOR | CPU_PAUSE | CPU_SECONDARY;
 	ctrl_cpu[0] &= ctrl_cpu_rev[0].clr;
 	vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu[0]);
-- 
1.8.1.1.298.ge7eed54


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

* [PATCH 2/5] VMX: Only use get_stage accessor
  2014-06-15 14:24 [PATCH 0/5] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
  2014-06-15 14:24 ` [PATCH 1/5] VMX: Add tests for CR3 and CR8 interception Jan Kiszka
@ 2014-06-15 14:24 ` Jan Kiszka
  2014-06-16 17:19   ` Bandan Das
  2014-06-15 14:24 ` [PATCH 3/5] VMX: Test both interception and execution of instructions Jan Kiszka
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2014-06-15 14:24 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

From: Jan Kiszka <jan.kiszka@siemens.com>

Consistently make sure we are not affected by any compiler reordering
when evaluating the current stage.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 x86/vmx_tests.c | 80 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index d0ce365..bf7aa2c 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -415,13 +415,13 @@ static void cr_shadowing_main()
 	// Test read through
 	set_stage(0);
 	guest_cr0 = read_cr0();
-	if (stage == 1)
+	if (get_stage() == 1)
 		report("Read through CR0", 0);
 	else
 		vmcall();
 	set_stage(1);
 	guest_cr4 = read_cr4();
-	if (stage == 2)
+	if (get_stage() == 2)
 		report("Read through CR4", 0);
 	else
 		vmcall();
@@ -430,13 +430,13 @@ static void cr_shadowing_main()
 	guest_cr4 = guest_cr4 ^ (X86_CR4_TSD | X86_CR4_DE);
 	set_stage(2);
 	write_cr0(guest_cr0);
-	if (stage == 3)
+	if (get_stage() == 3)
 		report("Write throuth CR0", 0);
 	else
 		vmcall();
 	set_stage(3);
 	write_cr4(guest_cr4);
-	if (stage == 4)
+	if (get_stage() == 4)
 		report("Write through CR4", 0);
 	else
 		vmcall();
@@ -444,7 +444,7 @@ static void cr_shadowing_main()
 	set_stage(4);
 	vmcall();
 	cr0 = read_cr0();
-	if (stage != 5) {
+	if (get_stage() != 5) {
 		if (cr0 == guest_cr0)
 			report("Read shadowing CR0", 1);
 		else
@@ -452,7 +452,7 @@ static void cr_shadowing_main()
 	}
 	set_stage(5);
 	cr4 = read_cr4();
-	if (stage != 6) {
+	if (get_stage() != 6) {
 		if (cr4 == guest_cr4)
 			report("Read shadowing CR4", 1);
 		else
@@ -461,13 +461,13 @@ static void cr_shadowing_main()
 	// Test write shadow (same value with shadow)
 	set_stage(6);
 	write_cr0(guest_cr0);
-	if (stage == 7)
+	if (get_stage() == 7)
 		report("Write shadowing CR0 (same value with shadow)", 0);
 	else
 		vmcall();
 	set_stage(7);
 	write_cr4(guest_cr4);
-	if (stage == 8)
+	if (get_stage() == 8)
 		report("Write shadowing CR4 (same value with shadow)", 0);
 	else
 		vmcall();
@@ -478,7 +478,7 @@ static void cr_shadowing_main()
 		"mov %%rsi, %%cr0\n\t"
 		::"m"(tmp)
 		:"rsi", "memory", "cc");
-	if (stage != 9)
+	if (get_stage() != 9)
 		report("Write shadowing different X86_CR0_TS", 0);
 	else
 		report("Write shadowing different X86_CR0_TS", 1);
@@ -488,7 +488,7 @@ static void cr_shadowing_main()
 		"mov %%rsi, %%cr0\n\t"
 		::"m"(tmp)
 		:"rsi", "memory", "cc");
-	if (stage != 10)
+	if (get_stage() != 10)
 		report("Write shadowing different X86_CR0_MP", 0);
 	else
 		report("Write shadowing different X86_CR0_MP", 1);
@@ -498,7 +498,7 @@ static void cr_shadowing_main()
 		"mov %%rsi, %%cr4\n\t"
 		::"m"(tmp)
 		:"rsi", "memory", "cc");
-	if (stage != 11)
+	if (get_stage() != 11)
 		report("Write shadowing different X86_CR4_TSD", 0);
 	else
 		report("Write shadowing different X86_CR4_TSD", 1);
@@ -508,7 +508,7 @@ static void cr_shadowing_main()
 		"mov %%rsi, %%cr4\n\t"
 		::"m"(tmp)
 		:"rsi", "memory", "cc");
-	if (stage != 12)
+	if (get_stage() != 12)
 		report("Write shadowing different X86_CR4_DE", 0);
 	else
 		report("Write shadowing different X86_CR4_DE", 1);
@@ -584,31 +584,31 @@ static int cr_shadowing_exit_handler()
 		switch (get_stage()) {
 		case 4:
 			report("Read shadowing CR0", 0);
-			set_stage(stage + 1);
+			set_stage(get_stage() + 1);
 			break;
 		case 5:
 			report("Read shadowing CR4", 0);
-			set_stage(stage + 1);
+			set_stage(get_stage() + 1);
 			break;
 		case 6:
 			report("Write shadowing CR0 (same value)", 0);
-			set_stage(stage + 1);
+			set_stage(get_stage() + 1);
 			break;
 		case 7:
 			report("Write shadowing CR4 (same value)", 0);
-			set_stage(stage + 1);
+			set_stage(get_stage() + 1);
 			break;
 		case 8:
 		case 9:
 			// 0x600 encodes "mov %esi, %cr0"
 			if (exit_qual == 0x600)
-				set_stage(stage + 1);
+				set_stage(get_stage() + 1);
 			break;
 		case 10:
 		case 11:
 			// 0x604 encodes "mov %esi, %cr4"
 			if (exit_qual == 0x604)
-				set_stage(stage + 1);
+				set_stage(get_stage() + 1);
 			break;
 		default:
 			// Should not reach here
@@ -648,7 +648,7 @@ static void iobmp_main()
 	set_stage(0);
 	inb(0x5000);
 	outb(0x0, 0x5000);
-	if (stage != 0)
+	if (get_stage() != 0)
 		report("I/O bitmap - I/O pass", 0);
 	else
 		report("I/O bitmap - I/O pass", 1);
@@ -656,39 +656,39 @@ static void iobmp_main()
 	((u8 *)io_bitmap_a)[0] = 0xFF;
 	set_stage(2);
 	inb(0x0);
-	if (stage != 3)
+	if (get_stage() != 3)
 		report("I/O bitmap - trap in", 0);
 	else
 		report("I/O bitmap - trap in", 1);
 	set_stage(3);
 	outw(0x0, 0x0);
-	if (stage != 4)
+	if (get_stage() != 4)
 		report("I/O bitmap - trap out", 0);
 	else
 		report("I/O bitmap - trap out", 1);
 	set_stage(4);
 	inl(0x0);
-	if (stage != 5)
+	if (get_stage() != 5)
 		report("I/O bitmap - I/O width, long", 0);
 	// test low/high IO port
 	set_stage(5);
 	((u8 *)io_bitmap_a)[0x5000 / 8] = (1 << (0x5000 % 8));
 	inb(0x5000);
-	if (stage == 6)
+	if (get_stage() == 6)
 		report("I/O bitmap - I/O port, low part", 1);
 	else
 		report("I/O bitmap - I/O port, low part", 0);
 	set_stage(6);
 	((u8 *)io_bitmap_b)[0x1000 / 8] = (1 << (0x1000 % 8));
 	inb(0x9000);
-	if (stage == 7)
+	if (get_stage() == 7)
 		report("I/O bitmap - I/O port, high part", 1);
 	else
 		report("I/O bitmap - I/O port, high part", 0);
 	// test partial pass
 	set_stage(7);
 	inl(0x4FFF);
-	if (stage == 8)
+	if (get_stage() == 8)
 		report("I/O bitmap - partial pass", 1);
 	else
 		report("I/O bitmap - partial pass", 0);
@@ -697,18 +697,18 @@ static void iobmp_main()
 	memset(io_bitmap_a, 0x0, PAGE_SIZE);
 	memset(io_bitmap_b, 0x0, PAGE_SIZE);
 	inl(0xFFFF);
-	if (stage == 9)
+	if (get_stage() == 9)
 		report("I/O bitmap - overrun", 1);
 	else
 		report("I/O bitmap - overrun", 0);
 	set_stage(9);
 	vmcall();
 	outb(0x0, 0x0);
-	report("I/O bitmap - ignore unconditional exiting", stage == 9);
+	report("I/O bitmap - ignore unconditional exiting", get_stage() == 9);
 	set_stage(10);
 	vmcall();
 	outb(0x0, 0x0);
-	report("I/O bitmap - unconditional exiting", stage == 11);
+	report("I/O bitmap - unconditional exiting", get_stage() == 11);
 }
 
 static int iobmp_exit_handler()
@@ -726,7 +726,7 @@ static int iobmp_exit_handler()
 		switch (get_stage()) {
 		case 0:
 		case 1:
-			set_stage(stage + 1);
+			set_stage(get_stage() + 1);
 			break;
 		case 2:
 			if ((exit_qual & VMX_IO_SIZE_MASK) != _VMX_IO_BYTE)
@@ -737,7 +737,7 @@ static int iobmp_exit_handler()
 				report("I/O bitmap - I/O direction, in", 0);
 			else
 				report("I/O bitmap - I/O direction, in", 1);
-			set_stage(stage + 1);
+			set_stage(get_stage() + 1);
 			break;
 		case 3:
 			if ((exit_qual & VMX_IO_SIZE_MASK) != _VMX_IO_WORD)
@@ -748,36 +748,36 @@ static int iobmp_exit_handler()
 				report("I/O bitmap - I/O direction, out", 1);
 			else
 				report("I/O bitmap - I/O direction, out", 0);
-			set_stage(stage + 1);
+			set_stage(get_stage() + 1);
 			break;
 		case 4:
 			if ((exit_qual & VMX_IO_SIZE_MASK) != _VMX_IO_LONG)
 				report("I/O bitmap - I/O width, long", 0);
 			else
 				report("I/O bitmap - I/O width, long", 1);
-			set_stage(stage + 1);
+			set_stage(get_stage() + 1);
 			break;
 		case 5:
 			if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x5000)
-				set_stage(stage + 1);
+				set_stage(get_stage() + 1);
 			break;
 		case 6:
 			if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x9000)
-				set_stage(stage + 1);
+				set_stage(get_stage() + 1);
 			break;
 		case 7:
 			if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x4FFF)
-				set_stage(stage + 1);
+				set_stage(get_stage() + 1);
 			break;
 		case 8:
 			if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0xFFFF)
-				set_stage(stage + 1);
+				set_stage(get_stage() + 1);
 			break;
 		case 9:
 		case 10:
 			ctrl_cpu0 = vmcs_read(CPU_EXEC_CTRL0);
 			vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu0 & ~CPU_IO);
-			set_stage(stage + 1);
+			set_stage(get_stage() + 1);
 			break;
 		default:
 			// Should not reach here
@@ -948,13 +948,13 @@ static void insn_intercept_main()
 		case INSN_CPU0:
 		case INSN_CPU1:
 		case INSN_ALWAYS_TRAP:
-			if (stage != cur_insn + 1)
+			if (get_stage() != cur_insn + 1)
 				report(insn_table[cur_insn].name, 0);
 			else
 				report(insn_table[cur_insn].name, 1);
 			break;
 		case INSN_NEVER_TRAP:
-			if (stage == cur_insn + 1)
+			if (get_stage() == cur_insn + 1)
 				report(insn_table[cur_insn].name, 0);
 			else
 				report(insn_table[cur_insn].name, 1);
@@ -985,7 +985,7 @@ static int insn_intercept_exit_handler()
 	if (insn_table[cur_insn].test_field & FIELD_INSN_INFO)
 		pass = pass && insn_table[cur_insn].insn_info == insn_info;
 	if (pass)
-		set_stage(stage + 1);
+		set_stage(get_stage() + 1);
 	vmcs_write(GUEST_RIP, guest_rip + insn_len);
 	return VMX_TEST_RESUME;
 }
-- 
1.8.1.1.298.ge7eed54


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

* [PATCH 3/5] VMX: Test both interception and execution of instructions
  2014-06-15 14:24 [PATCH 0/5] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
  2014-06-15 14:24 ` [PATCH 1/5] VMX: Add tests for CR3 and CR8 interception Jan Kiszka
  2014-06-15 14:24 ` [PATCH 2/5] VMX: Only use get_stage accessor Jan Kiszka
@ 2014-06-15 14:24 ` Jan Kiszka
  2014-06-15 14:24 ` [PATCH 4/5] VMX: Validate capability MSRs Jan Kiszka
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Jan Kiszka @ 2014-06-15 14:24 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

From: Jan Kiszka <jan.kiszka@siemens.com>

Extend the instruction interception test to also check for
interception-free execution.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 x86/vmx_tests.c | 121 +++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 72 insertions(+), 49 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index bf7aa2c..d0b67de 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -818,7 +818,6 @@ static int iobmp_exit_handler()
 #define INSN_CPU0		0
 #define INSN_CPU1		1
 #define INSN_ALWAYS_TRAP	2
-#define INSN_NEVER_TRAP		3
 
 #define FIELD_EXIT_QUAL		0x1
 #define FIELD_INSN_INFO		0x2
@@ -829,7 +828,7 @@ asm(
 	"insn_mwait: mwait;ret\n\t"
 	"insn_rdpmc: rdpmc;ret\n\t"
 	"insn_rdtsc: rdtsc;ret\n\t"
-	"insn_cr3_load: mov %rax,%cr3;ret\n\t"
+	"insn_cr3_load: mov cr3,%rax; mov %rax,%cr3;ret\n\t"
 	"insn_cr3_store: mov %cr3,%rax;ret\n\t"
 #ifdef __x86_64__
 	"insn_cr8_load: mov %rax,%cr8;ret\n\t"
@@ -859,6 +858,7 @@ extern void insn_cpuid();
 extern void insn_invd();
 
 u32 cur_insn;
+u64 cr3;
 
 struct insn_table {
 	const char *name;
@@ -912,55 +912,56 @@ static struct insn_table insn_table[] = {
 
 static int insn_intercept_init()
 {
-	u32 ctrl_cpu[2];
+	u32 ctrl_cpu;
 
-	ctrl_cpu[0] = vmcs_read(CPU_EXEC_CTRL0);
-	ctrl_cpu[0] |= CPU_HLT | CPU_INVLPG | CPU_MWAIT | CPU_RDPMC | CPU_RDTSC |
-		CPU_CR3_LOAD | CPU_CR3_STORE |
-#ifdef __x86_64__
-		CPU_CR8_LOAD | CPU_CR8_STORE |
-#endif
-		CPU_MONITOR | CPU_PAUSE | CPU_SECONDARY;
-	ctrl_cpu[0] &= ctrl_cpu_rev[0].clr;
-	vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu[0]);
-	ctrl_cpu[1] = vmcs_read(CPU_EXEC_CTRL1);
-	ctrl_cpu[1] |= CPU_WBINVD | CPU_RDRAND;
-	ctrl_cpu[1] &= ctrl_cpu_rev[1].clr;
-	vmcs_write(CPU_EXEC_CTRL1, ctrl_cpu[1]);
+	ctrl_cpu = ctrl_cpu_rev[0].set | CPU_SECONDARY;
+	ctrl_cpu &= ctrl_cpu_rev[0].clr;
+	vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu);
+	vmcs_write(CPU_EXEC_CTRL1, ctrl_cpu_rev[1].set);
+	cr3 = read_cr3();
 	return VMX_TEST_START;
 }
 
 static void insn_intercept_main()
 {
-	cur_insn = 0;
-	while(insn_table[cur_insn].name != NULL) {
-		set_stage(cur_insn);
-		if ((insn_table[cur_insn].type == INSN_CPU0
-			&& !(ctrl_cpu_rev[0].clr & insn_table[cur_insn].flag))
-			|| (insn_table[cur_insn].type == INSN_CPU1
-			&& !(ctrl_cpu_rev[1].clr & insn_table[cur_insn].flag))) {
-			printf("\tCPU_CTRL1.CPU_%s is not supported.\n",
-				insn_table[cur_insn].name);
+	char msg[80];
+
+	for (cur_insn = 0; insn_table[cur_insn].name != NULL; cur_insn++) {
+		set_stage(cur_insn * 2);
+		if ((insn_table[cur_insn].type == INSN_CPU0 &&
+		     !(ctrl_cpu_rev[0].clr & insn_table[cur_insn].flag)) ||
+		    (insn_table[cur_insn].type == INSN_CPU1 &&
+		     !(ctrl_cpu_rev[1].clr & insn_table[cur_insn].flag))) {
+			printf("\tCPU_CTRL%d.CPU_%s is not supported.\n",
+			       insn_table[cur_insn].type - INSN_CPU0,
+			       insn_table[cur_insn].name);
 			continue;
 		}
+
+		if ((insn_table[cur_insn].type == INSN_CPU0 &&
+		     !(ctrl_cpu_rev[0].set & insn_table[cur_insn].flag)) ||
+		    (insn_table[cur_insn].type == INSN_CPU1 &&
+		     !(ctrl_cpu_rev[1].set & insn_table[cur_insn].flag))) {
+			/* skip hlt, it stalls the guest and is tested below */
+			if (insn_table[cur_insn].insn_func != insn_hlt)
+				insn_table[cur_insn].insn_func();
+			snprintf(msg, sizeof(msg), "execute %s",
+				 insn_table[cur_insn].name);
+			report(msg, get_stage() == cur_insn * 2);
+		} else if (insn_table[cur_insn].type != INSN_ALWAYS_TRAP)
+			printf("\tCPU_CTRL%d.CPU_%s always traps.\n",
+			       insn_table[cur_insn].type - INSN_CPU0,
+			       insn_table[cur_insn].name);
+
+		vmcall();
+
 		insn_table[cur_insn].insn_func();
-		switch (insn_table[cur_insn].type) {
-		case INSN_CPU0:
-		case INSN_CPU1:
-		case INSN_ALWAYS_TRAP:
-			if (get_stage() != cur_insn + 1)
-				report(insn_table[cur_insn].name, 0);
-			else
-				report(insn_table[cur_insn].name, 1);
-			break;
-		case INSN_NEVER_TRAP:
-			if (get_stage() == cur_insn + 1)
-				report(insn_table[cur_insn].name, 0);
-			else
-				report(insn_table[cur_insn].name, 1);
-			break;
-		}
-		cur_insn ++;
+		snprintf(msg, sizeof(msg), "intercept %s",
+			 insn_table[cur_insn].name);
+		report(msg, get_stage() == cur_insn * 2 + 1);
+
+		set_stage(cur_insn * 2 + 1);
+		vmcall();
 	}
 }
 
@@ -978,14 +979,36 @@ static int insn_intercept_exit_handler()
 	exit_qual = vmcs_read(EXI_QUALIFICATION);
 	insn_len = vmcs_read(EXI_INST_LEN);
 	insn_info = vmcs_read(EXI_INST_INFO);
-	pass = (cur_insn == get_stage()) &&
+
+	if (reason == VMX_VMCALL) {
+		u32 val = 0;
+
+		if (insn_table[cur_insn].type == INSN_CPU0)
+			val = vmcs_read(CPU_EXEC_CTRL0);
+		else if (insn_table[cur_insn].type == INSN_CPU1)
+			val = vmcs_read(CPU_EXEC_CTRL1);
+
+		if (get_stage() & 1)
+			val &= ~insn_table[cur_insn].flag;
+		else
+			val |= insn_table[cur_insn].flag;
+
+		if (insn_table[cur_insn].type == INSN_CPU0)
+			vmcs_write(CPU_EXEC_CTRL0, val | ctrl_cpu_rev[0].set);
+		else if (insn_table[cur_insn].type == INSN_CPU1)
+			vmcs_write(CPU_EXEC_CTRL1, val | ctrl_cpu_rev[1].set);
+	} else {
+		pass = (cur_insn * 2 == get_stage()) &&
 			insn_table[cur_insn].reason == reason;
-	if (insn_table[cur_insn].test_field & FIELD_EXIT_QUAL)
-		pass = pass && insn_table[cur_insn].exit_qual == exit_qual;
-	if (insn_table[cur_insn].test_field & FIELD_INSN_INFO)
-		pass = pass && insn_table[cur_insn].insn_info == insn_info;
-	if (pass)
-		set_stage(get_stage() + 1);
+		if (insn_table[cur_insn].test_field & FIELD_EXIT_QUAL &&
+		    insn_table[cur_insn].exit_qual != exit_qual)
+			pass = false;
+		if (insn_table[cur_insn].test_field & FIELD_INSN_INFO &&
+		    insn_table[cur_insn].insn_info != insn_info)
+			pass = false;
+		if (pass)
+			set_stage(get_stage() + 1);
+	}
 	vmcs_write(GUEST_RIP, guest_rip + insn_len);
 	return VMX_TEST_RESUME;
 }
-- 
1.8.1.1.298.ge7eed54


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

* [PATCH 4/5] VMX: Validate capability MSRs
  2014-06-15 14:24 [PATCH 0/5] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
                   ` (2 preceding siblings ...)
  2014-06-15 14:24 ` [PATCH 3/5] VMX: Test both interception and execution of instructions Jan Kiszka
@ 2014-06-15 14:24 ` Jan Kiszka
  2014-06-16 11:00   ` Paolo Bonzini
  2014-06-15 14:24 ` [PATCH 5/5] VMX: Test behavior on set and cleared save/load debug controls Jan Kiszka
  2014-06-16 11:03 ` [PATCH 0/5] kvm-unit-tests: more instr. interceptions, debug control migration Paolo Bonzini
  5 siblings, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2014-06-15 14:24 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

From: Jan Kiszka <jan.kiszka@siemens.com>

Check for required-0 or required-1 bits as well as known field value
restrictions. Also check the consistency between VMX_*_CTLS and
VMX_TRUE_*_CTLS and between CR0/4_FIXED0 and CR0/4_FIXED1.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 x86/vmx.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 x86/vmx.h |  5 +++--
 2 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 1182eef..84c514b 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -635,6 +635,76 @@ static void test_vmptrst(void)
 	report("test vmptrst", (!ret) && (vmcs1 == vmcs2));
 }
 
+struct vmx_ctl_msr {
+	const char *name;
+	u32 index, true_index;
+	u32 default1;
+} vmx_ctl_msr[] = {
+	{ "MSR_IA32_VMX_PINBASED_CTLS", MSR_IA32_VMX_PINBASED_CTLS,
+	  MSR_IA32_VMX_TRUE_PIN, 0x16 },
+	{ "MSR_IA32_VMX_PROCBASED_CTLS", MSR_IA32_VMX_PROCBASED_CTLS,
+	  MSR_IA32_VMX_TRUE_PROC, 0x401e172 },
+	{ "MSR_IA32_VMX_PROCBASED_CTLS2", MSR_IA32_VMX_PROCBASED_CTLS2,
+	  MSR_IA32_VMX_PROCBASED_CTLS2, 0 },
+	{ "MSR_IA32_VMX_EXIT_CTLS", MSR_IA32_VMX_EXIT_CTLS,
+	  MSR_IA32_VMX_TRUE_EXIT, 0x36dff },
+	{ "MSR_IA32_VMX_ENTRY_CTLS", MSR_IA32_VMX_ENTRY_CTLS,
+	  MSR_IA32_VMX_TRUE_ENTRY, 0x11ff },
+};
+
+static void test_vmx_caps(void)
+{
+	u64 val, true_val, default1, fixed0, fixed1;
+	unsigned int n;
+	bool ok;
+
+	printf("\nTest suite: VMX capability reporting\n");
+
+	report("MSR_IA32_VMX_BASIC",
+	       (basic.revision & (1ul << 31)) == 0 &&
+	       basic.size > 0 && basic.size <= 4096 &&
+	       (basic.type == 0 || basic.type == 6) &&
+	       basic.reserved1 == 0 && basic.reserved2 == 0);
+
+	val = rdmsr(MSR_IA32_VMX_MISC);
+	report("MSR_IA32_VMX_MISC",
+	       (!(ctrl_cpu_rev[1].clr & CPU_URG) || val & (1ul << 5)) &&
+	       ((val >> 16) & 0x1ff) <= 256 &&
+	       (val & 0xc0007e00) == 0);
+
+	for (n = 0; n < ARRAY_SIZE(vmx_ctl_msr); n++) {
+		val = rdmsr(vmx_ctl_msr[n].index);
+		default1 = vmx_ctl_msr[n].default1;
+		ok = (val & default1) == default1 &&
+			((((u32)val) ^ (val >> 32)) & ~(val >> 32)) == 0;
+		if (ok && basic.ctrl) {
+			true_val = rdmsr(vmx_ctl_msr[n].true_index);
+			ok = (val >> 32) == (true_val >> 32) &&
+				((u32)(val ^ true_val) & ~default1) == 0;
+		}
+		report(vmx_ctl_msr[n].name, ok);
+	}
+
+	fixed0 = rdmsr(MSR_IA32_VMX_CR0_FIXED0);
+	fixed1 = rdmsr(MSR_IA32_VMX_CR0_FIXED1);
+	report("MSR_IA32_VMX_IA32_VMX_CR0_FIXED0/1",
+	       ((fixed0 ^ fixed1) & ~fixed1) == 0);
+
+	fixed0 = rdmsr(MSR_IA32_VMX_CR4_FIXED0);
+	fixed1 = rdmsr(MSR_IA32_VMX_CR4_FIXED1);
+	report("MSR_IA32_VMX_IA32_VMX_CR4_FIXED0/1",
+	       ((fixed0 ^ fixed1) & ~fixed1) == 0);
+
+	val = rdmsr(MSR_IA32_VMX_VMCS_ENUM);
+	report("MSR_IA32_VMX_VMCS_ENUM",
+	       (val & 0x3e) >= 0x2a &&
+	       (val & 0xfffffffffffffc01Ull) == 0);
+
+	val = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
+	report("MSR_IA32_VMX_EPT_VPID_CAP",
+	       (val & 0xfffff07ef9eebebeUll) == 0);
+}
+
 /* This function can only be called in guest */
 static void __attribute__((__used__)) hypercall(u32 hypercall_no)
 {
@@ -777,7 +847,7 @@ static int test_run(struct vmx_test *test)
 	regs = test->guest_regs;
 	vmcs_write(GUEST_RFLAGS, regs.rflags | 0x2);
 	launched = 0;
-	printf("\nTest suite : %s\n", test->name);
+	printf("\nTest suite: %s\n", test->name);
 	vmx_run();
 	if (vmx_off()) {
 		printf("%s : vmxoff failed.\n", __func__);
@@ -816,6 +886,7 @@ int main(void)
 		goto exit;
 	}
 	test_vmxoff();
+	test_vmx_caps();
 
 	while (vmx_tests[++i].name != NULL)
 		if (test_run(&vmx_tests[i]))
diff --git a/x86/vmx.h b/x86/vmx.h
index 69a5385..38ec3c5 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -46,12 +46,13 @@ union vmx_basic {
 	struct {
 		u32 revision;
 		u32	size:13,
-			: 3,
+			reserved1: 3,
 			width:1,
 			dual:1,
 			type:4,
 			insouts:1,
-			ctrl:1;
+			ctrl:1,
+			reserved2:8;
 	};
 };
 
-- 
1.8.1.1.298.ge7eed54


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

* [PATCH 5/5] VMX: Test behavior on set and cleared save/load debug controls
  2014-06-15 14:24 [PATCH 0/5] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
                   ` (3 preceding siblings ...)
  2014-06-15 14:24 ` [PATCH 4/5] VMX: Validate capability MSRs Jan Kiszka
@ 2014-06-15 14:24 ` Jan Kiszka
  2014-06-16 11:02   ` Paolo Bonzini
  2014-06-16 11:03 ` [PATCH 0/5] kvm-unit-tests: more instr. interceptions, debug control migration Paolo Bonzini
  5 siblings, 1 reply; 14+ messages in thread
From: Jan Kiszka @ 2014-06-15 14:24 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

From: Jan Kiszka <jan.kiszka@siemens.com>

This particularly checks the case when debug controls are not to be
loaded/saved on host-guest transitions.

We have to fake results related to IA32_DEBUGCTL as support for this MSR
is missing KVM. The test already contains all bits required once KVM
adds support.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 x86/vmx.h       |   2 ++
 x86/vmx_tests.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+)

diff --git a/x86/vmx.h b/x86/vmx.h
index 38ec3c5..3c4830f 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -326,6 +326,7 @@ enum Reason {
 #define X86_EFLAGS_ZF	0x00000040 /* Zero Flag */
 
 enum Ctrl_exi {
+	EXI_SAVE_DBGCTLS	= 1UL << 2,
 	EXI_HOST_64             = 1UL << 9,
 	EXI_LOAD_PERF		= 1UL << 12,
 	EXI_INTA                = 1UL << 15,
@@ -337,6 +338,7 @@ enum Ctrl_exi {
 };
 
 enum Ctrl_ent {
+	ENT_LOAD_DBGCTLS	= 1UL << 2,
 	ENT_GUEST_64            = 1UL << 9,
 	ENT_LOAD_PAT		= 1UL << 14,
 	ENT_LOAD_EFER           = 1UL << 15,
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index d0b67de..0f4cfc2 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1406,6 +1406,114 @@ static int interrupt_exit_handler(void)
 	return VMX_TEST_VMEXIT;
 }
 
+static int dbgctls_init(struct vmcs *vmcs)
+{
+	u64 dr7 = 0x402;
+	u64 zero = 0;
+
+	msr_bmp_init();
+	asm volatile(
+		"mov %0,%%dr0\n\t"
+		"mov %0,%%dr1\n\t"
+		"mov %0,%%dr2\n\t"
+		"mov %1,%%dr7\n\t"
+		: : "r" (zero), "r" (dr7));
+	wrmsr(MSR_IA32_DEBUGCTLMSR, 0x1);
+	vmcs_write(GUEST_DR7, 0x404);
+	vmcs_write(GUEST_DEBUGCTL, 0x2);
+
+	vmcs_write(ENT_CONTROLS, vmcs_read(ENT_CONTROLS) | ENT_LOAD_DBGCTLS);
+	vmcs_write(EXI_CONTROLS, vmcs_read(EXI_CONTROLS) | EXI_SAVE_DBGCTLS);
+
+	return VMX_TEST_START;
+}
+
+static void dbgctls_main(void)
+{
+	u64 dr7, debugctl;
+
+	asm volatile("mov %%dr7,%0" : "=r" (dr7));
+	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
+	debugctl = 0x2; /* KVM does not support DEBUGCTL so far */
+	report("Load debug controls", dr7 == 0x404 && debugctl == 0x2);
+
+	dr7 = 0x408;
+	asm volatile("mov %0,%%dr7" : : "r" (dr7));
+	wrmsr(MSR_IA32_DEBUGCTLMSR, 0x3);
+
+	set_stage(0);
+	vmcall();
+	report("Save debug controls", get_stage() == 1);
+
+	if (ctrl_enter_rev.set & ENT_LOAD_DBGCTLS ||
+	    ctrl_exit_rev.set & EXI_SAVE_DBGCTLS) {
+		printf("\tDebug controls are always loaded/saved\n");
+		return;
+	}
+	set_stage(2);
+	vmcall();
+
+	asm volatile("mov %%dr7,%0" : "=r" (dr7));
+	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
+	debugctl = 0x1; /* no KVM support */
+	report("Guest=host debug controls", dr7 == 0x402 && debugctl == 0x1);
+
+	dr7 = 0x408;
+	asm volatile("mov %0,%%dr7" : : "r" (dr7));
+	wrmsr(MSR_IA32_DEBUGCTLMSR, 0x3);
+
+	set_stage(3);
+	vmcall();
+	report("Don't save debug controls", get_stage() == 4);
+}
+
+static int dbgctls_exit_handler(void)
+{
+	unsigned int reason = vmcs_read(EXI_REASON) & 0xff;
+	u32 insn_len = vmcs_read(EXI_INST_LEN);
+	u64 guest_rip = vmcs_read(GUEST_RIP);
+	u64 dr7, debugctl;
+
+	asm volatile("mov %%dr7,%0" : "=r" (dr7));
+	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
+
+	switch (reason) {
+	case VMX_VMCALL:
+		switch (get_stage()) {
+		case 0:
+			if (dr7 == 0x400 && debugctl == 0 &&
+			    vmcs_read(GUEST_DR7) == 0x408 &&
+			    vmcs_read(GUEST_DEBUGCTL) == /* 0x3 no KVM support*/ 0x2)
+				set_stage(1);
+			break;
+		case 2:
+			dr7 = 0x402;
+			asm volatile("mov %0,%%dr7" : : "r" (dr7));
+			wrmsr(MSR_IA32_DEBUGCTLMSR, 0x1);
+			vmcs_write(GUEST_DR7, 0x404);
+			vmcs_write(GUEST_DEBUGCTL, 0x2);
+
+			vmcs_write(ENT_CONTROLS,
+				vmcs_read(ENT_CONTROLS) & ~ENT_LOAD_DBGCTLS);
+			vmcs_write(EXI_CONTROLS,
+				vmcs_read(EXI_CONTROLS) & ~EXI_SAVE_DBGCTLS);
+			break;
+		case 3:
+			if (dr7 == 0x400 && debugctl == 0 &&
+			    vmcs_read(GUEST_DR7) == 0x404 &&
+			    vmcs_read(GUEST_DEBUGCTL) == 0x2)
+				set_stage(4);
+			break;
+		}
+		vmcs_write(GUEST_RIP, guest_rip + insn_len);
+		return VMX_TEST_RESUME;
+	default:
+		printf("Unknown exit reason, %d\n", reason);
+		print_vmexit_info();
+	}
+	return VMX_TEST_VMEXIT;
+}
+
 /* 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} },
@@ -1425,5 +1533,7 @@ struct vmx_test vmx_tests[] = {
 	{ "EPT framework", ept_init, ept_main, ept_exit_handler, NULL, {0} },
 	{ "interrupt", interrupt_init, interrupt_main,
 		interrupt_exit_handler, NULL, {0} },
+	{ "debug controls", dbgctls_init, dbgctls_main, dbgctls_exit_handler,
+		NULL, {0} },
 	{ NULL, NULL, NULL, NULL, NULL, {0} },
 };
-- 
1.8.1.1.298.ge7eed54


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

* Re: [PATCH 1/5] VMX: Add tests for CR3 and CR8 interception
  2014-06-15 14:24 ` [PATCH 1/5] VMX: Add tests for CR3 and CR8 interception Jan Kiszka
@ 2014-06-16 10:53   ` Paolo Bonzini
  2014-06-16 11:31     ` Jan Kiszka
  0 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2014-06-16 10:53 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: kvm, Bandan Das

Il 15/06/2014 16:24, Jan Kiszka ha scritto:
> +++ b/x86/vmx_tests.c
> @@ -820,8 +820,8 @@ static int iobmp_exit_handler()
>  #define INSN_ALWAYS_TRAP	2
>  #define INSN_NEVER_TRAP		3
>
> -#define FIELD_EXIT_QUAL		0
> -#define FIELD_INSN_INFO		1
> +#define FIELD_EXIT_QUAL		0x1
> +#define FIELD_INSN_INFO		0x2

Small nit, using (1 << 0) and (1 << 1) would have reminded better this 
lazy maintainer that these fields form a bitmask. :)

Paolo

>
>  asm(



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

* Re: [PATCH 4/5] VMX: Validate capability MSRs
  2014-06-15 14:24 ` [PATCH 4/5] VMX: Validate capability MSRs Jan Kiszka
@ 2014-06-16 11:00   ` Paolo Bonzini
  2014-06-16 11:26     ` Jan Kiszka
  0 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2014-06-16 11:00 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: kvm, Bandan Das

Il 15/06/2014 16:24, Jan Kiszka ha scritto:
> +	for (n = 0; n < ARRAY_SIZE(vmx_ctl_msr); n++) {
> +		val = rdmsr(vmx_ctl_msr[n].index);
> +		default1 = vmx_ctl_msr[n].default1;
> +		ok = (val & default1) == default1 &&
> +			((((u32)val) ^ (val >> 32)) & ~(val >> 32)) == 0;

Ouch, this can sure be make this more readable.

Please unify these:

union vmx_ctrl_pin {
         u64 val;
         struct {
                 u32 set, clr;
         };
};

union vmx_ctrl_cpu {
         u64 val;
         struct {
                 u32 set, clr;
         };
};

union vmx_ctrl_exit {
         u64 val;
         struct {
                 u32 set, clr;
         };
};

union vmx_ctrl_ent {
         u64 val;
         struct {
                 u32 set, clr;
         };
};


into a single "union vmx_ctl_msr", and use this union for val and 
true_val as well.

Paolo

> +		if (ok && basic.ctrl) {
> +			true_val = rdmsr(vmx_ctl_msr[n].true_index);
> +			ok = (val >> 32) == (true_val >> 32) &&
> +				((u32)(val ^ true_val) & ~default1) == 0;
> +		}
> +		report(vmx_ctl_msr[n].name, ok);



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

* Re: [PATCH 5/5] VMX: Test behavior on set and cleared save/load debug controls
  2014-06-15 14:24 ` [PATCH 5/5] VMX: Test behavior on set and cleared save/load debug controls Jan Kiszka
@ 2014-06-16 11:02   ` Paolo Bonzini
  2014-06-16 11:28     ` Jan Kiszka
  0 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2014-06-16 11:02 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: kvm, Bandan Das

Il 15/06/2014 16:24, Jan Kiszka ha scritto:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> This particularly checks the case when debug controls are not to be
> loaded/saved on host-guest transitions.
> 
> We have to fake results related to IA32_DEBUGCTL as support for this MSR
> is missing KVM. The test already contains all bits required once KVM
> adds support.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  x86/vmx.h       |   2 ++
>  x86/vmx_tests.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 112 insertions(+)
> 
> diff --git a/x86/vmx.h b/x86/vmx.h
> index 38ec3c5..3c4830f 100644
> --- a/x86/vmx.h
> +++ b/x86/vmx.h
> @@ -326,6 +326,7 @@ enum Reason {
>  #define X86_EFLAGS_ZF	0x00000040 /* Zero Flag */
>  
>  enum Ctrl_exi {
> +	EXI_SAVE_DBGCTLS	= 1UL << 2,
>  	EXI_HOST_64             = 1UL << 9,
>  	EXI_LOAD_PERF		= 1UL << 12,
>  	EXI_INTA                = 1UL << 15,
> @@ -337,6 +338,7 @@ enum Ctrl_exi {
>  };
>  
>  enum Ctrl_ent {
> +	ENT_LOAD_DBGCTLS	= 1UL << 2,
>  	ENT_GUEST_64            = 1UL << 9,
>  	ENT_LOAD_PAT		= 1UL << 14,
>  	ENT_LOAD_EFER           = 1UL << 15,
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index d0b67de..0f4cfc2 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -1406,6 +1406,114 @@ static int interrupt_exit_handler(void)
>  	return VMX_TEST_VMEXIT;
>  }
>  
> +static int dbgctls_init(struct vmcs *vmcs)
> +{
> +	u64 dr7 = 0x402;
> +	u64 zero = 0;
> +
> +	msr_bmp_init();
> +	asm volatile(
> +		"mov %0,%%dr0\n\t"
> +		"mov %0,%%dr1\n\t"
> +		"mov %0,%%dr2\n\t"
> +		"mov %1,%%dr7\n\t"
> +		: : "r" (zero), "r" (dr7));
> +	wrmsr(MSR_IA32_DEBUGCTLMSR, 0x1);
> +	vmcs_write(GUEST_DR7, 0x404);
> +	vmcs_write(GUEST_DEBUGCTL, 0x2);
> +
> +	vmcs_write(ENT_CONTROLS, vmcs_read(ENT_CONTROLS) | ENT_LOAD_DBGCTLS);
> +	vmcs_write(EXI_CONTROLS, vmcs_read(EXI_CONTROLS) | EXI_SAVE_DBGCTLS);
> +
> +	return VMX_TEST_START;
> +}
> +
> +static void dbgctls_main(void)
> +{
> +	u64 dr7, debugctl;
> +
> +	asm volatile("mov %%dr7,%0" : "=r" (dr7));
> +	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> +	debugctl = 0x2; /* KVM does not support DEBUGCTL so far */
> +	report("Load debug controls", dr7 == 0x404 && debugctl == 0x2);

Please comment instead like this:

	report("Load debug controls", dr7 == 0x404 /* && debugctl == 0x2 */ );

> +	asm volatile("mov %%dr7,%0" : "=r" (dr7));
> +	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
> +	debugctl = 0x1; /* no KVM support */
> +	report("Guest=host debug controls", dr7 == 0x402 && debugctl == 0x1);

Same here.

> +			if (dr7 == 0x400 && debugctl == 0 &&
> +			    vmcs_read(GUEST_DR7) == 0x408 &&
> +			    vmcs_read(GUEST_DEBUGCTL) == /* 0x3 no KVM support*/ 0x2)

and here.

> +				set_stage(1);
> +			break;
> +		case 2:
> +			dr7 = 0x402;
> +			asm volatile("mov %0,%%dr7" : : "r" (dr7));
> +			wrmsr(MSR_IA32_DEBUGCTLMSR, 0x1);
> +			vmcs_write(GUEST_DR7, 0x404);
> +			vmcs_write(GUEST_DEBUGCTL, 0x2);
> +
> +			vmcs_write(ENT_CONTROLS,
> +				vmcs_read(ENT_CONTROLS) & ~ENT_LOAD_DBGCTLS);
> +			vmcs_write(EXI_CONTROLS,
> +				vmcs_read(EXI_CONTROLS) & ~EXI_SAVE_DBGCTLS);
> +			break;
> +		case 3:
> +			if (dr7 == 0x400 && debugctl == 0 &&
> +			    vmcs_read(GUEST_DR7) == 0x404 &&
> +			    vmcs_read(GUEST_DEBUGCTL) == 0x2)

and here too, even though it happens to work.

Paolo

> +				set_stage(4);
> +			break;
> +		}
> +		vmcs_write(GUEST_RIP, guest_rip + insn_len);
> +		return VMX_TEST_RESUME;
> +	default:
> +		printf("Unknown exit reason, %d\n", reason);
> +		print_vmexit_info();
> +	}
> +	return VMX_TEST_VMEXIT;
> +}
> +
>  /* 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} },
> @@ -1425,5 +1533,7 @@ struct vmx_test vmx_tests[] = {
>  	{ "EPT framework", ept_init, ept_main, ept_exit_handler, NULL, {0} },
>  	{ "interrupt", interrupt_init, interrupt_main,
>  		interrupt_exit_handler, NULL, {0} },
> +	{ "debug controls", dbgctls_init, dbgctls_main, dbgctls_exit_handler,
> +		NULL, {0} },
>  	{ NULL, NULL, NULL, NULL, NULL, {0} },
>  };
> 


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

* Re: [PATCH 0/5] kvm-unit-tests: more instr. interceptions, debug control migration
  2014-06-15 14:24 [PATCH 0/5] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
                   ` (4 preceding siblings ...)
  2014-06-15 14:24 ` [PATCH 5/5] VMX: Test behavior on set and cleared save/load debug controls Jan Kiszka
@ 2014-06-16 11:03 ` Paolo Bonzini
  5 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2014-06-16 11:03 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: kvm, Bandan Das

Il 15/06/2014 16:24, Jan Kiszka ha scritto:
> The tests corresponding to (and going beyond) the issues fixed in
> http://thread.gmane.org/gmane.comp.emulators.kvm.devel/123282
>
> Jan Kiszka (5):
>   VMX: Add tests for CR3 and CR8 interception
>   VMX: Only use get_stage accessor
>   VMX: Test both interception and execution of instructions
>   VMX: Validate capability MSRs
>   VMX: Test behavior on set and cleared save/load debug controls
>
>  x86/vmx.c       |  73 ++++++++++++-
>  x86/vmx.h       |   9 +-
>  x86/vmx_tests.c | 327 +++++++++++++++++++++++++++++++++++++++++---------------
>  3 files changed, 322 insertions(+), 87 deletions(-)
>

Thanks, I'll apply the first three patches as soon as Linus releases 
-rc1 and the corresponding KVM bits hit kernel.org.

I replied with comments to the last two patches.

Paolo

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

* Re: [PATCH 4/5] VMX: Validate capability MSRs
  2014-06-16 11:00   ` Paolo Bonzini
@ 2014-06-16 11:26     ` Jan Kiszka
  0 siblings, 0 replies; 14+ messages in thread
From: Jan Kiszka @ 2014-06-16 11:26 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

On 2014-06-16 13:00, Paolo Bonzini wrote:
> Il 15/06/2014 16:24, Jan Kiszka ha scritto:
>> +    for (n = 0; n < ARRAY_SIZE(vmx_ctl_msr); n++) {
>> +        val = rdmsr(vmx_ctl_msr[n].index);
>> +        default1 = vmx_ctl_msr[n].default1;
>> +        ok = (val & default1) == default1 &&
>> +            ((((u32)val) ^ (val >> 32)) & ~(val >> 32)) == 0;
> 
> Ouch, this can sure be make this more readable.
> 
> Please unify these:
> 
> union vmx_ctrl_pin {
>         u64 val;
>         struct {
>                 u32 set, clr;
>         };
> };
> 
> union vmx_ctrl_cpu {
>         u64 val;
>         struct {
>                 u32 set, clr;
>         };
> };
> 
> union vmx_ctrl_exit {
>         u64 val;
>         struct {
>                 u32 set, clr;
>         };
> };
> 
> union vmx_ctrl_ent {
>         u64 val;
>         struct {
>                 u32 set, clr;
>         };
> };
> 
> 
> into a single "union vmx_ctl_msr", and use this union for val and
> true_val as well.

OK, will do.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

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

* Re: [PATCH 5/5] VMX: Test behavior on set and cleared save/load debug controls
  2014-06-16 11:02   ` Paolo Bonzini
@ 2014-06-16 11:28     ` Jan Kiszka
  0 siblings, 0 replies; 14+ messages in thread
From: Jan Kiszka @ 2014-06-16 11:28 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

On 2014-06-16 13:02, Paolo Bonzini wrote:
> Il 15/06/2014 16:24, Jan Kiszka ha scritto:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> This particularly checks the case when debug controls are not to be
>> loaded/saved on host-guest transitions.
>>
>> We have to fake results related to IA32_DEBUGCTL as support for this MSR
>> is missing KVM. The test already contains all bits required once KVM
>> adds support.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  x86/vmx.h       |   2 ++
>>  x86/vmx_tests.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 112 insertions(+)
>>
>> diff --git a/x86/vmx.h b/x86/vmx.h
>> index 38ec3c5..3c4830f 100644
>> --- a/x86/vmx.h
>> +++ b/x86/vmx.h
>> @@ -326,6 +326,7 @@ enum Reason {
>>  #define X86_EFLAGS_ZF	0x00000040 /* Zero Flag */
>>  
>>  enum Ctrl_exi {
>> +	EXI_SAVE_DBGCTLS	= 1UL << 2,
>>  	EXI_HOST_64             = 1UL << 9,
>>  	EXI_LOAD_PERF		= 1UL << 12,
>>  	EXI_INTA                = 1UL << 15,
>> @@ -337,6 +338,7 @@ enum Ctrl_exi {
>>  };
>>  
>>  enum Ctrl_ent {
>> +	ENT_LOAD_DBGCTLS	= 1UL << 2,
>>  	ENT_GUEST_64            = 1UL << 9,
>>  	ENT_LOAD_PAT		= 1UL << 14,
>>  	ENT_LOAD_EFER           = 1UL << 15,
>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>> index d0b67de..0f4cfc2 100644
>> --- a/x86/vmx_tests.c
>> +++ b/x86/vmx_tests.c
>> @@ -1406,6 +1406,114 @@ static int interrupt_exit_handler(void)
>>  	return VMX_TEST_VMEXIT;
>>  }
>>  
>> +static int dbgctls_init(struct vmcs *vmcs)
>> +{
>> +	u64 dr7 = 0x402;
>> +	u64 zero = 0;
>> +
>> +	msr_bmp_init();
>> +	asm volatile(
>> +		"mov %0,%%dr0\n\t"
>> +		"mov %0,%%dr1\n\t"
>> +		"mov %0,%%dr2\n\t"
>> +		"mov %1,%%dr7\n\t"
>> +		: : "r" (zero), "r" (dr7));
>> +	wrmsr(MSR_IA32_DEBUGCTLMSR, 0x1);
>> +	vmcs_write(GUEST_DR7, 0x404);
>> +	vmcs_write(GUEST_DEBUGCTL, 0x2);
>> +
>> +	vmcs_write(ENT_CONTROLS, vmcs_read(ENT_CONTROLS) | ENT_LOAD_DBGCTLS);
>> +	vmcs_write(EXI_CONTROLS, vmcs_read(EXI_CONTROLS) | EXI_SAVE_DBGCTLS);
>> +
>> +	return VMX_TEST_START;
>> +}
>> +
>> +static void dbgctls_main(void)
>> +{
>> +	u64 dr7, debugctl;
>> +
>> +	asm volatile("mov %%dr7,%0" : "=r" (dr7));
>> +	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
>> +	debugctl = 0x2; /* KVM does not support DEBUGCTL so far */
>> +	report("Load debug controls", dr7 == 0x404 && debugctl == 0x2);
> 
> Please comment instead like this:
> 
> 	report("Load debug controls", dr7 == 0x404 /* && debugctl == 0x2 */ );
> 
>> +	asm volatile("mov %%dr7,%0" : "=r" (dr7));
>> +	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
>> +	debugctl = 0x1; /* no KVM support */
>> +	report("Guest=host debug controls", dr7 == 0x402 && debugctl == 0x1);
> 
> Same here.
> 
>> +			if (dr7 == 0x400 && debugctl == 0 &&
>> +			    vmcs_read(GUEST_DR7) == 0x408 &&
>> +			    vmcs_read(GUEST_DEBUGCTL) == /* 0x3 no KVM support*/ 0x2)
> 
> and here.
> 
>> +				set_stage(1);
>> +			break;
>> +		case 2:
>> +			dr7 = 0x402;
>> +			asm volatile("mov %0,%%dr7" : : "r" (dr7));
>> +			wrmsr(MSR_IA32_DEBUGCTLMSR, 0x1);
>> +			vmcs_write(GUEST_DR7, 0x404);
>> +			vmcs_write(GUEST_DEBUGCTL, 0x2);
>> +
>> +			vmcs_write(ENT_CONTROLS,
>> +				vmcs_read(ENT_CONTROLS) & ~ENT_LOAD_DBGCTLS);
>> +			vmcs_write(EXI_CONTROLS,
>> +				vmcs_read(EXI_CONTROLS) & ~EXI_SAVE_DBGCTLS);
>> +			break;
>> +		case 3:
>> +			if (dr7 == 0x400 && debugctl == 0 &&
>> +			    vmcs_read(GUEST_DR7) == 0x404 &&
>> +			    vmcs_read(GUEST_DEBUGCTL) == 0x2)
> 
> and here too, even though it happens to work.

OK, no problem.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

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

* Re: [PATCH 1/5] VMX: Add tests for CR3 and CR8 interception
  2014-06-16 10:53   ` Paolo Bonzini
@ 2014-06-16 11:31     ` Jan Kiszka
  0 siblings, 0 replies; 14+ messages in thread
From: Jan Kiszka @ 2014-06-16 11:31 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

On 2014-06-16 12:53, Paolo Bonzini wrote:
> Il 15/06/2014 16:24, Jan Kiszka ha scritto:
>> +++ b/x86/vmx_tests.c
>> @@ -820,8 +820,8 @@ static int iobmp_exit_handler()
>>  #define INSN_ALWAYS_TRAP    2
>>  #define INSN_NEVER_TRAP        3
>>
>> -#define FIELD_EXIT_QUAL        0
>> -#define FIELD_INSN_INFO        1
>> +#define FIELD_EXIT_QUAL        0x1
>> +#define FIELD_INSN_INFO        0x2
> 
> Small nit, using (1 << 0) and (1 << 1) would have reminded better this
> lazy maintainer that these fields form a bitmask. :)

Yes, see also the bug in KVM constants I posted recently. I'm inclined
to apply such a pattern exclusively for now on, at least where this
produces no inconsistencies with existing code.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

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

* Re: [PATCH 2/5] VMX: Only use get_stage accessor
  2014-06-15 14:24 ` [PATCH 2/5] VMX: Only use get_stage accessor Jan Kiszka
@ 2014-06-16 17:19   ` Bandan Das
  0 siblings, 0 replies; 14+ messages in thread
From: Bandan Das @ 2014-06-16 17:19 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Paolo Bonzini, kvm

Jan Kiszka <jan.kiszka@web.de> writes:

> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Consistently make sure we are not affected by any compiler reordering
> when evaluating the current stage.

Should we prevent accidental calls to the variable directly by moving
get/set to vmx.c or a separate file in lib/x86 altogether ?
 

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  x86/vmx_tests.c | 80 ++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 40 insertions(+), 40 deletions(-)
>
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index d0ce365..bf7aa2c 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -415,13 +415,13 @@ static void cr_shadowing_main()
>  	// Test read through
>  	set_stage(0);
>  	guest_cr0 = read_cr0();
> -	if (stage == 1)
> +	if (get_stage() == 1)
>  		report("Read through CR0", 0);
>  	else
>  		vmcall();
>  	set_stage(1);
>  	guest_cr4 = read_cr4();
> -	if (stage == 2)
> +	if (get_stage() == 2)
>  		report("Read through CR4", 0);
>  	else
>  		vmcall();
> @@ -430,13 +430,13 @@ static void cr_shadowing_main()
>  	guest_cr4 = guest_cr4 ^ (X86_CR4_TSD | X86_CR4_DE);
>  	set_stage(2);
>  	write_cr0(guest_cr0);
> -	if (stage == 3)
> +	if (get_stage() == 3)
>  		report("Write throuth CR0", 0);
>  	else
>  		vmcall();
>  	set_stage(3);
>  	write_cr4(guest_cr4);
> -	if (stage == 4)
> +	if (get_stage() == 4)
>  		report("Write through CR4", 0);
>  	else
>  		vmcall();
> @@ -444,7 +444,7 @@ static void cr_shadowing_main()
>  	set_stage(4);
>  	vmcall();
>  	cr0 = read_cr0();
> -	if (stage != 5) {
> +	if (get_stage() != 5) {
>  		if (cr0 == guest_cr0)
>  			report("Read shadowing CR0", 1);
>  		else
> @@ -452,7 +452,7 @@ static void cr_shadowing_main()
>  	}
>  	set_stage(5);
>  	cr4 = read_cr4();
> -	if (stage != 6) {
> +	if (get_stage() != 6) {
>  		if (cr4 == guest_cr4)
>  			report("Read shadowing CR4", 1);
>  		else
> @@ -461,13 +461,13 @@ static void cr_shadowing_main()
>  	// Test write shadow (same value with shadow)
>  	set_stage(6);
>  	write_cr0(guest_cr0);
> -	if (stage == 7)
> +	if (get_stage() == 7)
>  		report("Write shadowing CR0 (same value with shadow)", 0);
>  	else
>  		vmcall();
>  	set_stage(7);
>  	write_cr4(guest_cr4);
> -	if (stage == 8)
> +	if (get_stage() == 8)
>  		report("Write shadowing CR4 (same value with shadow)", 0);
>  	else
>  		vmcall();
> @@ -478,7 +478,7 @@ static void cr_shadowing_main()
>  		"mov %%rsi, %%cr0\n\t"
>  		::"m"(tmp)
>  		:"rsi", "memory", "cc");
> -	if (stage != 9)
> +	if (get_stage() != 9)
>  		report("Write shadowing different X86_CR0_TS", 0);
>  	else
>  		report("Write shadowing different X86_CR0_TS", 1);
> @@ -488,7 +488,7 @@ static void cr_shadowing_main()
>  		"mov %%rsi, %%cr0\n\t"
>  		::"m"(tmp)
>  		:"rsi", "memory", "cc");
> -	if (stage != 10)
> +	if (get_stage() != 10)
>  		report("Write shadowing different X86_CR0_MP", 0);
>  	else
>  		report("Write shadowing different X86_CR0_MP", 1);
> @@ -498,7 +498,7 @@ static void cr_shadowing_main()
>  		"mov %%rsi, %%cr4\n\t"
>  		::"m"(tmp)
>  		:"rsi", "memory", "cc");
> -	if (stage != 11)
> +	if (get_stage() != 11)
>  		report("Write shadowing different X86_CR4_TSD", 0);
>  	else
>  		report("Write shadowing different X86_CR4_TSD", 1);
> @@ -508,7 +508,7 @@ static void cr_shadowing_main()
>  		"mov %%rsi, %%cr4\n\t"
>  		::"m"(tmp)
>  		:"rsi", "memory", "cc");
> -	if (stage != 12)
> +	if (get_stage() != 12)
>  		report("Write shadowing different X86_CR4_DE", 0);
>  	else
>  		report("Write shadowing different X86_CR4_DE", 1);
> @@ -584,31 +584,31 @@ static int cr_shadowing_exit_handler()
>  		switch (get_stage()) {
>  		case 4:
>  			report("Read shadowing CR0", 0);
> -			set_stage(stage + 1);
> +			set_stage(get_stage() + 1);
>  			break;
>  		case 5:
>  			report("Read shadowing CR4", 0);
> -			set_stage(stage + 1);
> +			set_stage(get_stage() + 1);
>  			break;
>  		case 6:
>  			report("Write shadowing CR0 (same value)", 0);
> -			set_stage(stage + 1);
> +			set_stage(get_stage() + 1);
>  			break;
>  		case 7:
>  			report("Write shadowing CR4 (same value)", 0);
> -			set_stage(stage + 1);
> +			set_stage(get_stage() + 1);
>  			break;
>  		case 8:
>  		case 9:
>  			// 0x600 encodes "mov %esi, %cr0"
>  			if (exit_qual == 0x600)
> -				set_stage(stage + 1);
> +				set_stage(get_stage() + 1);
>  			break;
>  		case 10:
>  		case 11:
>  			// 0x604 encodes "mov %esi, %cr4"
>  			if (exit_qual == 0x604)
> -				set_stage(stage + 1);
> +				set_stage(get_stage() + 1);
>  			break;
>  		default:
>  			// Should not reach here
> @@ -648,7 +648,7 @@ static void iobmp_main()
>  	set_stage(0);
>  	inb(0x5000);
>  	outb(0x0, 0x5000);
> -	if (stage != 0)
> +	if (get_stage() != 0)
>  		report("I/O bitmap - I/O pass", 0);
>  	else
>  		report("I/O bitmap - I/O pass", 1);
> @@ -656,39 +656,39 @@ static void iobmp_main()
>  	((u8 *)io_bitmap_a)[0] = 0xFF;
>  	set_stage(2);
>  	inb(0x0);
> -	if (stage != 3)
> +	if (get_stage() != 3)
>  		report("I/O bitmap - trap in", 0);
>  	else
>  		report("I/O bitmap - trap in", 1);
>  	set_stage(3);
>  	outw(0x0, 0x0);
> -	if (stage != 4)
> +	if (get_stage() != 4)
>  		report("I/O bitmap - trap out", 0);
>  	else
>  		report("I/O bitmap - trap out", 1);
>  	set_stage(4);
>  	inl(0x0);
> -	if (stage != 5)
> +	if (get_stage() != 5)
>  		report("I/O bitmap - I/O width, long", 0);
>  	// test low/high IO port
>  	set_stage(5);
>  	((u8 *)io_bitmap_a)[0x5000 / 8] = (1 << (0x5000 % 8));
>  	inb(0x5000);
> -	if (stage == 6)
> +	if (get_stage() == 6)
>  		report("I/O bitmap - I/O port, low part", 1);
>  	else
>  		report("I/O bitmap - I/O port, low part", 0);
>  	set_stage(6);
>  	((u8 *)io_bitmap_b)[0x1000 / 8] = (1 << (0x1000 % 8));
>  	inb(0x9000);
> -	if (stage == 7)
> +	if (get_stage() == 7)
>  		report("I/O bitmap - I/O port, high part", 1);
>  	else
>  		report("I/O bitmap - I/O port, high part", 0);
>  	// test partial pass
>  	set_stage(7);
>  	inl(0x4FFF);
> -	if (stage == 8)
> +	if (get_stage() == 8)
>  		report("I/O bitmap - partial pass", 1);
>  	else
>  		report("I/O bitmap - partial pass", 0);
> @@ -697,18 +697,18 @@ static void iobmp_main()
>  	memset(io_bitmap_a, 0x0, PAGE_SIZE);
>  	memset(io_bitmap_b, 0x0, PAGE_SIZE);
>  	inl(0xFFFF);
> -	if (stage == 9)
> +	if (get_stage() == 9)
>  		report("I/O bitmap - overrun", 1);
>  	else
>  		report("I/O bitmap - overrun", 0);
>  	set_stage(9);
>  	vmcall();
>  	outb(0x0, 0x0);
> -	report("I/O bitmap - ignore unconditional exiting", stage == 9);
> +	report("I/O bitmap - ignore unconditional exiting", get_stage() == 9);
>  	set_stage(10);
>  	vmcall();
>  	outb(0x0, 0x0);
> -	report("I/O bitmap - unconditional exiting", stage == 11);
> +	report("I/O bitmap - unconditional exiting", get_stage() == 11);
>  }
>  
>  static int iobmp_exit_handler()
> @@ -726,7 +726,7 @@ static int iobmp_exit_handler()
>  		switch (get_stage()) {
>  		case 0:
>  		case 1:
> -			set_stage(stage + 1);
> +			set_stage(get_stage() + 1);
>  			break;
>  		case 2:
>  			if ((exit_qual & VMX_IO_SIZE_MASK) != _VMX_IO_BYTE)
> @@ -737,7 +737,7 @@ static int iobmp_exit_handler()
>  				report("I/O bitmap - I/O direction, in", 0);
>  			else
>  				report("I/O bitmap - I/O direction, in", 1);
> -			set_stage(stage + 1);
> +			set_stage(get_stage() + 1);
>  			break;
>  		case 3:
>  			if ((exit_qual & VMX_IO_SIZE_MASK) != _VMX_IO_WORD)
> @@ -748,36 +748,36 @@ static int iobmp_exit_handler()
>  				report("I/O bitmap - I/O direction, out", 1);
>  			else
>  				report("I/O bitmap - I/O direction, out", 0);
> -			set_stage(stage + 1);
> +			set_stage(get_stage() + 1);
>  			break;
>  		case 4:
>  			if ((exit_qual & VMX_IO_SIZE_MASK) != _VMX_IO_LONG)
>  				report("I/O bitmap - I/O width, long", 0);
>  			else
>  				report("I/O bitmap - I/O width, long", 1);
> -			set_stage(stage + 1);
> +			set_stage(get_stage() + 1);
>  			break;
>  		case 5:
>  			if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x5000)
> -				set_stage(stage + 1);
> +				set_stage(get_stage() + 1);
>  			break;
>  		case 6:
>  			if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x9000)
> -				set_stage(stage + 1);
> +				set_stage(get_stage() + 1);
>  			break;
>  		case 7:
>  			if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x4FFF)
> -				set_stage(stage + 1);
> +				set_stage(get_stage() + 1);
>  			break;
>  		case 8:
>  			if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0xFFFF)
> -				set_stage(stage + 1);
> +				set_stage(get_stage() + 1);
>  			break;
>  		case 9:
>  		case 10:
>  			ctrl_cpu0 = vmcs_read(CPU_EXEC_CTRL0);
>  			vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu0 & ~CPU_IO);
> -			set_stage(stage + 1);
> +			set_stage(get_stage() + 1);
>  			break;
>  		default:
>  			// Should not reach here
> @@ -948,13 +948,13 @@ static void insn_intercept_main()
>  		case INSN_CPU0:
>  		case INSN_CPU1:
>  		case INSN_ALWAYS_TRAP:
> -			if (stage != cur_insn + 1)
> +			if (get_stage() != cur_insn + 1)
>  				report(insn_table[cur_insn].name, 0);
>  			else
>  				report(insn_table[cur_insn].name, 1);
>  			break;
>  		case INSN_NEVER_TRAP:
> -			if (stage == cur_insn + 1)
> +			if (get_stage() == cur_insn + 1)
>  				report(insn_table[cur_insn].name, 0);
>  			else
>  				report(insn_table[cur_insn].name, 1);
> @@ -985,7 +985,7 @@ static int insn_intercept_exit_handler()
>  	if (insn_table[cur_insn].test_field & FIELD_INSN_INFO)
>  		pass = pass && insn_table[cur_insn].insn_info == insn_info;
>  	if (pass)
> -		set_stage(stage + 1);
> +		set_stage(get_stage() + 1);
>  	vmcs_write(GUEST_RIP, guest_rip + insn_len);
>  	return VMX_TEST_RESUME;
>  }

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

end of thread, other threads:[~2014-06-16 17:19 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-15 14:24 [PATCH 0/5] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
2014-06-15 14:24 ` [PATCH 1/5] VMX: Add tests for CR3 and CR8 interception Jan Kiszka
2014-06-16 10:53   ` Paolo Bonzini
2014-06-16 11:31     ` Jan Kiszka
2014-06-15 14:24 ` [PATCH 2/5] VMX: Only use get_stage accessor Jan Kiszka
2014-06-16 17:19   ` Bandan Das
2014-06-15 14:24 ` [PATCH 3/5] VMX: Test both interception and execution of instructions Jan Kiszka
2014-06-15 14:24 ` [PATCH 4/5] VMX: Validate capability MSRs Jan Kiszka
2014-06-16 11:00   ` Paolo Bonzini
2014-06-16 11:26     ` Jan Kiszka
2014-06-15 14:24 ` [PATCH 5/5] VMX: Test behavior on set and cleared save/load debug controls Jan Kiszka
2014-06-16 11:02   ` Paolo Bonzini
2014-06-16 11:28     ` Jan Kiszka
2014-06-16 11:03 ` [PATCH 0/5] kvm-unit-tests: more instr. interceptions, debug control migration 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.