Re: [PATCH v19 103/130] KVM: TDX: Handle EXIT_REASON_OTHER_SMI with MSMI

[Chao Gao]

On Mon, Feb 26, 2024 at 12:26:45AM -0800, isaku.yamahata@xxxxxxxxx wrote:
>From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
>
>When BIOS eMCA MCE-SMI morphing is enabled, the #MC is morphed to MSMI
>(Machine Check System Management Interrupt).  Then the SMI causes TD exit
>with the read reason of EXIT_REASON_OTHER_SMI with MSMI bit set in the exit
>qualification to KVM instead of EXIT_REASON_EXCEPTION_NMI with MC
>exception.
>
>Handle EXIT_REASON_OTHER_SMI with MSMI bit set in the exit qualification as
>MCE(Machine Check Exception) happened during TD guest running.
>
>Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
>---
> arch/x86/kvm/vmx/tdx.c      | 40 ++++++++++++++++++++++++++++++++++---
> arch/x86/kvm/vmx/tdx_arch.h |  2 ++
> 2 files changed, 39 insertions(+), 3 deletions(-)
>
>diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>index bdd74682b474..117c2315f087 100644
>--- a/arch/x86/kvm/vmx/tdx.c
>+++ b/arch/x86/kvm/vmx/tdx.c
>@@ -916,6 +916,30 @@ void tdx_handle_exit_irqoff(struct kvm_vcpu *vcpu)
> 						     tdexit_intr_info(vcpu));
> 	else if (exit_reason == EXIT_REASON_EXCEPTION_NMI)
> 		vmx_handle_exception_irqoff(vcpu, tdexit_intr_info(vcpu));
>+	else if (unlikely(tdx->exit_reason.non_recoverable ||
>+		 tdx->exit_reason.error)) {

why not just:
	else if (tdx->exit_reason.basic == EXIT_REASON_OTHER_SMI) {


i.e., does EXIT_REASON_OTHER_SMI imply exit_reason.non_recoverable or
exit_reason.error?

>+		/*
>+		 * The only reason it gets EXIT_REASON_OTHER_SMI is there is an
>+		 * #MSMI(Machine Check System Management Interrupt) with
>+		 * exit_qualification bit 0 set in TD guest.
>+		 * The #MSMI is delivered right after SEAMCALL returns,
>+		 * and an #MC is delivered to host kernel after SMI handler
>+		 * returns.
>+		 *
>+		 * The #MC right after SEAMCALL is fixed up and skipped in #MC

Looks fixing up and skipping #MC on the first instruction after TD-exit is
missing in v19?

>+		 * handler because it's an #MC happens in TD guest we cannot
>+		 * handle it with host's context.
>+		 *
>+		 * Call KVM's machine check handler explicitly here.
>+		 */
>+		if (tdx->exit_reason.basic == EXIT_REASON_OTHER_SMI) {
>+			unsigned long exit_qual;
>+
>+			exit_qual = tdexit_exit_qual(vcpu);
>+			if (exit_qual & TD_EXIT_OTHER_SMI_IS_MSMI)

>+				kvm_machine_check();
>+		}
>+	}
> }
> 
> static int tdx_handle_exception(struct kvm_vcpu *vcpu)
>@@ -1381,6 +1405,11 @@ int tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t fastpath)
> 			      exit_reason.full, exit_reason.basic,
> 			      to_kvm_tdx(vcpu->kvm)->hkid,
> 			      set_hkid_to_hpa(0, to_kvm_tdx(vcpu->kvm)->hkid));
>+
>+		/*
>+		 * tdx_handle_exit_irqoff() handled EXIT_REASON_OTHER_SMI.  It
>+		 * must be handled before enabling preemption because it's #MC.
>+		 */

Then EXIT_REASON_OTHER_SMI is handled, why still go to unhandled_exit?

> 		goto unhandled_exit;
> 	}
> 
>@@ -1419,9 +1448,14 @@ int tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t fastpath)
> 		return tdx_handle_ept_misconfig(vcpu);
> 	case EXIT_REASON_OTHER_SMI:
> 		/*
>-		 * If reach here, it's not a Machine Check System Management
>-		 * Interrupt(MSMI).  #SMI is delivered and handled right after
>-		 * SEAMRET, nothing needs to be done in KVM.
>+		 * Unlike VMX, all the SMI in SEAM non-root mode (i.e. when
>+		 * TD guest vcpu is running) will cause TD exit to TDX module,
>+		 * then SEAMRET to KVM. Once it exits to KVM, SMI is delivered
>+		 * and handled right away.
>+		 *
>+		 * - If it's an Machine Check System Management Interrupt
>+		 *   (MSMI), it's handled above due to non_recoverable bit set.
>+		 * - If it's not an MSMI, don't need to do anything here.

This corrects a comment added in patch 100. Maybe we can just merge patch 100 into 
this one?

> 		 */
> 		return 1;
> 	default:
>diff --git a/arch/x86/kvm/vmx/tdx_arch.h b/arch/x86/kvm/vmx/tdx_arch.h
>index efc3c61c14ab..87ef22e9cd49 100644
>--- a/arch/x86/kvm/vmx/tdx_arch.h
>+++ b/arch/x86/kvm/vmx/tdx_arch.h
>@@ -42,6 +42,8 @@
> #define TDH_VP_WR			43
> #define TDH_SYS_LP_SHUTDOWN		44
> 
>+#define TD_EXIT_OTHER_SMI_IS_MSMI	BIT(1)
>+
> /* TDX control structure (TDR/TDCS/TDVPS) field access codes */
> #define TDX_NON_ARCH			BIT_ULL(63)
> #define TDX_CLASS_SHIFT			56
>-- 
>2.25.1
>
>