From: Ard Biesheuvel <ardb@kernel.org>

There are two distinct callers of snp_cpuid(): one where the MSR
protocol is always used, and one where the GHCB page based interface is
always used.

The snp_cpuid() logic does not care about the distinction, which only
matters at a lower level. But the fact that it supports both interfaces
means that the GHCB page based logic is pulled into the early startup
code where PA to VA conversions are problematic, given that it runs from
the 1:1 mapping of memory.

So keep snp_cpuid() itself in the startup code, but factor out the
hypervisor calls via a callback, so that the GHCB page handling can be
moved out.

Code refactoring only - no functional change intended.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/boot/startup/sev-shared.c | 59 ++++----------------
 arch/x86/coco/sev/vc-shared.c      | 49 +++++++++++++++-
 arch/x86/include/asm/sev.h         |  3 +-
 3 files changed, 61 insertions(+), 50 deletions(-)

diff --git a/arch/x86/boot/startup/sev-shared.c b/arch/x86/boot/startup/sev-shared.c
index a34cd19796f9..ed88dfe7605e 100644
--- a/arch/x86/boot/startup/sev-shared.c
+++ b/arch/x86/boot/startup/sev-shared.c
@@ -342,44 +342,7 @@ static int __sev_cpuid_hv_msr(struct cpuid_leaf *leaf)
 	return ret;
 }
 
-static int __sev_cpuid_hv_ghcb(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf)
-{
-	u32 cr4 = native_read_cr4();
-	int ret;
-
-	ghcb_set_rax(ghcb, leaf->fn);
-	ghcb_set_rcx(ghcb, leaf->subfn);
-
-	if (cr4 & X86_CR4_OSXSAVE)
-		/* Safe to read xcr0 */
-		ghcb_set_xcr0(ghcb, xgetbv(XCR_XFEATURE_ENABLED_MASK));
-	else
-		/* xgetbv will cause #UD - use reset value for xcr0 */
-		ghcb_set_xcr0(ghcb, 1);
-
-	ret = sev_es_ghcb_hv_call(ghcb, ctxt, SVM_EXIT_CPUID, 0, 0);
-	if (ret != ES_OK)
-		return ret;
-
-	if (!(ghcb_rax_is_valid(ghcb) &&
-	      ghcb_rbx_is_valid(ghcb) &&
-	      ghcb_rcx_is_valid(ghcb) &&
-	      ghcb_rdx_is_valid(ghcb)))
-		return ES_VMM_ERROR;
 
-	leaf->eax = ghcb->save.rax;
-	leaf->ebx = ghcb->save.rbx;
-	leaf->ecx = ghcb->save.rcx;
-	leaf->edx = ghcb->save.rdx;
-
-	return ES_OK;
-}
-
-static int sev_cpuid_hv(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf)
-{
-	return ghcb ? __sev_cpuid_hv_ghcb(ghcb, ctxt, leaf)
-		    : __sev_cpuid_hv_msr(leaf);
-}
 
 /*
  * This may be called early while still running on the initial identity
@@ -484,21 +447,21 @@ snp_cpuid_get_validated_func(struct cpuid_leaf *leaf)
 	return false;
 }
 
-static void snp_cpuid_hv(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf)
+static void snp_cpuid_hv_msr(void *ctx, struct cpuid_leaf *leaf)
 {
-	if (sev_cpuid_hv(ghcb, ctxt, leaf))
+	if (__sev_cpuid_hv_msr(leaf))
 		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_CPUID_HV);
 }
 
 static int __head
-snp_cpuid_postprocess(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
-		      struct cpuid_leaf *leaf)
+snp_cpuid_postprocess(void (*cpuid_fn)(void *ctx, struct cpuid_leaf *leaf),
+		      void *ctx, struct cpuid_leaf *leaf)
 {
 	struct cpuid_leaf leaf_hv = *leaf;
 
 	switch (leaf->fn) {
 	case 0x1:
-		snp_cpuid_hv(ghcb, ctxt, &leaf_hv);
+		cpuid_fn(ctx, &leaf_hv);
 
 		/* initial APIC ID */
 		leaf->ebx = (leaf_hv.ebx & GENMASK(31, 24)) | (leaf->ebx & GENMASK(23, 0));
@@ -517,7 +480,7 @@ snp_cpuid_postprocess(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
 		break;
 	case 0xB:
 		leaf_hv.subfn = 0;
-		snp_cpuid_hv(ghcb, ctxt, &leaf_hv);
+		cpuid_fn(ctx, &leaf_hv);
 
 		/* extended APIC ID */
 		leaf->edx = leaf_hv.edx;
@@ -565,7 +528,7 @@ snp_cpuid_postprocess(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
 		}
 		break;
 	case 0x8000001E:
-		snp_cpuid_hv(ghcb, ctxt, &leaf_hv);
+		cpuid_fn(ctx, &leaf_hv);
 
 		/* extended APIC ID */
 		leaf->eax = leaf_hv.eax;
@@ -586,8 +549,8 @@ snp_cpuid_postprocess(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
  * Returns -EOPNOTSUPP if feature not enabled. Any other non-zero return value
  * should be treated as fatal by caller.
  */
-int __head
-snp_cpuid(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf)
+int __head snp_cpuid(void (*cpuid_fn)(void *ctx, struct cpuid_leaf *leaf),
+		     void *ctx, struct cpuid_leaf *leaf)
 {
 	const struct snp_cpuid_table *cpuid_table = snp_cpuid_get_table();
 
@@ -621,7 +584,7 @@ snp_cpuid(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf)
 			return 0;
 	}
 
-	return snp_cpuid_postprocess(ghcb, ctxt, leaf);
+	return snp_cpuid_postprocess(cpuid_fn, ctx, leaf);
 }
 
 /*
@@ -648,7 +611,7 @@ void __head do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
 	leaf.fn = fn;
 	leaf.subfn = subfn;
 
-	ret = snp_cpuid(NULL, NULL, &leaf);
+	ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
 	if (!ret)
 		goto cpuid_done;
 
diff --git a/arch/x86/coco/sev/vc-shared.c b/arch/x86/coco/sev/vc-shared.c
index 2c0ab0fdc060..b4688f69102e 100644
--- a/arch/x86/coco/sev/vc-shared.c
+++ b/arch/x86/coco/sev/vc-shared.c
@@ -409,15 +409,62 @@ static enum es_result vc_handle_ioio(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
 	return ret;
 }
 
+static int __sev_cpuid_hv_ghcb(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf)
+{
+	u32 cr4 = native_read_cr4();
+	int ret;
+
+	ghcb_set_rax(ghcb, leaf->fn);
+	ghcb_set_rcx(ghcb, leaf->subfn);
+
+	if (cr4 & X86_CR4_OSXSAVE)
+		/* Safe to read xcr0 */
+		ghcb_set_xcr0(ghcb, xgetbv(XCR_XFEATURE_ENABLED_MASK));
+	else
+		/* xgetbv will cause #UD - use reset value for xcr0 */
+		ghcb_set_xcr0(ghcb, 1);
+
+	ret = sev_es_ghcb_hv_call(ghcb, ctxt, SVM_EXIT_CPUID, 0, 0);
+	if (ret != ES_OK)
+		return ret;
+
+	if (!(ghcb_rax_is_valid(ghcb) &&
+	      ghcb_rbx_is_valid(ghcb) &&
+	      ghcb_rcx_is_valid(ghcb) &&
+	      ghcb_rdx_is_valid(ghcb)))
+		return ES_VMM_ERROR;
+
+	leaf->eax = ghcb->save.rax;
+	leaf->ebx = ghcb->save.rbx;
+	leaf->ecx = ghcb->save.rcx;
+	leaf->edx = ghcb->save.rdx;
+
+	return ES_OK;
+}
+
+struct cpuid_ctx {
+	struct ghcb *ghcb;
+	struct es_em_ctxt *ctxt;
+};
+
+static void snp_cpuid_hv_ghcb(void *p, struct cpuid_leaf *leaf)
+{
+	struct cpuid_ctx *ctx = p;
+
+	if (__sev_cpuid_hv_ghcb(ctx->ghcb, ctx->ctxt, leaf))
+		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_CPUID_HV);
+}
+
 static int vc_handle_cpuid_snp(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
 {
+	struct cpuid_ctx ctx = { ghcb, ctxt };
 	struct pt_regs *regs = ctxt->regs;
 	struct cpuid_leaf leaf;
 	int ret;
 
 	leaf.fn = regs->ax;
 	leaf.subfn = regs->cx;
-	ret = snp_cpuid(ghcb, ctxt, &leaf);
+	ret = snp_cpuid(snp_cpuid_hv_ghcb, &ctx, &leaf);
 	if (!ret) {
 		regs->ax = leaf.eax;
 		regs->bx = leaf.ebx;
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 02236962fdb1..e4622e470ceb 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -552,7 +552,8 @@ struct cpuid_leaf {
 	u32 edx;
 };
 
-int snp_cpuid(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf);
+int snp_cpuid(void (*cpuid_fn)(void *ctx, struct cpuid_leaf *leaf),
+	      void *ctx, struct cpuid_leaf *leaf);
 
 void __noreturn sev_es_terminate(unsigned int set, unsigned int reason);
 enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,
-- 
2.51.0.268.g9569e192d0-goog

+ Joerg and Mike to doublecheck me.

On Thu, Aug 28, 2025 at 12:22:04PM +0200, Ard Biesheuvel wrote:
> @@ -648,7 +611,7 @@ void __head do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
>  	leaf.fn = fn;
>  	leaf.subfn = subfn;
>  
> -	ret = snp_cpuid(NULL, NULL, &leaf);
> +	ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
>  	if (!ret)
>  		goto cpuid_done;
>  

So this code becomes now:

---
        ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
        if (!ret)
                goto cpuid_done;

<--- tries to find the CPUID leaf in the CPUID table
<--- otherwise uses the MSR protocol to read CPUID from HV and massage it

        if (ret != -EOPNOTSUPP)
                goto fail;

        if (__sev_cpuid_hv_msr(&leaf))
                goto fail;

<--- and now it tries to do the same - do CPUID over MSR protocol.

This flow made sense before your change because it'll try to use the GHCB
protocol but you're zapping that now so, IOW, you can zap that second call
too:


diff --git a/arch/x86/boot/startup/sev-shared.c b/arch/x86/boot/startup/sev-shared.c
index ed88dfe7605e..fbfdfe0dce70 100644
--- a/arch/x86/boot/startup/sev-shared.c
+++ b/arch/x86/boot/startup/sev-shared.c
@@ -612,16 +612,9 @@ void __head do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
 	leaf.subfn = subfn;
 
 	ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
-	if (!ret)
-		goto cpuid_done;
-
-	if (ret != -EOPNOTSUPP)
-		goto fail;
-
-	if (__sev_cpuid_hv_msr(&leaf))
+	if (ret && ret != -EOPNOTSUPP)
 		goto fail;
 
-cpuid_done:
 	regs->ax = leaf.eax;
 	regs->bx = leaf.ebx;
 	regs->cx = leaf.ecx;

-- 
Regards/Gruss,
    Boris.

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

On 8/28/25 10:33, Borislav Petkov wrote:
> + Joerg and Mike to doublecheck me.
> 
> On Thu, Aug 28, 2025 at 12:22:04PM +0200, Ard Biesheuvel wrote:
>> @@ -648,7 +611,7 @@ void __head do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
>>  	leaf.fn = fn;
>>  	leaf.subfn = subfn;
>>  
>> -	ret = snp_cpuid(NULL, NULL, &leaf);
>> +	ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
>>  	if (!ret)
>>  		goto cpuid_done;
>>  
> 
> So this code becomes now:
> 
> ---
>         ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
>         if (!ret)
>                 goto cpuid_done;
> 
> <--- tries to find the CPUID leaf in the CPUID table
> <--- otherwise uses the MSR protocol to read CPUID from HV and massage it

It only uses the MSR protocol for particular CPUID values in
snp_cpuid_postprocess(). If the CPUID leaf isn't in the CPUID table,
then it will set the CPUID values to all 0 and then call the
post-processing routine which may or may not call the HV.

The second call to __sev_cpuid_hv_msr() only happens if there is no
CPUID table - which will be the case for SEV-ES. So you can't remove the
second call.

Thanks,
Tom

> 
>         if (ret != -EOPNOTSUPP)
>                 goto fail;
> 
>         if (__sev_cpuid_hv_msr(&leaf))
>                 goto fail;
> 
> <--- and now it tries to do the same - do CPUID over MSR protocol.
> 
> This flow made sense before your change because it'll try to use the GHCB
> protocol but you're zapping that now so, IOW, you can zap that second call
> too:
> 
> 
> diff --git a/arch/x86/boot/startup/sev-shared.c b/arch/x86/boot/startup/sev-shared.c
> index ed88dfe7605e..fbfdfe0dce70 100644
> --- a/arch/x86/boot/startup/sev-shared.c
> +++ b/arch/x86/boot/startup/sev-shared.c
> @@ -612,16 +612,9 @@ void __head do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
>  	leaf.subfn = subfn;
>  
>  	ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
> -	if (!ret)
> -		goto cpuid_done;
> -
> -	if (ret != -EOPNOTSUPP)
> -		goto fail;
> -
> -	if (__sev_cpuid_hv_msr(&leaf))
> +	if (ret && ret != -EOPNOTSUPP)
>  		goto fail;
>  
> -cpuid_done:
>  	regs->ax = leaf.eax;
>  	regs->bx = leaf.ebx;
>  	regs->cx = leaf.ecx;
>

On Tue, Sep 09, 2025 at 04:44:07PM -0500, Tom Lendacky wrote:
> It only uses the MSR protocol for particular CPUID values in
> snp_cpuid_postprocess(). If the CPUID leaf isn't in the CPUID table,
> then it will set the CPUID values to all 0 and then call the
> post-processing routine which may or may not call the HV.
> 
> The second call to __sev_cpuid_hv_msr() only happens if there is no
> CPUID table - which will be the case for SEV-ES. So you can't remove the
> second call.

This needs to be turned into a proper comment, at least, and stuck above it as
the situation there is clear as mud. Especially after the dropping of the GHCB
protocol call, which makes the confusion even more probable...

I'll do it tomorrow if you don't beat me to it today. :)

-- 
Regards/Gruss,
    Boris.

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

On 9/9/25 17:20, Borislav Petkov wrote:
> On Tue, Sep 09, 2025 at 04:44:07PM -0500, Tom Lendacky wrote:
>> It only uses the MSR protocol for particular CPUID values in
>> snp_cpuid_postprocess(). If the CPUID leaf isn't in the CPUID table,
>> then it will set the CPUID values to all 0 and then call the
>> post-processing routine which may or may not call the HV.
>>
>> The second call to __sev_cpuid_hv_msr() only happens if there is no
>> CPUID table - which will be the case for SEV-ES. So you can't remove the
>> second call.
> 
> This needs to be turned into a proper comment, at least, and stuck above it as
> the situation there is clear as mud. Especially after the dropping of the GHCB
> protocol call, which makes the confusion even more probable...
> 
> I'll do it tomorrow if you don't beat me to it today. :)

Something like this?:

diff --git a/arch/x86/boot/startup/sev-shared.c b/arch/x86/boot/startup/sev-shared.c
index 08cc1568d8af..eb7a7b45f773 100644
--- a/arch/x86/boot/startup/sev-shared.c
+++ b/arch/x86/boot/startup/sev-shared.c
@@ -458,6 +458,13 @@ void do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
 	leaf.fn = fn;
 	leaf.subfn = subfn;
 
+	/*
+	 * If SNP is active, then snp_cpuid() uses the CPUID table to obtain the
+	 * CPUID values (with possible HV interaction during post-processing of
+	 * the values). But if SNP is not active (no CPUID table present), then
+	 * snp_cpuid() returns -EOPNOTSUPP so that an SEV-ES guest can call the
+	 * HV to obtain the CPUID information.
+	 */
 	ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
 	if (!ret)
 		goto cpuid_done;
@@ -465,6 +472,10 @@ void do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
 	if (ret != -EOPNOTSUPP)
 		goto fail;
 
+	/*
+	 * If we got here, we're an SEV-ES guest and need to invoke the HV for
+	 * the CPUID data.
+	 */
 	if (__sev_cpuid_hv_msr(&leaf))
 		goto fail;
 

>

On Thu, 28 Aug 2025 at 17:33, Borislav Petkov <bp@alien8.de> wrote:
>
> + Joerg and Mike to doublecheck me.
>
> On Thu, Aug 28, 2025 at 12:22:04PM +0200, Ard Biesheuvel wrote:
> > @@ -648,7 +611,7 @@ void __head do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
> >       leaf.fn = fn;
> >       leaf.subfn = subfn;
> >
> > -     ret = snp_cpuid(NULL, NULL, &leaf);
> > +     ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
> >       if (!ret)
> >               goto cpuid_done;
> >
>
> So this code becomes now:
>
> ---
>         ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
>         if (!ret)
>                 goto cpuid_done;
>
> <--- tries to find the CPUID leaf in the CPUID table
> <--- otherwise uses the MSR protocol to read CPUID from HV and massage it
>
>         if (ret != -EOPNOTSUPP)
>                 goto fail;
>
>         if (__sev_cpuid_hv_msr(&leaf))
>                 goto fail;
>
> <--- and now it tries to do the same - do CPUID over MSR protocol.
>
> This flow made sense before your change because it'll try to use the GHCB
> protocol but you're zapping that now so, IOW, you can zap that second call
> too:
>
>
> diff --git a/arch/x86/boot/startup/sev-shared.c b/arch/x86/boot/startup/sev-shared.c
> index ed88dfe7605e..fbfdfe0dce70 100644
> --- a/arch/x86/boot/startup/sev-shared.c
> +++ b/arch/x86/boot/startup/sev-shared.c
> @@ -612,16 +612,9 @@ void __head do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
>         leaf.subfn = subfn;
>
>         ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
> -       if (!ret)
> -               goto cpuid_done;
> -
> -       if (ret != -EOPNOTSUPP)
> -               goto fail;
> -
> -       if (__sev_cpuid_hv_msr(&leaf))
> +       if (ret && ret != -EOPNOTSUPP)
>                 goto fail;
>
> -cpuid_done:
>         regs->ax = leaf.eax;
>         regs->bx = leaf.ebx;
>         regs->cx = leaf.ecx;
>

This seems plausible but I'm not sure I understand 100% why this
fallback logic was introduced in the first place, so I'll defer to the
experts here.

On 8/28/25 11:14, Ard Biesheuvel wrote:
> On Thu, 28 Aug 2025 at 17:33, Borislav Petkov <bp@alien8.de> wrote:
>>
>> + Joerg and Mike to doublecheck me.
>>
>> On Thu, Aug 28, 2025 at 12:22:04PM +0200, Ard Biesheuvel wrote:
>>> @@ -648,7 +611,7 @@ void __head do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
>>>       leaf.fn = fn;
>>>       leaf.subfn = subfn;
>>>
>>> -     ret = snp_cpuid(NULL, NULL, &leaf);
>>> +     ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
>>>       if (!ret)
>>>               goto cpuid_done;
>>>
>>
>> So this code becomes now:
>>
>> ---
>>         ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
>>         if (!ret)
>>                 goto cpuid_done;
>>
>> <--- tries to find the CPUID leaf in the CPUID table
>> <--- otherwise uses the MSR protocol to read CPUID from HV and massage it
>>
>>         if (ret != -EOPNOTSUPP)
>>                 goto fail;
>>
>>         if (__sev_cpuid_hv_msr(&leaf))
>>                 goto fail;
>>
>> <--- and now it tries to do the same - do CPUID over MSR protocol.
>>
>> This flow made sense before your change because it'll try to use the GHCB
>> protocol but you're zapping that now so, IOW, you can zap that second call
>> too:
>>
>>
>> diff --git a/arch/x86/boot/startup/sev-shared.c b/arch/x86/boot/startup/sev-shared.c
>> index ed88dfe7605e..fbfdfe0dce70 100644
>> --- a/arch/x86/boot/startup/sev-shared.c
>> +++ b/arch/x86/boot/startup/sev-shared.c
>> @@ -612,16 +612,9 @@ void __head do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
>>         leaf.subfn = subfn;
>>
>>         ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
>> -       if (!ret)
>> -               goto cpuid_done;
>> -
>> -       if (ret != -EOPNOTSUPP)
>> -               goto fail;
>> -
>> -       if (__sev_cpuid_hv_msr(&leaf))
>> +       if (ret && ret != -EOPNOTSUPP)
>>                 goto fail;
>>
>> -cpuid_done:
>>         regs->ax = leaf.eax;
>>         regs->bx = leaf.ebx;
>>         regs->cx = leaf.ecx;
>>
> 
> This seems plausible but I'm not sure I understand 100% why this
> fallback logic was introduced in the first place, so I'll defer to the
> experts here.

The fallback logic is for supporting SNP guests (which will have a CPUID
table) and SEV-ES guest (which won't have a CPUID table).

Thanks,
Tom

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

Commit-ID:     e2e29752357f32feb69a68e9e6e7361405b3f289
Gitweb:        https://git.kernel.org/tip/e2e29752357f32feb69a68e9e6e7361405b3f289
Author:        Ard Biesheuvel <ardb@kernel.org>
AuthorDate:    Thu, 28 Aug 2025 12:22:04 +02:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Thu, 28 Aug 2025 16:52:05 +02:00

x86/sev: Separate MSR and GHCB based snp_cpuid() via a callback

There are two distinct callers of snp_cpuid(): the MSR protocol and the GHCB
page based interface.

The snp_cpuid() logic does not care about the distinction, which only matters
at a lower level. But the fact that it supports both interfaces means that the
GHCB page based logic is pulled into the early startup code where PA to VA
conversions are problematic, given that it runs from the 1:1 mapping of memory.

So keep snp_cpuid() itself in the startup code, but factor out the hypervisor
calls via a callback, so that the GHCB page handling can be moved out.

Code refactoring only - no functional change intended.

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://lore.kernel.org/20250828102202.1849035-25-ardb+git@google.com
---
 arch/x86/boot/startup/sev-shared.c | 59 +++++------------------------
 arch/x86/coco/sev/vc-shared.c      | 49 +++++++++++++++++++++++-
 arch/x86/include/asm/sev.h         |  3 +-
 3 files changed, 61 insertions(+), 50 deletions(-)

diff --git a/arch/x86/boot/startup/sev-shared.c b/arch/x86/boot/startup/sev-shared.c
index a34cd19..ed88dfe 100644
--- a/arch/x86/boot/startup/sev-shared.c
+++ b/arch/x86/boot/startup/sev-shared.c
@@ -342,44 +342,7 @@ static int __sev_cpuid_hv_msr(struct cpuid_leaf *leaf)
 	return ret;
 }
 
-static int __sev_cpuid_hv_ghcb(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf)
-{
-	u32 cr4 = native_read_cr4();
-	int ret;
-
-	ghcb_set_rax(ghcb, leaf->fn);
-	ghcb_set_rcx(ghcb, leaf->subfn);
-
-	if (cr4 & X86_CR4_OSXSAVE)
-		/* Safe to read xcr0 */
-		ghcb_set_xcr0(ghcb, xgetbv(XCR_XFEATURE_ENABLED_MASK));
-	else
-		/* xgetbv will cause #UD - use reset value for xcr0 */
-		ghcb_set_xcr0(ghcb, 1);
-
-	ret = sev_es_ghcb_hv_call(ghcb, ctxt, SVM_EXIT_CPUID, 0, 0);
-	if (ret != ES_OK)
-		return ret;
-
-	if (!(ghcb_rax_is_valid(ghcb) &&
-	      ghcb_rbx_is_valid(ghcb) &&
-	      ghcb_rcx_is_valid(ghcb) &&
-	      ghcb_rdx_is_valid(ghcb)))
-		return ES_VMM_ERROR;
 
-	leaf->eax = ghcb->save.rax;
-	leaf->ebx = ghcb->save.rbx;
-	leaf->ecx = ghcb->save.rcx;
-	leaf->edx = ghcb->save.rdx;
-
-	return ES_OK;
-}
-
-static int sev_cpuid_hv(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf)
-{
-	return ghcb ? __sev_cpuid_hv_ghcb(ghcb, ctxt, leaf)
-		    : __sev_cpuid_hv_msr(leaf);
-}
 
 /*
  * This may be called early while still running on the initial identity
@@ -484,21 +447,21 @@ snp_cpuid_get_validated_func(struct cpuid_leaf *leaf)
 	return false;
 }
 
-static void snp_cpuid_hv(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf)
+static void snp_cpuid_hv_msr(void *ctx, struct cpuid_leaf *leaf)
 {
-	if (sev_cpuid_hv(ghcb, ctxt, leaf))
+	if (__sev_cpuid_hv_msr(leaf))
 		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_CPUID_HV);
 }
 
 static int __head
-snp_cpuid_postprocess(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
-		      struct cpuid_leaf *leaf)
+snp_cpuid_postprocess(void (*cpuid_fn)(void *ctx, struct cpuid_leaf *leaf),
+		      void *ctx, struct cpuid_leaf *leaf)
 {
 	struct cpuid_leaf leaf_hv = *leaf;
 
 	switch (leaf->fn) {
 	case 0x1:
-		snp_cpuid_hv(ghcb, ctxt, &leaf_hv);
+		cpuid_fn(ctx, &leaf_hv);
 
 		/* initial APIC ID */
 		leaf->ebx = (leaf_hv.ebx & GENMASK(31, 24)) | (leaf->ebx & GENMASK(23, 0));
@@ -517,7 +480,7 @@ snp_cpuid_postprocess(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
 		break;
 	case 0xB:
 		leaf_hv.subfn = 0;
-		snp_cpuid_hv(ghcb, ctxt, &leaf_hv);
+		cpuid_fn(ctx, &leaf_hv);
 
 		/* extended APIC ID */
 		leaf->edx = leaf_hv.edx;
@@ -565,7 +528,7 @@ snp_cpuid_postprocess(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
 		}
 		break;
 	case 0x8000001E:
-		snp_cpuid_hv(ghcb, ctxt, &leaf_hv);
+		cpuid_fn(ctx, &leaf_hv);
 
 		/* extended APIC ID */
 		leaf->eax = leaf_hv.eax;
@@ -586,8 +549,8 @@ snp_cpuid_postprocess(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
  * Returns -EOPNOTSUPP if feature not enabled. Any other non-zero return value
  * should be treated as fatal by caller.
  */
-int __head
-snp_cpuid(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf)
+int __head snp_cpuid(void (*cpuid_fn)(void *ctx, struct cpuid_leaf *leaf),
+		     void *ctx, struct cpuid_leaf *leaf)
 {
 	const struct snp_cpuid_table *cpuid_table = snp_cpuid_get_table();
 
@@ -621,7 +584,7 @@ snp_cpuid(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf)
 			return 0;
 	}
 
-	return snp_cpuid_postprocess(ghcb, ctxt, leaf);
+	return snp_cpuid_postprocess(cpuid_fn, ctx, leaf);
 }
 
 /*
@@ -648,7 +611,7 @@ void __head do_vc_no_ghcb(struct pt_regs *regs, unsigned long exit_code)
 	leaf.fn = fn;
 	leaf.subfn = subfn;
 
-	ret = snp_cpuid(NULL, NULL, &leaf);
+	ret = snp_cpuid(snp_cpuid_hv_msr, NULL, &leaf);
 	if (!ret)
 		goto cpuid_done;
 
diff --git a/arch/x86/coco/sev/vc-shared.c b/arch/x86/coco/sev/vc-shared.c
index 2c0ab0f..b4688f6 100644
--- a/arch/x86/coco/sev/vc-shared.c
+++ b/arch/x86/coco/sev/vc-shared.c
@@ -409,15 +409,62 @@ static enum es_result vc_handle_ioio(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
 	return ret;
 }
 
+static int __sev_cpuid_hv_ghcb(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf)
+{
+	u32 cr4 = native_read_cr4();
+	int ret;
+
+	ghcb_set_rax(ghcb, leaf->fn);
+	ghcb_set_rcx(ghcb, leaf->subfn);
+
+	if (cr4 & X86_CR4_OSXSAVE)
+		/* Safe to read xcr0 */
+		ghcb_set_xcr0(ghcb, xgetbv(XCR_XFEATURE_ENABLED_MASK));
+	else
+		/* xgetbv will cause #UD - use reset value for xcr0 */
+		ghcb_set_xcr0(ghcb, 1);
+
+	ret = sev_es_ghcb_hv_call(ghcb, ctxt, SVM_EXIT_CPUID, 0, 0);
+	if (ret != ES_OK)
+		return ret;
+
+	if (!(ghcb_rax_is_valid(ghcb) &&
+	      ghcb_rbx_is_valid(ghcb) &&
+	      ghcb_rcx_is_valid(ghcb) &&
+	      ghcb_rdx_is_valid(ghcb)))
+		return ES_VMM_ERROR;
+
+	leaf->eax = ghcb->save.rax;
+	leaf->ebx = ghcb->save.rbx;
+	leaf->ecx = ghcb->save.rcx;
+	leaf->edx = ghcb->save.rdx;
+
+	return ES_OK;
+}
+
+struct cpuid_ctx {
+	struct ghcb *ghcb;
+	struct es_em_ctxt *ctxt;
+};
+
+static void snp_cpuid_hv_ghcb(void *p, struct cpuid_leaf *leaf)
+{
+	struct cpuid_ctx *ctx = p;
+
+	if (__sev_cpuid_hv_ghcb(ctx->ghcb, ctx->ctxt, leaf))
+		sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_CPUID_HV);
+}
+
 static int vc_handle_cpuid_snp(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
 {
+	struct cpuid_ctx ctx = { ghcb, ctxt };
 	struct pt_regs *regs = ctxt->regs;
 	struct cpuid_leaf leaf;
 	int ret;
 
 	leaf.fn = regs->ax;
 	leaf.subfn = regs->cx;
-	ret = snp_cpuid(ghcb, ctxt, &leaf);
+	ret = snp_cpuid(snp_cpuid_hv_ghcb, &ctx, &leaf);
 	if (!ret) {
 		regs->ax = leaf.eax;
 		regs->bx = leaf.ebx;
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 0223696..e4622e4 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -552,7 +552,8 @@ struct cpuid_leaf {
 	u32 edx;
 };
 
-int snp_cpuid(struct ghcb *ghcb, struct es_em_ctxt *ctxt, struct cpuid_leaf *leaf);
+int snp_cpuid(void (*cpuid_fn)(void *ctx, struct cpuid_leaf *leaf),
+	      void *ctx, struct cpuid_leaf *leaf);
 
 void __noreturn sev_es_terminate(unsigned int set, unsigned int reason);
 enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,