Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature

From: Peter Zijlstra
Date: Mon Jan 08 2018 - 11:14:57 EST


On Mon, Jan 08, 2018 at 01:47:07PM +0100, Peter Zijlstra wrote:
> > a good suggestion, but we encountered some issues with it either
> > crashing the kernel at boot or not properly turning on/off.

The below boots, but I lack stuff to test the enabling.

--- a/arch/x86/include/asm/spec_ctrl.h
+++ b/arch/x86/include/asm/spec_ctrl.h
@@ -9,9 +9,8 @@

void scan_spec_ctrl_feature(struct cpuinfo_x86 *c);
void rescan_spec_ctrl_feature(struct cpuinfo_x86 *c);
-bool ibrs_inuse(void);

-extern unsigned int dynamic_ibrs;
+DECLARE_STATIC_KEY_FALSE(ibrs_key);

static inline void __disable_indirect_speculation(void)
{
@@ -31,21 +30,14 @@ static inline void __enable_indirect_spe
static inline void unprotected_speculation_begin(void)
{
lockdep_assert_irqs_disabled();
- if (dynamic_ibrs)
+ if (static_branch_unlikely(&ibrs_key))
__enable_indirect_speculation();
}

static inline void unprotected_speculation_end(void)
{
- if (dynamic_ibrs) {
+ if (static_branch_unlikely(&ibrs_key))
__disable_indirect_speculation();
- } else {
- /*
- * rmb prevent unwanted speculation when we
- * are setting IBRS
- */
- rmb();
- }
}

#endif /* _ASM_X86_SPEC_CTRL_H */
--- a/arch/x86/kernel/cpu/spec_ctrl.c
+++ b/arch/x86/kernel/cpu/spec_ctrl.c
@@ -7,8 +7,7 @@
#include <asm/spec_ctrl.h>
#include <asm/cpufeature.h>

-unsigned int dynamic_ibrs __read_mostly;
-EXPORT_SYMBOL_GPL(dynamic_ibrs);
+DEFINE_STATIC_KEY_FALSE(ibrs_key);

enum {
IBRS_DISABLED,
@@ -27,7 +26,7 @@ DEFINE_MUTEX(spec_ctrl_mutex);
static inline void set_ibrs_feature(void)
{
if (!ibrs_admin_disabled) {
- dynamic_ibrs = 1;
+ static_branch_enable(&ibrs_key);
ibrs_enabled = IBRS_ENABLED;
}
}
@@ -54,12 +53,6 @@ void rescan_spec_ctrl_feature(struct cpu
}
EXPORT_SYMBOL_GPL(rescan_spec_ctrl_feature);

-bool ibrs_inuse(void)
-{
- return ibrs_enabled == IBRS_ENABLED;
-}
-EXPORT_SYMBOL_GPL(ibrs_inuse);
-
static int __init noibrs(char *str)
{
ibrs_admin_disabled = true;
@@ -124,19 +117,23 @@ static ssize_t ibrs_enabled_write(struct
if (enable == IBRS_DISABLED) {
/* disable IBRS usage */
ibrs_admin_disabled = true;
- dynamic_ibrs = 0;
+ static_branch_disable(&ibrs_key);
spec_ctrl_flush_all_cpus(MSR_IA32_SPEC_CTRL,
SPEC_CTRL_FEATURE_DISABLE_IBRS);

} else if (enable == IBRS_ENABLED) {
/* enable IBRS usage in kernel */
ibrs_admin_disabled = false;
- dynamic_ibrs = 1;
+ static_branch_enable(&ibrs_key);
+ /*
+ * This too needs an IPI to force all active CPUs into a known state.
+ */

} else if (enable == IBRS_ENABLED_USER) {
/* enable IBRS all the time in both userspace and kernel */
ibrs_admin_disabled = false;
- dynamic_ibrs = 0;
+ static_branch_disable(&ibrs_key);
+
spec_ctrl_flush_all_cpus(MSR_IA32_SPEC_CTRL,
SPEC_CTRL_FEATURE_ENABLE_IBRS);
}
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -373,22 +373,17 @@ For 32-bit we have the following convent
.endm

.macro ENABLE_IBRS
- testl $1, dynamic_ibrs
- jz .Lskip_\@
+ STATIC_JUMP_IF_FALSE .Lskip_\@, ibrs_key, def=0

PUSH_MSR_REGS
WRMSR_ASM $MSR_IA32_SPEC_CTRL, $SPEC_CTRL_FEATURE_ENABLE_IBRS
POP_MSR_REGS
- jmp .Ldone_\@

.Lskip_\@:
- lfence
-.Ldone_\@:
.endm

.macro DISABLE_IBRS
- testl $1, dynamic_ibrs
- jz .Lskip_\@
+ STATIC_JUMP_IF_FALSE .Lskip_\@, ibrs_key, def=0

PUSH_MSR_REGS
WRMSR_ASM $MSR_IA32_SPEC_CTRL, $SPEC_CTRL_FEATURE_DISABLE_IBRS
@@ -398,20 +393,15 @@ For 32-bit we have the following convent
.endm

.macro ENABLE_IBRS_CLOBBER
- testl $1, dynamic_ibrs
- jz .Lskip_\@
+ STATIC_JUMP_IF_FALSE .Lskip_\@, ibrs_key, def=0

WRMSR_ASM $MSR_IA32_SPEC_CTRL, $SPEC_CTRL_FEATURE_ENABLE_IBRS
- jmp .Ldone_\@

.Lskip_\@:
- lfence
-.Ldone_\@:
.endm

.macro DISABLE_IBRS_CLOBBER
- testl $1, dynamic_ibrs
- jz .Lskip_\@
+ STATIC_JUMP_IF_FALSE .Lskip_\@, ibrs_key, def=0

WRMSR_ASM $MSR_IA32_SPEC_CTRL, $SPEC_CTRL_FEATURE_DISABLE_IBRS

@@ -419,8 +409,7 @@ For 32-bit we have the following convent
.endm

.macro ENABLE_IBRS_SAVE_AND_CLOBBER save_reg:req
- testl $1, dynamic_ibrs
- jz .Lskip_\@
+ STATIC_JUMP_IF_FALSE .Lskip_\@, ibrs_key, def=0

movl $MSR_IA32_SPEC_CTRL, %ecx
rdmsr
@@ -429,25 +418,18 @@ For 32-bit we have the following convent
movl $0, %edx
movl $SPEC_CTRL_FEATURE_ENABLE_IBRS, %eax
wrmsr
- jmp .Ldone_\@

.Lskip_\@:
- lfence
-.Ldone_\@:
.endm

.macro RESTORE_IBRS_CLOBBER save_reg:req
- testl $1, dynamic_ibrs
- jz .Lskip_\@
+ STATIC_JUMP_IF_FALSE .Lskip_\@, ibrs_key, def=0

/* Set IBRS to the value saved in the save_reg */
movl $MSR_IA32_SPEC_CTRL, %ecx
movl $0, %edx
movl \save_reg, %eax
wrmsr
- jmp .Ldone_\@

.Lskip_\@:
- lfence
-.Ldone_\@:
.endm