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

Chao Gao
2024-04-01T09:14:15Z

On Mon, Feb 26, 2024 at 12:26:45AM -0800, isaku.yamahata@intel.com wrote:
>From: Isaku Yamahata <isaku.yamahata@intel.com>
>
>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@intel.com>
>---
> 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
>
>