Re: [PATCH v4 3/3] x86/efistub: Don't bother enabling SEV in the EFI stub

Ard Biesheuvel
2025-09-12T07:29:51Z

On Thu, 11 Sept 2025 at 23:53, Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 9/9/25 03:06, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > One of the last things the EFI stub does before handing over to the core
> > kernel when booting as a SEV guest is enabling SEV, even though this is
> > mostly redundant: one of the first things the core kernel does is
> > calling sme_enable(), after setting up the early GDT and IDT but before
> > even setting up the kernel page tables. sme_enable() performs the same
> > SEV-SNP initialization that the decompressor performs in sev_enable().
> >
> > So let's just drop this call to sev_enable(), and rely on the core
> > kernel to initiaize SEV correctly.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > arch/x86/include/asm/sev.h | 2 --
> > drivers/firmware/efi/libstub/x86-stub.c | 6 ------
> > 2 files changed, 8 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> > index d7be1ff3f7e0..b017e1dab705 100644
> > --- a/arch/x86/include/asm/sev.h
> > +++ b/arch/x86/include/asm/sev.h
> > @@ -462,7 +462,6 @@ static __always_inline void sev_es_nmi_complete(void)
> > __sev_es_nmi_complete();
> > }
> > extern int __init sev_es_efi_map_ghcbs_cas(pgd_t *pgd);
> > -extern void sev_enable(struct boot_params *bp);
> >
> > /*
> > * RMPADJUST modifies the RMP permissions of a page of a lesser-
> > @@ -588,7 +587,6 @@ static inline void sev_es_ist_exit(void) { }
> > static inline int sev_es_setup_ap_jump_table(struct real_mode_header *rmh) { return 0; }
> > static inline void sev_es_nmi_complete(void) { }
> > static inline int sev_es_efi_map_ghcbs_cas(pgd_t *pgd) { return 0; }
> > -static inline void sev_enable(struct boot_params *bp) { }
> > static inline int pvalidate(unsigned long vaddr, bool rmp_psize, bool validate) { return 0; }
> > static inline int rmpadjust(unsigned long vaddr, bool rmp_psize, unsigned long attrs) { return 0; }
> > static inline void setup_ghcb(void) { }
> > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> > index c4ef645762ec..354bc3901193 100644
> > --- a/drivers/firmware/efi/libstub/x86-stub.c
> > +++ b/drivers/firmware/efi/libstub/x86-stub.c
> > @@ -938,12 +938,6 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> > goto fail;
> > }
> >
> > - /*
> > - * Call the SEV init code while still running with the firmware's
> > - * GDT/IDT, so #VC exceptions will be handled by EFI.
> > - */
> > - sev_enable(boot_params);
>
> I think we lose the check for GHCB_HV_FT_SNP_MULTI_VMPL by doing this. It
> might need move into svsm_setup_ca() now.
>

Currently, this check only occurs inside sev_enable(), and so it
happens too late to have an impact, given that the core kernel will
set up all of this state from scratch right away.

So if this check is needed in the EFI stub to begin with, it should be
moved into early_is_sevsnp_guest() so that the check occurs before
attempting to accept memory.