V7 more sampling fun 0

From: mark gross <[email protected]>
Subject: [PATCH 3/4] x86/speculation: Special Register Buffer Data Sampling
(SRBDS) mitigation control.

SRBDS is an MDS-like speculative side channel that can leak bits from the RNG across cores and threads. New microcode serializes the processor access during the execution of RDRAND and RDSEED. This ensures that the shared buffer is overwritten before it is released for reuse.

While it is present on all affected CPU models, the microcode mitigation is not needed on models that enumerate ARCH_CAPABILITIES[MDS_NO] in the cases where TSX is not supported or has been disabled with TSX_CTRL.

The mitigation is activated by default on affected processors and it increases latency for RDRAND and RDSEED instructions. Among other effects this will reduce throughput from /dev/urandom.

* enable administrator to configure the mitigation off when desired using either mitigations=off or srbds=off.
* export vulnerability status via sysfs
* rename file scoped macros to apply for non-whitelist table initializations.

Signed-off-by Mark Gross <[email protected]>

Reviewed-by Tony Luck <[email protected]>

Reviewed-by Pawan Gupta <[email protected]>

Tested-by Neelima Krishnan <[email protected]>

---
 .../ABI/testing/sysfs-devices-system-cpu      |   1 +
 .../admin-guide/kernel-parameters.txt         |  20 +++
 arch/x86/include/asm/cpufeatures.h            |   2 +
 arch/x86/include/asm/msr-index.h              |   4 +
 arch/x86/kernel/cpu/bugs.c                    | 116 ++++++++++++++++++
 arch/x86/kernel/cpu/common.c                  |  53 ++++++++
 arch/x86/kernel/cpu/cpu.h                     |   2 +
 drivers/base/cpu.c                            |   8 ++
 8 files changed, 206 insertions(+)

Documentation/ABI/testing/sysfs-devices-system-cpu +1 -0

index 2e0e3b45d02a..b39531a3c5bc 100644
@@ -492,6 +492,7 @@ What:		/sys/devices/system/cpu/vulnerabilities
 		/sys/devices/system/cpu/vulnerabilities/spec_store_bypass
 		/sys/devices/system/cpu/vulnerabilities/l1tf
 		/sys/devices/system/cpu/vulnerabilities/mds
+		/sys/devices/system/cpu/vulnerabilities/srbds
 		/sys/devices/system/cpu/vulnerabilities/tsx_async_abort
 		/sys/devices/system/cpu/vulnerabilities/itlb_multihit
 Date:		January 2018

Documentation/admin-guide/kernel-parameters.txt +20 -0

index f2a93c8679e8..6bc125ff09da 100644
@@ -4757,6 +4757,26 @@
 			the kernel will oops in either "warn" or "fatal"
 			mode.
 
+	srbds=		[X86,INTEL]
+			Control mitigation for the Special Register
+			Buffer Data Sampling (SRBDS) mitigation.
+
+			Certain CPUs are vulnerable to an MDS-like
+			exploit which can leak bits from the random
+			number generator.
+
+			By default, this issue is mitigated by
+			microcode.  However, the microcode fix can cause
+			the RDRAND and RDSEED instructions to become
+			much slower.  Among other effects, this will
+			result in reduced throughput from /dev/urandom.
+
+			The microcode mitigation can be disabled with
+			the following option:
+
+			off:    Disable mitigation and remove
+				performance impact to RDRAND and RDSEED
+
 	srcutree.counter_wrap_check [KNL]
 			Specifies how frequently to check for
 			grace-period sequence counter wrap for the

arch/x86/include/asm/cpufeatures.h +2 -0

index db189945e9b0..02dabc9e77b0 100644
@@ -362,6 +362,7 @@
 #define X86_FEATURE_AVX512_4FMAPS	(18*32+ 3) /* AVX-512 Multiply Accumulation Single precision */
 #define X86_FEATURE_FSRM		(18*32+ 4) /* Fast Short Rep Mov */
 #define X86_FEATURE_AVX512_VP2INTERSECT (18*32+ 8) /* AVX-512 Intersect for D/Q */
+#define X86_FEATURE_SRBDS_CTRL		(18*32+ 9) /* "" SRBDS mitigation MSR available */
 #define X86_FEATURE_MD_CLEAR		(18*32+10) /* VERW clears CPU buffers */
 #define X86_FEATURE_TSX_FORCE_ABORT	(18*32+13) /* "" TSX_FORCE_ABORT */
 #define X86_FEATURE_PCONFIG		(18*32+18) /* Intel PCONFIG */

@@ -407,5 +408,6 @@
 #define X86_BUG_SWAPGS			X86_BUG(21) /* CPU is affected by speculation through SWAPGS */
 #define X86_BUG_TAA			X86_BUG(22) /* CPU is affected by TSX Async Abort(TAA) */
 #define X86_BUG_ITLB_MULTIHIT		X86_BUG(23) /* CPU may incur MCE during certain page attribute changes */
+#define X86_BUG_SRBDS			X86_BUG(24) /* CPU may leak RNG bits if not mitigated */
 
 #endif /* _ASM_X86_CPUFEATURES_H */

arch/x86/include/asm/msr-index.h +4 -0

index 12c9684d59ba..3efde600a674 100644
@@ -128,6 +128,10 @@
 #define TSX_CTRL_RTM_DISABLE		BIT(0)	/* Disable RTM feature */
 #define TSX_CTRL_CPUID_CLEAR		BIT(1)	/* Disable TSX enumeration */
 
+/* SRBDS support */
+#define MSR_IA32_MCU_OPT_CTRL		0x00000123
+#define RNGDS_MITG_DIS			BIT(0)
+
 #define MSR_IA32_SYSENTER_CS		0x00000174
 #define MSR_IA32_SYSENTER_ESP		0x00000175
 #define MSR_IA32_SYSENTER_EIP		0x00000176

arch/x86/kernel/cpu/bugs.c +75 -0

index ed54b3b21c39..addef92109fe 100644
@@ -41,6 +41,7 @@ static void __init l1tf_select_mitigation(void);
 static void __init mds_select_mitigation(void);
 static void __init mds_print_mitigation(void);
 static void __init taa_select_mitigation(void);
+static void __init srbds_select_mitigation(void);
 
 /* The base value of the SPEC_CTRL MSR that always has to be preserved. */
 u64 x86_spec_ctrl_base;

@@ -108,6 +109,7 @@ void __init check_bugs(void)
 	l1tf_select_mitigation();
 	mds_select_mitigation();
 	taa_select_mitigation();
+	srbds_select_mitigation();
 
 	/*
 	 * As MDS and TAA mitigations are inter-related, print MDS

@@ -397,6 +399,107 @@ static int __init tsx_async_abort_parse_cmdline(char *str)
 }
 early_param("tsx_async_abort", tsx_async_abort_parse_cmdline);
 
+#undef pr_fmt
+#define pr_fmt(fmt)	"SRBDS: " fmt
+
+enum srbds_mitigations {
+	SRBDS_MITIGATION_OFF,
+	SRBDS_MITIGATION_UCODE_NEEDED,
+	SRBDS_MITIGATION_FULL,
+	SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF,
+	SRBDS_MITIGATION_HYPERVISOR,
+};
+
+static enum srbds_mitigations srbds_mitigation __ro_after_init = SRBDS_MITIGATION_FULL;
+static const char * const srbds_strings[] = {
+	[SRBDS_MITIGATION_OFF]			= "Vulnerable",
+	[SRBDS_MITIGATION_UCODE_NEEDED]		= "Vulnerable: No microcode",
+	[SRBDS_MITIGATION_FULL]			= "Mitigated: Microcode",
+	[SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF]	= "Not affected (TSX disabled)",
+	[SRBDS_MITIGATION_HYPERVISOR]		= "Unknown: Dependent on hypervisor status",
+};
+
+static bool srbds_off;
+
+void update_srbds_msr(void)
+{
+	u64 mcu_ctrl;
+
+	if (!boot_cpu_has_bug(X86_BUG_SRBDS))
+		return;
+
+	if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
+		return;
+
+	if (srbds_mitigation == SRBDS_MITIGATION_UCODE_NEEDED)
+		return;
+
+	rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl);
+
+	switch (srbds_mitigation) {
+	case SRBDS_MITIGATION_OFF:
+	case SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF:
+		mcu_ctrl |= RNGDS_MITG_DIS;
+		break;
+	case SRBDS_MITIGATION_FULL:
+		mcu_ctrl &= ~RNGDS_MITG_DIS;
+		break;
+	default:
+		break;
+	}
+
+	wrmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl);
+}
+
+static void __init srbds_select_mitigation(void)
+{
+	u64 ia32_cap;
+
+	if (!boot_cpu_has_bug(X86_BUG_SRBDS))
+		return;
+	/*
+	 * Check to see if this is one of the MDS_NO systems supporting
+	 * TSX that are only exposed to SRBDS when TSX is enabled.
+	 */
+	ia32_cap = x86_read_arch_cap_msr();
+	if ((ia32_cap & ARCH_CAP_MDS_NO) && !boot_cpu_has(X86_FEATURE_RTM)) {
+		srbds_mitigation = SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF;
+		goto out;
+	}
+
+	if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
+		srbds_mitigation = SRBDS_MITIGATION_HYPERVISOR;
+		goto out;
+	}
+

arch/x86/kernel/cpu/bugs.c +31 -0

+	update_srbds_msr();
+	pr_info("%s\n", srbds_strings[srbds_mitigation]);
+}
+
+static int __init srbds_parse_cmdline(char *str)
+{
+	if (!str)
+		return -EINVAL;
+
+	if (!boot_cpu_has_bug(X86_BUG_SRBDS))
+		return 0;
+
+	srbds_off = !strcmp(str, "off");
+
+	return 0;
+}
+early_param("srbds", srbds_parse_cmdline);
+
 #undef pr_fmt
 #define pr_fmt(fmt)     "Spectre V1 : " fmt
 

@@ -1528,6 +1631,11 @@ static char *ibpb_state(void)
 	return "";
 }
 
+static ssize_t srbds_show_state(char *buf)
+{
+	return sprintf(buf, "%s\n", srbds_strings[srbds_mitigation]);
+}
+
 static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr,
 			       char *buf, unsigned int bug)
 {

@@ -1572,6 +1680,9 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr
 	case X86_BUG_ITLB_MULTIHIT:
 		return itlb_multihit_show_state(buf);
 
+	case X86_BUG_SRBDS:
+		return srbds_show_state(buf);
+
 	default:
 		break;
 	}

@@ -1618,4 +1729,9 @@ ssize_t cpu_show_itlb_multihit(struct device *dev, struct device_attribute *attr
 {
 	return cpu_show_common(dev, attr, buf, X86_BUG_ITLB_MULTIHIT);
 }
+
+ssize_t cpu_show_srbds(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	return cpu_show_common(dev, attr, buf, X86_BUG_SRBDS);
+}
 #endif

arch/x86/kernel/cpu/common.c +53 -0

index 2bea1cc8dcb4..2c9be1fd3c72 100644
@@ -1075,6 +1075,30 @@ static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = {
 	{}
 };
 
+#define VULNBL_INTEL_STEPPING(model, steppings, issues)			   \
+	X86_MATCH_VENDOR_FAM_MODEL_STEPPING_FEATURE(INTEL, 6,		   \
+					    INTEL_FAM6_##model, steppings, \
+					    X86_FEATURE_ANY, issues)
+
+#define SRBDS		BIT(0)
+#define SRBDS_IF_TSX	BIT(1)
+
+static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
+	VULNBL_INTEL_STEPPING(IVYBRIDGE,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(HASWELL,		X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(HASWELL_L,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(HASWELL_G,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(BROADWELL_G,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(BROADWELL,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(SKYLAKE_L,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(SKYLAKE,		X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0, 0xA),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0, 0xB),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),
+	{}
+};
+
 static bool __init cpu_matches(unsigned long which, const struct x86_cpu_id *table)
 {
 	const struct x86_cpu_id *m = x86_match_cpu(table);

@@ -1142,6 +1166,34 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
 	     (ia32_cap & ARCH_CAP_TSX_CTRL_MSR)))
 		setup_force_cpu_bug(X86_BUG_TAA);
 
+	if (cpu_matches(SRBDS|SRBDS_IF_TSX, cpu_vuln_blacklist)) {
+		/*
+		 * Some parts on the list don't have RDRAND or RDSEED. Make sure
+		 * they show as "Not affected".
+		 */
+		if (!cpu_has(c, X86_FEATURE_RDRAND) &&
+		    !cpu_has(c, X86_FEATURE_RDSEED))
+			goto srbds_not_affected;
+		/*
+		 * Parts in the blacklist that enumerate MDS_NO are only
+		 * vulneralbe if TSX can be used.  To handle cases where TSX
+		 * gets fused off check to see if TSX is fused off and thus not
+		 * affected.
+		 *
+		 * When running with up to day microcode TSX_CTRL is only
+		 * enumerated on parts where TSX fused on.
+		 * When running with microcode not supporting TSX_CTRL we check
+		 * for RTM
+		 */
+		if ((ia32_cap & ARCH_CAP_MDS_NO) &&
+		    !((ia32_cap & ARCH_CAP_TSX_CTRL_MSR) ||
+		      cpu_has(c, X86_FEATURE_RTM)))
+			goto srbds_not_affected;
+
+		setup_force_cpu_bug(X86_BUG_SRBDS);
+	}
+srbds_not_affected:
+
 	if (cpu_matches(NO_MELTDOWN, cpu_vuln_whitelist))
 		return;
 

@@ -1594,6 +1646,7 @@ void identify_secondary_cpu(struct cpuinfo_x86 *c)
 	mtrr_ap_init();
 	validate_apic_and_package_id(c);
 	x86_spec_ctrl_setup_ap();
+	update_srbds_msr();
 }
 
 static __init int setup_noclflush(char *arg)

arch/x86/kernel/cpu/cpu.h +2 -0

index 37fdefd14f28..f3e2fc44dba0 100644
@@ -44,6 +44,8 @@ struct _tlb_table {
 extern const struct cpu_dev *const __x86_cpu_dev_start[],
 			    *const __x86_cpu_dev_end[];
 
+void update_srbds_msr(void);
+
 #ifdef CONFIG_CPU_SUP_INTEL
 enum tsx_ctrl_states {
 	TSX_CTRL_ENABLE,

drivers/base/cpu.c +8 -0

index 9a1c00fbbaef..d2136ab9b14a 100644
@@ -562,6 +562,12 @@ ssize_t __weak cpu_show_itlb_multihit(struct device *dev,
 	return sprintf(buf, "Not affected\n");
 }
 
+ssize_t __weak cpu_show_srbds(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "Not affected\n");
+}
+
 static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL);
 static DEVICE_ATTR(spectre_v1, 0444, cpu_show_spectre_v1, NULL);
 static DEVICE_ATTR(spectre_v2, 0444, cpu_show_spectre_v2, NULL);

@@ -570,6 +576,7 @@ static DEVICE_ATTR(l1tf, 0444, cpu_show_l1tf, NULL);
 static DEVICE_ATTR(mds, 0444, cpu_show_mds, NULL);
 static DEVICE_ATTR(tsx_async_abort, 0444, cpu_show_tsx_async_abort, NULL);
 static DEVICE_ATTR(itlb_multihit, 0444, cpu_show_itlb_multihit, NULL);
+static DEVICE_ATTR(srbds, 0444, cpu_show_srbds, NULL);
 
 static struct attribute *cpu_root_vulnerabilities_attrs[] = {
 	&dev_attr_meltdown.attr,

@@ -580,6 +587,7 @@ static struct attribute *cpu_root_vulnerabilities_attrs[] = {
 	&dev_attr_mds.attr,
 	&dev_attr_tsx_async_abort.attr,
 	&dev_attr_itlb_multihit.attr,
+	&dev_attr_srbds.attr,
 	NULL
 };
 

Signature · 1 line

2.17.1

Quoted 19 lines · speck for mark gross

(quoted diff) +19 -0

+	/*
+	 * Check to see if this is one of the MDS_NO systems supporting
+	 * TSX that are only exposed to SRBDS when TSX is enabled.
+	 */
+	ia32_cap = x86_read_arch_cap_msr();
+	if ((ia32_cap & ARCH_CAP_MDS_NO) && !boot_cpu_has(X86_FEATURE_RTM)) {
+		srbds_mitigation = SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF;
+		goto out;
+	}
+
+	if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
+		srbds_mitigation = SRBDS_MITIGATION_HYPERVISOR;
+		goto out;
+	}
+
+	if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) {
+		srbds_mitigation = SRBDS_MITIGATION_UCODE_NEEDED;
+		goto out;
+	}

Review

Quoted 6 lines

(quoted diff) +6 -0

+
+	if (cpu_mitigations_off() || srbds_off) {
+		if (srbds_mitigation != SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF)
+			srbds_mitigation = SRBDS_MITIGATION_OFF;
+	}
+out:

The test for cpu_mitigations_off or srbds off needs to be after out:
Otherwise when TSX is off and srbds=off will report the wrong answer.

Mark,

Quoted 20 lines · speck for mark gross

Quoted 19 lines · speck for mark gross

(quoted diff) +19 -0

+	/*
+	 * Check to see if this is one of the MDS_NO systems supporting
+	 * TSX that are only exposed to SRBDS when TSX is enabled.
+	 */
+	ia32_cap = x86_read_arch_cap_msr();
+	if ((ia32_cap & ARCH_CAP_MDS_NO) && !boot_cpu_has(X86_FEATURE_RTM)) {
+		srbds_mitigation = SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF;
+		goto out;
+	}
+
+	if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
+		srbds_mitigation = SRBDS_MITIGATION_HYPERVISOR;
+		goto out;
+	}
+
+	if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) {
+		srbds_mitigation = SRBDS_MITIGATION_UCODE_NEEDED;
+		goto out;
+	}

Quoted 8 lines

Review

Quoted 6 lines

(quoted diff) +6 -0

+
+	if (cpu_mitigations_off() || srbds_off) {
+		if (srbds_mitigation != SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF)
+			srbds_mitigation = SRBDS_MITIGATION_OFF;
+	}
+out:

The test for cpu_mitigations_off or srbds off needs to be after out:
Otherwise when TSX is off and srbds=off will report the wrong answer.

If the CPU has SRBDS_IF_TSX and TSX is disabled then the correct answer is: Not affected (TSX off)

That's what we do with other issues as well. If the CPU is not affected then we print this even with mitigation disabled (all or particular).

(quoted diff) +6 -0

+		srbds_mitigation = SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF;
+		goto out;
+	}
+
+	if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
+		srbds_mitigation = SRBDS_MITIGATION_HYPERVISOR;

(quoted diff) +35 -0

+out:
+	update_srbds_msr();
+	pr_info("%s\n", srbds_strings[srbds_mitigation]);
+}
 
+#define SRBDS		BIT(0)
+#define SRBDS_IF_TSX	BIT(1)
+
+static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
+	VULNBL_INTEL_STEPPING(IVYBRIDGE,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(HASWELL,		X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(HASWELL_L,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(HASWELL_G,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(BROADWELL_G,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(BROADWELL,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(SKYLAKE_L,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(SKYLAKE,		X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0, 0xA),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0, 0xB),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),
+	{}
+};
+
 static bool __init cpu_matches(unsigned long which, const struct x86_cpu_id *table)
 {
 	const struct x86_cpu_id *m = x86_match_cpu(table);

@@ -1142,6 +1166,34 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
 	     (ia32_cap & ARCH_CAP_TSX_CTRL_MSR)))
 		setup_force_cpu_bug(X86_BUG_TAA);
 
+	if (cpu_matches(SRBDS|SRBDS_IF_TSX, cpu_vuln_blacklist)) {
+		/*
+		 * Some parts on the list don't have RDRAND or RDSEED. Make sure
+		 * they show as "Not affected".
+		 */
+		if (!cpu_has(c, X86_FEATURE_RDRAND) &&
+		    !cpu_has(c, X86_FEATURE_RDSEED))
+			goto srbds_not_affected;
+		/*
+		 * Parts in the blacklist that enumerate MDS_NO are only
+		 * vulneralbe if TSX can be used.  To handle cases where TSX
+		 * gets fused off check to see if TSX is fused off and thus not

Mark,

Review

Quoted 12 lines · speck for mark gross

(quoted diff) +12 -0

+static void __init srbds_select_mitigation(void)
+{
+	u64 ia32_cap;
+
+	if (!boot_cpu_has_bug(X86_BUG_SRBDS))
+		return;
+	/*
+	 * Check to see if this is one of the MDS_NO systems supporting
+	 * TSX that are only exposed to SRBDS when TSX is enabled.
+	 */
+	ia32_cap = x86_read_arch_cap_msr();
+	if ((ia32_cap & ARCH_CAP_MDS_NO) && !boot_cpu_has(X86_FEATURE_RTM)) {

This does not work for the CPUs which have the IF_TSX thing because your setup magic does not set X86_BUG_SRBDS for those.... See below.

Quoted 11 lines

(quoted diff) +10 -0

+		srbds_mitigation = SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF;
+		goto out;
+	}
+
+	if (boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
+		srbds_mitigation = SRBDS_MITIGATION_HYPERVISOR;
+		goto out;
+	}
+
+	if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) {

Review

Quoted 6 lines

(quoted diff) +6 -0

+		srbds_mitigation = SRBDS_MITIGATION_UCODE_NEEDED;
+		goto out;
+	}
+
+	if (cpu_mitigations_off() || srbds_off) {
+		if (srbds_mitigation != SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF)

That's pointless. You already jumped over this part in case of TSX off.

Review

Quoted 3 lines

(quoted diff) +2 -0

+			srbds_mitigation = SRBDS_MITIGATION_OFF;
+	}

The whole goto stuff can be completely avoided.

	if (!boot_cpu_has_bug(X86_BUG_SRBDS))
		return;

	if (boot_cpu_has(X86_BUG_SRBDS_IF_TSX) && !boot_cpu_has(X86_FEATURE_RTM)))
        	srbds_mitigation = SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF;
        else if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
		srbds_mitigation = SRBDS_MITIGATION_HYPERVISOR;
	else if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL))
		srbds_mitigation = SRBDS_MITIGATION_UCODE_NEEDED;
	else if (cpu_mitigations_off() || srbds_off)
		srbds_mitigation = SRBDS_MITIGATION_OFF;

Quoted 44 lines

(quoted diff) +35 -0

+out:
+	update_srbds_msr();
+	pr_info("%s\n", srbds_strings[srbds_mitigation]);
+}
 
+#define SRBDS		BIT(0)
+#define SRBDS_IF_TSX	BIT(1)
+
+static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
+	VULNBL_INTEL_STEPPING(IVYBRIDGE,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(HASWELL,		X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(HASWELL_L,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(HASWELL_G,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(BROADWELL_G,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(BROADWELL,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(SKYLAKE_L,	X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(SKYLAKE,		X86_STEPPING_ANY,		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0, 0xA),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0, 0xB),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),
+	{}
+};
+
 static bool __init cpu_matches(unsigned long which, const struct x86_cpu_id *table)
 {
 	const struct x86_cpu_id *m = x86_match_cpu(table);

@@ -1142,6 +1166,34 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
 	     (ia32_cap & ARCH_CAP_TSX_CTRL_MSR)))
 		setup_force_cpu_bug(X86_BUG_TAA);
 
+	if (cpu_matches(SRBDS|SRBDS_IF_TSX, cpu_vuln_blacklist)) {
+		/*
+		 * Some parts on the list don't have RDRAND or RDSEED. Make sure
+		 * they show as "Not affected".
+		 */
+		if (!cpu_has(c, X86_FEATURE_RDRAND) &&
+		    !cpu_has(c, X86_FEATURE_RDSEED))
+			goto srbds_not_affected;
+		/*
+		 * Parts in the blacklist that enumerate MDS_NO are only
+		 * vulneralbe if TSX can be used.  To handle cases where TSX
+		 * gets fused off check to see if TSX is fused off and thus not

Review static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) · line 1190

Quoted 10 lines

(quoted diff) +10 -0

+		 * affected.
+		 *
+		 * When running with up to day microcode TSX_CTRL is only
+		 * enumerated on parts where TSX fused on.
+		 * When running with microcode not supporting TSX_CTRL we check
+		 * for RTM
+		 */
+		if ((ia32_cap & ARCH_CAP_MDS_NO) &&
+		    !((ia32_cap & ARCH_CAP_TSX_CTRL_MSR) ||
+		      cpu_has(c, X86_FEATURE_RTM)))

So you added SRBDS_IF_TSX and then you check for both and still have that check for _all_ CPUs. Also the TSX_CTRL_MSR part is weird. That's completely irrelevant. The only interesting part is X86_FEATURE_RTM.

What you really want is:

	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS | SRBDS_IF_TSX),
	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0, 0xB),		SRBDS),
	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS | SRBDS_IF_TSX),

	/*
	 * CPUs which have neither RDRAND nor RDSEED are not affected.
	 */
	if (cpu_has(c, X86_FEATURE_RDRAND) || cpu_has(c, X86_FEATURE_RDSEED)) {
		if (cpu_matches(SRBDS, cpu_vuln_blacklist)) {
			setup_force_cpu_bug(X86_BUG_SRBDS);
			/* These CPUs are only vulnerable when TSX is usable. */
	                if (cpu_matches(SRBDS_IF_TSX, cpu_vuln_blacklist))
				setup_force_cpu_bug(X86_BUG_SRBDS_IF_TSX);
		}
	}

Hmm?

(quoted diff) +1 -0

@@ -1142,6 +1166,34 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
 	     (ia32_cap & ARCH_CAP_TSX_CTRL_MSR)))
 		setup_force_cpu_bug(X86_BUG_TAA);
 
+	if (cpu_matches(SRBDS|SRBDS_IF_TSX, cpu_vuln_blacklist)) {

Review

Quoted 13 lines · speck for mark gross

(quoted diff) +13 -0

+enum srbds_mitigations {
+	SRBDS_MITIGATION_OFF,
+	SRBDS_MITIGATION_UCODE_NEEDED,
+	SRBDS_MITIGATION_FULL,
+	SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF,
+	SRBDS_MITIGATION_HYPERVISOR,
+};
+
+static enum srbds_mitigations srbds_mitigation __ro_after_init = SRBDS_MITIGATION_FULL;
+static const char * const srbds_strings[] = {
+	[SRBDS_MITIGATION_OFF]			= "Vulnerable",
+	[SRBDS_MITIGATION_UCODE_NEEDED]		= "Vulnerable: No microcode",
+	[SRBDS_MITIGATION_FULL]			= "Mitigated: Microcode",

s/Mitigated/Mitigation/ for consistency with other issues

Review

Quoted 1 line

(quoted diff) +1 -0

+	[SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF]	= "Not affected (TSX disabled)",

The CPU *is* affected, it just happens to be mitigated, right?

Shouldn't it be SRBDS_MITIGATION_TSX_OFF and "Mitigation: TSX disabled"?

Quoted 6 lines

(quoted diff) +0 -0

@@ -1142,6 +1166,34 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
 	     (ia32_cap & ARCH_CAP_TSX_CTRL_MSR)))
 		setup_force_cpu_bug(X86_BUG_TAA);
 

Review static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) · line 1179

Quoted 11 lines

(quoted diff) +11 -0

+	if (cpu_matches(SRBDS|SRBDS_IF_TSX, cpu_vuln_blacklist)) {
+		/*
+		 * Some parts on the list don't have RDRAND or RDSEED. Make sure
+		 * they show as "Not affected".
+		 */
+		if (!cpu_has(c, X86_FEATURE_RDRAND) &&
+		    !cpu_has(c, X86_FEATURE_RDSEED))
+			goto srbds_not_affected;
+		/*
+		 * Parts in the blacklist that enumerate MDS_NO are only
+		 * vulneralbe if TSX can be used.  To handle cases where TSX

"vulnerable"

Review static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) · line 1183

Quoted 5 lines

(quoted diff) +4 -0

+		 * gets fused off check to see if TSX is fused off and thus not
+		 * affected.
+		 *
+		 * When running with up to day microcode TSX_CTRL is only

"up-to-date"

Review static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) · line 1184

Quoted 1 line

(quoted diff) +1 -0

+		 * enumerated on parts where TSX fused on.

where TSX *is* fused on.

Review static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) · line 1186

Quoted 2 lines

(quoted diff) +2 -0

+		 * When running with microcode not supporting TSX_CTRL we check
+		 * for RTM

Missing period

Quoted 7 lines

(quoted diff) +7 -0

+		 */
+		if ((ia32_cap & ARCH_CAP_MDS_NO) &&
+		    !((ia32_cap & ARCH_CAP_TSX_CTRL_MSR) ||
+		      cpu_has(c, X86_FEATURE_RTM)))
+			goto srbds_not_affected;
+
+		setup_force_cpu_bug(X86_BUG_SRBDS);

Review static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) · line 1198

Quoted 5 lines

(quoted diff) +3 -0

+	}
+srbds_not_affected:
+
 	if (cpu_matches(NO_MELTDOWN, cpu_vuln_whitelist))
 		return;

I'm thinking it would be more readable to have the newline between the bracket and the 'if', instead of between the label and the 'if'.

Quoted 3 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+	[SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF]	= "Not affected (TSX disabled)",

The CPU *is* affected, it just happens to be mitigated, right?

This depends on perspective.
The only mitigation to SRBS is for the uCode to serialize access to the off core hwRNG and to do a ghost transfer of a through away random number such that if it leaks you don't get the right random number. I think there may be some buffer clearing in there too.

Disabling TSX from that point of view is not mitigating the issue so much as hiding exposure to it.

I can see it either way. Not sure which is better. After reading my logic do you still think it would make more sence to change "Not affected (TSX disabled)" to "Mitigated: TSX disabled"?

Quoted 7 lines

Quoted 7 lines

(quoted diff) +7 -0

+		 */
+		if ((ia32_cap & ARCH_CAP_MDS_NO) &&
+		    !((ia32_cap & ARCH_CAP_TSX_CTRL_MSR) ||
+		      cpu_has(c, X86_FEATURE_RTM)))
+			goto srbds_not_affected;
+
+		setup_force_cpu_bug(X86_BUG_SRBDS);

Quoted 8 lines

Review

Quoted 5 lines

(quoted diff) +3 -0

+	}
+srbds_not_affected:
+
 	if (cpu_matches(NO_MELTDOWN, cpu_vuln_whitelist))
 		return;

I'm thinking it would be more readable to have the newline between the bracket and the 'if', instead of between the label and the 'if'.

so, lose the newline between the label and the if?

Quoted 12 lines · speck for mark gross

Quoted 3 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+	[SRBDS_MITIGATION_NOT_AFFECTED_TSX_OFF]	= "Not affected (TSX disabled)",

The CPU *is* affected, it just happens to be mitigated, right?

This depends on perspective.
The only mitigation to SRBS is for the uCode to serialize access to the off core hwRNG and to do a ghost transfer of a through away random number such that if it leaks you don't get the right random number. I think there may be some buffer clearing in there too.

Disabling TSX from that point of view is not mitigating the issue so much as hiding exposure to it.

Quoted 3 lines

I can see it either way. Not sure which is better. After reading my logic do you still think it would make more sence to change "Not affected (TSX disabled)" to "Mitigated: TSX disabled"?

From a user's perspective, "hiding exposure" is the same as a mitigation. So I think it makes sense to call it a mitigation.

Quoted 15 lines

Quoted 15 lines

Review

Quoted 12 lines

(quoted diff) +10 -0

+		 */
+		if ((ia32_cap & ARCH_CAP_MDS_NO) &&
+		    !((ia32_cap & ARCH_CAP_TSX_CTRL_MSR) ||
+		      cpu_has(c, X86_FEATURE_RTM)))
+			goto srbds_not_affected;
+
+		setup_force_cpu_bug(X86_BUG_SRBDS);
+	}
+srbds_not_affected:
+
 	if (cpu_matches(NO_MELTDOWN, cpu_vuln_whitelist))
 		return;

I'm thinking it would be more readable to have the newline between the bracket and the 'if', instead of between the label and the 'if'.

Quoted 1 line

so, lose the newline between the label and the if?

Yes, and add one between the '}' and the label.

Signature · 1 line

Josh

Review

Quoted 5 lines · speck for mark gross

(quoted diff) +5 -0

+static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0, 0xA),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0, 0xB),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),

Another readability tweak: "0x0" helps with vertical alignment:

	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0x0, 0xA),	SRBDS),
	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0x0, 0xB),	SRBDS),
	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),

Quoted 9 lines · speck for Josh Poimboeuf

Review

Quoted 5 lines · speck for mark gross

(quoted diff) +5 -0

+static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0, 0xA),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0, 0xB),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),

Another readability tweak: "0x0" helps with vertical alignment:

Quoted 4 lines

	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0x0, 0xA),	SRBDS),
	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0x0, 0xB),	SRBDS),
	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),

FWIW the white paper no longer calls out individual steppings as vulnerable only if TSX so I'm losing the SRBDS_IF_TSX stuff.

Given that the additional steppings 0xB, 0xC and 0xD no longer are treated differently I'm just going with the following:
VULNBL_INTEL_STEPPING(KABYLAKE_L, X86_STEPPINGS(0, 0xC), SRBDS)
VULNBL_INTEL_STEPPING(KABYLAKE, X86_STEPPINGS(0, 0xD), SRBDS)

Quoted 18 lines · speck for mark gross

Quoted 9 lines · speck for Josh Poimboeuf

Review

Quoted 5 lines · speck for mark gross

(quoted diff) +5 -0

+static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0, 0xA),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0, 0xB),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),

Another readability tweak: "0x0" helps with vertical alignment:

Quoted 4 lines

	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0x0, 0xA),	SRBDS),
	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0x0, 0xB),	SRBDS),
	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),

FWIW the white paper no longer calls out individual steppings as vulnerable only if TSX so I'm losing the SRBDS_IF_TSX stuff.

Quoted 4 lines

Given that the additional steppings 0xB, 0xC and 0xD no longer are treated differently I'm just going with the following:
VULNBL_INTEL_STEPPING(KABYLAKE_L, X86_STEPPINGS(0, 0xC), SRBDS)
VULNBL_INTEL_STEPPING(KABYLAKE, X86_STEPPINGS(0, 0xD), SRBDS)

Ok, though I still think "0x0" looks more consistent in this context ;-)

Signature · 1 line

Josh

Quoted 9 lines · speck for Josh Poimboeuf

Review

Quoted 5 lines · speck for mark gross

(quoted diff) +5 -0

+static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0, 0xA),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0, 0xB),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),

Another readability tweak: "0x0" helps with vertical alignment:

Quoted 4 lines

	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0x0, 0xA),	SRBDS),
	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0x0, 0xB),	SRBDS),
	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),

FWIW the white paper no longer calls out individual steppings as vulnerable only if TSX so I'm losing the SRBDS_IF_TSX stuff.

Mark,

Quoted 15 lines · speck for mark gross

Quoted 13 lines · speck for Josh Poimboeuf

Review

Quoted 5 lines · speck for mark gross

(quoted diff) +5 -0

+static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0, 0xA),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0, 0xB),		SRBDS),
+	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),

Another readability tweak: "0x0" helps with vertical alignment:

	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0x0, 0xA),	SRBDS),
	VULNBL_INTEL_STEPPING(KABYLAKE_L,	X86_STEPPINGS(0xB, 0xC),	SRBDS_IF_TSX),
	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0x0, 0xB),	SRBDS),
	VULNBL_INTEL_STEPPING(KABYLAKE,		X86_STEPPINGS(0xC, 0xD),	SRBDS_IF_TSX),

Quoted 2 lines

FWIW the white paper no longer calls out individual steppings as vulnerable only if TSX so I'm losing the SRBDS_IF_TSX stuff.

So we lose the complete TSX conditionals or do they now apply to all NO_MDS CPUs?

From: mark gross <[email protected]>
Subject: [PATCH 1/4] x86/cpu: Add stepping field to x86_cpu_id structure

Intel uses the same family/model for several CPUs. Sometimes the stepping must be checked to tell them apart.

On X86 there can be at most 16 steppings, add steppings bitmask to x86_cpu_id and X86_MATCH_VENDOR_FAMILY_MODEL_STEPPING_FEATURE macro and support for matching against family/model/stepping.

Signed-off-by Mark Gross <[email protected]>

Reviewed-by Tony Luck <[email protected]>

---
 arch/x86/include/asm/cpu_device_id.h | 27 ++++++++++++++++++++++++---
 arch/x86/kernel/cpu/match.c          |  7 ++++++-
 include/linux/mod_devicetable.h      |  2 ++
 3 files changed, 32 insertions(+), 4 deletions(-)

arch/x86/include/asm/cpu_device_id.h +0 -0

index cf3d621c6892..4f0df2e46c95 100644

arch/x86/include/asm/cpu_device_id.h +3 -2

  * @_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
  * @_model:	The model number, model constant or X86_MODEL_ANY
+ * @_steppings:	Bitmask for steppings, stepping constant or X86_STEPPING_ANY
  * @_feature:	A X86_FEATURE bit or X86_FEATURE_ANY
  * @_data:	Driver specific data or NULL. The internal storage
  *		format is unsigned long. The supplied value, pointer

@@ -37,15 +39,34 @@
  * 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_FEATURE(_vendor, _family, _model,	\
-					   _feature, _data) {		\
+#define X86_MATCH_VENDOR_FAM_MODEL_STEPPING_FEATURE(_vendor, _family, _model, \
+						    _steppings, _feature, _data) { \

Quoted 26 lines · speck for mark gross

From: mark gross <[email protected]>
Subject: [PATCH 1/4] x86/cpu: Add stepping field to x86_cpu_id structure

Intel uses the same family/model for several CPUs. Sometimes the stepping must be checked to tell them apart.

On X86 there can be at most 16 steppings, add steppings bitmask to x86_cpu_id and X86_MATCH_VENDOR_FAMILY_MODEL_STEPPING_FEATURE macro and support for matching against family/model/stepping.

Signed-off-by Mark Gross <[email protected]>

Reviewed-by Tony Luck <[email protected]>

---
 arch/x86/include/asm/cpu_device_id.h | 27 ++++++++++++++++++++++++---
 arch/x86/kernel/cpu/match.c          |  7 ++++++-
 include/linux/mod_devicetable.h      |  2 ++
 3 files changed, 32 insertions(+), 4 deletions(-)

arch/x86/include/asm/cpu_device_id.h +0 -0

index cf3d621c6892..4f0df2e46c95 100644
@@ -20,12 +20,14 @@
 #define X86_CENTAUR_FAM6_C7_D		0xd
 #define X86_CENTAUR_FAM6_NANO		0xf
 

arch/x86/include/asm/cpu_device_id.h line 25

Quoted 4 lines

arch/x86/include/asm/cpu_device_id.h +2 -1

+#define X86_STEPPINGS(mins, maxs)    GENMASK(maxs, mins)
 /**
- * X86_MATCH_VENDOR_FAM_MODEL_FEATURE - Base macro for CPU matching
+ * X86_MATCH_VENDOR_FAM_MODEL_STEPPING_FEATURE - Base macro for CPU matching

				 ^^^^^^^^

If the steppings argument is a bitmask of steppings, then the name should have "STEPPINGS" too to avoid confusion.

Quoted 24 lines

arch/x86/include/asm/cpu_device_id.h +4 -2

  * @_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
  * @_model:	The model number, model constant or X86_MODEL_ANY
+ * @_steppings:	Bitmask for steppings, stepping constant or X86_STEPPING_ANY
  * @_feature:	A X86_FEATURE bit or X86_FEATURE_ANY
  * @_data:	Driver specific data or NULL. The internal storage
  *		format is unsigned long. The supplied value, pointer

@@ -37,15 +39,34 @@
  * 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_FEATURE(_vendor, _family, _model,	\
-					   _feature, _data) {		\
+#define X86_MATCH_VENDOR_FAM_MODEL_STEPPING_FEATURE(_vendor, _family, _model, \
+						    _steppings, _feature, _data) { \
 	.vendor		= X86_VENDOR_##_vendor,				\
 	.family		= _family,					\
 	.model		= _model,					\
+	.steppings	= _steppings,					\
 	.feature	= _feature,					\
 	.driver_data	= (unsigned long) _data				\
 }
 

arch/x86/include/asm/cpu_device_id.h line 53

Quoted 2 lines

arch/x86/include/asm/cpu_device_id.h +2 -0

+/**
+ * X86_MATCH_VENDOR_FAM_MODEL_FEATURE - Base macro for CPU matching

That isn't the base macro anymore.

Signature · 5 lines

Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
--