* [PATCH kvm-unit-tests 3/4] x86: vmx: clean up vm{launch,resume} asm
2016-03-18 22:39 [PATCH kvm-unit-tests 0/4] x86: vmx: vm{launch,resume} improvements Peter Feiner
2016-03-18 22:39 ` [PATCH kvm-unit-tests 1/4] x86: vmx: fix vm{launch,resume} asm Peter Feiner
2016-03-18 22:39 ` [PATCH kvm-unit-tests 2/4] x86: vmx: fix vm{launch,resume} early exit logic Peter Feiner
@ 2016-03-18 22:39 ` Peter Feiner
2016-03-18 22:39 ` [PATCH kvm-unit-tests 4/4] x86: vmx: better vm{launch,resume} error handling Peter Feiner
2016-03-19 9:23 ` [PATCH kvm-unit-tests 0/4] x86: vmx: vm{launch,resume} improvements Paolo Bonzini
4 siblings, 0 replies; 8+ messages in thread
From: Peter Feiner @ 2016-03-18 22:39 UTC (permalink / raw)
To: jan.kiszka, pbonzini, kvm; +Cc: pfeiner
Did a few things to make the code easier to fllow:
* named I/O operands
* removed pointless LOAD_RFLAGS
* replaced global host_rflags with local
* only save rflags when they're meaningful (i.e., after early failure)
* got rid of unecessary %rsi scratch register
Signed-off-by: Peter Feiner <pfeiner@google.com>
---
x86/vmx.c | 23 +++++++++++------------
x86/vmx.h | 8 --------
2 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/x86/vmx.c b/x86/vmx.c
index 27e85eb..67f8650 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -46,7 +46,6 @@ struct regs regs;
struct vmx_test *current;
u64 hypercall_field;
bool launched;
-u64 host_rflags;
union vmx_basic basic;
union vmx_ctrl_msr ctrl_pin_rev;
@@ -909,33 +908,33 @@ static int exit_handler()
static int vmx_run()
{
u32 ret = 0, fail = 0;
+ unsigned long host_rflags;
while (1) {
asm volatile (
- "mov %%rsp, %%rsi\n\t"
- "mov %2, %%rdi\n\t"
- "vmwrite %%rsi, %%rdi\n\t"
-
+ "mov %[HOST_RSP], %%rdi\n\t"
+ "vmwrite %%rsp, %%rdi\n\t"
LOAD_GPR_C
- "cmpl $0, %1\n\t"
+ "cmpl $0, %[launched]\n\t"
"jne 1f\n\t"
- LOAD_RFLAGS
"vmlaunch\n\t"
"jmp 2f\n\t"
"1: "
"vmresume\n\t"
"2: "
SAVE_GPR_C
- "movl $1, %0\n\t"
+ "pushf\n\t"
+ "pop %%rdi\n\t"
+ "mov %%rdi, %[host_rflags]\n\t"
+ "movl $1, %[fail]\n\t"
"jmp 3f\n\t"
"vmx_return:\n\t"
SAVE_GPR_C
"3: \n\t"
- SAVE_RFLAGS
- : "+m"(fail)
- : "m"(launched), "i"(HOST_RSP)
- : "rdi", "rsi", "memory", "cc"
+ : [fail]"+m"(fail), [host_rflags]"=m"(host_rflags)
+ : [launched]"m"(launched), [HOST_RSP]"i"(HOST_RSP)
+ : "rdi", "memory", "cc"
);
if (fail)
diff --git a/x86/vmx.h b/x86/vmx.h
index 34e9be4..aba5642 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -396,14 +396,6 @@ enum Ctrl1 {
#define LOAD_GPR_C SAVE_GPR_C
-#define SAVE_RFLAGS \
- "pushf\n\t" \
- "pop host_rflags\n\t"
-
-#define LOAD_RFLAGS \
- "push host_rflags\n\t" \
- "popf\n\t"
-
#define VMX_IO_SIZE_MASK 0x7
#define _VMX_IO_BYTE 0
#define _VMX_IO_WORD 1
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH kvm-unit-tests 4/4] x86: vmx: better vm{launch,resume} error handling
2016-03-18 22:39 [PATCH kvm-unit-tests 0/4] x86: vmx: vm{launch,resume} improvements Peter Feiner
` (2 preceding siblings ...)
2016-03-18 22:39 ` [PATCH kvm-unit-tests 3/4] x86: vmx: clean up vm{launch,resume} asm Peter Feiner
@ 2016-03-18 22:39 ` Peter Feiner
2016-03-19 9:23 ` [PATCH kvm-unit-tests 0/4] x86: vmx: vm{launch,resume} improvements Paolo Bonzini
4 siblings, 0 replies; 8+ messages in thread
From: Peter Feiner @ 2016-03-18 22:39 UTC (permalink / raw)
To: jan.kiszka, pbonzini, kvm; +Cc: pfeiner
This patch splits out entry error handling from other exit handling
since most tests don't expect entry errors and thus don't check the
conditions properly. Also enables tests for early entry errors (i.e.,
an entry_error_handler can return VMX_TEST_RESUME).
Consolidates and simplifies control flow. Now, vmx_run is the central
validation point for exit handler statuses rather than splitting the
responsibility between exit_handler and vmx_run.
Signed-off-by: Peter Feiner <pfeiner@google.com>
---
lib/x86/processor.h | 4 ++
x86/vmx.c | 142 ++++++++++++++++++++++++++++++++++++----------------
x86/vmx.h | 19 ++++++-
x86/vmx_tests.c | 49 +++++++++++-------
4 files changed, 151 insertions(+), 63 deletions(-)
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index ce779d1..ee7f180 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -31,7 +31,11 @@
#define X86_CR4_PKE 0x00400000
#define X86_EFLAGS_CF 0x00000001
+#define X86_EFLAGS_PF 0x00000004
+#define X86_EFLAGS_AF 0x00000010
#define X86_EFLAGS_ZF 0x00000040
+#define X86_EFLAGS_SF 0x00000080
+#define X86_EFLAGS_OF 0x00000800
#define X86_EFLAGS_AC 0x00040000
#define X86_IA32_EFER 0xc0000080
diff --git a/x86/vmx.c b/x86/vmx.c
index 67f8650..6618008 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -157,6 +157,52 @@ void print_vmexit_info()
regs.r12, regs.r13, regs.r14, regs.r15);
}
+void
+print_vmentry_failure_info(struct vmentry_failure *failure) {
+ if (failure->early) {
+ printf("Early %s failure: ", failure->instr);
+ switch (failure->flags & VMX_ENTRY_FLAGS) {
+ case X86_EFLAGS_ZF:
+ printf("current-VMCS pointer is not valid.\n");
+ break;
+ case X86_EFLAGS_CF:
+ printf("error number is %ld. See Intel 30.4.\n",
+ vmcs_read(VMX_INST_ERROR));
+ break;
+ default:
+ printf("unexpected flags %lx!\n", failure->flags);
+ }
+ } else {
+ u64 reason = vmcs_read(EXI_REASON);
+ u64 qual = vmcs_read(EXI_QUALIFICATION);
+
+ printf("Non-early %s failure (reason=0x%lx, qual=0x%lx): ",
+ failure->instr, reason, qual);
+
+ switch (reason & 0xff) {
+ case VMX_FAIL_STATE:
+ printf("invalid guest state\n");
+ break;
+ case VMX_FAIL_MSR:
+ printf("MSR loading\n");
+ break;
+ case VMX_FAIL_MCHECK:
+ printf("machine-check event\n");
+ break;
+ default:
+ printf("unexpected basic exit reason %ld\n",
+ reason & 0xff);
+ }
+
+ if (!(reason & VMX_ENTRY_FAILURE))
+ printf("\tVMX_ENTRY_FAILURE BIT NOT SET!\n");
+
+ if (reason & 0x7fff0000)
+ printf("\tRESERVED BITS SET!\n");
+ }
+}
+
+
static void test_vmclear(void)
{
struct vmcs *tmp_root;
@@ -890,27 +936,34 @@ static int exit_handler()
else
ret = current->exit_handler();
vmcs_write(GUEST_RFLAGS, regs.rflags);
- switch (ret) {
- case VMX_TEST_VMEXIT:
- case VMX_TEST_RESUME:
- return ret;
- case VMX_TEST_EXIT:
- break;
- default:
- printf("ERROR : Invalid exit_handler return val %d.\n"
- , ret);
- }
- print_vmexit_info();
- abort();
- return 0;
+
+ return ret;
+}
+
+/*
+ * Called if vmlaunch or vmresume fails.
+ * @early - failure due to "VMX controls and host-state area" (26.2)
+ * @vmlaunch - was this a vmlaunch or vmresume
+ * @rflags - host rflags
+ */
+static int
+entry_failure_handler(struct vmentry_failure *failure)
+{
+ if (current->entry_failure_handler)
+ return current->entry_failure_handler(failure);
+ else
+ return VMX_TEST_EXIT;
}
static int vmx_run()
{
- u32 ret = 0, fail = 0;
unsigned long host_rflags;
while (1) {
+ u32 ret;
+ u32 fail = 0;
+ bool entered;
+ struct vmentry_failure failure;
asm volatile (
"mov %[HOST_RSP], %%rdi\n\t"
@@ -937,39 +990,44 @@ static int vmx_run()
: "rdi", "memory", "cc"
);
- if (fail)
- ret = launched ? VMX_TEST_RESUME_ERR :
- VMX_TEST_LAUNCH_ERR;
- else {
+
+ entered = !fail && !(vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE);
+
+ if (entered) {
+ /*
+ * VMCS isn't in "launched" state if there's been any
+ * entry failure (early or otherwise).
+ */
launched = 1;
ret = exit_handler();
+ } else {
+ failure.flags = host_rflags;
+ failure.vmlaunch = !launched;
+ failure.instr = launched ? "vmresume" : "vmlaunch";
+ failure.early = fail;
+ ret = entry_failure_handler(&failure);
}
- if (ret != VMX_TEST_RESUME)
+
+ switch (ret) {
+ case VMX_TEST_RESUME:
+ continue;
+ case VMX_TEST_VMEXIT:
+ return 0;
+ case VMX_TEST_EXIT:
break;
+ default:
+ printf("ERROR : Invalid %s_handler return val %d.\n",
+ entered ? "exit" : "entry_failure",
+ ret);
+ break;
+ }
+
+ if (entered)
+ print_vmexit_info();
+ else
+ print_vmentry_failure_info(&failure);
+ abort();
}
- launched = 0;
- switch (ret) {
- case VMX_TEST_VMEXIT:
- return 0;
- case VMX_TEST_LAUNCH_ERR:
- printf("%s : vmlaunch failed.\n", __func__);
- if ((!(host_rflags & X86_EFLAGS_CF) && !(host_rflags & X86_EFLAGS_ZF))
- || ((host_rflags & X86_EFLAGS_CF) && (host_rflags & X86_EFLAGS_ZF)))
- printf("\tvmlaunch set wrong flags\n");
- report("test vmlaunch", 0);
- break;
- case VMX_TEST_RESUME_ERR:
- printf("%s : vmresume failed.\n", __func__);
- if ((!(host_rflags & X86_EFLAGS_CF) && !(host_rflags & X86_EFLAGS_ZF))
- || ((host_rflags & X86_EFLAGS_CF) && (host_rflags & X86_EFLAGS_ZF)))
- printf("\tvmresume set wrong flags\n");
- report("test vmresume", 0);
- break;
- default:
- printf("%s : unhandled ret from exit_handler, ret=%d.\n", __func__, ret);
- break;
- }
- return 1;
}
static int test_run(struct vmx_test *test)
diff --git a/x86/vmx.h b/x86/vmx.h
index aba5642..0cb995d 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -32,6 +32,17 @@ struct regs {
u64 rflags;
};
+struct vmentry_failure {
+ /* Did a vmlaunch or vmresume fail? */
+ bool vmlaunch;
+ /* Instruction mnemonic (for convenience). */
+ const char *instr;
+ /* Did the instruction return right away, or did we jump to HOST_RIP? */
+ bool early;
+ /* Contents of [re]flags after failed entry. */
+ unsigned long flags;
+};
+
struct vmx_test {
const char *name;
int (*init)(struct vmcs *vmcs);
@@ -39,6 +50,7 @@ struct vmx_test {
int (*exit_handler)();
void (*syscall_handler)(u64 syscall_no);
struct regs guest_regs;
+ int (*entry_failure_handler)(struct vmentry_failure *failure);
struct vmcs *vmcs;
int exits;
};
@@ -249,6 +261,10 @@ enum Encoding {
HOST_RIP = 0x6c16ul
};
+#define VMX_ENTRY_FAILURE (1ul << 31)
+#define VMX_ENTRY_FLAGS (X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF | \
+ X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF)
+
enum Reason {
VMX_EXC_NMI = 0,
VMX_EXTINT = 1,
@@ -413,8 +429,6 @@ enum Ctrl1 {
#define VMX_TEST_VMEXIT 1
#define VMX_TEST_EXIT 2
#define VMX_TEST_RESUME 3
-#define VMX_TEST_LAUNCH_ERR 4
-#define VMX_TEST_RESUME_ERR 5
#define HYPERCALL_BIT (1ul << 12)
#define HYPERCALL_MASK 0xFFF
@@ -554,6 +568,7 @@ static inline void invvpid(unsigned long type, u16 vpid, u64 gva)
}
void print_vmexit_info();
+void print_vmentry_failure_info(struct vmentry_failure *failure);
void ept_sync(int type, u64 eptp);
void vpid_sync(int type, u16 vpid);
void install_ept_entry(unsigned long *pml4, int pte_level,
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 1ed0b0c..0145cad 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1523,24 +1523,35 @@ static int msr_switch_exit_handler()
ulong reason;
reason = vmcs_read(EXI_REASON);
- switch (reason) {
- case 0x80000000 | VMX_FAIL_MSR:
- if (vmx_get_test_stage() == 3) {
- report("VM entry MSR load: try to load FS_BASE",
- vmcs_read(EXI_QUALIFICATION) == 1);
- return VMX_TEST_VMEXIT;
- }
- break;
- case VMX_VMCALL:
- if (vmx_get_test_stage() == 2) {
- report("VM exit MSR store",
- exit_msr_store[0].value == MSR_MAGIC + 1);
- report("VM exit MSR load",
- rdmsr(MSR_KERNEL_GS_BASE) == MSR_MAGIC + 2);
- vmx_set_test_stage(3);
- entry_msr_load[0].index = MSR_FS_BASE;
- return VMX_TEST_RESUME;
- }
+ if (reason == VMX_VMCALL && vmx_get_test_stage() == 2) {
+ report("VM exit MSR store",
+ exit_msr_store[0].value == MSR_MAGIC + 1);
+ report("VM exit MSR load",
+ rdmsr(MSR_KERNEL_GS_BASE) == MSR_MAGIC + 2);
+ vmx_set_test_stage(3);
+ entry_msr_load[0].index = MSR_FS_BASE;
+ return VMX_TEST_RESUME;
+ }
+ printf("ERROR %s: unexpected stage=%u or reason=%lu\n",
+ __func__, vmx_get_test_stage(), reason);
+ return VMX_TEST_EXIT;
+}
+
+static int msr_switch_entry_failure(struct vmentry_failure *failure)
+{
+ ulong reason;
+
+ if (failure->early) {
+ printf("ERROR %s: early exit\n", __func__);
+ return VMX_TEST_EXIT;
+ }
+
+ reason = vmcs_read(EXI_REASON);
+ if (reason == (VMX_ENTRY_FAILURE | VMX_FAIL_MSR) &&
+ vmx_get_test_stage() == 3) {
+ report("VM entry MSR load: try to load FS_BASE",
+ vmcs_read(EXI_QUALIFICATION) == 1);
+ return VMX_TEST_VMEXIT;
}
printf("ERROR %s: unexpected stage=%u or reason=%lu\n",
__func__, vmx_get_test_stage(), reason);
@@ -1608,7 +1619,7 @@ struct vmx_test vmx_tests[] = {
{ "debug controls", dbgctls_init, dbgctls_main, dbgctls_exit_handler,
NULL, {0} },
{ "MSR switch", msr_switch_init, msr_switch_main,
- msr_switch_exit_handler, NULL, {0} },
+ msr_switch_exit_handler, NULL, {0}, msr_switch_entry_failure },
{ "vmmcall", vmmcall_init, vmmcall_main, vmmcall_exit_handler, NULL, {0} },
{ NULL, NULL, NULL, NULL, NULL, {0} },
};
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 8+ messages in thread