Skip to content

Commit 11df586

Browse files
committed
KVM: VMX: Handle NMI VM-Exits in noinstr region
Move VMX's handling of NMI VM-Exits into vmx_vcpu_enter_exit() so that the NMI is handled prior to leaving the safety of noinstr. Handling the NMI after leaving noinstr exposes the kernel to potential ordering problems as an instrumentation-induced fault, e.g. #DB, #BP, #PF, etc. will unblock NMIs when IRETing back to the faulting instruction. Reported-by: Peter Zijlstra <peterz@infradead.org> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lore.kernel.org/r/20221213060912.654668-8-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent 4f76e86 commit 11df586

File tree

4 files changed

+30
-22
lines changed

4 files changed

+30
-22
lines changed

arch/x86/kvm/vmx/vmcs.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ struct loaded_vmcs {
7575
struct vmcs_controls_shadow controls_shadow;
7676
};
7777

78-
static inline bool is_intr_type(u32 intr_info, u32 type)
78+
static __always_inline bool is_intr_type(u32 intr_info, u32 type)
7979
{
8080
const u32 mask = INTR_INFO_VALID_MASK | INTR_INFO_INTR_TYPE_MASK;
8181

@@ -146,7 +146,7 @@ static inline bool is_icebp(u32 intr_info)
146146
return is_intr_type(intr_info, INTR_TYPE_PRIV_SW_EXCEPTION);
147147
}
148148

149-
static inline bool is_nmi(u32 intr_info)
149+
static __always_inline bool is_nmi(u32 intr_info)
150150
{
151151
return is_intr_type(intr_info, INTR_TYPE_NMI_INTR);
152152
}

arch/x86/kvm/vmx/vmenter.S

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,10 @@ SYM_INNER_LABEL(vmx_vmexit, SYM_L_GLOBAL)
299299

300300
SYM_FUNC_END(__vmx_vcpu_run)
301301

302+
SYM_FUNC_START(vmx_do_nmi_irqoff)
303+
VMX_DO_EVENT_IRQOFF call asm_exc_nmi_kvm_vmx
304+
SYM_FUNC_END(vmx_do_nmi_irqoff)
305+
302306

303307
.section .text, "ax"
304308

@@ -353,10 +357,6 @@ SYM_FUNC_START(vmread_error_trampoline)
353357
SYM_FUNC_END(vmread_error_trampoline)
354358
#endif
355359

356-
SYM_FUNC_START(vmx_do_nmi_irqoff)
357-
VMX_DO_EVENT_IRQOFF call asm_exc_nmi_kvm_vmx
358-
SYM_FUNC_END(vmx_do_nmi_irqoff)
359-
360360
SYM_FUNC_START(vmx_do_interrupt_irqoff)
361361
VMX_DO_EVENT_IRQOFF CALL_NOSPEC _ASM_ARG1
362362
SYM_FUNC_END(vmx_do_interrupt_irqoff)

arch/x86/kvm/vmx/vmx.c

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5170,8 +5170,13 @@ static int handle_exception_nmi(struct kvm_vcpu *vcpu)
51705170
vect_info = vmx->idt_vectoring_info;
51715171
intr_info = vmx_get_intr_info(vcpu);
51725172

5173+
/*
5174+
* Machine checks are handled by handle_exception_irqoff(), or by
5175+
* vmx_vcpu_run() if a #MC occurs on VM-Entry. NMIs are handled by
5176+
* vmx_vcpu_enter_exit().
5177+
*/
51735178
if (is_machine_check(intr_info) || is_nmi(intr_info))
5174-
return 1; /* handled by handle_exception_nmi_irqoff() */
5179+
return 1;
51755180

51765181
/*
51775182
* Queue the exception here instead of in handle_nm_fault_irqoff().
@@ -6884,7 +6889,7 @@ static void handle_nm_fault_irqoff(struct kvm_vcpu *vcpu)
68846889
rdmsrl(MSR_IA32_XFD_ERR, vcpu->arch.guest_fpu.xfd_err);
68856890
}
68866891

6887-
static void handle_exception_nmi_irqoff(struct vcpu_vmx *vmx)
6892+
static void handle_exception_irqoff(struct vcpu_vmx *vmx)
68886893
{
68896894
u32 intr_info = vmx_get_intr_info(&vmx->vcpu);
68906895

@@ -6897,12 +6902,6 @@ static void handle_exception_nmi_irqoff(struct vcpu_vmx *vmx)
68976902
/* Handle machine checks before interrupts are enabled */
68986903
else if (is_machine_check(intr_info))
68996904
kvm_machine_check();
6900-
/* We need to handle NMIs before interrupts are enabled */
6901-
else if (is_nmi(intr_info)) {
6902-
kvm_before_interrupt(&vmx->vcpu, KVM_HANDLING_NMI);
6903-
vmx_do_nmi_irqoff();
6904-
kvm_after_interrupt(&vmx->vcpu);
6905-
}
69066905
}
69076906

69086907
static void handle_external_interrupt_irqoff(struct kvm_vcpu *vcpu)
@@ -6932,7 +6931,7 @@ static void vmx_handle_exit_irqoff(struct kvm_vcpu *vcpu)
69326931
if (vmx->exit_reason.basic == EXIT_REASON_EXTERNAL_INTERRUPT)
69336932
handle_external_interrupt_irqoff(vcpu);
69346933
else if (vmx->exit_reason.basic == EXIT_REASON_EXCEPTION_NMI)
6935-
handle_exception_nmi_irqoff(vmx);
6934+
handle_exception_irqoff(vmx);
69366935
}
69376936

69386937
/*
@@ -7194,6 +7193,18 @@ static noinstr void vmx_vcpu_enter_exit(struct kvm_vcpu *vcpu,
71947193

71957194
vmx_enable_fb_clear(vmx);
71967195

7196+
if (unlikely(vmx->fail))
7197+
vmx->exit_reason.full = 0xdead;
7198+
else
7199+
vmx->exit_reason.full = vmcs_read32(VM_EXIT_REASON);
7200+
7201+
if ((u16)vmx->exit_reason.basic == EXIT_REASON_EXCEPTION_NMI &&
7202+
is_nmi(vmx_get_intr_info(vcpu))) {
7203+
kvm_before_interrupt(vcpu, KVM_HANDLING_NMI);
7204+
vmx_do_nmi_irqoff();
7205+
kvm_after_interrupt(vcpu);
7206+
}
7207+
71977208
guest_state_exit_irqoff();
71987209
}
71997210

@@ -7335,12 +7346,9 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu)
73357346

73367347
vmx->idt_vectoring_info = 0;
73377348

7338-
if (unlikely(vmx->fail)) {
7339-
vmx->exit_reason.full = 0xdead;
7349+
if (unlikely(vmx->fail))
73407350
return EXIT_FASTPATH_NONE;
7341-
}
73427351

7343-
vmx->exit_reason.full = vmcs_read32(VM_EXIT_REASON);
73447352
if (unlikely((u16)vmx->exit_reason.basic == EXIT_REASON_MCE_DURING_VMENTRY))
73457353
kvm_machine_check();
73467354

arch/x86/kvm/x86.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -382,13 +382,13 @@ enum kvm_intr_type {
382382
KVM_HANDLING_NMI,
383383
};
384384

385-
static inline void kvm_before_interrupt(struct kvm_vcpu *vcpu,
386-
enum kvm_intr_type intr)
385+
static __always_inline void kvm_before_interrupt(struct kvm_vcpu *vcpu,
386+
enum kvm_intr_type intr)
387387
{
388388
WRITE_ONCE(vcpu->arch.handling_intr_from_guest, (u8)intr);
389389
}
390390

391-
static inline void kvm_after_interrupt(struct kvm_vcpu *vcpu)
391+
static __always_inline void kvm_after_interrupt(struct kvm_vcpu *vcpu)
392392
{
393393
WRITE_ONCE(vcpu->arch.handling_intr_from_guest, 0);
394394
}

0 commit comments

Comments
 (0)