x86: nVMX: Fix NMI/INTR-window tests
v2 cover x86: nVMX: Fix NMI/INTR-window tests
NMI/INTR-window test in VMX have a couple of bugs. Each of the patches fixes one. The first version of this patch-set claimed that one of the bugs might be a silicon issue. However, according to Sean, it is a just a test and KVM issue, once you read the SDM more carefully.
Cc Jim Mattson <[email protected]>
Cc Sean Christopherson <[email protected]>
Nadav Amit (2):
x86: nVMX: Use #DB in nmi- and intr-window tests
x86: nVMX: Set guest as active after NMI/INTR-window tests
x86/vmx_tests.c | 80 +++++++++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 42 deletions(-)
Signature · 1 line
2.17.1
PATCH v2 1/2 x86: nVMX: Use #DB in nmi- and intr-window tests
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)."
Theverify_nmi_window_exit() and verify_intr_window_exit() tests 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 Sean Christopherson <[email protected]>
Reviewed-by Jim Mattson <[email protected]>
Signed-off-by Nadav Amit <[email protected]>
--- x86/vmx_tests.c | 72 ++++++++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 36 deletions(-)
x86/vmx_tests.c +36 -36
@@ -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); asmvolatile ("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 ();
Signature · 1 line
2.17.1
PATCH v2 2/2 x86: nVMX: Set guest as active after NMI/INTR-window tests
In reply to Nadav AmitRunning 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. It appears, according to Sean, that the activity state should not change after NMI/INTR-window.
Remove the offending test and set the activity state to "active" after each test to prevent the whole test-suite from getting stuck.
Cc Sean Christopherson <[email protected]>
Reviewed-by Jim Mattson <[email protected]>
Signed-off-by Nadav Amit <[email protected]>
--- x86/vmx_tests.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
x86/vmx_tests.c +2 -6
@@ -7060,9 +7060,7 @@ static void verify_nmi_window_exit(u64 rip) exit_reason == VMX_NMI_WINDOW, exit_reason);report ("RIP (%#lx) is %#lx" ,vmcs_read (GUEST_RIP ) == rip,vmcs_read (GUEST_RIP ), 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 )
@@ -7196,9 +7194,7 @@ static void verify_intr_window_exit(u64 rip) exit_reason == VMX_INTR_WINDOW, exit_reason);report ("RIP (%#lx) is %#lx" ,vmcs_read (GUEST_RIP ) == rip,vmcs_read (GUEST_RIP ), 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 )
Signature · 1 line
2.17.1
Quoted 8 lines · Nadav Amit
NMI/INTR-window test in VMX have a couple of bugs. Each of the patches fixes one. The first version of this patch-set claimed that one of the bugs might be a silicon issue. However, according to Sean, it is a just a test and KVM issue, once you read the SDM more carefully.
Cc Jim Mattson <[email protected]>Cc Sean Christopherson <[email protected]>
Quoted 7 lines
Nadav Amit (2):
x86: nVMX: Use #DB in nmi- and intr-window tests
x86: nVMX: Set guest as active after NMI/INTR-window testsx86/vmx_tests.c | 80 +++++++++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 42 deletions(-)
Queued, thanks.
Paolo