Re: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to struct cpuinfo_x86

[Tony Luck]

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