Re: [tip: x86/urgent] x86/cpu: Add CPU model number for Bartlett Lake CPUs with Raptor Cove cores

Re: [tip: x86/urgent] x86/cpu: Add CPU model number for Bartlett Lake CPUs with Raptor Cove cores

[Christian Ludloff]

> +#define INTEL_RAPTORCOVE IFM(6, 0xD7) /* Bartlett Lake */

Please fix this. It has the core and the product reversed. That is, it
should be INTEL_BARTLETTLAKE and /* Raptor Cove */ to match
the bulk of that file.

And yes, you also want to fix this for INTEL_PANTHERCOVE_X
and /* Diamond Rapids */ entry.

The macros refer to products.
The comments refer to cores.

Consistency, please.
Sanity, please.

--
Christian

[PATCH -v2] x86/cpu: Add CPU model number for Bartlett Lake CPUs with Raptor Cove cores

[Ingo Molnar]

* Christian Ludloff <ludloff@gmail.com> wrote:

> > +#define INTEL_RAPTORCOVE IFM(6, 0xD7) /* Bartlett Lake */
> 
> Please fix this. It has the core and the product reversed. That is, it
> should be INTEL_BARTLETTLAKE and /* Raptor Cove */ to match
> the bulk of that file.

I switched it around for this commit - see the updated patch below.

> And yes, you also want to fix this for INTEL_PANTHERCOVE_X
> and /* Diamond Rapids */ entry.
> 
> The macros refer to products.
> The comments refer to cores.

Please send a patch if you have the time.

> Consistency, please.
> Sanity, please.

Amen!

Thanks,

	Ingo

==================>
From: Pi Xiange <xiange.pi@intel.com>
Date: Mon, 14 Apr 2025 11:28:39 +0800
Subject: [PATCH] x86/cpu: Add CPU model number for Bartlett Lake CPUs with Raptor Cove cores

Bartlett Lake has a P-core only product with Raptor Cove.

[ mingo: Switch around the define as pointed out by Christian Ludloff:
         Ratpr Cove is the core, Bartlett Lake is the product.

Signed-off-by: Pi Xiange <xiange.pi@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Christian Ludloff <ludloff@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: "Ahmed S. Darwish" <darwi@linutronix.de>
Cc: x86-cpuid@lists.linux.dev
Link: https://lore.kernel.org/r/20250414032839.5368-1-xiange.pi@intel.com
---
 arch/x86/include/asm/intel-family.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/include/asm/intel-family.h b/arch/x86/include/asm/intel-family.h
index 3a97a7eefb51..be10c188614f 100644
--- a/arch/x86/include/asm/intel-family.h
+++ b/arch/x86/include/asm/intel-family.h
@@ -126,6 +126,8 @@
 #define INTEL_GRANITERAPIDS_X		IFM(6, 0xAD) /* Redwood Cove */
 #define INTEL_GRANITERAPIDS_D		IFM(6, 0xAE)
 
+#define INTEL_BARTLETTLAKE		IFM(6, 0xD7) /* Raptor Cove */
+
 /* "Hybrid" Processors (P-Core/E-Core) */
 
 #define INTEL_LAKEFIELD			IFM(6, 0x8A) /* Sunny Cove / Tremont */

RE: [PATCH -v2] x86/cpu: Add CPU model number for Bartlett Lake CPUs with Raptor Cove cores

[Luck, Tony]

> > Please fix this. It has the core and the product reversed. That is, it
> > should be INTEL_BARTLETTLAKE and /* Raptor Cove */ to match
> > the bulk of that file.
>
> I switched it around for this commit - see the updated patch below.
>
> > And yes, you also want to fix this for INTEL_PANTHERCOVE_X
> > and /* Diamond Rapids */ entry.
> >
> > The macros refer to products.
> > The comments refer to cores.
>
> Please send a patch if you have the time.
>
> > Consistency, please.
> > Sanity, please.
>
> Amen!

PeterZ has been very vocal that he wants the "sane" way to be making the "#define"
name be based on the core rather than the product.  That way multiple products using
the same core show up together in switch statements for model specific features like
power and performance counters.

This does mean we have a transition between legacy names that were using the
SoC product codename and modern ones that use the core codename.

Can the X86 maintainers please get in a huddle and define a naming
policy. This discussion keeps happening.

-Tony

Re: [PATCH -v2] x86/cpu: Add CPU model number for Bartlett Lake CPUs with Raptor Cove cores

[Christian Ludloff]

On Mon, Apr 14, 2025 at 6:21 PM Luck, Tony <tony.luck@intel.com> wrote:
> > > The macros refer to products.
> > > The comments refer to cores.
> >
> > > Consistency, please.
> > > Sanity, please.
> >
> > Amen!
>
> PeterZ has been very vocal that he wants the "sane" way to be making the "#define"
> name be based on the core rather than the product.  That way multiple products using
> the same core show up together in switch statements for model specific features like
> power and performance counters.
>
> This does mean we have a transition between legacy names that were using the
> SoC product codename and modern ones that use the core codename.
>
> Can the X86 maintainers please get in a huddle and define a naming
> policy. This discussion keeps happening.

Consider a two-level abstraction.

One which gets "looked up" from FMS – fundamentally
that is what the existing file is trying to achieve, right?

Another which gets "looked up" from core (e.g. RPC) or
product (e.g. RPL,BTL) or whatever-else-is-used-often.

--
Christian

Re: [PATCH -v2] x86/cpu: Add CPU model number for Bartlett Lake CPUs with Raptor Cove cores

[Christian Ludloff]

On Mon, Apr 14, 2025 at 5:23 PM Ingo Molnar <mingo@kernel.org> wrote:
> * Christian Ludloff <ludloff@gmail.com> wrote:
> > > +#define INTEL_RAPTORCOVE IFM(6, 0xD7) /* Bartlett Lake */
> >
> > Please fix this. It has the core and the product reversed. That is, it
> > should be INTEL_BARTLETTLAKE and /* Raptor Cove */ to match
> > the bulk of that file.
>
> I switched it around for this commit - see the updated patch below.

Thanks.

> > And yes, you also want to fix this for INTEL_PANTHERCOVE_X
> > and /* Diamond Rapids */ entry.
>
> Please send a patch if you have the time.

Peter had proposed...

https://lore.kernel.org/lkml/20250214130205.GK14028@noisy.programming.kicks-ass.net/

--
Christian