[PATCH 00/15] kvm-unit-tests: Fix and enhance nVMX tests

[PATCH 00/15] kvm-unit-tests: Fix and enhance nVMX tests

[Jan Kiszka]

This series contains various smaller fixes and cleanups for the VMX unit
tests and also a few unrelated corners of the test suite. It ends with a
rework of the VMX capability test and the addition of a bunch of new
test cases for interrupt injection, halting and the preemption timer.
Most of them fail or lockup at the moment. More work is required on KVM
the fix this, I'm on it.

These apply on top of the vmx branch. They are also available at

    git://git.kiszka.org/kvm-unit-tests queue

Jan

Jan Kiszka (15):
  VMX: Fix initialization of GDT, IDT and TR descriptors
  VMX: Drop redundant return statements
  rmap_chain: Remove dead code
  emulator: Exclude test_lgdt_lidt from building
  init: Fix warning about returning without code
  lib/x86/smp: Fix compiler warnings
  VMX: Remove unneeded basic_init
  VMX: Fix capability check for EPT test
  VMX: Let init handler decide about test start
  VMX: Remove unused return code from setup_ept_range
  VMX: Make syscall handler optional
  VMX: Simplify basic handlers
  VMX: Rework and enhance initial feature detection/enabling
  VMX: Add test cases around interrupt injection and halting
  VMX: Add test case for preemption timer firing during hlt

 lib/x86/smp.c    |   3 +-
 x86/emulator.c   |   2 +
 x86/init.c       |   1 +
 x86/rmap_chain.c |   9 --
 x86/vmx.c        | 123 +++++++++++++++++-------
 x86/vmx.h        |  15 ++-
 x86/vmx_tests.c  | 286 ++++++++++++++++++++++++++++++++++++++-----------------
 7 files changed, 298 insertions(+), 141 deletions(-)

-- 
1.8.1.1.298.ge7eed54

[PATCH 01/15] VMX: Fix initialization of GDT, IDT and TR descriptors

[Jan Kiszka]

We were loading the addresses of the descriptor pointers, not the
addresses they are pointing to. By declaring the pointers with the
proper type, we can also save some castings + struct descr.

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

diff --git a/x86/vmx.c b/x86/vmx.c
index 31d7bd1..4c463fd 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -56,9 +56,9 @@ union vmx_ctrl_exit ctrl_exit_rev;
 union vmx_ctrl_ent ctrl_enter_rev;
 union vmx_ept_vpid  ept_vpid;
 
-extern u64 gdt64_desc[];
-extern u64 idt_descr[];
-extern u64 tss_descr[];
+extern struct descriptor_table_ptr gdt64_desc;
+extern struct descriptor_table_ptr idt_descr;
+extern struct descriptor_table_ptr tss_descr;
 extern void *vmx_return;
 extern void *entry_sysenter;
 extern void *guest_entry;
@@ -368,9 +368,9 @@ static void init_vmcs_host(void)
 	vmcs_write(HOST_SEL_FS, SEL_KERN_DATA_64);
 	vmcs_write(HOST_SEL_GS, SEL_KERN_DATA_64);
 	vmcs_write(HOST_SEL_TR, SEL_TSS_RUN);
-	vmcs_write(HOST_BASE_TR,   (u64)tss_descr);
-	vmcs_write(HOST_BASE_GDTR, (u64)gdt64_desc);
-	vmcs_write(HOST_BASE_IDTR, (u64)idt_descr);
+	vmcs_write(HOST_BASE_TR, tss_descr.base);
+	vmcs_write(HOST_BASE_GDTR, gdt64_desc.base);
+	vmcs_write(HOST_BASE_IDTR, idt_descr.base);
 	vmcs_write(HOST_BASE_FS, 0);
 	vmcs_write(HOST_BASE_GS, 0);
 
@@ -424,7 +424,7 @@ static void init_vmcs_guest(void)
 	vmcs_write(GUEST_BASE_DS, 0);
 	vmcs_write(GUEST_BASE_FS, 0);
 	vmcs_write(GUEST_BASE_GS, 0);
-	vmcs_write(GUEST_BASE_TR,   (u64)tss_descr);
+	vmcs_write(GUEST_BASE_TR, tss_descr.base);
 	vmcs_write(GUEST_BASE_LDTR, 0);
 
 	vmcs_write(GUEST_LIMIT_CS, 0xFFFFFFFF);
@@ -434,7 +434,7 @@ static void init_vmcs_guest(void)
 	vmcs_write(GUEST_LIMIT_FS, 0xFFFFFFFF);
 	vmcs_write(GUEST_LIMIT_GS, 0xFFFFFFFF);
 	vmcs_write(GUEST_LIMIT_LDTR, 0xffff);
-	vmcs_write(GUEST_LIMIT_TR, ((struct descr *)tss_descr)->limit);
+	vmcs_write(GUEST_LIMIT_TR, tss_descr.limit);
 
 	vmcs_write(GUEST_AR_CS, 0xa09b);
 	vmcs_write(GUEST_AR_DS, 0xc093);
@@ -446,12 +446,10 @@ static void init_vmcs_guest(void)
 	vmcs_write(GUEST_AR_TR, 0x8b);
 
 	/* 26.3.1.3 */
-	vmcs_write(GUEST_BASE_GDTR, (u64)gdt64_desc);
-	vmcs_write(GUEST_BASE_IDTR, (u64)idt_descr);
-	vmcs_write(GUEST_LIMIT_GDTR,
-		((struct descr *)gdt64_desc)->limit & 0xffff);
-	vmcs_write(GUEST_LIMIT_IDTR,
-		((struct descr *)idt_descr)->limit & 0xffff);
+	vmcs_write(GUEST_BASE_GDTR, gdt64_desc.base);
+	vmcs_write(GUEST_BASE_IDTR, idt_descr.base);
+	vmcs_write(GUEST_LIMIT_GDTR, gdt64_desc.limit);
+	vmcs_write(GUEST_LIMIT_IDTR, idt_descr.limit);
 
 	/* 26.3.1.4 */
 	vmcs_write(GUEST_RIP, (u64)(&guest_entry));
diff --git a/x86/vmx.h b/x86/vmx.h
index 7d967eb..59d627a 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -95,11 +95,6 @@ union vmx_ept_vpid {
 	};
 };
 
-struct descr {
-	u16 limit;
-	u64 addr;
-};
-
 enum Encoding {
 	/* 16-Bit Control Fields */
 	VPID			= 0x0000ul,
-- 
1.8.1.1.298.ge7eed54

[PATCH 02/15] VMX: Drop redundant return statements

[Jan Kiszka]

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

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 3f584ed..74d633b 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -668,8 +668,6 @@ static void iobmp_main()
 		report("I/O bitmap - overrun", 1);
 	else
 		report("I/O bitmap - overrun", 0);
-	
-	return;
 }
 
 static int iobmp_exit_handler()
@@ -1022,7 +1020,6 @@ t1:
 		report("EPT violation - paging structure", 1);
 	else
 		report("EPT violation - paging structure", 0);
-	return;
 }
 
 static int ept_exit_handler()
-- 
1.8.1.1.298.ge7eed54

[PATCH 03/15] rmap_chain: Remove dead code

[Jan Kiszka]

Fixes corresponding warning.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 x86/rmap_chain.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/x86/rmap_chain.c b/x86/rmap_chain.c
index 0df1bcb..b356a37 100644
--- a/x86/rmap_chain.c
+++ b/x86/rmap_chain.c
@@ -5,15 +5,6 @@
 #include "vm.h"
 #include "smp.h"
 
-void print(const char *s);
-
-static unsigned int inl(unsigned short port)
-{
-    unsigned int val;
-    asm volatile ("inl %w1, %0":"=a" (val):"Nd" (port));
-    return val;
-}
-
 int main (void)
 {
     int i;
-- 
1.8.1.1.298.ge7eed54

[PATCH 04/15] emulator: Exclude test_lgdt_lidt from building

[Jan Kiszka]

See commit 47c1461a5.

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

diff --git a/x86/emulator.c b/x86/emulator.c
index 68d2b93..4e70e8f 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -843,6 +843,7 @@ static void test_string_io_mmio(volatile uint8_t *mem)
 	report("string_io_mmio", mmio[1023] == 0x99);
 }
 
+/* kvm doesn't allow lidt/lgdt from mmio, so the test is disabled
 static void test_lgdt_lidt(volatile uint8_t *mem)
 {
     struct descriptor_table_ptr orig, fresh = {};
@@ -871,6 +872,7 @@ static void test_lgdt_lidt(volatile uint8_t *mem)
     sti();
     report("lidt (long address)", orig.limit == fresh.limit && orig.base == fresh.base);
 }
+*/
 
 static void ss_bad_rpl(struct ex_regs *regs)
 {
-- 
1.8.1.1.298.ge7eed54

[PATCH 05/15] init: Fix warning about returning without code

[Jan Kiszka]

Main does not return, tell this the compiler.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 x86/init.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/x86/init.c b/x86/init.c
index 717511e..344dc1c 100644
--- a/x86/init.c
+++ b/x86/init.c
@@ -107,6 +107,7 @@ int main(int argc, char **argv)
 
 	/* The resume code will get us back to main.  */
 	asm("cli; hlt");
+	__builtin_unreachable();
 }
 
 asm (
-- 
1.8.1.1.298.ge7eed54

[PATCH 06/15] lib/x86/smp: Fix compiler warnings

[Jan Kiszka]

Add missing include of desc.h for prototypes of setup_idt and
set_idt_entry and cast away the volatile of ipi_data - it's not volatile
while we run the IPI handler.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 lib/x86/smp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index d4c8106..75ac081 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -3,6 +3,7 @@
 #include "smp.h"
 #include "apic.h"
 #include "fwcfg.h"
+#include "desc.h"
 
 #define IPI_VECTOR 0x20
 
@@ -18,7 +19,7 @@ static int _cpu_count;
 static __attribute__((used)) void ipi()
 {
     void (*function)(void *data) = ipi_function;
-    void *data = ipi_data;
+    void *data = (void *)ipi_data;
     bool wait = ipi_wait;
 
     if (!wait) {
-- 
1.8.1.1.298.ge7eed54

[PATCH 07/15] VMX: Remove unneeded basic_init

[Jan Kiszka]

We already accept the init handler to be NULL if there is no need for
initialization.

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

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 74d633b..a25afdc 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -43,10 +43,6 @@ static inline u32 get_stage()
 	return s;
 }
 
-void basic_init()
-{
-}
-
 void basic_guest_main()
 {
 	/* Here is a basic guest_main, print Hello World */
@@ -1134,9 +1130,9 @@ static int ept_exit_handler()
 /* name/init/guest_main/exit_handler/syscall_handler/guest_regs
    basic_* just implement some basic functions */
 struct vmx_test vmx_tests[] = {
-	{ "null", basic_init, basic_guest_main, basic_exit_handler,
+	{ "null", NULL, basic_guest_main, basic_exit_handler,
 		basic_syscall_handler, {0} },
-	{ "vmenter", basic_init, vmenter_main, vmenter_exit_handler,
+	{ "vmenter", NULL, vmenter_main, vmenter_exit_handler,
 		basic_syscall_handler, {0} },
 	{ "preemption timer", preemption_timer_init, preemption_timer_main,
 		preemption_timer_exit_handler, basic_syscall_handler, {0} },
@@ -1144,7 +1140,7 @@ struct vmx_test vmx_tests[] = {
 		test_ctrl_pat_exit_handler, basic_syscall_handler, {0} },
 	{ "control field EFER", test_ctrl_efer_init, test_ctrl_efer_main,
 		test_ctrl_efer_exit_handler, basic_syscall_handler, {0} },
-	{ "CR shadowing", basic_init, cr_shadowing_main,
+	{ "CR shadowing", NULL, cr_shadowing_main,
 		cr_shadowing_exit_handler, basic_syscall_handler, {0} },
 	{ "I/O bitmap", iobmp_init, iobmp_main, iobmp_exit_handler,
 		basic_syscall_handler, {0} },
-- 
1.8.1.1.298.ge7eed54

[PATCH 08/15] VMX: Fix capability check for EPT test

[Jan Kiszka]

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

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index a25afdc..d330d5c 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -963,8 +963,8 @@ static void ept_main()
 {
 	if (init_fail)
 		return;
-	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY)
-		&& !(ctrl_cpu_rev[1].clr & CPU_EPT)) {
+	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) ||
+	    !(ctrl_cpu_rev[1].clr & CPU_EPT)) {
 		printf("\tEPT is not supported");
 		return;
 	}
-- 
1.8.1.1.298.ge7eed54

[PATCH 09/15] VMX: Let init handler decide about test start

[Jan Kiszka]

If some feature is not existing and, thus, a test case is not able to
run, already report this back from the init handler.

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

diff --git a/x86/vmx.c b/x86/vmx.c
index 4c463fd..2928e70 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -731,8 +731,8 @@ static int test_run(struct vmx_test *test)
 	init_vmcs(&(test->vmcs));
 	/* Directly call test->init is ok here, init_vmcs has done
 	   vmcs init, vmclear and vmptrld*/
-	if (test->init)
-		test->init(test->vmcs);
+	if (test->init && test->init(test->vmcs) != VMX_TEST_START)
+		return 0;
 	test->exits = 0;
 	current = test;
 	regs = test->guest_regs;
diff --git a/x86/vmx.h b/x86/vmx.h
index 59d627a..9d3b942 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -32,7 +32,7 @@ struct regs {
 
 struct vmx_test {
 	const char *name;
-	void (*init)(struct vmcs *vmcs);
+	int (*init)(struct vmcs *vmcs);
 	void (*guest_main)();
 	int (*exit_handler)();
 	void (*syscall_handler)(u64 syscall_no);
@@ -434,6 +434,7 @@ enum Ctrl1 {
 #define VMX_IO_PORT_MASK		0xFFFF0000
 #define VMX_IO_PORT_SHIFT		16
 
+#define VMX_TEST_START			0
 #define VMX_TEST_VMEXIT			1
 #define VMX_TEST_EXIT			2
 #define VMX_TEST_RESUME			3
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index d330d5c..ae6087c 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -16,7 +16,6 @@ volatile u32 stage;
 void *io_bitmap_a, *io_bitmap_b;
 u16 ioport;
 
-bool init_fail;
 unsigned long *pml4;
 u64 eptp;
 void *data_page1, *data_page2;
@@ -119,28 +118,26 @@ u32 preempt_scale;
 volatile unsigned long long tsc_val;
 volatile u32 preempt_val;
 
-void preemption_timer_init()
+int preemption_timer_init()
 {
-	u32 ctrl_pin;
-
-	ctrl_pin = vmcs_read(PIN_CONTROLS) | PIN_PREEMPT;
-	ctrl_pin &= ctrl_pin_rev.clr;
-	vmcs_write(PIN_CONTROLS, ctrl_pin);
+	if (!(ctrl_pin_rev.clr & PIN_PREEMPT)) {
+		printf("\tPreemption timer is not supported\n");
+		return VMX_TEST_EXIT;
+	}
+	vmcs_write(PIN_CONTROLS, vmcs_read(PIN_CONTROLS) | PIN_PREEMPT);
 	preempt_val = 10000000;
 	vmcs_write(PREEMPT_TIMER_VALUE, preempt_val);
 	preempt_scale = rdmsr(MSR_IA32_VMX_MISC) & 0x1F;
 
 	if (!(ctrl_exit_rev.clr & EXI_SAVE_PREEMPT))
 		printf("\tSave preemption value is not supported\n");
+
+	return VMX_TEST_START;
 }
 
 void preemption_timer_main()
 {
 	tsc_val = rdtsc();
-	if (!(ctrl_pin_rev.clr & PIN_PREEMPT)) {
-		printf("\tPreemption timer is not supported\n");
-		return;
-	}
 	if (ctrl_exit_rev.clr & EXI_SAVE_PREEMPT) {
 		set_stage(0);
 		vmcall();
@@ -223,7 +220,7 @@ void msr_bmp_init()
 	vmcs_write(MSR_BITMAP, (u64)msr_bitmap);
 }
 
-static void test_ctrl_pat_init()
+static int test_ctrl_pat_init()
 {
 	u64 ctrl_ent;
 	u64 ctrl_exi;
@@ -236,6 +233,7 @@ static void test_ctrl_pat_init()
 	ia32_pat = rdmsr(MSR_IA32_CR_PAT);
 	vmcs_write(GUEST_PAT, 0x0);
 	vmcs_write(HOST_PAT, ia32_pat);
+	return VMX_TEST_START;
 }
 
 static void test_ctrl_pat_main()
@@ -301,7 +299,7 @@ static int test_ctrl_pat_exit_handler()
 	return VMX_TEST_VMEXIT;
 }
 
-static void test_ctrl_efer_init()
+static int test_ctrl_efer_init()
 {
 	u64 ctrl_ent;
 	u64 ctrl_exi;
@@ -314,6 +312,7 @@ static void test_ctrl_efer_init()
 	ia32_efer = rdmsr(MSR_EFER);
 	vmcs_write(GUEST_EFER, ia32_efer ^ EFER_NX);
 	vmcs_write(HOST_EFER, ia32_efer ^ EFER_NX);
+	return VMX_TEST_START;
 }
 
 static void test_ctrl_efer_main()
@@ -589,7 +588,7 @@ static int cr_shadowing_exit_handler()
 	return VMX_TEST_VMEXIT;
 }
 
-static void iobmp_init()
+static int iobmp_init()
 {
 	u32 ctrl_cpu0;
 
@@ -603,6 +602,7 @@ static void iobmp_init()
 	vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu0);
 	vmcs_write(IO_BITMAP_A, (u64)io_bitmap_a);
 	vmcs_write(IO_BITMAP_B, (u64)io_bitmap_b);
+	return VMX_TEST_START;
 }
 
 static void iobmp_main()
@@ -814,7 +814,7 @@ static struct insn_table insn_table[] = {
 	{NULL},
 };
 
-static void insn_intercept_init()
+static int insn_intercept_init()
 {
 	u32 ctrl_cpu[2];
 
@@ -827,6 +827,7 @@ static void insn_intercept_init()
 	ctrl_cpu[1] |= CPU_WBINVD | CPU_RDRAND;
 	ctrl_cpu[1] &= ctrl_cpu_rev[1].clr;
 	vmcs_write(CPU_EXEC_CTRL1, ctrl_cpu[1]);
+	return VMX_TEST_START;
 }
 
 static void insn_intercept_main()
@@ -926,12 +927,17 @@ static int setup_ept()
 	return 0;
 }
 
-static void ept_init()
+static int ept_init()
 {
 	unsigned long base_addr1, base_addr2;
 	u32 ctrl_cpu[2];
 
-	init_fail = false;
+	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) ||
+	    !(ctrl_cpu_rev[1].clr & CPU_EPT)) {
+		printf("\tEPT is not supported");
+		return VMX_TEST_EXIT;
+	}
+
 	ctrl_cpu[0] = vmcs_read(CPU_EXEC_CTRL0);
 	ctrl_cpu[1] = vmcs_read(CPU_EXEC_CTRL1);
 	ctrl_cpu[0] = (ctrl_cpu[0] | CPU_SECONDARY)
@@ -941,7 +947,7 @@ static void ept_init()
 	vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu[0]);
 	vmcs_write(CPU_EXEC_CTRL1, ctrl_cpu[1]);
 	if (setup_ept())
-		init_fail = true;
+		return VMX_TEST_EXIT;
 	data_page1 = alloc_page();
 	data_page2 = alloc_page();
 	memset(data_page1, 0x0, PAGE_SIZE);
@@ -954,20 +960,14 @@ static void ept_init()
 			    EPT_WA | EPT_RA | EPT_EA) ||
 	    setup_ept_range(pml4, base_addr2, base_addr2 + PAGE_SIZE_2M, 0, 0,
 			    EPT_WA | EPT_RA | EPT_EA))
-		init_fail = true;
+		return VMX_TEST_EXIT;
 	install_ept(pml4, (unsigned long)data_page1, (unsigned long)data_page2,
 			EPT_RA | EPT_WA | EPT_EA);
+	return VMX_TEST_START;
 }
 
 static void ept_main()
 {
-	if (init_fail)
-		return;
-	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) ||
-	    !(ctrl_cpu_rev[1].clr & CPU_EPT)) {
-		printf("\tEPT is not supported");
-		return;
-	}
 	set_stage(0);
 	if (*((u32 *)data_page2) != MAGIC_VAL_1 ||
 			*((u32 *)data_page1) != MAGIC_VAL_1)
-- 
1.8.1.1.298.ge7eed54

[PATCH 10/15] VMX: Remove unused return code from setup_ept_range

[Jan Kiszka]

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

diff --git a/x86/vmx.c b/x86/vmx.c
index 2928e70..f220e13 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -248,8 +248,8 @@ void install_2m_ept(unsigned long *pml4,
 		@map_2m : whether 2M page map is used
 		@perm : permission for every page
  */
-int setup_ept_range(unsigned long *pml4, unsigned long start,
-		unsigned long len, int map_1g, int map_2m, u64 perm)
+void setup_ept_range(unsigned long *pml4, unsigned long start,
+		     unsigned long len, int map_1g, int map_2m, u64 perm)
 {
 	u64 phys = start;
 	u64 max = (u64)len + (u64)start;
@@ -270,7 +270,6 @@ int setup_ept_range(unsigned long *pml4, unsigned long start,
 		install_ept(pml4, phys, phys, perm);
 		phys += PAGE_SIZE;
 	}
-	return 0;
 }
 
 /* get_ept_pte : Get the PTE of a given level in EPT,
diff --git a/x86/vmx.h b/x86/vmx.h
index 9d3b942..bc8c86f 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -558,8 +558,8 @@ void install_2m_ept(unsigned long *pml4, unsigned long phys,
 		unsigned long guest_addr, u64 perm);
 void install_ept(unsigned long *pml4, unsigned long phys,
 		unsigned long guest_addr, u64 perm);
-int setup_ept_range(unsigned long *pml4, unsigned long start,
-		unsigned long len, int map_1g, int map_2m, u64 perm);
+void setup_ept_range(unsigned long *pml4, unsigned long start,
+		     unsigned long len, int map_1g, int map_2m, u64 perm);
 unsigned long get_ept_pte(unsigned long *pml4,
 		unsigned long guest_addr, int level);
 int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index ae6087c..04238ac 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -919,11 +919,8 @@ static int setup_ept()
 	end_of_memory = fwcfg_get_u64(FW_CFG_RAM_SIZE);
 	if (end_of_memory < (1ul << 32))
 		end_of_memory = (1ul << 32);
-	if (setup_ept_range(pml4, 0, end_of_memory,
-			0, support_2m, EPT_WA | EPT_RA | EPT_EA)) {
-		printf("\tSet ept tables failed.\n");
-		return 1;
-	}
+	setup_ept_range(pml4, 0, end_of_memory, 0, support_2m,
+			EPT_WA | EPT_RA | EPT_EA);
 	return 0;
 }
 
@@ -956,11 +953,10 @@ static int ept_init()
 	*((u32 *)data_page2) = MAGIC_VAL_2;
 	base_addr1 = (unsigned long)data_page1 & PAGE_MASK_2M;
 	base_addr2 = (unsigned long)data_page2 & PAGE_MASK_2M;
-	if (setup_ept_range(pml4, base_addr1, base_addr1 + PAGE_SIZE_2M, 0, 0,
-			    EPT_WA | EPT_RA | EPT_EA) ||
-	    setup_ept_range(pml4, base_addr2, base_addr2 + PAGE_SIZE_2M, 0, 0,
-			    EPT_WA | EPT_RA | EPT_EA))
-		return VMX_TEST_EXIT;
+	setup_ept_range(pml4, base_addr1, base_addr1 + PAGE_SIZE_2M, 0, 0,
+			EPT_WA | EPT_RA | EPT_EA);
+	setup_ept_range(pml4, base_addr2, base_addr2 + PAGE_SIZE_2M, 0, 0,
+			EPT_WA | EPT_RA | EPT_EA);
 	install_ept(pml4, (unsigned long)data_page1, (unsigned long)data_page2,
 			EPT_RA | EPT_WA | EPT_EA);
 	return VMX_TEST_START;
-- 
1.8.1.1.298.ge7eed54

[PATCH 11/15] VMX: Make syscall handler optional

[Jan Kiszka]

Instead of requiring to reference the basic handler, just allow NULL as
syscall handler for those tests that don't care. Makes test definitions
a bit more compact.

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

diff --git a/x86/vmx.c b/x86/vmx.c
index f220e13..f6f2f59 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -97,7 +97,8 @@ asm(
 
 static void __attribute__((__used__)) syscall_handler(u64 syscall_no)
 {
-	current->syscall_handler(syscall_no);
+	if (current->syscall_handler)
+		current->syscall_handler(syscall_no);
 }
 
 static inline int vmx_on()
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 04238ac..fe7bbdf 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -69,10 +69,6 @@ int basic_exit_handler()
 	return VMX_TEST_EXIT;
 }
 
-void basic_syscall_handler(u64 syscall_no)
-{
-}
-
 void vmenter_main()
 {
 	u64 rax;
@@ -1123,26 +1119,22 @@ static int ept_exit_handler()
 	return VMX_TEST_VMEXIT;
 }
 
-/* name/init/guest_main/exit_handler/syscall_handler/guest_regs
-   basic_* just implement some basic functions */
+/* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
 struct vmx_test vmx_tests[] = {
-	{ "null", NULL, basic_guest_main, basic_exit_handler,
-		basic_syscall_handler, {0} },
-	{ "vmenter", NULL, vmenter_main, vmenter_exit_handler,
-		basic_syscall_handler, {0} },
+	{ "null", NULL, basic_guest_main, basic_exit_handler, NULL, {0} },
+	{ "vmenter", NULL, vmenter_main, vmenter_exit_handler, NULL, {0} },
 	{ "preemption timer", preemption_timer_init, preemption_timer_main,
-		preemption_timer_exit_handler, basic_syscall_handler, {0} },
+		preemption_timer_exit_handler, NULL, {0} },
 	{ "control field PAT", test_ctrl_pat_init, test_ctrl_pat_main,
-		test_ctrl_pat_exit_handler, basic_syscall_handler, {0} },
+		test_ctrl_pat_exit_handler, NULL, {0} },
 	{ "control field EFER", test_ctrl_efer_init, test_ctrl_efer_main,
-		test_ctrl_efer_exit_handler, basic_syscall_handler, {0} },
+		test_ctrl_efer_exit_handler, NULL, {0} },
 	{ "CR shadowing", NULL, cr_shadowing_main,
-		cr_shadowing_exit_handler, basic_syscall_handler, {0} },
+		cr_shadowing_exit_handler, NULL, {0} },
 	{ "I/O bitmap", iobmp_init, iobmp_main, iobmp_exit_handler,
-		basic_syscall_handler, {0} },
+		NULL, {0} },
 	{ "instruction intercept", insn_intercept_init, insn_intercept_main,
-		insn_intercept_exit_handler, basic_syscall_handler, {0} },
-	{ "EPT framework", ept_init, ept_main, ept_exit_handler,
-		basic_syscall_handler, {0} },
+		insn_intercept_exit_handler, NULL, {0} },
+	{ "EPT framework", ept_init, ept_main, ept_exit_handler, NULL, {0} },
 	{ NULL, NULL, NULL, NULL, NULL, {0} },
 };
-- 
1.8.1.1.298.ge7eed54

[PATCH 12/15] VMX: Simplify basic handlers

[Jan Kiszka]

Be less verbose and remove any unused code from the basic handlers.
Properly fail if a vmexit occurs during the early tests.

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

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index fe7bbdf..36c56b4 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -44,27 +44,11 @@ static inline u32 get_stage()
 
 void basic_guest_main()
 {
-	/* Here is a basic guest_main, print Hello World */
-	printf("\tHello World, this is null_guest_main!\n");
 }
 
 int basic_exit_handler()
 {
-	u64 guest_rip;
-	ulong reason;
-
-	guest_rip = vmcs_read(GUEST_RIP);
-	reason = vmcs_read(EXI_REASON) & 0xff;
-
-	switch (reason) {
-	case VMX_VMCALL:
-		print_vmexit_info();
-		vmcs_write(GUEST_RIP, guest_rip + 3);
-		return VMX_TEST_RESUME;
-	default:
-		break;
-	}
-	printf("ERROR : Unhandled vmx exit.\n");
+	report("Basic VMX test", 0);
 	print_vmexit_info();
 	return VMX_TEST_EXIT;
 }
-- 
1.8.1.1.298.ge7eed54

[PATCH 13/15] VMX: Rework and enhance initial feature detection/enabling

[Jan Kiszka]

So far we ran into an unhandled GP when VMX was unavailable. Change the
logic to handle this gracefully. Rework test_vmx_capability to
test_vmx_feature_control which not only enables VMX in the feature
control MSR but also test related error behavior.

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

diff --git a/x86/vmx.c b/x86/vmx.c
index f6f2f59..fe950e6 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -539,22 +539,74 @@ static void init_vmx(void)
 	memset(guest_syscall_stack, 0, PAGE_SIZE);
 }
 
-static int test_vmx_capability(void)
+static bool exception;
+static void *exception_return;
+
+static void exception_handler(struct ex_regs *regs)
+{
+	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();
+	vmx_on();
+	vmx_off();
+resume:
+	return;
+}
+
+static void do_write_feature_control(void)
+{
+	exception_return = &&resume;
+	barrier();
+	wrmsr(MSR_IA32_FEATURE_CONTROL, 0);
+resume:
+	return;
+}
+
+static int test_vmx_feature_control(void)
 {
-	struct cpuid r;
-	u64 ret1, ret2;
 	u64 ia32_feature_control;
-	r = cpuid(1);
-	ret1 = ((r.c) >> 5) & 1;
+	bool vmx_enabled;
+
 	ia32_feature_control = rdmsr(MSR_IA32_FEATURE_CONTROL);
-	ret2 = ((ia32_feature_control & 0x5) == 0x5);
-	if ((!ret2) && ((ia32_feature_control & 0x1) == 0)) {
-		wrmsr(MSR_IA32_FEATURE_CONTROL, 0x5);
-		ia32_feature_control = rdmsr(MSR_IA32_FEATURE_CONTROL);
-		ret2 = ((ia32_feature_control & 0x5) == 0x5);
+	vmx_enabled = ((ia32_feature_control & 0x5) == 0x5);
+	if ((ia32_feature_control & 0x5) == 0x5) {
+		printf("VMX enabled and locked by BIOS\n");
+		return 0;
+	} else if (ia32_feature_control & 0x1) {
+		printf("ERROR: VMX locked out by BIOS!?\n");
+		return 1;
 	}
-	report("test vmx capability", ret1 & ret2);
-	return !(ret1 & ret2);
+
+	wrmsr(MSR_IA32_FEATURE_CONTROL, 0);
+	report("test vmxon with FEATURE_CONTROL cleared",
+	       test_for_exception(GP_VECTOR, &do_vmxon_off));
+
+	wrmsr(MSR_IA32_FEATURE_CONTROL, 0x4);
+	report("test vmxon without FEATURE_CONTROL lock",
+	       test_for_exception(GP_VECTOR, &do_vmxon_off));
+
+	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));
+
+	return !vmx_enabled;
 }
 
 static int test_vmxon(void)
@@ -758,11 +810,13 @@ int main(void)
 	fails = tests = 0;
 	hypercall_field = 0;
 
-	if (test_vmx_capability() != 0) {
-		printf("ERROR : vmx not supported, check +vmx option\n");
+	if (!(cpuid(1).c & (1 << 5))) {
+		printf("WARNING: vmx not supported, add '-cpu host'\n");
 		goto exit;
 	}
 	init_vmx();
+	if (test_vmx_feature_control() != 0)
+		goto exit;
 	/* Set basic test ctxt the same as "null" */
 	current = &vmx_tests[0];
 	if (test_vmxon() != 0)
-- 
1.8.1.1.298.ge7eed54

[PATCH 14/15] VMX: Add test cases around interrupt injection and halting

[Jan Kiszka]

This checks for interrupt delivery to L2, unintercepted hlt in L2 and
explicit L2 suspension via the activity state HLT.

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

diff --git a/x86/vmx.c b/x86/vmx.c
index fe950e6..a475aec 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -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;
diff --git a/x86/vmx.h b/x86/vmx.h
index bc8c86f..3867793 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -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;
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 36c56b4..27ab032 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -9,6 +9,8 @@
 #include "vm.h"
 #include "io.h"
 #include "fwcfg.h"
+#include "isr.h"
+#include "apic.h"
 
 u64 ia32_pat;
 u64 ia32_efer;
@@ -1103,6 +1105,127 @@ static int ept_exit_handler()
 	return VMX_TEST_VMEXIT;
 }
 
+#define TIMER_VECTOR	222
+
+static volatile bool timer_fired;
+
+static void timer_isr(isr_regs_t *regs)
+{
+	timer_fired = true;
+	apic_write(APIC_EOI, 0);
+}
+
+static int interrupt_init(struct vmcs *vmcs)
+{
+	msr_bmp_init();
+	vmcs_write(PIN_CONTROLS, vmcs_read(PIN_CONTROLS) & ~PIN_EXTINT);
+	handle_irq(TIMER_VECTOR, timer_isr);
+	return VMX_TEST_START;
+}
+
+static void interrupt_main(void)
+{
+	long long start;
+
+	set_stage(0);
+
+	apic_write(APIC_LVTT, TIMER_VECTOR);
+	irq_enable();
+
+	apic_write(APIC_TMICT, 1);
+	asm volatile ("nop");
+	report("direct interrupt without delay", timer_fired);
+
+	timer_fired = false;
+	irq_disable();
+	start = rdtsc();
+	apic_write(APIC_TMICT, 1000000);
+
+	asm volatile ("sti; hlt");
+
+	report("direct interrupt + hlt",
+	       (rdtsc() - start > 1000000 && timer_fired));
+
+	vmcall();
+	apic_write(APIC_TMICT, 0);
+	irq_disable();
+	timer_fired = false;
+	start = rdtsc();
+	apic_write(APIC_TMICT, 1000000);
+
+	asm volatile ("sti; hlt");
+
+	report("intercepted interrupt + hlt",
+	       (rdtsc() - start > 10000 && timer_fired));
+
+	apic_write(APIC_TMICT, 0);
+	irq_disable();
+	timer_fired = false;
+	start = rdtsc();
+	apic_write(APIC_TMICT, 1000000);
+
+	irq_enable();
+	asm volatile ("nop");
+	vmcall();
+
+	report("direct interrupt + activity state hlt",
+	       (rdtsc() - start > 10000 && timer_fired && get_stage() == 2));
+
+	vmcall();
+	apic_write(APIC_TMICT, 0);
+	irq_disable();
+	timer_fired = false;
+	start = rdtsc();
+	apic_write(APIC_TMICT, 1000000);
+
+	irq_enable();
+	asm volatile ("nop");
+	vmcall();
+
+	report("intercepted interrupt + activity state hlt",
+	       (rdtsc() - start > 10000 && timer_fired && get_stage() == 4));
+}
+
+static int interrupt_exit_handler(void)
+{
+	u64 guest_rip = vmcs_read(GUEST_RIP);
+	ulong reason = vmcs_read(EXI_REASON) & 0xff;
+	u32 insn_len = vmcs_read(EXI_INST_LEN);
+
+	switch (reason) {
+	case VMX_VMCALL:
+		switch (get_stage()) {
+		case 0:
+		case 2:
+			set_stage(get_stage() + 1);
+			vmcs_write(PIN_CONTROLS,
+				   vmcs_read(PIN_CONTROLS) | PIN_EXTINT);
+			break;
+		case 1:
+		case 3:
+			vmcs_write(GUEST_ACTV_STATE, ACTV_HLT);
+			set_stage(get_stage() + 1);
+			break;
+		}
+		vmcs_write(GUEST_RIP, guest_rip + insn_len);
+		return VMX_TEST_RESUME;
+	case VMX_EXTINT:
+		irq_enable();
+		asm volatile ("nop");
+		irq_disable();
+		vmcs_write(PIN_CONTROLS,
+			   vmcs_read(PIN_CONTROLS) & ~PIN_EXTINT);
+		vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE);
+		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} },
@@ -1120,5 +1243,7 @@ struct vmx_test vmx_tests[] = {
 	{ "instruction intercept", insn_intercept_init, insn_intercept_main,
 		insn_intercept_exit_handler, NULL, {0} },
 	{ "EPT framework", ept_init, ept_main, ept_exit_handler, NULL, {0} },
+	{ "interrupt", interrupt_init, interrupt_main,
+		interrupt_exit_handler, NULL, {0} },
 	{ NULL, NULL, NULL, NULL, NULL, {0} },
 };
-- 
1.8.1.1.298.ge7eed54

[PATCH 15/15] VMX: Add test case for preemption timer firing during hlt

[Jan Kiszka]

This checks that we properly expire the preemption timer also while the
guest is in HLT state.

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

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 27ab032..8952077 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -126,13 +126,18 @@ 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");
+	set_stage(4);
+	vmcall();
 }
 
 int preemption_timer_exit_handler()
@@ -147,33 +152,50 @@ 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:
+			report("preemption timer during hlt",
+			       ((rdtsc() - tsc_val) >> preempt_scale) >=
+			       preempt_val);
+			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);
+				set_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);
 			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);
 			return VMX_TEST_RESUME;
 		case 2:
-			report("Preemption timer", 0);
+			report("busy-wait for preemption timer", 0);
+			return VMX_TEST_RESUME;
+		case 3:
+			report("preemption timer during hlt", 0);
 			break;
 		default:
 			printf("Invalid stage.\n");
-- 
1.8.1.1.298.ge7eed54

Re: [PATCH 04/15] emulator: Exclude test_lgdt_lidt from building

[Paolo Bonzini]

Il 16/12/2013 10:57, Jan Kiszka ha scritto:
> See commit 47c1461a5.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  x86/emulator.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/x86/emulator.c b/x86/emulator.c
> index 68d2b93..4e70e8f 100644
> --- a/x86/emulator.c
> +++ b/x86/emulator.c
> @@ -843,6 +843,7 @@ static void test_string_io_mmio(volatile uint8_t *mem)
>  	report("string_io_mmio", mmio[1023] == 0x99);
>  }
>  
> +/* kvm doesn't allow lidt/lgdt from mmio, so the test is disabled
>  static void test_lgdt_lidt(volatile uint8_t *mem)
>  {
>      struct descriptor_table_ptr orig, fresh = {};
> @@ -871,6 +872,7 @@ static void test_lgdt_lidt(volatile uint8_t *mem)
>      sti();
>      report("lidt (long address)", orig.limit == fresh.limit && orig.base == fresh.base);
>  }
> +*/

I'm changing this to #if 0 ... #endif when applying.

Paolo

Re: [PATCH 06/15] lib/x86/smp: Fix compiler warnings

[Paolo Bonzini]

Il 16/12/2013 10:57, Jan Kiszka ha scritto:
> Add missing include of desc.h for prototypes of setup_idt and
> set_idt_entry and cast away the volatile of ipi_data - it's not volatile
> while we run the IPI handler.

Why not?

Paolo

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  lib/x86/smp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/x86/smp.c b/lib/x86/smp.c
> index d4c8106..75ac081 100644
> --- a/lib/x86/smp.c
> +++ b/lib/x86/smp.c
> @@ -3,6 +3,7 @@
>  #include "smp.h"
>  #include "apic.h"
>  #include "fwcfg.h"
> +#include "desc.h"
>  
>  #define IPI_VECTOR 0x20
>  
> @@ -18,7 +19,7 @@ static int _cpu_count;
>  static __attribute__((used)) void ipi()
>  {
>      void (*function)(void *data) = ipi_function;
> -    void *data = ipi_data;
> +    void *data = (void *)ipi_data;
>      bool wait = ipi_wait;
>  
>      if (!wait) {
> 

Re: [PATCH 06/15] lib/x86/smp: Fix compiler warnings

[Paolo Bonzini]

Il 16/12/2013 10:57, Jan Kiszka ha scritto:
> Add missing include of desc.h for prototypes of setup_idt and
> set_idt_entry and cast away the volatile of ipi_data - it's not volatile
> while we run the IPI handler.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

The right fix is to change the declaration from

  static volatile void *ipi_data;  // Pointer to volatile void

to

  static void *volatile ipi_data;  // Volatile pointer to void

Paolo


> ---
>  lib/x86/smp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/x86/smp.c b/lib/x86/smp.c
> index d4c8106..75ac081 100644
> --- a/lib/x86/smp.c
> +++ b/lib/x86/smp.c
> @@ -3,6 +3,7 @@
>  #include "smp.h"
>  #include "apic.h"
>  #include "fwcfg.h"
> +#include "desc.h"
>  
>  #define IPI_VECTOR 0x20
>  
> @@ -18,7 +19,7 @@ static int _cpu_count;
>  static __attribute__((used)) void ipi()
>  {
>      void (*function)(void *data) = ipi_function;
> -    void *data = ipi_data;
> +    void *data = (void *)ipi_data;
>      bool wait = ipi_wait;
>  
>      if (!wait) {
> 

[PATCH v2 06/15] lib/x86/smp: Fix compiler warnings

[Jan Kiszka]

On 2013-12-17 11:35, Paolo Bonzini wrote:
> Il 16/12/2013 10:57, Jan Kiszka ha scritto:
>> Add missing include of desc.h for prototypes of setup_idt and
>> set_idt_entry and cast away the volatile of ipi_data - it's not volatile
>> while we run the IPI handler.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> The right fix is to change the declaration from
> 
>   static volatile void *ipi_data;  // Pointer to volatile void
> 
> to
> 
>   static void *volatile ipi_data;  // Volatile pointer to void

Indeed, find v2 below.

Jan

---8<---

Add missing include of desc.h for prototypes of setup_idt and
set_idt_entry and adjust type of ipi_data to what it should actually be:
a volatile pointer the a void.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 lib/x86/smp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/x86/smp.c b/lib/x86/smp.c
index d4c8106..1eb49f2 100644
--- a/lib/x86/smp.c
+++ b/lib/x86/smp.c
@@ -3,6 +3,7 @@
 #include "smp.h"
 #include "apic.h"
 #include "fwcfg.h"
+#include "desc.h"

 #define IPI_VECTOR 0x20

@@ -10,7 +11,7 @@ typedef void (*ipi_function_type)(void *data);

 static struct spinlock ipi_lock;
 static volatile ipi_function_type ipi_function;
-static volatile void *ipi_data;
+static void *volatile ipi_data;
 static volatile int ipi_done;
 static volatile bool ipi_wait;
 static int _cpu_count;
-- 
1.8.1.1.298.ge7eed54

Re: [PATCH 00/15] kvm-unit-tests: Fix and enhance nVMX tests

[Paolo Bonzini]

Il 16/12/2013 10:57, Jan Kiszka ha scritto:
> This series contains various smaller fixes and cleanups for the VMX unit
> tests and also a few unrelated corners of the test suite. It ends with a
> rework of the VMX capability test and the addition of a bunch of new
> test cases for interrupt injection, halting and the preemption timer.
> Most of them fail or lockup at the moment. More work is required on KVM
> the fix this, I'm on it.
> 
> These apply on top of the vmx branch. They are also available at
> 
>     git://git.kiszka.org/kvm-unit-tests queue

I finally merged the vmx branch into master, and applied 1-13 on top.

I'm in the process of installing newer hardware, and for now I'm stuck
with an old Core 2.  So I'm holding up patches 14-15 until after I get
the new machine; I pushed them to the vmx branch.

Paolo

Re: [PATCH 14/15] VMX: Add test cases around interrupt injection and halting

[Paolo Bonzini]

Il 16/12/2013 10:57, Jan Kiszka ha scritto:
> This checks for interrupt delivery to L2, unintercepted hlt in L2 and
> explicit L2 suspension via the activity state HLT.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

I'm applying this patch and the next one.  However, I'm seeing one 
failure still:

Test suite : interrupt
PASS: direct interrupt while running guest
PASS: intercepted interrupt while running guest
PASS: direct interrupt + hlt
FAIL: intercepted interrupt + hlt
PASS: direct interrupt + activity state hlt
PASS: intercepted interrupt + activity state hlt

I'll look at it in the next few days.

Paolo

Re: [PATCH 14/15] VMX: Add test cases around interrupt injection and halting

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

On 2014-03-25 12:18, Paolo Bonzini wrote:
> Il 16/12/2013 10:57, Jan Kiszka ha scritto:
>> This checks for interrupt delivery to L2, unintercepted hlt in L2 and
>> explicit L2 suspension via the activity state HLT.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> I'm applying this patch and the next one.  However, I'm seeing one
> failure still:
> 
> Test suite : interrupt
> PASS: direct interrupt while running guest
> PASS: intercepted interrupt while running guest
> PASS: direct interrupt + hlt
> FAIL: intercepted interrupt + hlt
> PASS: direct interrupt + activity state hlt
> PASS: intercepted interrupt + activity state hlt
> 
> I'll look at it in the next few days.

Some of the tests were timing sensitive, though I do not remember which
one. Will recheck locally as well if I also see the issue.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]