Re: [PATCH] x86/sev: Remove redundant ghcbs_initialized checks around __sev_{get,put}_ghcb()

[Tom Lendacky]

On 5/18/26 05:22, Nikunj A Dadhania wrote:
> After commit 3645eb7e3915 ("x86/fred: Fix early boot failures on
> SEV-ES/SNP guests"), __sev_{get,put}_ghcb() handle the early-boot GHCB
> fallback internally, making the ghcbs_initialized guards in
> __set_pages_state() and svsm_perform_call_protocol() redundant.
> 
> Remove these redundant conditionals and call __sev_{get,put}_ghcb()
> unconditionally.
> 
> Also initialize state->ghcb to NULL in the early-boot path of
> __sev_get_ghcb() so that the ghcb_state is well-defined for all callers,
> even though __sev_put_ghcb() currently returns early before reading it.
> 
> No functional change intended.
> 
> Suggested-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> Signed-off-by: Nikunj A Dadhania <nikunj@xxxxxxx>

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

> ---
>  arch/x86/coco/sev/core.c    |  8 ++------
>  arch/x86/coco/sev/noinstr.c |  4 +++-
>  arch/x86/coco/sev/svsm.c    | 10 ++--------
>  3 files changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index 7ed3da998489d..ecd77d3217f3c 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -367,17 +367,13 @@ static unsigned long __set_pages_state(struct snp_psc_desc *data, unsigned long
>  
>  	local_irq_save(flags);
>  
> -	if (sev_cfg.ghcbs_initialized)
> -		ghcb = __sev_get_ghcb(&state);
> -	else
> -		ghcb = boot_ghcb;
> +	ghcb = __sev_get_ghcb(&state);
>  
>  	/* Invoke the hypervisor to perform the page state changes */
>  	if (!ghcb || vmgexit_psc(ghcb, data))
>  		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PSC);
>  
> -	if (sev_cfg.ghcbs_initialized)
> -		__sev_put_ghcb(&state);
> +	__sev_put_ghcb(&state);
>  
>  	local_irq_restore(flags);
>  
> diff --git a/arch/x86/coco/sev/noinstr.c b/arch/x86/coco/sev/noinstr.c
> index 5afd663a1c217..e1e03f12fc7bb 100644
> --- a/arch/x86/coco/sev/noinstr.c
> +++ b/arch/x86/coco/sev/noinstr.c
> @@ -121,8 +121,10 @@ noinstr struct ghcb *__sev_get_ghcb(struct ghcb_state *state)
>  
>  	WARN_ON(!irqs_disabled());
>  
> -	if (!sev_cfg.ghcbs_initialized)
> +	if (!sev_cfg.ghcbs_initialized) {
> +		state->ghcb = NULL;
>  		return boot_ghcb;
> +	}
>  
>  	data = this_cpu_read(runtime_data);
>  	ghcb = &data->ghcb_page;
> diff --git a/arch/x86/coco/sev/svsm.c b/arch/x86/coco/sev/svsm.c
> index 2acf4a76afe7a..916d62cd17dc7 100644
> --- a/arch/x86/coco/sev/svsm.c
> +++ b/arch/x86/coco/sev/svsm.c
> @@ -74,20 +74,14 @@ int svsm_perform_call_protocol(struct svsm_call *call)
>  
>  	flags = native_local_irq_save();
>  
> -	if (sev_cfg.ghcbs_initialized)
> -		ghcb = __sev_get_ghcb(&state);
> -	else if (boot_ghcb)
> -		ghcb = boot_ghcb;
> -	else
> -		ghcb = NULL;
> +	ghcb = __sev_get_ghcb(&state);
>  
>  	do {
>  		ret = ghcb ? svsm_perform_ghcb_protocol(ghcb, call)
>  			   : __pi_svsm_perform_msr_protocol(call);
>  	} while (ret == -EAGAIN);
>  
> -	if (sev_cfg.ghcbs_initialized)
> -		__sev_put_ghcb(&state);
> +	__sev_put_ghcb(&state);
>  
>  	native_local_irq_restore(flags);
>