Skip to contentSkip to conversation

SSB 0

PATCH Superseded Thomas Gleixner 6 participants 37 messages May 1, 2018 – May 2, 2018

Subsystems (6)

v9 of 11 Superseded Latest is v11 →

lore
Share & tools
lore
#0 lore raw

v9 cover SSB 0

Changes vs. V8:

Addressed review feed back and added Linus variant of alternative_write_msr().

- Use EINVAL in the prctl

- Fix the sbb typos

- Make AMD default to prctl as well (Borislav)

- Add the not affected CPU types (Tim)

I've updated the git repository as well

ssh://[email protected]/linux/speck/linux master (forced update)

There are also two stable backport branches now:

ssh://[email protected]/linux/speck/linux linux-4.16.y

ssh://[email protected]/linux/speck/linux linux-4.14.y

Delta patch against V8 below. Git bundle against 4.17-rc3 follows in separate mail.

Thanks,

tglx

8<-------------------

Documentation/admin-guide/kernel-parameters.txt +3 -3
index de1ca7510697..29984fd3dd18 100644
@@ -4052,10 +4052,10 @@
 			on     - Unconditionally disable Speculative Store Bypass
 			off    - Unconditionally enable Speculative Store Bypass
 			auto   - Kernel detects whether the CPU model contains a
-			         vulnerable implementation of Speculative Store
-			         Bypass and picks the most appropriate mitigation
+				 implementation of Speculative Store Bypass and
+				 picks the most appropriate mitigation
 			prctl  - Control Speculative Store Bypass for a thread
-			         via prctl. By default it is enabled. The state
+				 via prctl. By default it is enabled. The state
 				 is inherited on fork.
 
 			Not specifying this option is equivalent to
Documentation/userspace-api/spec_ctrl.rst +2 -3
index 867b8b435baa..8ff39a26a992 100644
@@ -56,11 +56,10 @@ Common error codes
 ======= =================================================================
 Value   Meaning
 ======= =================================================================
-EINVAL  The prctl is not implemented by the architecture
+EINVAL  The prctl is not implemented by the architecture or unused
+        prctl(2) arguments are not 0
 
 ENODEV  arg2 is selecting a not supported speculation misfeature
-
-EUCLEAN The unused prctl() arguments are not 0
 ======= =================================================================
 
 PR_SET_SPECULATION_CTRL error codes
arch/x86/include/asm/nospec-branch.h +11 -11
index 7a1be0b2c264..023e2edc0f3c 100644
@@ -259,22 +259,22 @@ static inline void vmexit_fill_RSB(void)
 #endif
 }
 
-#define alternative_msr_write(_msr, _val, _feature)		\
-	asm volatile(ALTERNATIVE("",				\
-				 "movl %[msr], %%ecx\n\t"	\
-				 "movl %[val], %%eax\n\t"	\
-				 "movl $0, %%edx\n\t"		\
-				 "wrmsr",			\
-				 _feature)			\
-		     : : [msr] "i" (_msr), [val] "m" (_val)	\
-		     : "eax", "ecx", "edx", "memory")
+static __always_inline
+void alternative_msr_write(unsigned int msr, u64 val, unsigned int feature)
+{
+	asm volatile(ALTERNATIVE("", "wrmsr", %c[feature])
+		: : "c" (msr),
+		    "a" (val),
+		    "d" (val >> 32),
+		    [feature] "i" (feature)
+		: "memory");
+}
 
 static inline void indirect_branch_prediction_barrier(void)
 {
 	u64 val = PRED_CMD_IBPB;
 
-	alternative_msr_write(MSR_IA32_PRED_CMD, val,
-			      X86_FEATURE_USE_IBPB);
+	alternative_msr_write(MSR_IA32_PRED_CMD, val, X86_FEATURE_USE_IBPB);
 }
 
 /*
arch/x86/kernel/cpu/bugs.c +4 -9
index d52ccd0ce507..c28856e475c8 100644
@@ -476,11 +476,6 @@ static enum ssb_mitigation_cmd __init __ssb_select_mitigation(void)
 
 	switch (cmd) {
 	case SPEC_STORE_BYPASS_CMD_AUTO:
-		/*
-		 * AMD platforms by default don't need SSB mitigation.
-		 */
-		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
-			break;
 		/* Choose prctl as the default mode */
 		mode = SPEC_STORE_BYPASS_PRCTL;
 		break;
@@ -531,7 +526,7 @@ static void ssb_select_mitigation()
 
 #undef pr_fmt
 
-static int sbb_prctl_set(unsigned long ctrl)
+static int ssb_prctl_set(unsigned long ctrl)
 {
 	bool rds = !!test_tsk_thread_flag(current, TIF_RDS);
 
@@ -549,7 +544,7 @@ static int sbb_prctl_set(unsigned long ctrl)
 	return 0;
 }
 
-static int sbb_prctl_get(void)
+static int ssb_prctl_get(void)
 {
 	switch (ssb_mode) {
 	case SPEC_STORE_BYPASS_DISABLE:
@@ -572,7 +567,7 @@ int arch_prctl_set_spec_ctrl(unsigned long which, unsigned long ctrl)
 
 	switch (which) {
 	case PR_SPEC_STORE_BYPASS:
-		return sbb_prctl_set(ctrl);
+		return ssb_prctl_set(ctrl);
 	default:
 		return -ENODEV;
 	}
@@ -582,7 +577,7 @@ int arch_prctl_get_spec_ctrl(unsigned long which)
 {
 	switch (which) {
 	case PR_SPEC_STORE_BYPASS:
-		return sbb_prctl_get();
+		return ssb_prctl_get();
 	default:
 		return -ENODEV;
 	}
arch/x86/kernel/cpu/common.c +3 -0
index c3b53bc30a29..6101e3dba2f6 100644
@@ -931,6 +931,9 @@ static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = {
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_CEDARVIEW },
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_SILVERMONT1 },
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_AIRMONT },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_SILVERMONT2 },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_MERRIFIELD },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_CORE_YONAH },
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_XEON_PHI_KNL },
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_XEON_PHI_KNM },
 	{ X86_VENDOR_CENTAUR,	5 },
kernel/sys.c +4 -6
index 8bb8051bd9a1..d7afe29319f1 100644
@@ -2464,15 +2464,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		break;
 	case PR_SET_SPECULATION_CTRL:
 		if (arg4 || arg5)
-			error = -EUCLEAN;
-		else
-			error = arch_prctl_set_spec_ctrl(arg2, arg3);
+			return -EINVAL;
+		error = arch_prctl_set_spec_ctrl(arg2, arg3);
 		break;
 	case PR_GET_SPECULATION_CTRL:
 		if (arg3 || arg4 || arg5)
-			error = -EUCLEAN;
-		else
-			error = arch_prctl_get_spec_ctrl(arg2);
+			return -EINVAL;
+		error = arch_prctl_get_spec_ctrl(arg2);
 		break;
 	default:
 		error = -EINVAL;
#1 lore raw

Re: [patch V9 00/16] SSB

Quoted 19 lines · speck for Thomas Gleixner

Changes vs. V8:

Addressed review feed back and added Linus variant of alternative_write_msr().

- Use EINVAL in the prctl

- Fix the sbb typos

- Make AMD default to prctl as well (Borislav)

- Add the not affected CPU types (Tim)

I've updated the git repository as well

ssh://[email protected]/linux/speck/linux master (forced update)

There are also two stable backport branches now:

Quoted 7 lines

ssh://[email protected]/linux/speck/linux linux-4.16.y

ssh://[email protected]/linux/speck/linux linux-4.14.y

Delta patch against V8 below. Git bundle against 4.17-rc3 follows in separate mail.

Attached.

#2 lore raw

PATCH v9 1/16 SSB 1

In reply to Thomas Gleixner

The macro is not type safe and I did look for why that "g" constraint for the asm doesn't work: it's because the asm is more fundamentally wrong.

It does

        movl %[val], %%eax

but "val" isn't a 32-bit value, so then gcc will pass it in a register, and generate code like

movl %rsi, %eax

and gas will complain about a nonsensical 'mov' instruction (it's moving a 64-bit register to a 32-bit one).

Passing it through memory will just hide the real bug - gcc still thinks the memory location is 64-bit, but the "movl" will only load the first 32 bits and it all happens to work because x86 is little-endian.

Convert it to a type safe inline function with a little trick which hands the feature into the ALTERNATIVE macro.

---
 arch/x86/include/asm/nospec-branch.h |   19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)
arch/x86/include/asm/nospec-branch.h +10 -9
@@ -241,15 +241,16 @@ static inline void vmexit_fill_RSB(void)
 #endif
 }
 
-#define alternative_msr_write(_msr, _val, _feature)		\
-	asm volatile(ALTERNATIVE("",				\
-				 "movl %[msr], %%ecx\n\t"	\
-				 "movl %[val], %%eax\n\t"	\
-				 "movl $0, %%edx\n\t"		\
-				 "wrmsr",			\
-				 _feature)			\
-		     : : [msr] "i" (_msr), [val] "i" (_val)	\
-		     : "eax", "ecx", "edx", "memory")
+static __always_inline
+void alternative_msr_write(unsigned int msr, u64 val, unsigned int feature)
+{
+	asm volatile(ALTERNATIVE("", "wrmsr", %c[feature])
+		: : "c" (msr),
+		    "a" (val),
+		    "d" (val >> 32),
+		    [feature] "i" (feature)
+		: "memory");
+}
 
 static inline void indirect_branch_prediction_barrier(void)
 {
#3 lore raw

PATCH v9 2/16 SSB 2

In reply to Thomas Gleixner

Combine the various logic which goes through all those x86_cpu_id matching structures in one function.

---
v2: New patch
v3: Added Reviewed-by
v4: Changed title of the patch

---
 arch/x86/kernel/cpu/common.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)
arch/x86/kernel/cpu/common.c +11 -10
index 348cf4821240..74722c38a836 100644
@@ -918,21 +918,27 @@ static const __initconst struct x86_cpu_id cpu_no_meltdown[] = {
 	{}
 };
 
-static bool __init cpu_vulnerable_to_meltdown(struct cpuinfo_x86 *c)
+static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
 {
 	u64 ia32_cap = 0;
 
+	if (x86_match_cpu(cpu_no_speculation))
+		return;
+
+	setup_force_cpu_bug(X86_BUG_SPECTRE_V1);
+	setup_force_cpu_bug(X86_BUG_SPECTRE_V2);
+
 	if (x86_match_cpu(cpu_no_meltdown))
-		return false;
+		return;
 
 	if (cpu_has(c, X86_FEATURE_ARCH_CAPABILITIES))
 		rdmsrl(MSR_IA32_ARCH_CAPABILITIES, ia32_cap);
 
 	/* Rogue Data Cache Load? No! */
 	if (ia32_cap & ARCH_CAP_RDCL_NO)
-		return false;
+		return;
 
-	return true;
+	setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
 }
 
 /*
@@ -982,12 +988,7 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
 
 	setup_force_cpu_cap(X86_FEATURE_ALWAYS);
 
-	if (!x86_match_cpu(cpu_no_speculation)) {
-		if (cpu_vulnerable_to_meltdown(c))
-			setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
-		setup_force_cpu_bug(X86_BUG_SPECTRE_V1);
-		setup_force_cpu_bug(X86_BUG_SPECTRE_V2);
-	}
+	cpu_set_bug_bits(c);
 
 	fpu__init_system(c);
 
Signature · 1 line
2.14.3
#4 lore raw

PATCH v9 3/16 SSB 3

In reply to Thomas Gleixner

Those SysFS functions have a similar preamble, as such make common code to handle them.

---
v2: New patch
v3: Added Reviewed-by
v4: Fixed the title

---
 arch/x86/kernel/cpu/bugs.c | 46 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 14 deletions(-)
arch/x86/kernel/cpu/bugs.c +32 -14
index bfca937bdcc3..ad613f73d071 100644
@@ -314,30 +314,48 @@ static void __init spectre_v2_select_mitigation(void)
 #undef pr_fmt
 
 #ifdef CONFIG_SYSFS
-ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr, char *buf)
+
+ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr,
+			char *buf, unsigned int bug)
 {
-	if (!boot_cpu_has_bug(X86_BUG_CPU_MELTDOWN))
+	if (!boot_cpu_has_bug(bug))
 		return sprintf(buf, "Not affected\n");
-	if (boot_cpu_has(X86_FEATURE_PTI))
-		return sprintf(buf, "Mitigation: PTI\n");
+
+	switch (bug) {
+	case X86_BUG_CPU_MELTDOWN:
+		if (boot_cpu_has(X86_FEATURE_PTI))
+			return sprintf(buf, "Mitigation: PTI\n");
+
+		break;
+
+	case X86_BUG_SPECTRE_V1:
+		return sprintf(buf, "Mitigation: __user pointer sanitization\n");
+
+	case X86_BUG_SPECTRE_V2:
+		return sprintf(buf, "%s%s%s%s\n", spectre_v2_strings[spectre_v2_enabled],
+			       boot_cpu_has(X86_FEATURE_USE_IBPB) ? ", IBPB" : "",
+			       boot_cpu_has(X86_FEATURE_USE_IBRS_FW) ? ", IBRS_FW" : "",
+			       spectre_v2_module_string());
+
+	default:
+		break;
+	}
+
 	return sprintf(buf, "Vulnerable\n");
 }
 
+ssize_t cpu_show_meltdown(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	return cpu_show_common(dev, attr, buf, X86_BUG_CPU_MELTDOWN);
+}
+
 ssize_t cpu_show_spectre_v1(struct device *dev, struct device_attribute *attr, char *buf)
 {
-	if (!boot_cpu_has_bug(X86_BUG_SPECTRE_V1))
-		return sprintf(buf, "Not affected\n");
-	return sprintf(buf, "Mitigation: __user pointer sanitization\n");
+	return cpu_show_common(dev, attr, buf, X86_BUG_SPECTRE_V1);
 }
 
 ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr, char *buf)
 {
-	if (!boot_cpu_has_bug(X86_BUG_SPECTRE_V2))
-		return sprintf(buf, "Not affected\n");
-
-	return sprintf(buf, "%s%s%s%s\n", spectre_v2_strings[spectre_v2_enabled],
-		       boot_cpu_has(X86_FEATURE_USE_IBPB) ? ", IBPB" : "",
-		       boot_cpu_has(X86_FEATURE_USE_IBRS_FW) ? ", IBRS_FW" : "",
-		       spectre_v2_module_string());
+	return cpu_show_common(dev, attr, buf, X86_BUG_SPECTRE_V2);
 }
 #endif
Signature · 1 line
2.14.3
#5 lore raw

PATCH v9 4/16 SSB 4

In reply to Thomas Gleixner

The 336996-Speculative-Execution-Side-Channel-Mitigations.pdf refers to all the other bits as reserved. The Intel SDM glossary defines reserved as implementation specific - aka unknown.

As such at bootup this must be taken it into account and proper masking for the bits in use applied.

A copy of this document is available at
https://bugzilla.kernel.org/show_bug.cgi?id=199511

[ tglx: Made x86_spec_ctrl_base __ro_after_init ]

---
 arch/x86/include/asm/nospec-branch.h |   24 ++++++++++++++++++++----
 arch/x86/kernel/cpu/bugs.c           |   28 ++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 4 deletions(-)
arch/x86/include/asm/nospec-branch.h +20 -4
@@ -217,6 +217,17 @@ enum spectre_v2_mitigation {
 	SPECTRE_V2_IBRS,
 };
 
+/*
+ * The Intel specification for the SPEC_CTRL MSR requires that we
+ * preserve any already set reserved bits at boot time (e.g. for
+ * future additions that this kernel is not currently aware of).
+ * We then set any additional mitigation bits that we want
+ * ourselves and always use this as the base for SPEC_CTRL.
+ * We also use this when handling guest entry/exit as below.
+ */
+extern void x86_set_spec_ctrl(u64);
+extern u64 x86_get_default_spec_ctrl(void);
+
 extern char __indirect_thunk_start[];
 extern char __indirect_thunk_end[];
 
@@ -254,8 +265,9 @@ void alternative_msr_write(unsigned int
 
 static inline void indirect_branch_prediction_barrier(void)
 {
-	alternative_msr_write(MSR_IA32_PRED_CMD, PRED_CMD_IBPB,
-			      X86_FEATURE_USE_IBPB);
+	u64 val = PRED_CMD_IBPB;
+
+	alternative_msr_write(MSR_IA32_PRED_CMD, val, X86_FEATURE_USE_IBPB);
 }
 
 /*
@@ -266,14 +278,18 @@ static inline void indirect_branch_predi
  */
 #define firmware_restrict_branch_speculation_start()			\
 do {									\
+	u64 val = x86_get_default_spec_ctrl() | SPEC_CTRL_IBRS;		\
+									\
 	preempt_disable();						\
-	alternative_msr_write(MSR_IA32_SPEC_CTRL, SPEC_CTRL_IBRS,	\
+	alternative_msr_write(MSR_IA32_SPEC_CTRL, val,			\
 			      X86_FEATURE_USE_IBRS_FW);			\
 } while (0)
 
 #define firmware_restrict_branch_speculation_end()			\
 do {									\
-	alternative_msr_write(MSR_IA32_SPEC_CTRL, 0,			\
+	u64 val = x86_get_default_spec_ctrl();				\
+									\
+	alternative_msr_write(MSR_IA32_SPEC_CTRL, val,			\
 			      X86_FEATURE_USE_IBRS_FW);			\
 	preempt_enable();						\
 } while (0)
arch/x86/kernel/cpu/bugs.c +28 -0
@@ -28,6 +28,12 @@
 
 static void __init spectre_v2_select_mitigation(void);
 
+/*
+ * Our boot-time value of SPEC_CTRL MSR. We read it once so that any
+ * writes to SPEC_CTRL contain whatever reserved bits have been set.
+ */
+static u64 __ro_after_init x86_spec_ctrl_base;
+
 void __init check_bugs(void)
 {
 	identify_boot_cpu();
@@ -37,6 +43,13 @@ void __init check_bugs(void)
 		print_cpu_info(&boot_cpu_data);
 	}
 
+	/*
+	 * Read the SPEC_CTRL MSR to account for reserved bits which may
+	 * have unknown values.
+	 */
+	if (boot_cpu_has(X86_FEATURE_IBRS))
+		rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+
 	/* Select the proper spectre mitigation before patching alternatives */
 	spectre_v2_select_mitigation();
 
@@ -95,6 +108,21 @@ static const char *spectre_v2_strings[]
 
 static enum spectre_v2_mitigation spectre_v2_enabled = SPECTRE_V2_NONE;
 
+void x86_set_spec_ctrl(u64 val)
+{
+	if (val & ~(SPEC_CTRL_IBRS))
+		WARN_ONCE(1, "SPEC_CTRL MSR value 0x%16llx is unknown.\n", val);
+	else
+		wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base | val);
+}
+EXPORT_SYMBOL_GPL(x86_set_spec_ctrl);
+
+u64 x86_get_default_spec_ctrl(void)
+{
+	return x86_spec_ctrl_base;
+}
+EXPORT_SYMBOL_GPL(x86_get_default_spec_ctrl);
+
 #ifdef RETPOLINE
 static bool spectre_v2_bad_module;
 
#6 lore raw

PATCH v9 5/16 SSB 5

In reply to Thomas Gleixner

A guest may modify the SPEC_CTRL MSR from the value used by the kernel. Since the kernel doesn't use IBRS, this means a value of zero is what is needed in the host.

But the 336996-Speculative-Execution-Side-Channel-Mitigations.pdf refers to the other bits as reserved so the kernel should respect the boot time SPEC_CTRL value and use that.

This allows to deal with future extensions to the SPEC_CTRL interface if any at all.

Note: This uses wrmsrl instead of native_wrmsl. I does not make any difference as paravirt will over-write the callq *0xfff.. with the wrmsrl assembler code.

A copy of this document is available at
https://bugzilla.kernel.org/show_bug.cgi?id=199511

[ tglx: Added a paranoia check for IBRS into the functions and simplified
them ]

---
v2: New patch
v3: Use the two accessory functions instead of poking at the global variable.
v4: Use x86_get_spec_ctrl instead of global variable.
v5: Use x86_get_default_spec_ctrl instead of x86_get_spec_ctrl

---
 arch/x86/include/asm/nospec-branch.h |   10 ++++++++++
 arch/x86/kernel/cpu/bugs.c           |   18 ++++++++++++++++++
 arch/x86/kvm/svm.c                   |    6 ++----
 arch/x86/kvm/vmx.c                   |    6 ++----
 4 files changed, 32 insertions(+), 8 deletions(-)
arch/x86/include/asm/nospec-branch.h +10 -0
@@ -228,6 +228,16 @@ enum spectre_v2_mitigation {
 extern void x86_set_spec_ctrl(u64);
 extern u64 x86_get_default_spec_ctrl(void);
 
+/*
+ * On VMENTER we must preserve whatever view of the SPEC_CTRL MSR
+ * the guest has, while on VMEXIT we restore the host view. This
+ * would be easier if SPEC_CTRL were architecturally maskable or
+ * shadowable for guests but this is not (currently) the case.
+ * Takes the guest view of SPEC_CTRL MSR as a parameter.
+ */
+extern void x86_set_guest_spec_ctrl(u64);
+extern void x86_restore_host_spec_ctrl(u64);
+
 extern char __indirect_thunk_start[];
 extern char __indirect_thunk_end[];
 
arch/x86/kernel/cpu/bugs.c +18 -0
@@ -123,6 +123,24 @@ u64 x86_get_default_spec_ctrl(void)
 }
 EXPORT_SYMBOL_GPL(x86_get_default_spec_ctrl);
 
+void x86_set_guest_spec_ctrl(u64 guest_spec_ctrl)
+{
+	if (!boot_cpu_has(X86_FEATURE_IBRS))
+		return;
+	if (x86_spec_ctrl_base != guest_spec_ctrl)
+		wrmsrl(MSR_IA32_SPEC_CTRL, guest_spec_ctrl);
+}
+EXPORT_SYMBOL_GPL(x86_set_guest_spec_ctrl);
+
+void x86_restore_host_spec_ctrl(u64 guest_spec_ctrl)
+{
+	if (!boot_cpu_has(X86_FEATURE_IBRS))
+		return;
+	if (x86_spec_ctrl_base != guest_spec_ctrl)
+		wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+}
+EXPORT_SYMBOL_GPL(x86_restore_host_spec_ctrl);
+
 #ifdef RETPOLINE
 static bool spectre_v2_bad_module;
 
arch/x86/kvm/svm.c +2 -4
@@ -5557,8 +5557,7 @@ static void svm_vcpu_run(struct kvm_vcpu
 	 * is no need to worry about the conditional branch over the wrmsr
 	 * being speculatively taken.
 	 */
-	if (svm->spec_ctrl)
-		native_wrmsrl(MSR_IA32_SPEC_CTRL, svm->spec_ctrl);
+	x86_set_guest_spec_ctrl(svm->spec_ctrl);
 
 	asm volatile (
 		"push %%" _ASM_BP "; \n\t"
@@ -5670,8 +5669,7 @@ static void svm_vcpu_run(struct kvm_vcpu
 	if (unlikely(!msr_write_intercepted(vcpu, MSR_IA32_SPEC_CTRL)))
 		svm->spec_ctrl = native_read_msr(MSR_IA32_SPEC_CTRL);
 
-	if (svm->spec_ctrl)
-		native_wrmsrl(MSR_IA32_SPEC_CTRL, 0);
+	x86_restore_host_spec_ctrl(svm->spec_ctrl);
 
 	/* Eliminate branch target predictions from guest mode */
 	vmexit_fill_RSB();
arch/x86/kvm/vmx.c +2 -4
@@ -9726,8 +9726,7 @@ static void __noclone vmx_vcpu_run(struc
 	 * is no need to worry about the conditional branch over the wrmsr
 	 * being speculatively taken.
 	 */
-	if (vmx->spec_ctrl)
-		native_wrmsrl(MSR_IA32_SPEC_CTRL, vmx->spec_ctrl);
+	x86_set_guest_spec_ctrl(vmx->spec_ctrl);
 
 	vmx->__launched = vmx->loaded_vmcs->launched;
 
@@ -9875,8 +9874,7 @@ static void __noclone vmx_vcpu_run(struc
 	if (unlikely(!msr_write_intercepted(vcpu, MSR_IA32_SPEC_CTRL)))
 		vmx->spec_ctrl = native_read_msr(MSR_IA32_SPEC_CTRL);
 
-	if (vmx->spec_ctrl)
-		native_wrmsrl(MSR_IA32_SPEC_CTRL, 0);
+	x86_restore_host_spec_ctrl(vmx->spec_ctrl);
 
 	/* Eliminate branch target predictions from guest mode */
 	vmexit_fill_RSB();
#7 lore raw

PATCH v9 6/16 SSB 6

In reply to Thomas Gleixner

It does not do much except show the words 'Vulnerable' for recent x86 cores. Intel cores prior to family 6 are known not to be vulnerable, and so are some Atoms and some Xeon Phi.

It assumes that older Cyrix, Centaur, etc. cores are immune.

---
v1.3: Remove AMD

    s/md/mdd/

v1.4: s/mdd/sbb/
v3: s/SSB/SPEC_STORE_BYPASS
Rework the logic in cpu_set_bug_bits to be inverse.
v4: Expanded the not affected array
- s/X86_BUG_CPU_SPEC_STORE_BYPASS/X86_BUG_SPEC_STORE_BYPASS/

---
 Documentation/ABI/testing/sysfs-devices-system-cpu |    1 
 arch/x86/include/asm/cpufeatures.h                 |    1 
 arch/x86/kernel/cpu/bugs.c                         |    5 ++++
 arch/x86/kernel/cpu/common.c                       |   23 +++++++++++++++++++++
 drivers/base/cpu.c                                 |    8 +++++++
 include/linux/cpu.h                                |    2 +
 6 files changed, 40 insertions(+)
Documentation/ABI/testing/sysfs-devices-system-cpu +1 -0
@@ -478,6 +478,7 @@ What:		/sys/devices/system/cpu/vulnerabi
 		/sys/devices/system/cpu/vulnerabilities/meltdown
 		/sys/devices/system/cpu/vulnerabilities/spectre_v1
 		/sys/devices/system/cpu/vulnerabilities/spectre_v2
+		/sys/devices/system/cpu/vulnerabilities/spec_store_bypass
 Date:		January 2018
 Contact:	Linux kernel mailing list <[email protected]>
 Description:	Information about CPU vulnerabilities
arch/x86/include/asm/cpufeatures.h +1 -0
@@ -363,5 +363,6 @@
 #define X86_BUG_CPU_MELTDOWN		X86_BUG(14) /* CPU is affected by meltdown attack and needs kernel page table isolation */
 #define X86_BUG_SPECTRE_V1		X86_BUG(15) /* CPU is affected by Spectre variant 1 attack with conditional branches */
 #define X86_BUG_SPECTRE_V2		X86_BUG(16) /* CPU is affected by Spectre variant 2 attack with indirect branches */
+#define X86_BUG_SPEC_STORE_BYPASS	X86_BUG(17) /* CPU is affected by speculative store bypass attack */
 
 #endif /* _ASM_X86_CPUFEATURES_H */
arch/x86/kernel/cpu/bugs.c +5 -0
@@ -404,4 +404,9 @@ ssize_t cpu_show_spectre_v2(struct devic
 {
 	return cpu_show_common(dev, attr, buf, X86_BUG_SPECTRE_V2);
 }
+
+ssize_t cpu_show_spec_store_bypass(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	return cpu_show_common(dev, attr, buf, X86_BUG_SPEC_STORE_BYPASS);
+}
 #endif
arch/x86/kernel/cpu/common.c +23 -0
@@ -923,10 +923,33 @@ static const __initconst struct x86_cpu_
 	{}
 };
 
+static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = {
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_PINEVIEW },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_LINCROFT },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_PENWELL },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_CLOVERVIEW },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_CEDARVIEW },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_SILVERMONT1 },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_AIRMONT },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_SILVERMONT2 },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_MERRIFIELD },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_CORE_YONAH },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_XEON_PHI_KNL },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_XEON_PHI_KNM },
+	{ X86_VENDOR_CENTAUR,	5 },
+	{ X86_VENDOR_INTEL,	5 },
+	{ X86_VENDOR_NSC,	5 },
+	{ X86_VENDOR_ANY,	4 },
+	{}
+};
+
 static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
 {
 	u64 ia32_cap = 0;
 
+	if (!x86_match_cpu(cpu_no_spec_store_bypass))
+		setup_force_cpu_bug(X86_BUG_SPEC_STORE_BYPASS);
+
 	if (x86_match_cpu(cpu_no_speculation))
 		return;
 
drivers/base/cpu.c +8 -0
@@ -534,14 +534,22 @@ ssize_t __weak cpu_show_spectre_v2(struc
 	return sprintf(buf, "Not affected\n");
 }
 
+ssize_t __weak cpu_show_spec_store_bypass(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);
+static DEVICE_ATTR(spec_store_bypass, 0444, cpu_show_spec_store_bypass, NULL);
 
 static struct attribute *cpu_root_vulnerabilities_attrs[] = {
 	&dev_attr_meltdown.attr,
 	&dev_attr_spectre_v1.attr,
 	&dev_attr_spectre_v2.attr,
+	&dev_attr_spec_store_bypass.attr,
 	NULL
 };
 
include/linux/cpu.h +2 -0
@@ -53,6 +53,8 @@ extern ssize_t cpu_show_spectre_v1(struc
 				   struct device_attribute *attr, char *buf);
 extern ssize_t cpu_show_spectre_v2(struct device *dev,
 				   struct device_attribute *attr, char *buf);
+extern ssize_t cpu_show_spec_store_bypass(struct device *dev,
+					  struct device_attribute *attr, char *buf);
 
 extern __printf(4, 5)
 struct device *cpu_device_create(struct device *parent, void *drvdata,
#8 lore raw

PATCH v9 7/16 SSB 7

In reply to Thomas Gleixner

Add the CPU feature bit CPUID.7.0.EDX[31] which indicates whether the CPU supports Reduced Data Speculation.

[ tglx: Split it out from a later patch ]

---
 arch/x86/include/asm/cpufeatures.h |    1 +
 1 file changed, 1 insertion(+)
arch/x86/include/asm/cpufeatures.h +1 -0
@@ -334,6 +334,7 @@
 #define X86_FEATURE_SPEC_CTRL		(18*32+26) /* "" Speculation Control (IBRS + IBPB) */
 #define X86_FEATURE_INTEL_STIBP		(18*32+27) /* "" Single Thread Indirect Branch Predictors */
 #define X86_FEATURE_ARCH_CAPABILITIES	(18*32+29) /* IA32_ARCH_CAPABILITIES MSR (Intel) */
+#define X86_FEATURE_RDS			(18*32+31) /* Reduced Data Speculation */
 
 /*
  * BUG word(s)
#9 lore raw

PATCH v9 8/16 SSB 8

In reply to Thomas Gleixner

Contemporary high performance processors use a common industry-wide optimization known as "Speculative Store Bypass" in which loads from addresses to which a recent store has occurred may (speculatively) see an older value. Intel refers to this feature as "Memory Disambiguation" which is part of their "Smart Memory Access" capability.

Memory Disambiguation can expose a cache side-channel attack against such speculatively read values. An attacker can create exploit code that allows them to read memory outside of a sandbox environment (for example, malicious JavaScript in a web page), or to perform more complex attacks against code running within the same privilege level, e.g. via the stack.

Provide two command line control knobs:

nospec_store_bypass_disable
spec_store_bypass_disable=[off,auto,on]

By default affected x86 processors will power on with Speculative Store Bypass enabled. Hence the provided kernel parameters are written from the point of view of whether to enable a mitigation or not. The parameters are as follows:

- auto - Kernel detects whether your CPU model contains a implementation of Speculative Store Bypass and picks the most appropriate mitigation.

- on - disable Speculative Store Bypass
- off - enable Speculative Store Bypass

[ tglx: Reordered the checks so that the whole evaluation is not done
when the CPU does not support RDS ]

---
v1.3:
- Wrong array was used when figuring mdd_v4= arguments
- If broken microcode found, also disable MD feature.
- s/mdd_v4/mdd/
- s/kernel/userpsace/
- Fix up the commit description
v1.4:
- Incorporate various feedback and suggestions from Jon Masters
- Change mdd parameters to allow only big hammer on/off for now
- Introduce "ssbd" industry standard term for cross-architecture
- Use mdd_ and MDD_ prefixes to clearly spell out that "enable" means we are actually disabling a CPU processor feature (MD)
v2:
- s/mdd/ssb/ in the x86 code to match with the 'ssb' part
v3:
- rip out mdd.
- remove 'Haswell' and such and replace with 'contemporary'
- s/ssb/spec_store_bypass/ and various other cleanups (Jon Masters)
v4: Rip out 'Memory Disambiguation Disable'
- Fix up documentation.
- Rip out the X86_FEATURE_STBUF_BYPASS
- s/X86_FEATURE_STBUF_MITIGATE/X86_FEATURE_STBUF_BYPASS_MITIGATE/
- On the functions change spec_store_bypass_ to ssb_
v5: Changed title.
- s/X86_FEATURE_STBUF_BYPASS_MITIGATE/X86_FEATURE_SPEC_STORE_BYPASS_DISABLE/
- Add logic to get out of ssb_parse_cmdline is bug is not present

---
 Documentation/admin-guide/kernel-parameters.txt |   33 +++++++
 arch/x86/include/asm/cpufeatures.h              |    1 
 arch/x86/include/asm/nospec-branch.h            |    6 +
 arch/x86/kernel/cpu/bugs.c                      |  103 ++++++++++++++++++++++++
 4 files changed, 143 insertions(+)
Documentation/admin-guide/kernel-parameters.txt +33 -0
@@ -2680,6 +2680,9 @@
 			allow data leaks with this option, which is equivalent
 			to spectre_v2=off.
 
+	nospec_store_bypass_disable
+			[HW] Disable all mitigations for the Speculative Store Bypass vulnerability
+
 	noxsave		[BUGS=X86] Disables x86 extended register state save
 			and restore using xsave. The kernel will fallback to
 			enabling legacy floating-point and sse state.
@@ -4025,6 +4028,36 @@
 			Not specifying this option is equivalent to
 			spectre_v2=auto.
 
+	spec_store_bypass_disable=
+			[HW] Control Speculative Store Bypass (SSB) Disable mitigation
+			(Speculative Store Bypass vulnerability)
+
+			Certain CPUs are vulnerable to an exploit against a
+			a common industry wide performance optimization known
+			as "Speculative Store Bypass" in which recent stores
+			to the same memory location may not be observed by
+			later loads during speculative execution. The idea
+			is that such stores are unlikely and that they can
+			be detected prior to instruction retirement at the
+			end of a particular speculation execution window.
+
+			In vulnerable processors, the speculatively forwarded
+			store can be used in a cache side channel attack, for
+			example to read memory to which the attacker does not
+			directly have access (e.g. inside sandboxed code).
+
+			This parameter controls whether the Speculative Store
+			Bypass optimization is used.
+
+			on     - Unconditionally disable Speculative Store Bypass
+			off    - Unconditionally enable Speculative Store Bypass
+			auto   - Kernel detects whether the CPU model contains a
+				 implementation of Speculative Store Bypass and
+				 picks the most appropriate mitigation
+
+			Not specifying this option is equivalent to
+			spec_store_bypass_disable=auto.
+
 	spia_io_base=	[HW,MTD]
 	spia_fio_base=
 	spia_pedr=
arch/x86/include/asm/cpufeatures.h +1 -0
@@ -214,6 +214,7 @@
 
 #define X86_FEATURE_USE_IBPB		( 7*32+21) /* "" Indirect Branch Prediction Barrier enabled */
 #define X86_FEATURE_USE_IBRS_FW		( 7*32+22) /* "" Use IBRS during runtime firmware calls */
+#define X86_FEATURE_SPEC_STORE_BYPASS_DISABLE	( 7*32+23) /* "" Disable Speculative Store Bypass. */
 
 /* Virtualization flags: Linux defined, word 8 */
 #define X86_FEATURE_TPR_SHADOW		( 8*32+ 0) /* Intel TPR Shadow */
arch/x86/include/asm/nospec-branch.h +6 -0
@@ -238,6 +238,12 @@ extern u64 x86_get_default_spec_ctrl(voi
 extern void x86_set_guest_spec_ctrl(u64);
 extern void x86_restore_host_spec_ctrl(u64);
 
+/* The Speculative Store Bypass disable variants */
+enum ssb_mitigation {
+	SPEC_STORE_BYPASS_NONE,
+	SPEC_STORE_BYPASS_DISABLE,
+};
+
 extern char __indirect_thunk_start[];
 extern char __indirect_thunk_end[];
 
arch/x86/kernel/cpu/bugs.c +103 -0

127 lines not on this page · raw message

#10 lore raw

PATCH v9 9/16 SSB 9

In reply to Thomas Gleixner

Intel CPUs expose methods to:

- Detect whether Reduced Data Speculation capability is available via CPUID.7.0.EDX[31],

- The SPEC_CTRL MSR(0x48), bit 2 set to enable Reduced Data Speculation.

- MSR_IA32_ARCH_CAPABILITIES, Bit(4) no need to enable Reduced Data Speculation.

With that in mind if spec_store_bypass_disable=[auto,on] is selected set at boot-time the SPEC_CTRL MSR to enable Reduced Data Speculation if the platform requires it.

Note that this does not fix the KVM case where the SPEC_CTRL is exposed to guests which can muck with, see patch titled : KVM/SVM/VMX/x86/spectre_v2:
Support the combination of guest and host IBRS.

And for the firmware (IBRS to be set), see patch titled:
x86/spectre_v2: Read SPEC_CTRL MSR during boot and re-use reserved bits

[ tglx: Distangled it from the intel implementation and kept the call order ]

---

v1.2: Expand on the commit description
s/md_v4/mdd/
s/spec_ctrl_msr_on/spec_ctrl_priv/
s/spec_ctrl_msr_off/spec_ctrp_unpriv/

v1.3:
- Add comment about privilege level changes.

v1.4: Simplify and incorporate various suggestions from Jon Masters
- Export a single x86_spec_ctrl_base value with initial bits

v2: Rip out the c_fix_cpu.
Depend on synthetic CPU flag
v3: Move the generic_identify to be done _after_ we figure out whether
we can do the mitigation.
v4: s/MDD/RDS/
s/Memory Disambiguation Disable/Reduced Data Speculation/
Tweak the various 'disable', enabled now that it is called RDS.
Set the x86_spec_ctrl with SPEC_CTRL_RDS if RDS is detected
Fixup x86_set_spec_ctrl to deal with two Bitfields.
v5: s/X86_FEATURE_DISABLE_SSB/X86_FEATURE_SPEC_STORE_BYPASS_DISABLE/
Also check MSR_IA32_ARCH_CAPABILITIES for Bit(4)
Add documentation on what those three flags mean
Add docs on why we set x86_spec_ctrl only on Intel
Add extra check in ssb_parse_cmdline for RDS be available
In init_intel drop the check for RDS as the X86_FEATURE_SPEC_STORE_BYPASS_DISABLE
is implicitly set only iff RDS has been set in ssb_parse_cmdline.

---
 arch/x86/include/asm/msr-index.h |    2 ++
 arch/x86/kernel/cpu/bugs.c       |   30 ++++++++++++++++++++++++++++--
 arch/x86/kernel/cpu/common.c     |   10 ++++++----
 arch/x86/kernel/cpu/cpu.h        |    2 ++
 arch/x86/kernel/cpu/intel.c      |    1 +
 5 files changed, 39 insertions(+), 6 deletions(-)
arch/x86/include/asm/msr-index.h +2 -0
@@ -42,6 +42,7 @@
 #define MSR_IA32_SPEC_CTRL		0x00000048 /* Speculation Control */
 #define SPEC_CTRL_IBRS			(1 << 0)   /* Indirect Branch Restricted Speculation */
 #define SPEC_CTRL_STIBP			(1 << 1)   /* Single Thread Indirect Branch Predictors */
+#define SPEC_CTRL_RDS			(1 << 2)   /* Reduced Data Speculation */
 
 #define MSR_IA32_PRED_CMD		0x00000049 /* Prediction Command */
 #define PRED_CMD_IBPB			(1 << 0)   /* Indirect Branch Prediction Barrier */
@@ -68,6 +69,7 @@
 #define MSR_IA32_ARCH_CAPABILITIES	0x0000010a
 #define ARCH_CAP_RDCL_NO		(1 << 0)   /* Not susceptible to Meltdown */
 #define ARCH_CAP_IBRS_ALL		(1 << 1)   /* Enhanced IBRS support */
+#define ARCH_CAP_RDS_NO			(1 << 4)   /* Not susceptible to speculative store bypass */
 
 #define MSR_IA32_BBL_CR_CTL		0x00000119
 #define MSR_IA32_BBL_CR_CTL3		0x0000011e
arch/x86/kernel/cpu/bugs.c +28 -2
@@ -117,7 +117,7 @@ static enum spectre_v2_mitigation spectr
 
 void x86_set_spec_ctrl(u64 val)
 {
-	if (val & ~(SPEC_CTRL_IBRS))
+	if (val & ~(SPEC_CTRL_IBRS | SPEC_CTRL_RDS))
 		WARN_ONCE(1, "SPEC_CTRL MSR value 0x%16llx is unknown.\n", val);
 	else
 		wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base | val);
@@ -444,8 +444,28 @@ static enum ssb_mitigation_cmd __init __
 		break;
 	}
 
-	if (mode != SPEC_STORE_BYPASS_NONE)
+	/*
+	 * We have three CPU feature flags that are in play here:
+	 *  - X86_BUG_SPEC_STORE_BYPASS - CPU is susceptible.
+	 *  - X86_FEATURE_RDS - CPU is able to turn off speculative store bypass
+	 *  - X86_FEATURE_SPEC_STORE_BYPASS_DISABLE - engage the mitigation
+	 */
+	if (mode != SPEC_STORE_BYPASS_NONE) {
 		setup_force_cpu_cap(X86_FEATURE_SPEC_STORE_BYPASS_DISABLE);
+		/*
+		 * Intel uses the SPEC CTRL MSR Bit(2) for this, while AMD uses
+		 * a completely different MSR and bit dependent on family.
+		 */
+		switch (boot_cpu_data.x86_vendor) {
+		case X86_VENDOR_INTEL:
+			x86_spec_ctrl_base |= SPEC_CTRL_RDS;
+			x86_set_spec_ctrl(SPEC_CTRL_RDS);
+			break;
+		case X86_VENDOR_AMD:
+			break;
+		}
+	}
+
 	return mode;
 }
 
@@ -459,6 +479,12 @@ static void ssb_select_mitigation()
 
 #undef pr_fmt
 
+void x86_setup_ap_spec_ctrl(void)
+{
+	if (boot_cpu_has(X86_FEATURE_IBRS))
+		x86_set_spec_ctrl(x86_spec_ctrl_base & (SPEC_CTRL_IBRS | SPEC_CTRL_RDS));
+}
+
 #ifdef CONFIG_SYSFS
 
 ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr,
arch/x86/kernel/cpu/common.c +6 -4
@@ -947,7 +947,11 @@ static void __init cpu_set_bug_bits(stru
 {
 	u64 ia32_cap = 0;
 
-	if (!x86_match_cpu(cpu_no_spec_store_bypass))
+	if (cpu_has(c, X86_FEATURE_ARCH_CAPABILITIES))
+		rdmsrl(MSR_IA32_ARCH_CAPABILITIES, ia32_cap);
+
+	if (!x86_match_cpu(cpu_no_spec_store_bypass) &&
+	   !(ia32_cap & ARCH_CAP_RDS_NO))
 		setup_force_cpu_bug(X86_BUG_SPEC_STORE_BYPASS);
 
 	if (x86_match_cpu(cpu_no_speculation))
@@ -959,9 +963,6 @@ static void __init cpu_set_bug_bits(stru
 	if (x86_match_cpu(cpu_no_meltdown))
 		return;
 
-	if (cpu_has(c, X86_FEATURE_ARCH_CAPABILITIES))
-		rdmsrl(MSR_IA32_ARCH_CAPABILITIES, ia32_cap);
-
 	/* Rogue Data Cache Load? No! */
 	if (ia32_cap & ARCH_CAP_RDCL_NO)
 		return;
@@ -1379,6 +1380,7 @@ void identify_secondary_cpu(struct cpuin
 #endif
 	mtrr_ap_init();
 	validate_apic_and_package_id(c);
+	x86_setup_ap_spec_ctrl();
 }
 
 static __init int setup_noclflush(char *arg)
arch/x86/kernel/cpu/cpu.h +2 -0
@@ -50,4 +50,6 @@ extern void cpu_detect_cache_sizes(struc
 
 unsigned int aperfmperf_get_khz(int cpu);
 
+extern void x86_setup_ap_spec_ctrl(void);
+
 #endif /* ARCH_X86_CPU_H */
arch/x86/kernel/cpu/intel.c +1 -0
@@ -189,6 +189,7 @@ static void early_init_intel(struct cpui
 		setup_clear_cpu_cap(X86_FEATURE_STIBP);
 		setup_clear_cpu_cap(X86_FEATURE_SPEC_CTRL);
 		setup_clear_cpu_cap(X86_FEATURE_INTEL_STIBP);
+		setup_clear_cpu_cap(X86_FEATURE_RDS);
 	}
 
 	/*
#11 lore raw

PATCH v9 10/16 SSB 10

In reply to Thomas Gleixner

Intel and AMD SPEC_CTRL (0x48) MSR semantics may differ in the future (or in fact use different MSRs for the same functionality).

As such a run-time mechanism is required to whitelist the appropriate MSR values.

[ tglx: Made the variable __ro_after_init ]

---
v5: New patch

---
 arch/x86/kernel/cpu/bugs.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/bugs.c +9 -2
@@ -35,6 +35,12 @@ static void __init ssb_select_mitigation
  */
 static u64 __ro_after_init x86_spec_ctrl_base;
 
+/*
+ * The vendor and possibly platform specific bits which can be modified in
+ * x86_spec_ctrl_base.
+ */
+static u64 __ro_after_init x86_spec_ctrl_mask = ~(SPEC_CTRL_IBRS);
+
 void __init check_bugs(void)
 {
 	identify_boot_cpu();
@@ -117,7 +123,7 @@ static enum spectre_v2_mitigation spectr
 
 void x86_set_spec_ctrl(u64 val)
 {
-	if (val & ~(SPEC_CTRL_IBRS | SPEC_CTRL_RDS))
+	if (val & x86_spec_ctrl_mask)
 		WARN_ONCE(1, "SPEC_CTRL MSR value 0x%16llx is unknown.\n", val);
 	else
 		wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base | val);
@@ -459,6 +465,7 @@ static enum ssb_mitigation_cmd __init __
 		switch (boot_cpu_data.x86_vendor) {
 		case X86_VENDOR_INTEL:
 			x86_spec_ctrl_base |= SPEC_CTRL_RDS;
+			x86_spec_ctrl_mask &= ~(SPEC_CTRL_RDS);
 			x86_set_spec_ctrl(SPEC_CTRL_RDS);
 			break;
 		case X86_VENDOR_AMD:
@@ -482,7 +489,7 @@ static void ssb_select_mitigation()
 void x86_setup_ap_spec_ctrl(void)
 {
 	if (boot_cpu_has(X86_FEATURE_IBRS))
-		x86_set_spec_ctrl(x86_spec_ctrl_base & (SPEC_CTRL_IBRS | SPEC_CTRL_RDS));
+		x86_set_spec_ctrl(x86_spec_ctrl_base & ~x86_spec_ctrl_mask);
 }
 
 #ifdef CONFIG_SYSFS
#12 lore raw

PATCH v9 11/16 SSB 11

In reply to Thomas Gleixner

AMD does not need the Speculative Store Bypass mitigation to be enabled.

The parameters for this are already available and can be done via MSR C001_1020. Each family uses a different bit in that MSR for this.

[ tglx: Expose the bit mask via a variable and move the actual MSR fiddling
into the bugs code as that's the right thing to do and also required to prepare for dynamic enable/disable ]

---
v2: New patch
v3: Use the right CPU features
Move the whole logic in early_init_amd and init_amd
v5: Set X86_FEATURE_RDS on Fam16->17h
Change title to have 'bugs' in it.
v7: Use a bit mask and a cached base
v8: Fix the bit mask ....

---
 arch/x86/include/asm/cpufeatures.h   |    1 +
 arch/x86/include/asm/nospec-branch.h |    4 ++++
 arch/x86/kernel/cpu/amd.c            |   21 +++++++++++++++++++++
 arch/x86/kernel/cpu/bugs.c           |   27 ++++++++++++++++++++++++++-
 4 files changed, 52 insertions(+), 1 deletion(-)
arch/x86/include/asm/cpufeatures.h +1 -0
@@ -215,6 +215,7 @@
 #define X86_FEATURE_USE_IBPB		( 7*32+21) /* "" Indirect Branch Prediction Barrier enabled */
 #define X86_FEATURE_USE_IBRS_FW		( 7*32+22) /* "" Use IBRS during runtime firmware calls */
 #define X86_FEATURE_SPEC_STORE_BYPASS_DISABLE	( 7*32+23) /* "" Disable Speculative Store Bypass. */
+#define X86_FEATURE_AMD_RDS		(7*32+24)  /* "" AMD RDS implementation */
 
 /* Virtualization flags: Linux defined, word 8 */
 #define X86_FEATURE_TPR_SHADOW		( 8*32+ 0) /* Intel TPR Shadow */
arch/x86/include/asm/nospec-branch.h +4 -0
@@ -244,6 +244,10 @@ enum ssb_mitigation {
 	SPEC_STORE_BYPASS_DISABLE,
 };
 
+/* AMD specific Speculative Store Bypass MSR data */
+extern u64 x86_amd_ls_cfg_base;
+extern u64 x86_amd_ls_cfg_rds_mask;
+
 extern char __indirect_thunk_start[];
 extern char __indirect_thunk_end[];
 
arch/x86/kernel/cpu/amd.c +21 -0
@@ -10,6 +10,7 @@
 #include <asm/processor.h>
 #include <asm/apic.h>
 #include <asm/cpu.h>
+#include <asm/nospec-branch.h>
 #include <asm/smp.h>
 #include <asm/pci-direct.h>
 #include <asm/delay.h>
@@ -673,6 +674,26 @@ static void early_init_amd(struct cpuinf
 		set_cpu_bug(c, X86_BUG_AMD_E400);
 
 	early_detect_mem_encrypt(c);
+
+	if (c->x86 >= 0x15 && c->x86 <= 0x17) {
+		unsigned int bit;
+
+		switch (c->x86) {
+		case 0x15: bit = 54; break;
+		case 0x16: bit = 33; break;
+		case 0x17: bit = 10; break;
+		default: return;
+		}
+		/*
+		 * Try to cache the base value so further operations can
+		 * avoid RMW. If that faults, do not enable RDS.
+		 */
+		if (!rdmsrl_safe(MSR_AMD64_LS_CFG, &x86_amd_ls_cfg_base)) {
+			set_cpu_cap(c, X86_FEATURE_RDS);
+			set_cpu_cap(c, X86_FEATURE_AMD_RDS);
+			x86_amd_ls_cfg_rds_mask = (1ULL << bit);
+		}
+	}
 }
 
 static void init_amd_k8(struct cpuinfo_x86 *c)
arch/x86/kernel/cpu/bugs.c +26 -1
@@ -41,6 +41,13 @@ static u64 __ro_after_init x86_spec_ctrl
  */
 static u64 __ro_after_init x86_spec_ctrl_mask = ~(SPEC_CTRL_IBRS);
 
+/*
+ * AMD specific MSR info for Store Bypass control.  x86_amd_ls_cfg_rds_mask
+ * is initialized in identify_boot_cpu().
+ */
+u64 __ro_after_init x86_amd_ls_cfg_base;
+u64 __ro_after_init x86_amd_ls_cfg_rds_mask;
+
 void __init check_bugs(void)
 {
 	identify_boot_cpu();
@@ -52,7 +59,8 @@ void __init check_bugs(void)
 
 	/*
 	 * Read the SPEC_CTRL MSR to account for reserved bits which may
-	 * have unknown values.
+	 * have unknown values. AMD64_LS_CFG msr is cached in the early AMD
+	 * init code as it is not enumerated and depends on the family.
 	 */
 	if (boot_cpu_has(X86_FEATURE_IBRS))
 		rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
@@ -154,6 +162,14 @@ void x86_restore_host_spec_ctrl(u64 gues
 }
 EXPORT_SYMBOL_GPL(x86_restore_host_spec_ctrl);
 
+static void x86_amd_rds_enable(void)
+{
+	u64 msrval = x86_amd_ls_cfg_base | x86_amd_ls_cfg_rds_mask;
+
+	if (boot_cpu_has(X86_FEATURE_AMD_RDS))
+		wrmsrl(MSR_AMD64_LS_CFG, msrval);
+}
+
 #ifdef RETPOLINE
 static bool spectre_v2_bad_module;
 
@@ -443,6 +459,11 @@ static enum ssb_mitigation_cmd __init __
 
 	switch (cmd) {
 	case SPEC_STORE_BYPASS_CMD_AUTO:
+		/*
+		 * AMD platforms by default don't need SSB mitigation.
+		 */
+		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+			break;
 	case SPEC_STORE_BYPASS_CMD_ON:
 		mode = SPEC_STORE_BYPASS_DISABLE;
 		break;
@@ -469,6 +490,7 @@ static enum ssb_mitigation_cmd __init __
 			x86_set_spec_ctrl(SPEC_CTRL_RDS);
 			break;
 		case X86_VENDOR_AMD:
+			x86_amd_rds_enable();
 			break;
 		}
 	}
@@ -490,6 +512,9 @@ void x86_setup_ap_spec_ctrl(void)
 {
 	if (boot_cpu_has(X86_FEATURE_IBRS))
 		x86_set_spec_ctrl(x86_spec_ctrl_base & ~x86_spec_ctrl_mask);
+
+	if (ssb_mode == SPEC_STORE_BYPASS_DISABLE)
+		x86_amd_rds_enable();
 }
 
 #ifdef CONFIG_SYSFS
#13 lore raw

Re: [patch V9 11/16] SSB 11

Review

Quoted 6 lines · speck for Thomas Gleixner
(quoted diff) +6 -0
+/*
+ * AMD specific MSR info for Store Bypass control.  x86_amd_ls_cfg_rds_mask
+ * is initialized in identify_boot_cpu().
+ */
+u64 __ro_after_init x86_amd_ls_cfg_base;
+u64 __ro_after_init x86_amd_ls_cfg_rds_mask;

That breaks CPU hotplug after boot due to __ro_after_init.

Fix below.

Thanks,

tglx

8<------------------

arch/x86/kernel/cpu/amd.c +25 -20
@@ -555,6 +555,26 @@ static void bsp_init_amd(struct cpuinfo_
 		rdmsrl(MSR_FAM10H_NODE_ID, value);
 		nodes_per_socket = ((value >> 3) & 7) + 1;
 	}
+
+	if (c->x86 >= 0x15 && c->x86 <= 0x17) {
+		unsigned int bit;
+
+		switch (c->x86) {
+		case 0x15: bit = 54; break;
+		case 0x16: bit = 33; break;
+		case 0x17: bit = 10; break;
+		default: return;
+		}
+		/*
+		 * Try to cache the base value so further operations can
+		 * avoid RMW. If that faults, do not enable RDS.
+		 */
+		if (!rdmsrl_safe(MSR_AMD64_LS_CFG, &x86_amd_ls_cfg_base)) {
+			set_cpu_cap(c, X86_FEATURE_RDS);
+			set_cpu_cap(c, X86_FEATURE_AMD_RDS);
+			x86_amd_ls_cfg_rds_mask = (1ULL << bit);
+		}
+	}
 }
 
 static void early_detect_mem_encrypt(struct cpuinfo_x86 *c)
@@ -674,26 +694,6 @@ static void early_init_amd(struct cpuinf
 		set_cpu_bug(c, X86_BUG_AMD_E400);
 
 	early_detect_mem_encrypt(c);
-
-	if (c->x86 >= 0x15 && c->x86 <= 0x17) {
-		unsigned int bit;
-
-		switch (c->x86) {
-		case 0x15: bit = 54; break;
-		case 0x16: bit = 33; break;
-		case 0x17: bit = 10; break;
-		default: return;
-		}
-		/*
-		 * Try to cache the base value so further operations can
-		 * avoid RMW. If that faults, do not enable RDS.
-		 */
-		if (!rdmsrl_safe(MSR_AMD64_LS_CFG, &x86_amd_ls_cfg_base)) {
-			set_cpu_cap(c, X86_FEATURE_RDS);
-			set_cpu_cap(c, X86_FEATURE_AMD_RDS);
-			x86_amd_ls_cfg_rds_mask = (1ULL << bit);
-		}
-	}
 }
 
 static void init_amd_k8(struct cpuinfo_x86 *c)
@@ -919,6 +919,11 @@ static void init_amd(struct cpuinfo_x86
 	/* AMD CPUs don't reset SS attributes on SYSRET, Xen does. */
 	if (!cpu_has(c, X86_FEATURE_XENPV))
 		set_cpu_bug(c, X86_BUG_SYSRET_SS_ATTRS);
+
+	if (boot_cpu_has(X86_FEATURE_AMD_RDS)) {
+		set_cpu_cap(c, X86_FEATURE_RDS);
+		set_cpu_cap(c, X86_FEATURE_AMD_RDS);
+	}
 }
 
 #ifdef CONFIG_X86_32
#14 lore raw

PATCH v9 12/16 SSB 12

In reply to Thomas Gleixner

Expose the CPUID.7.EDX[31] bit to the guest, and also guard against various combinations of SPEC_CTRL MSR values.

The handling of the MSR (to take into account the host value of SPEC_CTRL Bit(2)) is taken care of in patch:

KVM/SVM/VMX/x86/spectre_v2: Support the combination of guest and host IBRS

---
v2: New patch.
v4: s/MDD/RDS/

---
 arch/x86/kvm/cpuid.c |    2 +-
 arch/x86/kvm/vmx.c   |    8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)
arch/x86/kvm/cpuid.c +1 -1
@@ -407,7 +407,7 @@ static inline int __do_cpuid_ent(struct
 
 	/* cpuid 7.0.edx*/
 	const u32 kvm_cpuid_7_0_edx_x86_features =
-		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
+		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) | F(RDS) |
 		F(ARCH_CAPABILITIES);
 
 	/* all calls to cpuid_count() should be made on the same cpu */
arch/x86/kvm/vmx.c +5 -3
@@ -3524,7 +3524,8 @@ static int vmx_get_msr(struct kvm_vcpu *
 	case MSR_IA32_SPEC_CTRL:
 		if (!msr_info->host_initiated &&
 		    !guest_cpuid_has(vcpu, X86_FEATURE_IBRS) &&
-		    !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
+		    !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
+		    !guest_cpuid_has(vcpu, X86_FEATURE_RDS))
 			return 1;
 
 		msr_info->data = to_vmx(vcpu)->spec_ctrl;
@@ -3643,11 +3644,12 @@ static int vmx_set_msr(struct kvm_vcpu *
 	case MSR_IA32_SPEC_CTRL:
 		if (!msr_info->host_initiated &&
 		    !guest_cpuid_has(vcpu, X86_FEATURE_IBRS) &&
-		    !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
+		    !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
+		    !guest_cpuid_has(vcpu, X86_FEATURE_RDS))
 			return 1;
 
 		/* The STIBP bit doesn't fault even if it's not advertised */
-		if (data & ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP))
+		if (data & ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | SPEC_CTRL_RDS))
 			return 1;
 
 		vmx->spec_ctrl = data;
#15 lore raw

PATCH v9 13/16 SSB 13

In reply to Thomas Gleixner

Having everything in nospec-branch.h creates a hell of dependencies when adding the prctl based switching mechanism. Move everything which is not required in nospec-branch.h to spec-ctrl.h and fixup the includes in the relevant files.

---
 arch/x86/include/asm/nospec-branch.h |   14 --------------
 arch/x86/include/asm/spec-ctrl.h      |   21 +++++++++++++++++++++
 arch/x86/kernel/cpu/amd.c            |    2 +-
 arch/x86/kernel/cpu/bugs.c           |    2 +-
 arch/x86/kvm/svm.c                   |    2 +-
 arch/x86/kvm/vmx.c                   |    2 +-
 6 files changed, 25 insertions(+), 18 deletions(-)
arch/x86/include/asm/nospec-branch.h +0 -14
@@ -228,26 +228,12 @@ enum spectre_v2_mitigation {
 extern void x86_set_spec_ctrl(u64);
 extern u64 x86_get_default_spec_ctrl(void);
 
-/*
- * On VMENTER we must preserve whatever view of the SPEC_CTRL MSR
- * the guest has, while on VMEXIT we restore the host view. This
- * would be easier if SPEC_CTRL were architecturally maskable or
- * shadowable for guests but this is not (currently) the case.
- * Takes the guest view of SPEC_CTRL MSR as a parameter.
- */
-extern void x86_set_guest_spec_ctrl(u64);
-extern void x86_restore_host_spec_ctrl(u64);
-
 /* The Speculative Store Bypass disable variants */
 enum ssb_mitigation {
 	SPEC_STORE_BYPASS_NONE,
 	SPEC_STORE_BYPASS_DISABLE,
 };
 
-/* AMD specific Speculative Store Bypass MSR data */
-extern u64 x86_amd_ls_cfg_base;
-extern u64 x86_amd_ls_cfg_rds_mask;
-
 extern char __indirect_thunk_start[];
 extern char __indirect_thunk_end[];
 
arch/x86/include/asm/spec-ctrl.h new file +21 -0
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_SPECCTRL_H_
+#define _ASM_X86_SPECCTRL_H_
+
+#include <asm/nospec-branch.h>
+
+/*
+ * On VMENTER we must preserve whatever view of the SPEC_CTRL MSR
+ * the guest has, while on VMEXIT we restore the host view. This
+ * would be easier if SPEC_CTRL were architecturally maskable or
+ * shadowable for guests but this is not (currently) the case.
+ * Takes the guest view of SPEC_CTRL MSR as a parameter.
+ */
+extern void x86_set_guest_spec_ctrl(u64);
+extern void x86_restore_host_spec_ctrl(u64);
+
+/* AMD specific Speculative Store Bypass MSR data */
+extern u64 x86_amd_ls_cfg_base;
+extern u64 x86_amd_ls_cfg_rds_mask;
+
+#endif
arch/x86/kernel/cpu/amd.c +1 -1
@@ -10,7 +10,7 @@
 #include <asm/processor.h>
 #include <asm/apic.h>
 #include <asm/cpu.h>
-#include <asm/nospec-branch.h>
+#include <asm/spec-ctrl.h>
 #include <asm/smp.h>
 #include <asm/pci-direct.h>
 #include <asm/delay.h>
arch/x86/kernel/cpu/bugs.c +1 -1
@@ -13,7 +13,7 @@
 #include <linux/cpu.h>
 #include <linux/module.h>
 
-#include <asm/nospec-branch.h>
+#include <asm/spec-ctrl.h>
 #include <asm/cmdline.h>
 #include <asm/bugs.h>
 #include <asm/processor.h>
arch/x86/kvm/svm.c +1 -1
@@ -49,7 +49,7 @@
 #include <asm/debugreg.h>
 #include <asm/kvm_para.h>
 #include <asm/irq_remapping.h>
-#include <asm/nospec-branch.h>
+#include <asm/spec-ctrl.h>
 
 #include <asm/virtext.h>
 #include "trace.h"
arch/x86/kvm/vmx.c +1 -1
@@ -51,7 +51,7 @@
 #include <asm/apic.h>
 #include <asm/irq_remapping.h>
 #include <asm/mmu_context.h>
-#include <asm/nospec-branch.h>
+#include <asm/spec-ctrl.h>
 #include <asm/mshyperv.h>
 
 #include "trace.h"
#16 lore raw

PATCH v9 14/16 SSB 14

In reply to Thomas Gleixner

Add two new prctls to control aspects of speculation related vulnerabilites and their mitigations to provide finer grained control over performance impacting mitigations.

PR_GET_SPECULATION_CTRL returns the state of the speculation misfeature which is selected with arg2 of prctl(2). The return value uses bit 0-2 with the following meaning:

Bit  Define           Description
0    PR_SPEC_PRCTL    Mitigation can be controlled per task by

PR_SET_SPECULATION_CTRL
1 PR_SPEC_ENABLE The speculation feature is enabled, mitigation is
disabled
2 PR_SPEC_DISABLE The speculation feature is disabled, mitigation is
enabled

If all bits are 0 the CPU is not affected by the speculation misfeature.

If PR_SPEC_PRCTL is set, then the per task control of the mitigation is available. If not set, prctl(PR_SET_SPECULATION_CTRL) for the speculation misfeature will fail.

PR_SET_SPECULATION_CTRL allows to control the speculation misfeature, which is selected by arg2 of prctl(2) per task. arg3 is used to hand in the control value, i.e. either PR_SPEC_ENABLE or PR_SPEC_DISABLE.

The common return values are:

EINVAL prctl is not implemented by the architecture or the unused prctl()
arguments are not 0
ENODEV arg2 is selecting a not supported speculation misfeature

PR_SET_SPECULATION_CTRL has these additional return values:

ERANGE arg3 is incorrect, i.e. it's not either PR_SPEC_ENABLE or PR_SPEC_DISABLE
ENXIO prctl control of the selected speculation misfeature is disabled

The first supported controlable speculation misfeature is PR_SPEC_STORE_BYPASS. Add the define so this can be shared between architectures.

TODO: Write a man prctl(2) patch.

Based on an initial patch from Tim Chen and mostly rewritten.

---
V2: Reword documentation and add unsused argument check
V3: Use EINVAL for unused argument check

---
 Documentation/userspace-api/index.rst     |    1 
 Documentation/userspace-api/spec_ctrl.rst |   86 ++++++++++++++++++++++++++++++
 include/linux/nospec.h                    |    5 +
 include/uapi/linux/prctl.h                |   11 +++
 kernel/sys.c                              |   22 +++++++
 5 files changed, 125 insertions(+)
Documentation/userspace-api/index.rst +1 -0
@@ -19,6 +19,7 @@ place where this information is gathered
    no_new_privs
    seccomp_filter
    unshare
+   spec_ctrl
 
 .. only::  subproject and html
 
Documentation/userspace-api/spec_ctrl.rst new file +86 -0

86 lines not on this page · raw message

include/linux/nospec.h +5 -0
@@ -55,4 +55,9 @@ static inline unsigned long array_index_
 									\
 	(typeof(_i)) (_i & _mask);					\
 })
+
+/* Speculation control prctl */
+int arch_prctl_set_spec_ctrl(unsigned long which, unsigned long ctrl);
+int arch_prctl_get_spec_ctrl(unsigned long which);
+
 #endif /* _LINUX_NOSPEC_H */
include/uapi/linux/prctl.h +11 -0
@@ -207,4 +207,15 @@ struct prctl_mm_map {
 # define PR_SVE_VL_LEN_MASK		0xffff
 # define PR_SVE_VL_INHERIT		(1 << 17) /* inherit across exec */
 
+/* Per task speculation control */
+#define PR_SET_SPECULATION_CTRL		52
+#define PR_GET_SPECULATION_CTRL		53
+/* Speculation control variants */
+# define PR_SPEC_STORE_BYPASS		0
+/* Return and control values for PR_SET/GET_SPECULATION_CTRL */
+# define PR_SPEC_NOT_AFFECTED		0
+# define PR_SPEC_PRCTL			(1UL << 0)
+# define PR_SPEC_ENABLE			(1UL << 1)
+# define PR_SPEC_DISABLE		(1UL << 2)
+
 #endif /* _LINUX_PRCTL_H */
kernel/sys.c +22 -0
@@ -61,6 +61,8 @@
 #include <linux/uidgid.h>
 #include <linux/cred.h>
 
+#include <linux/nospec.h>
+
 #include <linux/kmsg_dump.h>
 /* Move somewhere else to avoid recompiling? */
 #include <generated/utsrelease.h>
@@ -2242,6 +2244,16 @@ static int propagate_has_child_subreaper
 	return 1;
 }
 
+int __weak arch_prctl_set_spec_ctrl(unsigned long which, unsigned long ctrl)
+{
+	return -EINVAL;
+}
+
+int __weak arch_prctl_get_spec_ctrl(unsigned long which)
+{
+	return -EINVAL;
+}
+
 SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		unsigned long, arg4, unsigned long, arg5)
 {
@@ -2450,6 +2462,16 @@ SYSCALL_DEFINE5(prctl, int, option, unsi
 	case PR_SVE_GET_VL:
 		error = SVE_GET_VL();
 		break;
+	case PR_SET_SPECULATION_CTRL:
+		if (arg4 || arg5)
+			return -EINVAL;
+		error = arch_prctl_set_spec_ctrl(arg2, arg3);
+		break;
+	case PR_GET_SPECULATION_CTRL:
+		if (arg3 || arg4 || arg5)
+			return -EINVAL;
+		error = arch_prctl_get_spec_ctrl(arg2);
+		break;
 	default:
 		error = -EINVAL;
 		break;
#17 lore raw

PATCH v9 15/16 SSB 15

In reply to Thomas Gleixner

The Speculative Store Bypass vulnerability can be mitigated with the Reduced Data Speculation control (RDS). To allow finer grained control of this eventually expensive mitigation a per task mitigation control is required.

Add a new TIF_RDS flag and put it into the group of TIF flags which are evaluated for mismatch in switch_to(). If these bits differ in the previous and the next task, then the slow path function __switch_to_xtra() is invoked. Implement the TIF_RDS dependent mitigation control in the slow path.

If the prctl for controlling Speculative Store Bypass is disabled or no task uses the prctl then there is no overhead in the switch_to() fast path.

Update the KVM related speculation control functions to take TID_RDS into account as well.

Based on a patch from Tim Chen. Completely rewritten.

---
 arch/x86/include/asm/msr-index.h   |    3 ++-
 arch/x86/include/asm/spec-ctrl.h   |   17 +++++++++++++++++
 arch/x86/include/asm/thread_info.h |    4 +++-
 arch/x86/kernel/cpu/bugs.c         |   20 ++++++++++++++++----
 arch/x86/kernel/process.c          |   22 ++++++++++++++++++++++
 5 files changed, 60 insertions(+), 6 deletions(-)
arch/x86/include/asm/msr-index.h +2 -1
@@ -42,7 +42,8 @@
 #define MSR_IA32_SPEC_CTRL		0x00000048 /* Speculation Control */
 #define SPEC_CTRL_IBRS			(1 << 0)   /* Indirect Branch Restricted Speculation */
 #define SPEC_CTRL_STIBP			(1 << 1)   /* Single Thread Indirect Branch Predictors */
-#define SPEC_CTRL_RDS			(1 << 2)   /* Reduced Data Speculation */
+#define SPEC_CTRL_RDS_SHIFT		2	   /* Reduced Data Speculation bit */
+#define SPEC_CTRL_RDS			(1 << SPEC_CTRL_RDS_SHIFT)   /* Reduced Data Speculation */
 
 #define MSR_IA32_PRED_CMD		0x00000049 /* Prediction Command */
 #define PRED_CMD_IBPB			(1 << 0)   /* Indirect Branch Prediction Barrier */
arch/x86/include/asm/spec-ctrl.h +17 -0
@@ -2,6 +2,7 @@
 #ifndef _ASM_X86_SPECCTRL_H_
 #define _ASM_X86_SPECCTRL_H_
 
+#include <linux/thread_info.h>
 #include <asm/nospec-branch.h>
 
 /*
@@ -18,4 +19,20 @@ extern void x86_restore_host_spec_ctrl(u
 extern u64 x86_amd_ls_cfg_base;
 extern u64 x86_amd_ls_cfg_rds_mask;
 
+/* The Intel SPEC CTRL MSR base value cache */
+extern u64 x86_spec_ctrl_base;
+
+static inline u64 rds_tif_to_spec_ctrl(u64 tifn)
+{
+	BUILD_BUG_ON(TIF_RDS < SPEC_CTRL_RDS_SHIFT);
+	return (tifn & _TIF_RDS) >> (TIF_RDS - SPEC_CTRL_RDS_SHIFT);
+}
+
+static inline u64 rds_tif_to_amd_ls_cfg(u64 tifn)
+{
+	return tifn & _TIF_RDS ? x86_amd_ls_cfg_rds_mask : 0ULL;
+}
+
+extern void speculative_store_bypass_update(void);
+
 #endif
arch/x86/include/asm/thread_info.h +3 -1
@@ -79,6 +79,7 @@ struct thread_info {
 #define TIF_SIGPENDING		2	/* signal pending */
 #define TIF_NEED_RESCHED	3	/* rescheduling necessary */
 #define TIF_SINGLESTEP		4	/* reenable singlestep on user return*/
+#define TIF_RDS			5	/* Reduced data speculation */
 #define TIF_SYSCALL_EMU		6	/* syscall emulation active */
 #define TIF_SYSCALL_AUDIT	7	/* syscall auditing active */
 #define TIF_SECCOMP		8	/* secure computing */
@@ -105,6 +106,7 @@ struct thread_info {
 #define _TIF_SIGPENDING		(1 << TIF_SIGPENDING)
 #define _TIF_NEED_RESCHED	(1 << TIF_NEED_RESCHED)
 #define _TIF_SINGLESTEP		(1 << TIF_SINGLESTEP)
+#define _TIF_RDS		(1 << TIF_RDS)
 #define _TIF_SYSCALL_EMU	(1 << TIF_SYSCALL_EMU)
 #define _TIF_SYSCALL_AUDIT	(1 << TIF_SYSCALL_AUDIT)
 #define _TIF_SECCOMP		(1 << TIF_SECCOMP)
@@ -144,7 +146,7 @@ struct thread_info {
 
 /* flags to check in __switch_to() */
 #define _TIF_WORK_CTXSW							\
-	(_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP)
+	(_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP|_TIF_RDS)
 
 #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)
 #define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
arch/x86/kernel/cpu/bugs.c +16 -4
@@ -33,7 +33,7 @@ static void __init ssb_select_mitigation
  * Our boot-time value of SPEC_CTRL MSR. We read it once so that any
  * writes to SPEC_CTRL contain whatever reserved bits have been set.
  */
-static u64 __ro_after_init x86_spec_ctrl_base;
+u64 __ro_after_init x86_spec_ctrl_base;
 
 /*
  * The vendor and possibly platform specific bits which can be modified in
@@ -146,19 +146,31 @@ EXPORT_SYMBOL_GPL(x86_get_default_spec_c
 
 void x86_set_guest_spec_ctrl(u64 guest_spec_ctrl)
 {
+	u64 host = x86_spec_ctrl_base;
+
 	if (!boot_cpu_has(X86_FEATURE_IBRS))
 		return;
-	if (x86_spec_ctrl_base != guest_spec_ctrl)
+
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+		host |= rds_tif_to_spec_ctrl(current_thread_info()->flags);
+
+	if (host != guest_spec_ctrl)
 		wrmsrl(MSR_IA32_SPEC_CTRL, guest_spec_ctrl);
 }
 EXPORT_SYMBOL_GPL(x86_set_guest_spec_ctrl);
 
 void x86_restore_host_spec_ctrl(u64 guest_spec_ctrl)
 {
+	u64 host = x86_spec_ctrl_base;
+
 	if (!boot_cpu_has(X86_FEATURE_IBRS))
 		return;
-	if (x86_spec_ctrl_base != guest_spec_ctrl)
-		wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
+		host |= rds_tif_to_spec_ctrl(current_thread_info()->flags);
+
+	if (host != guest_spec_ctrl)
+		wrmsrl(MSR_IA32_SPEC_CTRL, host);
 }
 EXPORT_SYMBOL_GPL(x86_restore_host_spec_ctrl);
 
arch/x86/kernel/process.c +22 -0
@@ -38,6 +38,7 @@
 #include <asm/switch_to.h>
 #include <asm/desc.h>
 #include <asm/prctl.h>
+#include <asm/spec-ctrl.h>
 
 /*
  * per-CPU TSS segments. Threads are completely 'soft' on Linux,
@@ -278,6 +279,24 @@ static inline void switch_to_bitmap(stru
 	}
 }
 
+static __always_inline void __speculative_store_bypass_update(unsigned long tifn)
+{
+	u64 msr;
+
+	if (static_cpu_has(X86_FEATURE_AMD_RDS)) {
+		msr = x86_amd_ls_cfg_base | rds_tif_to_amd_ls_cfg(tifn);
+		wrmsrl(MSR_AMD64_LS_CFG, msr);
+	} else {
+		msr = x86_spec_ctrl_base | rds_tif_to_spec_ctrl(tifn);
+		wrmsrl(MSR_IA32_SPEC_CTRL, msr);
+	}
+}
+
+void speculative_store_bypass_update(void)
+{
+	__speculative_store_bypass_update(current_thread_info()->flags);
+}
+
 void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
 		      struct tss_struct *tss)
 {
@@ -309,6 +328,9 @@ void __switch_to_xtra(struct task_struct
 
 	if ((tifp ^ tifn) & _TIF_NOCPUID)
 		set_cpuid_faulting(!!(tifn & _TIF_NOCPUID));
+
+	if ((tifp ^ tifn) & _TIF_RDS)
+		__speculative_store_bypass_update(tifn);
 }
 
 /*
#18 lore raw

PATCH v9 16/16 SSB 16

In reply to Thomas Gleixner

Add prctl based control for Speculative Store Bypass mitigation and make it the default mitigation for Intel and AMD.

Andi Kleen provided the following rationale (slightly redacted):

There are multiple levels of impact of Speculative Store Bypass:

1) JITed sandbox.
It cannot invoke system calls, but can do PRIME+PROBE and may have call interfaces to other code

2) Native code process.
No protection inside the process at this level.

3) Kernel.

4) Between processes.

The prctl tries to protect against case (1) doing attacks.

If the untrusted code can do random system calls then control is already lost in a much worse way. So there needs to be system call protection in some way (using a JIT not allowing them or seccomp). Or rather if the process can subvert its environment somehow to do the prctl it can already execute arbitrary code, which is much worse than SSB.

To put it differently, the point of the prctl is to not allow JITed code to read data it shouldn't read from its JITed sandbox. If it already has escaped its sandbox then it can already read everything it wants in its address space, and do much worse.

On the other hand if there is the ability to switch it freely gives more flexibility to do the protection which is needed for JITed code without impacting the overall system performance.

Based on an initial patch from Tim Chen. Completely rewritten.

---
V9: Fixed the sbb typo and made AMD default to prctl as well

---
 Documentation/admin-guide/kernel-parameters.txt |    3 
 arch/x86/include/asm/nospec-branch.h            |    1 
 arch/x86/kernel/cpu/bugs.c                      |   83 +++++++++++++++++++++---
 3 files changed, 77 insertions(+), 10 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +3 -0
@@ -4054,6 +4054,9 @@
 			auto   - Kernel detects whether the CPU model contains a
 				 implementation of Speculative Store Bypass and
 				 picks the most appropriate mitigation
+			prctl  - Control Speculative Store Bypass for a thread
+				 via prctl. By default it is enabled. The state
+				 is inherited on fork.
 
 			Not specifying this option is equivalent to
 			spec_store_bypass_disable=auto.
arch/x86/include/asm/nospec-branch.h +1 -0
@@ -232,6 +232,7 @@ extern u64 x86_get_default_spec_ctrl(voi
 enum ssb_mitigation {
 	SPEC_STORE_BYPASS_NONE,
 	SPEC_STORE_BYPASS_DISABLE,
+	SPEC_STORE_BYPASS_PRCTL,
 };
 
 extern char __indirect_thunk_start[];
arch/x86/kernel/cpu/bugs.c +73 -10

126 lines not on this page · raw message

#19 lore raw

Re: [patch V9 00/16] SSB 0 - Bundle

In reply to Thomas Gleixner
Quoted 19 lines · speck for Thomas Gleixner

Changes vs. V8:

Addressed review feed back and added Linus variant of alternative_write_msr().

- Use EINVAL in the prctl

- Fix the sbb typos

- Make AMD default to prctl as well (Borislav)

- Add the not affected CPU types (Tim)

I've updated the git repository as well

ssh://[email protected]/linux/speck/linux master (forced update)

There are also two stable backport branches now:

Quoted 7 lines

ssh://[email protected]/linux/speck/linux linux-4.16.y

ssh://[email protected]/linux/speck/linux linux-4.14.y

Delta patch against V8 below. Git bundle against 4.17-rc3 follows in separate mail.

Attached.

#20 lore raw

Re: [patch V9 00/16] SSB

In reply to Thomas Gleixner

On Tue, 1 May 2018, speck for Thomas Gleixner wrote:

Sorry, I sent out the mbox w/o mangling the subject lines, noticed and stopped the queue, but some made it through.

Sorry for the spam.

Thanks,

tglx

#21 lore raw

[MODERATED] Re: [patch V9 00/16] SSB

In reply to Thomas Gleixner
Quoted 24 lines · speck for Thomas Gleixner

Changes vs. V8:

Addressed review feed back and added Linus variant of alternative_write_msr().

- Use EINVAL in the prctl

- Fix the sbb typos

- Make AMD default to prctl as well (Borislav)

- Add the not affected CPU types (Tim)

I've updated the git repository as well

ssh://[email protected]/linux/speck/linux master (forced update)

There are also two stable backport branches now:

ssh://[email protected]/linux/speck/linux linux-4.16.y

ssh://[email protected]/linux/speck/linux linux-4.14.y

Quoted 7 lines

Delta patch against V8 below. Git bundle against 4.17-rc3 follows in separate mail.

Thanks,

tglx

This version also has a bug that's in the v8 patch. For whole platform disabling of speculation, only CPU 0 has RDS enabled.

root@otc-cfl-s-02:~# cat /sys/devices/system/cpu/vulnerabilities/spec_store_bypass 
Mitigation: Speculative Store Bypass disabled

msr 0x48 cpu 0 val: 4
msr 0x48 cpu 1 val: 0
msr 0x48 cpu 2 val: 0
msr 0x48 cpu 3 val: 0
msr 0x48 cpu 4 val: 0
msr 0x48 cpu 5 val: 0
msr 0x48 cpu 6 val: 0
msr 0x48 cpu 7 val: 0
msr 0x48 cpu 8 val: 0
msr 0x48 cpu 9 val: 0
msr 0x48 cpu 10 val: 0
msr 0x48 cpu 11 val: 0

I used the fix below to take care of Intel CPU. The fix also works for CPU taken offline and back online. AMD cpu has the proper setting of their MSR in boot up via identify_secondary_cpu's call to x86_setup_ap_spec_ctrl.

void x86_setup_ap_spec_ctrl(void)

{
        if (boot_cpu_has(X86_FEATURE_IBRS))
                x86_set_spec_ctrl(x86_spec_ctrl_base & ~x86_spec_ctrl_mask);

        if (ssb_mode == SPEC_STORE_BYPASS_DISABLE)
                x86_amd_rds_enable();
}

But AMD cpu may have issue when CPU is brought offline and back online as x86_setup_ap_spec_ctrl may not get called then.

Tim

---

arch/x86/kernel/cpu/intel.c +3 -0
index ef3f9c0..c799e48 100644
@@ -770,6 +770,9 @@ static void init_intel(struct cpuinfo_x86 *c)
 	init_intel_energy_perf(c);
 
 	init_intel_misc_features(c);
+
+	if (cpu_has(c, X86_FEATURE_SPEC_STORE_BYPASS_DISABLE))
+		x86_set_spec_ctrl(SPEC_CTRL_RDS);
 }
 
 #ifdef CONFIG_X86_32
#22 lore raw

Re: [patch V9 00/16] SSB

Quoted 3 lines · speck for Tim Chen

On 05/01/2018 08:24 AM, speck for Thomas Gleixner wrote:

I used the fix below to take care of Intel CPU. The fix also works for

Did you actually read what I wrote?

Obviosly not,

#23 lore raw

[MODERATED] Re: [patch V9 00/16] SSB

In reply to Tim Chen
Quoted 3 lines · speck for Tim Chen

This version also has a bug that's in the v8 patch. For whole platform disabling of speculation, only CPU 0 has RDS enabled.

The whole CPUID thing really messy. It's not just secondary CPU's. It's "we do some things early, some things late, and some things we fix up in random phases". The whole "boot vs secondary" CPU is just an added wrinkle on top of that.

This is not strictly related to the current issue. There's an unexplained "oops, some of the meltdown mitigation bits didn't get set" bug report against current -git too.

I wish we had actual rules (simple enough to check) about the whole cpuid thing.

Linus

#24 lore raw

Re: [patch V9 00/16] SSB

Quoted 9 lines · speck for Linus Torvalds
Quoted 3 lines · speck for Tim Chen

This version also has a bug that's in the v8 patch. For whole platform disabling of speculation, only CPU 0 has RDS enabled.

The whole CPUID thing really messy. It's not just secondary CPU's. It's "we do some things early, some things late, and some things we fix up in random phases". The whole "boot vs secondary" CPU is just an added wrinkle on top of that.

Don;t tell me. I hate it.

Quoted 3 lines

This is not strictly related to the current issue. There's an unexplained "oops, some of the meltdown mitigation bits didn't get set" bug report against current -git too.

Yes. I'm working with the reporter

Quoted 2 lines

I wish we had actual rules (simple enough to check) about the whole cpuid thing.

Borislav and I started looking into cleaning up that unholy mess after the last disaster, but then got distracted with this one again.

Thanks,

tglx

#25 lore raw

[MODERATED] Re: [patch V9 00/16] SSB

Quoted 2 lines · speck for Thomas Gleixner

Borislav and I started looking into cleaning up that unholy mess after the last disaster, but then got distracted with this one again.

If only the disaster mode will end already so that we can finally even *try* to cleanup that fucking mess.

Signature · 5 lines
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 
#26 lore raw

Re: [patch V9 00/16] SSB

In reply to Tim Chen
Quoted 14 lines · speck for Tim Chen

I used the fix below to take care of Intel CPU. The fix also works for CPU taken offline and back online. AMD cpu has the proper setting of their MSR in boot up via identify_secondary_cpu's call to x86_setup_ap_spec_ctrl.

void x86_setup_ap_spec_ctrl(void)

{
        if (boot_cpu_has(X86_FEATURE_IBRS))
                x86_set_spec_ctrl(x86_spec_ctrl_base & ~x86_spec_ctrl_mask);

        if (ssb_mode == SPEC_STORE_BYPASS_DISABLE)
                x86_amd_rds_enable();
}
Quoted 2 lines

But AMD cpu may have issue when CPU is brought offline and back online as x86_setup_ap_spec_ctrl may not get called then.

AMD has no issue at all neither has Intel. On both x86_setup_ap_spec_ctrl() is called everytime a CPU comes online, whether it's boot or after a offline/online sequence.

start_secondary()
smp_callin()

    identify_secondary_cpu()
      x86_setup_ap_spec_ctrl()

Care to read the code before making uneducated claims?

Thanks,

tglx

#27 lore raw

Re: [patch V9 00/16] SSB

In reply to Tim Chen
Quoted 2 lines · speck for Tim Chen

This version also has a bug that's in the v8 patch. For whole platform disabling of speculation, only CPU 0 has RDS enabled.

Please provide the output of /proc/cpuinfo

#28 lore raw

[MODERATED] Re: [patch V9 00/16] SSB

Quoted 5 lines · speck for Thomas Gleixner
Quoted 2 lines · speck for Tim Chen

This version also has a bug that's in the v8 patch. For whole platform disabling of speculation, only CPU 0 has RDS enabled.

Please provide the output of /proc/cpuinfo

+ full dmesg please.

Signature · 5 lines
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 
#29 lore raw

[MODERATED] Re: [patch V9 00/16] SSB

In reply to Thomas Gleixner
Quoted 19 lines · speck for Thomas Gleixner

Changes vs. V8:

Addressed review feed back and added Linus variant of alternative_write_msr().

- Use EINVAL in the prctl

- Fix the sbb typos

- Make AMD default to prctl as well (Borislav)

- Add the not affected CPU types (Tim)

I've updated the git repository as well

ssh://[email protected]/linux/speck/linux master (forced update)

There are also two stable backport branches now:

Quoted 7 lines

ssh://[email protected]/linux/speck/linux linux-4.16.y

ssh://[email protected]/linux/speck/linux linux-4.14.y

Delta patch against V8 below. Git bundle against 4.17-rc3 follows in separate mail.

On my Skylake box this does what is expected:

4
RHEL-64 <root@instance-20180426-1449:/home/opc> dmesg | grep -i spec

[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-4.17.0-rc3-v9+ root=UUID=533fa2dd-5b2f-4964-aa99-5ab349614dcd ro crashkernel=auto LANG=en_US.UTF-8 transparent_hugepage=never console=tty0 console=ttyS0,9600 rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0 netroot=iscsi:169.254.0.2:::1:iqn.2015-02.oracle.boot:uefi iscsi_param=node.session.timeo.replacement_timeout=6000 net.ifnames=1 nvme_core.shutdown_timeout=10 ipmi_si.tryacpi=0 ipmi_si.trydmi=0 ipmi_si.trydefaults=0 network-config=e2NvbmZpZzogZGlzYWJsZWR9Cg== spec_store_bypass_disable=on
[    0.000000] please specify mtrr_gran_size/mtrr_chunk_size
[    0.000000] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-4.17.0-rc3-v9+ root=UUID=533fa2dd-5b2f-4964-aa99-5ab349614dcd ro crashkernel=auto LANG=en_US.UTF-8 transparent_hugepage=never console=tty0 console=ttyS0,9600 rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0 netroot=iscsi:169.254.0.2:::1:iqn.2015-02.oracle.boot:uefi iscsi_param=node.session.timeo.replacement_timeout=6000 net.ifnames=1 nvme_core.shutdown_timeout=10 ipmi_si.tryacpi=0 ipmi_si.trydmi=0 ipmi_si.trydefaults=0 network-config=e2NvbmZpZzogZGlzYWJsZWR9Cg== spec_store_bypass_disable=on
[    0.145004] Spectre V2 : Mitigation: Full generic retpoline
[    0.146001] Spectre V2 : Spectre v2 mitigation: Filling RSB on context switch
[    0.147000] Spectre V2 : Spectre v2 mitigation: Enabling Indirect Branch Prediction Barrier
[    0.148000] Spectre V2 : Enabling Restricted Speculation for firmware calls
[    0.149004] Speculative Store Bypass: Mitigation: Speculative Store Bypass disabled

RHEL-64 <root@instance-20180426-1449:/home/opc> rdmsr 0x48 --all | uniq 4

And I don't see the weird issue that Tim reported?

With the normal:

RHEL-64 <root@instance-20180426-1449:/home/opc> dmesg | grep -i spec

[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-4.17.0-rc3-v9+ root=UUID=533fa2dd-5b2f-4964-aa99-5ab349614dcd ro crashkernel=auto LANG=en_US.UTF-8 transparent_hugepage=never console=tty0 console=ttyS0,9600 rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0 netroot=iscsi:169.254.0.2:::1:iqn.2015-02.oracle.boot:uefi iscsi_param=node.session.timeo.replacement_timeout=6000 net.ifnames=1 nvme_core.shutdown_timeout=10 ipmi_si.tryacpi=0 ipmi_si.trydmi=0 ipmi_si.trydefaults=0 network-config=e2NvbmZpZzogZGlzYWJsZWR9Cg== spec_store_bypass_disable=auto
[    0.000000] please specify mtrr_gran_size/mtrr_chunk_size
[    0.000000] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-4.17.0-rc3-v9+ root=UUID=533fa2dd-5b2f-4964-aa99-5ab349614dcd ro crashkernel=auto LANG=en_US.UTF-8 transparent_hugepage=never console=tty0 console=ttyS0,9600 rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0 netroot=iscsi:169.254.0.2:::1:iqn.2015-02.oracle.boot:uefi iscsi_param=node.session.timeo.replacement_timeout=6000 net.ifnames=1 nvme_core.shutdown_timeout=10 ipmi_si.tryacpi=0 ipmi_si.trydmi=0 ipmi_si.trydefaults=0 network-config=e2NvbmZpZzogZGlzYWJsZWR9Cg== spec_store_bypass_disable=auto
[    0.146004] Spectre V2 : Mitigation: Full generic retpoline
[    0.147000] Spectre V2 : Spectre v2 mitigation: Filling RSB on context switch
[    0.148000] Spectre V2 : Spectre v2 mitigation: Enabling Indirect Branch Prediction Barrier
[    0.149000] Spectre V2 : Enabling Restricted Speculation for firmware calls
[    0.150003] Speculative Store Bypass: Mitigation: Speculative Store Bypass disabled via prctl

RHEL-64 <root@instance-20180426-1449:/home/opc> rdmsr 0x48 --all | uniq 0
RHEL-64 <root@instance-20180426-1449:/home/opc> ./test-prctl Initial state
PR_GET_SPECULATION_CTRL PR_SPEC_STORE_BYPASS r=3
r=PR_SPEC_PRCTL PR_SPEC_ENABLE
PR_SET_SPECULATION_CTRL PR_SPEC_DISABLE r=0

        r=  
   PR_GET_SPECULATION_CTRL PR_SPEC_STORE_BYPASS r=5
        r=PR_SPEC_PRCTL  PR_SPEC_DISABLE
PR_SET_SPECULATION_CTRL PR_SPEC_DISABLE r=0
        r=  
   PR_GET_SPECULATION_CTRL PR_SPEC_STORE_BYPASS r=5
        r=PR_SPEC_PRCTL  PR_SPEC_DISABLE
PR_SET_SPECULATION_CTRL PR_SPEC_ENABLE r=0
        r=  
   PR_GET_SPECULATION_CTRL PR_SPEC_STORE_BYPASS r=3
        r=PR_SPEC_PRCTL PR_SPEC_ENABLE 
PR_SET_SPECULATION_CTRL PR_SPEC_ENABLE r=0
        r=  

PR_GET_SPECULATION_CTRL PR_SPEC_STORE_BYPASS r=3
r=PR_SPEC_PRCTL PR_SPEC_ENABLE

Real test-case
PR_SET_SPECULATION_CTRL PR_SPEC_DISABLE r=0

        r=  

PR_GET_SPECULATION_CTRL PR_SPEC_STORE_BYPASS r=5
r=PR_SPEC_PRCTL PR_SPEC_DISABLE

I will fix up the manpage patch and send it out shortly.

Quoted 171 lines

Thanks,

tglx

8<-------------------

Documentation/admin-guide/kernel-parameters.txt +3 -3
index de1ca7510697..29984fd3dd18 100644
@@ -4052,10 +4052,10 @@
 			on     - Unconditionally disable Speculative Store Bypass
 			off    - Unconditionally enable Speculative Store Bypass
 			auto   - Kernel detects whether the CPU model contains a
-			         vulnerable implementation of Speculative Store
-			         Bypass and picks the most appropriate mitigation
+				 implementation of Speculative Store Bypass and
+				 picks the most appropriate mitigation
 			prctl  - Control Speculative Store Bypass for a thread
-			         via prctl. By default it is enabled. The state
+				 via prctl. By default it is enabled. The state
 				 is inherited on fork.
 
 			Not specifying this option is equivalent to
Documentation/userspace-api/spec_ctrl.rst +2 -3
index 867b8b435baa..8ff39a26a992 100644
@@ -56,11 +56,10 @@ Common error codes
 ======= =================================================================
 Value   Meaning
 ======= =================================================================
-EINVAL  The prctl is not implemented by the architecture
+EINVAL  The prctl is not implemented by the architecture or unused
+        prctl(2) arguments are not 0
 
 ENODEV  arg2 is selecting a not supported speculation misfeature
-
-EUCLEAN The unused prctl() arguments are not 0
 ======= =================================================================
 
 PR_SET_SPECULATION_CTRL error codes
arch/x86/include/asm/nospec-branch.h +11 -11
index 7a1be0b2c264..023e2edc0f3c 100644
@@ -259,22 +259,22 @@ static inline void vmexit_fill_RSB(void)
 #endif
 }
 
-#define alternative_msr_write(_msr, _val, _feature)		\
-	asm volatile(ALTERNATIVE("",				\
-				 "movl %[msr], %%ecx\n\t"	\
-				 "movl %[val], %%eax\n\t"	\
-				 "movl $0, %%edx\n\t"		\
-				 "wrmsr",			\
-				 _feature)			\
-		     : : [msr] "i" (_msr), [val] "m" (_val)	\
-		     : "eax", "ecx", "edx", "memory")
+static __always_inline
+void alternative_msr_write(unsigned int msr, u64 val, unsigned int feature)
+{
+	asm volatile(ALTERNATIVE("", "wrmsr", %c[feature])
+		: : "c" (msr),
+		    "a" (val),
+		    "d" (val >> 32),
+		    [feature] "i" (feature)
+		: "memory");
+}
 
 static inline void indirect_branch_prediction_barrier(void)
 {
 	u64 val = PRED_CMD_IBPB;
 
-	alternative_msr_write(MSR_IA32_PRED_CMD, val,
-			      X86_FEATURE_USE_IBPB);
+	alternative_msr_write(MSR_IA32_PRED_CMD, val, X86_FEATURE_USE_IBPB);
 }
 
 /*
arch/x86/kernel/cpu/bugs.c +4 -9
index d52ccd0ce507..c28856e475c8 100644
@@ -476,11 +476,6 @@ static enum ssb_mitigation_cmd __init __ssb_select_mitigation(void)
 
 	switch (cmd) {
 	case SPEC_STORE_BYPASS_CMD_AUTO:
-		/*
-		 * AMD platforms by default don't need SSB mitigation.
-		 */
-		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
-			break;
 		/* Choose prctl as the default mode */
 		mode = SPEC_STORE_BYPASS_PRCTL;
 		break;
@@ -531,7 +526,7 @@ static void ssb_select_mitigation()
 
 #undef pr_fmt
 
-static int sbb_prctl_set(unsigned long ctrl)
+static int ssb_prctl_set(unsigned long ctrl)
 {
 	bool rds = !!test_tsk_thread_flag(current, TIF_RDS);
 
@@ -549,7 +544,7 @@ static int sbb_prctl_set(unsigned long ctrl)
 	return 0;
 }
 
-static int sbb_prctl_get(void)
+static int ssb_prctl_get(void)
 {
 	switch (ssb_mode) {
 	case SPEC_STORE_BYPASS_DISABLE:
@@ -572,7 +567,7 @@ int arch_prctl_set_spec_ctrl(unsigned long which, unsigned long ctrl)
 
 	switch (which) {
 	case PR_SPEC_STORE_BYPASS:
-		return sbb_prctl_set(ctrl);
+		return ssb_prctl_set(ctrl);
 	default:
 		return -ENODEV;
 	}
@@ -582,7 +577,7 @@ int arch_prctl_get_spec_ctrl(unsigned long which)
 {
 	switch (which) {
 	case PR_SPEC_STORE_BYPASS:
-		return sbb_prctl_get();
+		return ssb_prctl_get();
 	default:
 		return -ENODEV;
 	}
arch/x86/kernel/cpu/common.c +3 -0
index c3b53bc30a29..6101e3dba2f6 100644
@@ -931,6 +931,9 @@ static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = {
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_CEDARVIEW },
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_SILVERMONT1 },
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_AIRMONT },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_SILVERMONT2 },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_MERRIFIELD },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_CORE_YONAH },
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_XEON_PHI_KNL },
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_XEON_PHI_KNM },
 	{ X86_VENDOR_CENTAUR,	5 },
kernel/sys.c +4 -6
index 8bb8051bd9a1..d7afe29319f1 100644
@@ -2464,15 +2464,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		break;
 	case PR_SET_SPECULATION_CTRL:
 		if (arg4 || arg5)
-			error = -EUCLEAN;
-		else
-			error = arch_prctl_set_spec_ctrl(arg2, arg3);
+			return -EINVAL;
+		error = arch_prctl_set_spec_ctrl(arg2, arg3);
 		break;
 	case PR_GET_SPECULATION_CTRL:
 		if (arg3 || arg4 || arg5)
-			error = -EUCLEAN;
-		else
-			error = arch_prctl_get_spec_ctrl(arg2);
+			return -EINVAL;
+		error = arch_prctl_get_spec_ctrl(arg2);
 		break;
 	default:
 		error = -EINVAL;
#30 lore raw

Re: [patch V9 00/16] SSB

Quoted 4 lines · speck for Konrad Rzeszutek Wilk

On my Skylake box this does what is expected:

And I don't see the weird issue that Tim reported?

That depends on your ucode loading mechanism.

Quoted 1 line

RHEL-64 <root@instance-20180426-1449:/home/opc> ./test-prctl

Could you please send that test thing around? It's trivial enough but I'm a lousy user space programmer and lazy :)

Thanks,

tglx

#31 lore raw

[MODERATED] Re: [patch V9 00/16] SSB

Quoted 7 lines · speck for Thomas Gleixner
Quoted 4 lines · speck for Konrad Rzeszutek Wilk

On my Skylake box this does what is expected:

And I don't see the weird issue that Tim reported?

That depends on your ucode loading mechanism.

Oooh <duh>. Right, that would explain it:
RHEL-64 <opc@instance-20180426-1449:~> dmesg | grep micro

[    0.000000] microcode: microcode updated early to revision 0x200004a, date = 2018-03-28
[   28.499014] microcode: sig=0x50654, pf=0x80, revision=0x200004a
[   28.575504] microcode: Microcode Update Driver: v2.2.
Quoted 5 lines
Quoted 2 lines

RHEL-64 <root@instance-20180426-1449:/home/opc> ./test-prctl

Could you please send that test thing around? It's trivial enough but I'm a lousy user space programmer and lazy :)

It is very umm, crappy but sure - see inline and attached. And it does not do any ASSERTS or such to verify things. And I've no clue why the tabs all changed to spaces.

#include <stdio.h>
       #include <sys/prctl.h>

#define SPEC_CTRL_RDS_SHIFT 2 /* Reduced Data Speculation bit */
/* Per task speculation control */
#define PR_SET_SPECULATION_CTRL 52
#define PR_GET_SPECULATION_CTRL 53
/* Speculation control variants */

# define PR_SPEC_STORE_BYPASS          0
/* Return and control values for PR_SET/GET_SPECULATION_CTRL */
# define PR_SPEC_NOT_AFFECTED          0
# define PR_SPEC_PRCTL                 (1UL << 0)
# define PR_SPEC_ENABLE                        (1UL << 1)
# define PR_SPEC_DISABLE               (1UL << 2)

void print_bitfields(int r)
{
if ( r >= 0 )
fprintf(stderr, " r=%s %s %s\n", r & PR_SPEC_PRCTL ? "PR_SPEC_PRCTL" : "",

		r & PR_SPEC_ENABLE ? "PR_SPEC_ENABLE" : "",
		r & PR_SPEC_DISABLE ? "PR_SPEC_DISABLE" : "");

}
void get_v(void)
{
int r;
r = prctl(PR_GET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, NULL, NULL,NULL);
fprintf(stderr, " PR_GET_SPECULATION_CTRL PR_SPEC_STORE_BYPASS r=%d\n", r);
print_bitfields(r);

}

int main(void)
{
int r;

fprintf(stderr,"Initial state\n");
get_v();

r = prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_DISABLE, NULL, NULL); fprintf(stderr, "PR_SET_SPECULATION_CTRL PR_SPEC_DISABLE r=%d\n", r); print_bitfields(r);
get_v();

/* Two DISALBE in a row*/
r = prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_DISABLE, NULL, NULL); fprintf(stderr, "PR_SET_SPECULATION_CTRL PR_SPEC_DISABLE r=%d\n", r); print_bitfields(r);
get_v();

/* Two enable in a row. */
r = prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_ENABLE, NULL, NULL); fprintf(stderr, "PR_SET_SPECULATION_CTRL PR_SPEC_ENABLE r=%d\n", r); print_bitfields(r);
get_v();

r = prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_ENABLE, NULL, NULL); fprintf(stderr, "PR_SET_SPECULATION_CTRL PR_SPEC_ENABLE r=%d\n", r); print_bitfields(r);
get_v();
fprintf(stderr,"\nReal test-case\n");
/* The real normal test-case */
r = prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_DISABLE, NULL, NULL); fprintf(stderr, "PR_SET_SPECULATION_CTRL PR_SPEC_DISABLE r=%d\n", r); print_bitfields(r);
get_v();

do { } while (1);

	return 0;
}
Quoted 4 lines

Thanks,

tglx

#include <stdio.h>
       #include <sys/prctl.h>

#define SPEC_CTRL_RDS_SHIFT 2 /* Reduced Data Speculation bit */
/* Per task speculation control */
#define PR_SET_SPECULATION_CTRL 52
#define PR_GET_SPECULATION_CTRL 53
/* Speculation control variants */

# define PR_SPEC_STORE_BYPASS          0
/* Return and control values for PR_SET/GET_SPECULATION_CTRL */
# define PR_SPEC_NOT_AFFECTED          0
# define PR_SPEC_PRCTL                 (1UL << 0)
# define PR_SPEC_ENABLE                        (1UL << 1)
# define PR_SPEC_DISABLE               (1UL << 2)

void print_bitfields(int r)
{
if ( r >= 0 )
fprintf(stderr, " r=%s %s %s\n", r & PR_SPEC_PRCTL ? "PR_SPEC_PRCTL" : "",

		r & PR_SPEC_ENABLE ? "PR_SPEC_ENABLE" : "",
		r & PR_SPEC_DISABLE ? "PR_SPEC_DISABLE" : "");

}
void get_v(void)
{
int r;
r = prctl(PR_GET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, NULL, NULL,NULL);
fprintf(stderr, " PR_GET_SPECULATION_CTRL PR_SPEC_STORE_BYPASS r=%d\n", r);
print_bitfields(r);

}

int main(void)
{
int r;

fprintf(stderr,"Initial state\n");
get_v();

r = prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_DISABLE, NULL, NULL); fprintf(stderr, "PR_SET_SPECULATION_CTRL PR_SPEC_DISABLE r=%d\n", r); print_bitfields(r);
get_v();

/* Two DISALBE in a row*/
r = prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_DISABLE, NULL, NULL); fprintf(stderr, "PR_SET_SPECULATION_CTRL PR_SPEC_DISABLE r=%d\n", r); print_bitfields(r);
get_v();

/* Two enable in a row. */
r = prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_ENABLE, NULL, NULL); fprintf(stderr, "PR_SET_SPECULATION_CTRL PR_SPEC_ENABLE r=%d\n", r); print_bitfields(r);
get_v();

r = prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_ENABLE, NULL, NULL); fprintf(stderr, "PR_SET_SPECULATION_CTRL PR_SPEC_ENABLE r=%d\n", r); print_bitfields(r);
get_v();
fprintf(stderr,"\nReal test-case\n");
/* The real normal test-case */
r = prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_DISABLE, NULL, NULL); fprintf(stderr, "PR_SET_SPECULATION_CTRL PR_SPEC_DISABLE r=%d\n", r); print_bitfields(r);
get_v();

do { } while (1);

	return 0;
}
#32 lore raw

[MODERATED] Re: [patch V9 00/16] SSB

In reply to Konrad Rzeszutek Wilk
Quoted 40 lines · speck for Konrad Rzeszutek Wilk
Quoted 19 lines · speck for Thomas Gleixner

Changes vs. V8:

Addressed review feed back and added Linus variant of alternative_write_msr().

- Use EINVAL in the prctl

- Fix the sbb typos

- Make AMD default to prctl as well (Borislav)

- Add the not affected CPU types (Tim)

I've updated the git repository as well

ssh://[email protected]/linux/speck/linux master (forced update)

There are also two stable backport branches now:

Quoted 7 lines

ssh://[email protected]/linux/speck/linux linux-4.16.y

ssh://[email protected]/linux/speck/linux linux-4.14.y

Delta patch against V8 below. Git bundle against 4.17-rc3 follows in separate mail.

On my Skylake box this does what is expected:

4
RHEL-64 <root@instance-20180426-1449:/home/opc> dmesg | grep -i spec

[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-4.17.0-rc3-v9+ root=UUID=533fa2dd-5b2f-4964-aa99-5ab349614dcd ro crashkernel=auto LANG=en_US.UTF-8 transparent_hugepage=never console=tty0 console=ttyS0,9600 rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0 netroot=iscsi:169.254.0.2:::1:iqn.2015-02.oracle.boot:uefi iscsi_param=node.session.timeo.replacement_timeout=6000 net.ifnames=1 nvme_core.shutdown_timeout=10 ipmi_si.tryacpi=0 ipmi_si.trydmi=0 ipmi_si.trydefaults=0 network-config=e2NvbmZpZzogZGlzYWJsZWR9Cg== spec_store_bypass_disable=on
[    0.000000] please specify mtrr_gran_size/mtrr_chunk_size
[    0.000000] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-4.17.0-rc3-v9+ root=UUID=533fa2dd-5b2f-4964-aa99-5ab349614dcd ro crashkernel=auto LANG=en_US.UTF-8 transparent_hugepage=never console=tty0 console=ttyS0,9600 rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0 netroot=iscsi:169.254.0.2:::1:iqn.2015-02.oracle.boot:uefi iscsi_param=node.session.timeo.replacement_timeout=6000 net.ifnames=1 nvme_core.shutdown_timeout=10 ipmi_si.tryacpi=0 ipmi_si.trydmi=0 ipmi_si.trydefaults=0 network-config=e2NvbmZpZzogZGlzYWJsZWR9Cg== spec_store_bypass_disable=on
[    0.145004] Spectre V2 : Mitigation: Full generic retpoline
[    0.146001] Spectre V2 : Spectre v2 mitigation: Filling RSB on context switch
[    0.147000] Spectre V2 : Spectre v2 mitigation: Enabling Indirect Branch Prediction Barrier
[    0.148000] Spectre V2 : Enabling Restricted Speculation for firmware calls
[    0.149004] Speculative Store Bypass: Mitigation: Speculative Store Bypass disabled
Quoted 6 lines

RHEL-64 <root@instance-20180426-1449:/home/opc> rdmsr 0x48 --all | uniq 4

And I don't see the weird issue that Tim reported?

Did a trace_printk and IBRS feature was not set when my system check it in

void x86_setup_ap_spec_ctrl(void)

{
        if (boot_cpu_has(X86_FEATURE_IBRS))
                x86_set_spec_ctrl(x86_spec_ctrl_base & ~x86_spec_ctrl_mask);

I checked cpuid and the IBRS feature bit was there.
I will do some more digging to find out why X86_FEATURE_IBRS is not set by kernel on my system. Konrad, you said someone else already reported the IBRS issue and was that root caused?

Tim

#33 lore raw

Re: [patch V9 00/16] SSB

Quoted 8 lines · speck for Tim Chen

Did a trace_printk and IBRS feature was not set when my system check it in

void x86_setup_ap_spec_ctrl(void)

{
        if (boot_cpu_has(X86_FEATURE_IBRS))
                x86_set_spec_ctrl(x86_spec_ctrl_base & ~x86_spec_ctrl_mask);
Quoted 4 lines

I checked cpuid and the IBRS feature bit was there.
I will do some more digging to find out why X86_FEATURE_IBRS is not set by kernel on my system. Konrad, you said someone else already reported the IBRS issue and was that root caused?

Does the patch below fix it? Tripped over the same issue with AMD.

Thanks,

tglx

8<--------------

arch/x86/kernel/cpu/common.c +0 -21
@@ -750,26 +750,6 @@ static void apply_forced_caps(struct cpu
 	}
 }
 
-static void init_speculation_control(struct cpuinfo_x86 *c)
-{
-	/*
-	 * The Intel SPEC_CTRL CPUID bit implies IBRS and IBPB support,
-	 * and they also have a different bit for STIBP support. Also,
-	 * a hypervisor might have set the individual AMD bits even on
-	 * Intel CPUs, for finer-grained selection of what's available.
-	 *
-	 * We use the AMD bits in 0x8000_0008 EBX as the generic hardware
-	 * features, which are visible in /proc/cpuinfo and used by the
-	 * kernel. So set those accordingly from the Intel bits.
-	 */
-	if (cpu_has(c, X86_FEATURE_SPEC_CTRL)) {
-		set_cpu_cap(c, X86_FEATURE_IBRS);
-		set_cpu_cap(c, X86_FEATURE_IBPB);
-	}
-	if (cpu_has(c, X86_FEATURE_INTEL_STIBP))
-		set_cpu_cap(c, X86_FEATURE_STIBP);
-}
-
 void get_cpu_cap(struct cpuinfo_x86 *c)
 {
 	u32 eax, ebx, ecx, edx;
@@ -852,7 +832,6 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 		c->x86_capability[CPUID_8000_000A_EDX] = cpuid_edx(0x8000000a);
 
 	init_scattered_cpuid_features(c);
-	init_speculation_control(c);
 
 	/*
 	 * Clear/Set all flags overridden by options, after probe.
arch/x86/kernel/cpu/intel.c +21 -0
@@ -967,6 +967,26 @@ static void intel_detect_tlb(struct cpui
 	}
 }
 
+static void bsp_init(struct cpuinfo_x86 *c)
+{
+	/*
+	 * The Intel SPEC_CTRL CPUID bit implies IBRS and IBPB support,
+	 * and they also have a different bit for STIBP support. Also,
+	 * a hypervisor might have set the individual AMD bits even on
+	 * Intel CPUs, for finer-grained selection of what's available.
+	 *
+	 * We use the AMD bits in 0x8000_0008 EBX as the generic hardware
+	 * features, which are visible in /proc/cpuinfo and used by the
+	 * kernel. So set those accordingly from the Intel bits.
+	 */
+	if (cpu_has(c, X86_FEATURE_SPEC_CTRL)) {
+		force_setup_cpu_cap(X86_FEATURE_IBRS);
+		force_setup_cpu_cap(X86_FEATURE_IBPB);
+	}
+	if (cpu_has(c, X86_FEATURE_INTEL_STIBP))
+		force_setup_cpu_cap(X86_FEATURE_STIBP);
+}
+
 static const struct cpu_dev intel_cpu_dev = {
 	.c_vendor	= "Intel",
 	.c_ident	= { "GenuineIntel" },
@@ -1025,6 +1045,7 @@ static const struct cpu_dev intel_cpu_de
 #endif
 	.c_detect_tlb	= intel_detect_tlb,
 	.c_early_init   = early_init_intel,
+	.c_bsp_init	= bsp_init,
 	.c_init		= init_intel,
 	.c_bsp_resume	= intel_bsp_resume,
 	.c_x86_vendor	= X86_VENDOR_INTEL,
#34 lore raw

[MODERATED] Re: [patch V9 00/16] SSB

In reply to Tim Chen
Quoted 56 lines · speck for Tim Chen
Quoted 40 lines · speck for Konrad Rzeszutek Wilk
Quoted 19 lines · speck for Thomas Gleixner

Changes vs. V8:

Addressed review feed back and added Linus variant of alternative_write_msr().

- Use EINVAL in the prctl

- Fix the sbb typos

- Make AMD default to prctl as well (Borislav)

- Add the not affected CPU types (Tim)

I've updated the git repository as well

ssh://[email protected]/linux/speck/linux master (forced update)

There are also two stable backport branches now:

Quoted 7 lines

ssh://[email protected]/linux/speck/linux linux-4.16.y

ssh://[email protected]/linux/speck/linux linux-4.14.y

Delta patch against V8 below. Git bundle against 4.17-rc3 follows in separate mail.

On my Skylake box this does what is expected:

4
RHEL-64 <root@instance-20180426-1449:/home/opc> dmesg | grep -i spec

[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-4.17.0-rc3-v9+ root=UUID=533fa2dd-5b2f-4964-aa99-5ab349614dcd ro crashkernel=auto LANG=en_US.UTF-8 transparent_hugepage=never console=tty0 console=ttyS0,9600 rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0 netroot=iscsi:169.254.0.2:::1:iqn.2015-02.oracle.boot:uefi iscsi_param=node.session.timeo.replacement_timeout=6000 net.ifnames=1 nvme_core.shutdown_timeout=10 ipmi_si.tryacpi=0 ipmi_si.trydmi=0 ipmi_si.trydefaults=0 network-config=e2NvbmZpZzogZGlzYWJsZWR9Cg== spec_store_bypass_disable=on
[    0.000000] please specify mtrr_gran_size/mtrr_chunk_size
[    0.000000] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-4.17.0-rc3-v9+ root=UUID=533fa2dd-5b2f-4964-aa99-5ab349614dcd ro crashkernel=auto LANG=en_US.UTF-8 transparent_hugepage=never console=tty0 console=ttyS0,9600 rd.luks=0 rd.lvm=0 rd.md=0 rd.dm=0 netroot=iscsi:169.254.0.2:::1:iqn.2015-02.oracle.boot:uefi iscsi_param=node.session.timeo.replacement_timeout=6000 net.ifnames=1 nvme_core.shutdown_timeout=10 ipmi_si.tryacpi=0 ipmi_si.trydmi=0 ipmi_si.trydefaults=0 network-config=e2NvbmZpZzogZGlzYWJsZWR9Cg== spec_store_bypass_disable=on
[    0.145004] Spectre V2 : Mitigation: Full generic retpoline
[    0.146001] Spectre V2 : Spectre v2 mitigation: Filling RSB on context switch
[    0.147000] Spectre V2 : Spectre v2 mitigation: Enabling Indirect Branch Prediction Barrier
[    0.148000] Spectre V2 : Enabling Restricted Speculation for firmware calls
[    0.149004] Speculative Store Bypass: Mitigation: Speculative Store Bypass disabled
Quoted 6 lines

RHEL-64 <root@instance-20180426-1449:/home/opc> rdmsr 0x48 --all | uniq 4

And I don't see the weird issue that Tim reported?

Did a trace_printk and IBRS feature was not set when my system check it in

void x86_setup_ap_spec_ctrl(void)

{
        if (boot_cpu_has(X86_FEATURE_IBRS))
                x86_set_spec_ctrl(x86_spec_ctrl_base & ~x86_spec_ctrl_mask);
Quoted 5 lines

I checked cpuid and the IBRS feature bit was there.
I will do some more digging to find out why X86_FEATURE_IBRS is not set by kernel on my system. Konrad, you said someone else already reported the IBRS issue and was that root caused?

I dug out the other thread that reported the IBRS problem. Turns out that this commit

commit d94a155c59c98c19b98ee949eaab6a0312bbd6be

Author: Kirill A. Shutemov <[email protected]>
Date:   Tue Apr 10 12:27:04 2018 +0300

x86/cpu: Prevent cpuinfo_x86::x86_phys_bits adjustment corruption

was causing the IBRS_FEATURE bit to get unset.
I can see msr 0x48 set properly by x86_setup_ap_spec_ctrl after I reverted the patch. The cpu online/offline also works okay now for setting the MSR.

Tim

#35 lore raw

Re: [patch V9 00/16] SSB

Quoted 14 lines · speck for Tim Chen
Quoted 5 lines · speck for Tim Chen

I checked cpuid and the IBRS feature bit was there.
I will do some more digging to find out why X86_FEATURE_IBRS is not set by kernel on my system. Konrad, you said someone else already reported the IBRS issue and was that root caused?

I dug out the other thread that reported the IBRS problem. Turns out that this commit

commit d94a155c59c98c19b98ee949eaab6a0312bbd6be

Author: Kirill A. Shutemov <[email protected]>
Date:   Tue Apr 10 12:27:04 2018 +0300
Quoted 6 lines

x86/cpu: Prevent cpuinfo_x86::x86_phys_bits adjustment corruption

was causing the IBRS_FEATURE bit to get unset.
I can see msr 0x48 set properly by x86_setup_ap_spec_ctrl after I reverted the patch. The cpu online/offline also works okay now for setting the MSR.

So I gave you the proper fix for this in a previous reply for testing...

Can you please try that one instead of the revert?

Added again just in case.

Thanks,

tglx
8<---------------

arch/x86/kernel/cpu/common.c +5 -0
@@ -848,6 +848,11 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 		c->x86_power = edx;
 	}
 
+	if (c->extended_cpuid_level >= 0x80000008) {
+		cpuid(0x80000008, &eax, &ebx, &ecx, &edx);
+		c->x86_capability[CPUID_8000_0008_EBX] = ebx;
+	}
+
 	if (c->extended_cpuid_level >= 0x8000000a)
 		c->x86_capability[CPUID_8000_000A_EDX] = cpuid_edx(0x8000000a);
 
#36 lore raw

[MODERATED] Re: [patch V9 00/16] SSB

In reply to Thomas Gleixner

Sorry for lag yesterday - CFL box I was using in the office randomly ceased functioning so I swore loudly then ran out and bought a Kaby Lake box for which I have ucode (still waiting on SKL ucode for normal box). I've got the v9 patches running quite nicely on this new box and will do a bunch more testing over the next few days. Will test v10 tomorrow but it's a bit academic given the changes. In addition to RHEL patches, I'll see if we can throw the speck-v10 into a 4.17-rc3 test and run it through a bunch of the usual tests we do internally to shake out.

On userspace. So far we're still mostly worried about Java though our QE folks are working through lists of packages that have execmem and other privs looking for additional JITs and sandboxes to prctl.

Jon.

Quoted 197 lines · speck for Thomas Gleixner

Changes vs. V8:

Addressed review feed back and added Linus variant of alternative_write_msr().

- Use EINVAL in the prctl

- Fix the sbb typos

- Make AMD default to prctl as well (Borislav)

- Add the not affected CPU types (Tim)

I've updated the git repository as well

ssh://[email protected]/linux/speck/linux master (forced update)

There are also two stable backport branches now:

ssh://[email protected]/linux/speck/linux linux-4.16.y

ssh://[email protected]/linux/speck/linux linux-4.14.y

Delta patch against V8 below. Git bundle against 4.17-rc3 follows in separate mail.

Thanks,

tglx

8<-------------------

Documentation/admin-guide/kernel-parameters.txt +3 -3
index de1ca7510697..29984fd3dd18 100644
@@ -4052,10 +4052,10 @@
 			on     - Unconditionally disable Speculative Store Bypass
 			off    - Unconditionally enable Speculative Store Bypass
 			auto   - Kernel detects whether the CPU model contains a
-			         vulnerable implementation of Speculative Store
-			         Bypass and picks the most appropriate mitigation
+				 implementation of Speculative Store Bypass and
+				 picks the most appropriate mitigation
 			prctl  - Control Speculative Store Bypass for a thread
-			         via prctl. By default it is enabled. The state
+				 via prctl. By default it is enabled. The state
 				 is inherited on fork.
 
 			Not specifying this option is equivalent to
Documentation/userspace-api/spec_ctrl.rst +2 -3
index 867b8b435baa..8ff39a26a992 100644
@@ -56,11 +56,10 @@ Common error codes
 ======= =================================================================
 Value   Meaning
 ======= =================================================================
-EINVAL  The prctl is not implemented by the architecture
+EINVAL  The prctl is not implemented by the architecture or unused
+        prctl(2) arguments are not 0
 
 ENODEV  arg2 is selecting a not supported speculation misfeature
-
-EUCLEAN The unused prctl() arguments are not 0
 ======= =================================================================
 
 PR_SET_SPECULATION_CTRL error codes
arch/x86/include/asm/nospec-branch.h +11 -11
index 7a1be0b2c264..023e2edc0f3c 100644
@@ -259,22 +259,22 @@ static inline void vmexit_fill_RSB(void)
 #endif
 }
 
-#define alternative_msr_write(_msr, _val, _feature)		\
-	asm volatile(ALTERNATIVE("",				\
-				 "movl %[msr], %%ecx\n\t"	\
-				 "movl %[val], %%eax\n\t"	\
-				 "movl $0, %%edx\n\t"		\
-				 "wrmsr",			\
-				 _feature)			\
-		     : : [msr] "i" (_msr), [val] "m" (_val)	\
-		     : "eax", "ecx", "edx", "memory")
+static __always_inline
+void alternative_msr_write(unsigned int msr, u64 val, unsigned int feature)
+{
+	asm volatile(ALTERNATIVE("", "wrmsr", %c[feature])
+		: : "c" (msr),
+		    "a" (val),
+		    "d" (val >> 32),
+		    [feature] "i" (feature)
+		: "memory");
+}
 
 static inline void indirect_branch_prediction_barrier(void)
 {
 	u64 val = PRED_CMD_IBPB;
 
-	alternative_msr_write(MSR_IA32_PRED_CMD, val,
-			      X86_FEATURE_USE_IBPB);
+	alternative_msr_write(MSR_IA32_PRED_CMD, val, X86_FEATURE_USE_IBPB);
 }
 
 /*
arch/x86/kernel/cpu/bugs.c +4 -9
index d52ccd0ce507..c28856e475c8 100644
@@ -476,11 +476,6 @@ static enum ssb_mitigation_cmd __init __ssb_select_mitigation(void)
 
 	switch (cmd) {
 	case SPEC_STORE_BYPASS_CMD_AUTO:
-		/*
-		 * AMD platforms by default don't need SSB mitigation.
-		 */
-		if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
-			break;
 		/* Choose prctl as the default mode */
 		mode = SPEC_STORE_BYPASS_PRCTL;
 		break;
@@ -531,7 +526,7 @@ static void ssb_select_mitigation()
 
 #undef pr_fmt
 
-static int sbb_prctl_set(unsigned long ctrl)
+static int ssb_prctl_set(unsigned long ctrl)
 {
 	bool rds = !!test_tsk_thread_flag(current, TIF_RDS);
 
@@ -549,7 +544,7 @@ static int sbb_prctl_set(unsigned long ctrl)
 	return 0;
 }
 
-static int sbb_prctl_get(void)
+static int ssb_prctl_get(void)
 {
 	switch (ssb_mode) {
 	case SPEC_STORE_BYPASS_DISABLE:
@@ -572,7 +567,7 @@ int arch_prctl_set_spec_ctrl(unsigned long which, unsigned long ctrl)
 
 	switch (which) {
 	case PR_SPEC_STORE_BYPASS:
-		return sbb_prctl_set(ctrl);
+		return ssb_prctl_set(ctrl);
 	default:
 		return -ENODEV;
 	}
@@ -582,7 +577,7 @@ int arch_prctl_get_spec_ctrl(unsigned long which)
 {
 	switch (which) {
 	case PR_SPEC_STORE_BYPASS:
-		return sbb_prctl_get();
+		return ssb_prctl_get();
 	default:
 		return -ENODEV;
 	}
arch/x86/kernel/cpu/common.c +3 -0
index c3b53bc30a29..6101e3dba2f6 100644
@@ -931,6 +931,9 @@ static const __initconst struct x86_cpu_id cpu_no_spec_store_bypass[] = {
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_CEDARVIEW },
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_SILVERMONT1 },
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_AIRMONT },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_SILVERMONT2 },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_ATOM_MERRIFIELD },
+	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_CORE_YONAH },
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_XEON_PHI_KNL },
 	{ X86_VENDOR_INTEL,     6, INTEL_FAM6_XEON_PHI_KNM },
 	{ X86_VENDOR_CENTAUR,	5 },
kernel/sys.c +4 -6
index 8bb8051bd9a1..d7afe29319f1 100644
@@ -2464,15 +2464,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		break;
 	case PR_SET_SPECULATION_CTRL:
 		if (arg4 || arg5)
-			error = -EUCLEAN;
-		else
-			error = arch_prctl_set_spec_ctrl(arg2, arg3);
+			return -EINVAL;
+		error = arch_prctl_set_spec_ctrl(arg2, arg3);
 		break;
 	case PR_GET_SPECULATION_CTRL:
 		if (arg3 || arg4 || arg5)
-			error = -EUCLEAN;
-		else
-			error = arch_prctl_get_spec_ctrl(arg2);
+			return -EINVAL;
+		error = arch_prctl_get_spec_ctrl(arg2);
 		break;
 	default:
 		error = -EINVAL;
Signature · 1 line
Computer Architect | Sent from my Fedora powered laptop