Skip to content

Commit

Permalink
KVM/VMX: Invoke NMI non-IST entry instead of IST entry
Browse files Browse the repository at this point in the history
commit a217a65 upstream.

In VMX, the host NMI handler needs to be invoked after NMI VM-Exit.
Before commit 1a5488e ("KVM: VMX: Invoke NMI handler via indirect
call instead of INTn"), this was done by INTn ("int $2"). But INTn
microcode is relatively expensive, so the commit reworked NMI VM-Exit
handling to invoke the kernel handler by function call.

But this missed a detail. The NMI entry point for direct invocation is
fetched from the IDT table and called on the kernel stack.  But on 64-bit
the NMI entry installed in the IDT expects to be invoked on the IST stack.
It relies on the "NMI executing" variable on the IST stack to work
correctly, which is at a fixed position in the IST stack.  When the entry
point is unexpectedly called on the kernel stack, the RSP-addressed "NMI
executing" variable is obviously also on the kernel stack and is
"uninitialized" and can cause the NMI entry code to run in the wrong way.

Provide a non-ist entry point for VMX which shares the C-function with
the regular NMI entry and invoke the new asm entry point instead.

On 32-bit this just maps to the regular NMI entry point as 32-bit has no
ISTs and is not affected.

[ tglx: Made it independent for backporting, massaged changelog ]

Fixes: 1a5488e ("KVM: VMX: Invoke NMI handler via indirect call instead of INTn")
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Lai Jiangshan <laijs@linux.alibaba.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/87r1imi8i1.ffs@nanos.tec.linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Lai Jiangshan authored and gregkh committed May 19, 2021
1 parent 21f3178 commit bfccc4e
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 7 deletions.
15 changes: 15 additions & 0 deletions arch/x86/include/asm/idtentry.h
Expand Up @@ -588,6 +588,21 @@ DECLARE_IDTENTRY_RAW(X86_TRAP_MC, exc_machine_check);
#endif

/* NMI */

#if defined(CONFIG_X86_64) && IS_ENABLED(CONFIG_KVM_INTEL)
/*
* Special NOIST entry point for VMX which invokes this on the kernel
* stack. asm_exc_nmi() requires an IST to work correctly vs. the NMI
* 'executing' marker.
*
* On 32bit this just uses the regular NMI entry point because 32-bit does
* not have ISTs.
*/
DECLARE_IDTENTRY(X86_TRAP_NMI, exc_nmi_noist);
#else
#define asm_exc_nmi_noist asm_exc_nmi
#endif

DECLARE_IDTENTRY_NMI(X86_TRAP_NMI, exc_nmi);
#ifdef CONFIG_XEN_PV
DECLARE_IDTENTRY_RAW(X86_TRAP_NMI, xenpv_exc_nmi);
Expand Down
10 changes: 10 additions & 0 deletions arch/x86/kernel/nmi.c
Expand Up @@ -524,6 +524,16 @@ DEFINE_IDTENTRY_RAW(exc_nmi)
mds_user_clear_cpu_buffers();
}

#if defined(CONFIG_X86_64) && IS_ENABLED(CONFIG_KVM_INTEL)
DEFINE_IDTENTRY_RAW(exc_nmi_noist)
{
exc_nmi(regs);
}
#endif
#if IS_MODULE(CONFIG_KVM_INTEL)
EXPORT_SYMBOL_GPL(asm_exc_nmi_noist);
#endif

void stop_nmi(void)
{
ignore_nmis++;
Expand Down
16 changes: 9 additions & 7 deletions arch/x86/kvm/vmx/vmx.c
Expand Up @@ -36,6 +36,7 @@
#include <asm/debugreg.h>
#include <asm/desc.h>
#include <asm/fpu/internal.h>
#include <asm/idtentry.h>
#include <asm/io.h>
#include <asm/irq_remapping.h>
#include <asm/kexec.h>
Expand Down Expand Up @@ -6354,18 +6355,17 @@ static void vmx_apicv_post_state_restore(struct kvm_vcpu *vcpu)

void vmx_do_interrupt_nmi_irqoff(unsigned long entry);

static void handle_interrupt_nmi_irqoff(struct kvm_vcpu *vcpu, u32 intr_info)
static void handle_interrupt_nmi_irqoff(struct kvm_vcpu *vcpu,
unsigned long entry)
{
unsigned int vector = intr_info & INTR_INFO_VECTOR_MASK;
gate_desc *desc = (gate_desc *)host_idt_base + vector;

kvm_before_interrupt(vcpu);
vmx_do_interrupt_nmi_irqoff(gate_offset(desc));
vmx_do_interrupt_nmi_irqoff(entry);
kvm_after_interrupt(vcpu);
}

static void handle_exception_nmi_irqoff(struct vcpu_vmx *vmx)
{
const unsigned long nmi_entry = (unsigned long)asm_exc_nmi_noist;
u32 intr_info = vmx_get_intr_info(&vmx->vcpu);

/* if exit due to PF check for async PF */
Expand All @@ -6376,18 +6376,20 @@ static void handle_exception_nmi_irqoff(struct vcpu_vmx *vmx)
kvm_machine_check();
/* We need to handle NMIs before interrupts are enabled */
else if (is_nmi(intr_info))
handle_interrupt_nmi_irqoff(&vmx->vcpu, intr_info);
handle_interrupt_nmi_irqoff(&vmx->vcpu, nmi_entry);
}

static void handle_external_interrupt_irqoff(struct kvm_vcpu *vcpu)
{
u32 intr_info = vmx_get_intr_info(vcpu);
unsigned int vector = intr_info & INTR_INFO_VECTOR_MASK;
gate_desc *desc = (gate_desc *)host_idt_base + vector;

if (WARN_ONCE(!is_external_intr(intr_info),
"KVM: unexpected VM-Exit interrupt info: 0x%x", intr_info))
return;

handle_interrupt_nmi_irqoff(vcpu, intr_info);
handle_interrupt_nmi_irqoff(vcpu, gate_offset(desc));
}

static void vmx_handle_exit_irqoff(struct kvm_vcpu *vcpu)
Expand Down

0 comments on commit bfccc4e

Please sign in to comment.