[PATCH 0/5] x86/cpu: Remove duplicate microcode version matching infrastructure

[PATCH 0/5] x86/cpu: Remove duplicate microcode version matching infrastructure

[Dave Hansen]

Changes from RFC:
 * Convert stepping match helpers to always take a range and never
   take a raw stepping bitmap. - Ingo

--

x86 has generic CPU matching infrastructure. This lets you build
tables of CPUs with some property.  It's mostly used for enumerating
model-specific features, but it is quite a bit more flexible than
that. In includes a facility to match steppings and microcode
versions. This generic infrastructure is built around 'struct
x86_cpu_id'.

There is a less generic, parallel CPU matching facility built around
'struct x86_cpu_desc'. It is used only for matching specific microcode
revisions.  All of the 'struct x86_cpu_desc' users can be converted to
'struct x86_cpu_id'.

Do that conversion then remove the 'struct x86_cpu_desc'
infrastructure.

Testing or acks would be much appreciated!

--

 arch/x86/events/intel/core.c         |   63 +++++++++++--------------
 arch/x86/include/asm/cpu_device_id.h |   51 +++-----------------
 arch/x86/kernel/apic/apic.c          |   18 +++----
 arch/x86/kernel/cpu/amd.c            |    9 +--
 arch/x86/kernel/cpu/common.c         |   78 ++++++++++++++++----------------
 arch/x86/kernel/cpu/match.c          |   28 +----------
 drivers/edac/i10nm_base.c            |   20 ++++----
 drivers/edac/skx_base.c              |    2
 include/linux/mod_devicetable.h      |    2
 9 files changed, 105 insertions(+), 166 deletions(-)

[PATCH 1/5] x86/cpu: Introduce new microcode matching helper

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

The 'x86_cpu_id' and 'x86_cpu_desc' structures are very similar and
need to be consolidated.  There is a microcode version matching
function for 'x86_cpu_desc' but not 'x86_cpu_id'.

Create one for 'x86_cpu_id'.

This essentially just leverages the x86_cpu_id->driver_data field
to replace the less generic x86_cpu_desc->x86_microcode_rev field.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/arch/x86/include/asm/cpu_device_id.h |    1 +
 b/arch/x86/kernel/cpu/match.c          |   11 +++++++++++
 2 files changed, 12 insertions(+)

diff -puN arch/x86/include/asm/cpu_device_id.h~min-ucode-rev arch/x86/include/asm/cpu_device_id.h
--- a/arch/x86/include/asm/cpu_device_id.h~min-ucode-rev	2024-12-06 11:33:15.663128241 -0800
+++ b/arch/x86/include/asm/cpu_device_id.h	2024-12-06 11:33:15.667128399 -0800
@@ -278,5 +278,6 @@ struct x86_cpu_desc {
 
 extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match);
 extern bool x86_cpu_has_min_microcode_rev(const struct x86_cpu_desc *table);
+extern bool x86_match_min_microcode_rev(const struct x86_cpu_id *table);
 
 #endif /* _ASM_X86_CPU_DEVICE_ID */
diff -puN arch/x86/kernel/cpu/match.c~min-ucode-rev arch/x86/kernel/cpu/match.c
--- a/arch/x86/kernel/cpu/match.c~min-ucode-rev	2024-12-06 11:33:15.663128241 -0800
+++ b/arch/x86/kernel/cpu/match.c	2024-12-06 11:33:15.667128399 -0800
@@ -86,3 +86,14 @@ bool x86_cpu_has_min_microcode_rev(const
 	return true;
 }
 EXPORT_SYMBOL_GPL(x86_cpu_has_min_microcode_rev);
+
+bool x86_match_min_microcode_rev(const struct x86_cpu_id *table)
+{
+	const struct x86_cpu_id *res = x86_match_cpu(table);
+
+	if (!res || res->driver_data > boot_cpu_data.microcode)
+		return false;
+
+	return true;
+}
+EXPORT_SYMBOL_GPL(x86_match_min_microcode_rev);
_

Re: [PATCH 1/5] x86/cpu: Introduce new microcode matching helper

[Chao Gao]

On Fri, Dec 06, 2024 at 11:38:31AM -0800, Dave Hansen wrote:
>
>From: Dave Hansen <dave.hansen@linux.intel.com>
>
>The 'x86_cpu_id' and 'x86_cpu_desc' structures are very similar and
>need to be consolidated.  There is a microcode version matching
>function for 'x86_cpu_desc' but not 'x86_cpu_id'.
>
>Create one for 'x86_cpu_id'.
>
>This essentially just leverages the x86_cpu_id->driver_data field
>to replace the less generic x86_cpu_desc->x86_microcode_rev field.
>
>Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
>---
>
> b/arch/x86/include/asm/cpu_device_id.h |    1 +
> b/arch/x86/kernel/cpu/match.c          |   11 +++++++++++
> 2 files changed, 12 insertions(+)
>
>diff -puN arch/x86/include/asm/cpu_device_id.h~min-ucode-rev arch/x86/include/asm/cpu_device_id.h
>--- a/arch/x86/include/asm/cpu_device_id.h~min-ucode-rev	2024-12-06 11:33:15.663128241 -0800
>+++ b/arch/x86/include/asm/cpu_device_id.h	2024-12-06 11:33:15.667128399 -0800
>@@ -278,5 +278,6 @@ struct x86_cpu_desc {
> 
> extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match);
> extern bool x86_cpu_has_min_microcode_rev(const struct x86_cpu_desc *table);
>+extern bool x86_match_min_microcode_rev(const struct x86_cpu_id *table);
> 
> #endif /* _ASM_X86_CPU_DEVICE_ID */
>diff -puN arch/x86/kernel/cpu/match.c~min-ucode-rev arch/x86/kernel/cpu/match.c
>--- a/arch/x86/kernel/cpu/match.c~min-ucode-rev	2024-12-06 11:33:15.663128241 -0800
>+++ b/arch/x86/kernel/cpu/match.c	2024-12-06 11:33:15.667128399 -0800
>@@ -86,3 +86,14 @@ bool x86_cpu_has_min_microcode_rev(const
> 	return true;
> }
> EXPORT_SYMBOL_GPL(x86_cpu_has_min_microcode_rev);
>+
>+bool x86_match_min_microcode_rev(const struct x86_cpu_id *table)
>+{
>+	const struct x86_cpu_id *res = x86_match_cpu(table);
>+
>+	if (!res || res->driver_data > boot_cpu_data.microcode)
>+		return false;
>+
>+	return true;

Maybe we can simplify the logic to:

	return res && res->driver_data <= boot_cpu_data.microcode;

>+}
>+EXPORT_SYMBOL_GPL(x86_match_min_microcode_rev);
>_

Re: [PATCH 1/5] x86/cpu: Introduce new microcode matching helper

[Dave Hansen]

On 12/9/24 21:46, Chao Gao wrote:
>> +bool x86_match_min_microcode_rev(const struct x86_cpu_id *table)
>> +{
>> +	const struct x86_cpu_id *res = x86_match_cpu(table);
>> +
>> +	if (!res || res->driver_data > boot_cpu_data.microcode)
>> +		return false;
>> +
>> +	return true;
> Maybe we can simplify the logic to:
> 
> 	return res && res->driver_data <= boot_cpu_data.microcode;

So, yeah, it can be made shorter.

But it's 100% a style thing and I'm not at all in the camp of fewer
lines meaning better code. It's all short enough to see without even
really moving your eyeballs so it's short _enough_ for sure.  There's
only one line of real logic either way.

[PATCH 2/5] x86/cpu: Expose only stepping min/max interface

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

The x86_match_cpu() infrastructure can match CPU steppings. Since
there are only 16 possible steppings, the matching infrastructure goes
all out and stores the stepping match as a bitmap. That means it can
match any possible steppings in a single list entry. Fun.

But it exposes this bitmap to each of the X86_MATCH_*() helpers when
none of them really need a bitmap. It makes up for this by exporting a
helper (X86_STEPPINGS()) which converts a contiguous stepping range
into the bitmap which every single user leverages.

Instead of a bitmap, have the main helper for this sort of thing
(X86_MATCH_VFM_STEPPING()) just take a stepping range. This ends up
actually being even more compact than before.

Leave the helper in place (renamed to __X86_STEPPINGS()) to make it
more clear what is going on instead of just having a random GENMASK()
in the middle of an already complicated macro.

One oddity that I hit was this macro:

#define VULNBL_INTEL_STEPPINGS(vfm, max_stepping, issues)                 \
       X86_MATCH_VFM_STEPPINGS(vfm, X86_STEPPING_MIN, max_stepping, issues)

It *could* have been converted over to take a min/max stepping value
for each entry. But that would have been a bit too verbose and would
prevent the one oddball in the list (INTEL_COMETLAKE_L stepping 0)
from sticking out.

Instead, just have it take a *maximum* stepping and imply that the match
is from 0=>max_stepping. This is functional for all the cases now and
also retains the nice property of having INTEL_COMETLAKE_L stepping 0
stick out like a sore thumb.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Suggested-by: Ingo Molnar <mingo@kernel.org>
---

 b/arch/x86/include/asm/cpu_device_id.h |   15 +++---
 b/arch/x86/kernel/apic/apic.c          |   18 +++----
 b/arch/x86/kernel/cpu/common.c         |   78 ++++++++++++++++-----------------
 b/drivers/edac/i10nm_base.c            |   20 ++++----
 b/drivers/edac/skx_base.c              |    2 
 b/include/linux/mod_devicetable.h      |    2 
 6 files changed, 69 insertions(+), 66 deletions(-)

diff -puN arch/x86/include/asm/cpu_device_id.h~zap-X86_STEPPINGS arch/x86/include/asm/cpu_device_id.h
--- a/arch/x86/include/asm/cpu_device_id.h~zap-X86_STEPPINGS	2024-12-06 11:33:16.179148524 -0800
+++ b/arch/x86/include/asm/cpu_device_id.h	2024-12-06 11:33:16.191148995 -0800
@@ -56,7 +56,6 @@
 /* x86_cpu_id::flags */
 #define X86_CPU_ID_FLAG_ENTRY_VALID	BIT(0)
 
-#define X86_STEPPINGS(mins, maxs)    GENMASK(maxs, mins)
 /**
  * X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE - Base macro for CPU matching
  * @_vendor:	The vendor name, e.g. INTEL, AMD, HYGON, ..., ANY
@@ -208,6 +207,7 @@
 		VFM_MODEL(vfm),				\
 		X86_STEPPING_ANY, X86_FEATURE_ANY, data)
 
+#define __X86_STEPPINGS(mins, maxs)    GENMASK(maxs, mins)
 /**
  * X86_MATCH_VFM_STEPPINGS - Match encoded vendor/family/model/stepping
  * @vfm:	Encoded 8-bits each for vendor, family, model
@@ -218,12 +218,13 @@
  *
  * 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, min_step, max_step, data)	\
+	X86_MATCH_VENDORID_FAM_MODEL_STEPPINGS_FEATURE(		\
+		VFM_VENDOR(vfm),				\
+		VFM_FAMILY(vfm),				\
+		VFM_MODEL(vfm),					\
+		__X86_STEPPINGS(min_step, max_step),		\
+		X86_FEATURE_ANY, data)
 
 /**
  * X86_MATCH_VFM_FEATURE - Match encoded vendor/family/model/feature
diff -puN arch/x86/kernel/apic/apic.c~zap-X86_STEPPINGS arch/x86/kernel/apic/apic.c
--- a/arch/x86/kernel/apic/apic.c~zap-X86_STEPPINGS	2024-12-06 11:33:16.183148680 -0800
+++ b/arch/x86/kernel/apic/apic.c	2024-12-06 11:33:16.191148995 -0800
@@ -509,19 +509,19 @@ static struct clock_event_device lapic_c
 static DEFINE_PER_CPU(struct clock_event_device, lapic_events);
 
 static const struct x86_cpu_id deadline_match[] __initconst = {
-	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_X, X86_STEPPINGS(0x2, 0x2), 0x3a), /* EP */
-	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_X, X86_STEPPINGS(0x4, 0x4), 0x0f), /* EX */
+	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_X, 0x2, 0x2, 0x3a), /* EP */
+	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_X, 0x4, 0x4, 0x0f), /* EX */
 
 	X86_MATCH_VFM(INTEL_BROADWELL_X,	0x0b000020),
 
-	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D, X86_STEPPINGS(0x2, 0x2), 0x00000011),
-	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D, X86_STEPPINGS(0x3, 0x3), 0x0700000e),
-	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D, X86_STEPPINGS(0x4, 0x4), 0x0f00000c),
-	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D, X86_STEPPINGS(0x5, 0x5), 0x0e000003),
+	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D, 0x2, 0x2, 0x00000011),
+	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D, 0x3, 0x3, 0x0700000e),
+	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D, 0x4, 0x4, 0x0f00000c),
+	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D, 0x5, 0x5, 0x0e000003),
 
-	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X, X86_STEPPINGS(0x3, 0x3), 0x01000136),
-	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X, X86_STEPPINGS(0x4, 0x4), 0x02000014),
-	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X, X86_STEPPINGS(0x5, 0xf), 0),
+	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X, 0x3, 0x3, 0x01000136),
+	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X, 0x4, 0x4, 0x02000014),
+	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X, 0x5, 0xf, 0),
 
 	X86_MATCH_VFM(INTEL_HASWELL,		0x22),
 	X86_MATCH_VFM(INTEL_HASWELL_L,		0x20),
diff -puN arch/x86/kernel/cpu/common.c~zap-X86_STEPPINGS arch/x86/kernel/cpu/common.c
--- a/arch/x86/kernel/cpu/common.c~zap-X86_STEPPINGS	2024-12-06 11:33:16.183148680 -0800
+++ b/arch/x86/kernel/cpu/common.c	2024-12-06 11:33:16.191148995 -0800
@@ -1201,8 +1201,8 @@ static const __initconst struct x86_cpu_
 #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_STEPPINGS(vfm, max_stepping, issues)		   \
+	X86_MATCH_VFM_STEPPINGS(vfm, X86_STEPPING_MIN, max_stepping, issues)
 
 #define VULNBL_AMD(family, blacklist)		\
 	VULNBL(AMD, family, X86_MODEL_ANY, blacklist)
@@ -1227,43 +1227,43 @@ static const __initconst struct x86_cpu_
 #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_STEPPINGS(INTEL_IVYBRIDGE,		X86_STEPPING_MAX,		SRBDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_HASWELL,		X86_STEPPING_MAX,		SRBDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_HASWELL_L,		X86_STEPPING_MAX,		SRBDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_HASWELL_G,		X86_STEPPING_MAX,		SRBDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_HASWELL_X,		X86_STEPPING_MAX,		MMIO),
+	VULNBL_INTEL_STEPPINGS(INTEL_BROADWELL_D,	X86_STEPPING_MAX,		MMIO),
+	VULNBL_INTEL_STEPPINGS(INTEL_BROADWELL_G,	X86_STEPPING_MAX,		SRBDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_BROADWELL_X,	X86_STEPPING_MAX,		MMIO),
+	VULNBL_INTEL_STEPPINGS(INTEL_BROADWELL,		X86_STEPPING_MAX,		SRBDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_SKYLAKE_X,		X86_STEPPING_MAX,		MMIO | RETBLEED | GDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_SKYLAKE_L,		X86_STEPPING_MAX,		MMIO | RETBLEED | GDS | SRBDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_SKYLAKE,		X86_STEPPING_MAX,		MMIO | RETBLEED | GDS | SRBDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_KABYLAKE_L,	X86_STEPPING_MAX,		MMIO | RETBLEED | GDS | SRBDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_KABYLAKE,		X86_STEPPING_MAX,		MMIO | RETBLEED | GDS | SRBDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_CANNONLAKE_L,	X86_STEPPING_MAX,		RETBLEED),
+	VULNBL_INTEL_STEPPINGS(INTEL_ICELAKE_L,		X86_STEPPING_MAX,		MMIO | MMIO_SBDS | RETBLEED | GDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_ICELAKE_D,		X86_STEPPING_MAX,		MMIO | GDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_ICELAKE_X,		X86_STEPPING_MAX,		MMIO | GDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_COMETLAKE,		X86_STEPPING_MAX,		MMIO | MMIO_SBDS | RETBLEED | GDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_COMETLAKE_L,	0x0,				MMIO | RETBLEED),
+	VULNBL_INTEL_STEPPINGS(INTEL_COMETLAKE_L,	X86_STEPPING_MAX,		MMIO | MMIO_SBDS | RETBLEED | GDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_TIGERLAKE_L,	X86_STEPPING_MAX,		GDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_TIGERLAKE,		X86_STEPPING_MAX,		GDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_LAKEFIELD,		X86_STEPPING_MAX,		MMIO | MMIO_SBDS | RETBLEED),
+	VULNBL_INTEL_STEPPINGS(INTEL_ROCKETLAKE,	X86_STEPPING_MAX,		MMIO | RETBLEED | GDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_ALDERLAKE,		X86_STEPPING_MAX,		RFDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_ALDERLAKE_L,	X86_STEPPING_MAX,		RFDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_RAPTORLAKE,	X86_STEPPING_MAX,		RFDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_RAPTORLAKE_P,	X86_STEPPING_MAX,		RFDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_RAPTORLAKE_S,	X86_STEPPING_MAX,		RFDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_GRACEMONT,	X86_STEPPING_MAX,		RFDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_TREMONT,	X86_STEPPING_MAX,		MMIO | MMIO_SBDS | RFDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_TREMONT_D,	X86_STEPPING_MAX,		MMIO | RFDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_TREMONT_L,	X86_STEPPING_MAX,		MMIO | MMIO_SBDS | RFDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_GOLDMONT,	X86_STEPPING_MAX,		RFDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_GOLDMONT_D,	X86_STEPPING_MAX,		RFDS),
+	VULNBL_INTEL_STEPPINGS(INTEL_ATOM_GOLDMONT_PLUS, X86_STEPPING_MAX,		RFDS),
 
 	VULNBL_AMD(0x15, RETBLEED),
 	VULNBL_AMD(0x16, RETBLEED),
diff -puN drivers/edac/i10nm_base.c~zap-X86_STEPPINGS drivers/edac/i10nm_base.c
--- a/drivers/edac/i10nm_base.c~zap-X86_STEPPINGS	2024-12-06 11:33:16.187148838 -0800
+++ b/drivers/edac/i10nm_base.c	2024-12-06 11:33:16.191148995 -0800
@@ -938,16 +938,16 @@ static struct res_config gnr_cfg = {
 };
 
 static const struct x86_cpu_id i10nm_cpuids[] = {
-	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_TREMONT_D,	X86_STEPPINGS(0x0, 0x3), &i10nm_cfg0),
-	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_TREMONT_D,	X86_STEPPINGS(0x4, 0xf), &i10nm_cfg1),
-	X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_X,	X86_STEPPINGS(0x0, 0x3), &i10nm_cfg0),
-	X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_X,	X86_STEPPINGS(0x4, 0xf), &i10nm_cfg1),
-	X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_D,	X86_STEPPINGS(0x0, 0xf), &i10nm_cfg1),
-	X86_MATCH_VFM_STEPPINGS(INTEL_SAPPHIRERAPIDS_X,	X86_STEPPINGS(0x0, 0xf), &spr_cfg),
-	X86_MATCH_VFM_STEPPINGS(INTEL_EMERALDRAPIDS_X,	X86_STEPPINGS(0x0, 0xf), &spr_cfg),
-	X86_MATCH_VFM_STEPPINGS(INTEL_GRANITERAPIDS_X,	X86_STEPPINGS(0x0, 0xf), &gnr_cfg),
-	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT_X,	X86_STEPPINGS(0x0, 0xf), &gnr_cfg),
-	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT,	X86_STEPPINGS(0x0, 0xf), &gnr_cfg),
+	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_TREMONT_D,	0x0, 0x3, &i10nm_cfg0),
+	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_TREMONT_D,	0x4, 0xf, &i10nm_cfg1),
+	X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_X,	0x0, 0x3, &i10nm_cfg0),
+	X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_X,	0x4, 0xf, &i10nm_cfg1),
+	X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_D,	0x0, 0xf, &i10nm_cfg1),
+	X86_MATCH_VFM_STEPPINGS(INTEL_SAPPHIRERAPIDS_X,	0x0, 0xf, &spr_cfg),
+	X86_MATCH_VFM_STEPPINGS(INTEL_EMERALDRAPIDS_X,	0x0, 0xf, &spr_cfg),
+	X86_MATCH_VFM_STEPPINGS(INTEL_GRANITERAPIDS_X,	0x0, 0xf, &gnr_cfg),
+	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT_X,	0x0, 0xf, &gnr_cfg),
+	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT,	0x0, 0xf, &gnr_cfg),
 	{}
 };
 MODULE_DEVICE_TABLE(x86cpu, i10nm_cpuids);
diff -puN drivers/edac/skx_base.c~zap-X86_STEPPINGS drivers/edac/skx_base.c
--- a/drivers/edac/skx_base.c~zap-X86_STEPPINGS	2024-12-06 11:33:16.187148838 -0800
+++ b/drivers/edac/skx_base.c	2024-12-06 11:33:16.191148995 -0800
@@ -164,7 +164,7 @@ static struct res_config skx_cfg = {
 };
 
 static const struct x86_cpu_id skx_cpuids[] = {
-	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X, X86_STEPPINGS(0x0, 0xf), &skx_cfg),
+	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X, 0x0, 0xf, &skx_cfg),
 	{ }
 };
 MODULE_DEVICE_TABLE(x86cpu, skx_cpuids);
diff -puN include/linux/mod_devicetable.h~zap-X86_STEPPINGS include/linux/mod_devicetable.h
--- a/include/linux/mod_devicetable.h~zap-X86_STEPPINGS	2024-12-06 11:33:16.187148838 -0800
+++ b/include/linux/mod_devicetable.h	2024-12-06 11:33:16.191148995 -0800
@@ -700,6 +700,8 @@ struct x86_cpu_id {
 #define X86_FAMILY_ANY 0
 #define X86_MODEL_ANY  0
 #define X86_STEPPING_ANY 0
+#define X86_STEPPING_MIN 0
+#define X86_STEPPING_MAX 0xf
 #define X86_FEATURE_ANY 0	/* Same as FPU, you can't test for that */
 
 /*
_

Re: [PATCH 2/5] x86/cpu: Expose only stepping min/max interface

[Borislav Petkov]

On Fri, Dec 06, 2024 at 11:38:32AM -0800, Dave Hansen wrote:
> 
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> The x86_match_cpu() infrastructure can match CPU steppings. Since
> there are only 16 possible steppings, the matching infrastructure goes
> all out and stores the stepping match as a bitmap. That means it can
> match any possible steppings in a single list entry. Fun.
> 
> But it exposes this bitmap to each of the X86_MATCH_*() helpers when
> none of them really need a bitmap. It makes up for this by exporting a
> helper (X86_STEPPINGS()) which converts a contiguous stepping range
> into the bitmap which every single user leverages.
> 
> Instead of a bitmap, have the main helper for this sort of thing
> (X86_MATCH_VFM_STEPPING()) just take a stepping range. This ends up
> actually being even more compact than before.

Yap, better.

> Leave the helper in place (renamed to __X86_STEPPINGS()) to make it
> more clear what is going on instead of just having a random GENMASK()
> in the middle of an already complicated macro.
> 
> One oddity that I hit was this macro:
> 
> #define VULNBL_INTEL_STEPPINGS(vfm, max_stepping, issues)                 \
>        X86_MATCH_VFM_STEPPINGS(vfm, X86_STEPPING_MIN, max_stepping, issues)
> 
> It *could* have been converted over to take a min/max stepping value
> for each entry. But that would have been a bit too verbose and would
> prevent the one oddball in the list (INTEL_COMETLAKE_L stepping 0)
> from sticking out.
> 
> Instead, just have it take a *maximum* stepping and imply that the match
> is from 0=>max_stepping. This is functional for all the cases now and
> also retains the nice property of having INTEL_COMETLAKE_L stepping 0
> stick out like a sore thumb.
> 
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> Suggested-by: Ingo Molnar <mingo@kernel.org>
> ---
> 
>  b/arch/x86/include/asm/cpu_device_id.h |   15 +++---
>  b/arch/x86/kernel/apic/apic.c          |   18 +++----
>  b/arch/x86/kernel/cpu/common.c         |   78 ++++++++++++++++-----------------
>  b/drivers/edac/i10nm_base.c            |   20 ++++----
>  b/drivers/edac/skx_base.c              |    2 
>  b/include/linux/mod_devicetable.h      |    2 
>  6 files changed, 69 insertions(+), 66 deletions(-)

You missed a spot:

drivers/edac/i10nm_base.c:951:90: error: macro "X86_MATCH_VFM_STEPPINGS" requires 4 arguments, but only 3 given
  951 |         X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_DARKMONT_X,  X86_STEPPINGS(0x0, 0xf), &gnr_cfg),
      |                                                                                          ^
In file included from drivers/edac/i10nm_base.c:10:
./arch/x86/include/asm/cpu_device_id.h:221: note: macro "X86_MATCH_VFM_STEPPINGS" defined here
  221 | #define X86_MATCH_VFM_STEPPINGS(vfm, min_step, max_step, data)  \
      | 
drivers/edac/i10nm_base.c:951:9: error: ‘X86_MATCH_VFM_STEPPINGS’ undeclared here (not in a function)
  951 |         X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_DARKMONT_X,  X86_STEPPINGS(0x0, 0xf), &gnr_cfg),
      |         ^~~~~~~~~~~~~~~~~~~~~~~
make[4]: *** [scripts/Makefile.build:194: drivers/edac/i10nm_base.o] Error 1
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [scripts/Makefile.build:440: drivers/edac] Error 2
make[2]: *** [scripts/Makefile.build:440: drivers] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/mnt/kernel/kernel/2nd/linux/Makefile:1989: .] Error 2
make: *** [Makefile:251: __sub-make] Error 2

> diff -puN drivers/edac/i10nm_base.c~zap-X86_STEPPINGS drivers/edac/i10nm_base.c
> --- a/drivers/edac/i10nm_base.c~zap-X86_STEPPINGS	2024-12-06 11:33:16.187148838 -0800
> +++ b/drivers/edac/i10nm_base.c	2024-12-06 11:33:16.191148995 -0800
> @@ -938,16 +938,16 @@ static struct res_config gnr_cfg = {
>  };
>  
>  static const struct x86_cpu_id i10nm_cpuids[] = {
> -	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_TREMONT_D,	X86_STEPPINGS(0x0, 0x3), &i10nm_cfg0),
> -	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_TREMONT_D,	X86_STEPPINGS(0x4, 0xf), &i10nm_cfg1),
> -	X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_X,	X86_STEPPINGS(0x0, 0x3), &i10nm_cfg0),
> -	X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_X,	X86_STEPPINGS(0x4, 0xf), &i10nm_cfg1),
> -	X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_D,	X86_STEPPINGS(0x0, 0xf), &i10nm_cfg1),
> -	X86_MATCH_VFM_STEPPINGS(INTEL_SAPPHIRERAPIDS_X,	X86_STEPPINGS(0x0, 0xf), &spr_cfg),
> -	X86_MATCH_VFM_STEPPINGS(INTEL_EMERALDRAPIDS_X,	X86_STEPPINGS(0x0, 0xf), &spr_cfg),
> -	X86_MATCH_VFM_STEPPINGS(INTEL_GRANITERAPIDS_X,	X86_STEPPINGS(0x0, 0xf), &gnr_cfg),
> -	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT_X,	X86_STEPPINGS(0x0, 0xf), &gnr_cfg),
> -	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT,	X86_STEPPINGS(0x0, 0xf), &gnr_cfg),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_TREMONT_D,	0x0, 0x3, &i10nm_cfg0),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_TREMONT_D,	0x4, 0xf, &i10nm_cfg1),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_X,	0x0, 0x3, &i10nm_cfg0),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_X,	0x4, 0xf, &i10nm_cfg1),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_ICELAKE_D,	0x0, 0xf, &i10nm_cfg1),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_SAPPHIRERAPIDS_X,	0x0, 0xf, &spr_cfg),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_EMERALDRAPIDS_X,	0x0, 0xf, &spr_cfg),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_GRANITERAPIDS_X,	0x0, 0xf, &gnr_cfg),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT_X,	0x0, 0xf, &gnr_cfg),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT,	0x0, 0xf, &gnr_cfg),

Aren't those supposed to be:

	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT,	X86_STEPPING_MIN, X86_STEPPING_MAX, &gnr_cfg),

And while we're adding new defines, can we shorten them too?

	X86_MATCH_VFM_STP(INTEL_ATOM_CRESTMONT,	X86_STP_MIN, X86_STP_MAX, &gnr_cfg),

all that "STEPPING" is screaming at me! :-P

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 2/5] x86/cpu: Expose only stepping min/max interface

[Dave Hansen]

On 12/13/24 08:24, Borislav Petkov wrote:
> drivers/edac/i10nm_base.c:951:90: error: macro "X86_MATCH_VFM_STEPPINGS" requires 4 arguments, but only 3 given
>   951 |         X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_DARKMONT_X,  X86_STEPPINGS(0x0, 0xf), &gnr_cfg),
>       |                                                                                          ^

I'll fix that up.

>> +	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT_X,	0x0, 0xf, &gnr_cfg),
>> +	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT,	0x0, 0xf, &gnr_cfg),
> 
> Aren't those supposed to be:
> 
> 	X86_MATCH_VFM_STEPPINGS(INTEL_ATOM_CRESTMONT,	X86_STEPPING_MIN, X86_STEPPING_MAX, &gnr_cfg),
> 
> And while we're adding new defines, can we shorten them too?
> 
> 	X86_MATCH_VFM_STP(INTEL_ATOM_CRESTMONT,	X86_STP_MIN, X86_STP_MAX, &gnr_cfg),
> 
> all that "STEPPING" is screaming at me! :-P

I was trying to minimize the churn but that seems like a good thing to
add.  I'll also shorten the name.

Re: [PATCH 2/5] x86/cpu: Expose only stepping min/max interface

[Dave Hansen]

On 12/13/24 08:24, Borislav Petkov wrote:
> 
> And while we're adding new defines, can we shorten them too?
> 
> 	X86_MATCH_VFM_STP(INTEL_ATOM_CRESTMONT,	X86_STP_MIN, X86_STP_MAX, &gnr_cfg),

I did make these:

	X86_STEP_MIN/X86_STEP_MAX

The "STP" looks a little to TLA-ish to me. It's worth the extra byte to
have a real word.

[PATCH 3/5] x86/cpu: Replace PEBS use of 'x86_cpu_desc' use with 'x86_cpu_id'

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

The 'x86_cpu_desc' and 'x86_cpu_id' structures are very similar.
Reduce duplicate infrastructure by moving the few users of
'x86_cpu_id' to the much more common variant.

The existing X86_MATCH_VFM_STEPPINGS() helper matches ranges of
steppings. Instead of introducing a single-stepping match function
which could get confusing when paired with the range, just use
the stepping min/max match helper and use min==max.

Note that this makes the table more vertically compact because
multiple entries like this:

       INTEL_CPU_DESC(INTEL_SKYLAKE_X,          4, 0x00000000),
       INTEL_CPU_DESC(INTEL_SKYLAKE_X,          5, 0x00000000),
       INTEL_CPU_DESC(INTEL_SKYLAKE_X,          6, 0x00000000),
       INTEL_CPU_DESC(INTEL_SKYLAKE_X,          7, 0x00000000),

can be consolidated down to a single stepping range.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/arch/x86/events/intel/core.c |   63 +++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 36 deletions(-)

diff -puN arch/x86/events/intel/core.c~zap-x86_cpu_desc arch/x86/events/intel/core.c
--- a/arch/x86/events/intel/core.c~zap-x86_cpu_desc	2024-12-06 11:33:16.775171950 -0800
+++ b/arch/x86/events/intel/core.c	2024-12-06 11:33:16.779172107 -0800
@@ -5371,42 +5371,33 @@ static __init void intel_clovertown_quir
 	x86_pmu.pebs_constraints = NULL;
 }
 
-static const struct x86_cpu_desc isolation_ucodes[] = {
-	INTEL_CPU_DESC(INTEL_HASWELL,		 3, 0x0000001f),
-	INTEL_CPU_DESC(INTEL_HASWELL_L,		 1, 0x0000001e),
-	INTEL_CPU_DESC(INTEL_HASWELL_G,		 1, 0x00000015),
-	INTEL_CPU_DESC(INTEL_HASWELL_X,		 2, 0x00000037),
-	INTEL_CPU_DESC(INTEL_HASWELL_X,		 4, 0x0000000a),
-	INTEL_CPU_DESC(INTEL_BROADWELL,		 4, 0x00000023),
-	INTEL_CPU_DESC(INTEL_BROADWELL_G,	 1, 0x00000014),
-	INTEL_CPU_DESC(INTEL_BROADWELL_D,	 2, 0x00000010),
-	INTEL_CPU_DESC(INTEL_BROADWELL_D,	 3, 0x07000009),
-	INTEL_CPU_DESC(INTEL_BROADWELL_D,	 4, 0x0f000009),
-	INTEL_CPU_DESC(INTEL_BROADWELL_D,	 5, 0x0e000002),
-	INTEL_CPU_DESC(INTEL_BROADWELL_X,	 1, 0x0b000014),
-	INTEL_CPU_DESC(INTEL_SKYLAKE_X,		 3, 0x00000021),
-	INTEL_CPU_DESC(INTEL_SKYLAKE_X,		 4, 0x00000000),
-	INTEL_CPU_DESC(INTEL_SKYLAKE_X,		 5, 0x00000000),
-	INTEL_CPU_DESC(INTEL_SKYLAKE_X,		 6, 0x00000000),
-	INTEL_CPU_DESC(INTEL_SKYLAKE_X,		 7, 0x00000000),
-	INTEL_CPU_DESC(INTEL_SKYLAKE_X,		11, 0x00000000),
-	INTEL_CPU_DESC(INTEL_SKYLAKE_L,		 3, 0x0000007c),
-	INTEL_CPU_DESC(INTEL_SKYLAKE,		 3, 0x0000007c),
-	INTEL_CPU_DESC(INTEL_KABYLAKE,		 9, 0x0000004e),
-	INTEL_CPU_DESC(INTEL_KABYLAKE_L,	 9, 0x0000004e),
-	INTEL_CPU_DESC(INTEL_KABYLAKE_L,	10, 0x0000004e),
-	INTEL_CPU_DESC(INTEL_KABYLAKE_L,	11, 0x0000004e),
-	INTEL_CPU_DESC(INTEL_KABYLAKE_L,	12, 0x0000004e),
-	INTEL_CPU_DESC(INTEL_KABYLAKE,		10, 0x0000004e),
-	INTEL_CPU_DESC(INTEL_KABYLAKE,		11, 0x0000004e),
-	INTEL_CPU_DESC(INTEL_KABYLAKE,		12, 0x0000004e),
-	INTEL_CPU_DESC(INTEL_KABYLAKE,		13, 0x0000004e),
+static const struct x86_cpu_id isolation_ucodes[] = {
+	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL,		 3,  3, 0x0000001f),
+	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_L,	 1,  1, 0x0000001e),
+	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_G,	 1,  1, 0x00000015),
+	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_X,	 2,  2, 0x00000037),
+	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_X,	 4,  4, 0x0000000a),
+	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL,	 4,  4, 0x00000023),
+	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_G,	 1,  1, 0x00000014),
+	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D,	 2,  2, 0x00000010),
+	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D,	 3,  3, 0x07000009),
+	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D,	 4,  4, 0x0f000009),
+	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D,	 5,  5, 0x0e000002),
+	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_X,	 1,  1, 0x0b000014),
+	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X,	 3,  3, 0x00000021),
+	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X,	 4,  7, 0x00000000),
+	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X,	11, 11, 0x00000000),
+	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_L,	 3,  3, 0x0000007c),
+	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE,		 3,  3, 0x0000007c),
+	X86_MATCH_VFM_STEPPINGS(INTEL_KABYLAKE,		 9,  9, 0x0000004e),
+	X86_MATCH_VFM_STEPPINGS(INTEL_KABYLAKE_L,	 9, 12, 0x0000004e),
+	X86_MATCH_VFM_STEPPINGS(INTEL_KABYLAKE,		10, 13, 0x0000004e),
 	{}
 };
 
 static void intel_check_pebs_isolation(void)
 {
-	x86_pmu.pebs_no_isolation = !x86_cpu_has_min_microcode_rev(isolation_ucodes);
+	x86_pmu.pebs_no_isolation = !x86_match_min_microcode_rev(isolation_ucodes);
 }
 
 static __init void intel_pebs_isolation_quirk(void)
@@ -5416,16 +5407,16 @@ static __init void intel_pebs_isolation_
 	intel_check_pebs_isolation();
 }
 
-static const struct x86_cpu_desc pebs_ucodes[] = {
-	INTEL_CPU_DESC(INTEL_SANDYBRIDGE,	7, 0x00000028),
-	INTEL_CPU_DESC(INTEL_SANDYBRIDGE_X,	6, 0x00000618),
-	INTEL_CPU_DESC(INTEL_SANDYBRIDGE_X,	7, 0x0000070c),
+static const struct x86_cpu_id pebs_ucodes[] = {
+	X86_MATCH_VFM_STEPPINGS(INTEL_SANDYBRIDGE,	7, 7, 0x00000028),
+	X86_MATCH_VFM_STEPPINGS(INTEL_SANDYBRIDGE_X,	6, 6, 0x00000618),
+	X86_MATCH_VFM_STEPPINGS(INTEL_SANDYBRIDGE_X,	7, 7, 0x0000070c),
 	{}
 };
 
 static bool intel_snb_pebs_broken(void)
 {
-	return !x86_cpu_has_min_microcode_rev(pebs_ucodes);
+	return !x86_match_min_microcode_rev(pebs_ucodes);
 }
 
 static void intel_snb_check_microcode(void)
_

RE: [PATCH 3/5] x86/cpu: Replace PEBS use of 'x86_cpu_desc' use with 'x86_cpu_id'

[Luck, Tony]

> The 'x86_cpu_desc' and 'x86_cpu_id' structures are very similar.
> Reduce duplicate infrastructure by moving the few users of
> 'x86_cpu_id' to the much more common variant.

This paragraph is backwards. You are moving 'x86_cpu_desc to 'x86_cpu_id.

-Tony

Re: [PATCH 3/5] x86/cpu: Replace PEBS use of 'x86_cpu_desc' use with 'x86_cpu_id'

[Dave Hansen]

On 12/6/24 11:55, Luck, Tony wrote:
>> The 'x86_cpu_desc' and 'x86_cpu_id' structures are very similar.
>> Reduce duplicate infrastructure by moving the few users of
>> 'x86_cpu_id' to the much more common variant.
> This paragraph is backwards. You are moving 'x86_cpu_desc to 'x86_cpu_id.

Thanks for catching that, Tony! Fixed.

Re: [PATCH 3/5] x86/cpu: Replace PEBS use of 'x86_cpu_desc' use with 'x86_cpu_id'

[Pawan Gupta]

On Fri, Dec 06, 2024 at 11:38:34AM -0800, Dave Hansen wrote:
> diff -puN arch/x86/events/intel/core.c~zap-x86_cpu_desc arch/x86/events/intel/core.c
> --- a/arch/x86/events/intel/core.c~zap-x86_cpu_desc	2024-12-06 11:33:16.775171950 -0800
> +++ b/arch/x86/events/intel/core.c	2024-12-06 11:33:16.779172107 -0800
> @@ -5371,42 +5371,33 @@ static __init void intel_clovertown_quir
>  	x86_pmu.pebs_constraints = NULL;
>  }
>  
> -static const struct x86_cpu_desc isolation_ucodes[] = {
> -	INTEL_CPU_DESC(INTEL_HASWELL,		 3, 0x0000001f),
> -	INTEL_CPU_DESC(INTEL_HASWELL_L,		 1, 0x0000001e),
> -	INTEL_CPU_DESC(INTEL_HASWELL_G,		 1, 0x00000015),
> -	INTEL_CPU_DESC(INTEL_HASWELL_X,		 2, 0x00000037),
> -	INTEL_CPU_DESC(INTEL_HASWELL_X,		 4, 0x0000000a),
> -	INTEL_CPU_DESC(INTEL_BROADWELL,		 4, 0x00000023),
> -	INTEL_CPU_DESC(INTEL_BROADWELL_G,	 1, 0x00000014),
> -	INTEL_CPU_DESC(INTEL_BROADWELL_D,	 2, 0x00000010),
> -	INTEL_CPU_DESC(INTEL_BROADWELL_D,	 3, 0x07000009),
> -	INTEL_CPU_DESC(INTEL_BROADWELL_D,	 4, 0x0f000009),
> -	INTEL_CPU_DESC(INTEL_BROADWELL_D,	 5, 0x0e000002),
> -	INTEL_CPU_DESC(INTEL_BROADWELL_X,	 1, 0x0b000014),
> -	INTEL_CPU_DESC(INTEL_SKYLAKE_X,		 3, 0x00000021),
> -	INTEL_CPU_DESC(INTEL_SKYLAKE_X,		 4, 0x00000000),
> -	INTEL_CPU_DESC(INTEL_SKYLAKE_X,		 5, 0x00000000),
> -	INTEL_CPU_DESC(INTEL_SKYLAKE_X,		 6, 0x00000000),
> -	INTEL_CPU_DESC(INTEL_SKYLAKE_X,		 7, 0x00000000),
> -	INTEL_CPU_DESC(INTEL_SKYLAKE_X,		11, 0x00000000),
> -	INTEL_CPU_DESC(INTEL_SKYLAKE_L,		 3, 0x0000007c),
> -	INTEL_CPU_DESC(INTEL_SKYLAKE,		 3, 0x0000007c),
> -	INTEL_CPU_DESC(INTEL_KABYLAKE,		 9, 0x0000004e),
> -	INTEL_CPU_DESC(INTEL_KABYLAKE_L,	 9, 0x0000004e),
> -	INTEL_CPU_DESC(INTEL_KABYLAKE_L,	10, 0x0000004e),
> -	INTEL_CPU_DESC(INTEL_KABYLAKE_L,	11, 0x0000004e),
> -	INTEL_CPU_DESC(INTEL_KABYLAKE_L,	12, 0x0000004e),
> -	INTEL_CPU_DESC(INTEL_KABYLAKE,		10, 0x0000004e),
> -	INTEL_CPU_DESC(INTEL_KABYLAKE,		11, 0x0000004e),
> -	INTEL_CPU_DESC(INTEL_KABYLAKE,		12, 0x0000004e),
> -	INTEL_CPU_DESC(INTEL_KABYLAKE,		13, 0x0000004e),
> +static const struct x86_cpu_id isolation_ucodes[] = {
> +	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL,		 3,  3, 0x0000001f),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_L,	 1,  1, 0x0000001e),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_G,	 1,  1, 0x00000015),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_X,	 2,  2, 0x00000037),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_HASWELL_X,	 4,  4, 0x0000000a),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL,	 4,  4, 0x00000023),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_G,	 1,  1, 0x00000014),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D,	 2,  2, 0x00000010),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D,	 3,  3, 0x07000009),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D,	 4,  4, 0x0f000009),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_D,	 5,  5, 0x0e000002),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_BROADWELL_X,	 1,  1, 0x0b000014),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X,	 3,  3, 0x00000021),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X,	 4,  7, 0x00000000),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_X,	11, 11, 0x00000000),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_L,	 3,  3, 0x0000007c),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE,		 3,  3, 0x0000007c),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_KABYLAKE,		 9,  9, 0x0000004e),

This ...

> +	X86_MATCH_VFM_STEPPINGS(INTEL_KABYLAKE_L,	 9, 12, 0x0000004e),
> +	X86_MATCH_VFM_STEPPINGS(INTEL_KABYLAKE,		10, 13, 0x0000004e),

... and this can also be combined into a single entry.

Re: [PATCH 3/5] x86/cpu: Replace PEBS use of 'x86_cpu_desc' use with 'x86_cpu_id'

[Dave Hansen]

On 12/6/24 15:58, Pawan Gupta wrote:
>> +	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE_L,	 3,  3, 0x0000007c),
>> +	X86_MATCH_VFM_STEPPINGS(INTEL_SKYLAKE,		 3,  3, 0x0000007c),
>> +	X86_MATCH_VFM_STEPPINGS(INTEL_KABYLAKE,		 9,  9, 0x0000004e),
> This ...
> 
>> +	X86_MATCH_VFM_STEPPINGS(INTEL_KABYLAKE_L,	 9, 12, 0x0000004e),
>> +	X86_MATCH_VFM_STEPPINGS(INTEL_KABYLAKE,		10, 13, 0x0000004e),
> ... and this can also be combined into a single entry.

Good catch, thanks!

[PATCH 4/5] x86/cpu: Move AMD erratum 1386 table over to 'x86_cpu_id'

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

The AMD erratum 1386 detection code uses and old style 'x86_cpu_desc'
table. Replace it with 'x86_cpu_id' so the old style can be removed.

I did not create a new helper macro here. The new table is certainly
more noisy than the old and it can be improved on. But I was hesitant
to create a new macro just for a single site that is only two ugly
lines in the end.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/arch/x86/kernel/cpu/amd.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff -puN arch/x86/kernel/cpu/amd.c~amd-x86_cpu_id arch/x86/kernel/cpu/amd.c
--- a/arch/x86/kernel/cpu/amd.c~amd-x86_cpu_id	2024-12-06 11:33:17.271191446 -0800
+++ b/arch/x86/kernel/cpu/amd.c	2024-12-06 11:33:17.275191602 -0800
@@ -795,10 +795,9 @@ static void init_amd_bd(struct cpuinfo_x
 	clear_rdrand_cpuid_bit(c);
 }
 
-static const struct x86_cpu_desc erratum_1386_microcode[] = {
-	AMD_CPU_DESC(0x17,  0x1, 0x2, 0x0800126e),
-	AMD_CPU_DESC(0x17, 0x31, 0x0, 0x08301052),
-	{},
+static const struct x86_cpu_id erratum_1386_microcode[] = {
+	X86_MATCH_VFM_STEPPINGS(VFM_MAKE(X86_VENDOR_AMD, 0x17, 0x01), 0x2, 0x2, 0x0800126e),
+	X86_MATCH_VFM_STEPPINGS(VFM_MAKE(X86_VENDOR_AMD, 0x17, 0x31), 0x0, 0x0, 0x08301052),
 };
 
 static void fix_erratum_1386(struct cpuinfo_x86 *c)
@@ -814,7 +813,7 @@ static void fix_erratum_1386(struct cpui
 	 * Clear the feature flag only on microcode revisions which
 	 * don't have the fix.
 	 */
-	if (x86_cpu_has_min_microcode_rev(erratum_1386_microcode))
+	if (x86_match_min_microcode_rev(erratum_1386_microcode))
 		return;
 
 	clear_cpu_cap(c, X86_FEATURE_XSAVES);
_

[PATCH 5/5] x86/cpu: Remove 'x86_cpu_desc' infrastructure

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

All the users of 'x86_cpu_desc' are gone.  Zap it from the tree.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/arch/x86/include/asm/cpu_device_id.h |   35 ---------------------------------
 b/arch/x86/kernel/cpu/match.c          |   31 -----------------------------
 2 files changed, 66 deletions(-)

diff -puN arch/x86/include/asm/cpu_device_id.h~zap-x86_cpu_desc-3 arch/x86/include/asm/cpu_device_id.h
--- a/arch/x86/include/asm/cpu_device_id.h~zap-x86_cpu_desc-3	2024-12-06 11:33:17.751210312 -0800
+++ b/arch/x86/include/asm/cpu_device_id.h	2024-12-06 11:33:17.755210470 -0800
@@ -243,42 +243,7 @@
 		VFM_MODEL(vfm),				\
 		X86_STEPPING_ANY, feature, data)
 
-/*
- * Match specific microcode revisions.
- *
- * vendor/family/model/stepping must be all set.
- *
- * Only checks against the boot CPU.  When mixed-stepping configs are
- * valid for a CPU model, add a quirk for every valid stepping and
- * do the fine-tuning in the quirk handler.
- */
-
-struct x86_cpu_desc {
-	u8	x86_family;
-	u8	x86_vendor;
-	u8	x86_model;
-	u8	x86_stepping;
-	u32	x86_microcode_rev;
-};
-
-#define INTEL_CPU_DESC(vfm, stepping, revision) {		\
-	.x86_family		= VFM_FAMILY(vfm),		\
-	.x86_vendor		= VFM_VENDOR(vfm),		\
-	.x86_model		= VFM_MODEL(vfm),		\
-	.x86_stepping		= (stepping),			\
-	.x86_microcode_rev	= (revision),			\
-}
-
-#define AMD_CPU_DESC(fam, model, stepping, revision) {		\
-	.x86_family		= (fam),			\
-	.x86_vendor		= X86_VENDOR_AMD,		\
-	.x86_model		= (model),			\
-	.x86_stepping		= (stepping),			\
-	.x86_microcode_rev	= (revision),			\
-}
-
 extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match);
-extern bool x86_cpu_has_min_microcode_rev(const struct x86_cpu_desc *table);
 extern bool x86_match_min_microcode_rev(const struct x86_cpu_id *table);
 
 #endif /* _ASM_X86_CPU_DEVICE_ID */
diff -puN arch/x86/kernel/cpu/match.c~zap-x86_cpu_desc-3 arch/x86/kernel/cpu/match.c
--- a/arch/x86/kernel/cpu/match.c~zap-x86_cpu_desc-3	2024-12-06 11:33:17.755210470 -0800
+++ b/arch/x86/kernel/cpu/match.c	2024-12-06 11:33:17.755210470 -0800
@@ -56,37 +56,6 @@ const struct x86_cpu_id *x86_match_cpu(c
 }
 EXPORT_SYMBOL(x86_match_cpu);
 
-static const struct x86_cpu_desc *
-x86_match_cpu_with_stepping(const struct x86_cpu_desc *match)
-{
-	struct cpuinfo_x86 *c = &boot_cpu_data;
-	const struct x86_cpu_desc *m;
-
-	for (m = match; m->x86_family | m->x86_model; m++) {
-		if (c->x86_vendor != m->x86_vendor)
-			continue;
-		if (c->x86 != m->x86_family)
-			continue;
-		if (c->x86_model != m->x86_model)
-			continue;
-		if (c->x86_stepping != m->x86_stepping)
-			continue;
-		return m;
-	}
-	return NULL;
-}
-
-bool x86_cpu_has_min_microcode_rev(const struct x86_cpu_desc *table)
-{
-	const struct x86_cpu_desc *res = x86_match_cpu_with_stepping(table);
-
-	if (!res || res->x86_microcode_rev > boot_cpu_data.microcode)
-		return false;
-
-	return true;
-}
-EXPORT_SYMBOL_GPL(x86_cpu_has_min_microcode_rev);
-
 bool x86_match_min_microcode_rev(const struct x86_cpu_id *table)
 {
 	const struct x86_cpu_id *res = x86_match_cpu(table);
_

Re: [PATCH 0/5] x86/cpu: Remove duplicate microcode version matching infrastructure

[Qiuxu Zhuo]

Hi Dave,

> From: Dave Hansen <dave.hansen@linux.intel.com>
> [...]
> Changes from RFC:
>  * Convert stepping match helpers to always take a range and never
>    take a raw stepping bitmap. - Ingo
> 
> --
> 
> x86 has generic CPU matching infrastructure. This lets you build
> tables of CPUs with some property.  It's mostly used for enumerating
> model-specific features, but it is quite a bit more flexible than
> that. In includes a facility to match steppings and microcode
> versions. This generic infrastructure is built around 'struct
> x86_cpu_id'.
> 
> There is a less generic, parallel CPU matching facility built around
> 'struct x86_cpu_desc'. It is used only for matching specific microcode
> revisions.  All of the 'struct x86_cpu_desc' users can be converted to
> 'struct x86_cpu_id'.
> 
> Do that conversion then remove the 'struct x86_cpu_desc'
> infrastructure.
> 
> Testing or acks would be much appreciated!

I tested this series + v6.13-rc2 on both Intel Cascade Lake server and Sapphire Rapids server.

- Both systems booted successfully.
- Both {skx,i10nm}_edac drivers[1][2] worked well.

  [1] Covered patch2's drivers/edac/skx_base.c file.
  [2] Covered patch2's drivers/edac/i10nm_base.c file.

Feel free to add:

  Tested-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>