Messages in this thread | | | Date | Fri, 29 Mar 2024 09:46:25 -0700 | From | Tony Luck <> | 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
| |