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

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

Posted by Nikunj A Dadhania 6 days, 18 hours ago

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@amd.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
 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);
 
-- 
2.48.1

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

Posted by Tom Lendacky 5 days, 8 hours ago

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@amd.com>
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>

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

> ---
>  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);
>

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

Posted by tip-bot2 for Nikunj A Dadhania 5 days, 7 hours ago

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

Commit-ID:     9d8460a1c7a6b0f2dc6302e5d0f31d4e8c2a7913
Gitweb:        https://git.kernel.org/tip/9d8460a1c7a6b0f2dc6302e5d0f31d4e8c2a7913
Author:        Nikunj A Dadhania <nikunj@amd.com>
AuthorDate:    Mon, 18 May 2026 10:22:30 
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Tue, 19 May 2026 13:31:29 -07:00

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

After

  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 them.

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@amd.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Link: https://patch.msgid.link/20260518102230.3394603-1-nikunj@amd.com
---
 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 7ed3da9..ecd77d3 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 5afd663..e1e03f1 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 2acf4a7..916d62c 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);