linux-kernel - Re: [PATCH v6 21/22] x86/boot: Move startup code out of _

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <38f6fa08-41fb-4717-9763-39ec5fa54075@amd.com>
Date: Thu, 14 Aug 2025 12:17:40 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Borislav Petkov <bp@...en8.de>
Cc: Ard Biesheuvel <ardb+git@...gle.com>, linux-kernel@...r.kernel.org,
 linux-efi@...r.kernel.org, x86@...nel.org, Ard Biesheuvel <ardb@...nel.org>,
 Ingo Molnar <mingo@...nel.org>, Kevin Loughlin <kevinloughlin@...gle.com>,
 Josh Poimboeuf <jpoimboe@...nel.org>, Peter Zijlstra <peterz@...radead.org>,
 Nikunj A Dadhania <nikunj@....com>, Neeraj Upadhyay <Neeraj.Upadhyay@....com>
Subject: Re: [PATCH v6 21/22] x86/boot: Move startup code out of __head
 section

On 8/11/25 14:03, Borislav Petkov wrote:
> On Mon, Aug 11, 2025 at 01:05:42PM -0500, Tom Lendacky wrote:
>> Yes, that works. Or just get rid of snp_abort() and call
>> sev_es_terminate() directly. Secure AVIC could even use an
>> SEV_TERM_SET_LINUX specific code instead of the generic failure code.
> 
> I *love* deleting code. Here's something to start the debate:
> 
> ---
> diff --git a/arch/x86/boot/startup/sev-startup.c b/arch/x86/boot/startup/sev-startup.c
> index 7a8128dc076e..19b23e6d2dbe 100644
> --- a/arch/x86/boot/startup/sev-startup.c
> +++ b/arch/x86/boot/startup/sev-startup.c
> @@ -135,7 +135,7 @@ static struct cc_blob_sev_info *__init find_cc_blob(struct boot_params *bp)
>  
>  found_cc_info:
>  	if (cc_info->magic != CC_BLOB_SEV_HDR_MAGIC)
> -		snp_abort();
> +		sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
>  
>  	return cc_info;
>  }
> @@ -209,8 +209,3 @@ bool __init snp_init(struct boot_params *bp)
>  
>  	return true;
>  }
> -
> -void __init __noreturn snp_abort(void)
> -{
> -	sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
> -}
> diff --git a/arch/x86/boot/startup/sme.c b/arch/x86/boot/startup/sme.c
> index 39e7e9d18974..e389b39fa2a9 100644
> --- a/arch/x86/boot/startup/sme.c
> +++ b/arch/x86/boot/startup/sme.c
> @@ -531,7 +531,7 @@ void __init sme_enable(struct boot_params *bp)
>  	 * enablement abort the guest.
>  	 */
>  	if (snp_en ^ !!(msr & MSR_AMD64_SEV_SNP_ENABLED))
> -		snp_abort();
> +		sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
>  
>  	/* Check if memory encryption is enabled */
>  	if (feature_mask == AMD_SME_BIT) {
> diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
> index 0020d77a0800..01a6e4dbe423 100644
> --- a/arch/x86/include/asm/sev-common.h
> +++ b/arch/x86/include/asm/sev-common.h
> @@ -208,6 +208,7 @@ struct snp_psc_desc {
>  #define GHCB_TERM_SVSM_CAA		9	/* SVSM is present but CAA is not page aligned */
>  #define GHCB_TERM_SECURE_TSC		10	/* Secure TSC initialization failed */
>  #define GHCB_TERM_SVSM_CA_REMAP_FAIL	11	/* SVSM is present but CA could not be remapped */
> +#define GHCB_TERM_SAVIC_FAIL		12	/* Secure AVIC-specific failure */

We can get specific if desired, e.g., GHCB_TERM_SAVIC_NO_X2APIC

Thanks,
Tom

>  
>  #define GHCB_RESP_CODE(v)		((v) & GHCB_MSR_INFO_MASK)
>  
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 2b8a779f1477..e907646b4e4b 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -512,7 +512,6 @@ void snp_set_memory_shared(unsigned long vaddr, unsigned long npages);
>  void snp_set_memory_private(unsigned long vaddr, unsigned long npages);
>  void snp_set_wakeup_secondary_cpu(void);
>  bool snp_init(struct boot_params *bp);
> -void __noreturn snp_abort(void);
>  void snp_dmi_setup(void);
>  int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call, struct svsm_attest_call *input);
>  void snp_accept_memory(phys_addr_t start, phys_addr_t end);
> @@ -590,7 +589,6 @@ static inline void snp_set_memory_shared(unsigned long vaddr, unsigned long npag
>  static inline void snp_set_memory_private(unsigned long vaddr, unsigned long npages) { }
>  static inline void snp_set_wakeup_secondary_cpu(void) { }
>  static inline bool snp_init(struct boot_params *bp) { return false; }
> -static inline void snp_abort(void) { }
>  static inline void snp_dmi_setup(void) { }
>  static inline int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call, struct svsm_attest_call *input)
>  {
> diff --git a/arch/x86/kernel/apic/x2apic_savic.c b/arch/x86/kernel/apic/x2apic_savic.c
> index bea844f28192..f0270ce16e6c 100644
> --- a/arch/x86/kernel/apic/x2apic_savic.c
> +++ b/arch/x86/kernel/apic/x2apic_savic.c
> @@ -26,7 +26,7 @@ static int savic_probe(void)
>  
>  	if (!x2apic_mode) {
>  		pr_err("Secure AVIC enabled in non x2APIC mode\n");
> -		snp_abort();
> +		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SAVIC_FAIL);
>  		/* unreachable */
>  	}
>  
> diff --git a/tools/objtool/noreturns.h b/tools/objtool/noreturns.h
> index 6a922d046b8e..802895fae3ca 100644
> --- a/tools/objtool/noreturns.h
> +++ b/tools/objtool/noreturns.h
> @@ -45,7 +45,6 @@ NORETURN(rewind_stack_and_make_dead)
>  NORETURN(rust_begin_unwind)
>  NORETURN(rust_helper_BUG)
>  NORETURN(sev_es_terminate)
> -NORETURN(snp_abort)
>  NORETURN(start_kernel)
>  NORETURN(stop_this_cpu)
>  NORETURN(usercopy_abort)
>