diff mbox series

[v2,1/9] x86/cpu/topology: Add CPU type to struct cpuinfo_topology

Message ID 20240627-add-cpu-type-v2-1-f927bde83ad0@linux.intel.com
State New
Headers show
Series [v2,1/9] x86/cpu/topology: Add CPU type to struct cpuinfo_topology | expand

Commit Message

Pawan Gupta June 27, 2024, 8:44 p.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 hw_cpu_type to struct cpuinfo_topology which can be used to
match a CPU based on its type.

The hw_cpu_type is populated in the below debugfs file:

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

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 July 3, 2024, 11:07 p.m. UTC | #1
On Fri, Jun 28, 2024 at 10:32:09AM -0700, Pawan Gupta wrote:
> On Fri, Jun 28, 2024 at 10:03:05AM +0200, Borislav Petkov wrote:
> > On Thu, Jun 27, 2024 at 01:44:06PM -0700, Pawan Gupta wrote:
> > > The hw_cpu_type is populated in the below debugfs file:
> > > 
> > >   # cat /sys/kernel/debug/x86/topo/cpus/#
> > 
> > What "below debugfs file"? A '#'?
> 
> That is the number of the CPU. If it is causing confusion, I can will
> change it to N, or say # means the number of the CPU.

Or drop that sentence completely.

For some reason lately it seems to me folks feel the need to explain what the
patch does. Even if that is visible from the diff.

> I thought about that, but the other fields are also printed without a
> preceding "0x":
> 
>           seq_printf(m, "initial_apicid:      %x\n", c->topo.initial_apicid);
>           seq_printf(m, "apicid:              %x\n", c->topo.apicid);
> 	  ...
> 
> I will change those too, probably in a separate patch.

Yes please.

> > > +static void topo_set_hw_cpu_type(struct cpuinfo_x86 *c)
> > > +{
> > > +	c->topo.hw_cpu_type = X86_HW_CPU_TYPE_UNKNOWN;
> > > +
> > > +	if (c->x86_vendor == X86_VENDOR_INTEL && c->cpuid_level >= 0x1a)
> > > +		c->topo.hw_cpu_type = cpuid_eax(0x1a) >> X86_CPU_TYPE_INTEL_SHIFT;
> > > +}
> > 
> > Why isn't this happening in cpu/intel.c? And then you don't need yet
> > another silly function.
> 
> I was preferring to keep the topology related code in one place. Would it
> make sense to keep it in Intel specific leg in parse_topology() as below:

I guess.

Although looking around this code, I wonder why is this hw_cpu_type part of
the topology and not part of cpuinfo_x86 directly?

struct cpuinfo_topology has all the IDs but the type? Feels out of place
there.
Borislav Petkov July 9, 2024, 12:45 p.m. UTC | #2
On Mon, Jul 08, 2024 at 06:24:18PM -0700, Pawan Gupta wrote:
> The draft version had it in cpuinfo_x86, but it later got moved to
> cpuinfo_topology as it appears to be topology related. Below is from AMD
> documentation:
> 
>   E.4.24 Function 8000_0026—Extended CPU Topology

That's the name of the CPUID leaf.

>   CPUID Fn8000_0026 reports extended topology information for logical
>   processors, including asymmetric and heterogenous topology descriptions.
>   Individual logical processors may report different values in systems with
>   asynchronous and heterogeneous topologies.
>   https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24594.pdf
> 
> I can move it back to cpuinfo_x86 if you feel strongly about it.

Nah, it just felt weird. Not a biggie and we can always move it if we deem so.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index cb4f6c513c48..d8d715fcc25c 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -105,6 +105,9 @@  struct cpuinfo_topology {
 	// Cache level topology IDs
 	u32			llc_id;
 	u32			l2c_id;
+
+	// Hardware defined CPU-type
+	u8			hw_cpu_type;
 };
 
 struct cpuinfo_x86 {
diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index abe3a8f22cbd..717fdb928dc3 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -33,6 +33,14 @@ 
 #include <linux/numa.h>
 #include <linux/cpumask.h>
 
+#define X86_CPU_TYPE_INTEL_SHIFT	24
+
+enum x86_hw_topo_cpu_type {
+	X86_HW_CPU_TYPE_UNKNOWN		= 0,
+	X86_HW_CPU_TYPE_INTEL_ATOM	= 0x20,
+	X86_HW_CPU_TYPE_INTEL_CORE	= 0x40,
+};
+
 #ifdef CONFIG_NUMA
 
 #include <asm/mpspec.h>
@@ -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_hw_cpu_type(cpu)		(cpu_data(cpu).topo.hw_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..8082e03a5976 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, "hw_cpu_type:         %x\n", c->topo.hw_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..8b47bd6b0623 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_hw_cpu_type(struct cpuinfo_x86 *c)
+{
+	c->topo.hw_cpu_type = X86_HW_CPU_TYPE_UNKNOWN;
+
+	if (c->x86_vendor == X86_VENDOR_INTEL && c->cpuid_level >= 0x1a)
+		c->topo.hw_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_hw_cpu_type(c);
 }
 
 void __init cpu_init_topology(struct cpuinfo_x86 *c)