linux-kernel - Re: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to struct cpuinfo

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: <ZgbwYb3D3tBtXZ8y@agluck-desk3>
Date: Fri, 29 Mar 2024 09:46:25 -0700
From: Tony Luck <tony.luck@...el.com>
To: Borislav Petkov <bp@...en8.de>
Cc: "x86@...nel.org" <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to
 struct cpuinfo_x86

On Fri, Mar 29, 2024 at 12:40:07PM +0100, Borislav Petkov wrote:
> On Thu, Mar 28, 2024 at 06:32:35PM +0000, Luck, Tony wrote:
> > I don't think the format is really that big an issue. Including stepping in the
> > format adds complexity to a thousand places these checks are made while
> > only being useful in a few dozen.
> 
> I've figured out what the problem is with steppings - ranges. If you
> have a range of steppings which all belong to the same model, then you
> have to complicate the checks by either masking out the stepping or use
> the X86_STEPPING_ANY thing which forces you to use x86_match_cpu()
> instead of a simple integer comparison.

I think you are talking about a range of models that all belong to
the same family (rather than steppings in the same model).

> And you should talk to your folks what their plan is for the new
> families because if they do a range of model numbers which all belong to
> the same CPU model like AMD does, then your simple comparison scheme
> goes out the window because it can't really deal with ranges.

History of Intel model number allocations apparently looks like
we just throw a dart in the general area of a block of unused
model numbers :-)  I will check with the relevent folks, but this
seems unlikely. There's more push (from the Linux community!) to
assign CPUID feature bits for stuff that goes more than 2-3
CPU generations. That leaves the stuff that is different almost
every time (perfomaance counters, power management, EDAC, etc.).

> Because from looking at your set, I don't see a slick way to check
> whether a concrete f/m/s tuple belongs to a range without involved
> checking.
> 
> For example, models:
> 
>                 case 0x30 ... 0x4f:
>                 case 0x60 ... 0x7f:
>                 case 0x90 ... 0x91:
>                 case 0xa0 ... 0xaf:
> 
> are all Zen2. I could do a X86_MATCH_VF_MODEL_RANGE and we even had

I'm glad I don't have to keep track of groups of hex numbers like that.

> a patch like that at some point but it didn't go in. But even if I did
> that, I'd still need to do x86_match_cpu() instead of the current
> X86_FEATURE_ZEN* checks we're doing.

My patch doesn't help with this, but doesn't prevent you from doing
a switch (c->x86_model).  If that list of model number ranges shows
up more than twice you could add a helper that converts that list to
a #define AMD_ZEN2 to make the code clearer.

> So I don't think I can switch AMD to use that. It looks like the 'V' in
> "VFM" could just as well turn into "I".

Patch 3 includes:

#define IFM(_fam, _model)      VFM_MAKE(X86_VENDOR_INTEL, _fam, _model)

as a helper to build model numbers in <asm/intel-family.h>
> 
> :-)
> 
> I'd say.

So keep the "V" in the common code. Maybe one of the other x86
vendors will want to have #define names for their CPU models
some day.

Thanks for digging into this.

-Tony