linux-kernel - Re: [PATCH v7 01/22] x86/sev: Separate MSR and GHCB based snp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMj1kXFa5j2AoUWNvOmfBrQxg_hjwwPrCZSKickNwYjZ8RxkVA@mail.gmail.com>
Date: Thu, 28 Aug 2025 18:14:30 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Borislav Petkov <bp@...en8.de>
Cc: Ard Biesheuvel <ardb+git@...gle.com>, Tom Lendacky <thomas.lendacky@....com>, 
	Michael Roth <michael.roth@....com>, Jörg Rödel <joro@...tes.org>, 
	linux-kernel@...r.kernel.org, linux-efi@...r.kernel.org, x86@...nel.org, 
	Ingo Molnar <mingo@...nel.org>, Kevin Loughlin <kevinloughlin@...gle.com>, 
	Josh Poimboeuf <jpoimboe@...nel.org>, Peter Zijlstra <peterz@...radead.org>, 
	Nikunj A Dadhania <nikunj@....com>
Subject: Re: [PATCH v7 01/22] x86/sev: Separate MSR and GHCB based snp_cpuid()
 via a callback

On Thu, 28 Aug 2025 at 17:33, Borislav Petkov <bp@...en8.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.