Skip to contentSkip to conversation

x86/cpu: Change AMD uarch detection

RFC PATCH In review Borislav Petkov 3 participants 11 messages Oct 22 – Oct 31

Subsystems (11)
x86 ACPI perf events Crypto API +7 more subsystems
lore
Share & tools
lore

Borislav Petkov

author maintainer
#0 lore raw

RFC PATCH 1/1 x86/cpu: Change AMD uarch detection

Hi,

I have been wondering how to improve that Zen uarch detection and handling and be able to do range checks of uarches, i.e., "this code needs to run on Zen microarchitectures 3 to 5". And I asked AI.

It gave me a lot of blurb but the basic idea is good so I went and did it. See below.

It boots in a VM, I'm not even trying to cause the cat to catch fire by booting on a real machine but that will happen soon too.

The thing can easily be extended to pre-Zen but that is not really needed, I'd say.

X86_FEATURE_ZEN to mark all Zens remains, that will be a future patch. Perhaps something as simple as:

	if (boot_cpu_data.x86_uarch)

but I have to move Hygon off of that usage. We'll see. Small steps.

Thx.

---

Author: Borislav Petkov (AMD) <[email protected]>
Date:   Mon Oct 12 22:42:51 2026 -0700
Not-Signed-off-by Borislav Petkov (AMD) <[email protected]>
arch/x86/events/amd/core.c +3 -3
index 49b6b8fce566..e61857c74c16 100644
@@ -291,13 +291,13 @@ static const u64 amd_zen4_perfmon_event_map[PERF_COUNT_HW_MAX] =
 
 static u64 amd_pmu_event_map(int hw_event)
 {
-	if (cpu_feature_enabled(X86_FEATURE_ZEN4) || boot_cpu_data.x86 >= 0x1a)
+	if (boot_cpu_data.x86_uarch >= AMD_UARCH_ZEN4)
 		return amd_zen4_perfmon_event_map[hw_event];
 
-	if (cpu_feature_enabled(X86_FEATURE_ZEN2) || boot_cpu_data.x86 >= 0x19)
+	if (boot_cpu_data.x86_uarch >= AMD_UARCH_ZEN2)
 		return amd_zen2_perfmon_event_map[hw_event];
 
-	if (cpu_feature_enabled(X86_FEATURE_ZEN1))
+	if (boot_cpu_data.x86_uarch >= AMD_UARCH_ZEN1)
 		return amd_zen1_perfmon_event_map[hw_event];
 
 	return amd_perfmon_event_map[hw_event];
arch/x86/events/amd/ibs.c +2 -1
index 3531f9c23b8c..bc699050131a 100644
@@ -415,7 +415,8 @@ static int perf_ibs_init(struct perf_event *event)
 		config |= (ldlat - 1) << IBS_OP_LDLAT_THRSH_SHIFT;
 
 		config |= IBS_OP_LDLAT_EN;
-		if (cpu_feature_enabled(X86_FEATURE_ZEN5))
+
+		if (boot_cpu_data.x86_uarch == AMD_UARCH_ZEN5)
 			config |= IBS_OP_L3MISSONLY;
 	}
arch/x86/include/asm/cpu_device_id.h +4 -4
index c62d8fae52c3..50ea0eefcd06 100644
@@ -5,13 +5,13 @@
 /*
  * Can't use <linux/bitfield.h> because it generates expressions that
  * cannot be used in structure initializers. Bitfield construction
- * here must match the union in struct cpuinfo_86:
+ * here must match the union in struct cpuinfo_x86:
  *	union {
  *		struct {
  *			__u8	x86_model;
  *			__u8	x86;
  *			__u8	x86_vendor;
- *			__u8	x86_reserved;
+ *			__u8	x86_uarch;
  *		};
  *		__u32		x86_vfm;
  *	};

@@ -19,11 +19,11 @@
 #define VFM_MODEL_BIT	0
 #define VFM_FAMILY_BIT	8
 #define VFM_VENDOR_BIT	16
-#define VFM_RSVD_BIT	24
+#define VFM_UARCH_BIT	24
 
 #define	VFM_MODEL_MASK	GENMASK(VFM_FAMILY_BIT - 1, VFM_MODEL_BIT)
 #define	VFM_FAMILY_MASK	GENMASK(VFM_VENDOR_BIT - 1, VFM_FAMILY_BIT)
-#define	VFM_VENDOR_MASK	GENMASK(VFM_RSVD_BIT - 1, VFM_VENDOR_BIT)
+#define	VFM_VENDOR_MASK	GENMASK(VFM_UARCH_BIT - 1, VFM_VENDOR_BIT)
 
 #define VFM_MODEL(vfm)	(((vfm) & VFM_MODEL_MASK) >> VFM_MODEL_BIT)
 #define VFM_FAMILY(vfm)	(((vfm) & VFM_FAMILY_MASK) >> VFM_FAMILY_BIT)
arch/x86/include/asm/cpufeatures.h +6 -6
index f62944700975..1302cd3edfb8 100644
@@ -74,8 +74,8 @@
 #define X86_FEATURE_CYRIX_ARR		( 3*32+ 2) /* "cyrix_arr" Cyrix ARRs (= MTRRs) */
 #define X86_FEATURE_CENTAUR_MCR		( 3*32+ 3) /* "centaur_mcr" Centaur MCRs (= MTRRs) */
 #define X86_FEATURE_K8			( 3*32+ 4) /* Opteron, Athlon64 */
-#define X86_FEATURE_ZEN5		( 3*32+ 5) /* CPU based on Zen5 microarchitecture */
-#define X86_FEATURE_ZEN6		( 3*32+ 6) /* CPU based on Zen6 microarchitecture */
+/* unused, was: #define X86_FEATURE_ZEN5		( 3*32+ 5) * CPU based on Zen5 microarchitecture */
+/* unused, was: #define X86_FEATURE_ZEN6		( 3*32+ 6) * CPU based on Zen6 microarchitecture */
 #define X86_FEATURE_RMPOPT		( 3*32+ 7) /* Support for AMD RMPOPT instruction */
 #define X86_FEATURE_CONSTANT_TSC	( 3*32+ 8) /* "constant_tsc" TSC ticks at a constant rate */
 /* free: was #define X86_FEATURE_UP	( 3*32+ 9) * "up" SMP kernel running on UP */

@@ -313,10 +313,10 @@
 #define X86_FEATURE_SRSO_ALIAS		(11*32+25) /* AMD BTB untrain RETs through aliasing */
 #define X86_FEATURE_IBPB_ON_VMEXIT	(11*32+26) /* Issue an IBPB only on VMEXIT */
 #define X86_FEATURE_APIC_MSRS_FENCE	(11*32+27) /* IA32_TSC_DEADLINE and X2APIC MSRs need fencing */
-#define X86_FEATURE_ZEN2		(11*32+28) /* CPU based on Zen2 microarchitecture */
-#define X86_FEATURE_ZEN3		(11*32+29) /* CPU based on Zen3 microarchitecture */
-#define X86_FEATURE_ZEN4		(11*32+30) /* CPU based on Zen4 microarchitecture */
-#define X86_FEATURE_ZEN1		(11*32+31) /* CPU based on Zen1 microarchitecture */
+/* unused, was #define X86_FEATURE_ZEN2		(11*32+28) * CPU based on Zen2 microarchitecture */
+/* unused, was: #define X86_FEATURE_ZEN3	(11*32+29) * CPU based on Zen3 microarchitecture */
+/* unused, was: #define X86_FEATURE_ZEN4	(11*32+30) * CPU based on Zen4 microarchitecture */
+/* unused, was: #define X86_FEATURE_ZEN1	(11*32+31) * CPU based on Zen1 microarchitecture */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
 #define X86_FEATURE_SHA512		(12*32+ 0) /* SHA512 instructions */
arch/x86/include/asm/processor.h +15 -1
index 77488ff5eb8c..03e75b6ad9d3 100644
@@ -77,6 +77,19 @@ enum x86_topology_cpu_type {
 	TOPO_CPU_TYPE_UNKNOWN,
 };
 
+/*
+ * Ordered list of uarches so that range etc checks can work.
+ */
+enum amd_uarch {
+	AMD_UARCH_UNKNOWN = 0,
+	AMD_UARCH_ZEN1,
+	AMD_UARCH_ZEN2,
+	AMD_UARCH_ZEN3,
+	AMD_UARCH_ZEN4,
+	AMD_UARCH_ZEN5,
+	AMD_UARCH_ZEN6
+};
+
 struct cpuinfo_topology {
 	// Real APIC ID read from the local APIC
 	u32			apicid;

@@ -145,7 +158,8 @@ struct cpuinfo_x86 {
 			__u8	x86;
 			/* CPU vendor */
 			__u8	x86_vendor;
-			__u8	x86_reserved;
+			/* AMD: CPU uarch number */
+			__u8	x86_uarch;
 		};
 		/* combined vendor, family, model */
 		__u32		x86_vfm;
arch/x86/kernel/acpi/cppc.c +1 -1
index bbade0da5130..fc06a78b3cfc 100644
@@ -260,7 +260,7 @@ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator)
 	 * the highest performance level is set to 196.
 	 * https://bugzilla.kernel.org/show_bug.cgi?id=218759
 	 */
-	if (cpu_feature_enabled(X86_FEATURE_ZEN4)) {
+	if (boot_cpu_data.x86_uarch == AMD_UARCH_ZEN4) {
 		switch (boot_cpu_data.x86_model) {
 		case 0x70 ... 0x7f:
 			*numerator = CPPC_HIGHEST_PERF_PERFORMANCE;
arch/x86/kernel/cpu/amd.c +19 -21
index 3972966cb023..2df33d59c185 100644
@@ -367,8 +367,7 @@ static void bsp_determine_snp(struct cpuinfo_x86 *c)
 		 * instruction.
 		 */
 		if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
-		    (cpu_feature_enabled(X86_FEATURE_ZEN3) ||
-		     cpu_feature_enabled(X86_FEATURE_ZEN4) ||
+		    (c->x86_uarch >= AMD_UARCH_ZEN3 ||
 		     cpu_feature_enabled(X86_FEATURE_RMPREAD)) &&
 		    snp_probe_rmptable_info()) {
 			cc_platform_set(CC_ATTR_HOST_SEV_SNP);

@@ -409,8 +408,8 @@ static void tsa_init(struct cpuinfo_x86 *c)
 	if (cpu_has(c, X86_FEATURE_HYPERVISOR))
 		return;
 
-	if (cpu_has(c, X86_FEATURE_ZEN3) ||
-	    cpu_has(c, X86_FEATURE_ZEN4)) {
+	if (c->x86_uarch >= AMD_UARCH_ZEN3 &&
+	    c->x86_uarch <= AMD_UARCH_ZEN4) {
 		if (x86_match_min_microcode_rev(amd_tsa_microcode))
 			setup_force_cpu_cap(X86_FEATURE_VERW_CLEAR);
 		else

@@ -483,13 +482,13 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
 		switch (c->x86_model) {
 		case 0x00 ... 0x2f:
 		case 0x50 ... 0x5f:
-			setup_force_cpu_cap(X86_FEATURE_ZEN1);
+			c->x86_uarch = AMD_UARCH_ZEN1;
 			break;
 		case 0x30 ... 0x4f:
 		case 0x60 ... 0x7f:
 		case 0x90 ... 0x91:
 		case 0xa0 ... 0xaf:
-			setup_force_cpu_cap(X86_FEATURE_ZEN2);
+			c->x86_uarch = AMD_UARCH_ZEN2;
 			break;
 		default:
 			goto warn;

@@ -500,11 +499,11 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
 		switch (c->x86_model) {
 		case 0x00 ... 0x0f:
 		case 0x20 ... 0x5f:
-			setup_force_cpu_cap(X86_FEATURE_ZEN3);
+			c->x86_uarch = AMD_UARCH_ZEN3;
 			break;
 		case 0x10 ... 0x1f:
 		case 0x60 ... 0xaf:
-			setup_force_cpu_cap(X86_FEATURE_ZEN4);
+			c->x86_uarch = AMD_UARCH_ZEN4;
 			break;
 		default:
 			goto warn;

@@ -517,13 +516,13 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
 		case 0x40 ... 0x4f:
 		case 0x60 ... 0x7f:
 		case 0xd0 ... 0xd7:
-			setup_force_cpu_cap(X86_FEATURE_ZEN5);
+			c->x86_uarch = AMD_UARCH_ZEN5;
 			break;
 		case 0x50 ... 0x5f:
 		case 0x80 ... 0xaf:
 		case 0xc0 ... 0xcf:
 		case 0xd8 ... 0xef:
-			setup_force_cpu_cap(X86_FEATURE_ZEN6);
+			c->x86_uarch = AMD_UARCH_ZEN6;
 			break;
 		default:
 			goto warn;

@@ -1090,16 +1089,15 @@ static void init_amd(struct cpuinfo_x86 *c)
 	if (c->x86 >= 0x17)
 		init_amd_zen_common();
 
-	if (boot_cpu_has(X86_FEATURE_ZEN1))
-		init_amd_zen1(c);
-	else if (boot_cpu_has(X86_FEATURE_ZEN2))
-		init_amd_zen2(c);
-	else if (boot_cpu_has(X86_FEATURE_ZEN3))
-		init_amd_zen3(c);
-	else if (boot_cpu_has(X86_FEATURE_ZEN4))
-		init_amd_zen4(c);
-	else if (boot_cpu_has(X86_FEATURE_ZEN5))
-		init_amd_zen5(c);
+	switch (c->x86_uarch) {
+	case AMD_UARCH_ZEN1:	init_amd_zen1(c);	break;
+	case AMD_UARCH_ZEN2:	init_amd_zen2(c);	break;
+	case AMD_UARCH_ZEN3:	init_amd_zen3(c);	break;
+	case AMD_UARCH_ZEN4:	init_amd_zen4(c);	break;
+	case AMD_UARCH_ZEN5:	init_amd_zen5(c);	break;
+	default: break;
+	}
+
 
 	/*
 	 * Enable workaround for FXSAVE leak on CPUs

@@ -1329,7 +1327,7 @@ void amd_check_microcode(void)
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
 		return;
 
-	if (cpu_feature_enabled(X86_FEATURE_ZEN2))
+	if (boot_cpu_data.x86_uarch == AMD_UARCH_ZEN2)
 		on_each_cpu(zenbleed_check_cpu, NULL, 1);
 }
arch/x86/kvm/svm/avic.c +3 -7
index 3b037e385523..5b5177d44f8d 100644
@@ -1258,15 +1258,11 @@ static bool __init avic_want_avic_enabled(void)
 {
 	/*
 	 * In "auto" mode, enable AVIC by default for Zen4+ if x2AVIC is
-	 * supported (to avoid enabling partial support by default, and because
-	 * x2AVIC should be supported by all Zen4+ CPUs).  Explicitly check for
-	 * family 0x1A and later (Zen5+), as the kernel's synthetic ZenX flags
-	 * aren't inclusive of previous generations, i.e. the kernel will set
-	 * at most one ZenX feature flag.
+	 * supported (to avoid enabling partial support by default, and
+	 * because x2AVIC should be supported by all Zen4+ CPUs).
 	 */
 	if (avic == AVIC_AUTO_MODE)
-		avic = boot_cpu_has(X86_FEATURE_X2AVIC) &&
-		       (cpu_feature_enabled(X86_FEATURE_ZEN4) || boot_cpu_data.x86 >= 0x1A);
+		avic = boot_cpu_has(X86_FEATURE_X2AVIC) && boot_cpu_data.x86_uarch >= AMD_UARCH_ZEN4;
 
 	if (!avic || !npt_enabled)
 		return false;
drivers/cpufreq/amd-pstate.c +26 -18

60 lines not on this page · raw message

drivers/crypto/ccp/sev-dev.c +6 -3
index 5c996ab63895..0070421c6687 100644
@@ -3136,12 +3136,15 @@ static int get_v2_svn(struct sev_device *sev)
 static bool sev_firmware_allows_es(struct sev_device *sev)
 {
 	/* Documented in AMD-SB-3023 */
-	if (boot_cpu_has(X86_FEATURE_ZEN4) || boot_cpu_has(X86_FEATURE_ZEN3))
+	switch (boot_cpu_data.x86_uarch) {
+	case AMD_UARCH_ZEN3:
+	case AMD_UARCH_ZEN4:
 		return get_v1_svn(sev) < 0x1b;
-	else if (boot_cpu_has(X86_FEATURE_ZEN5))
+	case AMD_UARCH_ZEN5:
 		return get_v2_svn(sev) < 0x4;
-	else
+	default:
 		return true;
+	}
 }
 
 int sev_firmware_supported_vm_types(void)
drivers/platform/x86/amd/pmc/mp1_stb.c +2 -1
index 8a33e75b71c3..67a5c468171f 100644
@@ -245,7 +245,7 @@ static const struct file_operations amd_stb_debugfs_fops_v2 = {
 
 static void amd_stb_update_args(struct amd_pmc_dev *dev)
 {
-	if (cpu_feature_enabled(X86_FEATURE_ZEN5))
+	if (boot_cpu_data.x86_uarch == AMD_UARCH_ZEN5) {
 		switch (boot_cpu_data.x86_model) {
 		case 0x44:
 			dev->stb_arg.msg = AMD_GNR_REGISTER_MESSAGE;

@@ -254,6 +254,7 @@ static void amd_stb_update_args(struct amd_pmc_dev *dev)
 			return;
 		default:
 			break;
+		}
 	}
 
 	dev->stb_arg.msg = AMD_S2D_REGISTER_MESSAGE;
Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
maintainer
#1 lore raw
Quoted 15 lines · Borislav Petkov

Hi,

I have been wondering how to improve that Zen uarch detection and handling and be able to do range checks of uarches, i.e., "this code needs to run on Zen microarchitectures 3 to 5". And I asked AI.

It gave me a lot of blurb but the basic idea is good so I went and did it. See below.

It boots in a VM, I'm not even trying to cause the cat to catch fire by booting on a real machine but that will happen soon too.

The thing can easily be extended to pre-Zen but that is not really needed, I'd say.

Quoted 6 lines

X86_FEATURE_ZEN to mark all Zens remains, that will be a future patch. Perhaps something as simple as:

	if (boot_cpu_data.x86_uarch)

but I have to move Hygon off of that usage. We'll see. Small steps.

This is better than X86_FEATURE_ZENx, as I always found the lack of inclusivity, e.g. ZEN4 not lighting up ZEN3, to be confusing/surprising.

But why invent another scheme? What stops AMD from implementing precise macros as arch/x86/include/asm/intel-family.h does? Providing range-based checks would "just" require more macros, no? E.g. couldn't we do

	if (AMD_ZEN4_OR_LATER(boot_cpu_data.x86_vfm))
		return amd_zen4_perfmon_event_map[hw_event];

	if (AMD_ZEN2_OR_LATER(boot_cpu_data.x86_vfm))
		return amd_zen2_perfmon_event_map[hw_event];

	if (AMD_ZEN1_OR_LATER(boot_cpu_data.x86_vfm))
		return amd_zen1_perfmon_event_map[hw_event];

and

	if (AMD_ZEN3(boot_cpu_data.x86_vfm) ||
	    AMD_ZEN4(boot_cpu_data.x86_vfm)) {
		...

	}
author maintainer
#2 lore raw
Quoted 2 lines · Sean Christopherson

This is better than X86_FEATURE_ZENx, as I always found the lack of inclusivity, e.g. ZEN4 not lighting up ZEN3, to be confusing/surprising.

Yeah, I didn't think of this simple scheme back then.

Quoted 3 lines

But why invent another scheme? What stops AMD from implementing precise macros as arch/x86/include/asm/intel-family.h does? Providing range-based checks would "just" require more macros, no?

That's exactly the reason - moar macros. I wanna keep it simple and instead of having at least 5 OR_LATER macros, I wanna simply do:

	if x86_uarch >= AMD_UARCH...

And who knows, if in the future the scheme becomes even simpler, like it was before, we won't need those either.

But I might keep the uarch checks even then because this is a lot more telling
- we want the microarchitecture version, not some family, model or stepping.

And mapping f/m/s to uarch we already do.

Thx.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
maintainer
#3 lore raw
Quoted 15 lines · Borislav Petkov
Quoted 2 lines · Sean Christopherson

This is better than X86_FEATURE_ZENx, as I always found the lack of inclusivity, e.g. ZEN4 not lighting up ZEN3, to be confusing/surprising.

Yeah, I didn't think of this simple scheme back then.

Quoted 3 lines

But why invent another scheme? What stops AMD from implementing precise macros as arch/x86/include/asm/intel-family.h does? Providing range-based checks would "just" require more macros, no?

That's exactly the reason - moar macros. I wanna keep it simple and instead of having at least 5 OR_LATER macros, I wanna simply do:

	if x86_uarch >= AMD_UARCH...
Quoted 7 lines

And who knows, if in the future the scheme becomes even simpler, like it was before, we won't need those either.

But I might keep the uarch checks even then because this is a lot more telling
- we want the microarchitecture version, not some family, model or stepping.

And mapping f/m/s to uarch we already do.

I don't particularly care what the interface is, but as a "downstream" user, it would be helpful if Intel and AMD used the same scheme(s).

And FWIW, providing the human-friendly codenames is very helpful even if they're almost never used in code. E.g. effectively documenting that Turin is Zen5, Genoa is Zen4, Milan is Zen3, and so on is helpful, because even with search+AI, it's hard to find information you don't realize you're missing. Though that's probably orthogonal to the interface.

author maintainer
#4 lore raw
Quoted 2 lines · Sean Christopherson

I don't particularly care what the interface is, but as a "downstream" user, it would be helpful if Intel and AMD used the same scheme(s).

If you mean ->x86_vfm, then

git grep x86_vfm arch/x86/

shows only exactly one user in arch/x86/kvm :).

And then there's the X86_MATCH_VFM() gunk.

But I guess I can take a look and see whether I can use that thing on AMD too. It'll have a different meaning but at least it is the same cpuinfo_x86 field.

Quoted 5 lines

And FWIW, providing the human-friendly codenames is very helpful even if they're almost never used in code. E.g. effectively documenting that Turin is Zen5, Genoa is Zen4, Milan is Zen3, and so on is helpful, because even with search+AI, it's hard to find information you don't realize you're missing. Though that's probably orthogonal to the interface.

Yeah, try talking to the marketing wankers with their frenzy for naming everything in the dumbest and most unintuitive way possible...

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#5 lore raw

Looks easy enough but needs thorough testing and perhaps some commenting how we're using x86_vfm on AMD a bit differently.

amd-family.h is not really necessary right now but that's immaterial for this change - we can always convert to it later if absolutely necessary. And I don't think it will be.

Thx.

arch/x86/events/amd/core.c +3 -3
index 49b6b8fce566..baa6ce7f4d6d 100644
@@ -291,13 +291,13 @@ static const u64 amd_zen4_perfmon_event_map[PERF_COUNT_HW_MAX] =
 
 static u64 amd_pmu_event_map(int hw_event)
 {
-	if (cpu_feature_enabled(X86_FEATURE_ZEN4) || boot_cpu_data.x86 >= 0x1a)
+	if (boot_cpu_data.x86_vfm >= AMD_ZEN4)
 		return amd_zen4_perfmon_event_map[hw_event];
 
-	if (cpu_feature_enabled(X86_FEATURE_ZEN2) || boot_cpu_data.x86 >= 0x19)
+	if (boot_cpu_data.x86_vfm >= AMD_ZEN2)
 		return amd_zen2_perfmon_event_map[hw_event];
 
-	if (cpu_feature_enabled(X86_FEATURE_ZEN1))
+	if (boot_cpu_data.x86_vfm >= AMD_ZEN1)
 		return amd_zen1_perfmon_event_map[hw_event];
 
 	return amd_perfmon_event_map[hw_event];
arch/x86/events/amd/ibs.c +2 -1
index 3531f9c23b8c..000fcb7c3eb1 100644
@@ -415,7 +415,8 @@ static int perf_ibs_init(struct perf_event *event)
 		config |= (ldlat - 1) << IBS_OP_LDLAT_THRSH_SHIFT;
 
 		config |= IBS_OP_LDLAT_EN;
-		if (cpu_feature_enabled(X86_FEATURE_ZEN5))
+
+		if (boot_cpu_data.x86_vfm == AMD_ZEN5)
 			config |= IBS_OP_L3MISSONLY;
 	}
arch/x86/include/asm/cpu_device_id.h +1 -1
index c62d8fae52c3..3f3748752ba8 100644
@@ -5,7 +5,7 @@
 /*
  * Can't use <linux/bitfield.h> because it generates expressions that
  * cannot be used in structure initializers. Bitfield construction
- * here must match the union in struct cpuinfo_86:
+ * here must match the union in struct cpuinfo_x86:
  *	union {
  *		struct {
  *			__u8	x86_model;
arch/x86/include/asm/cpufeatures.h +6 -6
index 61930f0d4578..7c4851cd652c 100644
@@ -74,8 +74,8 @@
 #define X86_FEATURE_CYRIX_ARR		( 3*32+ 2) /* "cyrix_arr" Cyrix ARRs (= MTRRs) */
 #define X86_FEATURE_CENTAUR_MCR		( 3*32+ 3) /* "centaur_mcr" Centaur MCRs (= MTRRs) */
 #define X86_FEATURE_K8			( 3*32+ 4) /* Opteron, Athlon64 */
-#define X86_FEATURE_ZEN5		( 3*32+ 5) /* CPU based on Zen5 microarchitecture */
-#define X86_FEATURE_ZEN6		( 3*32+ 6) /* CPU based on Zen6 microarchitecture */
+/* unused, was: #define X86_FEATURE_ZEN5		( 3*32+ 5) * CPU based on Zen5 microarchitecture */
+/* unused, was: #define X86_FEATURE_ZEN6		( 3*32+ 6) * CPU based on Zen6 microarchitecture */
 #define X86_FEATURE_RMPOPT		( 3*32+ 7) /* Support for AMD RMPOPT instruction */
 #define X86_FEATURE_CONSTANT_TSC	( 3*32+ 8) /* "constant_tsc" TSC ticks at a constant rate */
 /* free: was #define X86_FEATURE_UP	( 3*32+ 9) * "up" SMP kernel running on UP */

@@ -314,10 +314,10 @@
 #define X86_FEATURE_SRSO_ALIAS		(11*32+25) /* AMD BTB untrain RETs through aliasing */
 #define X86_FEATURE_IBPB_ON_VMEXIT	(11*32+26) /* Issue an IBPB only on VMEXIT */
 #define X86_FEATURE_APIC_MSRS_FENCE	(11*32+27) /* IA32_TSC_DEADLINE and X2APIC MSRs need fencing */
-#define X86_FEATURE_ZEN2		(11*32+28) /* CPU based on Zen2 microarchitecture */
-#define X86_FEATURE_ZEN3		(11*32+29) /* CPU based on Zen3 microarchitecture */
-#define X86_FEATURE_ZEN4		(11*32+30) /* CPU based on Zen4 microarchitecture */
-#define X86_FEATURE_ZEN1		(11*32+31) /* CPU based on Zen1 microarchitecture */
+/* unused, was #define X86_FEATURE_ZEN2		(11*32+28) * CPU based on Zen2 microarchitecture */
+/* unused, was: #define X86_FEATURE_ZEN3	(11*32+29) * CPU based on Zen3 microarchitecture */
+/* unused, was: #define X86_FEATURE_ZEN4	(11*32+30) * CPU based on Zen4 microarchitecture */
+/* unused, was: #define X86_FEATURE_ZEN1	(11*32+31) * CPU based on Zen1 microarchitecture */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
 #define X86_FEATURE_SHA512		(12*32+ 0) /* SHA512 instructions */
arch/x86/include/asm/processor.h +13 -0
index 77488ff5eb8c..4df3942d969b 100644
@@ -77,6 +77,19 @@ enum x86_topology_cpu_type {
 	TOPO_CPU_TYPE_UNKNOWN,
 };
 
+/*
+ * Ordered list of uarches so that range etc checks can work.
+ */
+enum amd_uarch {
+	AMD_UNKNOWN = 0,
+	AMD_ZEN1,
+	AMD_ZEN2,
+	AMD_ZEN3,
+	AMD_ZEN4,
+	AMD_ZEN5,
+	AMD_ZEN6
+};
+
 struct cpuinfo_topology {
 	// Real APIC ID read from the local APIC
 	u32			apicid;
arch/x86/kernel/acpi/cppc.c +3 -3
index 1c6bf56c3215..cb49efa8a3ed 100644
@@ -257,7 +257,7 @@ int amd_get_effective_highest_perf(unsigned int cpu)
 	 * the highest performance level is set to 196.
 	 * https://bugzilla.kernel.org/show_bug.cgi?id=218759
 	 */
-	if (cpu_feature_enabled(X86_FEATURE_ZEN4)) {
+	if (boot_cpu_data.x86_vfm == AMD_ZEN4) {
 		switch (boot_cpu_data.x86_model) {
 		case 0x70 ... 0x7f:
 			return CPPC_HIGHEST_PERF_PERFORMANCE;

@@ -268,7 +268,7 @@ int amd_get_effective_highest_perf(unsigned int cpu)
 
 	/* detect if running on heterogeneous design */
 	if (cpu_feature_enabled(X86_FEATURE_AMD_HTR_CORES)) {
-		if (cpu_feature_enabled(X86_FEATURE_ZEN5) &&
+		if (boot_cpu_data.x86_vfm == AMD_ZEN5 &&
 		    cpu_data(cpu).topo.cpu_type == TOPO_CPU_TYPE_PERFORMANCE)
 			return CPPC_HIGHEST_PERF_PERFORMANCE;
 

@@ -301,7 +301,7 @@ int amd_get_max_frequency(unsigned int cpu)
 	enum x86_topology_cpu_type type = cpu_data(cpu).topo.cpu_type;
 	const struct amd_max_freq *max_freq = NULL;
 
-	if (cpu_feature_enabled(X86_FEATURE_ZEN6) &&
+	if (boot_cpu_data.x86_vfm == AMD_ZEN6 &&
 	    cpu_feature_enabled(X86_FEATURE_AMD_HTR_CORES))
 		max_freq = &max_freq_zen6_client;
arch/x86/kernel/cpu/amd.c +19 -21
index 3972966cb023..4ac268416fb1 100644
@@ -367,8 +367,7 @@ static void bsp_determine_snp(struct cpuinfo_x86 *c)
 		 * instruction.
 		 */
 		if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
-		    (cpu_feature_enabled(X86_FEATURE_ZEN3) ||
-		     cpu_feature_enabled(X86_FEATURE_ZEN4) ||
+		    (c->x86_vfm >= AMD_ZEN3 ||
 		     cpu_feature_enabled(X86_FEATURE_RMPREAD)) &&
 		    snp_probe_rmptable_info()) {
 			cc_platform_set(CC_ATTR_HOST_SEV_SNP);

@@ -409,8 +408,8 @@ static void tsa_init(struct cpuinfo_x86 *c)
 	if (cpu_has(c, X86_FEATURE_HYPERVISOR))
 		return;
 
-	if (cpu_has(c, X86_FEATURE_ZEN3) ||
-	    cpu_has(c, X86_FEATURE_ZEN4)) {
+	if (c->x86_vfm >= AMD_ZEN3 &&
+	    c->x86_vfm <= AMD_ZEN4) {
 		if (x86_match_min_microcode_rev(amd_tsa_microcode))
 			setup_force_cpu_cap(X86_FEATURE_VERW_CLEAR);
 		else

@@ -483,13 +482,13 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
 		switch (c->x86_model) {
 		case 0x00 ... 0x2f:
 		case 0x50 ... 0x5f:
-			setup_force_cpu_cap(X86_FEATURE_ZEN1);
+			c->x86_vfm = AMD_ZEN1;
 			break;
 		case 0x30 ... 0x4f:
 		case 0x60 ... 0x7f:
 		case 0x90 ... 0x91:
 		case 0xa0 ... 0xaf:
-			setup_force_cpu_cap(X86_FEATURE_ZEN2);
+			c->x86_vfm = AMD_ZEN2;
 			break;
 		default:
 			goto warn;

@@ -500,11 +499,11 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
 		switch (c->x86_model) {
 		case 0x00 ... 0x0f:
 		case 0x20 ... 0x5f:
-			setup_force_cpu_cap(X86_FEATURE_ZEN3);
+			c->x86_vfm = AMD_ZEN3;
 			break;
 		case 0x10 ... 0x1f:
 		case 0x60 ... 0xaf:
-			setup_force_cpu_cap(X86_FEATURE_ZEN4);
+			c->x86_vfm = AMD_ZEN4;
 			break;
 		default:
 			goto warn;

@@ -517,13 +516,13 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
 		case 0x40 ... 0x4f:
 		case 0x60 ... 0x7f:
 		case 0xd0 ... 0xd7:
-			setup_force_cpu_cap(X86_FEATURE_ZEN5);
+			c->x86_vfm = AMD_ZEN5;
 			break;
 		case 0x50 ... 0x5f:
 		case 0x80 ... 0xaf:
 		case 0xc0 ... 0xcf:
 		case 0xd8 ... 0xef:
-			setup_force_cpu_cap(X86_FEATURE_ZEN6);
+			c->x86_vfm = AMD_ZEN6;
 			break;
 		default:
 			goto warn;

@@ -1090,16 +1089,15 @@ static void init_amd(struct cpuinfo_x86 *c)
 	if (c->x86 >= 0x17)
 		init_amd_zen_common();
 
-	if (boot_cpu_has(X86_FEATURE_ZEN1))
-		init_amd_zen1(c);
-	else if (boot_cpu_has(X86_FEATURE_ZEN2))
-		init_amd_zen2(c);
-	else if (boot_cpu_has(X86_FEATURE_ZEN3))
-		init_amd_zen3(c);
-	else if (boot_cpu_has(X86_FEATURE_ZEN4))
-		init_amd_zen4(c);
-	else if (boot_cpu_has(X86_FEATURE_ZEN5))
-		init_amd_zen5(c);
+	switch (c->x86_vfm) {
+	case AMD_ZEN1:	init_amd_zen1(c);	break;
+	case AMD_ZEN2:	init_amd_zen2(c);	break;
+	case AMD_ZEN3:	init_amd_zen3(c);	break;
+	case AMD_ZEN4:	init_amd_zen4(c);	break;
+	case AMD_ZEN5:	init_amd_zen5(c);	break;
+	default: break;
+	}
+
 
 	/*
 	 * Enable workaround for FXSAVE leak on CPUs

@@ -1329,7 +1327,7 @@ void amd_check_microcode(void)
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
 		return;
 
-	if (cpu_feature_enabled(X86_FEATURE_ZEN2))
+	if (boot_cpu_data.x86_vfm == AMD_ZEN2)
 		on_each_cpu(zenbleed_check_cpu, NULL, 1);
 }
arch/x86/kvm/svm/avic.c +3 -7
index 96ca39c0045f..d018d1102654 100644
@@ -1268,15 +1268,11 @@ static bool __init avic_want_avic_enabled(void)
 {
 	/*
 	 * In "auto" mode, enable AVIC by default for Zen4+ if x2AVIC is
-	 * supported (to avoid enabling partial support by default, and because
-	 * x2AVIC should be supported by all Zen4+ CPUs).  Explicitly check for
-	 * family 0x1A and later (Zen5+), as the kernel's synthetic ZenX flags
-	 * aren't inclusive of previous generations, i.e. the kernel will set
-	 * at most one ZenX feature flag.
+	 * supported (to avoid enabling partial support by default, and
+	 * because x2AVIC should be supported by all Zen4+ CPUs).
 	 */
 	if (avic == AVIC_AUTO_MODE)
-		avic = boot_cpu_has(X86_FEATURE_X2AVIC) &&
-		       (cpu_feature_enabled(X86_FEATURE_ZEN4) || boot_cpu_data.x86 >= 0x1A);
+		avic = boot_cpu_has(X86_FEATURE_X2AVIC) && boot_cpu_data.x86_vfm >= AMD_ZEN4;
 
 	if (!avic || !npt_enabled)
 		return false;
drivers/cpufreq/amd-pstate.c +27 -19

68 lines not on this page · raw message

drivers/crypto/ccp/sev-dev.c +6 -3
index 5c996ab63895..7479da12f514 100644
@@ -3136,12 +3136,15 @@ static int get_v2_svn(struct sev_device *sev)
 static bool sev_firmware_allows_es(struct sev_device *sev)
 {
 	/* Documented in AMD-SB-3023 */
-	if (boot_cpu_has(X86_FEATURE_ZEN4) || boot_cpu_has(X86_FEATURE_ZEN3))
+	switch (boot_cpu_data.x86_vfm) {
+	case AMD_ZEN3:
+	case AMD_ZEN4:
 		return get_v1_svn(sev) < 0x1b;
-	else if (boot_cpu_has(X86_FEATURE_ZEN5))
+	case AMD_ZEN5:
 		return get_v2_svn(sev) < 0x4;
-	else
+	default:
 		return true;
+	}
 }
 
 int sev_firmware_supported_vm_types(void)
drivers/platform/x86/amd/pmc/mp1_stb.c +2 -1
index 8a33e75b71c3..e2d31016d0fc 100644
@@ -245,7 +245,7 @@ static const struct file_operations amd_stb_debugfs_fops_v2 = {
 
 static void amd_stb_update_args(struct amd_pmc_dev *dev)
 {
-	if (cpu_feature_enabled(X86_FEATURE_ZEN5))
+	if (boot_cpu_data.x86_vfm == AMD_ZEN5) {
 		switch (boot_cpu_data.x86_model) {
 		case 0x44:
 			dev->stb_arg.msg = AMD_GNR_REGISTER_MESSAGE;

@@ -254,6 +254,7 @@ static void amd_stb_update_args(struct amd_pmc_dev *dev)
 			return;
 		default:
 			break;
+		}
 	}
 
 	dev->stb_arg.msg = AMD_S2D_REGISTER_MESSAGE;
Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
maintainer
#6 lore raw
Quoted 6 lines · Borislav Petkov

Looks easy enough but needs thorough testing and perhaps some commenting how we're using x86_vfm on AMD a bit differently.

amd-family.h is not really necessary right now but that's immaterial for this change - we can always convert to it later if absolutely necessary. And I don't think it will be.

Ya, LGTM.

#7 lore raw
In reply to Borislav Petkov
Quoted 3 lines · Borislav Petkov

Looks easy enough but needs thorough testing and perhaps some commenting how we're using x86_vfm on AMD a bit differently.

I was working on adding guidance for x86_vfm usage for Intel in the kernel documentation when I saw this thread. Here's the initial draft I had for Intel:

Intel CPU model checks
----------------------

Intel CPU model checks should use the combined vendor/family/model field, x86_vfm, together with the INTEL_* defines in asm/intel-family.h. Avoid comparing x86_model against literal model numbers. Instead of writing::

if (c->x86 == 6 && c->x86_model == 0x97)

Please use::

if (c->x86_vfm == INTEL_ALDERLAKE)

To match several models, prefer an x86_match_cpu() table built with X86_MATCH_VFM(). If a model is missing from asm/intel-family.h, add a define for it.

Range checks on x86_vfm can work, but they are discouraged for newer CPUs. Model numbers are not assigned in release order, and the newer Families 18 and 19 interleave with Family 6, which makes ranges hard to get right.

IIUC, the AMD usage is primarily geared towards range checks for a group of CPUs whereas the Intel usage is for specific targeted CPUs.

Expressing uarch reusing x86_vfm and using it for range checks can be tricky. See below.

Quoted 4 lines

amd-family.h is not really necessary right now but that's immaterial for this change - we can always convert to it later if absolutely necessary. And I don't think it will be.

...

arch/x86/kernel/cpu/amd.c static void bsp_init_amd(struct cpuinfo_x86 *c) · line 485

Quoted 6 lines
arch/x86/kernel/cpu/amd.c +1 -1
@@ -483,13 +482,13 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
 		switch (c->x86_model) {
 		case 0x00 ... 0x2f:
 		case 0x50 ... 0x5f:
-			setup_force_cpu_cap(X86_FEATURE_ZEN1);
+			c->x86_vfm = AMD_ZEN1;

Since x86_vfm is a union, wouldn't this clobber the vendor, family and model? This will set the x86_vendor to 0 and get detected as an Intel CPU.

Quoted 9 lines
arch/x86/kernel/cpu/amd.c +1 -1
 			break;
 		case 0x30 ... 0x4f:
 		case 0x60 ... 0x7f:
 		case 0x90 ... 0x91:
 		case 0xa0 ... 0xaf:
-			setup_force_cpu_cap(X86_FEATURE_ZEN2);
+			c->x86_vfm = AMD_ZEN2;
 			break;
 		default:
author maintainer
#8 lore raw
Quoted 2 lines · Sohil Mehta

Since x86_vfm is a union, wouldn't this clobber the vendor, family and model? This will set the x86_vendor to 0 and get detected as an Intel CPU.

Yah. That was the reason why I did this x86_uarch field initially. But then Sean complained and I forgot about it, thinking I can make this a lot more unified.

I would've remembered again after testing it but yeah, thanks for reminding me.

So, we're going back to the initial method because that's what this is
- x86_uarch is the microarchitecture number I am giving it and not x86_vfm
- a family/model/stepping tuple.

Who knows, if you guys decide you need an uarch enumeration, you could use it too.

Thx.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
#9 lore raw
Quoted 4 lines · Borislav Petkov

So, we're going back to the initial method because that's what this is
- x86_uarch is the microarchitecture number I am giving it and not x86_vfm
- a family/model/stepping tuple.

Yes, that makes sense. Though, I am wondering if it would be better to define x86_uarch outside the x86_vfm union?

With your initial patch,

	union {
		struct {
			__u8	x86_model;
			__u8	x86;
			__u8	x86_vendor;
			__u8	x86_uarch;
		};
		__u32		x86_vfm;
	};

x86_vfm is calculated as:

    uarch << 24 | vendor << 16 | family << 8 | model

However, VFM_MAKE() is defined as

#define	VFM_MAKE(_vendor, _family, _model) (	\
	((_model) << VFM_MODEL_BIT) |		\
	((_family) << VFM_FAMILY_BIT) |		\
	((_vendor) << VFM_VENDOR_BIT)		\

)

Is the thinking that we could make x86_uarch work with all the VFM macros? Because, if they can't be used together, then placing x86_uarch in the union x86_vfm is confusing and likely to cause mistakes.

For example, I initially thought that amd_tsa_microcode comparison might fail, because it uses VFM_MAKE(). But, it doesn't do a direct x86_vfm comparison so it is safe.

author maintainer
#10 lore raw
Quoted 2 lines · Sohil Mehta

Is the thinking that we could make x86_uarch work with all the VFM macros?

No, x86_uarch is separate. I'm using it because I don't want to waste space unnecessarily and this was a reserved field.

But it probably will have to be a separate member for you, if you decide to need it because, if it is part of the union, it will botch x86_vfm comparisons.

Oh well, we'll cross that bridge when we get to it.

Signature · 4 lines
Regards/Gruss,
    Boris.

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