Messages in this thread | | | Date | Wed, 2 Aug 2023 08:40:36 -0500 | Subject | Re: [PATCH v2] x86/kexec: Add EFI config table identity mapping for kexec kernel | From | 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@amd.com>
> > --- > From: "Borislav Petkov (AMD)" <bp@alien8.de> > 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@redhat.com> > Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de> > Tested-by: Tao Liu <ltao@redhat.com> > Cc: <stable@kernel.org> > Link: https://lore.kernel.org/r/20230601072043.24439-1-ltao@redhat.com > --- > 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
| |