Re: [PATCH v4 1/7] x86/sev: Avoid WARN()s and panic()s in early boot code

Tom Lendacky
2025-01-06T15:23:32Z

On 12/5/24 05:28, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
>
> Using WARN() or panic() while executing from the early 1:1 mapping is
> unlikely to do anything useful: the string literals are passed using
> their kernel virtual addresses which are not even mapped yet. But even
> if they were, calling into the printk() machinery from the early 1:1
> mapped code is not going to get very far.
>
> So drop the WARN()s entirely, and replace panic() with a deadloop.
>
> Link: https://lore.kernel.org/all/6904c198-9047-14bb-858e-38b531589379@amd.com/T/#u
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/x86/coco/sev/core.c | 15 +++++----------
> arch/x86/coco/sev/shared.c | 9 +++++----
> 2 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index c5b0148b8c0a..499b41953e3c 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -777,15 +777,10 @@ early_set_pages_state(unsigned long vaddr, unsigned long paddr,
>
> val = sev_es_rd_ghcb_msr();
>
> - if (WARN(GHCB_RESP_CODE(val) != GHCB_MSR_PSC_RESP,
> - "Wrong PSC response code: 0x%x\n",
> - (unsigned int)GHCB_RESP_CODE(val)))
> + if (GHCB_RESP_CODE(val) != GHCB_MSR_PSC_RESP)
> goto e_term;
>
> - if (WARN(GHCB_MSR_PSC_RESP_VAL(val),
> - "Failed to change page state to '%s' paddr 0x%lx error 0x%llx\n",
> - op == SNP_PAGE_STATE_PRIVATE ? "private" : "shared",
> - paddr, GHCB_MSR_PSC_RESP_VAL(val)))
> + if (GHCB_MSR_PSC_RESP_VAL(val))
> goto e_term;
>
> /* Page validation must be performed after changing to private */
> @@ -821,7 +816,7 @@ void __head early_snp_set_memory_private(unsigned long vaddr, unsigned long padd
> early_set_pages_state(vaddr, paddr, npages, SNP_PAGE_STATE_PRIVATE);
> }
>
> -void __init early_snp_set_memory_shared(unsigned long vaddr, unsigned long paddr,
> +void __head early_snp_set_memory_shared(unsigned long vaddr, unsigned long paddr,
> unsigned long npages)
> {
> /*
> @@ -2361,8 +2356,8 @@ static __head void svsm_setup(struct cc_blob_sev_info *cc_info)
> call.rax = SVSM_CORE_CALL(SVSM_CORE_REMAP_CA);
> call.rcx = pa;
> ret = svsm_perform_call_protocol(&call);
> - if (ret)
> - panic("Can't remap the SVSM CA, ret=%d, rax_out=0x%llx\n", ret, call.rax_out);
> + while (ret)
> + cpu_relax(); /* too early to panic */

We should give some indication of the error and call sev_es_terminate()
here with a new reason code, something like:

sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_SVSM_CA_REMAP_FAIL);

Thanks,
Tom

>
> RIP_REL_REF(boot_svsm_caa) = (struct svsm_ca *)pa;
> RIP_REL_REF(boot_svsm_caa_pa) = pa;
> diff --git a/arch/x86/coco/sev/shared.c b/arch/x86/coco/sev/shared.c
> index 71de53194089..afb7ffc355fe 100644
> --- a/arch/x86/coco/sev/shared.c
> +++ b/arch/x86/coco/sev/shared.c
> @@ -1243,7 +1243,7 @@ static void svsm_pval_terminate(struct svsm_pvalidate_call *pc, int ret, u64 svs
> __pval_terminate(pfn, action, page_size, ret, svsm_ret);
> }
>
> -static void svsm_pval_4k_page(unsigned long paddr, bool validate)
> +static void __head svsm_pval_4k_page(unsigned long paddr, bool validate)
> {
> struct svsm_pvalidate_call *pc;
> struct svsm_call call = {};
> @@ -1275,12 +1275,13 @@ static void svsm_pval_4k_page(unsigned long paddr, bool validate)
>
> ret = svsm_perform_call_protocol(&call);
> if (ret)
> - svsm_pval_terminate(pc, ret, call.rax_out);
> + sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PVALIDATE);
>
> native_local_irq_restore(flags);
> }
>
> -static void pvalidate_4k_page(unsigned long vaddr, unsigned long paddr, bool validate)
> +static void __head pvalidate_4k_page(unsigned long vaddr, unsigned long paddr,
> + bool validate)
> {
> int ret;
>
> @@ -1293,7 +1294,7 @@ static void pvalidate_4k_page(unsigned long vaddr, unsigned long paddr, bool val
> } else {
> ret = pvalidate(vaddr, RMP_PG_SIZE_4K, validate);
> if (ret)
> - __pval_terminate(PHYS_PFN(paddr), validate, RMP_PG_SIZE_4K, ret, 0);
> + sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PVALIDATE);
> }
> }
>