Received-SPF: pass (google.com: domain of linux-kernel+bounces-125188-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 29 Mar 2024 09:46:25 -0700
From: Tony Luck <tony.luck@intel.com>
To: Borislav Petkov <bp@alien8.de>
Cc: "x86@kernel.org" <x86@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 01/74] x86/cpu/vfm: Add/initialize x86_vfm field to
 struct cpuinfo_x86
Message-ID: <ZgbwYb3D3tBtXZ8y@agluck-desk3>
References: <20240328163746.243023-1-tony.luck@intel.com>
 <20240328163746.243023-2-tony.luck@intel.com>
 <20240328164811.GDZgWfSzAWZXO7dUky@fat_crate.local>
 <20240328165251.GEZgWgY1Clb9z4t3VX@fat_crate.local>
 <SJ1PR11MB6083AADC97E50462C1137D71FC3B2@SJ1PR11MB6083.namprd11.prod.outlook.com>
 <20240328171204.GGZgWk5JNOzQzoaEql@fat_crate.local>
 <SJ1PR11MB6083D8BB65748452B204ED8AFC3B2@SJ1PR11MB6083.namprd11.prod.outlook.com>
 <20240329114007.GAZgaolwSFtjHStiuL@fat_crate.local>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240329114007.GAZgaolwSFtjHStiuL@fat_crate.local>

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