[v1] x86/cpu: Add two Intel CPU model numbers

[PATCH] x86/cpu: Add two Intel CPU model numbers

Posted by Tony Luck 1 year, 4 months ago

Pantherlake is a mobile CPU. Diamond Rapids next generation Xeon.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/include/asm/intel-family.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h
index 44949f972826..1a42f829667a 100644
--- a/arch/x86/include/asm/intel-family.h
+++ b/arch/x86/include/asm/intel-family.h
@@ -135,6 +135,8 @@
 
 #define INTEL_LUNARLAKE_M		IFM(6, 0xBD)
 
+#define INTEL_PANTHERLAKE_L		IFM(6, 0xCC)
+
 /* "Small Core" Processors (Atom/E-Core) */
 
 #define INTEL_ATOM_BONNELL		IFM(6, 0x1C) /* Diamondville, Pineview */
@@ -178,4 +180,7 @@
 #define INTEL_FAM5_QUARK_X1000		0x09 /* Quark X1000 SoC */
 #define INTEL_QUARK_X1000		IFM(5, 0x09) /* Quark X1000 SoC */
 
+/* Family 19 */
+#define INTEL_PANTHERCOVE_X		IFM(19, 0x01) /* Diamond Rapids */
+
 #endif /* _ASM_X86_INTEL_FAMILY_H */
-- 
2.46.1

Re: [PATCH] x86/cpu: Add two Intel CPU model numbers

Posted by Peter Zijlstra 11 months, 4 weeks ago

On Mon, Sep 23, 2024 at 10:37:50AM -0700, Tony Luck wrote:
> Pantherlake is a mobile CPU. Diamond Rapids next generation Xeon.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/include/asm/intel-family.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h
> index 44949f972826..1a42f829667a 100644
> --- a/arch/x86/include/asm/intel-family.h
> +++ b/arch/x86/include/asm/intel-family.h
> @@ -135,6 +135,8 @@
>  
>  #define INTEL_LUNARLAKE_M		IFM(6, 0xBD)
>  
> +#define INTEL_PANTHERLAKE_L		IFM(6, 0xCC)

That should have: /* Cougar Cove / Crestmont */ added on.

Gotta love how Pantherlake is Cougar Cove and DMR is Panther Cove,
someone must be doing this on purpose to mess with us.

Re: [PATCH] x86/cpu: Add two Intel CPU model numbers

Posted by Andrew Cooper 12 months ago

> @@ -178,4 +180,7 @@  #define INTEL_FAM5_QUARK_X1000		0x09 /* Quark X1000 SoC */
>  #define INTEL_QUARK_X1000		IFM(5, 0x09) /* Quark X1000 SoC */
>  
> +/* Family 19 */ +#define INTEL_PANTHERCOVE_X IFM(19, 0x01) /* Diamond
> Rapids */

Is it intentional that this is not INTEL_DIAMONDRAPIDS_X like
Sappire/Emerald/Granite ?

I was going to submit a patch, but there are other inconsistencies too;
the Cores and Atoms are mostly the opposite ways around.

Atoms uses the microarchitecture codename with SoC name in the comment.

Cores (starting with SKL where they begin to diverge) use the CPU
codname with the microarchitecture name(s), which matter for hybrid as
there are multiple uarches.

I'd argue that this wants to become INTEL_DIAMONDRAPIDS_X (and CMT/DMT
to SRF/CWF), or all of SPR/EMR/GNR want to turn into GLC/RPC/RWC so
they're consistent when used together.

Thanks,

~Andrew

RE: [PATCH] x86/cpu: Add two Intel CPU model numbers

Posted by Luck, Tony 12 months ago

>> +/* Family 19 */ +#define INTEL_PANTHERCOVE_X IFM(19, 0x01) /* Diamond
>> Rapids */
>
> Is it intentional that this is not INTEL_DIAMONDRAPIDS_X like
> Sapphire/Emerald/Granite ?

Andrew,

PeterZ wants to name based on core, not SoC (at least for mono-core CPUs ... this
doesn't work for hybrid).  Argue with him.

-Tony

Re: [PATCH] x86/cpu: Add two Intel CPU model numbers

Posted by Peter Zijlstra 11 months, 4 weeks ago

On Wed, Feb 12, 2025 at 04:09:11PM +0000, Luck, Tony wrote:
> >> +/* Family 19 */ +#define INTEL_PANTHERCOVE_X IFM(19, 0x01) /* Diamond
> >> Rapids */
> >
> > Is it intentional that this is not INTEL_DIAMONDRAPIDS_X like
> > Sapphire/Emerald/Granite ?
> 
> Andrew,
> 
> PeterZ wants to name based on core, not SoC (at least for mono-core CPUs ... this
> doesn't work for hybrid).  Argue with him.

Argh :-)

So yeah, its a trainwreck.

We used to use uarch, and that worked until skylake.

I'm not sure what exactly we continued as, but Kaby Lake was a Skylake
uarch.

The Atoms are uarch and still are, they weren't messed up.

But if you want to do DMR as PANTERCOVE then SPR should've been
GOLDENCOVE and we didn't do that either.


Also, since DMR is the direct continuation of GRANITERAPIDS, it should
also come below it.

Therefore, I'll concur with Andy that this is all highly irregular and
would propose we do the below.

Isn't the only reason we're doing a new Family because we can out of
module number space? It's not magically different from Fam6.

---
diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h
index 8359113e3e58..bf431dd4e271 100644
--- a/arch/x86/include/asm/intel-family.h
+++ b/arch/x86/include/asm/intel-family.h
@@ -115,6 +115,8 @@
 #define INTEL_GRANITERAPIDS_X		IFM(6, 0xAD)
 #define INTEL_GRANITERAPIDS_D		IFM(6, 0xAE)
 
+#define INTEL_DIAMONDRAPIDS_X		IFM(19, 0x01) /* Panther Cove */
+
 /* "Hybrid" Processors (P-Core/E-Core) */
 
 #define INTEL_LAKEFIELD			IFM(6, 0x8A) /* Sunny Cove / Tremont */
@@ -179,9 +181,6 @@
 /* Family 5 */
 #define INTEL_QUARK_X1000		IFM(5, 0x09) /* Quark X1000 SoC */
 
-/* Family 19 */
-#define INTEL_PANTHERCOVE_X		IFM(19, 0x01) /* Diamond Rapids */
-
 /* CPU core types */
 enum intel_cpu_type {
 	INTEL_CPU_TYPE_ATOM = 0x20,
diff --git a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
index dbcd3087aaa4..d4f1e222cdfc 100644
--- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
+++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
@@ -810,7 +810,7 @@ static const struct x86_cpu_id isst_cpu_ids[] = {
 	X86_MATCH_VFM(INTEL_GRANITERAPIDS_X,	SST_HPM_SUPPORTED),
 	X86_MATCH_VFM(INTEL_ICELAKE_D,		0),
 	X86_MATCH_VFM(INTEL_ICELAKE_X,		0),
-	X86_MATCH_VFM(INTEL_PANTHERCOVE_X,	SST_HPM_SUPPORTED),
+	X86_MATCH_VFM(INTEL_DIAMONDRAPIDS_X,	SST_HPM_SUPPORTED),
 	X86_MATCH_VFM(INTEL_SAPPHIRERAPIDS_X,	0),
 	X86_MATCH_VFM(INTEL_SKYLAKE_X,		SST_MBOX_SUPPORTED),
 	{}
diff --git a/drivers/platform/x86/intel/tpmi_power_domains.c b/drivers/platform/x86/intel/tpmi_power_domains.c
index 2f01cd22a6ee..0b3092c264f3 100644
--- a/drivers/platform/x86/intel/tpmi_power_domains.c
+++ b/drivers/platform/x86/intel/tpmi_power_domains.c
@@ -83,7 +83,7 @@ static const struct x86_cpu_id tpmi_cpu_ids[] = {
 	X86_MATCH_VFM(INTEL_ATOM_CRESTMONT,	NULL),
 	X86_MATCH_VFM(INTEL_ATOM_DARKMONT_X,	NULL),
 	X86_MATCH_VFM(INTEL_GRANITERAPIDS_D,	NULL),
-	X86_MATCH_VFM(INTEL_PANTHERCOVE_X,	NULL),
+	X86_MATCH_VFM(INTEL_DIAMONDRAPIDS_X,	NULL),
 	{}
 };
 MODULE_DEVICE_TABLE(x86cpu, tpmi_cpu_ids);

Re: [PATCH] x86/cpu: Add two Intel CPU model numbers

Posted by Ingo Molnar 9 months, 3 weeks ago

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, Feb 12, 2025 at 04:09:11PM +0000, Luck, Tony wrote:
> > >> +/* Family 19 */ +#define INTEL_PANTHERCOVE_X IFM(19, 0x01) /* Diamond
> > >> Rapids */
> > >
> > > Is it intentional that this is not INTEL_DIAMONDRAPIDS_X like
> > > Sapphire/Emerald/Granite ?
> > 
> > Andrew,
> > 
> > PeterZ wants to name based on core, not SoC (at least for mono-core CPUs ... this
> > doesn't work for hybrid).  Argue with him.
> 
> Argh :-)
> 
> So yeah, its a trainwreck.
> 
> We used to use uarch, and that worked until skylake.
> 
> I'm not sure what exactly we continued as, but Kaby Lake was a Skylake
> uarch.
> 
> The Atoms are uarch and still are, they weren't messed up.
> 
> But if you want to do DMR as PANTERCOVE then SPR should've been
> GOLDENCOVE and we didn't do that either.
> 
> 
> Also, since DMR is the direct continuation of GRANITERAPIDS, it should
> also come below it.
> 
> Therefore, I'll concur with Andy that this is all highly irregular and
> would propose we do the below.
> 
> Isn't the only reason we're doing a new Family because we can out of
> module number space? It's not magically different from Fam6.

Mind sending this with a changelog, or at least a SOB? :)

Thanks,

	Ingo

Re: [PATCH] x86/cpu: Add two Intel CPU model numbers

Posted by Dave Hansen 11 months, 4 weeks ago

On 2/14/25 05:02, Peter Zijlstra wrote:
> Isn't the only reason we're doing a new Family because we can out of
> module number space? It's not magically different from Fam6.

Right. That was the primary motivation.

But the new scheme should also make a little more logical sense. The
family numbers are supposed to move up at a steady rate and also
separate desktop and server.

Or maybe we'll realize we miss all the fun of family 6 and just start
shoving everything as models in family 19 randomly instead. ;)

RE: [PATCH] x86/cpu: Add two Intel CPU model numbers

Posted by Luck, Tony 11 months, 3 weeks ago

> > Isn't the only reason we're doing a new Family because we can out of
> > module number space? It's not magically different from Fam6.
>
> Right. That was the primary motivation.
>
> But the new scheme should also make a little more logical sense. The
> family numbers are supposed to move up at a steady rate and also
> separate desktop and server.

Nope. There isn't a commitment to put desktop/server into different families.

There's an upcoming patch for a CPU in family 18, when I post it I'll
add a big fat comment telling people not to extrapolate from two
data points with one CPU in each of family 18 and 19.

It doesn't seem like there would be any use case for that. I don't
envision code that looks like:

	if (family == 18) {
		// modern desktop/mobile stuff
	} else if (family == 19) {
		// modern Xeon stuff
	} else {
		// Legacy
	}

I'm fine with Peter's patch to move Diamond Rapids up just after Granite Rapids.
That will help make it clear that family numbers are no longer important. Just
a way to extend the "model" space beyond 8 bits.

> Or maybe we'll realize we miss all the fun of family 6 and just start
> shoving everything as models in family 19 randomly instead. ;)

After the early models in family 6 where Linux tried to check ranges of models,
it became clear that X86_FEATURE bits from other CPUID leaves was the right
way to do such checks. It shouldn't matter at all if new CPUs come out randomly
assigned to different families.

-Tony