2022-12-13 06:12:37

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH 5/7] x86/entry: KVM: Use dedicated VMX NMI entry for 32-bit kernels too

Use a dedicated entry for invoking the NMI handler from KVM VMX's VM-Exit
path for 32-bit even though using a dedicated entry for 32-bit isn't
strictly necessary. Exposing a single symbol will allow KVM to reference
the entry point in assembly code without having to resort to more #ifdefs
(or #defines). identry.h is intended to be included from asm files only
once, and so simply including idtentry.h in KVM assembly isn't an option.

Bypassing the ESP fixup and CR3 switching in the standard NMI entry code
is safe as KVM always handles NMIs that occur in the guest on a kernel
stack, with a kernel CR3.

Cc: Andy Lutomirski <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/include/asm/idtentry.h | 16 ++++++----------
arch/x86/kernel/nmi.c | 8 ++++----
arch/x86/kvm/vmx/vmx.c | 4 ++--
3 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
index 72184b0b2219..b241af4ce9b4 100644
--- a/arch/x86/include/asm/idtentry.h
+++ b/arch/x86/include/asm/idtentry.h
@@ -582,18 +582,14 @@ DECLARE_IDTENTRY_RAW(X86_TRAP_MC, xenpv_exc_machine_check);

/* NMI */

-#if defined(CONFIG_X86_64) && IS_ENABLED(CONFIG_KVM_INTEL)
+#if 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.
+ * Special entry point for VMX which invokes this on the kernel stack, even for
+ * 64-bit, i.e. without using an IST. asm_exc_nmi() requires an IST to work
+ * correctly vs. the NMI 'executing' marker. Used for 32-bit kernels as well
+ * to avoid more ifdeffery.
*/
-DECLARE_IDTENTRY(X86_TRAP_NMI, exc_nmi_noist);
-#else
-#define asm_exc_nmi_noist asm_exc_nmi
+DECLARE_IDTENTRY(X86_TRAP_NMI, exc_nmi_kvm_vmx);
#endif

DECLARE_IDTENTRY_NMI(X86_TRAP_NMI, exc_nmi);
diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index cec0bfa3bc04..e37faba95bb5 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -527,14 +527,14 @@ 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)
+#if IS_ENABLED(CONFIG_KVM_INTEL)
+DEFINE_IDTENTRY_RAW(exc_nmi_kvm_vmx)
{
exc_nmi(regs);
}
-#endif
#if IS_MODULE(CONFIG_KVM_INTEL)
-EXPORT_SYMBOL_GPL(asm_exc_nmi_noist);
+EXPORT_SYMBOL_GPL(asm_exc_nmi_kvm_vmx);
+#endif
#endif

void stop_nmi(void)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index e2c96f204b82..7ace22ee240d 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6791,7 +6791,7 @@ void vmx_do_interrupt_nmi_irqoff(unsigned long entry);
static void handle_interrupt_nmi_irqoff(struct kvm_vcpu *vcpu,
unsigned long entry)
{
- bool is_nmi = entry == (unsigned long)asm_exc_nmi_noist;
+ bool is_nmi = entry == (unsigned long)asm_exc_nmi_kvm_vmx;

kvm_before_interrupt(vcpu, is_nmi ? KVM_HANDLING_NMI : KVM_HANDLING_IRQ);
vmx_do_interrupt_nmi_irqoff(entry);
@@ -6820,7 +6820,7 @@ static void handle_nm_fault_irqoff(struct kvm_vcpu *vcpu)

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

/* if exit due to PF check for async PF */
--
2.39.0.rc1.256.g54fd8350bd-goog


2022-12-14 08:55:12

by Lai Jiangshan

[permalink] [raw]
Subject: Re: [PATCH 5/7] x86/entry: KVM: Use dedicated VMX NMI entry for 32-bit kernels too

On Tue, Dec 13, 2022 at 2:11 PM Sean Christopherson <[email protected]> wrote:
>
> Use a dedicated entry for invoking the NMI handler from KVM VMX's VM-Exit
> path for 32-bit even though using a dedicated entry for 32-bit isn't
> strictly necessary. Exposing a single symbol will allow KVM to reference
> the entry point in assembly code without having to resort to more #ifdefs
> (or #defines). identry.h is intended to be included from asm files only
> once, and so simply including idtentry.h in KVM assembly isn't an option.
>
> Bypassing the ESP fixup and CR3 switching in the standard NMI entry code
> is safe as KVM always handles NMIs that occur in the guest on a kernel
> stack, with a kernel CR3.
>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> arch/x86/include/asm/idtentry.h | 16 ++++++----------
> arch/x86/kernel/nmi.c | 8 ++++----
> arch/x86/kvm/vmx/vmx.c | 4 ++--
> 3 files changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
> index 72184b0b2219..b241af4ce9b4 100644
> --- a/arch/x86/include/asm/idtentry.h
> +++ b/arch/x86/include/asm/idtentry.h
> @@ -582,18 +582,14 @@ DECLARE_IDTENTRY_RAW(X86_TRAP_MC, xenpv_exc_machine_check);
>
> /* NMI */
>
> -#if defined(CONFIG_X86_64) && IS_ENABLED(CONFIG_KVM_INTEL)
> +#if 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.
> + * Special entry point for VMX which invokes this on the kernel stack, even for
> + * 64-bit, i.e. without using an IST. asm_exc_nmi() requires an IST to work
> + * correctly vs. the NMI 'executing' marker. Used for 32-bit kernels as well
> + * to avoid more ifdeffery.
> */
> -DECLARE_IDTENTRY(X86_TRAP_NMI, exc_nmi_noist);
> -#else
> -#define asm_exc_nmi_noist asm_exc_nmi
> +DECLARE_IDTENTRY(X86_TRAP_NMI, exc_nmi_kvm_vmx);

Reviewed-by: Lai Jiangshan <[email protected]>