Skip to contentSkip to conversation

kvm-unit-tests: Various improvements for x86 tests

PATCH Applied Jan Kiszka 2 participants 18 messages Jan 4, 2014 – Feb 25, 2014

Subsystems (3)
KVM Kbuild Library code
lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

cover kvm-unit-tests: Various improvements for x86 tests

Highlights:
- improved preemption timer and interrupt injection tests (obsoletes my two patches in vmx queue)
- tests for IA32_APIC_BASE writes
- test for unconditional IO exiting (VMX)
- basic test of debug facilities (hw breakpoints etc.)

Jan Kiszka (13):
VMX: Add test cases around interrupt injection and halting
VMX: Extend preemption timer tests
apic: Remove redundant enable_apic
VMX: Fix return label in fault-triggering handlers lib/x86: Move exception test code into library
x2apic: Test for invalid state transitions lib/x86/apic: Consolidate over MSR_IA32_APICBASE
apic: Add test case for relocation and writing reserved bits
VMX: Check unconditional I/O exiting
Provide common report and report_summary services Ignore *.elf build outputs
svm: Add missing build dependency
x86: Add debug facility test case

 .gitignore            |   1 +
 Makefile              |   3 +-
 config-x86-common.mak |   4 +-
 config-x86_64.mak     |   2 +-
 lib/libcflat.h        |   4 +
 lib/report.c          |  36 +++++++
 lib/x86/apic-defs.h   |   3 +
 lib/x86/apic.c        |   7 +-
 lib/x86/desc.c        |  24 +++++
 lib/x86/desc.h        |   6 ++
 x86/apic.c            |  84 +++++++++++++---
 x86/debug.c           | 113 +++++++++++++++++++++
 x86/emulator.c        |  16 +--
 x86/eventinj.c        |  15 +--
 x86/idt_test.c        |  21 +---
 x86/msr.c             |  15 +--
 x86/pcid.c            |  14 +--
 x86/pmu.c             |  37 +++----
 x86/taskswitch2.c     |  15 +--
 x86/unittests.cfg     |   3 +
 x86/vmx.c             |  57 +++--------
 x86/vmx.h             |   4 +-
 x86/vmx_tests.c       | 264 ++++++++++++++++++++++++++++++++++++++++++++++----
 23 files changed, 548 insertions(+), 200 deletions(-)
 create mode 100644 lib/report.c
 create mode 100644 x86/debug.c
Signature · 1 line
1.8.1.1.298.ge7eed54
author
#1 lore raw

PATCH 1/13 VMX: Add test cases around interrupt injection and halting

From: Jan Kiszka <[email protected]>

This checks for interrupt delivery to L2, unintercepted hlt in L2 and explicit L2 suspension via the activity state HLT. All tests are performed both with direct interrupt injection and external interrupt interception.

Signed-off-by Jan Kiszka <[email protected]>
---
 x86/vmx.c       |   3 +-
 x86/vmx.h       |   3 ++
 x86/vmx_tests.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 148 insertions(+), 2 deletions(-)
x86/vmx.c +1 -2
index fe950e6..a475aec 100644
@@ -457,7 +457,7 @@ static void init_vmcs_guest(void)
 	vmcs_write(GUEST_RFLAGS, 0x2);
 
 	/* 26.3.1.5 */
-	vmcs_write(GUEST_ACTV_STATE, 0);
+	vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE);
 	vmcs_write(GUEST_INTR_STATE, 0);
 }
 

@@ -482,7 +482,6 @@ static int init_vmcs(struct vmcs **vmcs)
 	ctrl_pin |= PIN_EXTINT | PIN_NMI | PIN_VIRT_NMI;
 	ctrl_exit = EXI_LOAD_EFER | EXI_HOST_64;
 	ctrl_enter = (ENT_LOAD_EFER | ENT_GUEST_64);
-	ctrl_cpu[0] |= CPU_HLT;
 	/* DIsable IO instruction VMEXIT now */
 	ctrl_cpu[0] &= (~(CPU_IO | CPU_IO_BITMAP));
 	ctrl_cpu[1] = 0;
x86/vmx.h +3 -0
index bc8c86f..3867793 100644
@@ -500,6 +500,9 @@ enum Ctrl1 {
 #define INVEPT_SINGLE		1
 #define INVEPT_GLOBAL		2
 
+#define ACTV_ACTIVE		0
+#define ACTV_HLT		1
+
 extern struct regs regs;
 
 extern union vmx_basic basic;
x86/vmx_tests.c +144 -0

161 lines not on this page · raw message

Signature · 1 line
1.8.1.1.298.ge7eed54
author
#2 lore raw

PATCH 2/13 VMX: Extend preemption timer tests

In reply to Jan Kiszka

From: Jan Kiszka <[email protected]>

This checks that we properly expire the preemption timer while the guest is in HLT state and that we do not progress guest execution of the preemption timer is activated with a timer value of 0.

Signed-off-by Jan Kiszka <[email protected]>
---
 x86/vmx_tests.c | 84 +++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 64 insertions(+), 20 deletions(-)
x86/vmx_tests.c +64 -20
index 70efb50..0077f3f 100644
@@ -99,6 +99,7 @@ int vmenter_exit_handler()
 u32 preempt_scale;
 volatile unsigned long long tsc_val;
 volatile u32 preempt_val;
+u64 saved_rip;
 
 int preemption_timer_init()
 {

@@ -126,17 +127,24 @@ void preemption_timer_main()
 		if (get_stage() == 1)
 			vmcall();
 	}
-	while (1) {
+	set_stage(1);
+	while (get_stage() == 1) {
 		if (((rdtsc() - tsc_val) >> preempt_scale)
 				> 10 * preempt_val) {
 			set_stage(2);
 			vmcall();
 		}
 	}
+	tsc_val = rdtsc();
+	asm volatile ("hlt");
+	vmcall();
+	set_stage(5);
+	vmcall();
 }
 
 int preemption_timer_exit_handler()
 {
+	bool guest_halted;
 	u64 guest_rip;
 	ulong reason;
 	u32 insn_len;

@@ -147,33 +155,69 @@ int preemption_timer_exit_handler()
 	insn_len = vmcs_read(EXI_INST_LEN);
 	switch (reason) {
 	case VMX_PREEMPT:
-		if (((rdtsc() - tsc_val) >> preempt_scale) < preempt_val)
-			report("Preemption timer", 0);
-		else
-			report("Preemption timer", 1);
+		switch (get_stage()) {
+		case 1:
+		case 2:
+			report("busy-wait for preemption timer",
+			       ((rdtsc() - tsc_val) >> preempt_scale) >=
+			       preempt_val);
+			set_stage(3);
+			vmcs_write(PREEMPT_TIMER_VALUE, preempt_val);
+			return VMX_TEST_RESUME;
+		case 3:
+			guest_halted =
+				(vmcs_read(GUEST_ACTV_STATE) == ACTV_HLT);
+			report("preemption timer during hlt",
+			       ((rdtsc() - tsc_val) >> preempt_scale) >=
+			       preempt_val && guest_halted);
+			set_stage(4);
+			vmcs_write(PIN_CONTROLS,
+				   vmcs_read(PIN_CONTROLS) & ~PIN_PREEMPT);
+			vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE);
+			return VMX_TEST_RESUME;
+		case 4:
+			report("preemption timer with 0 value",
+			       saved_rip == guest_rip);
+			break;
+		default:
+			printf("Invalid stage.\n");
+			print_vmexit_info();
+			break;
+		}
 		break;
 	case VMX_VMCALL:
+		vmcs_write(GUEST_RIP, guest_rip + insn_len);
 		switch (get_stage()) {
 		case 0:
-			if (vmcs_read(PREEMPT_TIMER_VALUE) != preempt_val)
-				report("Save preemption value", 0);
-			else {
-				set_stage(get_stage() + 1);
-				ctrl_exit = (vmcs_read(EXI_CONTROLS) |
-					EXI_SAVE_PREEMPT) & ctrl_exit_rev.clr;
-				vmcs_write(EXI_CONTROLS, ctrl_exit);
-			}
-			vmcs_write(GUEST_RIP, guest_rip + insn_len);
+			report("Keep preemption value",
+			       vmcs_read(PREEMPT_TIMER_VALUE) == preempt_val);
+			set_stage(1);
+			vmcs_write(PREEMPT_TIMER_VALUE, preempt_val);
+			ctrl_exit = (vmcs_read(EXI_CONTROLS) |
+				EXI_SAVE_PREEMPT) & ctrl_exit_rev.clr;
+			vmcs_write(EXI_CONTROLS, ctrl_exit);
 			return VMX_TEST_RESUME;
 		case 1:
-			if (vmcs_read(PREEMPT_TIMER_VALUE) >= preempt_val)
-				report("Save preemption value", 0);
-			else
-				report("Save preemption value", 1);
-			vmcs_write(GUEST_RIP, guest_rip + insn_len);
+			report("Save preemption value",
+			       vmcs_read(PREEMPT_TIMER_VALUE) < preempt_val);
 			return VMX_TEST_RESUME;
 		case 2:
-			report("Preemption timer", 0);
+			report("busy-wait for preemption timer", 0);
+			set_stage(3);
+			vmcs_write(PREEMPT_TIMER_VALUE, preempt_val);
+			return VMX_TEST_RESUME;
+		case 3:
+			report("preemption timer during hlt", 0);
+			set_stage(4);
+			/* fall through */
+		case 4:
+			vmcs_write(PIN_CONTROLS,
+				   vmcs_read(PIN_CONTROLS) | PIN_PREEMPT);
+			vmcs_write(PREEMPT_TIMER_VALUE, 0);
+			saved_rip = guest_rip + insn_len;
+			return VMX_TEST_RESUME;
+		case 5:
+			report("preemption timer with 0 value (vmcall stage 5)", 0);
 			break;
 		default:
 			// Should not reach here
Signature · 1 line
1.8.1.1.298.ge7eed54
author
#3 lore raw

PATCH 3/13 apic: Remove redundant enable_apic

In reply to Jan Kiszka

From: Jan Kiszka <[email protected]>

Already called by the bootstrap code.

Signed-off-by Jan Kiszka <[email protected]>
---
 x86/apic.c | 1 -
 1 file changed, 1 deletion(-)
x86/apic.c +0 -1
index 50e77fc..d06153f 100644
@@ -326,7 +326,6 @@ int main()
     test_lapic_existence();
 
     mask_pic_interrupts();
-    enable_apic();
     test_enable_x2apic();
 
     test_self_ipi();
Signature · 1 line
1.8.1.1.298.ge7eed54
author
#4 lore raw

PATCH 4/13 VMX: Fix return label in fault-triggering handlers

In reply to Jan Kiszka

From: Jan Kiszka <[email protected]>

Some compiler versions (seen with gcc 4.8.1) move the resume label after the return statement which, of course, causes sever problems.

Signed-off-by Jan Kiszka <[email protected]>
---
 x86/vmx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
x86/vmx.c +2 -2
index a475aec..f9d5493 100644
@@ -563,7 +563,7 @@ static void do_vmxon_off(void)
 	vmx_on();
 	vmx_off();
 resume:
-	return;
+	barrier();
 }
 
 static void do_write_feature_control(void)

@@ -572,7 +572,7 @@ static void do_write_feature_control(void)
 	barrier();
 	wrmsr(MSR_IA32_FEATURE_CONTROL, 0);
 resume:
-	return;
+	barrier();
 }
 
 static int test_vmx_feature_control(void)
Signature · 1 line
1.8.1.1.298.ge7eed54
maintainer
#5 lore raw

Re: [PATCH 04/13] VMX: Fix return label in fault-triggering handlers

Il 04/01/2014 18:59, Jan Kiszka ha scritto:

Quoted 4 lines

From: Jan Kiszka <[email protected]>

Some compiler versions (seen with gcc 4.8.1) move the resume label after the return statement which, of course, causes sever problems.

Can you include the assembly output? Do you mean after the "ret" instruction?

The return should not be needed except for the rule that a label must be followed by a statement. But I would not except any difference between a ";" and a barrier in this case.

Paolo

Quoted 28 lines
Signed-off-by Jan Kiszka <[email protected]>
---
 x86/vmx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
x86/vmx.c +2 -2
index a475aec..f9d5493 100644
@@ -563,7 +563,7 @@ static void do_vmxon_off(void)
 	vmx_on();
 	vmx_off();
 resume:
-	return;
+	barrier();
 }
 
 static void do_write_feature_control(void)

@@ -572,7 +572,7 @@ static void do_write_feature_control(void)
 	barrier();
 	wrmsr(MSR_IA32_FEATURE_CONTROL, 0);
 resume:
-	return;
+	barrier();
 }
 
 static int test_vmx_feature_control(void)
maintainer
#6 lore raw

Re: [PATCH 04/13] VMX: Fix return label in fault-triggering handlers

Il 22/01/2014 16:00, Paolo Bonzini ha scritto:

Quoted 8 lines

Il 04/01/2014 18:59, Jan Kiszka ha scritto:

Quoted 4 lines

From: Jan Kiszka <[email protected]>

Some compiler versions (seen with gcc 4.8.1) move the resume label after the return statement which, of course, causes sever problems.

Can you include the assembly output? Do you mean after the "ret" instruction?

Reproduced now, I think it's a compiler bug... with -O2 it's even weird, &&resume points to the *first* instruction in the function.

I'll report it to GCC.

Paolo

author
#7 lore raw

PATCH 5/13 lib/x86: Move exception test code into library

In reply to Jan Kiszka

From: Jan Kiszka <[email protected]>

Will also be used by the APIC test. Moving exception_return assignment out of line, we can drop the explicit compiler barrier.

Signed-off-by Jan Kiszka <[email protected]>
---
 lib/x86/desc.c | 24 ++++++++++++++++++++++++
 lib/x86/desc.h |  4 ++++
 x86/vmx.c      | 34 +++++++---------------------------
 3 files changed, 35 insertions(+), 27 deletions(-)
lib/x86/desc.c +24 -0
index 7c5c721..f75ec1d 100644
@@ -353,3 +353,27 @@ void print_current_tss_info(void)
                tr, tss[0].prev, tss[i].prev);
 }
 #endif
+
+static bool exception;
+static void *exception_return;
+
+static void exception_handler(struct ex_regs *regs)
+{
+	exception = true;
+	regs->rip = (unsigned long)exception_return;
+}
+
+bool test_for_exception(unsigned int ex, void (*trigger_func)(void *data),
+			void *data)
+{
+	handle_exception(ex, exception_handler);
+	exception = false;
+	trigger_func(data);
+	handle_exception(ex, NULL);
+	return exception;
+}
+
+void set_exception_return(void *addr)
+{
+	exception_return = addr;
+}
lib/x86/desc.h +4 -0
index f819452..5c850b2 100644
@@ -84,4 +84,8 @@ void set_intr_task_gate(int e, void *fn);
 void print_current_tss_info(void);
 void handle_exception(u8 v, void (*func)(struct ex_regs *regs));
 
+bool test_for_exception(unsigned int ex, void (*trigger_func)(void *data),
+			void *data);
+void set_exception_return(void *addr);
+
 #endif
x86/vmx.c +7 -27
index f9d5493..4f0bb8d 100644
@@ -538,38 +538,18 @@ static void init_vmx(void)
 	memset(guest_syscall_stack, 0, PAGE_SIZE);
 }
 
-static bool exception;
-static void *exception_return;
-
-static void exception_handler(struct ex_regs *regs)
+static void do_vmxon_off(void *data)
 {
-	exception = true;
-	regs->rip = (u64)exception_return;
-}
-
-static int test_for_exception(unsigned int ex, void (*func)(void))
-{
-	handle_exception(ex, exception_handler);
-	exception = false;
-	func();
-	handle_exception(ex, NULL);
-	return exception;
-}
-
-static void do_vmxon_off(void)
-{
-	exception_return = &&resume;
-	barrier();
+	set_exception_return(&&resume);
 	vmx_on();
 	vmx_off();
 resume:
 	barrier();
 }
 
-static void do_write_feature_control(void)
+static void do_write_feature_control(void *data)
 {
-	exception_return = &&resume;
-	barrier();
+	set_exception_return(&&resume);
 	wrmsr(MSR_IA32_FEATURE_CONTROL, 0);
 resume:
 	barrier();

@@ -592,18 +572,18 @@ static int test_vmx_feature_control(void)
 
 	wrmsr(MSR_IA32_FEATURE_CONTROL, 0);
 	report("test vmxon with FEATURE_CONTROL cleared",
-	       test_for_exception(GP_VECTOR, &do_vmxon_off));
+	       test_for_exception(GP_VECTOR, &do_vmxon_off, NULL));
 
 	wrmsr(MSR_IA32_FEATURE_CONTROL, 0x4);
 	report("test vmxon without FEATURE_CONTROL lock",
-	       test_for_exception(GP_VECTOR, &do_vmxon_off));
+	       test_for_exception(GP_VECTOR, &do_vmxon_off, NULL));
 
 	wrmsr(MSR_IA32_FEATURE_CONTROL, 0x5);
 	vmx_enabled = ((rdmsr(MSR_IA32_FEATURE_CONTROL) & 0x5) == 0x5);
 	report("test enable VMX in FEATURE_CONTROL", vmx_enabled);
 
 	report("test FEATURE_CONTROL lock bit",
-	       test_for_exception(GP_VECTOR, &do_write_feature_control));
+	       test_for_exception(GP_VECTOR, &do_write_feature_control, NULL));
 
 	return !vmx_enabled;
 }
Signature · 1 line
1.8.1.1.298.ge7eed54
author
#8 lore raw

PATCH 6/13 x2apic: Test for invalid state transitions

In reply to Jan Kiszka

From: Jan Kiszka <[email protected]>

This checks if KVM properly acknowledges invalid state transitions on MSR_APIC_BASE writes with a #GP.

Signed-off-by Jan Kiszka <[email protected]>
---
 lib/x86/apic-defs.h |  3 +++
 x86/apic.c          | 41 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 43 insertions(+), 1 deletion(-)
lib/x86/apic-defs.h +3 -0
index c061e3d..94112b4 100644
@@ -9,6 +9,9 @@
  */
 
 #define	APIC_DEFAULT_PHYS_BASE	0xfee00000
+#define APIC_BSP		(1UL << 8)
+#define APIC_EXTD		(1UL << 10)
+#define APIC_EN			(1UL << 11)
 
 #define	APIC_ID		0x20
x86/apic.c +40 -1

60 lines not on this page · raw message

Signature · 1 line
1.8.1.1.298.ge7eed54
author
#9 lore raw

PATCH 7/13 lib/x86/apic: Consolidate over MSR_IA32_APICBASE

In reply to Jan Kiszka

From: Jan Kiszka <[email protected]>

Signed-off-by Jan Kiszka <[email protected]>
---
 lib/x86/apic.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
lib/x86/apic.c +3 -4
index 7bb98ed..6876d85 100644
@@ -1,5 +1,6 @@
 #include "libcflat.h"
 #include "apic.h"
+#include "msr.h"
 
 static void *g_apic = (void *)0xfee00000;
 static void *g_ioapic = (void *)0xfec00000;

@@ -99,8 +100,6 @@ uint32_t apic_id(void)
     return apic_ops->id();
 }
 
-#define MSR_APIC_BASE 0x0000001b
-
 int enable_x2apic(void)
 {
     unsigned a, b, c, d;

@@ -108,9 +107,9 @@ int enable_x2apic(void)
     asm ("cpuid" : "=a"(a), "=b"(b), "=c"(c), "=d"(d) : "0"(1));
 
     if (c & (1 << 21)) {
-        asm ("rdmsr" : "=a"(a), "=d"(d) : "c"(MSR_APIC_BASE));
+        asm ("rdmsr" : "=a"(a), "=d"(d) : "c"(MSR_IA32_APICBASE));
         a |= 1 << 10;
-        asm ("wrmsr" : : "a"(a), "d"(d), "c"(MSR_APIC_BASE));
+        asm ("wrmsr" : : "a"(a), "d"(d), "c"(MSR_IA32_APICBASE));
         apic_ops = &x2apic_ops;
         return 1;
     } else {
Signature · 1 line
1.8.1.1.298.ge7eed54
author
#10 lore raw

PATCH 8/13 apic: Add test case for relocation and writing reserved bits

In reply to Jan Kiszka

From: Jan Kiszka <[email protected]>

Check that the xAPIC is relocatable and that writing a reserved bit to MSR_IA32_APICBASE triggers a #GP.

Signed-off-by Jan Kiszka <[email protected]>
---
 x86/apic.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
x86/apic.c +27 -0
index 4ebcd4f..8febfa2 100644
@@ -120,6 +120,32 @@ void test_enable_x2apic(void)
     }
 }
 
+#define ALTERNATE_APIC_BASE	0x42000000
+
+static void test_apicbase(void)
+{
+    u64 orig_apicbase = rdmsr(MSR_IA32_APICBASE);
+    u32 lvr = apic_read(APIC_LVR);
+    u64 value;
+
+    wrmsr(MSR_IA32_APICBASE, orig_apicbase & ~(APIC_EN | APIC_EXTD));
+    wrmsr(MSR_IA32_APICBASE, ALTERNATE_APIC_BASE | APIC_BSP | APIC_EN);
+
+    report("relocate apic",
+           *(volatile u32 *)(ALTERNATE_APIC_BASE + APIC_LVR) == lvr);
+
+    value = orig_apicbase | (1UL << (cpuid(0x80000008).a & 0xff));
+    report("apicbase: reserved physaddr bits",
+           test_for_exception(GP_VECTOR, do_write_apicbase, &value));
+
+    value = orig_apicbase | 1;
+    report("apicbase: reserved low bits",
+           test_for_exception(GP_VECTOR, do_write_apicbase, &value));
+
+    wrmsr(MSR_IA32_APICBASE, orig_apicbase);
+    apic_write(APIC_SPIV, 0x1ff);
+}
+
 static void eoi(void)
 {
     apic_write(APIC_EOI, 0);

@@ -366,6 +392,7 @@ int main()
 
     mask_pic_interrupts();
     test_enable_x2apic();
+    test_apicbase();
 
     test_self_ipi();
Signature · 1 line
1.8.1.1.298.ge7eed54
author
#11 lore raw

PATCH 9/13 VMX: Check unconditional I/O exiting

In reply to Jan Kiszka

From: Jan Kiszka <[email protected]>

Test if we ignore "unconditional I/O exiting" as long as "use I/O bitmap" is enabled. Also test if unconditional exiting itself works.

Signed-off-by Jan Kiszka <[email protected]>
---
 x86/vmx_tests.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)
x86/vmx_tests.c +35 -1
index 0077f3f..2c2d6c4 100644
@@ -701,13 +701,21 @@ static void iobmp_main()
 		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);
+	set_stage(10);
+	vmcall();
+	outb(0x0, 0x0);
+	report("I/O bitmap - unconditional exiting", stage == 11);
 }
 
 static int iobmp_exit_handler()
 {
 	u64 guest_rip;
 	ulong reason, exit_qual;
-	u32 insn_len;
+	u32 insn_len, ctrl_cpu0;
 
 	guest_rip = vmcs_read(GUEST_RIP);
 	reason = vmcs_read(EXI_REASON) & 0xff;

@@ -765,6 +773,32 @@ static int iobmp_exit_handler()
 			if (((exit_qual & VMX_IO_PORT_MASK) >> VMX_IO_PORT_SHIFT) == 0xFFFF)
 				set_stage(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);
+			break;
+		default:
+			// Should not reach here
+			printf("ERROR : unexpected stage, %d\n", get_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()) {
+		case 9:
+			ctrl_cpu0 = vmcs_read(CPU_EXEC_CTRL0);
+			ctrl_cpu0 |= CPU_IO | CPU_IO_BITMAP;
+			vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu0);
+			break;
+		case 10:
+			ctrl_cpu0 = vmcs_read(CPU_EXEC_CTRL0);
+			ctrl_cpu0 = (ctrl_cpu0 & ~CPU_IO_BITMAP) | CPU_IO;
+			vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu0);
+			break;
 		default:
 			// Should not reach here
 			printf("ERROR : unexpected stage, %d\n", get_stage());
Signature · 1 line
1.8.1.1.298.ge7eed54
author
#12 lore raw

PATCH 10/13 Provide common report and report_summary services

In reply to Jan Kiszka

From: Jan Kiszka <[email protected]>

This both reduces code duplication and standardizes the output format a bit more.

Signed-off-by Jan Kiszka <[email protected]>
---
 Makefile          |  3 ++-
 lib/libcflat.h    |  4 ++++
 lib/report.c      | 36 ++++++++++++++++++++++++++++++++++++
 x86/apic.c        | 15 +--------------
 x86/emulator.c    | 16 +---------------
 x86/eventinj.c    | 15 +--------------
 x86/idt_test.c    | 21 ++++-----------------
 x86/msr.c         | 15 +--------------
 x86/pcid.c        | 14 +-------------
 x86/pmu.c         | 37 +++++++++++++++----------------------
 x86/taskswitch2.c | 15 +--------------
 x86/vmx.c         | 16 +---------------
 x86/vmx.h         |  1 -
 13 files changed, 68 insertions(+), 140 deletions(-)
 create mode 100644 lib/report.c
Makefile +2 -1
index b6e8759..f5eccc7 100644
@@ -14,7 +14,8 @@ libcflat := lib/libcflat.a
 cflatobjs := \
 	lib/panic.o \
 	lib/printf.o \
-	lib/string.o
+	lib/string.o \
+	lib/report.o
 cflatobjs += lib/argv.o
 
 #include architecure specific make rules
lib/libcflat.h +4 -0
index fadc33d..f734fde 100644
@@ -45,6 +45,7 @@ extern char *strcat(char *dest, const char *src);
 extern int strcmp(const char *a, const char *b);
 
 extern int printf(const char *fmt, ...);
+extern int snprintf(char *buf, int size, const char *fmt, ...);
 extern int vsnprintf(char *buf, int size, const char *fmt, va_list va);
 
 extern void puts(const char *s);

@@ -58,4 +59,7 @@ extern long atol(const char *ptr);
 #define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)
 
 #define NULL ((void *)0UL)
+
+void report(const char *msg_fmt, bool pass, ...);
+int report_summary(void);
 #endif
lib/report.c new file +36 -0
new file mode 100644
index 0000000..ff562a1
@@ -0,0 +1,36 @@
+/*
+ * Test result reporting
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#include "libcflat.h"
+
+static unsigned int tests, failures;
+
+void report(const char *msg_fmt, bool pass, ...)
+{
+	char buf[2000];
+	va_list va;
+
+	tests++;
+	printf("%s: ", pass ? "PASS" : "FAIL");
+	va_start(va, pass);
+	vsnprintf(buf, sizeof(buf), msg_fmt, va);
+	va_end(va);
+	puts(buf);
+	puts("\n");
+	if (!pass)
+		failures++;
+}
+
+int report_summary(void)
+{
+	printf("\nSUMMARY: %d tests, %d failures\n", tests, failures);
+	return failures > 0 ? 1 : 0;
+}
x86/apic.c +1 -14
index 8febfa2..487c248 100644
@@ -6,17 +6,6 @@
 #include "isr.h"
 #include "msr.h"
 
-static int g_fail;
-static int g_tests;
-
-static void report(const char *msg, int pass)
-{
-    ++g_tests;
-    printf("%s: %s\n", msg, (pass ? "PASS" : "FAIL"));
-    if (!pass)
-        ++g_fail;
-}
-
 static void test_lapic_existence(void)
 {
     u32 lvr;

@@ -403,7 +392,5 @@ int main()
 
     test_tsc_deadline_timer();
 
-    printf("\nsummary: %d tests, %d failures\n", g_tests, g_fail);
-
-    return g_fail != 0;
+    return report_summary();
 }
x86/emulator.c +1 -15
index b70e540..2e25dd8 100644
@@ -7,8 +7,6 @@
 #define memset __builtin_memset
 #define TESTDEV_IO_PORT 0xe0
 
-int fails, tests;
-
 static int exceptions;
 
 struct regs {

@@ -25,17 +23,6 @@ struct insn_desc {
 	size_t len;
 };
 
-void report(const char *name, int result)
-{
-	++tests;
-	if (result)
-		printf("PASS: %s\n", name);
-	else {
-		printf("FAIL: %s\n", name);
-		++fails;
-	}
-}
-
 static char st1[] = "abcdefghijklmnop";
 
 void test_stringio()

@@ -1022,6 +1009,5 @@ int main()
 
 	test_string_io_mmio(mem);
 
-	printf("\nSUMMARY: %d tests, %d failures\n", tests, fails);
-	return fails ? 1 : 0;
+	return report_summary();
 }
x86/eventinj.c +1 -14
index 9d4392c..a218aaf 100644
@@ -12,9 +12,6 @@
 #  define R "e"
 #endif
 
-static int g_fail;
-static int g_tests;
-
 static inline void io_delay(void)
 {
 }

@@ -24,14 +21,6 @@ static inline void outl(int addr, int val)
         asm volatile ("outl %1, %w0" : : "d" (addr), "a" (val));
 }
 
-static void report(const char *msg, int pass)
-{
-    ++g_tests;
-    printf("%s: %s\n", msg, (pass ? "PASS" : "FAIL"));
-    if (!pass)
-        ++g_fail;
-}
-
 void apic_self_ipi(u8 v)
 {
 	apic_icr_write(APIC_DEST_SELF | APIC_DEST_PHYSICAL | APIC_DM_FIXED |

@@ -416,7 +405,5 @@ int main()
 	printf("After int 33 with shadowed stack\n");
 	report("int 33 with shadowed stack", test_count == 1);
 
-	printf("\nsummary: %d tests, %d failures\n", g_tests, g_fail);
-
-	return g_fail != 0;
+	return report_summary();
 }
x86/idt_test.c +4 -17
index 2d2e0c2..ecb76bb 100644
@@ -21,19 +21,6 @@ int test_gp(void)
     return exception_vector();
 }
 
-static int nr_fail, nr_test;
-
-static void report(int cond, const char *name)
-{
-    ++nr_test;
-    if (!cond) {
-        ++nr_fail;
-        printf("%s: FAIL\n", name);
-    } else {
-        printf("%s: PASS\n", name);
-    }
-}
-
 int main(void)
 {
     int r;

@@ -41,9 +28,9 @@ int main(void)
     printf("Starting IDT test\n");
     setup_idt();
     r = test_gp();
-    report(r == GP_VECTOR, "Testing #GP");
+    report("Testing #GP", r == GP_VECTOR);
     r = test_ud2();
-    report(r == UD_VECTOR, "Testing #UD");
-    printf("%d failures of %d tests\n", nr_fail, nr_test);
-    return !nr_fail ? 0 : 1;
+    report("Testing #UD", r == UD_VECTOR);
+
+    return report_summary();
 }
x86/msr.c +1 -14
index de7573d..db08a8d 100644
@@ -80,17 +80,6 @@ static int find_msr_info(int msr_index)
     return -1;
 }
 
-
-int nr_passed, nr_tests;
-
-static void report(const char *name, int passed)
-{
-	++nr_tests;
-	if (passed)
-		++nr_passed;
-	printf("%s: %s\n", name, passed ? "PASS" : "FAIL");
-}
-
 static void test_msr_rw(int msr_index, unsigned long long input, unsigned long long expected)
 {
     unsigned long long r = 0;

@@ -142,8 +131,6 @@ int main(int ac, char **av)
 
     test_syscall_lazy_load();
 
-    printf("%d tests, %d failures\n", nr_tests, nr_tests - nr_passed);
-
-    return nr_passed == nr_tests ? 0 : 1;
+    return report_summary();
 }
x86/pcid.c +1 -13
index 8bfeba2..45adfd5 100644
@@ -4,8 +4,6 @@
 #include "processor.h"
 #include "desc.h"
 
-int nr_passed, nr_tests;
-
 #define X86_FEATURE_PCID       (1 << 17)
 #define X86_FEATURE_INVPCID    (1 << 10)
 

@@ -22,14 +20,6 @@ struct invpcid_desc {
     unsigned long addr : 64;
 };
 
-static void report(const char *name, int passed)
-{
-    ++nr_tests;
-    if (passed)
-        ++nr_passed;
-    printf("%s: %s\n", name, passed ? "PASS" : "FAIL");
-}
-
 int write_cr0_checking(unsigned long val)
 {
     asm volatile(ASM_TRY("1f")

@@ -180,7 +170,5 @@ int main(int ac, char **av)
     else
         test_invpcid_disabled();
 
-    printf("%d tests, %d failures\n", nr_tests, nr_tests - nr_passed);
-
-    return nr_passed == nr_tests ? 0 : 1;
+    return report_summary();
 }
x86/pmu.c +15 -22
index 42b5a59..5c85146 100644
@@ -92,7 +92,6 @@ struct pmu_event {
 };
 
 static int num_counters;
-static int tests, failures;
 
 char *buf;
 

@@ -211,13 +210,6 @@ static void measure(pmu_counter_t *evt, int count)
 		stop_event(&evt[i]);
 }
 
-static void report(const char *name, int n, bool pass)
-{
-    printf("%s: pmu %s-%d\n", pass ? "PASS" : "FAIL", name, n);
-    tests += 1;
-    failures += !pass;
-}
-
 static bool verify_event(uint64_t count, struct pmu_event *e)
 {
 	// printf("%lld >= %lld <= %lld\n", e->min, count, e->max);

@@ -236,12 +228,14 @@ static void check_gp_counter(struct pmu_event *evt)
 		.ctr = MSR_IA32_PERFCTR0,
 		.config = EVNTSEL_OS | EVNTSEL_USR | evt->unit_sel,
 	};
+	char fmt[100];
 	int i;
 
 	for (i = 0; i < num_counters; i++, cnt.ctr++) {
 		cnt.count = 0;
 		measure(&cnt, 1);
-		report(evt->name, i, verify_event(cnt.count, evt));
+		snprintf(fmt, sizeof(fmt), "%s-%%d", evt->name);
+		report(fmt, verify_event(cnt.count, evt), i);
 	}
 }
 

@@ -268,7 +262,7 @@ static void check_fixed_counters(void)
 		cnt.count = 0;
 		cnt.ctr = fixed_events[i].unit_sel;
 		measure(&cnt, 1);
-		report("fixed", i, verify_event(cnt.count, &fixed_events[i]));
+		report("fixed-%d", verify_event(cnt.count, &fixed_events[i]), i);
 	}
 }
 

@@ -299,7 +293,7 @@ static void check_counters_many(void)
 		if (!verify_counter(&cnt[i]))
 			break;
 
-	report("all counters", 0, i == n);
+	report("all counters", i == n);
 }
 
 static void check_counter_overflow(void)

@@ -329,13 +323,13 @@ static void check_counter_overflow(void)
 		idx = event_to_global_idx(&cnt);
 		cnt.count = 1 - count;
 		measure(&cnt, 1);
-		report("overflow", i, cnt.count == 1);
+		report("overflow-%d", cnt.count == 1, i);
 		status = rdmsr(MSR_CORE_PERF_GLOBAL_STATUS);
-		report("overflow status", i, status & (1ull << idx));
+		report("overflow status-%d", status & (1ull << idx), i);
 		wrmsr(MSR_CORE_PERF_GLOBAL_OVF_CTRL, status);
 		status = rdmsr(MSR_CORE_PERF_GLOBAL_STATUS);
-		report("overflow status clear", i, !(status & (1ull << idx)));
-		report("overflow irq", i, check_irq() == (i % 2));
+		report("overflow status clear-%d", !(status & (1ull << idx)), i);
+		report("overflow irq-%d", check_irq() == (i % 2), i);
 	}
 }
 

@@ -348,7 +342,7 @@ static void check_gp_counter_cmask(void)
 	};
 	cnt.config |= (0x2 << EVNTSEL_CMASK_SHIFT);
 	measure(&cnt, 1);
-	report("cmask", 0, cnt.count < gp_events[1].min);
+	report("cmask", cnt.count < gp_events[1].min);
 }
 
 static void check_rdpmc(void)

@@ -360,15 +354,15 @@ static void check_rdpmc(void)
 		uint64_t x = (val & 0xffffffff) |
 			((1ull << (eax.split.bit_width - 32)) - 1) << 32;
 		wrmsr(MSR_IA32_PERFCTR0 + i, val);
-		report("rdpmc", i, rdpmc(i) == x);
-		report("rdpmc fast", i, rdpmc(i | (1<<31)) == (u32)val);
+		report("rdpmc-%d", rdpmc(i) == x, i);
+		report("rdpmc fast-%d", rdpmc(i | (1<<31)) == (u32)val, i);
 	}
 	for (i = 0; i < edx.split.num_counters_fixed; i++) {
 		uint64_t x = (val & 0xffffffff) |
 			((1ull << (edx.split.bit_width_fixed - 32)) - 1) << 32;
 		wrmsr(MSR_CORE_PERF_FIXED_CTR0 + i, val);
-		report("rdpmc fixed", i, rdpmc(i | (1 << 30)) == x);
-		report("rdpmc fixed fast", i, rdpmc(i | (3<<30)) == (u32)val);
+		report("rdpmc fixed-%d", rdpmc(i | (1 << 30)) == x, i);
+		report("rdpmc fixed fast-%d", rdpmc(i | (3<<30)) == (u32)val, i);
 	}
 }
 

@@ -409,6 +403,5 @@ int main(int ac, char **av)
 	check_counter_overflow();
 	check_gp_counter_cmask();
 
-	printf("\n%d tests, %d failures\n", tests, failures);
-	return !failures ? 0 : 1;
+	return report_summary();
 }
x86/taskswitch2.c +1 -14
index 6573696..3c8418e 100644
@@ -18,21 +18,10 @@ static volatile unsigned int test_divider;
 static char *fault_addr;
 static ulong fault_phys;
 
-static int g_fail;
-static int g_tests;
-
 static inline void io_delay(void)
 {
 }
 
-static void report(const char *msg, int pass)
-{
-    ++g_tests;
-    printf("%s: %s\n", msg, (pass ? "PASS" : "FAIL"));
-    if (!pass)
-        ++g_fail;
-}
-
 static void nmi_tss(void)
 {
 start:

@@ -273,7 +262,5 @@ int main()
 	test_kernel_mode_int();
 	test_vm86_switch();
 
-	printf("\nsummary: %d tests, %d failures\n", g_tests, g_fail);
-
-	return g_fail != 0;
+	return report_summary();
 }
x86/vmx.c +1 -15
index 4f0bb8d..2278078 100644
@@ -37,7 +37,6 @@
 #include "smp.h"
 #include "io.h"
 
-int fails, tests;
 u32 *vmxon_region;
 struct vmcs *vmcs_root;
 u32 vpid_cnt;

@@ -63,17 +62,6 @@ extern void *vmx_return;
 extern void *entry_sysenter;
 extern void *guest_entry;
 
-void report(const char *name, int result)
-{
-	++tests;
-	if (result)
-		printf("PASS: %s\n", name);
-	else {
-		printf("FAIL: %s\n", name);
-		++fails;
-	}
-}
-
 static int make_vmcs_current(struct vmcs *vmcs)
 {
 	bool ret;

@@ -786,7 +774,6 @@ int main(void)
 
 	setup_vm();
 	setup_idt();
-	fails = tests = 0;
 	hypercall_field = 0;
 
 	if (!(cpuid(1).c & (1 << 5))) {

@@ -815,6 +802,5 @@ int main(void)
 			goto exit;
 
 exit:
-	printf("\nSUMMARY: %d tests, %d failures\n", tests, fails);
-	return fails ? 1 : 0;
+	return report_summary();
 }
x86/vmx.h +0 -1
index 3867793..351c3d9 100644
@@ -550,7 +550,6 @@ static inline void invept(unsigned long type, u64 eptp)
 	asm volatile("invept %0, %1\n" ::"m"(operand),"r"(type));
 }
 
-void report(const char *name, int result);
 void print_vmexit_info();
 void install_ept_entry(unsigned long *pml4, int pte_level,
 		unsigned long guest_addr, unsigned long pte,
Signature · 1 line
1.8.1.1.298.ge7eed54
author
#14 lore raw

PATCH 12/13 svm: Add missing build dependency

In reply to Jan Kiszka

From: Jan Kiszka <[email protected]>

Signed-off-by Jan Kiszka <[email protected]>
---
 config-x86-common.mak | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
config-x86-common.mak +1 -1
index bf88c67..32da7fb 100644
@@ -86,7 +86,7 @@ $(TEST_DIR)/xsave.elf: $(cstart.o) $(TEST_DIR)/xsave.o
 
 $(TEST_DIR)/rmap_chain.elf: $(cstart.o) $(TEST_DIR)/rmap_chain.o
 
-$(TEST_DIR)/svm.elf: $(cstart.o)
+$(TEST_DIR)/svm.elf: $(cstart.o) $(TEST_DIR)/svm.o
 
 $(TEST_DIR)/kvmclock_test.elf: $(cstart.o) $(TEST_DIR)/kvmclock.o \
                                 $(TEST_DIR)/kvmclock_test.o
Signature · 1 line
1.8.1.1.298.ge7eed54
author
#15 lore raw

PATCH 13/13 x86: Add debug facility test case

In reply to Jan Kiszka

From: Jan Kiszka <[email protected]>

This adds a basic test for INT3/#BP, hardware breakpoints, hardware watchpoints and single-stepping.

Signed-off-by Jan Kiszka <[email protected]>
---
 config-x86-common.mak |   2 +
 config-x86_64.mak     |   2 +-
 lib/x86/desc.h        |   2 +
 x86/debug.c           | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++
 x86/unittests.cfg     |   3 ++
 5 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 x86/debug.c
config-x86-common.mak +2 -0
index 32da7fb..aa5a439 100644
@@ -103,6 +103,8 @@ $(TEST_DIR)/pcid.elf: $(cstart.o) $(TEST_DIR)/pcid.o
 
 $(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o $(TEST_DIR)/vmx_tests.o
 
+$(TEST_DIR)/debug.elf: $(cstart.o) $(TEST_DIR)/debug.o
+
 arch_clean:
 	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
 	$(TEST_DIR)/.*.d $(TEST_DIR)/lib/.*.d $(TEST_DIR)/lib/*.o
config-x86_64.mak +1 -1
index bb8ee89..a9a2a9e 100644
@@ -7,7 +7,7 @@ CFLAGS += -D__x86_64__
 tests = $(TEST_DIR)/access.flat $(TEST_DIR)/apic.flat \
 	  $(TEST_DIR)/emulator.flat $(TEST_DIR)/idt_test.flat \
 	  $(TEST_DIR)/xsave.flat $(TEST_DIR)/rmap_chain.flat \
-	  $(TEST_DIR)/pcid.flat
+	  $(TEST_DIR)/pcid.flat $(TEST_DIR)/debug.flat
 tests += $(TEST_DIR)/svm.flat
 tests += $(TEST_DIR)/vmx.flat
lib/x86/desc.h +2 -0
index 5c850b2..b795aad 100644
@@ -66,6 +66,8 @@ typedef struct {
     ".popsection \n\t"                                  \
     "1111:"
 
+#define DB_VECTOR   1
+#define BP_VECTOR   3
 #define UD_VECTOR   6
 #define GP_VECTOR   13
x86/debug.c new file +113 -0

113 lines not on this page · raw message

x86/unittests.cfg +3 -0
index 85c36aa..7930c02 100644
@@ -155,3 +155,6 @@ file = vmx.flat
 extra_params = -cpu host,+vmx
 arch = x86_64
 
+[debug]
+file = debug.flag
+arch = x86_64
Signature · 1 line
1.8.1.1.298.ge7eed54
maintainer
#16 lore raw
In reply to Jan Kiszka

Il 04/01/2014 18:59, Jan Kiszka ha scritto:

Quoted 40 lines

Highlights:
- improved preemption timer and interrupt injection tests (obsoletes my two patches in vmx queue)
- tests for IA32_APIC_BASE writes
- test for unconditional IO exiting (VMX)
- basic test of debug facilities (hw breakpoints etc.)

Jan Kiszka (13):
VMX: Add test cases around interrupt injection and halting
VMX: Extend preemption timer tests
apic: Remove redundant enable_apic
VMX: Fix return label in fault-triggering handlers lib/x86: Move exception test code into library
x2apic: Test for invalid state transitions lib/x86/apic: Consolidate over MSR_IA32_APICBASE
apic: Add test case for relocation and writing reserved bits
VMX: Check unconditional I/O exiting
Provide common report and report_summary services Ignore *.elf build outputs
svm: Add missing build dependency
x86: Add debug facility test case

 .gitignore            |   1 +
 Makefile              |   3 +-
 config-x86-common.mak |   4 +-
 config-x86_64.mak     |   2 +-
 lib/libcflat.h        |   4 +
 lib/report.c          |  36 +++++++
 lib/x86/apic-defs.h   |   3 +
 lib/x86/apic.c        |   7 +-
 lib/x86/desc.c        |  24 +++++
 lib/x86/desc.h        |   6 ++
 x86/apic.c            |  84 +++++++++++++---
 x86/debug.c           | 113 +++++++++++++++++++++
 x86/emulator.c        |  16 +--
 x86/eventinj.c        |  15 +--
 x86/idt_test.c        |  21 +---
 x86/msr.c             |  15 +--
 x86/pcid.c            |  14 +--
 x86/pmu.c             |  37 +++----
Quoted 9 lines
 x86/taskswitch2.c     |  15 +--
 x86/unittests.cfg     |   3 +
 x86/vmx.c             |  57 +++--------
 x86/vmx.h             |   4 +-
 x86/vmx_tests.c       | 264 ++++++++++++++++++++++++++++++++++++++++++++++----
 23 files changed, 548 insertions(+), 200 deletions(-)
 create mode 100644 lib/report.c
 create mode 100644 x86/debug.c

I'm applying patches 3-13 to the master branch, and patches 1-2 to the vmx branch.

Thanks!

Paolo

maintainer Applied
#17 lore raw
In reply to Jan Kiszka

Il 04/01/2014 18:59, Jan Kiszka ha scritto:

Quoted 7 lines

Highlights:
- improved preemption timer and interrupt injection tests (obsoletes my two patches in vmx queue)
- tests for IA32_APIC_BASE writes
- test for unconditional IO exiting (VMX)
- basic test of debug facilities (hw breakpoints etc.)

Quoted 14 lines

Jan Kiszka (13):
VMX: Add test cases around interrupt injection and halting
VMX: Extend preemption timer tests
apic: Remove redundant enable_apic
VMX: Fix return label in fault-triggering handlers lib/x86: Move exception test code into library
x2apic: Test for invalid state transitions lib/x86/apic: Consolidate over MSR_IA32_APICBASE
apic: Add test case for relocation and writing reserved bits
VMX: Check unconditional I/O exiting
Provide common report and report_summary services Ignore *.elf build outputs
svm: Add missing build dependency
x86: Add debug facility test case

Applied patches 3-13 for now. Will go back to 1 and 2 when I look at your pending KVM patches.

Thanks, and sorry for the delay. I was sure I had already pushed this (and I had been testing with them for one month).

Paolo