ACK: [SRU][N:intel][PATCH 1/1] x86/mce: Implement recovery for errors in TDX/SEAM non-root mode

[Guoqing Jiang]

On 9/12/24 05:52, Thibault Ferrante wrote:
> From: Tony Luck <tony.luck at intel.com>
>
> BugLink: https://bugs.launchpad.net/bugs/2080352
>
> Machine check SMIs (MSMI) signaled during SEAM operation (typically
> inside TDX guests), on a system with Intel eMCA enabled, might eventually
> be reported to the kernel #MC handler with the saved RIP on the stack
> pointing to the instruction in kernel code after the SEAMCALL instruction
> that entered the SEAM operation. Linux currently says that is a fatal
> error and shuts down.
>
> There is a new bit in IA32_MCG_STATUS that, when set to 1, indicates
> that the machine check didn't originally occur at that saved RIP, but
> during SEAM non-root operation.
>
> Add new entries to the severity table to detect this for both data load
> and instruction fetch that set the severity to "AR" (action required).
>
> Increase the width of the mcgmask/mcgres fields in "struct severity"
> from unsigned char to unsigned short since the new bit is in position 12.
>
> Action required for these errors is just mark the page as poisoned and
> return from the machine check handler.
>
> HW ABI notes:
> =============
>
> The SEAM_NR bit in IA32_MCG_STATUS hasn't yet made it into the Intel
> Software Developers' Manual. But it is described in section 16.5.2
> of "Intel(R) Trust Domain Extensions (Intel(R) TDX) Module Base
> Architecture Specification" downloadable from:
>
>    https://cdrdv2.intel.com/v1/dl/getContent/733575
>
> Backport notes:
> ===============
>
> Little value in backporting this patch to stable or LTS kernels as
> this is only relevant with support for TDX, which I assume won't be
> backported. But for anyone taking this to v6.1 or older, you also
> need commit:
>
>    a51cbd0d86d3 ("x86/mce: Use severity table to handle uncorrected errors in kernel")
>
> Signed-off-by: Tony Luck <tony.luck at intel.com>
> Signed-off-by: Ingo Molnar <mingo at kernel.org>
> Link: https://lore.kernel.org/r/20240408180944.44638-1-tony.luck@intel.com
> (cherry picked from commit 7911f145de5fecbee1d67f27f73bec12f0fbc472)
> Signed-off-by: Thibault Ferrante <thibault.ferrante at canonical.com>
> ---
>   arch/x86/include/asm/mce.h         |  2 ++
>   arch/x86/kernel/cpu/mce/core.c     | 18 ++++++++++++++++++
>   arch/x86/kernel/cpu/mce/severity.c | 16 ++++++++++++++--
>   3 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
> index de3118305838..dfd2e9699bd7 100644
> --- a/arch/x86/include/asm/mce.h
> +++ b/arch/x86/include/asm/mce.h
> @@ -13,6 +13,7 @@
>   #define MCG_CTL_P		BIT_ULL(8)   /* MCG_CTL register available */
>   #define MCG_EXT_P		BIT_ULL(9)   /* Extended registers available */
>   #define MCG_CMCI_P		BIT_ULL(10)  /* CMCI supported */
> +#define MCG_SEAM_NR		BIT_ULL(12)  /* MCG_STATUS_SEAM_NR supported */
>   #define MCG_EXT_CNT_MASK	0xff0000     /* Number of Extended registers */
>   #define MCG_EXT_CNT_SHIFT	16
>   #define MCG_EXT_CNT(c)		(((c) & MCG_EXT_CNT_MASK) >> MCG_EXT_CNT_SHIFT)
> @@ -25,6 +26,7 @@
>   #define MCG_STATUS_EIPV		BIT_ULL(1)   /* ip points to correct instruction */
>   #define MCG_STATUS_MCIP		BIT_ULL(2)   /* machine check in progress */
>   #define MCG_STATUS_LMCES	BIT_ULL(3)   /* LMCE signaled */
> +#define MCG_STATUS_SEAM_NR	BIT_ULL(12)  /* Machine check inside SEAM non-root mode */
>   
>   /* MCG_EXT_CTL register defines */
>   #define MCG_EXT_CTL_LMCE_EN	BIT_ULL(0) /* Enable LMCE */
> diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
> index c17f9403cf6c..6ebb50091230 100644
> --- a/arch/x86/kernel/cpu/mce/core.c
> +++ b/arch/x86/kernel/cpu/mce/core.c
> @@ -1592,6 +1592,24 @@ noinstr void do_machine_check(struct pt_regs *regs)
>   		else
>   			queue_task_work(&m, msg, kill_me_maybe);
>   
> +	} else if (m.mcgstatus & MCG_STATUS_SEAM_NR) {
> +		/*
> +		 * Saved RIP on stack makes it look like the machine check
> +		 * was taken in the kernel on the instruction following
> +		 * the entry to SEAM mode. But MCG_STATUS_SEAM_NR indicates
> +		 * that the machine check was taken inside SEAM non-root
> +		 * mode.  CPU core has already marked that guest as dead.
> +		 * It is OK for the kernel to resume execution at the
> +		 * apparent point of the machine check as the fault did
> +		 * not occur there. Mark the page as poisoned so it won't
> +		 * be added to free list when the guest is terminated.
> +		 */
> +		if (mce_usable_address(&m)) {
> +			struct page *p = pfn_to_online_page(m.addr >> PAGE_SHIFT);
> +
> +			if (p)
> +				SetPageHWPoison(p);
> +		}
>   	} else {
>   		/*
>   		 * Handle an MCE which has happened in kernel space but from
> diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c
> index c4477162c07d..fc8988cfe1c3 100644
> --- a/arch/x86/kernel/cpu/mce/severity.c
> +++ b/arch/x86/kernel/cpu/mce/severity.c
> @@ -39,8 +39,8 @@ static struct severity {
>   	u64 mask;
>   	u64 result;
>   	unsigned char sev;
> -	unsigned char mcgmask;
> -	unsigned char mcgres;
> +	unsigned short mcgmask;
> +	unsigned short mcgres;
>   	unsigned char ser;
>   	unsigned char context;
>   	unsigned char excp;
> @@ -173,6 +173,18 @@ static struct severity {
>   		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR|MCACOD, MCI_UC_SAR|MCI_ADDR|MCACOD_INSTR),
>   		USER
>   		),
> +	MCESEV(
> +		AR, "Data load error in SEAM non-root mode",
> +		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR|MCACOD, MCI_UC_SAR|MCI_ADDR|MCACOD_DATA),
> +		MCGMASK(MCG_STATUS_SEAM_NR, MCG_STATUS_SEAM_NR),
> +		KERNEL
> +		),
> +	MCESEV(
> +		AR, "Instruction fetch error in SEAM non-root mode",
> +		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR|MCACOD, MCI_UC_SAR|MCI_ADDR|MCACOD_INSTR),
> +		MCGMASK(MCG_STATUS_SEAM_NR, MCG_STATUS_SEAM_NR),
> +		KERNEL
> +		),
>   	MCESEV(
>   		PANIC, "Data load in unrecoverable area of kernel",
>   		SER, MASK(MCI_STATUS_OVER|MCI_UC_SAR|MCI_ADDR|MCACOD, MCI_UC_SAR|MCI_ADDR|MCACOD_DATA),

Acked-by: Guoqing Jiang <guoqing.jiang at canonical.com>