* [kvm-unit-tests PATCH 1/5] nVMX: Enable x2APIC mode for virtual-interrupt delivery tests
2023-12-11 18:55 [kvm-unit-tests PATCH 0/5] nVMX: Simple posted interrupts test Jim Mattson
@ 2023-12-11 18:55 ` Jim Mattson
2023-12-11 18:55 ` [kvm-unit-tests PATCH 2/5] nVMX: test nested "virtual-interrupt delivery" Jim Mattson
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Jim Mattson @ 2023-12-11 18:55 UTC (permalink / raw)
To: seanjc, kvm, pbonzini; +Cc: Jim Mattson
Since "virtualize x2APIC mode" is enabled for these tests, call
enable_x2apic() so that the x2apic_ops function table will be
installed.
Signed-off-by: Jim Mattson <jmattson@google.com>
---
x86/vmx_tests.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index c1540d396da8..e5ed79b7da4a 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -9305,6 +9305,7 @@ static void enable_vid(void)
assert(cpu_has_apicv());
+ enable_x2apic();
disable_intercept_for_x2apic_msrs();
virtual_apic_page = alloc_page();
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [kvm-unit-tests PATCH 2/5] nVMX: test nested "virtual-interrupt delivery"
2023-12-11 18:55 [kvm-unit-tests PATCH 0/5] nVMX: Simple posted interrupts test Jim Mattson
2023-12-11 18:55 ` [kvm-unit-tests PATCH 1/5] nVMX: Enable x2APIC mode for virtual-interrupt delivery tests Jim Mattson
@ 2023-12-11 18:55 ` Jim Mattson
2023-12-11 18:55 ` [kvm-unit-tests PATCH 3/5] nVMX: test nested EOI virtualization Jim Mattson
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Jim Mattson @ 2023-12-11 18:55 UTC (permalink / raw)
To: seanjc, kvm, pbonzini; +Cc: Marc Orr (Google), Oliver Upton, Jim Mattson
From: "Marc Orr (Google)" <marc.orr@gmail.com>
Add test coverage for recognizing and delivering virtual interrupts via
VMX's "virtual-interrupt delivery" feature, in the following two scenarios:
1. There's a pending interrupt at VM-entry.
2. There's a pending interrupt during TPR virtualization.
Signed-off-by: Marc Orr (Google) <marc.orr@gmail.com>
Co-developed-by: Oliver Upton <oliver.upton@linux.dev>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Co-developed-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Jim Mattson <jmattson@google.com>
---
lib/x86/apic.h | 5 ++
x86/unittests.cfg | 2 +-
x86/vmx_tests.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 171 insertions(+), 1 deletion(-)
diff --git a/lib/x86/apic.h b/lib/x86/apic.h
index c389d40e169a..8df889b2d1e4 100644
--- a/lib/x86/apic.h
+++ b/lib/x86/apic.h
@@ -81,6 +81,11 @@ static inline bool apic_lvt_entry_supported(int idx)
return GET_APIC_MAXLVT(apic_read(APIC_LVR)) >= idx;
}
+static inline u8 task_priority_class(u8 vector)
+{
+ return vector >> 4;
+}
+
enum x2apic_reg_semantics {
X2APIC_INVALID = 0,
X2APIC_READABLE = BIT(0),
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 3fe59449b650..dd086d9e2bf4 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -361,7 +361,7 @@ timeout = 10
[vmx_apicv_test]
file = vmx.flat
-extra_params = -cpu max,+vmx -append "apic_reg_virt_test virt_x2apic_mode_test"
+extra_params = -cpu max,+vmx -append "apic_reg_virt_test virt_x2apic_mode_test vmx_basic_vid_test"
arch = x86_64
groups = vmx
timeout = 10
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index e5ed79b7da4a..0fb7e1466c50 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -10711,6 +10711,170 @@ static void vmx_exception_test(void)
test_set_guest_finished();
}
+enum Vid_op {
+ VID_OP_SET_ISR,
+ VID_OP_NOP,
+ VID_OP_SET_CR8,
+ VID_OP_TERMINATE,
+};
+
+struct vmx_basic_vid_test_guest_args {
+ enum Vid_op op;
+ u8 nr;
+ bool isr_fired;
+} vmx_basic_vid_test_guest_args;
+
+static void vmx_vid_test_isr(isr_regs_t *regs)
+{
+ volatile struct vmx_basic_vid_test_guest_args *args =
+ &vmx_basic_vid_test_guest_args;
+
+ args->isr_fired = true;
+ barrier();
+ eoi();
+}
+
+static void vmx_basic_vid_test_guest(void)
+{
+ volatile struct vmx_basic_vid_test_guest_args *args =
+ &vmx_basic_vid_test_guest_args;
+
+ sti_nop();
+ for (;;) {
+ enum Vid_op op = args->op;
+ u8 nr = args->nr;
+
+ switch (op) {
+ case VID_OP_TERMINATE:
+ return;
+ case VID_OP_SET_ISR:
+ handle_irq(nr, vmx_vid_test_isr);
+ break;
+ case VID_OP_SET_CR8:
+ write_cr8(nr);
+ break;
+ default:
+ break;
+ }
+
+ vmcall();
+ }
+}
+
+/*
+ * Test virtual interrupt delivery (VID) at VM-entry or TPR virtualization
+ *
+ * Args:
+ * nr: vector under test
+ * tpr: task priority under test
+ * tpr_virt: If true, then test VID during TPR virtualization. Otherwise,
+ * test VID during VM-entry.
+ */
+static void test_basic_vid(u8 nr, u8 tpr, bool tpr_virt)
+{
+ volatile struct vmx_basic_vid_test_guest_args *args =
+ &vmx_basic_vid_test_guest_args;
+ bool isr_fired_want =
+ task_priority_class(nr) > task_priority_class(tpr);
+ u16 rvi_want = isr_fired_want ? 0 : nr;
+ u16 int_status;
+
+ /*
+ * From the SDM:
+ * IF "interrupt-window exiting" is 0 AND
+ * RVI[7:4] > VPPR[7:4] (see Section 29.1.1 for definition of VPPR)
+ * THEN recognize a pending virtual interrupt;
+ * ELSE
+ * do not recognize a pending virtual interrupt;
+ * FI;
+ *
+ * Thus, VPPR dictates whether a virtual interrupt is recognized.
+ * However, PPR virtualization, which occurs before virtual interrupt
+ * delivery, sets VPPR to VTPR, when SVI is 0.
+ */
+ vmcs_write(GUEST_INT_STATUS, nr);
+ args->isr_fired = false;
+ if (tpr_virt) {
+ args->op = VID_OP_SET_CR8;
+ args->nr = task_priority_class(tpr);
+ set_vtpr(0xff);
+ } else {
+ args->op = VID_OP_NOP;
+ set_vtpr(tpr);
+ }
+
+ enter_guest();
+ skip_exit_vmcall();
+ TEST_ASSERT_EQ(args->isr_fired, isr_fired_want);
+ int_status = vmcs_read(GUEST_INT_STATUS);
+ TEST_ASSERT_EQ(int_status, rvi_want);
+}
+
+/*
+ * Test recognizing and delivering virtual interrupts via "Virtual-interrupt
+ * delivery" for two scenarios:
+ * 1. When there is a pending interrupt at VM-entry.
+ * 2. When there is a pending interrupt during TPR virtualization.
+ */
+static void vmx_basic_vid_test(void)
+{
+ volatile struct vmx_basic_vid_test_guest_args *args =
+ &vmx_basic_vid_test_guest_args;
+ u8 nr_class;
+ u16 nr;
+
+ if (!cpu_has_apicv()) {
+ report_skip("%s : Not all required APICv bits supported", __func__);
+ return;
+ }
+
+ enable_vid();
+ test_set_guest(vmx_basic_vid_test_guest);
+
+ /*
+ * kvm-unit-tests uses vector 32 for IPIs, so don't install a test ISR
+ * for that vector.
+ */
+ for (nr = 0x21; nr < 0x100; nr++) {
+ vmcs_write(GUEST_INT_STATUS, 0);
+ args->op = VID_OP_SET_ISR;
+ args->nr = nr;
+ args->isr_fired = false;
+ enter_guest();
+ skip_exit_vmcall();
+ TEST_ASSERT(!args->isr_fired);
+ }
+ report(true, "Set ISR for vectors 33-255.");
+
+ for (nr_class = 2; nr_class < 16; nr_class++) {
+ u8 nr_sub_class;
+
+ for (nr_sub_class = 0; nr_sub_class < 16; nr_sub_class++) {
+ u16 tpr;
+
+ nr = (nr_class << 4) | nr_sub_class;
+
+ /*
+ * Don't test the reserved IPI vector, as the test ISR
+ * was not installed.
+ */
+ if (nr == 0x20)
+ continue;
+
+ for (tpr = 0; tpr < 256; tpr++) {
+ test_basic_vid(nr, tpr, /*tpr_virt=*/false);
+ test_basic_vid(nr, tpr, /*tpr_virt=*/true);
+ }
+ report(true, "TPR 0-255 for vector 0x%x.", nr);
+ }
+ }
+
+ /* Terminate the guest */
+ args->op = VID_OP_TERMINATE;
+ enter_guest();
+ assert_exit_reason(VMX_VMCALL);
+}
+
#define TEST(name) { #name, .v2 = name }
/* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
@@ -10765,6 +10929,7 @@ struct vmx_test vmx_tests[] = {
TEST(vmx_hlt_with_rvi_test),
TEST(apic_reg_virt_test),
TEST(virt_x2apic_mode_test),
+ TEST(vmx_basic_vid_test),
/* APIC pass-through tests */
TEST(vmx_apic_passthrough_test),
TEST(vmx_apic_passthrough_thread_test),
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [kvm-unit-tests PATCH 3/5] nVMX: test nested EOI virtualization
2023-12-11 18:55 [kvm-unit-tests PATCH 0/5] nVMX: Simple posted interrupts test Jim Mattson
2023-12-11 18:55 ` [kvm-unit-tests PATCH 1/5] nVMX: Enable x2APIC mode for virtual-interrupt delivery tests Jim Mattson
2023-12-11 18:55 ` [kvm-unit-tests PATCH 2/5] nVMX: test nested "virtual-interrupt delivery" Jim Mattson
@ 2023-12-11 18:55 ` Jim Mattson
2023-12-11 18:55 ` [kvm-unit-tests PATCH 4/5] nVMX: add self-IPI tests to vmx_basic_vid_test Jim Mattson
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Jim Mattson @ 2023-12-11 18:55 UTC (permalink / raw)
To: seanjc, kvm, pbonzini; +Cc: Marc Orr (Google), Oliver Upton, Jim Mattson
From: "Marc Orr (Google)" <marc.orr@gmail.com>
Add a test for nested VMs that invoke EOI virtualization. Specifically,
check that a pending low-priority interrupt, masked by a higher-priority
interrupt, is scheduled via "virtual-interrupt delivery," after the
higher-priority interrupt executes EOI.
Signed-off-by: Marc Orr (Google) <marc.orr@gmail.com>
Co-developed-by: Oliver Upton <oliver.upton@linux.dev>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Co-developed-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Jim Mattson <jmattson@google.com>
---
x86/unittests.cfg | 2 +-
x86/vmx_tests.c | 161 ++++++++++++++++++++++++++++++++++++++--------
2 files changed, 136 insertions(+), 27 deletions(-)
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index dd086d9e2bf4..f307168b0e01 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -361,7 +361,7 @@ timeout = 10
[vmx_apicv_test]
file = vmx.flat
-extra_params = -cpu max,+vmx -append "apic_reg_virt_test virt_x2apic_mode_test vmx_basic_vid_test"
+extra_params = -cpu max,+vmx -append "apic_reg_virt_test virt_x2apic_mode_test vmx_basic_vid_test vmx_eoi_virt_test"
arch = x86_64
groups = vmx
timeout = 10
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 0fb7e1466c50..ce480431bf58 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -60,6 +60,11 @@ static inline void vmcall(void)
asm volatile("vmcall");
}
+static u32 *get_vapic_page(void)
+{
+ return (u32 *)phys_to_virt(vmcs_read(APIC_VIRT_ADDR));
+}
+
static void basic_guest_main(void)
{
report_pass("Basic VMX test");
@@ -10721,15 +10726,36 @@ enum Vid_op {
struct vmx_basic_vid_test_guest_args {
enum Vid_op op;
u8 nr;
- bool isr_fired;
+ u32 isr_exec_cnt;
} vmx_basic_vid_test_guest_args;
+/*
+ * From the SDM, Bit x of the VIRR is
+ * at bit position (x & 1FH)
+ * at offset (200H | ((x & E0H) >> 1)).
+ */
+static void set_virr_bit(volatile u32 *virtual_apic_page, u8 nr)
+{
+ u32 page_offset = (0x200 | ((nr & 0xE0) >> 1)) / sizeof(u32);
+ u32 mask = 1 << (nr & 0x1f);
+
+ virtual_apic_page[page_offset] |= mask;
+}
+
+static bool get_virr_bit(volatile u32 *virtual_apic_page, u8 nr)
+{
+ u32 page_offset = (0x200 | ((nr & 0xE0) >> 1)) / sizeof(u32);
+ u32 mask = 1 << (nr & 0x1f);
+
+ return virtual_apic_page[page_offset] & mask;
+}
+
static void vmx_vid_test_isr(isr_regs_t *regs)
{
volatile struct vmx_basic_vid_test_guest_args *args =
&vmx_basic_vid_test_guest_args;
- args->isr_fired = true;
+ args->isr_exec_cnt++;
barrier();
eoi();
}
@@ -10761,6 +10787,27 @@ static void vmx_basic_vid_test_guest(void)
}
}
+static void set_isrs_for_vmx_basic_vid_test(void)
+{
+ volatile struct vmx_basic_vid_test_guest_args *args =
+ &vmx_basic_vid_test_guest_args;
+ u16 nr;
+
+ /*
+ * kvm-unit-tests uses vector 32 for IPIs, so don't install a test ISR
+ * for that vector.
+ */
+ for (nr = 0x21; nr < 0x100; nr++) {
+ vmcs_write(GUEST_INT_STATUS, 0);
+ args->op = VID_OP_SET_ISR;
+ args->nr = nr;
+ args->isr_exec_cnt = 0;
+ enter_guest();
+ skip_exit_vmcall();
+ }
+ report(true, "Set ISR for vectors 33-255.");
+}
+
/*
* Test virtual interrupt delivery (VID) at VM-entry or TPR virtualization
*
@@ -10770,13 +10817,12 @@ static void vmx_basic_vid_test_guest(void)
* tpr_virt: If true, then test VID during TPR virtualization. Otherwise,
* test VID during VM-entry.
*/
-static void test_basic_vid(u8 nr, u8 tpr, bool tpr_virt)
+static void test_basic_vid(u8 nr, u8 tpr, bool tpr_virt, u32 isr_exec_cnt_want,
+ bool eoi_exit_induced)
{
volatile struct vmx_basic_vid_test_guest_args *args =
&vmx_basic_vid_test_guest_args;
- bool isr_fired_want =
- task_priority_class(nr) > task_priority_class(tpr);
- u16 rvi_want = isr_fired_want ? 0 : nr;
+ u16 rvi_want = isr_exec_cnt_want ? 0 : nr;
u16 int_status;
/*
@@ -10793,7 +10839,7 @@ static void test_basic_vid(u8 nr, u8 tpr, bool tpr_virt)
* delivery, sets VPPR to VTPR, when SVI is 0.
*/
vmcs_write(GUEST_INT_STATUS, nr);
- args->isr_fired = false;
+ args->isr_exec_cnt = 0;
if (tpr_virt) {
args->op = VID_OP_SET_CR8;
args->nr = task_priority_class(tpr);
@@ -10804,8 +10850,18 @@ static void test_basic_vid(u8 nr, u8 tpr, bool tpr_virt)
}
enter_guest();
+ if (eoi_exit_induced) {
+ u32 exit_cnt;
+
+ assert_exit_reason(VMX_EOI_INDUCED);
+ for (exit_cnt = 1; exit_cnt < isr_exec_cnt_want; exit_cnt++) {
+ enter_guest();
+ assert_exit_reason(VMX_EOI_INDUCED);
+ }
+ enter_guest();
+ }
skip_exit_vmcall();
- TEST_ASSERT_EQ(args->isr_fired, isr_fired_want);
+ TEST_ASSERT_EQ(args->isr_exec_cnt, isr_exec_cnt_want);
int_status = vmcs_read(GUEST_INT_STATUS);
TEST_ASSERT_EQ(int_status, rvi_want);
}
@@ -10821,7 +10877,6 @@ static void vmx_basic_vid_test(void)
volatile struct vmx_basic_vid_test_guest_args *args =
&vmx_basic_vid_test_guest_args;
u8 nr_class;
- u16 nr;
if (!cpu_has_apicv()) {
report_skip("%s : Not all required APICv bits supported", __func__);
@@ -10830,23 +10885,10 @@ static void vmx_basic_vid_test(void)
enable_vid();
test_set_guest(vmx_basic_vid_test_guest);
-
- /*
- * kvm-unit-tests uses vector 32 for IPIs, so don't install a test ISR
- * for that vector.
- */
- for (nr = 0x21; nr < 0x100; nr++) {
- vmcs_write(GUEST_INT_STATUS, 0);
- args->op = VID_OP_SET_ISR;
- args->nr = nr;
- args->isr_fired = false;
- enter_guest();
- skip_exit_vmcall();
- TEST_ASSERT(!args->isr_fired);
- }
- report(true, "Set ISR for vectors 33-255.");
+ set_isrs_for_vmx_basic_vid_test();
for (nr_class = 2; nr_class < 16; nr_class++) {
+ u16 nr;
u8 nr_sub_class;
for (nr_sub_class = 0; nr_sub_class < 16; nr_sub_class++) {
@@ -10862,8 +10904,16 @@ static void vmx_basic_vid_test(void)
continue;
for (tpr = 0; tpr < 256; tpr++) {
- test_basic_vid(nr, tpr, /*tpr_virt=*/false);
- test_basic_vid(nr, tpr, /*tpr_virt=*/true);
+ u32 isr_exec_cnt_want =
+ task_priority_class(nr) >
+ task_priority_class(tpr) ? 1 : 0;
+
+ test_basic_vid(nr, tpr, /*tpr_virt=*/false,
+ isr_exec_cnt_want,
+ /*eoi_exit_induced=*/false);
+ test_basic_vid(nr, tpr, /*tpr_virt=*/true,
+ isr_exec_cnt_want,
+ /*eoi_exit_induced=*/false);
}
report(true, "TPR 0-255 for vector 0x%x.", nr);
}
@@ -10875,6 +10925,64 @@ static void vmx_basic_vid_test(void)
assert_exit_reason(VMX_VMCALL);
}
+static void test_eoi_virt(u8 nr, u8 lo_pri_nr, bool eoi_exit_induced)
+{
+ u32 *virtual_apic_page = get_vapic_page();
+
+ set_virr_bit(virtual_apic_page, lo_pri_nr);
+ test_basic_vid(nr, /*tpr=*/0, /*tpr_virt=*/false,
+ /*isr_exec_cnt_want=*/2, eoi_exit_induced);
+ TEST_ASSERT(!get_virr_bit(virtual_apic_page, lo_pri_nr));
+ TEST_ASSERT(!get_virr_bit(virtual_apic_page, nr));
+}
+
+static void vmx_eoi_virt_test(void)
+{
+ volatile struct vmx_basic_vid_test_guest_args *args =
+ &vmx_basic_vid_test_guest_args;
+ u16 nr;
+ u16 lo_pri_nr;
+
+ if (!cpu_has_apicv()) {
+ report_skip("%s : Not all required APICv bits supported", __func__);
+ return;
+ }
+
+ enable_vid(); /* Note, enable_vid sets APIC_VIRT_ADDR field in VMCS. */
+ test_set_guest(vmx_basic_vid_test_guest);
+ set_isrs_for_vmx_basic_vid_test();
+
+ /* Now test EOI virtualization without induced EOI exits. */
+ for (nr = 0x22; nr < 0x100; nr++) {
+ for (lo_pri_nr = 0x21; lo_pri_nr < nr; lo_pri_nr++)
+ test_eoi_virt(nr, lo_pri_nr,
+ /*eoi_exit_induced=*/false);
+
+ report(true, "Low priority nrs 0x21-0x%x for nr 0x%x.",
+ nr - 1, nr);
+ }
+
+ /* Finally, test EOI virtualization with induced EOI exits. */
+ vmcs_write(EOI_EXIT_BITMAP0, GENMASK_ULL(63, 0));
+ vmcs_write(EOI_EXIT_BITMAP1, GENMASK_ULL(63, 0));
+ vmcs_write(EOI_EXIT_BITMAP2, GENMASK_ULL(63, 0));
+ vmcs_write(EOI_EXIT_BITMAP3, GENMASK_ULL(63, 0));
+ for (nr = 0x22; nr < 0x100; nr++) {
+ for (lo_pri_nr = 0x21; lo_pri_nr < nr; lo_pri_nr++)
+ test_eoi_virt(nr, lo_pri_nr,
+ /*eoi_exit_induced=*/true);
+
+ report(true,
+ "Low priority nrs 0x21-0x%x for nr 0x%x, with induced EOI exits.",
+ nr - 1, nr);
+ }
+
+ /* Terminate the guest */
+ args->op = VID_OP_TERMINATE;
+ enter_guest();
+ assert_exit_reason(VMX_VMCALL);
+}
+
#define TEST(name) { #name, .v2 = name }
/* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
@@ -10930,6 +11038,7 @@ struct vmx_test vmx_tests[] = {
TEST(apic_reg_virt_test),
TEST(virt_x2apic_mode_test),
TEST(vmx_basic_vid_test),
+ TEST(vmx_eoi_virt_test),
/* APIC pass-through tests */
TEST(vmx_apic_passthrough_test),
TEST(vmx_apic_passthrough_thread_test),
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [kvm-unit-tests PATCH 4/5] nVMX: add self-IPI tests to vmx_basic_vid_test
2023-12-11 18:55 [kvm-unit-tests PATCH 0/5] nVMX: Simple posted interrupts test Jim Mattson
` (2 preceding siblings ...)
2023-12-11 18:55 ` [kvm-unit-tests PATCH 3/5] nVMX: test nested EOI virtualization Jim Mattson
@ 2023-12-11 18:55 ` Jim Mattson
2023-12-11 18:55 ` [kvm-unit-tests PATCH 5/5] nVMX: add test for posted interrupts Jim Mattson
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Jim Mattson @ 2023-12-11 18:55 UTC (permalink / raw)
To: seanjc, kvm, pbonzini; +Cc: Marc Orr (Google), Jim Mattson
From: "Marc Orr (Google)" <marc.orr@gmail.com>
Extend the VMX "virtual-interrupt delivery test", vmx_basic_vid_test,
to verify that virtual-interrupt delivery is triggered by a self-IPI
in L2.
Signed-off-by: Marc Orr (Google) <marc.orr@gmail.com>
Co-developed-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Jim Mattson <jmattson@google.com>
---
x86/vmx_tests.c | 35 +++++++++++++++++++++++++----------
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index ce480431bf58..a26f77e92f72 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -10720,6 +10720,7 @@ enum Vid_op {
VID_OP_SET_ISR,
VID_OP_NOP,
VID_OP_SET_CR8,
+ VID_OP_SELF_IPI,
VID_OP_TERMINATE,
};
@@ -10779,6 +10780,9 @@ static void vmx_basic_vid_test_guest(void)
case VID_OP_SET_CR8:
write_cr8(nr);
break;
+ case VID_OP_SELF_IPI:
+ vmx_x2apic_write(APIC_SELF_IPI, nr);
+ break;
default:
break;
}
@@ -10817,7 +10821,7 @@ static void set_isrs_for_vmx_basic_vid_test(void)
* tpr_virt: If true, then test VID during TPR virtualization. Otherwise,
* test VID during VM-entry.
*/
-static void test_basic_vid(u8 nr, u8 tpr, bool tpr_virt, u32 isr_exec_cnt_want,
+static void test_basic_vid(u8 nr, u8 tpr, enum Vid_op op, u32 isr_exec_cnt_want,
bool eoi_exit_induced)
{
volatile struct vmx_basic_vid_test_guest_args *args =
@@ -10838,15 +10842,23 @@ static void test_basic_vid(u8 nr, u8 tpr, bool tpr_virt, u32 isr_exec_cnt_want,
* However, PPR virtualization, which occurs before virtual interrupt
* delivery, sets VPPR to VTPR, when SVI is 0.
*/
- vmcs_write(GUEST_INT_STATUS, nr);
args->isr_exec_cnt = 0;
- if (tpr_virt) {
- args->op = VID_OP_SET_CR8;
+ args->op = op;
+ switch (op) {
+ case VID_OP_SELF_IPI:
+ vmcs_write(GUEST_INT_STATUS, 0);
+ args->nr = nr;
+ set_vtpr(0);
+ break;
+ case VID_OP_SET_CR8:
+ vmcs_write(GUEST_INT_STATUS, nr);
args->nr = task_priority_class(tpr);
set_vtpr(0xff);
- } else {
- args->op = VID_OP_NOP;
+ break;
+ default:
+ vmcs_write(GUEST_INT_STATUS, nr);
set_vtpr(tpr);
+ break;
}
enter_guest();
@@ -10903,15 +10915,18 @@ static void vmx_basic_vid_test(void)
if (nr == 0x20)
continue;
+ test_basic_vid(nr, /*tpr=*/0, VID_OP_SELF_IPI,
+ /*isr_exec_cnt_want=*/1,
+ /*eoi_exit_induced=*/false);
for (tpr = 0; tpr < 256; tpr++) {
u32 isr_exec_cnt_want =
task_priority_class(nr) >
task_priority_class(tpr) ? 1 : 0;
- test_basic_vid(nr, tpr, /*tpr_virt=*/false,
+ test_basic_vid(nr, tpr, VID_OP_NOP,
isr_exec_cnt_want,
/*eoi_exit_induced=*/false);
- test_basic_vid(nr, tpr, /*tpr_virt=*/true,
+ test_basic_vid(nr, tpr, VID_OP_SET_CR8,
isr_exec_cnt_want,
/*eoi_exit_induced=*/false);
}
@@ -10930,8 +10945,8 @@ static void test_eoi_virt(u8 nr, u8 lo_pri_nr, bool eoi_exit_induced)
u32 *virtual_apic_page = get_vapic_page();
set_virr_bit(virtual_apic_page, lo_pri_nr);
- test_basic_vid(nr, /*tpr=*/0, /*tpr_virt=*/false,
- /*isr_exec_cnt_want=*/2, eoi_exit_induced);
+ test_basic_vid(nr, /*tpr=*/0, VID_OP_NOP, /*isr_exec_cnt_want=*/2,
+ eoi_exit_induced);
TEST_ASSERT(!get_virr_bit(virtual_apic_page, lo_pri_nr));
TEST_ASSERT(!get_virr_bit(virtual_apic_page, nr));
}
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [kvm-unit-tests PATCH 5/5] nVMX: add test for posted interrupts
2023-12-11 18:55 [kvm-unit-tests PATCH 0/5] nVMX: Simple posted interrupts test Jim Mattson
` (3 preceding siblings ...)
2023-12-11 18:55 ` [kvm-unit-tests PATCH 4/5] nVMX: add self-IPI tests to vmx_basic_vid_test Jim Mattson
@ 2023-12-11 18:55 ` Jim Mattson
2024-06-05 23:20 ` [kvm-unit-tests PATCH 0/5] nVMX: Simple posted interrupts test Sean Christopherson
2024-10-09 6:48 ` Like Xu
6 siblings, 0 replies; 10+ messages in thread
From: Jim Mattson @ 2023-12-11 18:55 UTC (permalink / raw)
To: seanjc, kvm, pbonzini; +Cc: Oliver Upton, Jim Mattson
From: Oliver Upton <oliver.upton@linux.dev>
Test virtual posted interrupts under the following conditions:
- vTPR[7:4] >= VECTOR[7:4]: Expect the L2 interrupt to be blocked.
The bit corresponding to the posted interrupt should be set in L2's
vIRR. Test with a running guest.
- vTPR[7:4] < VECTOR[7:4]: Expect the interrupt to be delivered and the
ISR to execute once. Test with a running and halted guest.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
Co-developed-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Jim Mattson <jmattson@google.com>
---
lib/x86/asm/bitops.h | 8 +++
x86/unittests.cfg | 8 +++
x86/vmx_tests.c | 133 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 149 insertions(+)
diff --git a/lib/x86/asm/bitops.h b/lib/x86/asm/bitops.h
index 13a25ec9853d..54ec9c424cd6 100644
--- a/lib/x86/asm/bitops.h
+++ b/lib/x86/asm/bitops.h
@@ -13,4 +13,12 @@
#define HAVE_BUILTIN_FLS 1
+static inline void test_and_set_bit(long nr, unsigned long *addr)
+{
+ asm volatile("lock; bts %1,%0"
+ : "+m" (*addr)
+ : "Ir" (nr)
+ : "memory");
+}
+
#endif
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index f307168b0e01..9598c61ef7ac 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -366,6 +366,14 @@ arch = x86_64
groups = vmx
timeout = 10
+[vmx_posted_intr_test]
+file = vmx.flat
+smp = 2
+extra_params = -cpu max,+vmx -append "vmx_posted_interrupts_test"
+arch = x86_64
+groups = vmx
+timeout = 10
+
[vmx_apic_passthrough_thread]
file = vmx.flat
smp = 2
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index a26f77e92f72..1a3da59632dc 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -65,6 +65,11 @@ static u32 *get_vapic_page(void)
return (u32 *)phys_to_virt(vmcs_read(APIC_VIRT_ADDR));
}
+static u64 *get_pi_desc(void)
+{
+ return (u64 *)phys_to_virt(vmcs_read(POSTED_INTR_DESC_ADDR));
+}
+
static void basic_guest_main(void)
{
report_pass("Basic VMX test");
@@ -9327,6 +9332,18 @@ static void enable_vid(void)
vmcs_set_bits(CPU_EXEC_CTRL1, CPU_VINTD | CPU_VIRT_X2APIC);
}
+#define PI_VECTOR 255
+
+static void enable_posted_interrupts(void)
+{
+ void *pi_desc = alloc_page();
+
+ vmcs_set_bits(PIN_CONTROLS, PIN_POST_INTR);
+ vmcs_set_bits(EXI_CONTROLS, EXI_INTA);
+ vmcs_write(PINV, PI_VECTOR);
+ vmcs_write(POSTED_INTR_DESC_ADDR, (u64)pi_desc);
+}
+
static void trigger_ioapic_scan_thread(void *data)
{
/* Wait until other CPU entered L2 */
@@ -10722,12 +10739,18 @@ enum Vid_op {
VID_OP_SET_CR8,
VID_OP_SELF_IPI,
VID_OP_TERMINATE,
+ VID_OP_SPIN,
+ VID_OP_HLT,
};
struct vmx_basic_vid_test_guest_args {
enum Vid_op op;
u8 nr;
u32 isr_exec_cnt;
+ u32 *virtual_apic_page;
+ u64 *pi_desc;
+ u32 dest;
+ bool in_guest;
} vmx_basic_vid_test_guest_args;
/*
@@ -10743,6 +10766,14 @@ static void set_virr_bit(volatile u32 *virtual_apic_page, u8 nr)
virtual_apic_page[page_offset] |= mask;
}
+static void clear_virr_bit(volatile u32 *virtual_apic_page, u8 nr)
+{
+ u32 page_offset = (0x200 | ((nr & 0xE0) >> 1)) / sizeof(u32);
+ u32 mask = 1 << (nr & 0x1f);
+
+ virtual_apic_page[page_offset] &= ~mask;
+}
+
static bool get_virr_bit(volatile u32 *virtual_apic_page, u8 nr)
{
u32 page_offset = (0x200 | ((nr & 0xE0) >> 1)) / sizeof(u32);
@@ -10783,6 +10814,24 @@ static void vmx_basic_vid_test_guest(void)
case VID_OP_SELF_IPI:
vmx_x2apic_write(APIC_SELF_IPI, nr);
break;
+ case VID_OP_HLT:
+ cli();
+ barrier();
+ args->in_guest = true;
+ barrier();
+ safe_halt();
+ break;
+ case VID_OP_SPIN: {
+ u32 *virtual_apic_page = args->virtual_apic_page;
+ u32 prev_cnt = args->isr_exec_cnt;
+ u8 nr = args->nr;
+
+ args->in_guest = true;
+ while (args->isr_exec_cnt == prev_cnt &&
+ !get_virr_bit(virtual_apic_page, nr))
+ pause();
+ clear_virr_bit(virtual_apic_page, nr);
+ }
default:
break;
}
@@ -10803,6 +10852,7 @@ static void set_isrs_for_vmx_basic_vid_test(void)
*/
for (nr = 0x21; nr < 0x100; nr++) {
vmcs_write(GUEST_INT_STATUS, 0);
+ args->virtual_apic_page = get_vapic_page();
args->op = VID_OP_SET_ISR;
args->nr = nr;
args->isr_exec_cnt = 0;
@@ -10812,6 +10862,27 @@ static void set_isrs_for_vmx_basic_vid_test(void)
report(true, "Set ISR for vectors 33-255.");
}
+static void post_interrupt(u8 vector, u32 dest)
+{
+ volatile struct vmx_basic_vid_test_guest_args *args =
+ &vmx_basic_vid_test_guest_args;
+
+ test_and_set_bit(vector, args->pi_desc);
+ test_and_set_bit(256, args->pi_desc);
+ apic_icr_write(PI_VECTOR, dest);
+}
+
+static void vmx_posted_interrupts_test_worker(void *data)
+{
+ volatile struct vmx_basic_vid_test_guest_args *args =
+ &vmx_basic_vid_test_guest_args;
+
+ while (!args->in_guest)
+ pause();
+
+ post_interrupt(args->nr, args->dest);
+}
+
/*
* Test virtual interrupt delivery (VID) at VM-entry or TPR virtualization
*
@@ -10843,6 +10914,7 @@ static void test_basic_vid(u8 nr, u8 tpr, enum Vid_op op, u32 isr_exec_cnt_want,
* delivery, sets VPPR to VTPR, when SVI is 0.
*/
args->isr_exec_cnt = 0;
+ args->virtual_apic_page = get_vapic_page();
args->op = op;
switch (op) {
case VID_OP_SELF_IPI:
@@ -10855,6 +10927,15 @@ static void test_basic_vid(u8 nr, u8 tpr, enum Vid_op op, u32 isr_exec_cnt_want,
args->nr = task_priority_class(tpr);
set_vtpr(0xff);
break;
+ case VID_OP_SPIN:
+ case VID_OP_HLT:
+ vmcs_write(GUEST_INT_STATUS, 0);
+ args->nr = nr;
+ set_vtpr(tpr);
+ args->in_guest = false;
+ barrier();
+ on_cpu_async(1, vmx_posted_interrupts_test_worker, NULL);
+ break;
default:
vmcs_write(GUEST_INT_STATUS, nr);
set_vtpr(tpr);
@@ -10998,6 +11079,57 @@ static void vmx_eoi_virt_test(void)
assert_exit_reason(VMX_VMCALL);
}
+static void vmx_posted_interrupts_test(void)
+{
+ volatile struct vmx_basic_vid_test_guest_args *args =
+ &vmx_basic_vid_test_guest_args;
+ u16 vector;
+ u8 class;
+
+ if (!cpu_has_apicv()) {
+ report_skip("%s : Not all required APICv bits supported", __func__);
+ return;
+ }
+
+ if (cpu_count() < 2) {
+ report_skip("%s : CPU count < 2", __func__);
+ return;
+ }
+
+ enable_vid();
+ enable_posted_interrupts();
+ args->pi_desc = get_pi_desc();
+ args->dest = apic_id();
+
+ test_set_guest(vmx_basic_vid_test_guest);
+ set_isrs_for_vmx_basic_vid_test();
+
+ for (class = 0; class < 16; class++) {
+ for (vector = 33; vector < 256; vector++) {
+ u32 isr_exec_cnt_want =
+ (task_priority_class(vector) > class) ?
+ 1 : 0;
+
+ test_basic_vid(vector, class << 4, VID_OP_SPIN,
+ isr_exec_cnt_want, false);
+
+ /*
+ * Only test posted interrupts to a halted vCPU if we
+ * expect the interrupt to be serviced. Otherwise, the
+ * vCPU could HLT indefinitely.
+ */
+ if (isr_exec_cnt_want)
+ test_basic_vid(vector, class << 4, VID_OP_HLT,
+ isr_exec_cnt_want, false);
+ }
+ }
+ report(true, "Posted vectors 33-25 cross TPR classes 0-0xf, running and sometimes halted\n");
+
+ /* Terminate the guest */
+ args->op = VID_OP_TERMINATE;
+ enter_guest();
+}
+
#define TEST(name) { #name, .v2 = name }
/* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
@@ -11054,6 +11186,7 @@ struct vmx_test vmx_tests[] = {
TEST(virt_x2apic_mode_test),
TEST(vmx_basic_vid_test),
TEST(vmx_eoi_virt_test),
+ TEST(vmx_posted_interrupts_test),
/* APIC pass-through tests */
TEST(vmx_apic_passthrough_test),
TEST(vmx_apic_passthrough_thread_test),
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [kvm-unit-tests PATCH 0/5] nVMX: Simple posted interrupts test
2023-12-11 18:55 [kvm-unit-tests PATCH 0/5] nVMX: Simple posted interrupts test Jim Mattson
` (4 preceding siblings ...)
2023-12-11 18:55 ` [kvm-unit-tests PATCH 5/5] nVMX: add test for posted interrupts Jim Mattson
@ 2024-06-05 23:20 ` Sean Christopherson
2024-10-09 6:48 ` Like Xu
6 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2024-06-05 23:20 UTC (permalink / raw)
To: Sean Christopherson, kvm, pbonzini, Jim Mattson
On Mon, 11 Dec 2023 10:55:47 -0800, Jim Mattson wrote:
> I reported recently that commit 26844fee6ade ("KVM: x86: never write to
> memory from kvm_vcpu_check_block()") broke delivery of a virtualized posted
> interrupt from an L1 vCPU to a halted L2 vCPU (see
> https://lore.kernel.org/all/20231207010302.2240506-1-jmattson@google.com/).
> The test that exposed the regression is the final patch of this series. The
> others are prerequisites.
>
> [...]
Applied to kvm-x86 next. I tweaked the commits to exclude the new tests from
the base VMX test, i.e. added:
-vmx_basic_vid_test -vmx_eoi_virt_test -vmx_posted_interrupts_test
to not duplicate coverage, and so that only the dedicated vmx_posted_intr_test
config fails.
I am hoping that landing vmx_posted_intr_test motivates me (or someone) to
actually fix that issue.
[1/5] nVMX: Enable x2APIC mode for virtual-interrupt delivery tests
https://github.com/kvm-x86/kvm-unit-tests/commit/eef1e3d21e5a
[2/5] nVMX: test nested "virtual-interrupt delivery"
https://github.com/kvm-x86/kvm-unit-tests/commit/a917f7c7e1e2
[3/5] nVMX: test nested EOI virtualization
https://github.com/kvm-x86/kvm-unit-tests/commit/d485a75d8c52
[4/5] nVMX: add self-IPI tests to vmx_basic_vid_test
https://github.com/kvm-x86/kvm-unit-tests/commit/3238da6bc526
[5/5] nVMX: add test for posted interrupts
https://github.com/kvm-x86/kvm-unit-tests/commit/765b349b6c4b
--
https://github.com/kvm-x86/kvm-unit-tests/tree/next
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [kvm-unit-tests PATCH 0/5] nVMX: Simple posted interrupts test
2023-12-11 18:55 [kvm-unit-tests PATCH 0/5] nVMX: Simple posted interrupts test Jim Mattson
` (5 preceding siblings ...)
2024-06-05 23:20 ` [kvm-unit-tests PATCH 0/5] nVMX: Simple posted interrupts test Sean Christopherson
@ 2024-10-09 6:48 ` Like Xu
2024-10-14 8:56 ` Chao Gao
6 siblings, 1 reply; 10+ messages in thread
From: Like Xu @ 2024-10-09 6:48 UTC (permalink / raw)
To: kvm; +Cc: Jim Mattson, seanjc, pbonzini
On 12/12/23 2:55 AM, Jim Mattson wrote:
> I reported recently that commit 26844fee6ade ("KVM: x86: never write to
> memory from kvm_vcpu_check_block()") broke delivery of a virtualized posted
> interrupt from an L1 vCPU to a halted L2 vCPU (see
> https://lore.kernel.org/all/20231207010302.2240506-1-jmattson@google.com/).
> The test that exposed the regression is the final patch of this series. The
> others are prerequisites.
>
> It would make sense to add "vmx_posted_interrupts_test" to the set of tests
> to be run under the unit test name, "vmx_apicv_test," but that is
> non-trivial. The vmx_posted_interrupts_test requires "smp = 2," but I find
> that adding that to the vmx_apicv_tests causes virt_x2apic_mode_test to
> fail with:
>
> FAIL: x2apic - reading 0x310: x86/vmx_tests.c:2151: Assertion failed: (expected) == (actual)
> LHS: 0x0000000000000012 - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0001'0010 - 18
> RHS: 0x0000000000000001 - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0001 - 1
> Expected VMX_VMCALL, got VMX_EXTINT.
> STACK: 406ef8 40725a 41299f 402036 403f59 4001bd
>
> I haven't investigated.
This vmx_apicv_test test still fails when 'ept=N' (SPR + v6.12-rc2):
--- Virtualize APIC accesses + Use TPR shadow test ---
FAIL: xapic - reading 0x080: read 0x0, expected 0x70.
FAIL: xapic - writing 0x12345678 to 0x080: exitless write; val is 0x0,
want 0x70
--- APIC-register virtualization test ---
FAIL: xapic - reading 0x020: read 0x0, expected 0x12345678.
FAIL: xapic - writing 0x12345678 to 0x020: x86/vmx_tests.c:2164:
Assertion failed: (expected) == (actual)
LHS: 0x0000000000000038 -
0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0011'1000
- 56
RHS: 0x0000000000000012 -
0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0001'0010
- 18
Expected VMX_APIC_WRITE, got VMX_VMCALL.
STACK: 406f7f 40d178 40202f 403f54 4001bd
>
>
> Jim Mattson (1):
> nVMX: Enable x2APIC mode for virtual-interrupt delivery tests
>
> Marc Orr (Google) (3):
> nVMX: test nested "virtual-interrupt delivery"
> nVMX: test nested EOI virtualization
> nVMX: add self-IPI tests to vmx_basic_vid_test
>
> Oliver Upton (1):
> nVMX: add test for posted interrupts
>
> lib/x86/apic.h | 5 +
> lib/x86/asm/bitops.h | 8 +
> x86/unittests.cfg | 10 +-
> x86/vmx_tests.c | 423 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 445 insertions(+), 1 deletion(-)
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [kvm-unit-tests PATCH 0/5] nVMX: Simple posted interrupts test
2024-10-09 6:48 ` Like Xu
@ 2024-10-14 8:56 ` Chao Gao
2024-10-16 15:59 ` Sean Christopherson
0 siblings, 1 reply; 10+ messages in thread
From: Chao Gao @ 2024-10-14 8:56 UTC (permalink / raw)
To: Like Xu; +Cc: kvm, Jim Mattson, seanjc, pbonzini
On Wed, Oct 09, 2024 at 02:48:28PM +0800, Like Xu wrote:
>On 12/12/23 2:55 AM, Jim Mattson wrote:
>> I reported recently that commit 26844fee6ade ("KVM: x86: never write to
>> memory from kvm_vcpu_check_block()") broke delivery of a virtualized posted
>> interrupt from an L1 vCPU to a halted L2 vCPU (see
>> https://lore.kernel.org/all/20231207010302.2240506-1-jmattson@google.com/).
>> The test that exposed the regression is the final patch of this series. The
>> others are prerequisites.
>>
>> It would make sense to add "vmx_posted_interrupts_test" to the set of tests
>> to be run under the unit test name, "vmx_apicv_test," but that is
>> non-trivial. The vmx_posted_interrupts_test requires "smp = 2," but I find
>> that adding that to the vmx_apicv_tests causes virt_x2apic_mode_test to
>> fail with:
>>
>> FAIL: x2apic - reading 0x310: x86/vmx_tests.c:2151: Assertion failed: (expected) == (actual)
>> LHS: 0x0000000000000012 - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0001'0010 - 18
>> RHS: 0x0000000000000001 - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0001 - 1
>> Expected VMX_VMCALL, got VMX_EXTINT.
>> STACK: 406ef8 40725a 41299f 402036 403f59 4001bd
>>
>> I haven't investigated.
>
>This vmx_apicv_test test still fails when 'ept=N' (SPR + v6.12-rc2):
>
>--- Virtualize APIC accesses + Use TPR shadow test ---
>FAIL: xapic - reading 0x080: read 0x0, expected 0x70.
>FAIL: xapic - writing 0x12345678 to 0x080: exitless write; val is 0x0, want
>0x70
>
>--- APIC-register virtualization test ---
>FAIL: xapic - reading 0x020: read 0x0, expected 0x12345678.
>FAIL: xapic - writing 0x12345678 to 0x020: x86/vmx_tests.c:2164: Assertion
>failed: (expected) == (actual)
> LHS: 0x0000000000000038 - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0011'1000
>- 56
> RHS: 0x0000000000000012 - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0001'0010
>- 18
>Expected VMX_APIC_WRITE, got VMX_VMCALL.
> STACK: 406f7f 40d178 40202f 403f54 4001bd
These failures occur because KVM flushes TLB with the wrong VPID, causing TLB
for the 'APIC-access page' to be retained across nested transitions. This TLB
entry exists because L1 writes to that page before entering the guest, see
test_xapic_rd():
/* Setup virtual APIC page */
if (!expectation->virtualize_apic_accesses) {
apic_access_address[apic_reg_index(reg)] = val;
virtual_apic_page[apic_reg_index(reg)] = 0;
} else if (exit_reason_want == VMX_VMCALL) {
apic_access_address[apic_reg_index(reg)] = 0;
virtual_apic_page[apic_reg_index(reg)] = val;
}
Specifically, in the failing scenario, EPT is disabled, and VPID is enabled in
L0 but disabled in L1. As a result, vmcs01 and vmcs02 share the same VPID
(vmx->vpid, see prepare_vmcs02_early_rare()), and vmx->nested.vpid02 is never
used. But during nested transitions, KVM incorrectly flushes TLB using
vmx->nested.vpid02. The sequence is as follows:
nested_vmx_transition_tlb_flush ->
kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu) ->
kvm_vcpu_flush_tlb_guest ->
vmx_flush_tlb_guest ->
vmx_get_current_vpid ->
With the diff below applied, these failures disappear.
diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h
index cce4e2aa30fb..246d9c6e20d0 100644
--- a/arch/x86/kvm/vmx/nested.h
+++ b/arch/x86/kvm/vmx/nested.h
@@ -61,13 +61,6 @@ static inline int vmx_has_valid_vmcs12(struct kvm_vcpu *vcpu)
nested_vmx_is_evmptr12_set(vmx);
}
-static inline u16 nested_get_vpid02(struct kvm_vcpu *vcpu)
-{
- struct vcpu_vmx *vmx = to_vmx(vcpu);
-
- return vmx->nested.vpid02 ? vmx->nested.vpid02 : vmx->vpid;
-}
-
static inline unsigned long nested_ept_get_eptp(struct kvm_vcpu *vcpu)
{
/* return the page table to be shadowed - in our case, EPT12 */
@@ -187,6 +180,16 @@ static inline bool nested_cpu_has_vpid(struct vmcs12 *vmcs12)
return nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENABLE_VPID);
}
+static inline u16 nested_get_vpid02(struct kvm_vcpu *vcpu)
+{
+ struct vcpu_vmx *vmx = to_vmx(vcpu);
+
+ if (nested_cpu_has_vpid(get_vmcs12(vcpu)) && vmx->nested.vpid02)
+ return vmx->nested.vpid02;
+
+ return vmx->vpid;
+}
+
static inline bool nested_cpu_has_apic_reg_virt(struct vmcs12 *vmcs12)
{
return nested_cpu_has2(vmcs12, SECONDARY_EXEC_APIC_REGISTER_VIRT);
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [kvm-unit-tests PATCH 0/5] nVMX: Simple posted interrupts test
2024-10-14 8:56 ` Chao Gao
@ 2024-10-16 15:59 ` Sean Christopherson
0 siblings, 0 replies; 10+ messages in thread
From: Sean Christopherson @ 2024-10-16 15:59 UTC (permalink / raw)
To: Chao Gao; +Cc: Like Xu, kvm, Jim Mattson, pbonzini
On Mon, Oct 14, 2024, Chao Gao wrote:
> On Wed, Oct 09, 2024 at 02:48:28PM +0800, Like Xu wrote:
> >On 12/12/23 2:55 AM, Jim Mattson wrote:
> >> I reported recently that commit 26844fee6ade ("KVM: x86: never write to
> >> memory from kvm_vcpu_check_block()") broke delivery of a virtualized posted
> >> interrupt from an L1 vCPU to a halted L2 vCPU (see
> >> https://lore.kernel.org/all/20231207010302.2240506-1-jmattson@google.com/).
> >> The test that exposed the regression is the final patch of this series. The
> >> others are prerequisites.
> >>
> >> It would make sense to add "vmx_posted_interrupts_test" to the set of tests
> >> to be run under the unit test name, "vmx_apicv_test," but that is
> >> non-trivial. The vmx_posted_interrupts_test requires "smp = 2," but I find
> >> that adding that to the vmx_apicv_tests causes virt_x2apic_mode_test to
> >> fail with:
> >>
> >> FAIL: x2apic - reading 0x310: x86/vmx_tests.c:2151: Assertion failed: (expected) == (actual)
> >> LHS: 0x0000000000000012 - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0001'0010 - 18
> >> RHS: 0x0000000000000001 - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0001 - 1
> >> Expected VMX_VMCALL, got VMX_EXTINT.
> >> STACK: 406ef8 40725a 41299f 402036 403f59 4001bd
> >>
> >> I haven't investigated.
> >
> >This vmx_apicv_test test still fails when 'ept=N' (SPR + v6.12-rc2):
> >
> >--- Virtualize APIC accesses + Use TPR shadow test ---
> >FAIL: xapic - reading 0x080: read 0x0, expected 0x70.
> >FAIL: xapic - writing 0x12345678 to 0x080: exitless write; val is 0x0, want
> >0x70
> >
> >--- APIC-register virtualization test ---
> >FAIL: xapic - reading 0x020: read 0x0, expected 0x12345678.
> >FAIL: xapic - writing 0x12345678 to 0x020: x86/vmx_tests.c:2164: Assertion
> >failed: (expected) == (actual)
> > LHS: 0x0000000000000038 - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0011'1000
> >- 56
> > RHS: 0x0000000000000012 - 0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0000'0001'0010
> >- 18
> >Expected VMX_APIC_WRITE, got VMX_VMCALL.
> > STACK: 406f7f 40d178 40202f 403f54 4001bd
>
> These failures occur because KVM flushes TLB with the wrong VPID, causing TLB
> for the 'APIC-access page' to be retained across nested transitions. This TLB
> entry exists because L1 writes to that page before entering the guest, see
> test_xapic_rd():
>
> /* Setup virtual APIC page */
> if (!expectation->virtualize_apic_accesses) {
> apic_access_address[apic_reg_index(reg)] = val;
> virtual_apic_page[apic_reg_index(reg)] = 0;
> } else if (exit_reason_want == VMX_VMCALL) {
> apic_access_address[apic_reg_index(reg)] = 0;
> virtual_apic_page[apic_reg_index(reg)] = val;
> }
>
>
> Specifically, in the failing scenario, EPT is disabled, and VPID is enabled in
> L0 but disabled in L1. As a result, vmcs01 and vmcs02 share the same VPID
> (vmx->vpid, see prepare_vmcs02_early_rare()), and vmx->nested.vpid02 is never
> used. But during nested transitions, KVM incorrectly flushes TLB using
> vmx->nested.vpid02. The sequence is as follows:
>
> nested_vmx_transition_tlb_flush ->
> kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu) ->
> kvm_vcpu_flush_tlb_guest ->
> vmx_flush_tlb_guest ->
> vmx_get_current_vpid ->
>
> With the diff below applied, these failures disappear.
>
> diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h
> index cce4e2aa30fb..246d9c6e20d0 100644
> --- a/arch/x86/kvm/vmx/nested.h
> +++ b/arch/x86/kvm/vmx/nested.h
> @@ -61,13 +61,6 @@ static inline int vmx_has_valid_vmcs12(struct kvm_vcpu *vcpu)
> nested_vmx_is_evmptr12_set(vmx);
> }
>
> -static inline u16 nested_get_vpid02(struct kvm_vcpu *vcpu)
> -{
> - struct vcpu_vmx *vmx = to_vmx(vcpu);
> -
> - return vmx->nested.vpid02 ? vmx->nested.vpid02 : vmx->vpid;
> -}
> -
> static inline unsigned long nested_ept_get_eptp(struct kvm_vcpu *vcpu)
> {
> /* return the page table to be shadowed - in our case, EPT12 */
> @@ -187,6 +180,16 @@ static inline bool nested_cpu_has_vpid(struct vmcs12 *vmcs12)
> return nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENABLE_VPID);
> }
>
> +static inline u16 nested_get_vpid02(struct kvm_vcpu *vcpu)
> +{
> + struct vcpu_vmx *vmx = to_vmx(vcpu);
> +
> + if (nested_cpu_has_vpid(get_vmcs12(vcpu)) && vmx->nested.vpid02)
This isn't quite right. When KVM emulates INVVPID for L1, there is no way to
know which vmcs12 will be used and thus no way to know if KVM should invalidate
vpid02 or vpid01. So I'm fairly certain the correct fix is:
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 1a4438358c5e..896f0fea0306 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -3216,7 +3216,7 @@ void vmx_flush_tlb_all(struct kvm_vcpu *vcpu)
static inline int vmx_get_current_vpid(struct kvm_vcpu *vcpu)
{
- if (is_guest_mode(vcpu))
+ if (is_guest_mode(vcpu) && nested_cpu_has_vpid(get_vmcs12(vcpu)))
return nested_get_vpid02(vcpu);
return to_vmx(vcpu)->vpid;
}
Note, it's tempting, but subtly wrong (though not a violation of the architecture),
to use vpid02 when VPID is disabled in vmcs12, i.e. this would also resolve the
issue, but would result in over-flushing.
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index a8e7bc04d9bf..ce89e2e27681 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -2316,7 +2316,7 @@ static void prepare_vmcs02_early_rare(struct vcpu_vmx *vmx,
vmcs_write64(VMCS_LINK_POINTER, INVALID_GPA);
if (enable_vpid) {
- if (nested_cpu_has_vpid(vmcs12) && vmx->nested.vpid02)
+ if (vmx->nested.vpid02)
vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->nested.vpid02);
else
vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->vpid);
And it's wrong because the comment in nested_vmx_transition_tlb_flush() is wrong:
* If vmcs12 doesn't use VPID, L1 expects linear and combined mappings
* for *all* contexts to be flushed on VM-Enter/VM-Exit, i.e. it's a
* full TLB flush from the guest's perspective
Per the SDM, only VPID=0 is flushed:
If the “enable VPID” VM-execution control is 0, VM entries and VM exits invalidate
linear mappings and combined mappings associated with VPID 0000H (for all PCIDs).
Combined mappings for VPID 0000H are invalidated for all EPTRTAs.
so using vpid01, which mimics using L1's host VPID (of '0'), is correct. As above,
the fallout is simply that KVM will over-flush, e.g. if L1 runs L2 X with VPID=1,
then L2 Y with VPID disabled, and then runs X again, it's legal to retain TLB
entries for VPID=1 even though there was a VMX transition with VPID disabled.
I'll post a proper patch with lots of comments.
Thanks Chao!
^ permalink raw reply related [flat|nested] 10+ messages in thread