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

Changes in v2 according to review remarks:
- refactored get/set_stage interface
- unified vmx_ctrl_* unions
- used vmx_ctrl_msr in capability test
- changed commented-out debugctl tests

Jan Kiszka (6):
  VMX: Add tests for CR3 and CR8 interception
  VMX: Rework test stage interface
  VMX: Test both interception and execution of instructions
  VMX: Unify vmx_ctrl_* unions to vmx_ctrl_msr
  VMX: Validate capability MSRs
  VMX: Test behavior on set and cleared save/load debug controls

 x86/vmx.c       | 108 ++++++++++++-
 x86/vmx.h       |  44 ++---
 x86/vmx_tests.c | 495 ++++++++++++++++++++++++++++++++++++--------------------
 3 files changed, 443 insertions(+), 204 deletions(-)

-- 
1.8.1.1.298.ge7eed54


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

* [PATCH v2 1/6] VMX: Add tests for CR3 and CR8 interception
  2014-06-17  7:04 [PATCH v2 0/6] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
@ 2014-06-17  7:04 ` Jan Kiszka
  2014-06-17  7:41   ` Paolo Bonzini
  2014-06-17  7:04 ` [PATCH v2 2/6] VMX: Rework test stage interface Jan Kiszka
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Jan Kiszka @ 2014-06-17  7:04 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

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..149a591 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		(1 << 1)
+#define FIELD_INSN_INFO		(1 << 2)
 
 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] 12+ messages in thread

* [PATCH v2 2/6] VMX: Rework test stage interface
  2014-06-17  7:04 [PATCH v2 0/6] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
  2014-06-17  7:04 ` [PATCH v2 1/6] VMX: Add tests for CR3 and CR8 interception Jan Kiszka
@ 2014-06-17  7:04 ` Jan Kiszka
  2014-06-17  7:04 ` [PATCH v2 3/6] VMX: Test both interception and execution of instructions Jan Kiszka
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Jan Kiszka @ 2014-06-17  7:04 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

Consistently access the stage only via the helper functions. To enforce
this, move them from vmx_tests.c to vmx.c. At this chance, introduce a
stage incrementation helper.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 x86/vmx.c       |  26 ++++++
 x86/vmx.h       |   4 +
 x86/vmx_tests.c | 250 +++++++++++++++++++++++++++-----------------------------
 3 files changed, 151 insertions(+), 129 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 1182eef..ba6a02b 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -62,6 +62,32 @@ extern void *vmx_return;
 extern void *entry_sysenter;
 extern void *guest_entry;
 
+static volatile u32 stage;
+
+void vmx_set_test_stage(u32 s)
+{
+	barrier();
+	stage = s;
+	barrier();
+}
+
+u32 vmx_get_test_stage(void)
+{
+	u32 s;
+
+	barrier();
+	s = stage;
+	barrier();
+	return s;
+}
+
+void vmx_inc_test_stage(void)
+{
+	barrier();
+	stage++;
+	barrier();
+}
+
 static int make_vmcs_current(struct vmcs *vmcs)
 {
 	bool ret;
diff --git a/x86/vmx.h b/x86/vmx.h
index 69a5385..1a8ae4c 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -514,6 +514,10 @@ extern union vmx_ctrl_exit ctrl_exit_rev;
 extern union vmx_ctrl_ent ctrl_enter_rev;
 extern union vmx_ept_vpid  ept_vpid;
 
+void vmx_set_test_stage(u32 s);
+u32 vmx_get_test_stage(void);
+void vmx_inc_test_stage(void);
+
 static inline int vmcs_clear(struct vmcs *vmcs)
 {
 	bool ret;
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 149a591..5485e4c 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -14,7 +14,6 @@
 
 u64 ia32_pat;
 u64 ia32_efer;
-volatile u32 stage;
 void *io_bitmap_a, *io_bitmap_b;
 u16 ioport;
 
@@ -27,23 +26,6 @@ static inline void vmcall()
 	asm volatile("vmcall");
 }
 
-static inline void set_stage(u32 s)
-{
-	barrier();
-	stage = s;
-	barrier();
-}
-
-static inline u32 get_stage()
-{
-	u32 s;
-
-	barrier();
-	s = stage;
-	barrier();
-	return s;
-}
-
 void basic_guest_main()
 {
 }
@@ -122,23 +104,23 @@ void preemption_timer_main()
 {
 	tsc_val = rdtsc();
 	if (ctrl_exit_rev.clr & EXI_SAVE_PREEMPT) {
-		set_stage(0);
+		vmx_set_test_stage(0);
 		vmcall();
-		if (get_stage() == 1)
+		if (vmx_get_test_stage() == 1)
 			vmcall();
 	}
-	set_stage(1);
-	while (get_stage() == 1) {
+	vmx_set_test_stage(1);
+	while (vmx_get_test_stage() == 1) {
 		if (((rdtsc() - tsc_val) >> preempt_scale)
 				> 10 * preempt_val) {
-			set_stage(2);
+			vmx_set_test_stage(2);
 			vmcall();
 		}
 	}
 	tsc_val = rdtsc();
 	asm volatile ("hlt");
 	vmcall();
-	set_stage(5);
+	vmx_set_test_stage(5);
 	vmcall();
 }
 
@@ -155,13 +137,13 @@ int preemption_timer_exit_handler()
 	insn_len = vmcs_read(EXI_INST_LEN);
 	switch (reason) {
 	case VMX_PREEMPT:
-		switch (get_stage()) {
+		switch (vmx_get_test_stage()) {
 		case 1:
 		case 2:
 			report("busy-wait for preemption timer",
 			       ((rdtsc() - tsc_val) >> preempt_scale) >=
 			       preempt_val);
-			set_stage(3);
+			vmx_set_test_stage(3);
 			vmcs_write(PREEMPT_TIMER_VALUE, preempt_val);
 			return VMX_TEST_RESUME;
 		case 3:
@@ -170,7 +152,7 @@ int preemption_timer_exit_handler()
 			report("preemption timer during hlt",
 			       ((rdtsc() - tsc_val) >> preempt_scale) >=
 			       preempt_val && guest_halted);
-			set_stage(4);
+			vmx_set_test_stage(4);
 			vmcs_write(PIN_CONTROLS,
 				   vmcs_read(PIN_CONTROLS) & ~PIN_PREEMPT);
 			vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE);
@@ -187,11 +169,11 @@ int preemption_timer_exit_handler()
 		break;
 	case VMX_VMCALL:
 		vmcs_write(GUEST_RIP, guest_rip + insn_len);
-		switch (get_stage()) {
+		switch (vmx_get_test_stage()) {
 		case 0:
 			report("Keep preemption value",
 			       vmcs_read(PREEMPT_TIMER_VALUE) == preempt_val);
-			set_stage(1);
+			vmx_set_test_stage(1);
 			vmcs_write(PREEMPT_TIMER_VALUE, preempt_val);
 			ctrl_exit = (vmcs_read(EXI_CONTROLS) |
 				EXI_SAVE_PREEMPT) & ctrl_exit_rev.clr;
@@ -203,12 +185,12 @@ int preemption_timer_exit_handler()
 			return VMX_TEST_RESUME;
 		case 2:
 			report("busy-wait for preemption timer", 0);
-			set_stage(3);
+			vmx_set_test_stage(3);
 			vmcs_write(PREEMPT_TIMER_VALUE, preempt_val);
 			return VMX_TEST_RESUME;
 		case 3:
 			report("preemption timer during hlt", 0);
-			set_stage(4);
+			vmx_set_test_stage(4);
 			/* fall through */
 		case 4:
 			vmcs_write(PIN_CONTROLS,
@@ -221,7 +203,8 @@ int preemption_timer_exit_handler()
 			break;
 		default:
 			// Should not reach here
-			printf("ERROR : unexpected stage, %d\n", get_stage());
+			printf("ERROR : unexpected stage, %d\n",
+			       vmx_get_test_stage());
 			print_vmexit_info();
 			return VMX_TEST_VMEXIT;
 		}
@@ -413,102 +396,102 @@ static void cr_shadowing_main()
 	u32 cr0, cr4, tmp;
 
 	// Test read through
-	set_stage(0);
+	vmx_set_test_stage(0);
 	guest_cr0 = read_cr0();
-	if (stage == 1)
+	if (vmx_get_test_stage() == 1)
 		report("Read through CR0", 0);
 	else
 		vmcall();
-	set_stage(1);
+	vmx_set_test_stage(1);
 	guest_cr4 = read_cr4();
-	if (stage == 2)
+	if (vmx_get_test_stage() == 2)
 		report("Read through CR4", 0);
 	else
 		vmcall();
 	// Test write through
 	guest_cr0 = guest_cr0 ^ (X86_CR0_TS | X86_CR0_MP);
 	guest_cr4 = guest_cr4 ^ (X86_CR4_TSD | X86_CR4_DE);
-	set_stage(2);
+	vmx_set_test_stage(2);
 	write_cr0(guest_cr0);
-	if (stage == 3)
+	if (vmx_get_test_stage() == 3)
 		report("Write throuth CR0", 0);
 	else
 		vmcall();
-	set_stage(3);
+	vmx_set_test_stage(3);
 	write_cr4(guest_cr4);
-	if (stage == 4)
+	if (vmx_get_test_stage() == 4)
 		report("Write through CR4", 0);
 	else
 		vmcall();
 	// Test read shadow
-	set_stage(4);
+	vmx_set_test_stage(4);
 	vmcall();
 	cr0 = read_cr0();
-	if (stage != 5) {
+	if (vmx_get_test_stage() != 5) {
 		if (cr0 == guest_cr0)
 			report("Read shadowing CR0", 1);
 		else
 			report("Read shadowing CR0", 0);
 	}
-	set_stage(5);
+	vmx_set_test_stage(5);
 	cr4 = read_cr4();
-	if (stage != 6) {
+	if (vmx_get_test_stage() != 6) {
 		if (cr4 == guest_cr4)
 			report("Read shadowing CR4", 1);
 		else
 			report("Read shadowing CR4", 0);
 	}
 	// Test write shadow (same value with shadow)
-	set_stage(6);
+	vmx_set_test_stage(6);
 	write_cr0(guest_cr0);
-	if (stage == 7)
+	if (vmx_get_test_stage() == 7)
 		report("Write shadowing CR0 (same value with shadow)", 0);
 	else
 		vmcall();
-	set_stage(7);
+	vmx_set_test_stage(7);
 	write_cr4(guest_cr4);
-	if (stage == 8)
+	if (vmx_get_test_stage() == 8)
 		report("Write shadowing CR4 (same value with shadow)", 0);
 	else
 		vmcall();
 	// Test write shadow (different value)
-	set_stage(8);
+	vmx_set_test_stage(8);
 	tmp = guest_cr0 ^ X86_CR0_TS;
 	asm volatile("mov %0, %%rsi\n\t"
 		"mov %%rsi, %%cr0\n\t"
 		::"m"(tmp)
 		:"rsi", "memory", "cc");
-	if (stage != 9)
+	if (vmx_get_test_stage() != 9)
 		report("Write shadowing different X86_CR0_TS", 0);
 	else
 		report("Write shadowing different X86_CR0_TS", 1);
-	set_stage(9);
+	vmx_set_test_stage(9);
 	tmp = guest_cr0 ^ X86_CR0_MP;
 	asm volatile("mov %0, %%rsi\n\t"
 		"mov %%rsi, %%cr0\n\t"
 		::"m"(tmp)
 		:"rsi", "memory", "cc");
-	if (stage != 10)
+	if (vmx_get_test_stage() != 10)
 		report("Write shadowing different X86_CR0_MP", 0);
 	else
 		report("Write shadowing different X86_CR0_MP", 1);
-	set_stage(10);
+	vmx_set_test_stage(10);
 	tmp = guest_cr4 ^ X86_CR4_TSD;
 	asm volatile("mov %0, %%rsi\n\t"
 		"mov %%rsi, %%cr4\n\t"
 		::"m"(tmp)
 		:"rsi", "memory", "cc");
-	if (stage != 11)
+	if (vmx_get_test_stage() != 11)
 		report("Write shadowing different X86_CR4_TSD", 0);
 	else
 		report("Write shadowing different X86_CR4_TSD", 1);
-	set_stage(11);
+	vmx_set_test_stage(11);
 	tmp = guest_cr4 ^ X86_CR4_DE;
 	asm volatile("mov %0, %%rsi\n\t"
 		"mov %%rsi, %%cr4\n\t"
 		::"m"(tmp)
 		:"rsi", "memory", "cc");
-	if (stage != 12)
+	if (vmx_get_test_stage() != 12)
 		report("Write shadowing different X86_CR4_DE", 0);
 	else
 		report("Write shadowing different X86_CR4_DE", 1);
@@ -527,7 +510,7 @@ static int cr_shadowing_exit_handler()
 	exit_qual = vmcs_read(EXI_QUALIFICATION);
 	switch (reason) {
 	case VMX_VMCALL:
-		switch (get_stage()) {
+		switch (vmx_get_test_stage()) {
 		case 0:
 			if (guest_cr0 == vmcs_read(GUEST_CR0))
 				report("Read through CR0", 1);
@@ -574,45 +557,47 @@ static int cr_shadowing_exit_handler()
 			break;
 		default:
 			// Should not reach here
-			printf("ERROR : unexpected stage, %d\n", get_stage());
+			printf("ERROR : unexpected stage, %d\n",
+			       vmx_get_test_stage());
 			print_vmexit_info();
 			return VMX_TEST_VMEXIT;
 		}
 		vmcs_write(GUEST_RIP, guest_rip + insn_len);
 		return VMX_TEST_RESUME;
 	case VMX_CR:
-		switch (get_stage()) {
+		switch (vmx_get_test_stage()) {
 		case 4:
 			report("Read shadowing CR0", 0);
-			set_stage(stage + 1);
+			vmx_inc_test_stage();
 			break;
 		case 5:
 			report("Read shadowing CR4", 0);
-			set_stage(stage + 1);
+			vmx_inc_test_stage();
 			break;
 		case 6:
 			report("Write shadowing CR0 (same value)", 0);
-			set_stage(stage + 1);
+			vmx_inc_test_stage();
 			break;
 		case 7:
 			report("Write shadowing CR4 (same value)", 0);
-			set_stage(stage + 1);
+			vmx_inc_test_stage();
 			break;
 		case 8:
 		case 9:
 			// 0x600 encodes "mov %esi, %cr0"
 			if (exit_qual == 0x600)
-				set_stage(stage + 1);
+				vmx_inc_test_stage();
 			break;
 		case 10:
 		case 11:
 			// 0x604 encodes "mov %esi, %cr4"
 			if (exit_qual == 0x604)
-				set_stage(stage + 1);
+				vmx_inc_test_stage();
 			break;
 		default:
 			// Should not reach here
-			printf("ERROR : unexpected stage, %d\n", get_stage());
+			printf("ERROR : unexpected stage, %d\n",
+			       vmx_get_test_stage());
 			print_vmexit_info();
 			return VMX_TEST_VMEXIT;
 		}
@@ -645,70 +630,72 @@ static int iobmp_init()
 static void iobmp_main()
 {
 	// stage 0, test IO pass
-	set_stage(0);
+	vmx_set_test_stage(0);
 	inb(0x5000);
 	outb(0x0, 0x5000);
-	if (stage != 0)
+	if (vmx_get_test_stage() != 0)
 		report("I/O bitmap - I/O pass", 0);
 	else
 		report("I/O bitmap - I/O pass", 1);
 	// test IO width, in/out
 	((u8 *)io_bitmap_a)[0] = 0xFF;
-	set_stage(2);
+	vmx_set_test_stage(2);
 	inb(0x0);
-	if (stage != 3)
+	if (vmx_get_test_stage() != 3)
 		report("I/O bitmap - trap in", 0);
 	else
 		report("I/O bitmap - trap in", 1);
-	set_stage(3);
+	vmx_set_test_stage(3);
 	outw(0x0, 0x0);
-	if (stage != 4)
+	if (vmx_get_test_stage() != 4)
 		report("I/O bitmap - trap out", 0);
 	else
 		report("I/O bitmap - trap out", 1);
-	set_stage(4);
+	vmx_set_test_stage(4);
 	inl(0x0);
-	if (stage != 5)
+	if (vmx_get_test_stage() != 5)
 		report("I/O bitmap - I/O width, long", 0);
 	// test low/high IO port
-	set_stage(5);
+	vmx_set_test_stage(5);
 	((u8 *)io_bitmap_a)[0x5000 / 8] = (1 << (0x5000 % 8));
 	inb(0x5000);
-	if (stage == 6)
+	if (vmx_get_test_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);
+	vmx_set_test_stage(6);
 	((u8 *)io_bitmap_b)[0x1000 / 8] = (1 << (0x1000 % 8));
 	inb(0x9000);
-	if (stage == 7)
+	if (vmx_get_test_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);
+	vmx_set_test_stage(7);
 	inl(0x4FFF);
-	if (stage == 8)
+	if (vmx_get_test_stage() == 8)
 		report("I/O bitmap - partial pass", 1);
 	else
 		report("I/O bitmap - partial pass", 0);
 	// test overrun
-	set_stage(8);
+	vmx_set_test_stage(8);
 	memset(io_bitmap_a, 0x0, PAGE_SIZE);
 	memset(io_bitmap_b, 0x0, PAGE_SIZE);
 	inl(0xFFFF);
-	if (stage == 9)
+	if (vmx_get_test_stage() == 9)
 		report("I/O bitmap - overrun", 1);
 	else
 		report("I/O bitmap - overrun", 0);
-	set_stage(9);
+	vmx_set_test_stage(9);
 	vmcall();
 	outb(0x0, 0x0);
-	report("I/O bitmap - ignore unconditional exiting", stage == 9);
-	set_stage(10);
+	report("I/O bitmap - ignore unconditional exiting",
+	       vmx_get_test_stage() == 9);
+	vmx_set_test_stage(10);
 	vmcall();
 	outb(0x0, 0x0);
-	report("I/O bitmap - unconditional exiting", stage == 11);
+	report("I/O bitmap - unconditional exiting",
+	       vmx_get_test_stage() == 11);
 }
 
 static int iobmp_exit_handler()
@@ -723,10 +710,10 @@ static int iobmp_exit_handler()
 	insn_len = vmcs_read(EXI_INST_LEN);
 	switch (reason) {
 	case VMX_IO:
-		switch (get_stage()) {
+		switch (vmx_get_test_stage()) {
 		case 0:
 		case 1:
-			set_stage(stage + 1);
+			vmx_inc_test_stage();
 			break;
 		case 2:
 			if ((exit_qual & VMX_IO_SIZE_MASK) != _VMX_IO_BYTE)
@@ -737,7 +724,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);
+			vmx_inc_test_stage();
 			break;
 		case 3:
 			if ((exit_qual & VMX_IO_SIZE_MASK) != _VMX_IO_WORD)
@@ -748,47 +735,48 @@ 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);
+			vmx_inc_test_stage();
 			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);
+			vmx_inc_test_stage();
 			break;
 		case 5:
 			if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x5000)
-				set_stage(stage + 1);
+				vmx_inc_test_stage();
 			break;
 		case 6:
 			if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x9000)
-				set_stage(stage + 1);
+				vmx_inc_test_stage();
 			break;
 		case 7:
 			if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0x4FFF)
-				set_stage(stage + 1);
+				vmx_inc_test_stage();
 			break;
 		case 8:
 			if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0xFFFF)
-				set_stage(stage + 1);
+				vmx_inc_test_stage();
 			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);
+			vmx_inc_test_stage();
 			break;
 		default:
 			// Should not reach here
-			printf("ERROR : unexpected stage, %d\n", get_stage());
+			printf("ERROR : unexpected stage, %d\n",
+			       vmx_get_test_stage());
 			print_vmexit_info();
 			return VMX_TEST_VMEXIT;
 		}
 		vmcs_write(GUEST_RIP, guest_rip + insn_len);
 		return VMX_TEST_RESUME;
 	case VMX_VMCALL:
-		switch (get_stage()) {
+		switch (vmx_get_test_stage()) {
 		case 9:
 			ctrl_cpu0 = vmcs_read(CPU_EXEC_CTRL0);
 			ctrl_cpu0 |= CPU_IO | CPU_IO_BITMAP;
@@ -801,7 +789,8 @@ static int iobmp_exit_handler()
 			break;
 		default:
 			// Should not reach here
-			printf("ERROR : unexpected stage, %d\n", get_stage());
+			printf("ERROR : unexpected stage, %d\n",
+			       vmx_get_test_stage());
 			print_vmexit_info();
 			return VMX_TEST_VMEXIT;
 		}
@@ -934,7 +923,7 @@ static void insn_intercept_main()
 {
 	cur_insn = 0;
 	while(insn_table[cur_insn].name != NULL) {
-		set_stage(cur_insn);
+		vmx_set_test_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
@@ -948,13 +937,13 @@ static void insn_intercept_main()
 		case INSN_CPU0:
 		case INSN_CPU1:
 		case INSN_ALWAYS_TRAP:
-			if (stage != cur_insn + 1)
+			if (vmx_get_test_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 (vmx_get_test_stage() == cur_insn + 1)
 				report(insn_table[cur_insn].name, 0);
 			else
 				report(insn_table[cur_insn].name, 1);
@@ -978,14 +967,14 @@ 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()) &&
+	pass = (cur_insn == vmx_get_test_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(stage + 1);
+		vmx_inc_test_stage();
 	vmcs_write(GUEST_RIP, guest_rip + insn_len);
 	return VMX_TEST_RESUME;
 }
@@ -1064,14 +1053,14 @@ static int ept_init()
 
 static void ept_main()
 {
-	set_stage(0);
+	vmx_set_test_stage(0);
 	if (*((u32 *)data_page2) != MAGIC_VAL_1 ||
 			*((u32 *)data_page1) != MAGIC_VAL_1)
 		report("EPT basic framework - read", 0);
 	else {
 		*((u32 *)data_page2) = MAGIC_VAL_3;
 		vmcall();
-		if (get_stage() == 1) {
+		if (vmx_get_test_stage() == 1) {
 			if (*((u32 *)data_page1) == MAGIC_VAL_3 &&
 					*((u32 *)data_page2) == MAGIC_VAL_2)
 				report("EPT basic framework", 1);
@@ -1080,35 +1069,35 @@ static void ept_main()
 		}
 	}
 	// Test EPT Misconfigurations
-	set_stage(1);
+	vmx_set_test_stage(1);
 	vmcall();
 	*((u32 *)data_page1) = MAGIC_VAL_1;
-	if (get_stage() != 2) {
+	if (vmx_get_test_stage() != 2) {
 		report("EPT misconfigurations", 0);
 		goto t1;
 	}
-	set_stage(2);
+	vmx_set_test_stage(2);
 	vmcall();
 	*((u32 *)data_page1) = MAGIC_VAL_1;
-	if (get_stage() != 3) {
+	if (vmx_get_test_stage() != 3) {
 		report("EPT misconfigurations", 0);
 		goto t1;
 	}
 	report("EPT misconfigurations", 1);
 t1:
 	// Test EPT violation
-	set_stage(3);
+	vmx_set_test_stage(3);
 	vmcall();
 	*((u32 *)data_page1) = MAGIC_VAL_1;
-	if (get_stage() == 4)
+	if (vmx_get_test_stage() == 4)
 		report("EPT violation - page permission", 1);
 	else
 		report("EPT violation - page permission", 0);
 	// Violation caused by EPT paging structure
-	set_stage(4);
+	vmx_set_test_stage(4);
 	vmcall();
 	*((u32 *)data_page1) = MAGIC_VAL_2;
-	if (get_stage() == 5)
+	if (vmx_get_test_stage() == 5)
 		report("EPT violation - paging structure", 1);
 	else
 		report("EPT violation - paging structure", 0);
@@ -1128,11 +1117,11 @@ static int ept_exit_handler()
 	exit_qual = vmcs_read(EXI_QUALIFICATION);
 	switch (reason) {
 	case VMX_VMCALL:
-		switch (get_stage()) {
+		switch (vmx_get_test_stage()) {
 		case 0:
 			if (*((u32 *)data_page1) == MAGIC_VAL_3 &&
 					*((u32 *)data_page2) == MAGIC_VAL_2) {
-				set_stage(get_stage() + 1);
+				vmx_inc_test_stage();
 				install_ept(pml4, (unsigned long)data_page2,
 						(unsigned long)data_page2,
 						EPT_RA | EPT_WA | EPT_EA);
@@ -1169,17 +1158,18 @@ static int ept_exit_handler()
 			break;
 		// Should not reach here
 		default:
-			printf("ERROR - unexpected stage, %d.\n", get_stage());
+			printf("ERROR - unexpected stage, %d.\n",
+			       vmx_get_test_stage());
 			print_vmexit_info();
 			return VMX_TEST_VMEXIT;
 		}
 		vmcs_write(GUEST_RIP, guest_rip + insn_len);
 		return VMX_TEST_RESUME;
 	case VMX_EPT_MISCONFIG:
-		switch (get_stage()) {
+		switch (vmx_get_test_stage()) {
 		case 1:
 		case 2:
-			set_stage(get_stage() + 1);
+			vmx_inc_test_stage();
 			install_ept(pml4, (unsigned long)data_page1,
  				(unsigned long)data_page1,
  				EPT_RA | EPT_WA | EPT_EA);
@@ -1187,31 +1177,33 @@ static int ept_exit_handler()
 			break;
 		// Should not reach here
 		default:
-			printf("ERROR - unexpected stage, %d.\n", get_stage());
+			printf("ERROR - unexpected stage, %d.\n",
+			       vmx_get_test_stage());
 			print_vmexit_info();
 			return VMX_TEST_VMEXIT;
 		}
 		return VMX_TEST_RESUME;
 	case VMX_EPT_VIOLATION:
-		switch(get_stage()) {
+		switch(vmx_get_test_stage()) {
 		case 3:
 			if (exit_qual == (EPT_VLT_WR | EPT_VLT_LADDR_VLD |
 					EPT_VLT_PADDR))
-				set_stage(get_stage() + 1);
+				vmx_inc_test_stage();
 			set_ept_pte(pml4, (unsigned long)data_page1, 
 				1, data_page1_pte | (EPT_PRESENT));
 			ept_sync(INVEPT_SINGLE, eptp);
 			break;
 		case 4:
 			if (exit_qual == (EPT_VLT_RD | EPT_VLT_LADDR_VLD))
-				set_stage(get_stage() + 1);
+				vmx_inc_test_stage();
 			set_ept_pte(pml4, data_page1_pte, 2,
 				data_page1_pte_pte | (EPT_PRESENT));
 			ept_sync(INVEPT_SINGLE, eptp);
 			break;
 		default:
 			// Should not reach here
-			printf("ERROR : unexpected stage, %d\n", get_stage());
+			printf("ERROR : unexpected stage, %d\n",
+			       vmx_get_test_stage());
 			print_vmexit_info();
 			return VMX_TEST_VMEXIT;
 		}
@@ -1245,7 +1237,7 @@ static void interrupt_main(void)
 {
 	long long start, loops;
 
-	set_stage(0);
+	vmx_set_test_stage(0);
 
 	apic_write(APIC_LVTT, TIMER_VECTOR);
 	irq_enable();
@@ -1319,7 +1311,7 @@ static void interrupt_main(void)
 
 	apic_write(APIC_TMICT, 0);
 	irq_disable();
-	set_stage(7);
+	vmx_set_test_stage(7);
 	vmcall();
 	timer_fired = false;
 	apic_write(APIC_TMICT, 1);
@@ -1336,7 +1328,7 @@ static int interrupt_exit_handler(void)
 
 	switch (reason) {
 	case VMX_VMCALL:
-		switch (get_stage()) {
+		switch (vmx_get_test_stage()) {
 		case 0:
 		case 2:
 		case 5:
@@ -1358,7 +1350,7 @@ static int interrupt_exit_handler(void)
 			vmcs_write(GUEST_ACTV_STATE, ACTV_HLT);
 			break;
 		}
-		set_stage(get_stage() + 1);
+		vmx_inc_test_stage();
 		vmcs_write(GUEST_RIP, guest_rip + insn_len);
 		return VMX_TEST_RESUME;
 	case VMX_EXTINT:
@@ -1370,7 +1362,7 @@ static int interrupt_exit_handler(void)
 			asm volatile ("nop");
 			irq_disable();
 		}
-		if (get_stage() >= 2) {
+		if (vmx_get_test_stage() >= 2) {
 			vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE);
 			vmcs_write(GUEST_RIP, guest_rip + insn_len);
 		}
-- 
1.8.1.1.298.ge7eed54


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

* [PATCH v2 3/6] VMX: Test both interception and execution of instructions
  2014-06-17  7:04 [PATCH v2 0/6] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
  2014-06-17  7:04 ` [PATCH v2 1/6] VMX: Add tests for CR3 and CR8 interception Jan Kiszka
  2014-06-17  7:04 ` [PATCH v2 2/6] VMX: Rework test stage interface Jan Kiszka
@ 2014-06-17  7:04 ` Jan Kiszka
  2014-06-17  7:04 ` [PATCH v2 4/6] VMX: Unify vmx_ctrl_* unions to vmx_ctrl_msr Jan Kiszka
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Jan Kiszka @ 2014-06-17  7:04 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

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 5485e4c..f02de3d 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -807,7 +807,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		(1 << 1)
 #define FIELD_INSN_INFO		(1 << 2)
@@ -818,7 +817,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"
@@ -848,6 +847,7 @@ extern void insn_cpuid();
 extern void insn_invd();
 
 u32 cur_insn;
+u64 cr3;
 
 struct insn_table {
 	const char *name;
@@ -901,55 +901,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) {
-		vmx_set_test_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++) {
+		vmx_set_test_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, vmx_get_test_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 (vmx_get_test_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 (vmx_get_test_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, vmx_get_test_stage() == cur_insn * 2 + 1);
+
+		vmx_set_test_stage(cur_insn * 2 + 1);
+		vmcall();
 	}
 }
 
@@ -967,14 +968,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 == vmx_get_test_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 (vmx_get_test_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 == vmx_get_test_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)
-		vmx_inc_test_stage();
+		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)
+			vmx_inc_test_stage();
+	}
 	vmcs_write(GUEST_RIP, guest_rip + insn_len);
 	return VMX_TEST_RESUME;
 }
-- 
1.8.1.1.298.ge7eed54


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

* [PATCH v2 4/6] VMX: Unify vmx_ctrl_* unions to vmx_ctrl_msr
  2014-06-17  7:04 [PATCH v2 0/6] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
                   ` (2 preceding siblings ...)
  2014-06-17  7:04 ` [PATCH v2 3/6] VMX: Test both interception and execution of instructions Jan Kiszka
@ 2014-06-17  7:04 ` Jan Kiszka
  2014-06-17  7:04 ` [PATCH v2 5/6] VMX: Validate capability MSRs Jan Kiszka
  2014-06-17  7:04 ` [PATCH v2 6/6] VMX: Test behavior on set and cleared save/load debug controls Jan Kiszka
  5 siblings, 0 replies; 12+ messages in thread
From: Jan Kiszka @ 2014-06-17  7:04 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

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

diff --git a/x86/vmx.c b/x86/vmx.c
index ba6a02b..f01e443 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -49,10 +49,10 @@ bool launched;
 u64 host_rflags;
 
 union vmx_basic basic;
-union vmx_ctrl_pin ctrl_pin_rev;
-union vmx_ctrl_cpu ctrl_cpu_rev[2];
-union vmx_ctrl_exit ctrl_exit_rev;
-union vmx_ctrl_ent ctrl_enter_rev;
+union vmx_ctrl_msr ctrl_pin_rev;
+union vmx_ctrl_msr ctrl_cpu_rev[2];
+union vmx_ctrl_msr ctrl_exit_rev;
+union vmx_ctrl_msr ctrl_enter_rev;
 union vmx_ept_vpid  ept_vpid;
 
 extern struct descriptor_table_ptr gdt64_desc;
diff --git a/x86/vmx.h b/x86/vmx.h
index 1a8ae4c..00f2842 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -55,28 +55,7 @@ union vmx_basic {
 	};
 };
 
-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 {
+union vmx_ctrl_msr {
 	u64 val;
 	struct {
 		u32 set, clr;
@@ -508,10 +487,10 @@ enum Ctrl1 {
 extern struct regs regs;
 
 extern union vmx_basic basic;
-extern union vmx_ctrl_pin ctrl_pin_rev;
-extern union vmx_ctrl_cpu ctrl_cpu_rev[2];
-extern union vmx_ctrl_exit ctrl_exit_rev;
-extern union vmx_ctrl_ent ctrl_enter_rev;
+extern union vmx_ctrl_msr ctrl_pin_rev;
+extern union vmx_ctrl_msr ctrl_cpu_rev[2];
+extern union vmx_ctrl_msr ctrl_exit_rev;
+extern union vmx_ctrl_msr ctrl_enter_rev;
 extern union vmx_ept_vpid  ept_vpid;
 
 void vmx_set_test_stage(u32 s);
-- 
1.8.1.1.298.ge7eed54


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

* [PATCH v2 5/6] VMX: Validate capability MSRs
  2014-06-17  7:04 [PATCH v2 0/6] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
                   ` (3 preceding siblings ...)
  2014-06-17  7:04 ` [PATCH v2 4/6] VMX: Unify vmx_ctrl_* unions to vmx_ctrl_msr Jan Kiszka
@ 2014-06-17  7:04 ` Jan Kiszka
  2014-06-17  8:00   ` Paolo Bonzini
  2014-06-17  7:04 ` [PATCH v2 6/6] VMX: Test behavior on set and cleared save/load debug controls Jan Kiszka
  5 siblings, 1 reply; 12+ messages in thread
From: Jan Kiszka @ 2014-06-17  7:04 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

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 | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 x86/vmx.h |  5 +++--
 2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index f01e443..5c3498a 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -661,6 +661,77 @@ 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, default1, fixed0, fixed1;
+	union vmx_ctrl_msr ctrl, true_ctrl;
+	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++) {
+		ctrl.val = rdmsr(vmx_ctl_msr[n].index);
+		default1 = vmx_ctl_msr[n].default1;
+		ok = (ctrl.set & default1) == default1 &&
+			((ctrl.set ^ ctrl.clr) & ~ctrl.clr) == 0;
+		if (ok && basic.ctrl) {
+			true_ctrl.val = rdmsr(vmx_ctl_msr[n].true_index);
+			ok = ctrl.clr == true_ctrl.clr &&
+				((ctrl.set ^ true_ctrl.set) & ~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)
 {
@@ -803,7 +874,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__);
@@ -842,6 +913,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 00f2842..87457b1 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] 12+ messages in thread

* [PATCH v2 6/6] VMX: Test behavior on set and cleared save/load debug controls
  2014-06-17  7:04 [PATCH v2 0/6] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
                   ` (4 preceding siblings ...)
  2014-06-17  7:04 ` [PATCH v2 5/6] VMX: Validate capability MSRs Jan Kiszka
@ 2014-06-17  7:04 ` Jan Kiszka
  5 siblings, 0 replies; 12+ messages in thread
From: Jan Kiszka @ 2014-06-17  7:04 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

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 | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 114 insertions(+)

diff --git a/x86/vmx.h b/x86/vmx.h
index 87457b1..022f1df 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -305,6 +305,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,
@@ -316,6 +317,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 f02de3d..a97b6d6 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1398,6 +1398,116 @@ 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);
+	/* Commented out: 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);
+
+	vmx_set_test_stage(0);
+	vmcall();
+	report("Save debug controls", vmx_get_test_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;
+	}
+	vmx_set_test_stage(2);
+	vmcall();
+
+	asm volatile("mov %%dr7,%0" : "=r" (dr7));
+	debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
+	/* Commented out: KVM does not support DEBUGCTL so far */
+	report("Guest=host debug controls", dr7 == 0x402 /* && debugctl == 0x1 */);
+
+	dr7 = 0x408;
+	asm volatile("mov %0,%%dr7" : : "r" (dr7));
+	wrmsr(MSR_IA32_DEBUGCTLMSR, 0x3);
+
+	vmx_set_test_stage(3);
+	vmcall();
+	report("Don't save debug controls", vmx_get_test_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 (vmx_get_test_stage()) {
+		case 0:
+			if (dr7 == 0x400 && debugctl == 0 &&
+			    vmcs_read(GUEST_DR7) == 0x408 /* &&
+			    Commented out: KVM does not support DEBUGCTL so far
+			    vmcs_read(GUEST_DEBUGCTL) == 0x3 */)
+				vmx_inc_test_stage();
+			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 /* &&
+			    Commented out: KVM does not support DEBUGCTL so far
+			    vmcs_read(GUEST_DEBUGCTL) == 0x2 */)
+				vmx_inc_test_stage();
+			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} },
@@ -1417,5 +1527,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] 12+ messages in thread

* Re: [PATCH v2 1/6] VMX: Add tests for CR3 and CR8 interception
  2014-06-17  7:04 ` [PATCH v2 1/6] VMX: Add tests for CR3 and CR8 interception Jan Kiszka
@ 2014-06-17  7:41   ` Paolo Bonzini
  2014-06-17  7:42     ` Jan Kiszka
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2014-06-17  7:41 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: kvm, Bandan Das

Il 17/06/2014 09:04, Jan Kiszka ha scritto:
>
> -#define FIELD_EXIT_QUAL		0
> -#define FIELD_INSN_INFO		1
> +#define FIELD_EXIT_QUAL		(1 << 1)
> +#define FIELD_INSN_INFO		(1 << 2)

Heh, you probably wanted 1<<0 and 1<<1.  I'll fix it up. :)

Paolo

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

* Re: [PATCH v2 1/6] VMX: Add tests for CR3 and CR8 interception
  2014-06-17  7:41   ` Paolo Bonzini
@ 2014-06-17  7:42     ` Jan Kiszka
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Kiszka @ 2014-06-17  7:42 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

On 2014-06-17 09:41, Paolo Bonzini wrote:
> Il 17/06/2014 09:04, Jan Kiszka ha scritto:
>>
>> -#define FIELD_EXIT_QUAL        0
>> -#define FIELD_INSN_INFO        1
>> +#define FIELD_EXIT_QUAL        (1 << 1)
>> +#define FIELD_INSN_INFO        (1 << 2)
> 
> Heh, you probably wanted 1<<0 and 1<<1.  I'll fix it up. :)

Yeah... thanks in advance.

Jan

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

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

* Re: [PATCH v2 5/6] VMX: Validate capability MSRs
  2014-06-17  7:04 ` [PATCH v2 5/6] VMX: Validate capability MSRs Jan Kiszka
@ 2014-06-17  8:00   ` Paolo Bonzini
  2014-06-18  5:32     ` [PATCH v3 " Jan Kiszka
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2014-06-17  8:00 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: kvm, Bandan Das

Il 17/06/2014 09:04, Jan Kiszka ha scritto:
> +		default1 = vmx_ctl_msr[n].default1;
> +		ok = (ctrl.set & default1) == default1 &&
> +			((ctrl.set ^ ctrl.clr) & ~ctrl.clr) == 0;

Thanks, now I can understand what's going on. :)  It can still be 
simplified though.

This is just (ctrl.set & ~ctrl.clr) == 0:

	(a ^ b) & ~b == 0
->	(a & ~b) ^ (b & ~b) == 0
->	a & ~b == 0

This expresses just as well the concept that set=1, clr=0 is an 
impossible combination.

Also, it's a bit easier to read if the second line is written as a 
separate statement:

	ok = ok && (ctrl.set & ~ctrl.clr) == 0;

> +		if (ok && basic.ctrl) {
> +			true_ctrl.val = rdmsr(vmx_ctl_msr[n].true_index);
> +			ok = ctrl.clr == true_ctrl.clr &&
> +				((ctrl.set ^ true_ctrl.set) & ~default1) == 0;

This is

	((ctrl.set ^ true_ctrl.set) & ~default1 == 0
->	((ctrl.set & ~default1) ^ (true_ctrl.set & ~default1) == 0
->	((ctrl.set & ~default1) == (true_ctrl.set & ~default1)

A bit longer, but clearer: the difference between ctrl.set and 
true_ctrl.set is only that true_ctrl.set can clear some default-1 bits.

Or you can simplify it further:

->      (ctrl.set | default1) == (true_ctrl.set | default1)
->      ctrl.set == (true_ctrl.set | default1)

Also clearer: the difference between ctrl.set and true_ctrl.set is only 
that default-1 bits must be 1 in ctrl.set.  Pick the one you prefer.

Again, using a separate statement makes it easier to read in my opinion; 
in fact I would also write both statements as

	ok = ok && ...

even though it's redundant for the clr test.

Can you submit v3 of this patch only?

Paolo

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

* [PATCH v3 5/6] VMX: Validate capability MSRs
  2014-06-17  8:00   ` Paolo Bonzini
@ 2014-06-18  5:32     ` Jan Kiszka
  2014-06-18  9:38       ` Paolo Bonzini
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Kiszka @ 2014-06-18  5:32 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, Bandan Das

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>
---

Changes in v3:
- integrated suggestions of Paolo

 x86/vmx.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 x86/vmx.h |  5 +++--
 2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index f01e443..5bb5969 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -661,6 +661,77 @@ 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, default1, fixed0, fixed1;
+	union vmx_ctrl_msr ctrl, true_ctrl;
+	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++) {
+		ctrl.val = rdmsr(vmx_ctl_msr[n].index);
+		default1 = vmx_ctl_msr[n].default1;
+		ok = (ctrl.set & default1) == default1;
+		ok = ok && (ctrl.set & ~ctrl.clr) == 0;
+		if (ok && basic.ctrl) {
+			true_ctrl.val = rdmsr(vmx_ctl_msr[n].true_index);
+			ok = ctrl.clr == true_ctrl.clr;
+			ok = ok && ctrl.set == (true_ctrl.set | default1);
+		}
+		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)
 {
@@ -803,7 +874,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__);
@@ -842,6 +913,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 00f2842..87457b1 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] 12+ messages in thread

* Re: [PATCH v3 5/6] VMX: Validate capability MSRs
  2014-06-18  5:32     ` [PATCH v3 " Jan Kiszka
@ 2014-06-18  9:38       ` Paolo Bonzini
  0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2014-06-18  9:38 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: kvm, Bandan Das

Il 18/06/2014 07:32, Jan Kiszka ha scritto:
> 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>
> ---
>
> Changes in v3:
> - integrated suggestions of Paolo
>
>  x86/vmx.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  x86/vmx.h |  5 +++--
>  2 files changed, 76 insertions(+), 3 deletions(-)
>
> diff --git a/x86/vmx.c b/x86/vmx.c
> index f01e443..5bb5969 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -661,6 +661,77 @@ 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, default1, fixed0, fixed1;
> +	union vmx_ctrl_msr ctrl, true_ctrl;
> +	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++) {
> +		ctrl.val = rdmsr(vmx_ctl_msr[n].index);
> +		default1 = vmx_ctl_msr[n].default1;
> +		ok = (ctrl.set & default1) == default1;
> +		ok = ok && (ctrl.set & ~ctrl.clr) == 0;
> +		if (ok && basic.ctrl) {
> +			true_ctrl.val = rdmsr(vmx_ctl_msr[n].true_index);
> +			ok = ctrl.clr == true_ctrl.clr;
> +			ok = ok && ctrl.set == (true_ctrl.set | default1);
> +		}
> +		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)
>  {
> @@ -803,7 +874,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__);
> @@ -842,6 +913,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 00f2842..87457b1 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;
>  	};
>  };
>
>

Thanks,

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

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

end of thread, other threads:[~2014-06-18  9:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-17  7:04 [PATCH v2 0/6] kvm-unit-tests: more instr. interceptions, debug control migration Jan Kiszka
2014-06-17  7:04 ` [PATCH v2 1/6] VMX: Add tests for CR3 and CR8 interception Jan Kiszka
2014-06-17  7:41   ` Paolo Bonzini
2014-06-17  7:42     ` Jan Kiszka
2014-06-17  7:04 ` [PATCH v2 2/6] VMX: Rework test stage interface Jan Kiszka
2014-06-17  7:04 ` [PATCH v2 3/6] VMX: Test both interception and execution of instructions Jan Kiszka
2014-06-17  7:04 ` [PATCH v2 4/6] VMX: Unify vmx_ctrl_* unions to vmx_ctrl_msr Jan Kiszka
2014-06-17  7:04 ` [PATCH v2 5/6] VMX: Validate capability MSRs Jan Kiszka
2014-06-17  8:00   ` Paolo Bonzini
2014-06-18  5:32     ` [PATCH v3 " Jan Kiszka
2014-06-18  9:38       ` Paolo Bonzini
2014-06-17  7:04 ` [PATCH v2 6/6] VMX: Test behavior on set and cleared save/load debug controls Jan Kiszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox