x86: vmx: vm{launch,resume} improvements
cover x86: vmx: vm{launch,resume} improvements
Fixed some bugs in the vmlaunch / vmresume code and improved entry failure logic.
Peter Feiner (4):
x86: vmx: fix vm{launch,resume} asm
x86: vmx: fix vm{launch,resume} early exit logic
x86: vmx: clean up vm{launch,resume} asm
x86: vmx: better vm{launch,resume} error handling
lib/x86/processor.h | 4 ++ x86/vmx.c | 169 +++++++++++++++++++++++++++++++++++----------------- x86/vmx.h | 27 +++++---- x86/vmx_tests.c | 49 +++++++++------ 4 files changed, 166 insertions(+), 83 deletions(-)
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 1/4 x86: vmx: fix vm{launch,resume} asm
Fixed two problems:
1) Output operands with the '=' constraint are dead-on-arrival. Thus gcc was free to ignore the fail = 0 initialization. Since the asm only set fail when vm{launch,resume} exited early, fail was technically undefined when vm{launch,resume} entered the guest! Using the '+' constraint instead tells gcc the value is live.
2) On early vm{launch,resume} failure, the 'setbe %0' instruction was running with the guest's GPRs since SAVE_GPR_C hadn't run yet. Since %0 is typically replaced with OFFSET(%%rbp), some arbitrary guest stack memory is modified. Solution is to restore the host's registers before using any asm code generated by gcc.
--- x86/vmx.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
x86/vmx.c +4 -1
@@ -925,11 +925,14 @@ static int vmx_run()"1: " "vmresume\n\t" "2: " +SAVE_GPR_C "setbe %0\n\t" +"jmp 3f\n\t" "vmx_return:\n\t" SAVE_GPR_C +"3: \n\t" SAVE_RFLAGS - : "=m"(fail) + : "+m"(fail) : "m"(launched), "i"(HOST_RSP) : "rdi", "rsi", "memory", "cc"
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 2/4 x86: vmx: fix vm{launch,resume} early exit logic
In reply to Peter FeinerIf vmlaunch or vmresume returns immediately, there's definitely been an error -- rflags only needs to be consulted to determine what error occurred. By setting fail=1 unconditionally, the test framework will now detect problems with rflags *not* being set properly.
--- x86/vmx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
x86/vmx.c +2 -1
@@ -911,6 +911,7 @@ static int vmx_run()u32 ret =0 , fail =0 ;while (1 ) { + asmvolatile ("mov %%rsp, %%rsi\n\t" "mov %2, %%rdi\n\t"
@@ -926,7 +927,7 @@ static int vmx_run()"vmresume\n\t" "2: " SAVE_GPR_C -"setbe %0\n\t" +"movl $1, %0\n\t" "jmp 3f\n\t" "vmx_return:\n\t" SAVE_GPR_C
Signature · 1 line
2.8.0.rc3.226.g39d4020
Paolo Bonzini
maintainerRe: [PATCH kvm-unit-tests 2/4] x86: vmx: fix vm{launch,resume} early exit logic
Quoted 4 lines · Peter Feiner
If vmlaunch or vmresume returns immediately, there's definitely been an error -- rflags only needs to be consulted to determine what error occurred. By setting fail=1 unconditionally, the test framework will now detect problems with rflags *not* being set properly.
You should still do the setbe, and follow it with orl $2, %0. Then if fail==2 there is a problem.
Paolo
Quoted 27 lines
Signed-off-by Peter Feiner <[email protected]>--- x86/vmx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)x86/vmx.c +2
-1@@ -911,6 +911,7 @@ static int vmx_run()u32 ret =0 , fail =0 ;while (1 ) { + asmvolatile ("mov %%rsp, %%rsi\n\t" "mov %2, %%rdi\n\t"@@ -926,7 +927,7 @@ static int vmx_run()"vmresume\n\t" "2: " SAVE_GPR_C -"setbe %0\n\t" +"movl $1, %0\n\t" "jmp 3f\n\t" "vmx_return:\n\t" SAVE_GPR_C
Paolo Bonzini
maintainerRe: [PATCH kvm-unit-tests 2/4] x86: vmx: fix vm{launch,resume} early exit logic
Quoted 10 lines · Paolo Bonzini
Quoted 4 lines · Peter Feiner
If vmlaunch or vmresume returns immediately, there's definitely been an error -- rflags only needs to be consulted to determine what error occurred. By setting fail=1 unconditionally, the test framework will now detect problems with rflags *not* being set properly.
You should still do the setbe, and follow it with orl $2, %0. Then if fail==2 there is a problem.
Nevermind, this is checked already below.
Paolo
PATCH 3/4 x86: vmx: clean up vm{launch,resume} asm
In reply to Peter FeinerDid 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
--- x86/vmx.c | 23 +++++++++++------------ x86/vmx.h | 8 -------- 2 files changed, 11 insertions(+), 20 deletions(-)
x86/vmx.c +11 -12
@@ -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 ) { asmvolatile ( -"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)
x86/vmx.h +0 -8
@@ -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
Signature · 1 line
2.8.0.rc3.226.g39d4020
PATCH 4/4 x86: vmx: better vm{launch,resume} error handling
In reply to Peter FeinerThis 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.
--- 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(-)
lib/x86/processor.h +4 -0
@@ -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
x86/vmx.c +100 -42
172 lines not on this page · raw message
x86/vmx.h +17 -2
@@ -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,
x86/vmx_tests.c +30 -19
61 lines not on this page · raw message
Signature · 1 line
2.8.0.rc3.226.g39d4020
Quoted 9 lines · Peter Feiner
Fixed some bugs in the vmlaunch / vmresume code and improved entry failure logic.
Peter Feiner (4):
x86: vmx: fix vm{launch,resume} asm
x86: vmx: fix vm{launch,resume} early exit logic
x86: vmx: clean up vm{launch,resume} asm
x86: vmx: better vm{launch,resume} error handling
Quoted 6 lines
lib/x86/processor.h | 4 ++ x86/vmx.c | 169 +++++++++++++++++++++++++++++++++++----------------- x86/vmx.h | 27 +++++---- x86/vmx_tests.c | 49 +++++++++------ 4 files changed, 166 insertions(+), 83 deletions(-)
Thanks, applied.
Paolo