Re: [PATCH v7 01/22] x86/sev: Separate MSR and GHCB based snp_cpuid() via a callback

Tom Lendacky
2025-09-09T21:44:13Z

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