linux-kernel - Re: [PATCH v3 3/8] x86/sev: Require the RMPREAD instruction after Fam19h

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <45d69348-44bf-94c8-528d-9f5ac1aea163@amd.com>
Date: Mon, 30 Sep 2024 13:59:54 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: Dave Hansen <dave.hansen@...el.com>, linux-kernel@...r.kernel.org,
 x86@...nel.org
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 Michael Roth <michael.roth@....com>, Ashish Kalra <ashish.kalra@....com>
Subject: Re: [PATCH v3 3/8] x86/sev: Require the RMPREAD instruction after
 Fam19h

On 9/30/24 12:03, Dave Hansen wrote:
> On 9/30/24 08:22, Tom Lendacky wrote:
>>  		if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
>> -		    c->x86 >= 0x19 && snp_probe_rmptable_info()) {
>> +		    (c->x86 == 0x19 || cpu_feature_enabled(X86_FEATURE_RMPREAD)) &&
>> +		    snp_probe_rmptable_info()) {
> 
> One humble suggestion (and not a NAK of the series): Could we please
> start using #define'd names for these family numbers?  We started doing
> it for Intel models and I think it's been really successful.  We used to
> do greps like:
> 
> 	grep -r 'x86_model == 0x0f' arch/x86/
> 
> But that misses things like '>=' or macros that build the x86_model
> comparison.  But now we can do things like:
> 
> 	grep -r INTEL_CORE2_MEROM arch/x86
> 
> which does a much better job of finding references.

The one issue we run into is that family 0x19 contains both Milan (zen3)
and Genoa (zen4), so I'm not sure what to use as a good #define name. We
have the same problem with family 0x17 which contains zen1 and zen2.

I might be able to change the if statement to something like:

	if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
	    (cpu_feature_enabled(X86_FEATURE_ZEN3) ||
	     cpu_feature_enabled(X86_FEATURE_ZEN4) ||
	     cpu_feature_enabled(X86_FEATURE_RMPREAD)) &&
	    snp_probe_rmptable_info()) {

which might make the intent clearer.

But, yes, I get your point about making grepping much easier, along with
code readability. Maybe Boris and I can put our heads together to figure
something out.

Thanks,
Tom