[PATCH v2 0/9] Add CPU-type to topology

[PATCH v2 0/9] Add CPU-type to topology

[Pawan Gupta]

v2:
- Move CPU-type to the end of the CPU topology structure (Andrew).
- Use c->cpuid_level instead of cpuid_eax(0) (Andrew).
- Move CPU-type enum out of ifdef CONFIG_NUMA (kernel test robot).
- Rename cpu_type to hw_cpu_type (Borislav).
- Explain replacing get_this_hybrid_cpu_type() with topology_hw_cpu_type()
  in the commit message (Dave).
- Fix the alignment in cpu_vuln_whitelist (Andrew).
- Add the obj compare note in the commit message (Dave/Tony).
- s/X86_CPU_TYPE_INTEL_ATOM/ATOM/ in cpu_vuln_whitelist (Dave).

v1: https://lore.kernel.org/r/20240617-add-cpu-type-v1-0-b88998c01e76@linux.intel.com

Hi,

This series adds support for CPU-type (CPUID.1A.EAX[31-24] on Intel) to
differentiate between hybrid variants P+E, P-only, E-only that share the
same Family/Model/Stepping. One of the use case for CPU-type is the
affected CPU table for CPU vulnerabilities, which can now use the CPU-type
to filter the unaffected variants.

* Patch 1 adds hardware cpu-type to CPU topology structure and introduces
  topology_hw_cpu_type().

* Patch 2-4 replaces usages of get_this_hybrid_cpu_type() with
  topology_hw_cpu_type().

* Patch 5-7 Updates CPU-matching infrastructure to use CPU-type.

* Patch 8 cleans up the affected CPU list.

* Patch 9 uses the CPU-type to exclude P-only parts from the RFDS affected
  list.

Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
Pawan Gupta (9):
      x86/cpu/topology: Add CPU type to struct cpuinfo_topology
      cpufreq: intel_pstate: Use topology_cpu_type()
      perf/x86/intel: Use topology_hw_cpu_type()
      x86/cpu: Remove get_this_hybrid_cpu_type()
      x86/cpu: Name CPU matching macro more generically (and shorten)
      x86/cpu: Add cpu_type to struct x86_cpu_id
      x86/cpu: Update x86_match_cpu() to also use cpu-type
      x86/bugs: Declutter vulnerable CPU list
      x86/rfds: Exclude P-only parts from the RFDS affected list

 .../admin-guide/hw-vuln/reg-file-data-sampling.rst |   8 --
 arch/x86/events/intel/core.c                       |   2 +-
 arch/x86/include/asm/cpu.h                         |   6 -
 arch/x86/include/asm/cpu_device_id.h               | 117 +++++++----------
 arch/x86/include/asm/processor.h                   |   3 +
 arch/x86/include/asm/topology.h                    |   9 ++
 arch/x86/kernel/cpu/common.c                       | 138 +++++++++++----------
 arch/x86/kernel/cpu/debugfs.c                      |   1 +
 arch/x86/kernel/cpu/intel.c                        |  16 ---
 arch/x86/kernel/cpu/match.c                        |  22 ++++
 arch/x86/kernel/cpu/topology_common.c              |   9 ++
 drivers/cpufreq/intel_pstate.c                     |  14 +--
 include/linux/mod_devicetable.h                    |   2 +
 13 files changed, 168 insertions(+), 179 deletions(-)
---
base-commit: f2661062f16b2de5d7b6a5c42a9a5c96326b8454
change-id: 20240617-add-cpu-type-4d5e47efc117

Best regards,
-- 
Thanks,
Pawan

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

[Pawan Gupta]

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(+)

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)

-- 
2.34.1

[PATCH PATCH v2 2/9] cpufreq: intel_pstate: Use topology_cpu_type()

[Pawan Gupta]

Intel pstate driver uses hybrid_get_type() to get the cpu-type of a given
CPU. It uses smp_call_function_single() which is sub-optimal. Avoid it by
using topology_hw_cpu_type(cpu) that returns the cached cpu-type.

Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/cpufreq/intel_pstate.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 15de5e3d96fd..0a1e832c7536 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1956,24 +1956,16 @@ static int knl_get_turbo_pstate(int cpu)
 	return ret;
 }
 
-static void hybrid_get_type(void *data)
-{
-	u8 *cpu_type = data;
-
-	*cpu_type = get_this_hybrid_cpu_type();
-}
-
 static int hwp_get_cpu_scaling(int cpu)
 {
-	u8 cpu_type = 0;
+	u8 cpu_type = topology_hw_cpu_type(cpu);
 
-	smp_call_function_single(cpu, hybrid_get_type, &cpu_type, 1);
 	/* P-cores have a smaller perf level-to-freqency scaling factor. */
-	if (cpu_type == 0x40)
+	if (cpu_type == X86_HW_CPU_TYPE_INTEL_CORE)
 		return hybrid_scaling_factor;
 
 	/* Use default core scaling for E-cores */
-	if (cpu_type == 0x20)
+	if (cpu_type == X86_HW_CPU_TYPE_INTEL_ATOM)
 		return core_get_scaling();
 
 	/*

-- 
2.34.1

[PATCH PATCH v2 3/9] perf/x86/intel: Use topology_hw_cpu_type()

[Pawan Gupta]

get_this_hybrid_cpu_type() misses a case when cpu-type is populated
regardless of X86_FEATURE_HYBRID_CPU. This is particularly true for hybrid
variants that have P or E cores fused off.

Instead use topology_hw_cpu_type() as it does not rely on hybrid feature to
enumerate cpu-type. This can also help avoid the model-specific fixup
get_hybrid_cpu_type().

Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
 arch/x86/events/intel/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 38c1b1f1deaa..0da1fd14b0ea 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4753,7 +4753,7 @@ static void intel_pmu_check_hybrid_pmus(struct x86_hybrid_pmu *pmu)
 
 static struct x86_hybrid_pmu *find_hybrid_pmu_for_cpu(void)
 {
-	u8 cpu_type = get_this_hybrid_cpu_type();
+	u8 cpu_type = topology_hw_cpu_type(smp_processor_id());
 	int i;
 
 	/*

-- 
2.34.1

[PATCH PATCH v2 4/9] x86/cpu: Remove get_this_hybrid_cpu_type()

[Pawan Gupta]

get_this_hybrid_cpu_type() is replaced by topology_hw_cpu_type(). There are
no more callers, remove it.

Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
 arch/x86/include/asm/cpu.h  |  6 ------
 arch/x86/kernel/cpu/intel.c | 16 ----------------
 2 files changed, 22 deletions(-)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index aa30fd8cad7f..20e491c22b98 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -31,7 +31,6 @@ extern void __init sld_setup(struct cpuinfo_x86 *c);
 extern bool handle_user_split_lock(struct pt_regs *regs, long error_code);
 extern bool handle_guest_split_lock(unsigned long ip);
 extern void handle_bus_lock(struct pt_regs *regs);
-u8 get_this_hybrid_cpu_type(void);
 #else
 static inline void __init sld_setup(struct cpuinfo_x86 *c) {}
 static inline bool handle_user_split_lock(struct pt_regs *regs, long error_code)
@@ -45,11 +44,6 @@ static inline bool handle_guest_split_lock(unsigned long ip)
 }
 
 static inline void handle_bus_lock(struct pt_regs *regs) {}
-
-static inline u8 get_this_hybrid_cpu_type(void)
-{
-	return 0;
-}
 #endif
 #ifdef CONFIG_IA32_FEAT_CTL
 void init_ia32_feat_ctl(struct cpuinfo_x86 *c);
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index fdf3489d92a4..ac6c68a39051 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -1335,19 +1335,3 @@ void __init sld_setup(struct cpuinfo_x86 *c)
 	sld_state_setup();
 	sld_state_show();
 }
-
-#define X86_HYBRID_CPU_TYPE_ID_SHIFT	24
-
-/**
- * get_this_hybrid_cpu_type() - Get the type of this hybrid CPU
- *
- * Returns the CPU type [31:24] (i.e., Atom or Core) of a CPU in
- * a hybrid processor. If the processor is not hybrid, returns 0.
- */
-u8 get_this_hybrid_cpu_type(void)
-{
-	if (!cpu_feature_enabled(X86_FEATURE_HYBRID_CPU))
-		return 0;
-
-	return cpuid_eax(0x0000001a) >> X86_HYBRID_CPU_TYPE_ID_SHIFT;
-}

-- 
2.34.1

[PATCH PATCH v2 5/9] x86/cpu: Name CPU matching macro more generically (and shorten)

[Pawan Gupta]

To add cpu-type to the existing CPU matching infrastructure, the base macro
X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE need to append _CPU_TYPE. This
makes an already long name longer, and somewhat incomprehensible.

To avoid this, rename the base macro to X86_MATCH_CPU. The macro name
doesn't need to explicitly tell everything that it matches. The arguments
to the macro already hints what it matches.

For consistency, use this base macro to define X86_MATCH_VFM and friends.

Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
 arch/x86/include/asm/cpu_device_id.h | 104 +++++++++++------------------------
 1 file changed, 31 insertions(+), 73 deletions(-)

diff --git a/arch/x86/include/asm/cpu_device_id.h b/arch/x86/include/asm/cpu_device_id.h
index b6325ee30871..6c8f4cf03cae 100644
--- a/arch/x86/include/asm/cpu_device_id.h
+++ b/arch/x86/include/asm/cpu_device_id.h
@@ -58,7 +58,7 @@
 
 #define X86_STEPPINGS(mins, maxs)    GENMASK(maxs, mins)
 /**
- * X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE - Base macro for CPU matching
+ * X86_MATCH_CPU -  Base macro for CPU matching
  * @_vendor:	The vendor name, e.g. INTEL, AMD, HYGON, ..., ANY
  *		The name is expanded to X86_VENDOR_@_vendor
  * @_family:	The family number or X86_FAMILY_ANY
@@ -75,19 +75,7 @@
  * into another macro at the usage site for good reasons, then please
  * start this local macro with X86_MATCH to allow easy grepping.
  */
-#define X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE(_vendor, _family, _model, \
-						    _steppings, _feature, _data) { \
-	.vendor		= X86_VENDOR_##_vendor,				\
-	.family		= _family,					\
-	.model		= _model,					\
-	.steppings	= _steppings,					\
-	.feature	= _feature,					\
-	.flags		= X86_CPU_ID_FLAG_ENTRY_VALID,			\
-	.driver_data	= (unsigned long) _data				\
-}
-
-#define X86_MATCH_VENDORID_FAM_MODEL_STEPPINGS_FEATURE(_vendor, _family, _model, \
-						    _steppings, _feature, _data) { \
+#define X86_MATCH_CPU(_vendor, _family, _model, _steppings, _feature, _data) { \
 	.vendor		= _vendor,					\
 	.family		= _family,					\
 	.model		= _model,					\
@@ -107,13 +95,10 @@
  * @_data:	Driver specific data or NULL. The internal storage
  *		format is unsigned long. The supplied value, pointer
  *		etc. is casted to unsigned long internally.
- *
- * The steppings arguments of X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE() is
- * set to wildcards.
  */
-#define X86_MATCH_VENDOR_FAM_MODEL_FEATURE(vendor, family, model, feature, data) \
-	X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE(vendor, family, model, \
-						X86_STEPPING_ANY, feature, data)
+#define X86_MATCH_VENDOR_FAM_MODEL_FEATURE(vendor, family, model, feature, data)	\
+	X86_MATCH_CPU(X86_VENDOR_##vendor, family, model, X86_STEPPING_ANY,		\
+		      feature, data)
 
 /**
  * X86_MATCH_VENDOR_FAM_FEATURE - Macro for matching vendor, family and CPU feature
@@ -124,13 +109,10 @@
  * @data:	Driver specific data or NULL. The internal storage
  *		format is unsigned long. The supplied value, pointer
  *		etc. is casted to unsigned long internally.
- *
- * All other missing arguments of X86_MATCH_VENDOR_FAM_MODEL_FEATURE() are
- * set to wildcards.
  */
-#define X86_MATCH_VENDOR_FAM_FEATURE(vendor, family, feature, data)	\
-	X86_MATCH_VENDOR_FAM_MODEL_FEATURE(vendor, family,		\
-					   X86_MODEL_ANY, feature, data)
+#define X86_MATCH_VENDOR_FAM_FEATURE(vendor, family, feature, data)		\
+	X86_MATCH_CPU(X86_VENDOR_##vendor, family, X86_MODEL_ANY,		\
+		      X86_STEPPING_ANY, feature, data)
 
 /**
  * X86_MATCH_VENDOR_FEATURE - Macro for matching vendor and CPU feature
@@ -140,12 +122,10 @@
  * @data:	Driver specific data or NULL. The internal storage
  *		format is unsigned long. The supplied value, pointer
  *		etc. is casted to unsigned long internally.
- *
- * All other missing arguments of X86_MATCH_VENDOR_FAM_MODEL_FEATURE() are
- * set to wildcards.
  */
-#define X86_MATCH_VENDOR_FEATURE(vendor, feature, data)			\
-	X86_MATCH_VENDOR_FAM_FEATURE(vendor, X86_FAMILY_ANY, feature, data)
+#define X86_MATCH_VENDOR_FEATURE(vendor, feature, data)				\
+	X86_MATCH_CPU(X86_VENDOR_##vendor, X86_FAMILY_ANY, X86_MODEL_ANY,	\
+		      X86_STEPPING_ANY, feature, data)
 
 /**
  * X86_MATCH_FEATURE - Macro for matching a CPU feature
@@ -153,12 +133,10 @@
  * @data:	Driver specific data or NULL. The internal storage
  *		format is unsigned long. The supplied value, pointer
  *		etc. is casted to unsigned long internally.
- *
- * All other missing arguments of X86_MATCH_VENDOR_FAM_MODEL_FEATURE() are
- * set to wildcards.
  */
-#define X86_MATCH_FEATURE(feature, data)				\
-	X86_MATCH_VENDOR_FEATURE(ANY, feature, data)
+#define X86_MATCH_FEATURE(feature, data)					\
+	X86_MATCH_CPU(X86_VENDOR_ANY, X86_FAMILY_ANY, X86_MODEL_ANY,		\
+		      X86_STEPPING_ANY, feature, data)
 
 /**
  * X86_MATCH_VENDOR_FAM_MODEL - Match vendor, family and model
@@ -169,13 +147,10 @@
  * @data:	Driver specific data or NULL. The internal storage
  *		format is unsigned long. The supplied value, pointer
  *		etc. is casted to unsigned long internally.
- *
- * All other missing arguments of X86_MATCH_VENDOR_FAM_MODEL_FEATURE() are
- * set to wildcards.
  */
-#define X86_MATCH_VENDOR_FAM_MODEL(vendor, family, model, data)		\
-	X86_MATCH_VENDOR_FAM_MODEL_FEATURE(vendor, family, model,	\
-					   X86_FEATURE_ANY, data)
+#define X86_MATCH_VENDOR_FAM_MODEL(vendor, family, model, data)			\
+	X86_MATCH_CPU(X86_VENDOR_##vendor, family, model, X86_STEPPING_ANY,	\
+		      X86_FEATURE_ANY, data)
 
 /**
  * X86_MATCH_VENDOR_FAM - Match vendor and family
@@ -185,12 +160,10 @@
  * @data:	Driver specific data or NULL. The internal storage
  *		format is unsigned long. The supplied value, pointer
  *		etc. is casted to unsigned long internally.
- *
- * All other missing arguments to X86_MATCH_VENDOR_FAM_MODEL_FEATURE() are
- * set of wildcards.
  */
-#define X86_MATCH_VENDOR_FAM(vendor, family, data)			\
-	X86_MATCH_VENDOR_FAM_MODEL(vendor, family, X86_MODEL_ANY, data)
+#define X86_MATCH_VENDOR_FAM(vendor, family, data)				\
+	X86_MATCH_CPU(X86_VENDOR_##vendor, family, X86_MODEL_ANY,		\
+		      X86_STEPPING_ANY, X86_FEATURE_ANY, data)
 
 /**
  * X86_MATCH_INTEL_FAM6_MODEL - Match vendor INTEL, family 6 and model
@@ -209,8 +182,8 @@
 	X86_MATCH_VENDOR_FAM_MODEL(INTEL, 6, INTEL_FAM6_##model, data)
 
 #define X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS(model, steppings, data)	\
-	X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE(INTEL, 6, INTEL_FAM6_##model, \
-						     steppings, X86_FEATURE_ANY, data)
+	X86_MATCH_CPU(X86_VENDOR_INTEL, 6, INTEL_FAM6_##model,		\
+		      steppings, X86_FEATURE_ANY, data)
 
 /**
  * X86_MATCH_VFM - Match encoded vendor/family/model
@@ -218,15 +191,10 @@
  * @data:	Driver specific data or NULL. The internal storage
  *		format is unsigned long. The supplied value, pointer
  *		etc. is cast to unsigned long internally.
- *
- * Stepping and feature are set to wildcards
  */
-#define X86_MATCH_VFM(vfm, data)			\
-	X86_MATCH_VENDORID_FAM_MODEL_STEPPINGS_FEATURE(	\
-		VFM_VENDOR(vfm),			\
-		VFM_FAMILY(vfm),			\
-		VFM_MODEL(vfm),				\
-		X86_STEPPING_ANY, X86_FEATURE_ANY, data)
+#define X86_MATCH_VFM(vfm, data)						\
+	X86_MATCH_CPU(VFM_VENDOR(vfm), VFM_FAMILY(vfm),	VFM_MODEL(vfm),		\
+		      X86_STEPPING_ANY, X86_FEATURE_ANY, data)
 
 /**
  * X86_MATCH_VFM_STEPPINGS - Match encoded vendor/family/model/stepping
@@ -235,15 +203,10 @@
  * @data:	Driver specific data or NULL. The internal storage
  *		format is unsigned long. The supplied value, pointer
  *		etc. is cast to unsigned long internally.
- *
- * feature is set to wildcard
  */
-#define X86_MATCH_VFM_STEPPINGS(vfm, steppings, data)	\
-	X86_MATCH_VENDORID_FAM_MODEL_STEPPINGS_FEATURE(	\
-		VFM_VENDOR(vfm),			\
-		VFM_FAMILY(vfm),			\
-		VFM_MODEL(vfm),				\
-		steppings, X86_FEATURE_ANY, data)
+#define X86_MATCH_VFM_STEPPINGS(vfm, steppings, data)				\
+	X86_MATCH_CPU(VFM_VENDOR(vfm), VFM_FAMILY(vfm), VFM_MODEL(vfm),		\
+		      steppings, X86_FEATURE_ANY, data)
 
 /**
  * X86_MATCH_VFM_FEATURE - Match encoded vendor/family/model/feature
@@ -252,15 +215,10 @@
  * @data:	Driver specific data or NULL. The internal storage
  *		format is unsigned long. The supplied value, pointer
  *		etc. is cast to unsigned long internally.
- *
- * Steppings is set to wildcard
  */
-#define X86_MATCH_VFM_FEATURE(vfm, feature, data)	\
-	X86_MATCH_VENDORID_FAM_MODEL_STEPPINGS_FEATURE(	\
-		VFM_VENDOR(vfm),			\
-		VFM_FAMILY(vfm),			\
-		VFM_MODEL(vfm),				\
-		X86_STEPPING_ANY, feature, data)
+#define X86_MATCH_VFM_FEATURE(vfm, feature, data)				\
+	X86_MATCH_CPU(VFM_VENDOR(vfm), VFM_FAMILY(vfm), VFM_MODEL(vfm),		\
+		      X86_STEPPING_ANY, feature, data)
 
 /*
  * Match specific microcode revisions.

-- 
2.34.1

[PATCH PATCH v2 6/9] x86/cpu: Add cpu_type to struct x86_cpu_id

[Pawan Gupta]

In addition to matching vendor/family/model/feature, for hybrid variants it
is required to also match cpu-type also. For example some CPU
vulnerabilities only affect a specific cpu-type. RFDS only affects Intel
Atom parts.

To be able to also match CPUs based on type add a new field cpu_type to
struct x86_cpu_id which is used by the CPU-matching tables. Introduce
X86_CPU_TYPE_ANY for the cases that don't care about the cpu-type.

Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
 arch/x86/include/asm/cpu_device_id.h | 35 ++++++++++++++++++++++++-----------
 include/linux/mod_devicetable.h      |  2 ++
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/cpu_device_id.h b/arch/x86/include/asm/cpu_device_id.h
index 6c8f4cf03cae..08c2efa6dfdf 100644
--- a/arch/x86/include/asm/cpu_device_id.h
+++ b/arch/x86/include/asm/cpu_device_id.h
@@ -75,13 +75,14 @@
  * into another macro at the usage site for good reasons, then please
  * start this local macro with X86_MATCH to allow easy grepping.
  */
-#define X86_MATCH_CPU(_vendor, _family, _model, _steppings, _feature, _data) { \
+#define X86_MATCH_CPU(_vendor, _family, _model, _steppings, _feature, _cpu_type, _data) { \
 	.vendor		= _vendor,					\
 	.family		= _family,					\
 	.model		= _model,					\
 	.steppings	= _steppings,					\
 	.feature	= _feature,					\
 	.flags		= X86_CPU_ID_FLAG_ENTRY_VALID,			\
+	.cpu_type	= _cpu_type,					\
 	.driver_data	= (unsigned long) _data				\
 }
 
@@ -98,7 +99,7 @@
  */
 #define X86_MATCH_VENDOR_FAM_MODEL_FEATURE(vendor, family, model, feature, data)	\
 	X86_MATCH_CPU(X86_VENDOR_##vendor, family, model, X86_STEPPING_ANY,		\
-		      feature, data)
+		      feature, X86_CPU_TYPE_ANY, data)
 
 /**
  * X86_MATCH_VENDOR_FAM_FEATURE - Macro for matching vendor, family and CPU feature
@@ -112,7 +113,7 @@
  */
 #define X86_MATCH_VENDOR_FAM_FEATURE(vendor, family, feature, data)		\
 	X86_MATCH_CPU(X86_VENDOR_##vendor, family, X86_MODEL_ANY,		\
-		      X86_STEPPING_ANY, feature, data)
+		      X86_STEPPING_ANY, feature, X86_CPU_TYPE_ANY, data)
 
 /**
  * X86_MATCH_VENDOR_FEATURE - Macro for matching vendor and CPU feature
@@ -125,7 +126,7 @@
  */
 #define X86_MATCH_VENDOR_FEATURE(vendor, feature, data)				\
 	X86_MATCH_CPU(X86_VENDOR_##vendor, X86_FAMILY_ANY, X86_MODEL_ANY,	\
-		      X86_STEPPING_ANY, feature, data)
+		      X86_STEPPING_ANY, feature, X86_CPU_TYPE_ANY, data)
 
 /**
  * X86_MATCH_FEATURE - Macro for matching a CPU feature
@@ -136,7 +137,7 @@
  */
 #define X86_MATCH_FEATURE(feature, data)					\
 	X86_MATCH_CPU(X86_VENDOR_ANY, X86_FAMILY_ANY, X86_MODEL_ANY,		\
-		      X86_STEPPING_ANY, feature, data)
+		      X86_STEPPING_ANY, feature, X86_CPU_TYPE_ANY, data)
 
 /**
  * X86_MATCH_VENDOR_FAM_MODEL - Match vendor, family and model
@@ -150,7 +151,7 @@
  */
 #define X86_MATCH_VENDOR_FAM_MODEL(vendor, family, model, data)			\
 	X86_MATCH_CPU(X86_VENDOR_##vendor, family, model, X86_STEPPING_ANY,	\
-		      X86_FEATURE_ANY, data)
+		      X86_FEATURE_ANY, X86_CPU_TYPE_ANY, data)
 
 /**
  * X86_MATCH_VENDOR_FAM - Match vendor and family
@@ -163,7 +164,7 @@
  */
 #define X86_MATCH_VENDOR_FAM(vendor, family, data)				\
 	X86_MATCH_CPU(X86_VENDOR_##vendor, family, X86_MODEL_ANY,		\
-		      X86_STEPPING_ANY, X86_FEATURE_ANY, data)
+		      X86_STEPPING_ANY, X86_FEATURE_ANY, X86_CPU_TYPE_ANY, data)
 
 /**
  * X86_MATCH_INTEL_FAM6_MODEL - Match vendor INTEL, family 6 and model
@@ -183,7 +184,7 @@
 
 #define X86_MATCH_INTEL_FAM6_MODEL_STEPPINGS(model, steppings, data)	\
 	X86_MATCH_CPU(X86_VENDOR_INTEL, 6, INTEL_FAM6_##model,		\
-		      steppings, X86_FEATURE_ANY, data)
+		      steppings, X86_FEATURE_ANY, X86_CPU_TYPE_ANY, data)
 
 /**
  * X86_MATCH_VFM - Match encoded vendor/family/model
@@ -194,7 +195,7 @@
  */
 #define X86_MATCH_VFM(vfm, data)						\
 	X86_MATCH_CPU(VFM_VENDOR(vfm), VFM_FAMILY(vfm),	VFM_MODEL(vfm),		\
-		      X86_STEPPING_ANY, X86_FEATURE_ANY, data)
+		      X86_STEPPING_ANY, X86_FEATURE_ANY, X86_CPU_TYPE_ANY, data)
 
 /**
  * X86_MATCH_VFM_STEPPINGS - Match encoded vendor/family/model/stepping
@@ -206,7 +207,7 @@
  */
 #define X86_MATCH_VFM_STEPPINGS(vfm, steppings, data)				\
 	X86_MATCH_CPU(VFM_VENDOR(vfm), VFM_FAMILY(vfm), VFM_MODEL(vfm),		\
-		      steppings, X86_FEATURE_ANY, data)
+		      steppings, X86_FEATURE_ANY, X86_CPU_TYPE_ANY, data)
 
 /**
  * X86_MATCH_VFM_FEATURE - Match encoded vendor/family/model/feature
@@ -218,7 +219,19 @@
  */
 #define X86_MATCH_VFM_FEATURE(vfm, feature, data)				\
 	X86_MATCH_CPU(VFM_VENDOR(vfm), VFM_FAMILY(vfm), VFM_MODEL(vfm),		\
-		      X86_STEPPING_ANY, feature, data)
+		      X86_STEPPING_ANY, feature, X86_CPU_TYPE_ANY, data)
+
+/**
+ * X86_MATCH_VFM_CPU_TYPE - Match encoded vendor/family/model/cpu-type
+ * @vfm:	Encoded 8-bits each for vendor, family, model
+ * @cpu_type:	CPU type e.g. P-core, E-core on Intel
+ * @data:	Driver specific data or NULL. The internal storage
+ *		format is unsigned long. The supplied value, pointer
+ *		etc. is cast to unsigned long internally.
+ */
+#define X86_MATCH_VFM_CPU_TYPE(vfm, cpu_type, data)			\
+	X86_MATCH_CPU(VFM_VENDOR(vfm), VFM_FAMILY(vfm), VFM_MODEL(vfm),	\
+		      X86_STEPPING_ANY, X86_FEATURE_ANY, cpu_type, data)
 
 /*
  * Match specific microcode revisions.
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 4338b1b4ac44..b8a2e88f966f 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -692,6 +692,7 @@ struct x86_cpu_id {
 	__u16 feature;	/* bit index */
 	/* Solely for kernel-internal use: DO NOT EXPORT to userspace! */
 	__u16 flags;
+	__u8  cpu_type;
 	kernel_ulong_t driver_data;
 };
 
@@ -701,6 +702,7 @@ struct x86_cpu_id {
 #define X86_MODEL_ANY  0
 #define X86_STEPPING_ANY 0
 #define X86_FEATURE_ANY 0	/* Same as FPU, you can't test for that */
+#define X86_CPU_TYPE_ANY 0
 
 /*
  * Generic table type for matching CPU features.

-- 
2.34.1

[PATCH PATCH v2 7/9] x86/cpu: Update x86_match_cpu() to also use cpu-type

[Pawan Gupta]

Non-hybrid CPU variants that share the same Family/Model could be
differentiated by their cpu-type. x86_match_cpu() currently does not use
cpu-type for CPU matching.

Dave Hansen suggested to use below conditions to match CPU-type:

  1. If CPU_TYPE_ANY (the wildcard), then matched
  2. If hybrid, then matched
  3. If !hybrid, look at the boot CPU and compare the cpu-type to determine
     if it is a match.

  This special case for hybrid systems allows more compact vulnerability
  list.  Imagine that "Haswell" CPUs might or might not be hybrid and that
  only Atom cores are vulnerable to Meltdown.  That means there are three
  possibilities:

  	1. P-core only
  	2. Atom only
  	3. Atom + P-core (aka. hybrid)

  One might be tempted to code up the vulnerability list like this:

  	MATCH(     HASWELL, X86_FEATURE_HYBRID, MELTDOWN)
  	MATCH_TYPE(HASWELL, ATOM,               MELTDOWN)

  Logically, this matches #2 and #3. But that's a little silly. You would
  only ask for the "ATOM" match in cases where there *WERE* hybrid cores in
  play. You shouldn't have to _also_ ask for hybrid cores explicitly.

  In short, assume that processors that enumerate Hybrid==1 have a
  vulnerable core type.

Update x86_match_cpu() to also match cpu-type. Also treat hybrid systems as
special, and match them to any cpu-type.

Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
 arch/x86/kernel/cpu/match.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/x86/kernel/cpu/match.c b/arch/x86/kernel/cpu/match.c
index 8e7de733320a..85ef17325c06 100644
--- a/arch/x86/kernel/cpu/match.c
+++ b/arch/x86/kernel/cpu/match.c
@@ -5,6 +5,26 @@
 #include <linux/export.h>
 #include <linux/slab.h>
 
+/**
+ * x86_match_hw_cpu_type - helper function to match the hardware defined
+ *                         cpu-type for a single entry in the x86_cpu_id table.
+ * @c: Pointer to the cpuinfo_x86 structure of the CPU to match.
+ * @m: Pointer to the x86_cpu_id entry to match against.
+ *
+ * Return: true if the cpu-type matches, false otherwise.
+ */
+static bool x86_match_hw_cpu_type(struct cpuinfo_x86 *c, const struct x86_cpu_id *m)
+{
+	if (m->cpu_type == X86_CPU_TYPE_ANY)
+		return true;
+
+	/* Hybrid CPUs are special, they are assumed to match all cpu-types */
+	if (boot_cpu_has(X86_FEATURE_HYBRID_CPU))
+		return true;
+
+	return c->topo.hw_cpu_type == m->cpu_type;
+}
+
 /**
  * x86_match_cpu - match current CPU again an array of x86_cpu_ids
  * @match: Pointer to array of x86_cpu_ids. Last entry terminated with
@@ -50,6 +70,8 @@ const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match)
 			continue;
 		if (m->feature != X86_FEATURE_ANY && !cpu_has(c, m->feature))
 			continue;
+		if (!x86_match_hw_cpu_type(c, m))
+			continue;
 		return m;
 	}
 	return NULL;

-- 
2.34.1

[PATCH PATCH v2 8/9] x86/bugs: Declutter vulnerable CPU list

[Pawan Gupta]

The affected processor table has a lot of repetition and redundant
information that can be omitted. For example:

  VULNBL_INTEL_STEPPINGS(INTEL_IVYBRIDGE,		X86_STEPPING_ANY,		SRBDS),

can easily be simplified to:

  VULNBL_INTEL(IVYBRIDGE,	SRBDS),

Apply this to all the entries in the affected processor table.

No functional change. Disassembly of arch/x86/kernel/cpu/common.o does not
show any difference before and after the change.

Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
 arch/x86/kernel/cpu/common.c | 133 ++++++++++++++++++++++---------------------
 1 file changed, 69 insertions(+), 64 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index d4e539d4e158..7e5cd14e509f 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1128,7 +1128,7 @@ static void identify_cpu_without_cpuid(struct cpuinfo_x86 *c)
 	X86_MATCH_VENDOR_FAM_MODEL(vendor, family, model, whitelist)
 
 #define VULNWL_INTEL(vfm, whitelist)		\
-	X86_MATCH_VFM(vfm, whitelist)
+	X86_MATCH_VFM(INTEL_##vfm, whitelist)
 
 #define VULNWL_AMD(family, whitelist)		\
 	VULNWL(AMD, family, X86_MODEL_ANY, whitelist)
@@ -1145,32 +1145,32 @@ static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = {
 	VULNWL(VORTEX,	6, X86_MODEL_ANY,	NO_SPECULATION),
 
 	/* Intel Family 6 */
-	VULNWL_INTEL(INTEL_TIGERLAKE,		NO_MMIO),
-	VULNWL_INTEL(INTEL_TIGERLAKE_L,		NO_MMIO),
-	VULNWL_INTEL(INTEL_ALDERLAKE,		NO_MMIO),
-	VULNWL_INTEL(INTEL_ALDERLAKE_L,		NO_MMIO),
+	VULNWL_INTEL(TIGERLAKE,			NO_MMIO),
+	VULNWL_INTEL(TIGERLAKE_L,		NO_MMIO),
+	VULNWL_INTEL(ALDERLAKE,			NO_MMIO),
+	VULNWL_INTEL(ALDERLAKE_L,		NO_MMIO),
 
-	VULNWL_INTEL(INTEL_ATOM_SALTWELL,	NO_SPECULATION | NO_ITLB_MULTIHIT),
-	VULNWL_INTEL(INTEL_ATOM_SALTWELL_TABLET, NO_SPECULATION | NO_ITLB_MULTIHIT),
-	VULNWL_INTEL(INTEL_ATOM_SALTWELL_MID,	NO_SPECULATION | NO_ITLB_MULTIHIT),
-	VULNWL_INTEL(INTEL_ATOM_BONNELL,	NO_SPECULATION | NO_ITLB_MULTIHIT),
-	VULNWL_INTEL(INTEL_ATOM_BONNELL_MID,	NO_SPECULATION | NO_ITLB_MULTIHIT),
+	VULNWL_INTEL(ATOM_SALTWELL,		NO_SPECULATION | NO_ITLB_MULTIHIT),
+	VULNWL_INTEL(ATOM_SALTWELL_TABLET,	NO_SPECULATION | NO_ITLB_MULTIHIT),
+	VULNWL_INTEL(ATOM_SALTWELL_MID,		NO_SPECULATION | NO_ITLB_MULTIHIT),
+	VULNWL_INTEL(ATOM_BONNELL,		NO_SPECULATION | NO_ITLB_MULTIHIT),
+	VULNWL_INTEL(ATOM_BONNELL_MID,		NO_SPECULATION | NO_ITLB_MULTIHIT),
 
-	VULNWL_INTEL(INTEL_ATOM_SILVERMONT,	NO_SSB | NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
-	VULNWL_INTEL(INTEL_ATOM_SILVERMONT_D,	NO_SSB | NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
-	VULNWL_INTEL(INTEL_ATOM_SILVERMONT_MID,	NO_SSB | NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
-	VULNWL_INTEL(INTEL_ATOM_AIRMONT,	NO_SSB | NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
-	VULNWL_INTEL(INTEL_XEON_PHI_KNL,	NO_SSB | NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
-	VULNWL_INTEL(INTEL_XEON_PHI_KNM,	NO_SSB | NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
+	VULNWL_INTEL(ATOM_SILVERMONT,		NO_SSB | NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
+	VULNWL_INTEL(ATOM_SILVERMONT_D,		NO_SSB | NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
+	VULNWL_INTEL(ATOM_SILVERMONT_MID,	NO_SSB | NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
+	VULNWL_INTEL(ATOM_AIRMONT,		NO_SSB | NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
+	VULNWL_INTEL(XEON_PHI_KNL,		NO_SSB | NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
+	VULNWL_INTEL(XEON_PHI_KNM,		NO_SSB | NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
 
-	VULNWL_INTEL(INTEL_CORE_YONAH,		NO_SSB),
+	VULNWL_INTEL(CORE_YONAH,		NO_SSB),
 
-	VULNWL_INTEL(INTEL_ATOM_AIRMONT_MID,	NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
-	VULNWL_INTEL(INTEL_ATOM_AIRMONT_NP,	NO_L1TF | NO_SWAPGS | NO_ITLB_MULTIHIT),
+	VULNWL_INTEL(ATOM_AIRMONT_MID,		NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT),
+	VULNWL_INTEL(ATOM_AIRMONT_NP,		NO_L1TF | NO_SWAPGS | NO_ITLB_MULTIHIT),
 
-	VULNWL_INTEL(INTEL_ATOM_GOLDMONT,	NO_MDS | NO_L1TF | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO),
-	VULNWL_INTEL(INTEL_ATOM_GOLDMONT_D,	NO_MDS | NO_L1TF | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO),
-	VULNWL_INTEL(INTEL_ATOM_GOLDMONT_PLUS,	NO_MDS | NO_L1TF | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO | NO_EIBRS_PBRSB),
+	VULNWL_INTEL(ATOM_GOLDMONT,		NO_MDS | NO_L1TF | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO),
+	VULNWL_INTEL(ATOM_GOLDMONT_D,		NO_MDS | NO_L1TF | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO),
+	VULNWL_INTEL(ATOM_GOLDMONT_PLUS,	NO_MDS | NO_L1TF | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO | NO_EIBRS_PBRSB),
 
 	/*
 	 * Technically, swapgs isn't serializing on AMD (despite it previously
@@ -1180,9 +1180,9 @@ static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = {
 	 * good enough for our purposes.
 	 */
 
-	VULNWL_INTEL(INTEL_ATOM_TREMONT,	NO_EIBRS_PBRSB),
-	VULNWL_INTEL(INTEL_ATOM_TREMONT_L,	NO_EIBRS_PBRSB),
-	VULNWL_INTEL(INTEL_ATOM_TREMONT_D,	NO_ITLB_MULTIHIT | NO_EIBRS_PBRSB),
+	VULNWL_INTEL(ATOM_TREMONT,	NO_EIBRS_PBRSB),
+	VULNWL_INTEL(ATOM_TREMONT_L,	NO_EIBRS_PBRSB),
+	VULNWL_INTEL(ATOM_TREMONT_D,	NO_ITLB_MULTIHIT | NO_EIBRS_PBRSB),
 
 	/* AMD Family 0xf - 0x12 */
 	VULNWL_AMD(0x0f,	NO_MELTDOWN | NO_SSB | NO_L1TF | NO_MDS | NO_SWAPGS | NO_ITLB_MULTIHIT | NO_MMIO | NO_BHI),
@@ -1203,8 +1203,11 @@ static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = {
 #define VULNBL(vendor, family, model, blacklist)	\
 	X86_MATCH_VENDOR_FAM_MODEL(vendor, family, model, blacklist)
 
-#define VULNBL_INTEL_STEPPINGS(vfm, steppings, issues)		   \
-	X86_MATCH_VFM_STEPPINGS(vfm, steppings, issues)
+#define VULNBL_INTEL(vfm, issues)			\
+	X86_MATCH_VFM(INTEL_##vfm, issues)
+
+#define VULNBL_INTEL_STEPPINGS(vfm, steppings, issues)	\
+	X86_MATCH_VFM_STEPPINGS(INTEL_##vfm, steppings, issues)
 
 #define VULNBL_AMD(family, blacklist)		\
 	VULNBL(AMD, family, X86_MODEL_ANY, blacklist)
@@ -1229,43 +1232,45 @@ static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = {
 #define RFDS		BIT(7)
 
 static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
-	VULNBL_INTEL_STEPPINGS(INTEL_IVYBRIDGE,		X86_STEPPING_ANY,		SRBDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_HASWELL,		X86_STEPPING_ANY,		SRBDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_HASWELL_L,		X86_STEPPING_ANY,		SRBDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_HASWELL_G,		X86_STEPPING_ANY,		SRBDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_HASWELL_X,		X86_STEPPING_ANY,		MMIO),
-	VULNBL_INTEL_STEPPINGS(INTEL_BROADWELL_D,	X86_STEPPING_ANY,		MMIO),
-	VULNBL_INTEL_STEPPINGS(INTEL_BROADWELL_G,	X86_STEPPING_ANY,		SRBDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_BROADWELL_X,	X86_STEPPING_ANY,		MMIO),
-	VULNBL_INTEL_STEPPINGS(INTEL_BROADWELL,		X86_STEPPING_ANY,		SRBDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_SKYLAKE_X,		X86_STEPPING_ANY,		MMIO | RETBLEED | GDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_SKYLAKE_L,		X86_STEPPING_ANY,		MMIO | RETBLEED | GDS | SRBDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_SKYLAKE,		X86_STEPPING_ANY,		MMIO | RETBLEED | GDS | SRBDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_KABYLAKE_L,	X86_STEPPING_ANY,		MMIO | RETBLEED | GDS | SRBDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_KABYLAKE,		X86_STEPPING_ANY,		MMIO | RETBLEED | GDS | SRBDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_CANNONLAKE_L,	X86_STEPPING_ANY,		RETBLEED),
-	VULNBL_INTEL_STEPPINGS(INTEL_ICELAKE_L,		X86_STEPPING_ANY,		MMIO | MMIO_SBDS | RETBLEED | GDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_ICELAKE_D,		X86_STEPPING_ANY,		MMIO | GDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_ICELAKE_X,		X86_STEPPING_ANY,		MMIO | GDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_COMETLAKE,		X86_STEPPING_ANY,		MMIO | MMIO_SBDS | RETBLEED | GDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_COMETLAKE_L,	X86_STEPPINGS(0x0, 0x0),	MMIO | RETBLEED),
-	VULNBL_INTEL_STEPPINGS(INTEL_COMETLAKE_L,	X86_STEPPING_ANY,		MMIO | MMIO_SBDS | RETBLEED | GDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_TIGERLAKE_L,	X86_STEPPING_ANY,		GDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_TIGERLAKE,		X86_STEPPING_ANY,		GDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_LAKEFIELD,		X86_STEPPING_ANY,		MMIO | MMIO_SBDS | RETBLEED),
-	VULNBL_INTEL_STEPPINGS(INTEL_ROCKETLAKE,	X86_STEPPING_ANY,		MMIO | RETBLEED | GDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_ALDERLAKE,		X86_STEPPING_ANY,		RFDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_ALDERLAKE_L,	X86_STEPPING_ANY,		RFDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_RAPTORLAKE,	X86_STEPPING_ANY,		RFDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_RAPTORLAKE_P,	X86_STEPPING_ANY,		RFDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_RAPTORLAKE_S,	X86_STEPPING_ANY,		RFDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_GRACEMONT,	X86_STEPPING_ANY,		RFDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_TREMONT,	X86_STEPPING_ANY,		MMIO | MMIO_SBDS | RFDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_TREMONT_D,	X86_STEPPING_ANY,		MMIO | RFDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_TREMONT_L,	X86_STEPPING_ANY,		MMIO | MMIO_SBDS | RFDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_GOLDMONT,	X86_STEPPING_ANY,		RFDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_GOLDMONT_D,	X86_STEPPING_ANY,		RFDS),
-	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_GOLDMONT_PLUS, X86_STEPPING_ANY,		RFDS),
+	VULNBL_INTEL(IVYBRIDGE,			SRBDS),
+	VULNBL_INTEL(HASWELL,			SRBDS),
+	VULNBL_INTEL(HASWELL_L,			SRBDS),
+	VULNBL_INTEL(HASWELL_G,			SRBDS),
+	VULNBL_INTEL(HASWELL_X,			MMIO),
+	VULNBL_INTEL(BROADWELL_D,		MMIO),
+	VULNBL_INTEL(BROADWELL_G,		SRBDS),
+	VULNBL_INTEL(BROADWELL_X,		MMIO),
+	VULNBL_INTEL(BROADWELL,			SRBDS),
+	VULNBL_INTEL(SKYLAKE_X,			MMIO | RETBLEED | GDS),
+	VULNBL_INTEL(SKYLAKE_L,			MMIO | RETBLEED | GDS | SRBDS),
+	VULNBL_INTEL(SKYLAKE,			MMIO | RETBLEED | GDS | SRBDS),
+	VULNBL_INTEL(KABYLAKE_L,		MMIO | RETBLEED | GDS | SRBDS),
+	VULNBL_INTEL(KABYLAKE,			MMIO | RETBLEED | GDS | SRBDS),
+	VULNBL_INTEL(CANNONLAKE_L,		RETBLEED),
+	VULNBL_INTEL(ICELAKE_L,			MMIO | MMIO_SBDS | RETBLEED | GDS),
+	VULNBL_INTEL(ICELAKE_D,			MMIO | GDS),
+	VULNBL_INTEL(ICELAKE_X,			MMIO | GDS),
+	VULNBL_INTEL(COMETLAKE,			MMIO | MMIO_SBDS | RETBLEED | GDS),
+	VULNBL_INTEL(TIGERLAKE_L,		GDS),
+	VULNBL_INTEL(TIGERLAKE,			GDS),
+	VULNBL_INTEL(LAKEFIELD,			MMIO | MMIO_SBDS | RETBLEED),
+	VULNBL_INTEL(ROCKETLAKE,		MMIO | RETBLEED | GDS),
+	VULNBL_INTEL(ALDERLAKE,			RFDS),
+	VULNBL_INTEL(ALDERLAKE_L,		RFDS),
+	VULNBL_INTEL(RAPTORLAKE,		RFDS),
+	VULNBL_INTEL(RAPTORLAKE_P,		RFDS),
+	VULNBL_INTEL(RAPTORLAKE_S,		RFDS),
+	VULNBL_INTEL(ATOM_GRACEMONT,		RFDS),
+	VULNBL_INTEL(ATOM_TREMONT,		MMIO | MMIO_SBDS | RFDS),
+	VULNBL_INTEL(ATOM_TREMONT_D,		MMIO | RFDS),
+	VULNBL_INTEL(ATOM_TREMONT_L,		MMIO | MMIO_SBDS | RFDS),
+	VULNBL_INTEL(ATOM_GOLDMONT,		RFDS),
+	VULNBL_INTEL(ATOM_GOLDMONT_D,		RFDS),
+	VULNBL_INTEL(ATOM_GOLDMONT_PLUS,	RFDS),
+
+	/* Match more than Vendor/Family/Model */
+	VULNBL_INTEL_STEPPINGS(COMETLAKE_L,	X86_STEPPINGS(0x0, 0x0),	MMIO | RETBLEED),
+	VULNBL_INTEL	      (COMETLAKE_L,					MMIO | MMIO_SBDS | RETBLEED | GDS),
 
 	VULNBL_AMD(0x15, RETBLEED),
 	VULNBL_AMD(0x16, RETBLEED),

-- 
2.34.1

[PATCH PATCH v2 9/9] x86/rfds: Exclude P-only parts from the RFDS affected list

[Pawan Gupta]

RFDS only affects Atom parts. Vendor/Family/Model matching in the affected
processor table makes Alderlake and Raptorlake P-only parts affected (which
are not affected in reality). This is because the affected hybrid and
E-only parts have the same Family/Model as the unaffected P-only parts.

Match CPU-type as Atom to exclude P-only parts as RFDS affected.

Note, a guest with the same Family/Model as the affected part may not have
leaf 1A enumerated to know its CPU-type, but it should not be a problem as
guest's Family/Model can anyways be inaccurate. Moreover, RFDS_NO or
RFDS_CLEAR enumeration by the VMM decides the affected status of the guest.

Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
 Documentation/admin-guide/hw-vuln/reg-file-data-sampling.rst | 8 --------
 arch/x86/kernel/cpu/common.c                                 | 9 +++++++--
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/Documentation/admin-guide/hw-vuln/reg-file-data-sampling.rst b/Documentation/admin-guide/hw-vuln/reg-file-data-sampling.rst
index 0585d02b9a6c..ad15417d39f9 100644
--- a/Documentation/admin-guide/hw-vuln/reg-file-data-sampling.rst
+++ b/Documentation/admin-guide/hw-vuln/reg-file-data-sampling.rst
@@ -29,14 +29,6 @@ Below is the list of affected Intel processors [#f1]_:
    RAPTORLAKE_S            06_BFH
    ===================  ============
 
-As an exception to this table, Intel Xeon E family parts ALDERLAKE(06_97H) and
-RAPTORLAKE(06_B7H) codenamed Catlow are not affected. They are reported as
-vulnerable in Linux because they share the same family/model with an affected
-part. Unlike their affected counterparts, they do not enumerate RFDS_CLEAR or
-CPUID.HYBRID. This information could be used to distinguish between the
-affected and unaffected parts, but it is deemed not worth adding complexity as
-the reporting is fixed automatically when these parts enumerate RFDS_NO.
-
 Mitigation
 ==========
 Intel released a microcode update that enables software to clear sensitive
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 7e5cd14e509f..86e0c69a60f6 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1209,6 +1209,11 @@ static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = {
 #define VULNBL_INTEL_STEPPINGS(vfm, steppings, issues)	\
 	X86_MATCH_VFM_STEPPINGS(INTEL_##vfm, steppings, issues)
 
+#define VULNBL_INTEL_TYPE(vfm, cpu_type, issues)			\
+	X86_MATCH_VFM_CPU_TYPE(INTEL_##vfm,				\
+			       X86_HW_CPU_TYPE_INTEL_##cpu_type,	\
+			       issues)
+
 #define VULNBL_AMD(family, blacklist)		\
 	VULNBL(AMD, family, X86_MODEL_ANY, blacklist)
 
@@ -1255,9 +1260,7 @@ static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
 	VULNBL_INTEL(TIGERLAKE,			GDS),
 	VULNBL_INTEL(LAKEFIELD,			MMIO | MMIO_SBDS | RETBLEED),
 	VULNBL_INTEL(ROCKETLAKE,		MMIO | RETBLEED | GDS),
-	VULNBL_INTEL(ALDERLAKE,			RFDS),
 	VULNBL_INTEL(ALDERLAKE_L,		RFDS),
-	VULNBL_INTEL(RAPTORLAKE,		RFDS),
 	VULNBL_INTEL(RAPTORLAKE_P,		RFDS),
 	VULNBL_INTEL(RAPTORLAKE_S,		RFDS),
 	VULNBL_INTEL(ATOM_GRACEMONT,		RFDS),
@@ -1271,6 +1274,8 @@ static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
 	/* Match more than Vendor/Family/Model */
 	VULNBL_INTEL_STEPPINGS(COMETLAKE_L,	X86_STEPPINGS(0x0, 0x0),	MMIO | RETBLEED),
 	VULNBL_INTEL	      (COMETLAKE_L,					MMIO | MMIO_SBDS | RETBLEED | GDS),
+	VULNBL_INTEL_TYPE     (ALDERLAKE,	ATOM,				RFDS),
+	VULNBL_INTEL_TYPE     (RAPTORLAKE,	ATOM,				RFDS),
 
 	VULNBL_AMD(0x15, RETBLEED),
 	VULNBL_AMD(0x16, RETBLEED),

-- 
2.34.1

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

[Borislav Petkov]

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 '#'?

> 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);

Yeah, no, we're not going to perpetuate this silliness of printing hex
values without a preceding "0x".

>  	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;
> +}

Why isn't this happening in cpu/intel.c? And then you don't need yet
another silly function.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH PATCH v2 3/9] perf/x86/intel: Use topology_hw_cpu_type()

[Mi, Dapeng]

On 6/28/2024 4:44 AM, Pawan Gupta wrote:
> get_this_hybrid_cpu_type() misses a case when cpu-type is populated
> regardless of X86_FEATURE_HYBRID_CPU. This is particularly true for hybrid
> variants that have P or E cores fused off.
>
> Instead use topology_hw_cpu_type() as it does not rely on hybrid feature to
> enumerate cpu-type. This can also help avoid the model-specific fixup
> get_hybrid_cpu_type().
>
> Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> ---
>  arch/x86/events/intel/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 38c1b1f1deaa..0da1fd14b0ea 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -4753,7 +4753,7 @@ static void intel_pmu_check_hybrid_pmus(struct x86_hybrid_pmu *pmu)
>  
>  static struct x86_hybrid_pmu *find_hybrid_pmu_for_cpu(void)
>  {
> -	u8 cpu_type = get_this_hybrid_cpu_type();
> +	u8 cpu_type = topology_hw_cpu_type(smp_processor_id());

As Kan said, ARL-H would have two different atom uarchs, so we have to use
the extra native model id to identify them for PMU enabling. I'm not sure
if we need a similar helper topology_hw_cpu_native_id(), it may depend on
if the native id needs to be exposed to user space? such as whether there
are different vulnerabilities between these two atom uarchs?

For PMU enabling, we don't need to expose the native model ID to user
space, so we define a new helper get_this_hybrid_cpu_native_id() and
leverage it to identify the atom uarch.


>  	int i;
>  
>  	/*
>

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

[Pawan Gupta]

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.

> > 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);
> 
> Yeah, no, we're not going to perpetuate this silliness of printing hex
> values without a preceding "0x".

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.

> > +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:

---
diff --git a/arch/x86/kernel/cpu/topology_common.c b/arch/x86/kernel/cpu/topology_common.c
index 8b47bd6b0623..c8869e75365f 100644
--- a/arch/x86/kernel/cpu/topology_common.c
+++ b/arch/x86/kernel/cpu/topology_common.c
@@ -87,6 +87,7 @@ static void parse_topology(struct topo_scan *tscan, bool early)
 		.cu_id			= 0xff,
 		.llc_id			= BAD_APICID,
 		.l2c_id			= BAD_APICID,
+		.hw_cpu_type		= X86_HW_CPU_TYPE_UNKNOWN,
 	};
 	struct cpuinfo_x86 *c = tscan->c;
 	struct {
@@ -132,6 +133,8 @@ static void parse_topology(struct topo_scan *tscan, bool early)
 	case X86_VENDOR_INTEL:
 		if (!IS_ENABLED(CONFIG_CPU_SUP_INTEL) || !cpu_parse_topology_ext(tscan))
 			parse_legacy(tscan);
+		if (c->cpuid_level >= 0x1a)
+			c->topo.hw_cpu_type = cpuid_eax(0x1a) >> X86_CPU_TYPE_INTEL_SHIFT;
 		break;
 	case X86_VENDOR_HYGON:
 		if (IS_ENABLED(CONFIG_CPU_SUP_HYGON))

Re: [PATCH v2 3/9] perf/x86/intel: Use topology_hw_cpu_type()

[Pawan Gupta]

On Fri, Jun 28, 2024 at 04:59:18PM +0800, Mi, Dapeng wrote:
> 
> On 6/28/2024 4:44 AM, Pawan Gupta wrote:
> > get_this_hybrid_cpu_type() misses a case when cpu-type is populated
> > regardless of X86_FEATURE_HYBRID_CPU. This is particularly true for hybrid
> > variants that have P or E cores fused off.
> >
> > Instead use topology_hw_cpu_type() as it does not rely on hybrid feature to
> > enumerate cpu-type. This can also help avoid the model-specific fixup
> > get_hybrid_cpu_type().
> >
> > Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > ---
> >  arch/x86/events/intel/core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> > index 38c1b1f1deaa..0da1fd14b0ea 100644
> > --- a/arch/x86/events/intel/core.c
> > +++ b/arch/x86/events/intel/core.c
> > @@ -4753,7 +4753,7 @@ static void intel_pmu_check_hybrid_pmus(struct x86_hybrid_pmu *pmu)
> >  
> >  static struct x86_hybrid_pmu *find_hybrid_pmu_for_cpu(void)
> >  {
> > -	u8 cpu_type = get_this_hybrid_cpu_type();
> > +	u8 cpu_type = topology_hw_cpu_type(smp_processor_id());
> 
> As Kan said, ARL-H would have two different atom uarchs, so we have to use
> the extra native model id to identify them for PMU enabling. I'm not sure
> if we need a similar helper topology_hw_cpu_native_id(), it may depend on
> if the native id needs to be exposed to user space? such as whether there
> are different vulnerabilities between these two atom uarchs?
> 
> For PMU enabling, we don't need to expose the native model ID to user
> space, so we define a new helper get_this_hybrid_cpu_native_id() and
> leverage it to identify the atom uarch.

As CPUID.1A.EAX returns both the native model ID and core type, we can
store it raw in topo->hw_cpu_type. Such that topo->hw_cpu_type contains
both the native model ID and core-type. Then use accessors to get them
separately?

As per Intel SDM Vol 2A, table 3-8, combination of core-type and native
model ID identifies the microarchitecture uniquely:

  EAX Enumerates the native model ID and core type.

	Bits 31-24: Core type*
		10H: Reserved
		20H: Intel Atom®
		30H: Reserved
		40H: Intel® Core™

	Bits 23-00: Native model ID of the core. The core-type and native
	model ID can be used to uniquely identify the microarchitecture of
	the core.

Let me know if below patch that implements intel_get_native_model_id()
would work for your use case:

---
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 20e491c22b98..eaa84e4e4771 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -26,11 +26,13 @@ int mwait_usable(const struct cpuinfo_x86 *);
 unsigned int x86_family(unsigned int sig);
 unsigned int x86_model(unsigned int sig);
 unsigned int x86_stepping(unsigned int sig);
+enum x86_hw_topo_cpu_type x86_get_hw_cpu_type(struct cpuinfo_x86 *c);
 #ifdef CONFIG_CPU_SUP_INTEL
 extern void __init sld_setup(struct cpuinfo_x86 *c);
 extern bool handle_user_split_lock(struct pt_regs *regs, long error_code);
 extern bool handle_guest_split_lock(unsigned long ip);
 extern void handle_bus_lock(struct pt_regs *regs);
+u32 intel_get_native_model_id(struct cpuinfo_x86 *c);
 #else
 static inline void __init sld_setup(struct cpuinfo_x86 *c) {}
 static inline bool handle_user_split_lock(struct pt_regs *regs, long error_code)
@@ -44,6 +46,7 @@ static inline bool handle_guest_split_lock(unsigned long ip)
 }
 
 static inline void handle_bus_lock(struct pt_regs *regs) {}
+u32 intel_get_native_model_id(struct cpuinfo_x86 *c)  { return 0; }
 #endif
 #ifdef CONFIG_IA32_FEAT_CTL
 void init_ia32_feat_ctl(struct cpuinfo_x86 *c);
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index d8d715fcc25c..08794668750f 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -107,7 +107,7 @@ struct cpuinfo_topology {
 	u32			l2c_id;
 
 	// Hardware defined CPU-type
-	u8			hw_cpu_type;
+	u32			hw_cpu_type;
 };
 
 struct cpuinfo_x86 {
diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index 717fdb928dc3..70f1a9661ce3 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -33,8 +33,6 @@
 #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,
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 86e0c69a60f6..151c2377df21 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1292,6 +1292,11 @@ static bool __init cpu_matches(const struct x86_cpu_id *table, unsigned long whi
 	return m && !!(m->driver_data & which);
 }
 
+enum x86_hw_topo_cpu_type __weak x86_get_hw_cpu_type(struct cpuinfo_x86 *c)
+{
+	return X86_HW_CPU_TYPE_UNKNOWN;
+}
+
 u64 x86_read_arch_cap_msr(void)
 {
 	u64 x86_arch_cap_msr = 0;
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index ac6c68a39051..3a951632d210 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -1335,3 +1335,16 @@ void __init sld_setup(struct cpuinfo_x86 *c)
 	sld_state_setup();
 	sld_state_show();
 }
+
+#define X86_INTEL_CPU_TYPE_SHIFT	24
+#define X86_INTEL_NATIVE_MODEL_ID_MASK	((1 << X86_INTEL_CPU_TYPE_SHIFT) - 1)
+
+enum x86_hw_topo_cpu_type x86_get_hw_cpu_type(struct cpuinfo_x86 *c)
+{
+	return c->topo.hw_cpu_type >> X86_INTEL_CPU_TYPE_SHIFT;
+}
+
+u32 intel_get_native_model_id(struct cpuinfo_x86 *c)
+{
+	return c->topo.hw_cpu_type & X86_INTEL_NATIVE_MODEL_ID_MASK;
+}
diff --git a/arch/x86/kernel/cpu/match.c b/arch/x86/kernel/cpu/match.c
index 85ef17325c06..8493bcd4db0b 100644
--- a/arch/x86/kernel/cpu/match.c
+++ b/arch/x86/kernel/cpu/match.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <asm/cpu_device_id.h>
 #include <asm/cpufeature.h>
+#include <asm/cpu.h>
 #include <linux/cpu.h>
 #include <linux/export.h>
 #include <linux/slab.h>
@@ -22,7 +23,7 @@ static bool x86_match_hw_cpu_type(struct cpuinfo_x86 *c, const struct x86_cpu_id
 	if (boot_cpu_has(X86_FEATURE_HYBRID_CPU))
 		return true;
 
-	return c->topo.hw_cpu_type == m->cpu_type;
+	return m->cpu_type == x86_get_hw_cpu_type(c);
 }
 
 /**
diff --git a/arch/x86/kernel/cpu/topology_common.c b/arch/x86/kernel/cpu/topology_common.c
index 8b47bd6b0623..819f850b1960 100644
--- a/arch/x86/kernel/cpu/topology_common.c
+++ b/arch/x86/kernel/cpu/topology_common.c
@@ -87,6 +87,7 @@ static void parse_topology(struct topo_scan *tscan, bool early)
 		.cu_id			= 0xff,
 		.llc_id			= BAD_APICID,
 		.l2c_id			= BAD_APICID,
+		.hw_cpu_type		= X86_HW_CPU_TYPE_UNKNOWN,
 	};
 	struct cpuinfo_x86 *c = tscan->c;
 	struct {
@@ -132,6 +133,8 @@ static void parse_topology(struct topo_scan *tscan, bool early)
 	case X86_VENDOR_INTEL:
 		if (!IS_ENABLED(CONFIG_CPU_SUP_INTEL) || !cpu_parse_topology_ext(tscan))
 			parse_legacy(tscan);
+		if (c->cpuid_level >= 0x1a)
+			c->topo.hw_cpu_type = cpuid_eax(0x1a);
 		break;
 	case X86_VENDOR_HYGON:
 		if (IS_ENABLED(CONFIG_CPU_SUP_HYGON))
@@ -140,14 +143,6 @@ 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;
@@ -198,7 +193,6 @@ 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)

Re: [PATCH v2 3/9] perf/x86/intel: Use topology_hw_cpu_type()

[Mi, Dapeng]

On 6/29/2024 2:51 AM, Pawan Gupta wrote:
> On Fri, Jun 28, 2024 at 04:59:18PM +0800, Mi, Dapeng wrote:
>> On 6/28/2024 4:44 AM, Pawan Gupta wrote:
>>> get_this_hybrid_cpu_type() misses a case when cpu-type is populated
>>> regardless of X86_FEATURE_HYBRID_CPU. This is particularly true for hybrid
>>> variants that have P or E cores fused off.
>>>
>>> Instead use topology_hw_cpu_type() as it does not rely on hybrid feature to
>>> enumerate cpu-type. This can also help avoid the model-specific fixup
>>> get_hybrid_cpu_type().
>>>
>>> Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
>>> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
>>> ---
>>>  arch/x86/events/intel/core.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>>> index 38c1b1f1deaa..0da1fd14b0ea 100644
>>> --- a/arch/x86/events/intel/core.c
>>> +++ b/arch/x86/events/intel/core.c
>>> @@ -4753,7 +4753,7 @@ static void intel_pmu_check_hybrid_pmus(struct x86_hybrid_pmu *pmu)
>>>  
>>>  static struct x86_hybrid_pmu *find_hybrid_pmu_for_cpu(void)
>>>  {
>>> -	u8 cpu_type = get_this_hybrid_cpu_type();
>>> +	u8 cpu_type = topology_hw_cpu_type(smp_processor_id());
>> As Kan said, ARL-H would have two different atom uarchs, so we have to use
>> the extra native model id to identify them for PMU enabling. I'm not sure
>> if we need a similar helper topology_hw_cpu_native_id(), it may depend on
>> if the native id needs to be exposed to user space? such as whether there
>> are different vulnerabilities between these two atom uarchs?
>>
>> For PMU enabling, we don't need to expose the native model ID to user
>> space, so we define a new helper get_this_hybrid_cpu_native_id() and
>> leverage it to identify the atom uarch.
> As CPUID.1A.EAX returns both the native model ID and core type, we can
> store it raw in topo->hw_cpu_type. Such that topo->hw_cpu_type contains
> both the native model ID and core-type. Then use accessors to get them
> separately?
>
> As per Intel SDM Vol 2A, table 3-8, combination of core-type and native
> model ID identifies the microarchitecture uniquely:
>
>   EAX Enumerates the native model ID and core type.
>
> 	Bits 31-24: Core type*
> 		10H: Reserved
> 		20H: Intel Atom®
> 		30H: Reserved
> 		40H: Intel® Core™
>
> 	Bits 23-00: Native model ID of the core. The core-type and native
> 	model ID can be used to uniquely identify the microarchitecture of
> 	the core.
>
> Let me know if below patch that implements intel_get_native_model_id()
> would work for your use case:

It looks good for me. Thanks.


>
> ---
> diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
> index 20e491c22b98..eaa84e4e4771 100644
> --- a/arch/x86/include/asm/cpu.h
> +++ b/arch/x86/include/asm/cpu.h
> @@ -26,11 +26,13 @@ int mwait_usable(const struct cpuinfo_x86 *);
>  unsigned int x86_family(unsigned int sig);
>  unsigned int x86_model(unsigned int sig);
>  unsigned int x86_stepping(unsigned int sig);
> +enum x86_hw_topo_cpu_type x86_get_hw_cpu_type(struct cpuinfo_x86 *c);
>  #ifdef CONFIG_CPU_SUP_INTEL
>  extern void __init sld_setup(struct cpuinfo_x86 *c);
>  extern bool handle_user_split_lock(struct pt_regs *regs, long error_code);
>  extern bool handle_guest_split_lock(unsigned long ip);
>  extern void handle_bus_lock(struct pt_regs *regs);
> +u32 intel_get_native_model_id(struct cpuinfo_x86 *c);
>  #else
>  static inline void __init sld_setup(struct cpuinfo_x86 *c) {}
>  static inline bool handle_user_split_lock(struct pt_regs *regs, long error_code)
> @@ -44,6 +46,7 @@ static inline bool handle_guest_split_lock(unsigned long ip)
>  }
>  
>  static inline void handle_bus_lock(struct pt_regs *regs) {}
> +u32 intel_get_native_model_id(struct cpuinfo_x86 *c)  { return 0; }
>  #endif
>  #ifdef CONFIG_IA32_FEAT_CTL
>  void init_ia32_feat_ctl(struct cpuinfo_x86 *c);
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index d8d715fcc25c..08794668750f 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -107,7 +107,7 @@ struct cpuinfo_topology {
>  	u32			l2c_id;
>  
>  	// Hardware defined CPU-type
> -	u8			hw_cpu_type;
> +	u32			hw_cpu_type;

Since hw_cpu_type represents the whole EAX right now, it may need to be
changed to a more generic name, like hw_cpu_model, or others.


>  };
>  
>  struct cpuinfo_x86 {
> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> index 717fdb928dc3..70f1a9661ce3 100644
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -33,8 +33,6 @@
>  #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,
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 86e0c69a60f6..151c2377df21 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1292,6 +1292,11 @@ static bool __init cpu_matches(const struct x86_cpu_id *table, unsigned long whi
>  	return m && !!(m->driver_data & which);
>  }
>  
> +enum x86_hw_topo_cpu_type __weak x86_get_hw_cpu_type(struct cpuinfo_x86 *c)
> +{
> +	return X86_HW_CPU_TYPE_UNKNOWN;
> +}
> +
>  u64 x86_read_arch_cap_msr(void)
>  {
>  	u64 x86_arch_cap_msr = 0;
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index ac6c68a39051..3a951632d210 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -1335,3 +1335,16 @@ void __init sld_setup(struct cpuinfo_x86 *c)
>  	sld_state_setup();
>  	sld_state_show();
>  }
> +
> +#define X86_INTEL_CPU_TYPE_SHIFT	24
> +#define X86_INTEL_NATIVE_MODEL_ID_MASK	((1 << X86_INTEL_CPU_TYPE_SHIFT) - 1)
> +
> +enum x86_hw_topo_cpu_type x86_get_hw_cpu_type(struct cpuinfo_x86 *c)
> +{
> +	return c->topo.hw_cpu_type >> X86_INTEL_CPU_TYPE_SHIFT;
> +}
> +
> +u32 intel_get_native_model_id(struct cpuinfo_x86 *c)
> +{
> +	return c->topo.hw_cpu_type & X86_INTEL_NATIVE_MODEL_ID_MASK;
> +}
> diff --git a/arch/x86/kernel/cpu/match.c b/arch/x86/kernel/cpu/match.c
> index 85ef17325c06..8493bcd4db0b 100644
> --- a/arch/x86/kernel/cpu/match.c
> +++ b/arch/x86/kernel/cpu/match.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #include <asm/cpu_device_id.h>
>  #include <asm/cpufeature.h>
> +#include <asm/cpu.h>
>  #include <linux/cpu.h>
>  #include <linux/export.h>
>  #include <linux/slab.h>
> @@ -22,7 +23,7 @@ static bool x86_match_hw_cpu_type(struct cpuinfo_x86 *c, const struct x86_cpu_id
>  	if (boot_cpu_has(X86_FEATURE_HYBRID_CPU))
>  		return true;
>  
> -	return c->topo.hw_cpu_type == m->cpu_type;
> +	return m->cpu_type == x86_get_hw_cpu_type(c);
>  }
>  
>  /**
> diff --git a/arch/x86/kernel/cpu/topology_common.c b/arch/x86/kernel/cpu/topology_common.c
> index 8b47bd6b0623..819f850b1960 100644
> --- a/arch/x86/kernel/cpu/topology_common.c
> +++ b/arch/x86/kernel/cpu/topology_common.c
> @@ -87,6 +87,7 @@ static void parse_topology(struct topo_scan *tscan, bool early)
>  		.cu_id			= 0xff,
>  		.llc_id			= BAD_APICID,
>  		.l2c_id			= BAD_APICID,
> +		.hw_cpu_type		= X86_HW_CPU_TYPE_UNKNOWN,
>  	};
>  	struct cpuinfo_x86 *c = tscan->c;
>  	struct {
> @@ -132,6 +133,8 @@ static void parse_topology(struct topo_scan *tscan, bool early)
>  	case X86_VENDOR_INTEL:
>  		if (!IS_ENABLED(CONFIG_CPU_SUP_INTEL) || !cpu_parse_topology_ext(tscan))
>  			parse_legacy(tscan);
> +		if (c->cpuid_level >= 0x1a)
> +			c->topo.hw_cpu_type = cpuid_eax(0x1a);
>  		break;
>  	case X86_VENDOR_HYGON:
>  		if (IS_ENABLED(CONFIG_CPU_SUP_HYGON))
> @@ -140,14 +143,6 @@ 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;
> @@ -198,7 +193,6 @@ 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)

Re: [PATCH PATCH v2 2/9] cpufreq: intel_pstate: Use topology_cpu_type()

[Rafael J. Wysocki]

On Thu, Jun 27, 2024 at 10:44 PM Pawan Gupta
<pawan.kumar.gupta@linux.intel.com> wrote:
>
> Intel pstate driver uses hybrid_get_type() to get the cpu-type of a given
> CPU. It uses smp_call_function_single() which is sub-optimal. Avoid it by
> using topology_hw_cpu_type(cpu) that returns the cached cpu-type.
>
> Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

and I'm assuming that it will be routed along with the rest of the series.

Thanks!

> ---
>  drivers/cpufreq/intel_pstate.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 15de5e3d96fd..0a1e832c7536 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1956,24 +1956,16 @@ static int knl_get_turbo_pstate(int cpu)
>         return ret;
>  }
>
> -static void hybrid_get_type(void *data)
> -{
> -       u8 *cpu_type = data;
> -
> -       *cpu_type = get_this_hybrid_cpu_type();
> -}
> -
>  static int hwp_get_cpu_scaling(int cpu)
>  {
> -       u8 cpu_type = 0;
> +       u8 cpu_type = topology_hw_cpu_type(cpu);
>
> -       smp_call_function_single(cpu, hybrid_get_type, &cpu_type, 1);
>         /* P-cores have a smaller perf level-to-freqency scaling factor. */
> -       if (cpu_type == 0x40)
> +       if (cpu_type == X86_HW_CPU_TYPE_INTEL_CORE)
>                 return hybrid_scaling_factor;
>
>         /* Use default core scaling for E-cores */
> -       if (cpu_type == 0x20)
> +       if (cpu_type == X86_HW_CPU_TYPE_INTEL_ATOM)
>                 return core_get_scaling();
>
>         /*
>
> --

Re: [PATCH PATCH v2 2/9] cpufreq: intel_pstate: Use topology_cpu_type()

[Pawan Gupta]

On Mon, Jul 01, 2024 at 07:08:41PM +0200, Rafael J. Wysocki wrote:
> On Thu, Jun 27, 2024 at 10:44 PM Pawan Gupta
> <pawan.kumar.gupta@linux.intel.com> wrote:
> >
> > Intel pstate driver uses hybrid_get_type() to get the cpu-type of a given
> > CPU. It uses smp_call_function_single() which is sub-optimal. Avoid it by
> > using topology_hw_cpu_type(cpu) that returns the cached cpu-type.
> >
> > Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
> > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Thanks.

> and I'm assuming that it will be routed along with the rest of the series.

I believe yes, as it has a dependency on the first patch.

Re: [PATCH v2 3/9] perf/x86/intel: Use topology_hw_cpu_type()

[Pawan Gupta]

On Mon, Jul 01, 2024 at 11:37:26AM +0800, Mi, Dapeng wrote:
> > diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> > index d8d715fcc25c..08794668750f 100644
> > --- a/arch/x86/include/asm/processor.h
> > +++ b/arch/x86/include/asm/processor.h
> > @@ -107,7 +107,7 @@ struct cpuinfo_topology {
> >  	u32			l2c_id;
> >  
> >  	// Hardware defined CPU-type
> > -	u8			hw_cpu_type;
> > +	u32			hw_cpu_type;
> 
> Since hw_cpu_type represents the whole EAX right now, it may need to be
> changed to a more generic name, like hw_cpu_model, or others.

Even with whole EAX it still identifies the CPU-type (just more granular).

Since hw_cpu_type will be used by AMD as well, I think it is better to keep
it as is. Interpretation of what hw_cpu_type means can be left to vendor
specific code.

Re: [PATCH PATCH v2 8/9] x86/bugs: Declutter vulnerable CPU list

[Josh Poimboeuf]

On Thu, Jun 27, 2024 at 01:44:48PM -0700, Pawan Gupta wrote:
> The affected processor table has a lot of repetition and redundant
> information that can be omitted. For example:
> 
>   VULNBL_INTEL_STEPPINGS(INTEL_IVYBRIDGE,		X86_STEPPING_ANY,		SRBDS),
> 
> can easily be simplified to:
> 
>   VULNBL_INTEL(IVYBRIDGE,	SRBDS),
> 
> Apply this to all the entries in the affected processor table.
> 
> No functional change. Disassembly of arch/x86/kernel/cpu/common.o does not
> show any difference before and after the change.

This patch only changes data, not code.  So there's not much point in
diffing the disassembly ;-)

A diff of the .init.rodata sections actually shows one (non-functional)
difference in cpu_vuln_blacklist[].

The COMETLAKE_L entries were moved to a new section below the rest of
the entries:

	/* Match more than Vendor/Family/Model */
	VULNBL_INTEL_STEPPINGS(COMETLAKE_L,	X86_STEPPINGS(0x0, 0x0),	MMIO | RETBLEED),
	VULNBL_INTEL	      (COMETLAKE_L,					MMIO | MMIO_SBDS | RETBLEED | GDS),

While that's functionally correct, it breaks the visual sorting, which
is confusing and even a bit dangerous.  One would reasonably expect the
COMETLAKE_L entries to come immediately after COMETLAKE, so it would be
quite possible for somebody to come along later and add a new
COMETLAKE_L there which conflicts with the later entries.

I'd much rather leave the STEPPINGS entry in the original list where it
belongs.  Something like:

	...
	VULNBL_INTEL(ICELAKE_L,			MMIO | MMIO_SBDS | RETBLEED | GDS),
	VULNBL_INTEL(ICELAKE_D,			MMIO | GDS),
	VULNBL_INTEL(ICELAKE_X,			MMIO | GDS),
	VULNBL_INTEL(COMETLAKE,			MMIO | MMIO_SBDS | RETBLEED | GDS),
	VULNBL_INTEL_STEPPINGS(COMETLAKE_L,
			       X86_STEPPINGS(0x0, 0x0),
			       MMIO | RETBLEED),
	VULNBL_INTEL(COMETLAKE_L,		MMIO | MMIO_SBDS | RETBLEED | GDS),
	VULNBL_INTEL(TIGERLAKE_L,		GDS),
	VULNBL_INTEL(TIGERLAKE,			GDS),
	...

Yes, that's a little ugly, but at least the sorting is correct so it's
less confusing and more robust overall.

-- 
Josh

Re: [PATCH PATCH v2 9/9] x86/rfds: Exclude P-only parts from the RFDS affected list

[Josh Poimboeuf]

On Thu, Jun 27, 2024 at 01:44:55PM -0700, Pawan Gupta wrote:
> @@ -1255,9 +1260,7 @@ static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
>  	VULNBL_INTEL(TIGERLAKE,			GDS),
>  	VULNBL_INTEL(LAKEFIELD,			MMIO | MMIO_SBDS | RETBLEED),
>  	VULNBL_INTEL(ROCKETLAKE,		MMIO | RETBLEED | GDS),
> -	VULNBL_INTEL(ALDERLAKE,			RFDS),
>  	VULNBL_INTEL(ALDERLAKE_L,		RFDS),
> -	VULNBL_INTEL(RAPTORLAKE,		RFDS),
>  	VULNBL_INTEL(RAPTORLAKE_P,		RFDS),
>  	VULNBL_INTEL(RAPTORLAKE_S,		RFDS),
>  	VULNBL_INTEL(ATOM_GRACEMONT,		RFDS),
> @@ -1271,6 +1274,8 @@ static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
>  	/* Match more than Vendor/Family/Model */
>  	VULNBL_INTEL_STEPPINGS(COMETLAKE_L,	X86_STEPPINGS(0x0, 0x0),	MMIO | RETBLEED),
>  	VULNBL_INTEL	      (COMETLAKE_L,					MMIO | MMIO_SBDS | RETBLEED | GDS),
> +	VULNBL_INTEL_TYPE     (ALDERLAKE,	ATOM,				RFDS),
> +	VULNBL_INTEL_TYPE     (RAPTORLAKE,	ATOM,				RFDS),

Same comment here, these should be inline with the main list.  Maybe
there's some way to structure the indentations so they align better
vertically with the STEPPINGS/TYPE variants.

-- 
Josh

Re: [PATCH PATCH v2 8/9] x86/bugs: Declutter vulnerable CPU list

[Pawan Gupta]

On Tue, Jul 02, 2024 at 06:00:18PM -0700, Josh Poimboeuf wrote:
> On Thu, Jun 27, 2024 at 01:44:48PM -0700, Pawan Gupta wrote:
> > The affected processor table has a lot of repetition and redundant
> > information that can be omitted. For example:
> > 
> >   VULNBL_INTEL_STEPPINGS(INTEL_IVYBRIDGE,		X86_STEPPING_ANY,		SRBDS),
> > 
> > can easily be simplified to:
> > 
> >   VULNBL_INTEL(IVYBRIDGE,	SRBDS),
> > 
> > Apply this to all the entries in the affected processor table.
> > 
> > No functional change. Disassembly of arch/x86/kernel/cpu/common.o does not
> > show any difference before and after the change.
> 
> This patch only changes data, not code.  So there's not much point in
> diffing the disassembly ;-)

You are right.

> A diff of the .init.rodata sections actually shows one (non-functional)
> difference in cpu_vuln_blacklist[].
> 
> The COMETLAKE_L entries were moved to a new section below the rest of
> the entries:
> 
> 	/* Match more than Vendor/Family/Model */
> 	VULNBL_INTEL_STEPPINGS(COMETLAKE_L,	X86_STEPPINGS(0x0, 0x0),	MMIO | RETBLEED),
> 	VULNBL_INTEL	      (COMETLAKE_L,					MMIO | MMIO_SBDS | RETBLEED | GDS),
> 
> While that's functionally correct, it breaks the visual sorting, which
> is confusing and even a bit dangerous.  One would reasonably expect the
> COMETLAKE_L entries to come immediately after COMETLAKE, so it would be
> quite possible for somebody to come along later and add a new
> COMETLAKE_L there which conflicts with the later entries.
> 
> I'd much rather leave the STEPPINGS entry in the original list where it
> belongs.  Something like:
> 
> 	...
> 	VULNBL_INTEL(ICELAKE_L,			MMIO | MMIO_SBDS | RETBLEED | GDS),
> 	VULNBL_INTEL(ICELAKE_D,			MMIO | GDS),
> 	VULNBL_INTEL(ICELAKE_X,			MMIO | GDS),
> 	VULNBL_INTEL(COMETLAKE,			MMIO | MMIO_SBDS | RETBLEED | GDS),
> 	VULNBL_INTEL_STEPPINGS(COMETLAKE_L,
> 			       X86_STEPPINGS(0x0, 0x0),
> 			       MMIO | RETBLEED),
> 	VULNBL_INTEL(COMETLAKE_L,		MMIO | MMIO_SBDS | RETBLEED | GDS),
> 	VULNBL_INTEL(TIGERLAKE_L,		GDS),
> 	VULNBL_INTEL(TIGERLAKE,			GDS),
> 	...
> 
> Yes, that's a little ugly, but at least the sorting is correct so it's
> less confusing and more robust overall.

That makes sense, I will make that change.

Re: [PATCH PATCH v2 9/9] x86/rfds: Exclude P-only parts from the RFDS affected list

[Pawan Gupta]

On Tue, Jul 02, 2024 at 06:04:33PM -0700, Josh Poimboeuf wrote:
> On Thu, Jun 27, 2024 at 01:44:55PM -0700, Pawan Gupta wrote:
> > @@ -1255,9 +1260,7 @@ static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
> >  	VULNBL_INTEL(TIGERLAKE,			GDS),
> >  	VULNBL_INTEL(LAKEFIELD,			MMIO | MMIO_SBDS | RETBLEED),
> >  	VULNBL_INTEL(ROCKETLAKE,		MMIO | RETBLEED | GDS),
> > -	VULNBL_INTEL(ALDERLAKE,			RFDS),
> >  	VULNBL_INTEL(ALDERLAKE_L,		RFDS),
> > -	VULNBL_INTEL(RAPTORLAKE,		RFDS),
> >  	VULNBL_INTEL(RAPTORLAKE_P,		RFDS),
> >  	VULNBL_INTEL(RAPTORLAKE_S,		RFDS),
> >  	VULNBL_INTEL(ATOM_GRACEMONT,		RFDS),
> > @@ -1271,6 +1274,8 @@ static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
> >  	/* Match more than Vendor/Family/Model */
> >  	VULNBL_INTEL_STEPPINGS(COMETLAKE_L,	X86_STEPPINGS(0x0, 0x0),	MMIO | RETBLEED),
> >  	VULNBL_INTEL	      (COMETLAKE_L,					MMIO | MMIO_SBDS | RETBLEED | GDS),
> > +	VULNBL_INTEL_TYPE     (ALDERLAKE,	ATOM,				RFDS),
> > +	VULNBL_INTEL_TYPE     (RAPTORLAKE,	ATOM,				RFDS),
> 
> Same comment here, these should be inline with the main list.  Maybe
> there's some way to structure the indentations so they align better
> vertically with the STEPPINGS/TYPE variants.

This is how it is turning out to be:

---
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 151c2377df21..75bbdf0cf8ae 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1237,45 +1237,43 @@ static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = {
 #define RFDS		BIT(7)
 
 static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
-	VULNBL_INTEL(IVYBRIDGE,			SRBDS),
-	VULNBL_INTEL(HASWELL,			SRBDS),
-	VULNBL_INTEL(HASWELL_L,			SRBDS),
-	VULNBL_INTEL(HASWELL_G,			SRBDS),
-	VULNBL_INTEL(HASWELL_X,			MMIO),
-	VULNBL_INTEL(BROADWELL_D,		MMIO),
-	VULNBL_INTEL(BROADWELL_G,		SRBDS),
-	VULNBL_INTEL(BROADWELL_X,		MMIO),
-	VULNBL_INTEL(BROADWELL,			SRBDS),
-	VULNBL_INTEL(SKYLAKE_X,			MMIO | RETBLEED | GDS),
-	VULNBL_INTEL(SKYLAKE_L,			MMIO | RETBLEED | GDS | SRBDS),
-	VULNBL_INTEL(SKYLAKE,			MMIO | RETBLEED | GDS | SRBDS),
-	VULNBL_INTEL(KABYLAKE_L,		MMIO | RETBLEED | GDS | SRBDS),
-	VULNBL_INTEL(KABYLAKE,			MMIO | RETBLEED | GDS | SRBDS),
-	VULNBL_INTEL(CANNONLAKE_L,		RETBLEED),
-	VULNBL_INTEL(ICELAKE_L,			MMIO | MMIO_SBDS | RETBLEED | GDS),
-	VULNBL_INTEL(ICELAKE_D,			MMIO | GDS),
-	VULNBL_INTEL(ICELAKE_X,			MMIO | GDS),
-	VULNBL_INTEL(COMETLAKE,			MMIO | MMIO_SBDS | RETBLEED | GDS),
-	VULNBL_INTEL(TIGERLAKE_L,		GDS),
-	VULNBL_INTEL(TIGERLAKE,			GDS),
-	VULNBL_INTEL(LAKEFIELD,			MMIO | MMIO_SBDS | RETBLEED),
-	VULNBL_INTEL(ROCKETLAKE,		MMIO | RETBLEED | GDS),
-	VULNBL_INTEL(ALDERLAKE_L,		RFDS),
-	VULNBL_INTEL(RAPTORLAKE_P,		RFDS),
-	VULNBL_INTEL(RAPTORLAKE_S,		RFDS),
-	VULNBL_INTEL(ATOM_GRACEMONT,		RFDS),
-	VULNBL_INTEL(ATOM_TREMONT,		MMIO | MMIO_SBDS | RFDS),
-	VULNBL_INTEL(ATOM_TREMONT_D,		MMIO | RFDS),
-	VULNBL_INTEL(ATOM_TREMONT_L,		MMIO | MMIO_SBDS | RFDS),
-	VULNBL_INTEL(ATOM_GOLDMONT,		RFDS),
-	VULNBL_INTEL(ATOM_GOLDMONT_D,		RFDS),
-	VULNBL_INTEL(ATOM_GOLDMONT_PLUS,	RFDS),
-
-	/* Match more than Vendor/Family/Model */
-	VULNBL_INTEL_STEPPINGS(COMETLAKE_L,	X86_STEPPINGS(0x0, 0x0),	MMIO | RETBLEED),
-	VULNBL_INTEL	      (COMETLAKE_L,					MMIO | MMIO_SBDS | RETBLEED | GDS),
-	VULNBL_INTEL_TYPE     (ALDERLAKE,	ATOM,				RFDS),
-	VULNBL_INTEL_TYPE     (RAPTORLAKE,	ATOM,				RFDS),
+	VULNBL_INTEL(		IVYBRIDGE,		SRBDS),
+	VULNBL_INTEL(		HASWELL,		SRBDS),
+	VULNBL_INTEL(		HASWELL_L,		SRBDS),
+	VULNBL_INTEL(		HASWELL_G,		SRBDS),
+	VULNBL_INTEL(		HASWELL_X,		MMIO),
+	VULNBL_INTEL(		BROADWELL_D,		MMIO),
+	VULNBL_INTEL(		BROADWELL_G,		SRBDS),
+	VULNBL_INTEL(		BROADWELL_X,		MMIO),
+	VULNBL_INTEL(		BROADWELL,		SRBDS),
+	VULNBL_INTEL(		SKYLAKE_X,		MMIO | RETBLEED | GDS),
+	VULNBL_INTEL(		SKYLAKE_L,		MMIO | RETBLEED | GDS | SRBDS),
+	VULNBL_INTEL(		SKYLAKE,		MMIO | RETBLEED | GDS | SRBDS),
+	VULNBL_INTEL(		KABYLAKE_L,		MMIO | RETBLEED | GDS | SRBDS),
+	VULNBL_INTEL(		KABYLAKE,		MMIO | RETBLEED | GDS | SRBDS),
+	VULNBL_INTEL(		CANNONLAKE_L,		RETBLEED),
+	VULNBL_INTEL(		ICELAKE_L,		MMIO | MMIO_SBDS | RETBLEED | GDS),
+	VULNBL_INTEL(		ICELAKE_D,		MMIO | GDS),
+	VULNBL_INTEL(		ICELAKE_X,		MMIO | GDS),
+	VULNBL_INTEL(		COMETLAKE,		MMIO | MMIO_SBDS | RETBLEED | GDS),
+	VULNBL_INTEL_STEPPINGS(	COMETLAKE_L,	X86_STEPPINGS(0x0, 0x0),	MMIO | RETBLEED),
+	VULNBL_INTEL(		COMETLAKE_L,		MMIO | MMIO_SBDS | RETBLEED | GDS),
+	VULNBL_INTEL(		TIGERLAKE_L,		GDS),
+	VULNBL_INTEL(		TIGERLAKE,		GDS),
+	VULNBL_INTEL(		LAKEFIELD,		MMIO | MMIO_SBDS | RETBLEED),
+	VULNBL_INTEL(		ROCKETLAKE,		MMIO | RETBLEED | GDS),
+	VULNBL_INTEL_TYPE(	ALDERLAKE,	ATOM,	RFDS),
+	VULNBL_INTEL(		ALDERLAKE_L,		RFDS),
+	VULNBL_INTEL_TYPE(	RAPTORLAKE,	ATOM,	RFDS),
+	VULNBL_INTEL(		RAPTORLAKE_P,		RFDS),
+	VULNBL_INTEL(		RAPTORLAKE_S,		RFDS),
+	VULNBL_INTEL(		ATOM_GRACEMONT,		RFDS),
+	VULNBL_INTEL(		ATOM_TREMONT,		MMIO | MMIO_SBDS | RFDS),
+	VULNBL_INTEL(		ATOM_TREMONT_D,		MMIO | RFDS),
+	VULNBL_INTEL(		ATOM_TREMONT_L,		MMIO | MMIO_SBDS | RFDS),
+	VULNBL_INTEL(		ATOM_GOLDMONT,		RFDS),
+	VULNBL_INTEL(		ATOM_GOLDMONT_D,	RFDS),
+	VULNBL_INTEL(		ATOM_GOLDMONT_PLUS,	RFDS),
 
 	VULNBL_AMD(0x15, RETBLEED),
 	VULNBL_AMD(0x16, RETBLEED),

Re: [PATCH PATCH v2 9/9] x86/rfds: Exclude P-only parts from the RFDS affected list

[Josh Poimboeuf]

On Wed, Jul 03, 2024 at 11:26:13AM -0700, Pawan Gupta wrote:
> On Tue, Jul 02, 2024 at 06:04:33PM -0700, Josh Poimboeuf wrote:
> > Same comment here, these should be inline with the main list.  Maybe
> > there's some way to structure the indentations so they align better
> > vertically with the STEPPINGS/TYPE variants.
> 
> This is how it is turning out to be:
> +	VULNBL_INTEL(		IVYBRIDGE,		SRBDS),
> +	VULNBL_INTEL(		HASWELL,		SRBDS),
> +	VULNBL_INTEL(		HASWELL_L,		SRBDS),
> +	VULNBL_INTEL(		HASWELL_G,		SRBDS),
> +	VULNBL_INTEL(		HASWELL_X,		MMIO),
> +	VULNBL_INTEL(		BROADWELL_D,		MMIO),
> +	VULNBL_INTEL(		BROADWELL_G,		SRBDS),
> +	VULNBL_INTEL(		BROADWELL_X,		MMIO),
> +	VULNBL_INTEL(		BROADWELL,		SRBDS),
> +	VULNBL_INTEL(		SKYLAKE_X,		MMIO | RETBLEED | GDS),
> +	VULNBL_INTEL(		SKYLAKE_L,		MMIO | RETBLEED | GDS | SRBDS),
> +	VULNBL_INTEL(		SKYLAKE,		MMIO | RETBLEED | GDS | SRBDS),
> +	VULNBL_INTEL(		KABYLAKE_L,		MMIO | RETBLEED | GDS | SRBDS),
> +	VULNBL_INTEL(		KABYLAKE,		MMIO | RETBLEED | GDS | SRBDS),
> +	VULNBL_INTEL(		CANNONLAKE_L,		RETBLEED),
> +	VULNBL_INTEL(		ICELAKE_L,		MMIO | MMIO_SBDS | RETBLEED | GDS),
> +	VULNBL_INTEL(		ICELAKE_D,		MMIO | GDS),
> +	VULNBL_INTEL(		ICELAKE_X,		MMIO | GDS),
> +	VULNBL_INTEL(		COMETLAKE,		MMIO | MMIO_SBDS | RETBLEED | GDS),
> +	VULNBL_INTEL_STEPPINGS(	COMETLAKE_L,	X86_STEPPINGS(0x0, 0x0),	MMIO | RETBLEED),
> +	VULNBL_INTEL(		COMETLAKE_L,		MMIO | MMIO_SBDS | RETBLEED | GDS),
> +	VULNBL_INTEL(		TIGERLAKE_L,		GDS),
> +	VULNBL_INTEL(		TIGERLAKE,		GDS),
> +	VULNBL_INTEL(		LAKEFIELD,		MMIO | MMIO_SBDS | RETBLEED),
> +	VULNBL_INTEL(		ROCKETLAKE,		MMIO | RETBLEED | GDS),
> +	VULNBL_INTEL_TYPE(	ALDERLAKE,	ATOM,	RFDS),
> +	VULNBL_INTEL(		ALDERLAKE_L,		RFDS),
> +	VULNBL_INTEL_TYPE(	RAPTORLAKE,	ATOM,	RFDS),
> +	VULNBL_INTEL(		RAPTORLAKE_P,		RFDS),
> +	VULNBL_INTEL(		RAPTORLAKE_S,		RFDS),
> +	VULNBL_INTEL(		ATOM_GRACEMONT,		RFDS),
> +	VULNBL_INTEL(		ATOM_TREMONT,		MMIO | MMIO_SBDS | RFDS),
> +	VULNBL_INTEL(		ATOM_TREMONT_D,		MMIO | RFDS),
> +	VULNBL_INTEL(		ATOM_TREMONT_L,		MMIO | MMIO_SBDS | RFDS),
> +	VULNBL_INTEL(		ATOM_GOLDMONT,		RFDS),
> +	VULNBL_INTEL(		ATOM_GOLDMONT_D,	RFDS),
> +	VULNBL_INTEL(		ATOM_GOLDMONT_PLUS,	RFDS),

Looks good to me.

The bug bits could also be vertically aligned, but that might offend
those with undersized terminals :-)

-- 
Josh

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

[Borislav Petkov]

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.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Pawan Gupta]

On Thu, Jul 04, 2024 at 01:07:24AM +0200, Borislav Petkov wrote:
> 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.

Ok.

> > > > +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.

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

  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.

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

[Borislav Petkov]

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.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette