diff mbox series

[1/9] x86/cpu/topology: Add x86_cpu_type to struct cpuinfo_topology

Message ID 20240617-add-cpu-type-v1-1-b88998c01e76@linux.intel.com
State New
Headers show
Series [1/9] x86/cpu/topology: Add x86_cpu_type to struct cpuinfo_topology | expand

Commit Message

Pawan Gupta June 17, 2024, 9:11 a.m. UTC
Sometimes it is required to identify the type of a core for taking specific
actions e.g. intel_pstate driver uses the core type to determine CPU
scaling. Also, some CPU vulnerabilities only affect a specific CPU type
e.g. RFDS only affects Intel Atom. For hybrid systems that have variants
P+E, P-only(Core) and E-only(Atom), it gets challenging to identify which
variant is vulnerable to a specific vulnerability, as these variants share
the same family, model and stepping.

Such processors do have CPUID fields that uniquely identify them. Like,
P+E, P-only and E-only enumerates CPUID.1A.CORE_TYPE, while P+E
additionally enumerates CPUID.7.HYBRID. Linux does not currently use this
field.

Add a new field cpu_type to struct cpuinfo_topology which can be used to
match a CPU based on its type.

The cpu_type is populated in the below debugfs file:

  # cat /sys/kernel/debug/x86/topo/cpus/N

Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
 arch/x86/include/asm/processor.h      | 3 +++
 arch/x86/include/asm/topology.h       | 9 +++++++++
 arch/x86/kernel/cpu/debugfs.c         | 1 +
 arch/x86/kernel/cpu/topology_common.c | 9 +++++++++
 4 files changed, 22 insertions(+)

Comments

Borislav Petkov June 18, 2024, 9:28 p.m. UTC | #1
On Mon, Jun 17, 2024 at 02:11:26AM -0700, Pawan Gupta wrote:
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -95,6 +95,9 @@ struct cpuinfo_topology {
>  	// Core ID relative to the package
>  	u32			core_id;
>  
> +	// CPU-type e.g. performance, efficiency etc.
> +	u8			cpu_type;
> +
>  	// Logical ID mappings
>  	u32			logical_pkg_id;
>  	u32			logical_die_id;
> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> index abe3a8f22cbd..b28ad9422afb 100644
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -41,6 +41,14 @@
>  /* Mappings between logical cpu number and node number */
>  DECLARE_EARLY_PER_CPU(int, x86_cpu_to_node_map);
>  
> +#define X86_CPU_TYPE_INTEL_SHIFT	24
> +
> +enum x86_topo_cpu_type {
> +	X86_CPU_TYPE_UNKNOWN		= 0,
> +	X86_CPU_TYPE_INTEL_ATOM		= 0x20,
> +	X86_CPU_TYPE_INTEL_CORE		= 0x40,

Can we unify those core types and do our own (our == Linux) defines instead of
using Intels or AMDs?

There will be AMD variants too soon:

https://lore.kernel.org/r/7aad57a98b37fa5893d4fe602d3dcef5c3f755d5.1718606975.git.perry.yuan@amd.com

so can we have generic defines like

PERF_CORE
EFF_CORE
bla_CORE

and so on

?

And then map each vendor's types to the Linux types?

Thx.
Mario Limonciello June 18, 2024, 9:33 p.m. UTC | #2
On 6/17/2024 04:11, Pawan Gupta wrote:
> Sometimes it is required to identify the type of a core for taking specific
> actions e.g. intel_pstate driver uses the core type to determine CPU
> scaling. Also, some CPU vulnerabilities only affect a specific CPU type
> e.g. RFDS only affects Intel Atom. For hybrid systems that have variants
> P+E, P-only(Core) and E-only(Atom), it gets challenging to identify which
> variant is vulnerable to a specific vulnerability, as these variants share
> the same family, model and stepping.
> 
> Such processors do have CPUID fields that uniquely identify them. Like,
> P+E, P-only and E-only enumerates CPUID.1A.CORE_TYPE, while P+E
> additionally enumerates CPUID.7.HYBRID. Linux does not currently use this
> field.
> 
> Add a new field cpu_type to struct cpuinfo_topology which can be used to
> match a CPU based on its type.
> 
> The cpu_type is populated in the below debugfs file:
> 
>    # cat /sys/kernel/debug/x86/topo/cpus/N
> 
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> ---
>   arch/x86/include/asm/processor.h      | 3 +++
>   arch/x86/include/asm/topology.h       | 9 +++++++++
>   arch/x86/kernel/cpu/debugfs.c         | 1 +
>   arch/x86/kernel/cpu/topology_common.c | 9 +++++++++
>   4 files changed, 22 insertions(+)
> 
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index cb4f6c513c48..f310a7fb4e00 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -95,6 +95,9 @@ struct cpuinfo_topology {
>   	// Core ID relative to the package
>   	u32			core_id;
>   
> +	// CPU-type e.g. performance, efficiency etc.
> +	u8			cpu_type;
> +
>   	// Logical ID mappings
>   	u32			logical_pkg_id;
>   	u32			logical_die_id;
> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> index abe3a8f22cbd..b28ad9422afb 100644
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -41,6 +41,14 @@
>   /* Mappings between logical cpu number and node number */
>   DECLARE_EARLY_PER_CPU(int, x86_cpu_to_node_map);
>   
> +#define X86_CPU_TYPE_INTEL_SHIFT	24
> +
> +enum x86_topo_cpu_type {
> +	X86_CPU_TYPE_UNKNOWN		= 0,
> +	X86_CPU_TYPE_INTEL_ATOM		= 0x20,
> +	X86_CPU_TYPE_INTEL_CORE		= 0x40,
> +};
> +

There is another patch series [1] in flight right now that is trying to 
get CPU types for AMD hetero designs as well.

I think the way you're doing it is a bit better though.

What do you think about having a common enum rather with words that are 
marketing strings?  We could have a monotonically increasing enum then 
too instead of a random bit mask.

Something like:

enum x86_topo_cpu_type {
	X86_CPU_TYPE_UNKNOWN		= 0,
	X86_CPU_TYPE_PERFORMANCE	= 1,
	X86_CPU_TYPE_EFFICIENCY		= 2,
};

Then as additional core types are added for any vendor those can be 
added here with a description of what they are.

The logic for topo_set_cpu_type() below can map out any bit mask and 
shift from a lookup path into this standard enum then.

If you do it this way we can lump the AMD hetero look up stuff into 
topo_set_cpu_type() as another case.

[1] 
https://lore.kernel.org/linux-pm/cover.1718606975.git.perry.yuan@amd.com/T/#mc65457f33331bba0d7d4fef1907d2fef14fc3fd8

>   #ifdef CONFIG_DEBUG_PER_CPU_MAPS
>   /*
>    * override generic percpu implementation of cpu_to_node
> @@ -139,6 +147,7 @@ extern const struct cpumask *cpu_clustergroup_mask(int cpu);
>   #define topology_logical_die_id(cpu)		(cpu_data(cpu).topo.logical_die_id)
>   #define topology_die_id(cpu)			(cpu_data(cpu).topo.die_id)
>   #define topology_core_id(cpu)			(cpu_data(cpu).topo.core_id)
> +#define topology_cpu_type(cpu)			(cpu_data(cpu).topo.cpu_type)
>   #define topology_ppin(cpu)			(cpu_data(cpu).ppin)
>   
>   #define topology_amd_node_id(cpu)		(cpu_data(cpu).topo.amd_node_id)
> diff --git a/arch/x86/kernel/cpu/debugfs.c b/arch/x86/kernel/cpu/debugfs.c
> index 3baf3e435834..b1c9bafe6c39 100644
> --- a/arch/x86/kernel/cpu/debugfs.c
> +++ b/arch/x86/kernel/cpu/debugfs.c
> @@ -22,6 +22,7 @@ static int cpu_debug_show(struct seq_file *m, void *p)
>   	seq_printf(m, "die_id:              %u\n", c->topo.die_id);
>   	seq_printf(m, "cu_id:               %u\n", c->topo.cu_id);
>   	seq_printf(m, "core_id:             %u\n", c->topo.core_id);
> +	seq_printf(m, "cpu_type:            %x\n", c->topo.cpu_type);
>   	seq_printf(m, "logical_pkg_id:      %u\n", c->topo.logical_pkg_id);
>   	seq_printf(m, "logical_die_id:      %u\n", c->topo.logical_die_id);
>   	seq_printf(m, "llc_id:              %u\n", c->topo.llc_id);
> diff --git a/arch/x86/kernel/cpu/topology_common.c b/arch/x86/kernel/cpu/topology_common.c
> index 9a6069e7133c..be82c8769bb2 100644
> --- a/arch/x86/kernel/cpu/topology_common.c
> +++ b/arch/x86/kernel/cpu/topology_common.c
> @@ -140,6 +140,14 @@ static void parse_topology(struct topo_scan *tscan, bool early)
>   	}
>   }
>   
> +static void topo_set_cpu_type(struct cpuinfo_x86 *c)
> +{
> +	c->topo.cpu_type = X86_CPU_TYPE_UNKNOWN;
> +
> +	if (c->x86_vendor == X86_VENDOR_INTEL && cpuid_eax(0) >= 0x1a)
> +		c->topo.cpu_type = cpuid_eax(0x1a) >> X86_CPU_TYPE_INTEL_SHIFT;
> +}
> +
>   static void topo_set_ids(struct topo_scan *tscan, bool early)
>   {
>   	struct cpuinfo_x86 *c = tscan->c;
> @@ -190,6 +198,7 @@ void cpu_parse_topology(struct cpuinfo_x86 *c)
>   	}
>   
>   	topo_set_ids(&tscan, false);
> +	topo_set_cpu_type(c);
>   }
>   
>   void __init cpu_init_topology(struct cpuinfo_x86 *c)
>
Dave Hansen June 18, 2024, 10:03 p.m. UTC | #3
On 6/18/24 14:33, Mario Limonciello wrote:
>> +enum x86_topo_cpu_type {
>> +    X86_CPU_TYPE_UNKNOWN        = 0,
>> +    X86_CPU_TYPE_INTEL_ATOM        = 0x20,
>> +    X86_CPU_TYPE_INTEL_CORE        = 0x40,
>> +};
>> +
...
> What do you think about having a common enum rather with words that are
> marketing strings? 

They're not really marketing strings.  They really are architectural and
have specific functional meaning just like ->x86_model and ->x86_family.

For instance, we are effectively doing this today:

	if (c->cpu_x86_vfm == INTEL_ALDER_LAKE &&
	    c->cpu_type == X86_CPU_TYPE_INTEL_ATOM)
		setup_force_cpu_bug(FOO);

That check is truly specific to the core being an "ATOM" and not being
an efficient core.  There might be a future non-Atom efficient CPU or a
future non-Core performance CPU.

We very well might have a use in the future to tag a processor as
"efficient" or "performance" in a vendor-neutral manner.  That mechanism
could very well be derived from ->cpu_type.  But I think that's actually
independent from Pawan's series.
Pawan Gupta June 19, 2024, 3:31 a.m. UTC | #4
On Tue, Jun 18, 2024 at 11:28:01PM +0200, Borislav Petkov wrote:
> On Mon, Jun 17, 2024 at 02:11:26AM -0700, Pawan Gupta wrote:
> > --- a/arch/x86/include/asm/processor.h
> > +++ b/arch/x86/include/asm/processor.h
> > @@ -95,6 +95,9 @@ struct cpuinfo_topology {
> >  	// Core ID relative to the package
> >  	u32			core_id;
> >  
> > +	// CPU-type e.g. performance, efficiency etc.
> > +	u8			cpu_type;
> > +
> >  	// Logical ID mappings
> >  	u32			logical_pkg_id;
> >  	u32			logical_die_id;
> > diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> > index abe3a8f22cbd..b28ad9422afb 100644
> > --- a/arch/x86/include/asm/topology.h
> > +++ b/arch/x86/include/asm/topology.h
> > @@ -41,6 +41,14 @@
> >  /* Mappings between logical cpu number and node number */
> >  DECLARE_EARLY_PER_CPU(int, x86_cpu_to_node_map);
> >  
> > +#define X86_CPU_TYPE_INTEL_SHIFT	24
> > +
> > +enum x86_topo_cpu_type {
> > +	X86_CPU_TYPE_UNKNOWN		= 0,
> > +	X86_CPU_TYPE_INTEL_ATOM		= 0x20,
> > +	X86_CPU_TYPE_INTEL_CORE		= 0x40,
> 
> Can we unify those core types and do our own (our == Linux) defines instead of
> using Intels or AMDs?

As Dave pointed out in the other email, atleast mitigations have a use case
to match the raw CPU type defined by the vendor. It could get tricky if
ever there will be different types of performance cores, with different
hardware characteristics.

> There will be AMD variants too soon:
> 
> https://lore.kernel.org/r/7aad57a98b37fa5893d4fe602d3dcef5c3f755d5.1718606975.git.perry.yuan@amd.com
> 
> so can we have generic defines like
> 
> PERF_CORE
> EFF_CORE
> bla_CORE
> 
> and so on
> 
> ?
> 
> And then map each vendor's types to the Linux types?

I am no expert, but I do think generic Linux types could also be useful in
future. Hypothetically speaking, these can be used to make better
scheduling decisions. For example, CPU bound processes can be scheduled
more on performance cores and I/O bound on efficiency cores.

To accommodate for that we can name the vendor specific types in this
series as vendor_cpu_type. And if we ever need to add generic types, we can
call them cpu_type?
Pawan Gupta June 21, 2024, 6:36 a.m. UTC | #5
On Thu, Jun 20, 2024 at 05:51:11PM +0200, Borislav Petkov wrote:
> On Tue, Jun 18, 2024 at 08:31:26PM -0700, Pawan Gupta wrote:
> > To accommodate for that we can name the vendor specific types in this
> > series as vendor_cpu_type. And if we ever need to add generic types, we can
> > call them cpu_type?
> 
> Yes, then that member you're adding should not just be called "cpu_type" but
> either "vendor_cpu_type" or "hw_cpu_type" or whatnot and every vendor can then
> put its own values there.

I like hw_cpu_type better, as it leaves room for architecture defined
types as well.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index cb4f6c513c48..f310a7fb4e00 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -95,6 +95,9 @@  struct cpuinfo_topology {
 	// Core ID relative to the package
 	u32			core_id;
 
+	// CPU-type e.g. performance, efficiency etc.
+	u8			cpu_type;
+
 	// Logical ID mappings
 	u32			logical_pkg_id;
 	u32			logical_die_id;
diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index abe3a8f22cbd..b28ad9422afb 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -41,6 +41,14 @@ 
 /* Mappings between logical cpu number and node number */
 DECLARE_EARLY_PER_CPU(int, x86_cpu_to_node_map);
 
+#define X86_CPU_TYPE_INTEL_SHIFT	24
+
+enum x86_topo_cpu_type {
+	X86_CPU_TYPE_UNKNOWN		= 0,
+	X86_CPU_TYPE_INTEL_ATOM		= 0x20,
+	X86_CPU_TYPE_INTEL_CORE		= 0x40,
+};
+
 #ifdef CONFIG_DEBUG_PER_CPU_MAPS
 /*
  * override generic percpu implementation of cpu_to_node
@@ -139,6 +147,7 @@  extern const struct cpumask *cpu_clustergroup_mask(int cpu);
 #define topology_logical_die_id(cpu)		(cpu_data(cpu).topo.logical_die_id)
 #define topology_die_id(cpu)			(cpu_data(cpu).topo.die_id)
 #define topology_core_id(cpu)			(cpu_data(cpu).topo.core_id)
+#define topology_cpu_type(cpu)			(cpu_data(cpu).topo.cpu_type)
 #define topology_ppin(cpu)			(cpu_data(cpu).ppin)
 
 #define topology_amd_node_id(cpu)		(cpu_data(cpu).topo.amd_node_id)
diff --git a/arch/x86/kernel/cpu/debugfs.c b/arch/x86/kernel/cpu/debugfs.c
index 3baf3e435834..b1c9bafe6c39 100644
--- a/arch/x86/kernel/cpu/debugfs.c
+++ b/arch/x86/kernel/cpu/debugfs.c
@@ -22,6 +22,7 @@  static int cpu_debug_show(struct seq_file *m, void *p)
 	seq_printf(m, "die_id:              %u\n", c->topo.die_id);
 	seq_printf(m, "cu_id:               %u\n", c->topo.cu_id);
 	seq_printf(m, "core_id:             %u\n", c->topo.core_id);
+	seq_printf(m, "cpu_type:            %x\n", c->topo.cpu_type);
 	seq_printf(m, "logical_pkg_id:      %u\n", c->topo.logical_pkg_id);
 	seq_printf(m, "logical_die_id:      %u\n", c->topo.logical_die_id);
 	seq_printf(m, "llc_id:              %u\n", c->topo.llc_id);
diff --git a/arch/x86/kernel/cpu/topology_common.c b/arch/x86/kernel/cpu/topology_common.c
index 9a6069e7133c..be82c8769bb2 100644
--- a/arch/x86/kernel/cpu/topology_common.c
+++ b/arch/x86/kernel/cpu/topology_common.c
@@ -140,6 +140,14 @@  static void parse_topology(struct topo_scan *tscan, bool early)
 	}
 }
 
+static void topo_set_cpu_type(struct cpuinfo_x86 *c)
+{
+	c->topo.cpu_type = X86_CPU_TYPE_UNKNOWN;
+
+	if (c->x86_vendor == X86_VENDOR_INTEL && cpuid_eax(0) >= 0x1a)
+		c->topo.cpu_type = cpuid_eax(0x1a) >> X86_CPU_TYPE_INTEL_SHIFT;
+}
+
 static void topo_set_ids(struct topo_scan *tscan, bool early)
 {
 	struct cpuinfo_x86 *c = tscan->c;
@@ -190,6 +198,7 @@  void cpu_parse_topology(struct cpuinfo_x86 *c)
 	}
 
 	topo_set_ids(&tscan, false);
+	topo_set_cpu_type(c);
 }
 
 void __init cpu_init_topology(struct cpuinfo_x86 *c)