Message ID | 20210204232951.104755-2-krish.sadhukhan@oracle.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | nSVM: Test host RFLAGS.TF on VMRUN | expand |
On 05/02/21 00:29, Krish Sadhukhan wrote: > > +static void host_rflags_prepare(struct svm_test *test) > +{ > + default_prepare(test); > + handle_exception(DB_VECTOR, host_rflags_db_handler); > + set_test_stage(test, 0); > + /* > + * We trigger a #UD in order to find out the RIP of VMRUN instruction > + */ > + wrmsr(MSR_EFER, rdmsr(MSR_EFER) & ~EFER_SVME); > + handle_exception(UD_VECTOR, host_rflags_ud_handler); > +} > + I think you'd get the RIP of VMLOAD, not VMRUN. Maybe something like: diff --git a/x86/svm.c b/x86/svm.c index a1808c7..88d8452 100644 --- a/x86/svm.c +++ b/x86/svm.c @@ -208,14 +208,14 @@ struct regs get_regs(void) struct svm_test *v2_test; -#define ASM_VMRUN_CMD \ +#define ASM_PRE_VMRUN \ "vmload %%rax\n\t" \ "mov regs+0x80, %%r15\n\t" \ "mov %%r15, 0x170(%%rax)\n\t" \ "mov regs, %%r15\n\t" \ "mov %%r15, 0x1f8(%%rax)\n\t" \ - LOAD_GPR_C \ - "vmrun %%rax\n\t" \ + LOAD_GPR_C +#define ASM_POST_VMRUN \ SAVE_GPR_C \ "mov 0x170(%%rax), %%r15\n\t" \ "mov %%r15, regs+0x80\n\t" \ @@ -232,7 +232,9 @@ int svm_vmrun(void) regs.rdi = (ulong)v2_test; asm volatile ( - ASM_VMRUN_CMD + ASM_PRE_VMRUN + "vmrun %%rax\n\t" + ASM_POST_VMRUN : : "a" (virt_to_phys(vmcb)) : "memory", "r15"); @@ -240,6 +242,7 @@ int svm_vmrun(void) return (vmcb->control.exit_code); } +extern unsigned char vmrun_rip; static void test_run(struct svm_test *test) { u64 vmcb_phys = virt_to_phys(vmcb); @@ -258,7 +261,9 @@ static void test_run(struct svm_test *test) "sti \n\t" "call *%c[PREPARE_GIF_CLEAR](%[test]) \n \t" "mov %[vmcb_phys], %%rax \n\t" - ASM_VMRUN_CMD + ASM_PRE_VMRUN + "vmrun_rip: vmrun %%rax\n\t" + ASM_POST_VMRUN "cli \n\t" "stgi" : // inputs clobbered by the guest: (untested) Paolo
On 2/5/21 12:14 AM, Paolo Bonzini wrote: > On 05/02/21 00:29, Krish Sadhukhan wrote: >> >> +static void host_rflags_prepare(struct svm_test *test) >> +{ >> + default_prepare(test); >> + handle_exception(DB_VECTOR, host_rflags_db_handler); >> + set_test_stage(test, 0); >> + /* >> + * We trigger a #UD in order to find out the RIP of VMRUN >> instruction >> + */ >> + wrmsr(MSR_EFER, rdmsr(MSR_EFER) & ~EFER_SVME); >> + handle_exception(UD_VECTOR, host_rflags_ud_handler); >> +} >> + > > I think you'd get the RIP of VMLOAD, not VMRUN. > > Maybe something like: > > diff --git a/x86/svm.c b/x86/svm.c > index a1808c7..88d8452 100644 > --- a/x86/svm.c > +++ b/x86/svm.c > @@ -208,14 +208,14 @@ struct regs get_regs(void) > > struct svm_test *v2_test; > > -#define ASM_VMRUN_CMD \ > +#define ASM_PRE_VMRUN \ > "vmload %%rax\n\t" \ > "mov regs+0x80, %%r15\n\t" \ > "mov %%r15, 0x170(%%rax)\n\t" \ > "mov regs, %%r15\n\t" \ > "mov %%r15, 0x1f8(%%rax)\n\t" \ > - LOAD_GPR_C \ > - "vmrun %%rax\n\t" \ > + LOAD_GPR_C > +#define ASM_POST_VMRUN \ > SAVE_GPR_C \ > "mov 0x170(%%rax), %%r15\n\t" \ > "mov %%r15, regs+0x80\n\t" \ > @@ -232,7 +232,9 @@ int svm_vmrun(void) > regs.rdi = (ulong)v2_test; > > asm volatile ( > - ASM_VMRUN_CMD > + ASM_PRE_VMRUN > + "vmrun %%rax\n\t" > + ASM_POST_VMRUN > : > : "a" (virt_to_phys(vmcb)) > : "memory", "r15"); > @@ -240,6 +242,7 @@ int svm_vmrun(void) > return (vmcb->control.exit_code); > } > > +extern unsigned char vmrun_rip; > static void test_run(struct svm_test *test) > { > u64 vmcb_phys = virt_to_phys(vmcb); > @@ -258,7 +261,9 @@ static void test_run(struct svm_test *test) > "sti \n\t" > "call *%c[PREPARE_GIF_CLEAR](%[test]) \n \t" > "mov %[vmcb_phys], %%rax \n\t" > - ASM_VMRUN_CMD > + ASM_PRE_VMRUN > + "vmrun_rip: vmrun %%rax\n\t" > + ASM_POST_VMRUN > "cli \n\t" > "stgi" > : // inputs clobbered by the guest: > > (untested) > > Paolo > Thanks for the suggestion. I have implemented it in v3 that I have sent out. The reason why my test (v2) had passed, was because virtual VMLOAD/VMSAVE is enabled by default and no #VMEXIT happens due to intercepts being disabled (via init_vmcb()). So my #UD handler didn't get invoked on VMLOAD but got invoked on VMRUN. This brings out an interesting point. When intercept for VMLOAD/VMSAVE is disabled, there is no effect of unsetting EFER.SVME on VMLOAD/VMLOAD, even thought APM vol 2 says it should generate a #UD. I couldn't find any text in the APM that describes the effect of unsetting EFER.SVME on the virtual VMLOAD/VMSAVE. Is this the expected behavior of the SVM hardware or is this a bug in KVM and KVM should handle this ? I plan to add some more tests based on the correct behavior of virtual VMLOAD/VMSAVE.
On 23/02/21 21:28, Krish Sadhukhan wrote: > I couldn't find any text in the APM that describes the effect of > unsetting EFER.SVME on the virtual VMLOAD/VMSAVE. Is this the expected > behavior of the SVM hardware or is this a bug in KVM and KVM should > handle this ? The guest always runs with EFER.SVME=1 (it's part of the VMRUN requirements), so yeah, vVMLOAD/VMSAVE must be enabled only if the guest has EFER.SVME=1. Nice! Paolo
diff --git a/x86/svm_tests.c b/x86/svm_tests.c index 29a0b59..def5880 100644 --- a/x86/svm_tests.c +++ b/x86/svm_tests.c @@ -2002,6 +2002,132 @@ static bool init_intercept_check(struct svm_test *test) return init_intercept; } +/* + * Setting host EFLAGS.TF causes a #DB trap after the VMRUN completes on the + * host side (i.e., after the #VMEXIT from the guest). + * + * [AMD APM] + */ +static volatile u8 host_rflags_guest_main_flag = 0; +static volatile u8 host_rflags_db_handler_flag = 0; +static volatile bool host_rflags_ss_on_vmrun = false; +static u64 vmrun_rip; +static volatile bool host_rflags_vmrun_reached = false; +static volatile bool host_rflags_set_tf = false; + +static void host_rflags_ud_handler(struct ex_regs *r) +{ + vmrun_rip = r->rip; + wrmsr(MSR_EFER, rdmsr(MSR_EFER) | EFER_SVME); +} + +static void host_rflags_db_handler(struct ex_regs *r) +{ + if (host_rflags_ss_on_vmrun) { + if (host_rflags_vmrun_reached) { + host_rflags_db_handler_flag = + (host_rflags_guest_main_flag == 1) ? 2 : 1; + r->rflags &= ~X86_EFLAGS_TF; + } else { + if (r->rip == vmrun_rip) { + host_rflags_guest_main_flag = + host_rflags_db_handler_flag = 0; + host_rflags_vmrun_reached = true; + } + } + } else { + host_rflags_db_handler_flag = + (host_rflags_guest_main_flag == 1) ? 2 : 1; + r->rflags &= ~X86_EFLAGS_TF; + } +} + +static void host_rflags_prepare(struct svm_test *test) +{ + default_prepare(test); + handle_exception(DB_VECTOR, host_rflags_db_handler); + set_test_stage(test, 0); + /* + * We trigger a #UD in order to find out the RIP of VMRUN instruction + */ + wrmsr(MSR_EFER, rdmsr(MSR_EFER) & ~EFER_SVME); + handle_exception(UD_VECTOR, host_rflags_ud_handler); +} + +static void host_rflags_prepare_gif_clear(struct svm_test *test) +{ + if (host_rflags_set_tf) { + write_rflags(read_rflags() | X86_EFLAGS_TF); + } +} + +static void host_rflags_test(struct svm_test *test) +{ + while (1) { + if (get_test_stage(test) > 0) + host_rflags_guest_main_flag = + (host_rflags_db_handler_flag == 1) ? 2 : 1; + vmmcall(); + if (get_test_stage(test) == 3) + break; + } +} + +static bool host_rflags_finished(struct svm_test *test) +{ + switch (get_test_stage(test)) { + case 0: + if (vmcb->control.exit_code != SVM_EXIT_VMMCALL && + host_rflags_db_handler_flag != 0 && host_rflags_guest_main_flag != 0) { + report(false, "Unexpected VMEXIT. Exit reason 0x%x", + vmcb->control.exit_code); + return true; + } + vmcb->save.rip += 3; + /* + * Setting host EFLAGS.TF not immediately before VMRUN, causes + * #DB trap before first guest instruction is executed + */ + host_rflags_set_tf = true; + break; + case 1: + if (vmcb->control.exit_code != SVM_EXIT_VMMCALL && + host_rflags_db_handler_flag != 1 && host_rflags_guest_main_flag != 2) { + report(false, "Unexpected VMEXIT. Exit reason 0x%x", + vmcb->control.exit_code); + return true; + } + host_rflags_guest_main_flag = host_rflags_db_handler_flag = 0; + vmcb->save.rip += 3; + /* + * Setting host EFLAGS.TF immediately before VMRUN, causes #DB + * trap after VMRUN completes on the host side (i.e., after + * VMEXIT from guest). + */ + host_rflags_ss_on_vmrun = true; + break; + case 2: + if (vmcb->control.exit_code != SVM_EXIT_VMMCALL && + host_rflags_db_handler_flag != 2 && host_rflags_guest_main_flag != 1) { + report(false, "Unexpected VMEXIT. Exit reason 0x%x", + vmcb->control.exit_code); + return true; + } + host_rflags_set_tf = false; + vmcb->save.rip += 3; + break; + default: + return true; + } + inc_test_stage(test); + return get_test_stage(test) == 4; +} + +static bool host_rflags_check(struct svm_test *test) +{ + return get_test_stage(test) == 3; +} + #define TEST(name) { #name, .v2 = name } /* @@ -2492,6 +2618,9 @@ struct svm_test svm_tests[] = { { "svm_init_intercept_test", smp_supported, init_intercept_prepare, default_prepare_gif_clear, init_intercept_test, init_intercept_finished, init_intercept_check, .on_vcpu = 2 }, + { "host_rflags", default_supported, host_rflags_prepare, + host_rflags_prepare_gif_clear, host_rflags_test, + host_rflags_finished, host_rflags_check }, TEST(svm_cr4_osxsave_test), TEST(svm_guest_state_test), TEST(svm_vmrun_errata_test),
According to section "VMRUN and TF/RF Bits in EFLAGS" in AMD APM vol 2, "From the host point of view, VMRUN acts like a single instruction, even though an arbitrary number of guest instructions may execute before a #VMEXIT effectively completes the VMRUN. As a single host instruction, VMRUN interacts with EFLAGS.TF like ordinary instructions. EFLAGS.TF causes a #DB trap after the VMRUN completes on the host side (i.e., after the #VMEXIT from the guest). Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> --- x86/svm_tests.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 129 insertions(+)