* [kvm-unit-tests PATCH 0/2] x86: nVMX: Fix NMI/INTR-window tests @ 2019-05-08 10:27 Nadav Amit 2019-05-08 10:27 ` [kvm-unit-tests PATCH 1/2] x86: nVMX: Use #DB in nmi and intr tests Nadav Amit 2019-05-08 10:27 ` [kvm-unit-tests PATCH 2/2] x86: nVMX: Set guest as active after NMI/INTR-window tests Nadav Amit 0 siblings, 2 replies; 14+ messages in thread From: Nadav Amit @ 2019-05-08 10:27 UTC (permalink / raw) To: Paolo Bonzini; +Cc: kvm, Nadav Amit, Jim Mattson, Sean Christopherson From: Nadav Amit <nadav.amit@gmail.com> The NMI/INTR-window test in VMX have a couple of bugs. The first bug is clearly a test issue (and a KVM bug), and is fixed by the first patch. The second bug is more interesting, and looks as a silicon bug (or documentation bug, as Intel likes to call them). The second patch just ensures that a failure of the test would allow the next tests to run. Cc: Jim Mattson <jmattson@google.com> Cc: Sean Christopherson <sean.j.christopherson@intel.com> Nadav Amit (2): x86: nVMX: Use #DB in nmi and intr tests x86: nVMX: Set guest as active after NMI/INTR-window tests x86/vmx_tests.c | 74 +++++++++++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 36 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [kvm-unit-tests PATCH 1/2] x86: nVMX: Use #DB in nmi and intr tests 2019-05-08 10:27 [kvm-unit-tests PATCH 0/2] x86: nVMX: Fix NMI/INTR-window tests Nadav Amit @ 2019-05-08 10:27 ` Nadav Amit 2019-05-08 23:11 ` Jim Mattson 2019-05-08 10:27 ` [kvm-unit-tests PATCH 2/2] x86: nVMX: Set guest as active after NMI/INTR-window tests Nadav Amit 1 sibling, 1 reply; 14+ messages in thread From: Nadav Amit @ 2019-05-08 10:27 UTC (permalink / raw) To: Paolo Bonzini; +Cc: kvm, Nadav Amit, Jim Mattson, Sean Christopherson From: Nadav Amit <nadav.amit@gmail.com> According to Intel SDM 26.3.1.5 "Checks on Guest Non-Register State", if the activity state is HLT, the only events that can be injected are NMI, MTF and "Those with interruption type hardware exception and vector 1 (debug exception) or vector 18 (machine-check exception)." Two tests, verify_nmi_window_exit() and verify_intr_window_exit(), try to do something that real hardware disallows (i.e., fail the VM-entry) by injecting #UD in HLT-state. Inject #DB instead as the injection should succeed in these tests. Cc: Jim Mattson <jmattson@google.com> Cc: Sean Christopherson <sean.j.christopherson@intel.com> Signed-off-by: Nadav Amit <nadav.amit@gmail.com> --- x86/vmx_tests.c | 72 ++++++++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c index d0ce1af..f921286 100644 --- a/x86/vmx_tests.c +++ b/x86/vmx_tests.c @@ -7035,21 +7035,21 @@ static void vmx_pending_event_hlt_test(void) vmx_pending_event_test_core(true); } -static int vmx_window_test_ud_count; +static int vmx_window_test_db_count; -static void vmx_window_test_ud_handler(struct ex_regs *regs) +static void vmx_window_test_db_handler(struct ex_regs *regs) { - vmx_window_test_ud_count++; + vmx_window_test_db_count++; } static void vmx_nmi_window_test_guest(void) { - handle_exception(UD_VECTOR, vmx_window_test_ud_handler); + handle_exception(DB_VECTOR, vmx_window_test_db_handler); asm volatile("vmcall\n\t" "nop\n\t"); - handle_exception(UD_VECTOR, NULL); + handle_exception(DB_VECTOR, NULL); } static void verify_nmi_window_exit(u64 rip) @@ -7068,7 +7068,7 @@ static void verify_nmi_window_exit(u64 rip) static void vmx_nmi_window_test(void) { u64 nop_addr; - void *ud_fault_addr = get_idt_addr(&boot_idt[UD_VECTOR]); + void *db_fault_addr = get_idt_addr(&boot_idt[DB_VECTOR]); if (!(ctrl_pin_rev.clr & PIN_VIRT_NMI)) { report_skip("CPU does not support the \"Virtual NMIs\" VM-execution control."); @@ -7080,7 +7080,7 @@ static void vmx_nmi_window_test(void) return; } - vmx_window_test_ud_count = 0; + vmx_window_test_db_count = 0; report_prefix_push("NMI-window"); test_set_guest(vmx_nmi_window_test_guest); @@ -7113,27 +7113,27 @@ static void vmx_nmi_window_test(void) /* * Ask for "NMI-window exiting" (with event injection), and * expect a VM-exit after the event is injected. (RIP should - * be at the address specified in the IDT entry for #UD.) + * be at the address specified in the IDT entry for #DB.) */ - report_prefix_push("active, no blocking, injecting #UD"); + report_prefix_push("active, no blocking, injecting #DB"); vmcs_write(ENT_INTR_INFO, - INTR_INFO_VALID_MASK | INTR_TYPE_HARD_EXCEPTION | UD_VECTOR); + INTR_INFO_VALID_MASK | INTR_TYPE_HARD_EXCEPTION | DB_VECTOR); enter_guest(); - verify_nmi_window_exit((u64)ud_fault_addr); + verify_nmi_window_exit((u64)db_fault_addr); report_prefix_pop(); /* * Ask for "NMI-window exiting" with NMI blocking, and expect - * a VM-exit after the next IRET (i.e. after the #UD handler + * a VM-exit after the next IRET (i.e. after the #DB handler * returns). So, RIP should be back at one byte past the nop. */ report_prefix_push("active, blocking by NMI"); vmcs_write(GUEST_INTR_STATE, GUEST_INTR_STATE_NMI); enter_guest(); verify_nmi_window_exit(nop_addr + 1); - report("#UD handler executed once (actual %d times)", - vmx_window_test_ud_count == 1, - vmx_window_test_ud_count); + report("#DB handler executed once (actual %d times)", + vmx_window_test_db_count == 1, + vmx_window_test_db_count); report_prefix_pop(); if (!(rdmsr(MSR_IA32_VMX_MISC) & (1 << 6))) { @@ -7154,15 +7154,15 @@ static void vmx_nmi_window_test(void) * Ask for "NMI-window exiting" when entering activity * state HLT (with event injection), and expect a * VM-exit after the event is injected. (RIP should be - * at the address specified in the IDT entry for #UD.) + * at the address specified in the IDT entry for #DB.) */ - report_prefix_push("halted, no blocking, injecting #UD"); + report_prefix_push("halted, no blocking, injecting #DB"); vmcs_write(GUEST_ACTV_STATE, ACTV_HLT); vmcs_write(ENT_INTR_INFO, INTR_INFO_VALID_MASK | INTR_TYPE_HARD_EXCEPTION | - UD_VECTOR); + DB_VECTOR); enter_guest(); - verify_nmi_window_exit((u64)ud_fault_addr); + verify_nmi_window_exit((u64)db_fault_addr); report_prefix_pop(); } @@ -7173,7 +7173,7 @@ static void vmx_nmi_window_test(void) static void vmx_intr_window_test_guest(void) { - handle_exception(UD_VECTOR, vmx_window_test_ud_handler); + handle_exception(DB_VECTOR, vmx_window_test_db_handler); /* * The two consecutive STIs are to ensure that only the first @@ -7185,7 +7185,7 @@ static void vmx_intr_window_test_guest(void) "sti\n\t" "sti\n\t"); - handle_exception(UD_VECTOR, NULL); + handle_exception(DB_VECTOR, NULL); } static void verify_intr_window_exit(u64 rip) @@ -7205,8 +7205,8 @@ static void vmx_intr_window_test(void) { u64 vmcall_addr; u64 nop_addr; - unsigned int orig_ud_gate_type; - void *ud_fault_addr = get_idt_addr(&boot_idt[UD_VECTOR]); + unsigned int orig_db_gate_type; + void *db_fault_addr = get_idt_addr(&boot_idt[DB_VECTOR]); if (!(ctrl_cpu_rev[0].clr & CPU_INTR_WINDOW)) { report_skip("CPU does not support the \"interrupt-window exiting\" VM-execution control."); @@ -7214,12 +7214,12 @@ static void vmx_intr_window_test(void) } /* - * Change the IDT entry for #UD from interrupt gate to trap gate, + * Change the IDT entry for #DB from interrupt gate to trap gate, * so that it won't clear RFLAGS.IF. We don't want interrupts to - * be disabled after vectoring a #UD. + * be disabled after vectoring a #DB. */ - orig_ud_gate_type = boot_idt[UD_VECTOR].type; - boot_idt[UD_VECTOR].type = 15; + orig_db_gate_type = boot_idt[DB_VECTOR].type; + boot_idt[DB_VECTOR].type = 15; report_prefix_push("interrupt-window"); test_set_guest(vmx_intr_window_test_guest); @@ -7244,14 +7244,14 @@ static void vmx_intr_window_test(void) * Ask for "interrupt-window exiting" (with event injection) * with RFLAGS.IF set and no blocking; expect a VM-exit after * the event is injected. That is, RIP should should be at the - * address specified in the IDT entry for #UD. + * address specified in the IDT entry for #DB. */ - report_prefix_push("active, no blocking, RFLAGS.IF=1, injecting #UD"); + report_prefix_push("active, no blocking, RFLAGS.IF=1, injecting #DB"); vmcs_write(ENT_INTR_INFO, - INTR_INFO_VALID_MASK | INTR_TYPE_HARD_EXCEPTION | UD_VECTOR); + INTR_INFO_VALID_MASK | INTR_TYPE_HARD_EXCEPTION | DB_VECTOR); vmcall_addr = vmcs_read(GUEST_RIP); enter_guest(); - verify_intr_window_exit((u64)ud_fault_addr); + verify_intr_window_exit((u64)db_fault_addr); report_prefix_pop(); /* @@ -7323,19 +7323,19 @@ static void vmx_intr_window_test(void) * activity state HLT (with event injection), and * expect a VM-exit after the event is injected. That * is, RIP should should be at the address specified - * in the IDT entry for #UD. + * in the IDT entry for #DB. */ - report_prefix_push("halted, no blocking, injecting #UD"); + report_prefix_push("halted, no blocking, injecting #DB"); vmcs_write(GUEST_ACTV_STATE, ACTV_HLT); vmcs_write(ENT_INTR_INFO, INTR_INFO_VALID_MASK | INTR_TYPE_HARD_EXCEPTION | - UD_VECTOR); + DB_VECTOR); enter_guest(); - verify_intr_window_exit((u64)ud_fault_addr); + verify_intr_window_exit((u64)db_fault_addr); report_prefix_pop(); } - boot_idt[UD_VECTOR].type = orig_ud_gate_type; + boot_idt[DB_VECTOR].type = orig_db_gate_type; vmcs_clear_bits(CPU_EXEC_CTRL0, CPU_INTR_WINDOW); enter_guest(); report_prefix_pop(); -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] x86: nVMX: Use #DB in nmi and intr tests 2019-05-08 10:27 ` [kvm-unit-tests PATCH 1/2] x86: nVMX: Use #DB in nmi and intr tests Nadav Amit @ 2019-05-08 23:11 ` Jim Mattson 2019-05-08 23:35 ` Nadav Amit 0 siblings, 1 reply; 14+ messages in thread From: Jim Mattson @ 2019-05-08 23:11 UTC (permalink / raw) To: Nadav Amit; +Cc: Paolo Bonzini, kvm list, Sean Christopherson From: Nadav Amit <nadav.amit@gmail.com> Date: Wed, May 8, 2019 at 10:47 AM To: Paolo Bonzini Cc: <kvm@vger.kernel.org>, Nadav Amit, Jim Mattson, Sean Christopherson > From: Nadav Amit <nadav.amit@gmail.com> > > According to Intel SDM 26.3.1.5 "Checks on Guest Non-Register State", if > the activity state is HLT, the only events that can be injected are NMI, > MTF and "Those with interruption type hardware exception and vector 1 > (debug exception) or vector 18 (machine-check exception)." > > Two tests, verify_nmi_window_exit() and verify_intr_window_exit(), try > to do something that real hardware disallows (i.e., fail the VM-entry) > by injecting #UD in HLT-state. Inject #DB instead as the injection > should succeed in these tests. > > Cc: Jim Mattson <jmattson@google.com> > Cc: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Nadav Amit <nadav.amit@gmail.com> Reviewed-by: Jim Mattson <jmattson@google.com> Thanks for the fix! It has always bothered me that there is no easy way to validate a kvm-unit-test on physical hardware. Do you have a mechanism for doing so? If so, would you be willing to share? I don't suppose you have a patch for kvm to fail the VM-entry in this case? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] x86: nVMX: Use #DB in nmi and intr tests 2019-05-08 23:11 ` Jim Mattson @ 2019-05-08 23:35 ` Nadav Amit 2019-05-20 15:52 ` Paolo Bonzini 0 siblings, 1 reply; 14+ messages in thread From: Nadav Amit @ 2019-05-08 23:35 UTC (permalink / raw) To: Jim Mattson; +Cc: Paolo Bonzini, kvm list, Sean Christopherson > On May 8, 2019, at 4:11 PM, Jim Mattson <jmattson@google.com> wrote: > > From: Nadav Amit <nadav.amit@gmail.com> > Date: Wed, May 8, 2019 at 10:47 AM > To: Paolo Bonzini > Cc: <kvm@vger.kernel.org>, Nadav Amit, Jim Mattson, Sean Christopherson > >> From: Nadav Amit <nadav.amit@gmail.com> >> >> According to Intel SDM 26.3.1.5 "Checks on Guest Non-Register State", if >> the activity state is HLT, the only events that can be injected are NMI, >> MTF and "Those with interruption type hardware exception and vector 1 >> (debug exception) or vector 18 (machine-check exception)." >> >> Two tests, verify_nmi_window_exit() and verify_intr_window_exit(), try >> to do something that real hardware disallows (i.e., fail the VM-entry) >> by injecting #UD in HLT-state. Inject #DB instead as the injection >> should succeed in these tests. >> >> Cc: Jim Mattson <jmattson@google.com> >> Cc: Sean Christopherson <sean.j.christopherson@intel.com> >> Signed-off-by: Nadav Amit <nadav.amit@gmail.com> > Reviewed-by: Jim Mattson <jmattson@google.com> > > Thanks for the fix! > > It has always bothered me that there is no easy way to validate a > kvm-unit-test on physical hardware. Do you have a mechanism for doing > so? If so, would you be willing to share? I call this mechanism “grub”. ;-) If you saw my recent kvm-unit-tests patches - they are needed to run kvm-unit-tests on physical hardware. Once I am done sending the remaining fixes, I’ll send an RFC that enable test execution on physical hardware (e.g., by skipping tests that require test devices). I just hope that this support would convince you, and others, to prefer (when possible) kvm-unit-tests over the selftest environment. > I don't suppose you have a patch for kvm to fail the VM-entry in this case? I am trying to keep my day job. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] x86: nVMX: Use #DB in nmi and intr tests 2019-05-08 23:35 ` Nadav Amit @ 2019-05-20 15:52 ` Paolo Bonzini 2019-05-20 16:39 ` Nadav Amit 0 siblings, 1 reply; 14+ messages in thread From: Paolo Bonzini @ 2019-05-20 15:52 UTC (permalink / raw) To: Nadav Amit, Jim Mattson; +Cc: kvm list, Sean Christopherson On 09/05/19 01:35, Nadav Amit wrote: > I just hope that this support would convince you, and others, to prefer > (when possible) kvm-unit-tests over the selftest environment. kvm-unit-tests are not superseded by selftests; selftests are mostly meant to test the KVM API. While they are more easily debuggable than kvm-unit-tests, the benefit is not big enough to justify the effort of rewriting everything. Furthermore, being able to run kvm-unit-tests on bare metal is useful, even if it's not something that people commonly do, and consistent with KVM's design of not departing radically for what bare metal does. Thanks, Paolo ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] x86: nVMX: Use #DB in nmi and intr tests 2019-05-20 15:52 ` Paolo Bonzini @ 2019-05-20 16:39 ` Nadav Amit 2019-05-20 17:21 ` Paolo Bonzini 0 siblings, 1 reply; 14+ messages in thread From: Nadav Amit @ 2019-05-20 16:39 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Jim Mattson, kvm list, Sean Christopherson > On May 20, 2019, at 8:52 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 09/05/19 01:35, Nadav Amit wrote: >> I just hope that this support would convince you, and others, to prefer >> (when possible) kvm-unit-tests over the selftest environment. > > kvm-unit-tests are not superseded by selftests; selftests are mostly > meant to test the KVM API. While they are more easily debuggable than > kvm-unit-tests, the benefit is not big enough to justify the effort of > rewriting everything. > > Furthermore, being able to run kvm-unit-tests on bare metal is useful, > even if it's not something that people commonly do, and consistent with > KVM's design of not departing radically for what bare metal does. I understand. And just to clarify - my corporate overload deserves the credit for this work. I just prefer not to shout it too loudly. I’m sorry for not collecting the patches into a set, but I know that it would just cause me to resend all of them for individual patch issue. I still have some more pending patches that I will send after rebasing and cleanup. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] x86: nVMX: Use #DB in nmi and intr tests 2019-05-20 16:39 ` Nadav Amit @ 2019-05-20 17:21 ` Paolo Bonzini 0 siblings, 0 replies; 14+ messages in thread From: Paolo Bonzini @ 2019-05-20 17:21 UTC (permalink / raw) To: Nadav Amit; +Cc: Jim Mattson, kvm list, Sean Christopherson On 20/05/19 18:39, Nadav Amit wrote: > I’m sorry for not collecting the patches into a set, but I know that it > would just cause me to resend all of them for individual patch issue. All the patches you've sent make sense individually. Thanks to you (and the corporate overlord ;)) for doing the work and for sharing it, really. Paolo ^ permalink raw reply [flat|nested] 14+ messages in thread
* [kvm-unit-tests PATCH 2/2] x86: nVMX: Set guest as active after NMI/INTR-window tests 2019-05-08 10:27 [kvm-unit-tests PATCH 0/2] x86: nVMX: Fix NMI/INTR-window tests Nadav Amit 2019-05-08 10:27 ` [kvm-unit-tests PATCH 1/2] x86: nVMX: Use #DB in nmi and intr tests Nadav Amit @ 2019-05-08 10:27 ` Nadav Amit 2019-05-08 23:21 ` Jim Mattson 2019-05-09 20:32 ` Sean Christopherson 1 sibling, 2 replies; 14+ messages in thread From: Nadav Amit @ 2019-05-08 10:27 UTC (permalink / raw) To: Paolo Bonzini; +Cc: kvm, Nadav Amit, Jim Mattson, Sean Christopherson From: Nadav Amit <nadav.amit@gmail.com> Intel SDM 26.6.5 says regarding interrupt-window exiting that: "These events wake the logical processor if it just entered the HLT state because of a VM entry." A similar statement is told about NMI-window exiting. However, running tests which are similar to verify_nmi_window_exit() and verify_intr_window_exit() on bare-metal suggests that real CPUs do not wake up. Until someone figures what the correct behavior is, just reset the activity state to "active" after each test to prevent the whole test-suite from getting stuck. Cc: Jim Mattson <jmattson@google.com> Cc: Sean Christopherson <sean.j.christopherson@intel.com> Signed-off-by: Nadav Amit <nadav.amit@gmail.com> --- x86/vmx_tests.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c index f921286..2d6b12d 100644 --- a/x86/vmx_tests.c +++ b/x86/vmx_tests.c @@ -7063,6 +7063,7 @@ static void verify_nmi_window_exit(u64 rip) report("Activity state (%ld) is 'ACTIVE'", vmcs_read(GUEST_ACTV_STATE) == ACTV_ACTIVE, vmcs_read(GUEST_ACTV_STATE)); + vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE); } static void vmx_nmi_window_test(void) @@ -7199,6 +7200,7 @@ static void verify_intr_window_exit(u64 rip) report("Activity state (%ld) is 'ACTIVE'", vmcs_read(GUEST_ACTV_STATE) == ACTV_ACTIVE, vmcs_read(GUEST_ACTV_STATE)); + vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE); } static void vmx_intr_window_test(void) -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] x86: nVMX: Set guest as active after NMI/INTR-window tests 2019-05-08 10:27 ` [kvm-unit-tests PATCH 2/2] x86: nVMX: Set guest as active after NMI/INTR-window tests Nadav Amit @ 2019-05-08 23:21 ` Jim Mattson 2019-05-08 23:38 ` Nadav Amit 2019-05-09 20:32 ` Sean Christopherson 1 sibling, 1 reply; 14+ messages in thread From: Jim Mattson @ 2019-05-08 23:21 UTC (permalink / raw) To: Nadav Amit; +Cc: Paolo Bonzini, kvm list, Sean Christopherson From: Nadav Amit <nadav.amit@gmail.com> Date: Wed, May 8, 2019 at 10:47 AM To: Paolo Bonzini Cc: <kvm@vger.kernel.org>, Nadav Amit, Jim Mattson, Sean Christopherson > From: Nadav Amit <nadav.amit@gmail.com> > > Intel SDM 26.6.5 says regarding interrupt-window exiting that: "These > events wake the logical processor if it just entered the HLT state > because of a VM entry." A similar statement is told about NMI-window > exiting. > > However, running tests which are similar to verify_nmi_window_exit() and > verify_intr_window_exit() on bare-metal suggests that real CPUs do not > wake up. Until someone figures what the correct behavior is, just reset > the activity state to "active" after each test to prevent the whole > test-suite from getting stuck. > > Cc: Jim Mattson <jmattson@google.com> > Cc: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Nadav Amit <nadav.amit@gmail.com> Reviewed-by: Jim Mattson <jmattson@google.com> I think I have been assuming that "wake the logical processor" means "causes the logical processor to enter the 'active' activity state." Maybe that's not what "wake" means? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] x86: nVMX: Set guest as active after NMI/INTR-window tests 2019-05-08 23:21 ` Jim Mattson @ 2019-05-08 23:38 ` Nadav Amit 2019-05-09 20:48 ` Sean Christopherson 0 siblings, 1 reply; 14+ messages in thread From: Nadav Amit @ 2019-05-08 23:38 UTC (permalink / raw) To: Jim Mattson; +Cc: Paolo Bonzini, kvm list, Sean Christopherson > On May 8, 2019, at 4:21 PM, Jim Mattson <jmattson@google.com> wrote: > > From: Nadav Amit <nadav.amit@gmail.com> > Date: Wed, May 8, 2019 at 10:47 AM > To: Paolo Bonzini > Cc: <kvm@vger.kernel.org>, Nadav Amit, Jim Mattson, Sean Christopherson > >> From: Nadav Amit <nadav.amit@gmail.com> >> >> Intel SDM 26.6.5 says regarding interrupt-window exiting that: "These >> events wake the logical processor if it just entered the HLT state >> because of a VM entry." A similar statement is told about NMI-window >> exiting. >> >> However, running tests which are similar to verify_nmi_window_exit() and >> verify_intr_window_exit() on bare-metal suggests that real CPUs do not >> wake up. Until someone figures what the correct behavior is, just reset >> the activity state to "active" after each test to prevent the whole >> test-suite from getting stuck. >> >> Cc: Jim Mattson <jmattson@google.com> >> Cc: Sean Christopherson <sean.j.christopherson@intel.com> >> Signed-off-by: Nadav Amit <nadav.amit@gmail.com> > Reviewed-by: Jim Mattson <jmattson@google.com> > > I think I have been assuming that "wake the logical processor" means > "causes the logical processor to enter the 'active' activity state." > Maybe that's not what "wake" means? I really don’t know. Reading the specifications, I thought that the test is valid. I don’t manage to read it any differently than you did. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] x86: nVMX: Set guest as active after NMI/INTR-window tests 2019-05-08 23:38 ` Nadav Amit @ 2019-05-09 20:48 ` Sean Christopherson 0 siblings, 0 replies; 14+ messages in thread From: Sean Christopherson @ 2019-05-09 20:48 UTC (permalink / raw) To: Nadav Amit; +Cc: Jim Mattson, Paolo Bonzini, kvm list On Wed, May 08, 2019 at 04:38:10PM -0700, Nadav Amit wrote: > > On May 8, 2019, at 4:21 PM, Jim Mattson <jmattson@google.com> wrote: > > > > From: Nadav Amit <nadav.amit@gmail.com> > > Date: Wed, May 8, 2019 at 10:47 AM > > To: Paolo Bonzini > > Cc: <kvm@vger.kernel.org>, Nadav Amit, Jim Mattson, Sean Christopherson > > > >> From: Nadav Amit <nadav.amit@gmail.com> > >> > >> Intel SDM 26.6.5 says regarding interrupt-window exiting that: "These > >> events wake the logical processor if it just entered the HLT state > >> because of a VM entry." A similar statement is told about NMI-window > >> exiting. > >> > >> However, running tests which are similar to verify_nmi_window_exit() and > >> verify_intr_window_exit() on bare-metal suggests that real CPUs do not > >> wake up. Until someone figures what the correct behavior is, just reset > >> the activity state to "active" after each test to prevent the whole > >> test-suite from getting stuck. > >> > >> Cc: Jim Mattson <jmattson@google.com> > >> Cc: Sean Christopherson <sean.j.christopherson@intel.com> > >> Signed-off-by: Nadav Amit <nadav.amit@gmail.com> > > Reviewed-by: Jim Mattson <jmattson@google.com> > > > > I think I have been assuming that "wake the logical processor" means > > "causes the logical processor to enter the 'active' activity state." > > Maybe that's not what "wake" means? > > I really don’t know. Reading the specifications, I thought that the test is > valid. I don’t manage to read it any differently than you did. "logic processor" in this context means the physical CPU, it doesn't imply anything about what gets saved into the VMCS. I assume the purpose of that blurb is to make it clear that the guest won't get stuck in HLT state if there's a virtual interrupt pending. The relevant SDM section is "Saving Non-Register State": The activity-state field is saved with the logical processor's activity state before the VM exit[1]. See Section 27.1 for details of how events leading to a VM exit may affect the activity state. The revelant bits of Section 27.1 - "Architectural State Before a VM Exit": If the logical processor is in an inactive state and not executing instructions, some events may be blocked but other may return the logical processor to the active state. Unblocked events may cause VM exits. If an unblocked event causes a VM exit directly, a return to the active state occurs only after the VM exit completes. <more irrevelant words> In other words, because the CPU was in HLT before VM-Exit, that's what gets saved into the VMCS. My guess is that the behavior is defined this way because technically the vCPU hasn't received a wake event, the VMM has simply requested a VM Exit. The wake event (from the vCPU's perspective) comes when the VMM actually injects an interrupt/NMI. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] x86: nVMX: Set guest as active after NMI/INTR-window tests 2019-05-08 10:27 ` [kvm-unit-tests PATCH 2/2] x86: nVMX: Set guest as active after NMI/INTR-window tests Nadav Amit 2019-05-08 23:21 ` Jim Mattson @ 2019-05-09 20:32 ` Sean Christopherson 2019-05-09 21:29 ` Nadav Amit 1 sibling, 1 reply; 14+ messages in thread From: Sean Christopherson @ 2019-05-09 20:32 UTC (permalink / raw) To: Nadav Amit; +Cc: Paolo Bonzini, kvm, Jim Mattson On Wed, May 08, 2019 at 03:27:15AM -0700, Nadav Amit wrote: > From: Nadav Amit <nadav.amit@gmail.com> > > Intel SDM 26.6.5 says regarding interrupt-window exiting that: "These > events wake the logical processor if it just entered the HLT state > because of a VM entry." A similar statement is told about NMI-window > exiting. > > However, running tests which are similar to verify_nmi_window_exit() and > verify_intr_window_exit() on bare-metal suggests that real CPUs do not > wake up. Until someone figures what the correct behavior is, just reset > the activity state to "active" after each test to prevent the whole > test-suite from getting stuck. > > Cc: Jim Mattson <jmattson@google.com> > Cc: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Nadav Amit <nadav.amit@gmail.com> > --- > x86/vmx_tests.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index f921286..2d6b12d 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -7063,6 +7063,7 @@ static void verify_nmi_window_exit(u64 rip) > report("Activity state (%ld) is 'ACTIVE'", > vmcs_read(GUEST_ACTV_STATE) == ACTV_ACTIVE, > vmcs_read(GUEST_ACTV_STATE)); > + vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE); Don't you need to remove (or modify) the above report() as well to avoid failing the current test? > } > > static void vmx_nmi_window_test(void) > @@ -7199,6 +7200,7 @@ static void verify_intr_window_exit(u64 rip) > report("Activity state (%ld) is 'ACTIVE'", > vmcs_read(GUEST_ACTV_STATE) == ACTV_ACTIVE, > vmcs_read(GUEST_ACTV_STATE)); > + vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE); > } > > static void vmx_intr_window_test(void) > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] x86: nVMX: Set guest as active after NMI/INTR-window tests 2019-05-09 20:32 ` Sean Christopherson @ 2019-05-09 21:29 ` Nadav Amit 2019-05-15 16:57 ` Sean Christopherson 0 siblings, 1 reply; 14+ messages in thread From: Nadav Amit @ 2019-05-09 21:29 UTC (permalink / raw) To: Sean Christopherson; +Cc: Paolo Bonzini, kvm, Jim Mattson > On May 9, 2019, at 1:32 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > On Wed, May 08, 2019 at 03:27:15AM -0700, Nadav Amit wrote: >> From: Nadav Amit <nadav.amit@gmail.com> >> >> Intel SDM 26.6.5 says regarding interrupt-window exiting that: "These >> events wake the logical processor if it just entered the HLT state >> because of a VM entry." A similar statement is told about NMI-window >> exiting. >> >> However, running tests which are similar to verify_nmi_window_exit() and >> verify_intr_window_exit() on bare-metal suggests that real CPUs do not >> wake up. Until someone figures what the correct behavior is, just reset >> the activity state to "active" after each test to prevent the whole >> test-suite from getting stuck. >> >> Cc: Jim Mattson <jmattson@google.com> >> Cc: Sean Christopherson <sean.j.christopherson@intel.com> >> Signed-off-by: Nadav Amit <nadav.amit@gmail.com> >> --- >> x86/vmx_tests.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c >> index f921286..2d6b12d 100644 >> --- a/x86/vmx_tests.c >> +++ b/x86/vmx_tests.c >> @@ -7063,6 +7063,7 @@ static void verify_nmi_window_exit(u64 rip) >> report("Activity state (%ld) is 'ACTIVE'", >> vmcs_read(GUEST_ACTV_STATE) == ACTV_ACTIVE, >> vmcs_read(GUEST_ACTV_STATE)); >> + vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE); > > Don't you need to remove (or modify) the above report() as well to avoid > failing the current test? Thanks for checking it (in your second email). So should I remove this test completely for v2? Or do you have any different test you want to run? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] x86: nVMX: Set guest as active after NMI/INTR-window tests 2019-05-09 21:29 ` Nadav Amit @ 2019-05-15 16:57 ` Sean Christopherson 0 siblings, 0 replies; 14+ messages in thread From: Sean Christopherson @ 2019-05-15 16:57 UTC (permalink / raw) To: Nadav Amit; +Cc: Paolo Bonzini, kvm, Jim Mattson On Thu, May 09, 2019 at 02:29:45PM -0700, Nadav Amit wrote: > > On May 9, 2019, at 1:32 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > > > On Wed, May 08, 2019 at 03:27:15AM -0700, Nadav Amit wrote: > >> From: Nadav Amit <nadav.amit@gmail.com> > >> > >> Intel SDM 26.6.5 says regarding interrupt-window exiting that: "These > >> events wake the logical processor if it just entered the HLT state > >> because of a VM entry." A similar statement is told about NMI-window > >> exiting. > >> > >> However, running tests which are similar to verify_nmi_window_exit() and > >> verify_intr_window_exit() on bare-metal suggests that real CPUs do not > >> wake up. Until someone figures what the correct behavior is, just reset > >> the activity state to "active" after each test to prevent the whole > >> test-suite from getting stuck. > >> > >> Cc: Jim Mattson <jmattson@google.com> > >> Cc: Sean Christopherson <sean.j.christopherson@intel.com> > >> Signed-off-by: Nadav Amit <nadav.amit@gmail.com> > >> --- > >> x86/vmx_tests.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > >> index f921286..2d6b12d 100644 > >> --- a/x86/vmx_tests.c > >> +++ b/x86/vmx_tests.c > >> @@ -7063,6 +7063,7 @@ static void verify_nmi_window_exit(u64 rip) > >> report("Activity state (%ld) is 'ACTIVE'", > >> vmcs_read(GUEST_ACTV_STATE) == ACTV_ACTIVE, > >> vmcs_read(GUEST_ACTV_STATE)); > >> + vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE); > > > > Don't you need to remove (or modify) the above report() as well to avoid > > failing the current test? > > Thanks for checking it (in your second email). > > So should I remove this test completely for v2? Or do you have any different > test you want to run? I'd say just remove the activity state check. KVM is technically broken, and is unlikely to be fixed any time soon. I don't see much value in adding more code to the test just to highlight that KVM doesn't strictly adhere to the SDM for activity state transitions. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-05-20 17:21 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-08 10:27 [kvm-unit-tests PATCH 0/2] x86: nVMX: Fix NMI/INTR-window tests Nadav Amit 2019-05-08 10:27 ` [kvm-unit-tests PATCH 1/2] x86: nVMX: Use #DB in nmi and intr tests Nadav Amit 2019-05-08 23:11 ` Jim Mattson 2019-05-08 23:35 ` Nadav Amit 2019-05-20 15:52 ` Paolo Bonzini 2019-05-20 16:39 ` Nadav Amit 2019-05-20 17:21 ` Paolo Bonzini 2019-05-08 10:27 ` [kvm-unit-tests PATCH 2/2] x86: nVMX: Set guest as active after NMI/INTR-window tests Nadav Amit 2019-05-08 23:21 ` Jim Mattson 2019-05-08 23:38 ` Nadav Amit 2019-05-09 20:48 ` Sean Christopherson 2019-05-09 20:32 ` Sean Christopherson 2019-05-09 21:29 ` Nadav Amit 2019-05-15 16:57 ` 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.