Re: [PATCH v2] x86/kexec: Add EFI config table identity mapping for kexec kernel

[Tom Lendacky]

On 8/2/23 04:39, Borislav Petkov wrote:
> On Wed, Aug 02, 2023 at 04:22:54PM +0800, Tao Liu wrote:
> > Thanks for the patch! I have tested it on the lenovo machine in the
> > past few days, no issue found, so the patch tests OK.
>
> Thanks for testing!
>
> Mike, Tom, the below ok this way?

Short of figuring out how to map page accesses earlier through the 
boot_page_fault IDT routine, this seems reasonable.

Acked-by: Tom Lendacky <thomas.lendacky@xxxxxxx>

> ---
> From: "Borislav Petkov (AMD)" <bp@xxxxxxxxx>
> Date: Sun, 16 Jul 2023 20:22:20 +0200
> Subject: [PATCH] x86/sev: Do not try to parse for the CC blob on non-AMD
>   hardware
>
> Tao Liu reported a boot hang on an Intel Atom machine due to an unmapped
> EFI config table. The reason being that the CC blob which contains the
> CPUID page for AMD SNP guests is parsed for before even checking
> whether the machine runs on AMD hardware.
>
> Usually that's not a problem on !AMD hw - it simply won't find the CC
> blob's GUID and return. However, if any parts of the config table
> pointers array is not mapped, the kernel will #PF very early in the
> decompressor stage without any opportunity to recover.
>
> Therefore, do a superficial CPUID check before poking for the CC blob.
> This will fix the current issue on real hardware. It would also work as
> a guest on a non-lying hypervisor.
>
> For the lying hypervisor, the check is done again, *after* parsing the
> CC blob as the real CPUID page will be present then.
>
> Clear the #VC handler in case SEV-{ES,SNP} hasn't been detected, as
> a precaution.
>
> Fixes: c01fce9cef84 ("x86/compressed: Add SEV-SNP feature detection/setup")
> Reported-by: Tao Liu <ltao@xxxxxxxxxx>
> Signed-off-by: Borislav Petkov (AMD) <bp@xxxxxxxxx>
> Tested-by: Tao Liu <ltao@xxxxxxxxxx>
> Cc: <stable@xxxxxxxxxx>
> Link: https://lore.kernel.org/r/20230601072043.24439-1-ltao@xxxxxxxxxx
> ---
>   arch/x86/boot/compressed/idt_64.c |  9 +++++++-
>   arch/x86/boot/compressed/sev.c    | 37 +++++++++++++++++++++++++++++--
>   2 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/idt_64.c b/arch/x86/boot/compressed/idt_64.c
> index 6debb816e83d..3cdf94b41456 100644
> --- a/arch/x86/boot/compressed/idt_64.c
> +++ b/arch/x86/boot/compressed/idt_64.c
> @@ -63,7 +63,14 @@ void load_stage2_idt(void)
>   	set_idt_entry(X86_TRAP_PF, boot_page_fault);
>   
>   #ifdef CONFIG_AMD_MEM_ENCRYPT
> -	set_idt_entry(X86_TRAP_VC, boot_stage2_vc);
> +	/*
> +	 * Clear the second stage #VC handler in case guest types
> +	 * needing #VC have not been detected.
> +	 */
> +	if (sev_status & BIT(1))
> +		set_idt_entry(X86_TRAP_VC, boot_stage2_vc);
> +	else
> +		set_idt_entry(X86_TRAP_VC, NULL);
>   #endif
>   
>   	load_boot_idt(&boot_idt_desc);
> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> index 09dc8c187b3c..c3e343bd4760 100644
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c
> @@ -404,13 +404,46 @@ void sev_enable(struct boot_params *bp)
>   	if (bp)
>   		bp->cc_blob_address = 0;
>   
> +	/*
> +	 * Do an initial SEV capability check before snp_init() which
> +	 * loads the CPUID page and the same checks afterwards are done
> +	 * without the hypervisor and are trustworthy.
> +	 *
> +	 * If the HV fakes SEV support, the guest will crash'n'burn
> +	 * which is good enough.
> +	 */
> +
> +	/* Check for the SME/SEV support leaf */
> +	eax = 0x80000000;
> +	ecx = 0;
> +	native_cpuid(&eax, &ebx, &ecx, &edx);
> +	if (eax < 0x8000001f)
> +		return;
> +
> +	/*
> +	 * Check for the SME/SEV feature:
> +	 *   CPUID Fn8000_001F[EAX]
> +	 *   - Bit 0 - Secure Memory Encryption support
> +	 *   - Bit 1 - Secure Encrypted Virtualization support
> +	 *   CPUID Fn8000_001F[EBX]
> +	 *   - Bits 5:0 - Pagetable bit position used to indicate encryption
> +	 */
> +	eax = 0x8000001f;
> +	ecx = 0;
> +	native_cpuid(&eax, &ebx, &ecx, &edx);
> +	/* Check whether SEV is supported */
> +	if (!(eax & BIT(1)))
> +		return;
> +
>   	/*
>   	 * Setup/preliminary detection of SNP. This will be sanity-checked
>   	 * against CPUID/MSR values later.
>   	 */
>   	snp = snp_init(bp);
>   
> -	/* Check for the SME/SEV support leaf */
> +	/* Now repeat the checks with the SNP CPUID table. */
> +
> +	/* Recheck the SME/SEV support leaf */
>   	eax = 0x80000000;
>   	ecx = 0;
>   	native_cpuid(&eax, &ebx, &ecx, &edx);
> @@ -418,7 +451,7 @@ void sev_enable(struct boot_params *bp)
>   		return;
>   
>   	/*
> -	 * Check for the SME/SEV feature:
> +	 * Recheck for the SME/SEV feature:
>   	 *   CPUID Fn8000_001F[EAX]
>   	 *   - Bit 0 - Secure Memory Encryption support
>   	 *   - Bit 1 - Secure Encrypted Virtualization support