[PATCH v4 0/3] efi: Don't initalize SEV-SNP from the EFI stub

[PATCH v4 0/3] efi: Don't initalize SEV-SNP from the EFI stub

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

The EFI stub no longer invokes the legacy decompressor, and so there is
no longer any reason to perform all SEV-SNP initialization twice: it is
sufficient to rely on the SEV-SNP work done by the core kernel.

Changes since v3: [2]
- Drop patches that have been merged in the meantime
- Rebase onto tip/x86/sev

Note that the issue pointed out by Tom in reply to the v3 cover letter
has been fixed in the meantime as well [3]

Changes since v2: [1]
- rebase onto tip/x86/boot
- add patch to remove unused static inline fallback implementation of
  sev_enable()

Changes since v1: [0]
- address shortcomings pointed out by Tom, related to missing checks and
  to discovery of the CC blob table from the EFI stub

[0] https://lore.kernel.org/all/20250414130417.1486395-2-ardb+git@google.com/T/#u
[1] https://lore.kernel.org/all/20250416165743.4080995-6-ardb+git@google.com/T/#u
[2] https://lore.kernel.org/all/20250422100728.208479-7-ardb+git@google.com/T/#u
[3] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=8ed12ab1319b2d8e4a529504777aacacf71371e4

Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Borislav Petkov <bp@alien8.de>

Ard Biesheuvel (3):
  x86/boot: Drop unused sev_enable() fallback
  x86/efistub: Obtain SEV CC blob address from the stub
  x86/efistub: Don't bother enabling SEV in the EFI stub

 arch/x86/boot/compressed/misc.h         | 11 --------
 arch/x86/include/asm/sev.h              |  2 --
 drivers/firmware/efi/libstub/x86-stub.c | 27 ++++++++++++--------
 3 files changed, 16 insertions(+), 24 deletions(-)


base-commit: 0ca77f8d33e8136b8926775380506f78a8d04811
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH v4 1/3] x86/boot: Drop unused sev_enable() fallback

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

The misc.h header is not included by the EFI stub, which is the only C
caller of sev_enable(). This means the fallback for cases where
CONFIG_AMD_MEM_ENCRYPT is not set is never used, so it can be dropped.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/boot/compressed/misc.h | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index db1048621ea2..fd855e32c9b9 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -152,17 +152,6 @@ bool insn_has_rep_prefix(struct insn *insn);
 void sev_insn_decode_init(void);
 bool early_setup_ghcb(void);
 #else
-static inline void sev_enable(struct boot_params *bp)
-{
-	/*
-	 * bp->cc_blob_address should only be set by boot/compressed kernel.
-	 * Initialize it to 0 unconditionally (thus here in this stub too) to
-	 * ensure that uninitialized values from buggy bootloaders aren't
-	 * propagated.
-	 */
-	if (bp)
-		bp->cc_blob_address = 0;
-}
 static inline void snp_check_features(void) { }
 static inline void sev_es_shutdown_ghcb(void) { }
 static inline bool sev_es_check_ghcb_fault(unsigned long address)
-- 
2.51.0.384.g4c02a37b29-goog

[PATCH v4 2/3] x86/efistub: Obtain SEV CC blob address from the stub

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

The x86 EFI stub no longer boots the core kernel via the traditional
decompressor but jumps straight to it, avoiding all the page fault
handling and other complexity that is entirely unnecessary when booting
via EFI, which guarantees that all system memory is mapped 1:1.

The SEV startup code in the core kernel expects the address of the CC
blob configuration table in boot_params, so store it there when booting
from EFI with SEV-SNP enabled. This removes the need to call
sev_enable() from the EFI stub.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/x86-stub.c | 21 +++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 0d05eac7c72b..c4ef645762ec 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -681,17 +681,28 @@ static efi_status_t exit_boot(struct boot_params *boot_params, void *handle)
 	return EFI_SUCCESS;
 }
 
-static bool have_unsupported_snp_features(void)
+static bool check_snp_features(struct boot_params *bp)
 {
+	u64 status = sev_get_status();
 	u64 unsupported;
 
-	unsupported = snp_get_unsupported_features(sev_get_status());
+	unsupported = snp_get_unsupported_features(status);
 	if (unsupported) {
 		efi_err("Unsupported SEV-SNP features detected: 0x%llx\n",
 			unsupported);
-		return true;
+		return false;
 	}
-	return false;
+
+	if (status & MSR_AMD64_SEV_SNP_ENABLED) {
+		void *tbl = get_efi_config_table(EFI_CC_BLOB_GUID);
+
+		if (!tbl) {
+			efi_err("SEV-SNP is enabled but CC blob not found\n");
+			return false;
+		}
+		bp->cc_blob_address = (u32)(unsigned long)tbl;
+	}
+	return true;
 }
 
 static void efi_get_seed(void *seed, int size)
@@ -831,7 +842,7 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
 
 	hdr = &boot_params->hdr;
 
-	if (have_unsupported_snp_features())
+	if (!check_snp_features(boot_params))
 		efi_exit(handle, EFI_UNSUPPORTED);
 
 	if (IS_ENABLED(CONFIG_EFI_DXE_MEM_ATTRIBUTES)) {
-- 
2.51.0.384.g4c02a37b29-goog

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

[Ard Biesheuvel]

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);
-
 	efi_5level_switch();
 
 	enter_kernel(kernel_entry, boot_params);
-- 
2.51.0.384.g4c02a37b29-goog

Re: [PATCH v4 2/3] x86/efistub: Obtain SEV CC blob address from the stub

[Tom Lendacky]

On 9/9/25 03:06, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> The x86 EFI stub no longer boots the core kernel via the traditional
> decompressor but jumps straight to it, avoiding all the page fault
> handling and other complexity that is entirely unnecessary when booting
> via EFI, which guarantees that all system memory is mapped 1:1.
> 
> The SEV startup code in the core kernel expects the address of the CC
> blob configuration table in boot_params, so store it there when booting
> from EFI with SEV-SNP enabled. This removes the need to call
> sev_enable() from the EFI stub.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/firmware/efi/libstub/x86-stub.c | 21 +++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index 0d05eac7c72b..c4ef645762ec 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -681,17 +681,28 @@ static efi_status_t exit_boot(struct boot_params *boot_params, void *handle)
>  	return EFI_SUCCESS;
>  }
>  
> -static bool have_unsupported_snp_features(void)
> +static bool check_snp_features(struct boot_params *bp)
>  {
> +	u64 status = sev_get_status();
>  	u64 unsupported;
>  
> -	unsupported = snp_get_unsupported_features(sev_get_status());
> +	unsupported = snp_get_unsupported_features(status);
>  	if (unsupported) {
>  		efi_err("Unsupported SEV-SNP features detected: 0x%llx\n",
>  			unsupported);
> -		return true;
> +		return false;
>  	}
> -	return false;
> +
> +	if (status & MSR_AMD64_SEV_SNP_ENABLED) {
> +		void *tbl = get_efi_config_table(EFI_CC_BLOB_GUID);
> +
> +		if (!tbl) {
> +			efi_err("SEV-SNP is enabled but CC blob not found\n");
> +			return false;
> +		}
> +		bp->cc_blob_address = (u32)(unsigned long)tbl;

I think we ran into bugs where the cc_blob_address was random data from a
boot loader when SNP wasn't active and that's why we always initialize it
to 0:

4b1c74240757 ("x86/boot: Don't propagate uninitialized boot_params->cc_blob_address")

So we probably need the same statement that is at the beginning of the
decompressor sev_enable() at the very beginning of this function to ensure
cc_blob_address is set to zero:

	/*    
	 * bp->cc_blob_address should only be set by boot/compressed kernel.
	 * Initialize it to 0 to ensure that uninitialized values from
	 * buggy bootloaders aren't propagated.
	 */
	if (bp)
		bp->cc_blob_address = 0;

Thanks,
Tom

> +	}
> +	return true;
>  }
>  
>  static void efi_get_seed(void *seed, int size)
> @@ -831,7 +842,7 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
>  
>  	hdr = &boot_params->hdr;
>  
> -	if (have_unsupported_snp_features())
> +	if (!check_snp_features(boot_params))
>  		efi_exit(handle, EFI_UNSUPPORTED);
>  
>  	if (IS_ENABLED(CONFIG_EFI_DXE_MEM_ATTRIBUTES)) {

Re: [PATCH v4 1/3] x86/boot: Drop unused sev_enable() fallback

[Tom Lendacky]

On 9/9/25 03:06, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> The misc.h header is not included by the EFI stub, which is the only C
> caller of sev_enable(). This means the fallback for cases where
> CONFIG_AMD_MEM_ENCRYPT is not set is never used, so it can be dropped.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  arch/x86/boot/compressed/misc.h | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
> index db1048621ea2..fd855e32c9b9 100644
> --- a/arch/x86/boot/compressed/misc.h
> +++ b/arch/x86/boot/compressed/misc.h
> @@ -152,17 +152,6 @@ bool insn_has_rep_prefix(struct insn *insn);
>  void sev_insn_decode_init(void);
>  bool early_setup_ghcb(void);
>  #else
> -static inline void sev_enable(struct boot_params *bp)
> -{
> -	/*
> -	 * bp->cc_blob_address should only be set by boot/compressed kernel.
> -	 * Initialize it to 0 unconditionally (thus here in this stub too) to
> -	 * ensure that uninitialized values from buggy bootloaders aren't
> -	 * propagated.
> -	 */
> -	if (bp)
> -		bp->cc_blob_address = 0;
> -}
>  static inline void snp_check_features(void) { }
>  static inline void sev_es_shutdown_ghcb(void) { }
>  static inline bool sev_es_check_ghcb_fault(unsigned long address)

Re: [PATCH v4 2/3] x86/efistub: Obtain SEV CC blob address from the stub

[Ard Biesheuvel]

On Thu, 11 Sept 2025 at 23:27, Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 9/9/25 03:06, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > The x86 EFI stub no longer boots the core kernel via the traditional
> > decompressor but jumps straight to it, avoiding all the page fault
> > handling and other complexity that is entirely unnecessary when booting
> > via EFI, which guarantees that all system memory is mapped 1:1.
> >
> > The SEV startup code in the core kernel expects the address of the CC
> > blob configuration table in boot_params, so store it there when booting
> > from EFI with SEV-SNP enabled. This removes the need to call
> > sev_enable() from the EFI stub.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  drivers/firmware/efi/libstub/x86-stub.c | 21 +++++++++++++++-----
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> > index 0d05eac7c72b..c4ef645762ec 100644
> > --- a/drivers/firmware/efi/libstub/x86-stub.c
> > +++ b/drivers/firmware/efi/libstub/x86-stub.c
> > @@ -681,17 +681,28 @@ static efi_status_t exit_boot(struct boot_params *boot_params, void *handle)
> >       return EFI_SUCCESS;
> >  }
> >
> > -static bool have_unsupported_snp_features(void)
> > +static bool check_snp_features(struct boot_params *bp)
> >  {
> > +     u64 status = sev_get_status();
> >       u64 unsupported;
> >
> > -     unsupported = snp_get_unsupported_features(sev_get_status());
> > +     unsupported = snp_get_unsupported_features(status);
> >       if (unsupported) {
> >               efi_err("Unsupported SEV-SNP features detected: 0x%llx\n",
> >                       unsupported);
> > -             return true;
> > +             return false;
> >       }
> > -     return false;
> > +
> > +     if (status & MSR_AMD64_SEV_SNP_ENABLED) {
> > +             void *tbl = get_efi_config_table(EFI_CC_BLOB_GUID);
> > +
> > +             if (!tbl) {
> > +                     efi_err("SEV-SNP is enabled but CC blob not found\n");
> > +                     return false;
> > +             }
> > +             bp->cc_blob_address = (u32)(unsigned long)tbl;
>
> I think we ran into bugs where the cc_blob_address was random data from a
> boot loader when SNP wasn't active and that's why we always initialize it
> to 0:
>
> 4b1c74240757 ("x86/boot: Don't propagate uninitialized boot_params->cc_blob_address")
>
> So we probably need the same statement that is at the beginning of the
> decompressor sev_enable() at the very beginning of this function to ensure
> cc_blob_address is set to zero:
>
>         /*
>          * bp->cc_blob_address should only be set by boot/compressed kernel.
>          * Initialize it to 0 to ensure that uninitialized values from
>          * buggy bootloaders aren't propagated.
>          */
>         if (bp)
>                 bp->cc_blob_address = 0;
>

AIUI this was needed for bootloaders like SYSLINUX, which boot in
pseudo-EFI mode, i.e., not via the EFI stub but with the EFI fields in
struct boot_params populated. This means zeroing the field in the stub
is not going to make a difference.

It doesn't hurt either, so I can find a place to stick this, but I'm
not convinced we still need this here. Note that GRUB no longer boots
via the EFI handover protocol (and mainline GRUB never did), and so
struct boot_params is typically allocated (and wiped) by the EFI stub
and not taken from the bootloader.

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

[Tom Lendacky]

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.

Thanks,
Tom

> -
>  	efi_5level_switch();
>  
>  	enter_kernel(kernel_entry, boot_params);

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

[Ard Biesheuvel]

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.

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

[Ard Biesheuvel]

On Fri, 12 Sept 2025 at 09:29, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> 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.
>

Hmm, I only just spotted that this check only happens in the legacy
decompressor.

I think it makes sense for this check to live in svsm_setup_ca(), but
what is your take on the need to perform this check when accepting
memory from the stub using the CA address obtained from the firmware?
(i.e., way before sev_enable() is called)

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

[Tom Lendacky]

On 9/12/25 03:26, Ard Biesheuvel wrote:
> On Fri, 12 Sept 2025 at 09:29, Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> 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.
>>
> 
> Hmm, I only just spotted that this check only happens in the legacy
> decompressor.
> 
> I think it makes sense for this check to live in svsm_setup_ca(), but
> what is your take on the need to perform this check when accepting
> memory from the stub using the CA address obtained from the firmware?
> (i.e., way before sev_enable() is called)

Yes, it seems like it should be checked before memory acceptance if we're
using an SVSM, so early_is_sevsnp_guest() looks appropriate. But since
this may not be called, the check also has to be performed by the core
kernel in svsm_setup_ca(), too.

Just wondering if it is truly necessary to check in the stub just for this
case. Theoretically, the SVSM should have validated that the HV has the
necessary support before ever invoking the firmware. Just putting it in
svsm_setup_ca() seems enough to me.

@Boris, what do you think?

Thanks,
Tom

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

[Ashish Kalra]

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.

If the EFI stub no longer boots the core kernel via the traditional 
decompressor and jumps straight to it, there are some specific things 
which i see are being setup by the decompressed kernel before passing
control to the uncompressed kernel such as calling sev_prep_identity_maps()
as part of setting up the identity map: 

From sev_prep_identity_maps(): 

The Confidential Computing blob is used very early in uncompressed
kernel to find the in-memory CPUID table to handle CPUID
instructions. Make sure an identity-mapping exists so it can be
accessed after switchover.

Won't this setup in identity mapping be needed to find the 
in-memory CPUID table as this won't exist if the EFI stub boots
directly boots the core kernel skipping the decompressor ?

Thanks,
Ashish

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

[Ard Biesheuvel]

On Fri, 12 Sept 2025 at 22:36, Ashish Kalra <Ashish.Kalra@amd.com> 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.
>
> If the EFI stub no longer boots the core kernel via the traditional
> decompressor and jumps straight to it, there are some specific things
> which i see are being setup by the decompressed kernel before passing
> control to the uncompressed kernel such as calling sev_prep_identity_maps()
> as part of setting up the identity map:
>
> From sev_prep_identity_maps():
>
> The Confidential Computing blob is used very early in uncompressed
> kernel to find the in-memory CPUID table to handle CPUID
> instructions. Make sure an identity-mapping exists so it can be
> accessed after switchover.
>
> Won't this setup in identity mapping be needed to find the
> in-memory CPUID table as this won't exist if the EFI stub boots
> directly boots the core kernel skipping the decompressor ?
>

EFI maps all memory 1:1 so none of this is needed.

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

[Borislav Petkov]

On Fri, Sep 12, 2025 at 08:32:30AM -0500, Tom Lendacky wrote:
> @Boris, what do you think?

Right, as we just talked, this should be ok, but it needs a more fine-grained
review to check whether the code that goes away is present in kernel proper.
It should be but...

And then we probably should delay this until the next cycle so that it gets
a full cycle of testing instead of rushing it in now and then patching it out
again...

That's me being overly cautious ofc.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[tip: x86/sev] x86/boot: Drop unused sev_enable() fallback

[tip-bot2 for Ard Biesheuvel]

The following commit has been merged into the x86/sev branch of tip:

Commit-ID:     a3e69071289288e2721ba15254e7c5274eddd05a
Gitweb:        https://git.kernel.org/tip/a3e69071289288e2721ba15254e7c5274eddd05a
Author:        Ard Biesheuvel <ardb@kernel.org>
AuthorDate:    Tue, 09 Sep 2025 10:06:33 +02:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Thu, 20 Nov 2025 21:12:48 +01:00

x86/boot: Drop unused sev_enable() fallback

The misc.h header is not included by the EFI stub, which is the only
C caller of sev_enable(). This means the fallback for cases where
CONFIG_AMD_MEM_ENCRYPT is not set is never used, so it can be dropped.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Link: https://patch.msgid.link/20250909080631.2867579-6-ardb+git@google.com
---
 arch/x86/boot/compressed/misc.h | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index db10486..fd855e3 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -152,17 +152,6 @@ bool insn_has_rep_prefix(struct insn *insn);
 void sev_insn_decode_init(void);
 bool early_setup_ghcb(void);
 #else
-static inline void sev_enable(struct boot_params *bp)
-{
-	/*
-	 * bp->cc_blob_address should only be set by boot/compressed kernel.
-	 * Initialize it to 0 unconditionally (thus here in this stub too) to
-	 * ensure that uninitialized values from buggy bootloaders aren't
-	 * propagated.
-	 */
-	if (bp)
-		bp->cc_blob_address = 0;
-}
 static inline void snp_check_features(void) { }
 static inline void sev_es_shutdown_ghcb(void) { }
 static inline bool sev_es_check_ghcb_fault(unsigned long address)