From 6650cdd9a8ccf00555dbbe743d58541ad8feb6a7 Mon Sep 17 00:00:00 2001 From: "Peter Zijlstra (Intel)" Date: Sun, 26 Jan 2020 12:05:35 -0800 Subject: [PATCH 1/3] x86/split_lock: Enable split lock detection by kernel A split-lock occurs when an atomic instruction operates on data that spans two cache lines. In order to maintain atomicity the core takes a global bus lock. This is typically >1000 cycles slower than an atomic operation within a cache line. It also disrupts performance on other cores (which must wait for the bus lock to be released before their memory operations can complete). For real-time systems this may mean missing deadlines. For other systems it may just be very annoying. Some CPUs have the capability to raise an #AC trap when a split lock is attempted. Provide a command line option to give the user choices on how to handle this: split_lock_detect= off - not enabled (no traps for split locks) warn - warn once when an application does a split lock, but allow it to continue running. fatal - Send SIGBUS to applications that cause split lock On systems that support split lock detection the default is "warn". Note that if the kernel hits a split lock in any mode other than "off" it will OOPs. One implementation wrinkle is that the MSR to control the split lock detection is per-core, not per thread. This might result in some short lived races on HT systems in "warn" mode if Linux tries to enable on one thread while disabling on the other. Race analysis by Sean Christopherson: - Toggling of split-lock is only done in "warn" mode. Worst case scenario of a race is that a misbehaving task will generate multiple #AC exceptions on the same instruction. And this race will only occur if both siblings are running tasks that generate split-lock #ACs, e.g. a race where sibling threads are writing different values will only occur if CPUx is disabling split-lock after an #AC and CPUy is re-enabling split-lock after *its* previous task generated an #AC. - Transitioning between off/warn/fatal modes at runtime isn't supported and disabling is tracked per task, so hardware will always reach a steady state that matches the configured mode. I.e. split-lock is guaranteed to be enabled in hardware once all _TIF_SLD threads have been scheduled out. Signed-off-by: Peter Zijlstra (Intel) Co-developed-by: Fenghua Yu Signed-off-by: Fenghua Yu Co-developed-by: Tony Luck Signed-off-by: Tony Luck Signed-off-by: Thomas Gleixner Signed-off-by: Borislav Petkov Link: https://lore.kernel.org/r/20200126200535.GB30377@agluck-desk2.amr.corp.intel.com --- .../admin-guide/kernel-parameters.txt | 22 +++ arch/x86/include/asm/cpu.h | 12 ++ arch/x86/include/asm/cpufeatures.h | 2 + arch/x86/include/asm/msr-index.h | 9 + arch/x86/include/asm/thread_info.h | 4 +- arch/x86/kernel/cpu/common.c | 2 + arch/x86/kernel/cpu/intel.c | 175 ++++++++++++++++++ arch/x86/kernel/process.c | 3 + arch/x86/kernel/traps.c | 24 ++- 9 files changed, 250 insertions(+), 3 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index dbc22d684627..62c2b0b6922e 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -4655,6 +4655,28 @@ spia_pedr= spia_peddr= + split_lock_detect= + [X86] Enable split lock detection + + When enabled (and if hardware support is present), atomic + instructions that access data across cache line + boundaries will result in an alignment check exception. + + off - not enabled + + warn - the kernel will emit rate limited warnings + about applications triggering the #AC + exception. This mode is the default on CPUs + that supports split lock detection. + + fatal - the kernel will send SIGBUS to applications + that trigger the #AC exception. + + If an #AC exception is hit in the kernel or in + firmware (i.e. not while executing in user mode) + the kernel will oops in either "warn" or "fatal" + mode. + srcutree.counter_wrap_check [KNL] Specifies how frequently to check for grace-period sequence counter wrap for the diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h index adc6cc86b062..ff6f3ca649b3 100644 --- a/arch/x86/include/asm/cpu.h +++ b/arch/x86/include/asm/cpu.h @@ -40,4 +40,16 @@ int mwait_usable(const struct cpuinfo_x86 *); unsigned int x86_family(unsigned int sig); unsigned int x86_model(unsigned int sig); unsigned int x86_stepping(unsigned int sig); +#ifdef CONFIG_CPU_SUP_INTEL +extern void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c); +extern void switch_to_sld(unsigned long tifn); +extern bool handle_user_split_lock(struct pt_regs *regs, long error_code); +#else +static inline void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c) {} +static inline void switch_to_sld(unsigned long tifn) {} +static inline bool handle_user_split_lock(struct pt_regs *regs, long error_code) +{ + return false; +} +#endif #endif /* _ASM_X86_CPU_H */ diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index f3327cb56edf..cd56ad5d308e 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -285,6 +285,7 @@ #define X86_FEATURE_CQM_MBM_LOCAL (11*32+ 3) /* LLC Local MBM monitoring */ #define X86_FEATURE_FENCE_SWAPGS_USER (11*32+ 4) /* "" LFENCE in user entry SWAPGS path */ #define X86_FEATURE_FENCE_SWAPGS_KERNEL (11*32+ 5) /* "" LFENCE in kernel entry SWAPGS path */ +#define X86_FEATURE_SPLIT_LOCK_DETECT (11*32+ 6) /* #AC for split lock */ /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */ #define X86_FEATURE_AVX512_BF16 (12*32+ 5) /* AVX512 BFLOAT16 instructions */ @@ -367,6 +368,7 @@ #define X86_FEATURE_INTEL_STIBP (18*32+27) /* "" Single Thread Indirect Branch Predictors */ #define X86_FEATURE_FLUSH_L1D (18*32+28) /* Flush L1D cache */ #define X86_FEATURE_ARCH_CAPABILITIES (18*32+29) /* IA32_ARCH_CAPABILITIES MSR (Intel) */ +#define X86_FEATURE_CORE_CAPABILITIES (18*32+30) /* "" IA32_CORE_CAPABILITIES MSR */ #define X86_FEATURE_SPEC_CTRL_SSBD (18*32+31) /* "" Speculative Store Bypass Disable */ /* diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h index ebe1685e92dd..8821697a7549 100644 --- a/arch/x86/include/asm/msr-index.h +++ b/arch/x86/include/asm/msr-index.h @@ -41,6 +41,10 @@ /* Intel MSRs. Some also available on other CPUs */ +#define MSR_TEST_CTRL 0x00000033 +#define MSR_TEST_CTRL_SPLIT_LOCK_DETECT_BIT 29 +#define MSR_TEST_CTRL_SPLIT_LOCK_DETECT BIT(MSR_TEST_CTRL_SPLIT_LOCK_DETECT_BIT) + #define MSR_IA32_SPEC_CTRL 0x00000048 /* Speculation Control */ #define SPEC_CTRL_IBRS BIT(0) /* Indirect Branch Restricted Speculation */ #define SPEC_CTRL_STIBP_SHIFT 1 /* Single Thread Indirect Branch Predictor (STIBP) bit */ @@ -70,6 +74,11 @@ */ #define MSR_IA32_UMWAIT_CONTROL_TIME_MASK (~0x03U) +/* Abbreviated from Intel SDM name IA32_CORE_CAPABILITIES */ +#define MSR_IA32_CORE_CAPS 0x000000cf +#define MSR_IA32_CORE_CAPS_SPLIT_LOCK_DETECT_BIT 5 +#define MSR_IA32_CORE_CAPS_SPLIT_LOCK_DETECT BIT(MSR_IA32_CORE_CAPS_SPLIT_LOCK_DETECT_BIT) + #define MSR_PKG_CST_CONFIG_CONTROL 0x000000e2 #define NHM_C3_AUTO_DEMOTE (1UL << 25) #define NHM_C1_AUTO_DEMOTE (1UL << 26) diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h index cf4327986e98..f807930bd763 100644 --- a/arch/x86/include/asm/thread_info.h +++ b/arch/x86/include/asm/thread_info.h @@ -92,6 +92,7 @@ struct thread_info { #define TIF_NOCPUID 15 /* CPUID is not accessible in userland */ #define TIF_NOTSC 16 /* TSC is not accessible in userland */ #define TIF_IA32 17 /* IA32 compatibility process */ +#define TIF_SLD 18 /* Restore split lock detection on context switch */ #define TIF_NOHZ 19 /* in adaptive nohz mode */ #define TIF_MEMDIE 20 /* is terminating due to OOM killer */ #define TIF_POLLING_NRFLAG 21 /* idle is polling for TIF_NEED_RESCHED */ @@ -122,6 +123,7 @@ struct thread_info { #define _TIF_NOCPUID (1 << TIF_NOCPUID) #define _TIF_NOTSC (1 << TIF_NOTSC) #define _TIF_IA32 (1 << TIF_IA32) +#define _TIF_SLD (1 << TIF_SLD) #define _TIF_NOHZ (1 << TIF_NOHZ) #define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG) #define _TIF_IO_BITMAP (1 << TIF_IO_BITMAP) @@ -145,7 +147,7 @@ struct thread_info { /* flags to check in __switch_to() */ #define _TIF_WORK_CTXSW_BASE \ (_TIF_NOCPUID | _TIF_NOTSC | _TIF_BLOCKSTEP | \ - _TIF_SSBD | _TIF_SPEC_FORCE_UPDATE) + _TIF_SSBD | _TIF_SPEC_FORCE_UPDATE | _TIF_SLD) /* * Avoid calls to __switch_to_xtra() on UP as STIBP is not evaluated. diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 52c9bfbbdb2a..bf282d7805de 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -1224,6 +1224,8 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c) cpu_set_bug_bits(c); + cpu_set_core_cap_bits(c); + fpu__init_system(c); #ifdef CONFIG_X86_32 diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index be82cd5841c3..db3e745e5d47 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -19,6 +19,8 @@ #include #include #include +#include +#include #ifdef CONFIG_X86_64 #include @@ -31,6 +33,19 @@ #include #endif +enum split_lock_detect_state { + sld_off = 0, + sld_warn, + sld_fatal, +}; + +/* + * Default to sld_off because most systems do not support split lock detection + * split_lock_setup() will switch this to sld_warn on systems that support + * split lock detect, unless there is a command line override. + */ +static enum split_lock_detect_state sld_state = sld_off; + /* * Processors which have self-snooping capability can handle conflicting * memory type across CPUs by snooping its own cache. However, there exists @@ -570,6 +585,8 @@ static void init_intel_misc_features(struct cpuinfo_x86 *c) wrmsrl(MSR_MISC_FEATURES_ENABLES, msr); } +static void split_lock_init(void); + static void init_intel(struct cpuinfo_x86 *c) { early_init_intel(c); @@ -684,6 +701,8 @@ static void init_intel(struct cpuinfo_x86 *c) tsx_enable(); if (tsx_ctrl_state == TSX_CTRL_DISABLE) tsx_disable(); + + split_lock_init(); } #ifdef CONFIG_X86_32 @@ -945,3 +964,159 @@ static const struct cpu_dev intel_cpu_dev = { }; cpu_dev_register(intel_cpu_dev); + +#undef pr_fmt +#define pr_fmt(fmt) "x86/split lock detection: " fmt + +static const struct { + const char *option; + enum split_lock_detect_state state; +} sld_options[] __initconst = { + { "off", sld_off }, + { "warn", sld_warn }, + { "fatal", sld_fatal }, +}; + +static inline bool match_option(const char *arg, int arglen, const char *opt) +{ + int len = strlen(opt); + + return len == arglen && !strncmp(arg, opt, len); +} + +static void __init split_lock_setup(void) +{ + char arg[20]; + int i, ret; + + setup_force_cpu_cap(X86_FEATURE_SPLIT_LOCK_DETECT); + sld_state = sld_warn; + + ret = cmdline_find_option(boot_command_line, "split_lock_detect", + arg, sizeof(arg)); + if (ret >= 0) { + for (i = 0; i < ARRAY_SIZE(sld_options); i++) { + if (match_option(arg, ret, sld_options[i].option)) { + sld_state = sld_options[i].state; + break; + } + } + } + + switch (sld_state) { + case sld_off: + pr_info("disabled\n"); + break; + + case sld_warn: + pr_info("warning about user-space split_locks\n"); + break; + + case sld_fatal: + pr_info("sending SIGBUS on user-space split_locks\n"); + break; + } +} + +/* + * Locking is not required at the moment because only bit 29 of this + * MSR is implemented and locking would not prevent that the operation + * of one thread is immediately undone by the sibling thread. + * Use the "safe" versions of rdmsr/wrmsr here because although code + * checks CPUID and MSR bits to make sure the TEST_CTRL MSR should + * exist, there may be glitches in virtualization that leave a guest + * with an incorrect view of real h/w capabilities. + */ +static bool __sld_msr_set(bool on) +{ + u64 test_ctrl_val; + + if (rdmsrl_safe(MSR_TEST_CTRL, &test_ctrl_val)) + return false; + + if (on) + test_ctrl_val |= MSR_TEST_CTRL_SPLIT_LOCK_DETECT; + else + test_ctrl_val &= ~MSR_TEST_CTRL_SPLIT_LOCK_DETECT; + + return !wrmsrl_safe(MSR_TEST_CTRL, test_ctrl_val); +} + +static void split_lock_init(void) +{ + if (sld_state == sld_off) + return; + + if (__sld_msr_set(true)) + return; + + /* + * If this is anything other than the boot-cpu, you've done + * funny things and you get to keep whatever pieces. + */ + pr_warn("MSR fail -- disabled\n"); + sld_state = sld_off; +} + +bool handle_user_split_lock(struct pt_regs *regs, long error_code) +{ + if ((regs->flags & X86_EFLAGS_AC) || sld_state == sld_fatal) + return false; + + pr_warn_ratelimited("#AC: %s/%d took a split_lock trap at address: 0x%lx\n", + current->comm, current->pid, regs->ip); + + /* + * Disable the split lock detection for this task so it can make + * progress and set TIF_SLD so the detection is re-enabled via + * switch_to_sld() when the task is scheduled out. + */ + __sld_msr_set(false); + set_tsk_thread_flag(current, TIF_SLD); + return true; +} + +/* + * This function is called only when switching between tasks with + * different split-lock detection modes. It sets the MSR for the + * mode of the new task. This is right most of the time, but since + * the MSR is shared by hyperthreads on a physical core there can + * be glitches when the two threads need different modes. + */ +void switch_to_sld(unsigned long tifn) +{ + __sld_msr_set(!(tifn & _TIF_SLD)); +} + +#define SPLIT_LOCK_CPU(model) {X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY} + +/* + * The following processors have the split lock detection feature. But + * since they don't have the IA32_CORE_CAPABILITIES MSR, the feature cannot + * be enumerated. Enable it by family and model matching on these + * processors. + */ +static const struct x86_cpu_id split_lock_cpu_ids[] __initconst = { + SPLIT_LOCK_CPU(INTEL_FAM6_ICELAKE_X), + SPLIT_LOCK_CPU(INTEL_FAM6_ICELAKE_L), + {} +}; + +void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c) +{ + u64 ia32_core_caps = 0; + + if (c->x86_vendor != X86_VENDOR_INTEL) + return; + if (cpu_has(c, X86_FEATURE_CORE_CAPABILITIES)) { + /* Enumerate features reported in IA32_CORE_CAPABILITIES MSR. */ + rdmsrl(MSR_IA32_CORE_CAPS, ia32_core_caps); + } else if (!boot_cpu_has(X86_FEATURE_HYPERVISOR)) { + /* Enumerate split lock detection by family and model. */ + if (x86_match_cpu(split_lock_cpu_ids)) + ia32_core_caps |= MSR_IA32_CORE_CAPS_SPLIT_LOCK_DETECT; + } + + if (ia32_core_caps & MSR_IA32_CORE_CAPS_SPLIT_LOCK_DETECT) + split_lock_setup(); +} diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c index 839b5244e3b7..a43c32868c3c 100644 --- a/arch/x86/kernel/process.c +++ b/arch/x86/kernel/process.c @@ -650,6 +650,9 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p) /* Enforce MSR update to ensure consistent state */ __speculation_ctrl_update(~tifn, tifn); } + + if ((tifp ^ tifn) & _TIF_SLD) + switch_to_sld(tifn); } /* diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index 6ef00eb6fbb9..0ef5befaed7d 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -46,6 +46,7 @@ #include #include #include +#include #include #include #include @@ -242,7 +243,6 @@ do_trap(int trapnr, int signr, char *str, struct pt_regs *regs, { struct task_struct *tsk = current; - if (!do_trap_no_signal(tsk, trapnr, str, regs, error_code)) return; @@ -288,9 +288,29 @@ DO_ERROR(X86_TRAP_OLD_MF, SIGFPE, 0, NULL, "coprocessor segment overru DO_ERROR(X86_TRAP_TS, SIGSEGV, 0, NULL, "invalid TSS", invalid_TSS) DO_ERROR(X86_TRAP_NP, SIGBUS, 0, NULL, "segment not present", segment_not_present) DO_ERROR(X86_TRAP_SS, SIGBUS, 0, NULL, "stack segment", stack_segment) -DO_ERROR(X86_TRAP_AC, SIGBUS, BUS_ADRALN, NULL, "alignment check", alignment_check) #undef IP +dotraplinkage void do_alignment_check(struct pt_regs *regs, long error_code) +{ + char *str = "alignment check"; + + RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU"); + + if (notify_die(DIE_TRAP, str, regs, error_code, X86_TRAP_AC, SIGBUS) == NOTIFY_STOP) + return; + + if (!user_mode(regs)) + die("Split lock detected\n", regs, error_code); + + local_irq_enable(); + + if (handle_user_split_lock(regs, error_code)) + return; + + do_trap(X86_TRAP_AC, SIGBUS, "alignment check", regs, + error_code, BUS_ADRALN, NULL); +} + #ifdef CONFIG_VMAP_STACK __visible void __noreturn handle_stack_overflow(const char *message, struct pt_regs *regs, From dbaba47085b0c2aa793ce849750164bd3765e163 Mon Sep 17 00:00:00 2001 From: Xiaoyao Li Date: Wed, 25 Mar 2020 11:09:23 +0800 Subject: [PATCH 2/3] x86/split_lock: Rework the initialization flow of split lock detection Current initialization flow of split lock detection has following issues: 1. It assumes the initial value of MSR_TEST_CTRL.SPLIT_LOCK_DETECT to be zero. However, it's possible that BIOS/firmware has set it. 2. X86_FEATURE_SPLIT_LOCK_DETECT flag is unconditionally set even if there is a virtualization flaw that FMS indicates the existence while it's actually not supported. Rework the initialization flow to solve above issues. In detail, explicitly clear and set split_lock_detect bit to verify MSR_TEST_CTRL can be accessed, and rdmsr after wrmsr to ensure bit is cleared/set successfully. X86_FEATURE_SPLIT_LOCK_DETECT flag is set only when the feature does exist and the feature is not disabled with kernel param "split_lock_detect=off" On each processor, explicitly updating the SPLIT_LOCK_DETECT bit based on sld_sate in split_lock_init() since BIOS/firmware may touch it. Originally-by: Thomas Gleixner Signed-off-by: Xiaoyao Li Signed-off-by: Thomas Gleixner Link: https://lkml.kernel.org/r/20200325030924.132881-2-xiaoyao.li@intel.com --- arch/x86/kernel/cpu/intel.c | 75 +++++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index db3e745e5d47..0c859c91d008 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -44,7 +44,7 @@ enum split_lock_detect_state { * split_lock_setup() will switch this to sld_warn on systems that support * split lock detect, unless there is a command line override. */ -static enum split_lock_detect_state sld_state = sld_off; +static enum split_lock_detect_state sld_state __ro_after_init = sld_off; /* * Processors which have self-snooping capability can handle conflicting @@ -984,78 +984,87 @@ static inline bool match_option(const char *arg, int arglen, const char *opt) return len == arglen && !strncmp(arg, opt, len); } +static bool split_lock_verify_msr(bool on) +{ + u64 ctrl, tmp; + + if (rdmsrl_safe(MSR_TEST_CTRL, &ctrl)) + return false; + if (on) + ctrl |= MSR_TEST_CTRL_SPLIT_LOCK_DETECT; + else + ctrl &= ~MSR_TEST_CTRL_SPLIT_LOCK_DETECT; + if (wrmsrl_safe(MSR_TEST_CTRL, ctrl)) + return false; + rdmsrl(MSR_TEST_CTRL, tmp); + return ctrl == tmp; +} + static void __init split_lock_setup(void) { + enum split_lock_detect_state state = sld_warn; char arg[20]; int i, ret; - setup_force_cpu_cap(X86_FEATURE_SPLIT_LOCK_DETECT); - sld_state = sld_warn; + if (!split_lock_verify_msr(false)) { + pr_info("MSR access failed: Disabled\n"); + return; + } ret = cmdline_find_option(boot_command_line, "split_lock_detect", arg, sizeof(arg)); if (ret >= 0) { for (i = 0; i < ARRAY_SIZE(sld_options); i++) { if (match_option(arg, ret, sld_options[i].option)) { - sld_state = sld_options[i].state; + state = sld_options[i].state; break; } } } - switch (sld_state) { + switch (state) { case sld_off: pr_info("disabled\n"); - break; - + return; case sld_warn: pr_info("warning about user-space split_locks\n"); break; - case sld_fatal: pr_info("sending SIGBUS on user-space split_locks\n"); break; } + + if (!split_lock_verify_msr(true)) { + pr_info("MSR access failed: Disabled\n"); + return; + } + + sld_state = state; + setup_force_cpu_cap(X86_FEATURE_SPLIT_LOCK_DETECT); } /* - * Locking is not required at the moment because only bit 29 of this - * MSR is implemented and locking would not prevent that the operation - * of one thread is immediately undone by the sibling thread. - * Use the "safe" versions of rdmsr/wrmsr here because although code - * checks CPUID and MSR bits to make sure the TEST_CTRL MSR should - * exist, there may be glitches in virtualization that leave a guest - * with an incorrect view of real h/w capabilities. + * MSR_TEST_CTRL is per core, but we treat it like a per CPU MSR. Locking + * is not implemented as one thread could undo the setting of the other + * thread immediately after dropping the lock anyway. */ -static bool __sld_msr_set(bool on) +static void sld_update_msr(bool on) { u64 test_ctrl_val; - if (rdmsrl_safe(MSR_TEST_CTRL, &test_ctrl_val)) - return false; + rdmsrl(MSR_TEST_CTRL, test_ctrl_val); if (on) test_ctrl_val |= MSR_TEST_CTRL_SPLIT_LOCK_DETECT; else test_ctrl_val &= ~MSR_TEST_CTRL_SPLIT_LOCK_DETECT; - return !wrmsrl_safe(MSR_TEST_CTRL, test_ctrl_val); + wrmsrl(MSR_TEST_CTRL, test_ctrl_val); } static void split_lock_init(void) { - if (sld_state == sld_off) - return; - - if (__sld_msr_set(true)) - return; - - /* - * If this is anything other than the boot-cpu, you've done - * funny things and you get to keep whatever pieces. - */ - pr_warn("MSR fail -- disabled\n"); - sld_state = sld_off; + split_lock_verify_msr(sld_state != sld_off); } bool handle_user_split_lock(struct pt_regs *regs, long error_code) @@ -1071,7 +1080,7 @@ bool handle_user_split_lock(struct pt_regs *regs, long error_code) * progress and set TIF_SLD so the detection is re-enabled via * switch_to_sld() when the task is scheduled out. */ - __sld_msr_set(false); + sld_update_msr(false); set_tsk_thread_flag(current, TIF_SLD); return true; } @@ -1085,7 +1094,7 @@ bool handle_user_split_lock(struct pt_regs *regs, long error_code) */ void switch_to_sld(unsigned long tifn) { - __sld_msr_set(!(tifn & _TIF_SLD)); + sld_update_msr(!(tifn & _TIF_SLD)); } #define SPLIT_LOCK_CPU(model) {X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY} From a6a60741035bb48ca8d9f92a138958818148064c Mon Sep 17 00:00:00 2001 From: Xiaoyao Li Date: Wed, 25 Mar 2020 11:09:24 +0800 Subject: [PATCH 3/3] x86/split_lock: Avoid runtime reads of the TEST_CTRL MSR In a context switch from a task that is detecting split locks to one that is not (or vice versa) we need to update the TEST_CTRL MSR. Currently this is done with the common sequence: read the MSR flip the bit write the MSR in order to avoid changing the value of any reserved bits in the MSR. Cache unused and reserved bits of TEST_CTRL MSR with SPLIT_LOCK_DETECT bit cleared during initialization, so we can avoid an expensive RDMSR instruction during context switch. Suggested-by: Sean Christopherson Originally-by: Tony Luck Signed-off-by: Xiaoyao Li Signed-off-by: Thomas Gleixner Link: https://lkml.kernel.org/r/20200325030924.132881-3-xiaoyao.li@intel.com --- arch/x86/kernel/cpu/intel.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index 0c859c91d008..9a26e972cdea 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -45,6 +45,7 @@ enum split_lock_detect_state { * split lock detect, unless there is a command line override. */ static enum split_lock_detect_state sld_state __ro_after_init = sld_off; +static u64 msr_test_ctrl_cache __ro_after_init; /* * Processors which have self-snooping capability can handle conflicting @@ -1034,6 +1035,8 @@ static void __init split_lock_setup(void) break; } + rdmsrl(MSR_TEST_CTRL, msr_test_ctrl_cache); + if (!split_lock_verify_msr(true)) { pr_info("MSR access failed: Disabled\n"); return; @@ -1050,14 +1053,10 @@ static void __init split_lock_setup(void) */ static void sld_update_msr(bool on) { - u64 test_ctrl_val; - - rdmsrl(MSR_TEST_CTRL, test_ctrl_val); + u64 test_ctrl_val = msr_test_ctrl_cache; if (on) test_ctrl_val |= MSR_TEST_CTRL_SPLIT_LOCK_DETECT; - else - test_ctrl_val &= ~MSR_TEST_CTRL_SPLIT_LOCK_DETECT; wrmsrl(MSR_TEST_CTRL, test_ctrl_val); }