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: <4914b8ad-6a9e-75df-4f57-13529ca9b58d@amd.com>
Date: Wed, 10 Sep 2025 08:57:26 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Borislav Petkov <bp@...en8.de>
Cc: Ard Biesheuvel <ardb+git@...gle.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,
 Ard Biesheuvel <ardb@...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 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;
 

>