* [kvm-unit-tests PATCHv2] x86: nSVM: Add support for VNMI test
@ 2022-08-10 6:19 Santosh Shukla
2023-04-05 20:45 ` Sean Christopherson
0 siblings, 1 reply; 2+ messages in thread
From: Santosh Shukla @ 2022-08-10 6:19 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Sean Christopherson, kvm, santosh.shukla
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(-)
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 },
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [kvm-unit-tests PATCHv2] x86: nSVM: Add support for VNMI test
2022-08-10 6:19 [kvm-unit-tests PATCHv2] x86: nSVM: Add support for VNMI test Santosh Shukla
@ 2023-04-05 20:45 ` Sean Christopherson
0 siblings, 0 replies; 2+ messages in thread
From: Sean Christopherson @ 2023-04-05 20:45 UTC (permalink / raw)
To: Santosh Shukla; +Cc: Paolo Bonzini, kvm
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.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-04-05 20:46 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-10 6:19 [kvm-unit-tests PATCHv2] x86: nSVM: Add support for VNMI test Santosh Shukla
2023-04-05 20:45 ` Sean Christopherson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.