| Message ID | 20210722131718.11667-1-laramglazier@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests] nSVM: Added test for VGIF feature | expand |
Nit, s/Added/Add in the shortlog On Thu, Jul 22, 2021, Lara Lazier wrote: > When VGIF is enabled STGI executed in guest mode > sets bit 9, while CLGI clears bit 9 in the int_ctl (offset 60h) > of the VMCB. > > Signed-off-by: Lara Lazier <laramglazier@gmail.com> > --- ... > --- a/x86/svm_tests.c > +++ b/x86/svm_tests.c > @@ -2772,6 +2772,73 @@ static void svm_vmload_vmsave(void) > vmcb->control.intercept = intercept_saved; > } > > +static void prepare_vgif_enabled(struct svm_test *test) > +{ > + default_prepare(test); > +} > + > +static void test_vgif(struct svm_test *test) > +{ > + asm volatile ("vmmcall\n\tstgi\n\tvmmcall\n\tclgi\n\tvmmcall\n\t"); While amusing, this isn't very readable :-) The SVM tests that use this for back-to-back VMMCALL are setting a bad example. The space between "volatile" and the opening "(" can go too. asm volatile("vmmcall\n\t" "stgi\n\t" "vmmcall\n\t" "clgi\n\t" "vmmcall\n\t"); > + Unnecessary newline. > +} > + > +static bool vgif_finished(struct svm_test *test) > +{ > + switch (get_test_stage(test)) > + { > + case 0: > + if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) { > + report(false, "VMEXIT not due to vmmcall."); > + return true; > + } > + vmcb->control.int_ctl |= V_GIF_ENABLED_MASK; Setting and restoring a control flag should be done in setup/teardown, e.g. this approach will leave V_GIF_ENABLED_MASK set if a VMMCALL check fails. > + vmcb->save.rip += 3; > + inc_test_stage(test); > + break; > + case 1: > + if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) { > + report(false, "VMEXIT not due to vmmcall."); > + return true; > + } > + if (!(vmcb->control.int_ctl & V_GIF_MASK)) { > + report(false, "Failed to set VGIF when executing STGI."); > + vmcb->control.int_ctl &= ~V_GIF_ENABLED_MASK; > + return true; Are the VGIF checks really fatal? I.e. can we keep running of a STGI/CLGI test fails? That would allow for slightly cleaner code. > + } > + report(true, "STGI set VGIF bit."); > + vmcb->save.rip += 3; > + inc_test_stage(test); > + break; > + case 2: > + if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) { > + report(false, "VMEXIT not due to vmmcall."); > + return true; > + } > + if (vmcb->control.int_ctl & V_GIF_MASK) { > + report(false, "Failed to clear VGIF when executing CLGI."); > + vmcb->control.int_ctl &= ~V_GIF_ENABLED_MASK; > + return true; > + } > + report(true, "CLGI cleared VGIF bit."); > + vmcb->save.rip += 3; > + inc_test_stage(test); > + vmcb->control.int_ctl &= ~V_GIF_ENABLED_MASK; > + break; Something like this is more reader friendly as the boilerplate is consoliated, abd the interesting test code is isolated in the switch statement. bool is_vmmcall_exit = (vmcb->control.exit_code == SVM_EXIT_VMMCALL); report(is_vmmcall_exit, ...); if (!is_vmmcall_exit) return true; switch (get_test_stage()) { case 0: vmcb->control.int_ctl |= V_GIF_ENABLED_MASK; break; case 1: report(vmcb->control.int_ctl & V_GIF_MASK, ...); break; case 2: report(!(vmcb->control.int_ctl & V_GIF_MASK), ...); break; default: break; } vmcb->save.rip += 3; inc_test_stage(test); return get_test_stage() >= 3; > + default: > + return true; > + break; > + }
diff --git a/lib/x86/processor.h b/lib/x86/processor.h index 173520f..f4d1757 100644 --- a/lib/x86/processor.h +++ b/lib/x86/processor.h @@ -187,6 +187,7 @@ static inline bool is_intel(void) #define X86_FEATURE_AMD_IBPB (CPUID(0x80000008, 0, EBX, 12)) #define X86_FEATURE_NPT (CPUID(0x8000000A, 0, EDX, 0)) #define X86_FEATURE_NRIPS (CPUID(0x8000000A, 0, EDX, 3)) +#define X86_FEATURE_VGIF (CPUID(0x8000000A, 0, EDX, 16)) static inline bool this_cpu_has(u64 feature) diff --git a/x86/svm.c b/x86/svm.c index 4eaa97e..b0096a9 100644 --- a/x86/svm.c +++ b/x86/svm.c @@ -65,6 +65,11 @@ bool default_supported(void) return true; } +bool vgif_supported(void) +{ + return this_cpu_has(X86_FEATURE_VGIF); +} + void default_prepare(struct svm_test *test) { vmcb_ident(vmcb); diff --git a/x86/svm.h b/x86/svm.h index 995b0f8..339b9f7 100644 --- a/x86/svm.h +++ b/x86/svm.h @@ -115,6 +115,12 @@ struct __attribute__ ((__packed__)) vmcb_control_area { #define V_IRQ_SHIFT 8 #define V_IRQ_MASK (1 << V_IRQ_SHIFT) +#define V_GIF_ENABLED_SHIFT 25 +#define V_GIF_ENABLED_MASK (1 << V_GIF_ENABLED_SHIFT) + +#define V_GIF_SHIFT 9 +#define V_GIF_MASK (1 << V_GIF_SHIFT) + #define V_INTR_PRIO_SHIFT 16 #define V_INTR_PRIO_MASK (0x0f << V_INTR_PRIO_SHIFT) @@ -398,6 +404,7 @@ u64 *npt_get_pdpe(void); u64 *npt_get_pml4e(void); bool smp_supported(void); bool default_supported(void); +bool vgif_supported(void); void default_prepare(struct svm_test *test); void default_prepare_gif_clear(struct svm_test *test); bool default_finished(struct svm_test *test); diff --git a/x86/svm_tests.c b/x86/svm_tests.c index 3777208..7c7b19d 100644 --- a/x86/svm_tests.c +++ b/x86/svm_tests.c @@ -2772,6 +2772,73 @@ static void svm_vmload_vmsave(void) vmcb->control.intercept = intercept_saved; } +static void prepare_vgif_enabled(struct svm_test *test) +{ + default_prepare(test); +} + +static void test_vgif(struct svm_test *test) +{ + asm volatile ("vmmcall\n\tstgi\n\tvmmcall\n\tclgi\n\tvmmcall\n\t"); + +} + +static bool vgif_finished(struct svm_test *test) +{ + switch (get_test_stage(test)) + { + case 0: + if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) { + report(false, "VMEXIT not due to vmmcall."); + return true; + } + vmcb->control.int_ctl |= V_GIF_ENABLED_MASK; + vmcb->save.rip += 3; + inc_test_stage(test); + break; + case 1: + if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) { + report(false, "VMEXIT not due to vmmcall."); + return true; + } + if (!(vmcb->control.int_ctl & V_GIF_MASK)) { + report(false, "Failed to set VGIF when executing STGI."); + vmcb->control.int_ctl &= ~V_GIF_ENABLED_MASK; + return true; + } + report(true, "STGI set VGIF bit."); + vmcb->save.rip += 3; + inc_test_stage(test); + break; + case 2: + if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) { + report(false, "VMEXIT not due to vmmcall."); + return true; + } + if (vmcb->control.int_ctl & V_GIF_MASK) { + report(false, "Failed to clear VGIF when executing CLGI."); + vmcb->control.int_ctl &= ~V_GIF_ENABLED_MASK; + return true; + } + report(true, "CLGI cleared VGIF bit."); + vmcb->save.rip += 3; + inc_test_stage(test); + vmcb->control.int_ctl &= ~V_GIF_ENABLED_MASK; + break; + default: + return true; + break; + } + + return get_test_stage(test) == 3; +} + +static bool vgif_check(struct svm_test *test) +{ + return get_test_stage(test) == 3; +} + + struct svm_test svm_tests[] = { { "null", default_supported, default_prepare, default_prepare_gif_clear, null_test, @@ -2882,6 +2949,9 @@ struct svm_test svm_tests[] = { { "host_rflags", default_supported, host_rflags_prepare, host_rflags_prepare_gif_clear, host_rflags_test, host_rflags_finished, host_rflags_check }, + { "vgif", vgif_supported, prepare_vgif_enabled, + default_prepare_gif_clear, test_vgif, vgif_finished, + vgif_check }, TEST(svm_cr4_osxsave_test), TEST(svm_guest_state_test), TEST(svm_npt_rsvd_bits_test),
When VGIF is enabled STGI executed in guest mode sets bit 9, while CLGI clears bit 9 in the int_ctl (offset 60h) of the VMCB. Signed-off-by: Lara Lazier <laramglazier@gmail.com> --- lib/x86/processor.h | 1 + x86/svm.c | 5 ++++ x86/svm.h | 7 +++++ x86/svm_tests.c | 70 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+)