Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934656AbeAHQOy (ORCPT + 1 other); Mon, 8 Jan 2018 11:14:54 -0500 Received: from merlin.infradead.org ([205.233.59.134]:53378 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932217AbeAHQOw (ORCPT ); Mon, 8 Jan 2018 11:14:52 -0500 Date: Mon, 8 Jan 2018 17:14:37 +0100 From: Peter Zijlstra To: Dave Hansen Cc: Tim Chen , Thomas Gleixner , Andy Lutomirski , Linus Torvalds , Greg KH , Andrea Arcangeli , Andi Kleen , Arjan Van De Ven , David Woodhouse , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 4/8] x86/spec_ctrl: Add sysctl knobs to enable/disable SPEC_CTRL feature Message-ID: <20180108161437.GN32035@hirez.programming.kicks-ass.net> References: <20180108124707.GH32035@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180108124707.GH32035@hirez.programming.kicks-ass.net> User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: 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 #include -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