| Message ID | 20220810061924.1418-1-santosh.shukla@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests,PATCHv2] x86: nSVM: Add support for VNMI test | expand |
On Wed, Aug 10, 2022, Santosh Shukla wrote: > Add a VNMI test case to test Virtual NMI in a nested environment, > The test covers the Virtual NMI (VNMI) delivery. > > Signed-off-by: Santosh Shukla <santosh.shukla@amd.com> > --- Quite a few comments, but I'll post a v3 with everything cleaned up. More below. > diff --git a/x86/svm.h b/x86/svm.h > index 766ff7e36449..91a0dee2c864 100644 > --- a/x86/svm.h > +++ b/x86/svm.h > @@ -131,6 +131,13 @@ struct __attribute__ ((__packed__)) vmcb_control_area { > #define V_INTR_MASKING_SHIFT 24 > #define V_INTR_MASKING_MASK (1 << V_INTR_MASKING_SHIFT) > > +#define V_NMI_PENDING_SHIFT 11 > +#define V_NMI_PENDING (1 << V_NMI_PENDING_SHIFT) > +#define V_NMI_MASK_SHIFT 12 > +#define V_NMI_MASK (1 << V_NMI_MASK_SHIFT) > +#define V_NMI_ENABLE_SHIFT 26 > +#define V_NMI_ENABLE (1 << V_NMI_ENABLE_SHIFT) Same complaints as I had for the kernel side, e.g. VM_NMI_MASK vs. V_NMI_BLOCKING_MASK. > + > #define SVM_INTERRUPT_SHADOW_MASK 1 > > #define SVM_IOIO_STR_SHIFT 2 > @@ -419,6 +426,7 @@ void default_prepare(struct svm_test *test); > void default_prepare_gif_clear(struct svm_test *test); > bool default_finished(struct svm_test *test); > bool npt_supported(void); > +bool vnmi_supported(void); > int get_test_stage(struct svm_test *test); > void set_test_stage(struct svm_test *test, int s); > void inc_test_stage(struct svm_test *test); > diff --git a/x86/svm_tests.c b/x86/svm_tests.c > index e2ec9541fd29..f83a2b56ce52 100644 > --- a/x86/svm_tests.c > +++ b/x86/svm_tests.c > @@ -1445,6 +1445,93 @@ static bool nmi_hlt_check(struct svm_test *test) > return get_test_stage(test) == 3; > } > > +static volatile bool vnmi_fired; There's no need to use a separate flag, just reuse nmi_fired. And then this test can also reuse nmi_prepare(). > +static void vnmi_handler(isr_regs_t *regs) > +{ > + vnmi_fired = true; Use tabs, not spaces. > +} > + > +static void vnmi_prepare(struct svm_test *test) > +{ > + default_prepare(test); > + vnmi_fired = false; > + vmcb->control.int_ctl = V_NMI_ENABLE; > + vmcb->control.int_vector = NMI_VECTOR; > + handle_irq(NMI_VECTOR, vnmi_handler); > + set_test_stage(test, 0); > +} > + > +static void vnmi_test(struct svm_test *test) > +{ > + if (vnmi_fired) { > + report(!vnmi_fired, "vNMI dispatched even before injection"); > + set_test_stage(test, -1); Ugh, so much copy+paste in the test. I'll add a patch to provide a macro to dedup the report_fail() + set_test_stage() + vmmcall(), the last of which is unnecessarily dependent on non-failing code in many cases. > +static bool vnmi_finished(struct svm_test *test) > +{ > + switch (get_test_stage(test)) { > + case 0: > + if (vmcb->control.exit_code != SVM_EXIT_ERR) { Took me a bit of staring to understand why an error is expected. The setup path really needs a comment explaining that it deliberately creates a bad configuration, and it should explicitly clear INTERCEPT_NMI. > + report_fail("VMEXIT not due to error. Exit reason 0x%x", > + vmcb->control.exit_code); > + return true; > + } > + report(!vnmi_fired, "vNMI enabled but NMI_INTERCEPT unset!"); > + vmcb->control.intercept |= (1ULL << INTERCEPT_NMI); > + vmcb->save.rip += 3; > + break; > + > + case 1: > + if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) { > + report_fail("VMEXIT not due to error. Exit reason 0x%x", VMMCALL expected, not ERR.
diff --git a/lib/x86/processor.h b/lib/x86/processor.h index 0324220630c6..e514abd2e6a3 100644 --- a/lib/x86/processor.h +++ b/lib/x86/processor.h @@ -265,7 +265,7 @@ static inline bool is_intel(void) #define X86_FEATURE_PAUSEFILTER (CPUID(0x8000000A, 0, EDX, 10)) #define X86_FEATURE_PFTHRESHOLD (CPUID(0x8000000A, 0, EDX, 12)) #define X86_FEATURE_VGIF (CPUID(0x8000000A, 0, EDX, 16)) - +#define X86_FEATURE_V_NMI (CPUID(0x8000000A, 0, EDX, 25)) static inline bool this_cpu_has(u64 feature) { diff --git a/x86/svm.c b/x86/svm.c index ba435b4ac3af..022a0fde4336 100644 --- a/x86/svm.c +++ b/x86/svm.c @@ -99,6 +99,11 @@ bool npt_supported(void) return this_cpu_has(X86_FEATURE_NPT); } +bool vnmi_supported(void) +{ + return this_cpu_has(X86_FEATURE_V_NMI); +} + int get_test_stage(struct svm_test *test) { barrier(); diff --git a/x86/svm.h b/x86/svm.h index 766ff7e36449..91a0dee2c864 100644 --- a/x86/svm.h +++ b/x86/svm.h @@ -131,6 +131,13 @@ struct __attribute__ ((__packed__)) vmcb_control_area { #define V_INTR_MASKING_SHIFT 24 #define V_INTR_MASKING_MASK (1 << V_INTR_MASKING_SHIFT) +#define V_NMI_PENDING_SHIFT 11 +#define V_NMI_PENDING (1 << V_NMI_PENDING_SHIFT) +#define V_NMI_MASK_SHIFT 12 +#define V_NMI_MASK (1 << V_NMI_MASK_SHIFT) +#define V_NMI_ENABLE_SHIFT 26 +#define V_NMI_ENABLE (1 << V_NMI_ENABLE_SHIFT) + #define SVM_INTERRUPT_SHADOW_MASK 1 #define SVM_IOIO_STR_SHIFT 2 @@ -419,6 +426,7 @@ void default_prepare(struct svm_test *test); void default_prepare_gif_clear(struct svm_test *test); bool default_finished(struct svm_test *test); bool npt_supported(void); +bool vnmi_supported(void); int get_test_stage(struct svm_test *test); void set_test_stage(struct svm_test *test, int s); void inc_test_stage(struct svm_test *test); diff --git a/x86/svm_tests.c b/x86/svm_tests.c index e2ec9541fd29..f83a2b56ce52 100644 --- a/x86/svm_tests.c +++ b/x86/svm_tests.c @@ -1445,6 +1445,93 @@ static bool nmi_hlt_check(struct svm_test *test) return get_test_stage(test) == 3; } +static volatile bool vnmi_fired; +static void vnmi_handler(isr_regs_t *regs) +{ + vnmi_fired = true; +} + +static void vnmi_prepare(struct svm_test *test) +{ + default_prepare(test); + vnmi_fired = false; + vmcb->control.int_ctl = V_NMI_ENABLE; + vmcb->control.int_vector = NMI_VECTOR; + handle_irq(NMI_VECTOR, vnmi_handler); + set_test_stage(test, 0); +} + +static void vnmi_test(struct svm_test *test) +{ + if (vnmi_fired) { + report(!vnmi_fired, "vNMI dispatched even before injection"); + set_test_stage(test, -1); + } + + vmmcall(); + + if (!vnmi_fired) { + report(vnmi_fired, "pending vNMI not dispatched"); + set_test_stage(test, -1); + } + report(vnmi_fired, "vNMI delivered to guest"); + + vmmcall(); +} + +static bool vnmi_finished(struct svm_test *test) +{ + switch (get_test_stage(test)) { + case 0: + if (vmcb->control.exit_code != SVM_EXIT_ERR) { + report_fail("VMEXIT not due to error. Exit reason 0x%x", + vmcb->control.exit_code); + return true; + } + report(!vnmi_fired, "vNMI enabled but NMI_INTERCEPT unset!"); + vmcb->control.intercept |= (1ULL << INTERCEPT_NMI); + vmcb->save.rip += 3; + break; + + case 1: + if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) { + report_fail("VMEXIT not due to error. Exit reason 0x%x", + vmcb->control.exit_code); + return true; + } + report(!vnmi_fired, "vNMI with vector 2 not injected"); + vmcb->control.int_ctl |= V_NMI_PENDING; + vmcb->save.rip += 3; + break; + + case 2: + if (vmcb->control.exit_code != SVM_EXIT_VMMCALL) { + report_fail("VMEXIT not due to vmmcall. Exit reason 0x%x", + vmcb->control.exit_code); + return true; + } + if (vmcb->control.int_ctl & V_NMI_MASK) { + report_fail("V_NMI_MASK not cleared on VMEXIT"); + return true; + } + report_pass("VNMI serviced"); + vmcb->save.rip += 3; + break; + + default: + return true; + } + + inc_test_stage(test); + + return get_test_stage(test) == 3; +} + +static bool vnmi_check(struct svm_test *test) +{ + return get_test_stage(test) == 3; +} + static volatile int count_exc = 0; static void my_isr(struct ex_regs *r) @@ -3363,6 +3450,9 @@ struct svm_test svm_tests[] = { { "nmi_hlt", smp_supported, nmi_prepare, default_prepare_gif_clear, nmi_hlt_test, nmi_hlt_finished, nmi_hlt_check }, + { "vnmi", vnmi_supported, vnmi_prepare, + default_prepare_gif_clear, vnmi_test, + vnmi_finished, vnmi_check }, { "virq_inject", default_supported, virq_inject_prepare, default_prepare_gif_clear, virq_inject_test, virq_inject_finished, virq_inject_check },
Add a VNMI test case to test Virtual NMI in a nested environment, The test covers the Virtual NMI (VNMI) delivery. Signed-off-by: Santosh Shukla <santosh.shukla@amd.com> --- v2: - Added test to test NMI_INTERCEPT unset case. - Rebased on tip. v1: - https://www.spinics.net/lists/kvm/msg279165.html lib/x86/processor.h | 2 +- x86/svm.c | 5 +++ x86/svm.h | 8 ++++ x86/svm_tests.c | 90 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 104 insertions(+), 1 deletion(-)