[PATCH 1/2] x86/speculation: Change STIPB to STIBP

[PATCH 1/2] x86/speculation: Change STIPB to STIBP

[Waiman Long]

STIBP stands for Single Thread Indirect Branch Predictors. The acronym,
however, can be easily mis-spelled as STIPB. It is perhaps due to the
presence of another related term - IBPB (Indirect Branch Predictor
Barrier).

Fix the mis-spelling in the code.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 arch/x86/kernel/cpu/bugs.c | 6 +++---
 arch/x86/kernel/process.h  | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 500278f..a68b32c 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -54,7 +54,7 @@
 u64 __ro_after_init x86_amd_ls_cfg_base;
 u64 __ro_after_init x86_amd_ls_cfg_ssbd_mask;
 
-/* Control conditional STIPB in switch_to() */
+/* Control conditional STIBP in switch_to() */
 DEFINE_STATIC_KEY_FALSE(switch_to_cond_stibp);
 /* Control conditional IBPB in switch_mm() */
 DEFINE_STATIC_KEY_FALSE(switch_mm_cond_ibpb);
@@ -379,12 +379,12 @@ static void __init spec_v2_user_print_cond(const char *reason, bool secure)
 			"always-on" : "conditional");
 	}
 
-	/* If enhanced IBRS is enabled no STIPB required */
+	/* If enhanced IBRS is enabled no STIBP required */
 	if (spectre_v2_enabled == SPECTRE_V2_IBRS_ENHANCED)
 		return;
 
 	/*
-	 * If SMT is not possible or STIBP is not available clear the STIPB
+	 * If SMT is not possible or STIBP is not available clear the STIBP
 	 * mode.
 	 */
 	if (!smt_possible || !boot_cpu_has(X86_FEATURE_STIBP))
diff --git a/arch/x86/kernel/process.h b/arch/x86/kernel/process.h
index 898e97c..320ab97 100644
--- a/arch/x86/kernel/process.h
+++ b/arch/x86/kernel/process.h
@@ -19,7 +19,7 @@ static inline void switch_to_extra(struct task_struct *prev,
 	if (IS_ENABLED(CONFIG_SMP)) {
 		/*
 		 * Avoid __switch_to_xtra() invocation when conditional
-		 * STIPB is disabled and the only different bit is
+		 * STIBP is disabled and the only different bit is
 		 * TIF_SPEC_IB. For CONFIG_SMP=n TIF_SPEC_IB is not
 		 * in the TIF_WORK_CTXSW masks.
 		 */
-- 
1.8.3.1

[PATCH 2/2] x86/speculation: switch_to_cond_stibp on is the likely case

[Waiman Long]

Since conditional STIBP is the default, it should be treated as
the likely case. Changes the use of static_branch_unlikely() to
static_branch_likely() for switch_to_cond_stibp.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 arch/x86/kernel/cpu/bugs.c | 2 +-
 arch/x86/kernel/process.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index a68b32c..bd11119 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -153,7 +153,7 @@ void __init check_bugs(void)
 			hostval |= ssbd_tif_to_spec_ctrl(ti->flags);
 
 		/* Conditional STIBP enabled? */
-		if (static_branch_unlikely(&switch_to_cond_stibp))
+		if (static_branch_likely(&switch_to_cond_stibp))
 			hostval |= stibp_tif_to_spec_ctrl(ti->flags);
 
 		if (hostval != guestval) {
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 7d31192..39a21ae 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -434,7 +434,7 @@ static __always_inline void __speculation_ctrl_update(unsigned long tifp,
 	 * otherwise avoid the MSR write.
 	 */
 	if (IS_ENABLED(CONFIG_SMP) &&
-	    static_branch_unlikely(&switch_to_cond_stibp)) {
+	    static_branch_likely(&switch_to_cond_stibp)) {
 		updmsr |= !!(tif_diff & _TIF_SPEC_IB);
 		msr |= stibp_tif_to_spec_ctrl(tifn);
 	}
-- 
1.8.3.1

Re: [PATCH 2/2] x86/speculation: switch_to_cond_stibp on is the likely case

[Peter Zijlstra]

On Wed, Dec 05, 2018 at 02:49:28PM -0500, Waiman Long wrote:
> Since conditional STIBP is the default, it should be treated as
> the likely case. Changes the use of static_branch_unlikely() to
> static_branch_likely() for switch_to_cond_stibp.

So now you're making kernels on 'fixed' or unaffected hardware slower.

> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>  arch/x86/kernel/cpu/bugs.c | 2 +-
>  arch/x86/kernel/process.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index a68b32c..bd11119 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -153,7 +153,7 @@ void __init check_bugs(void)
>  			hostval |= ssbd_tif_to_spec_ctrl(ti->flags);
>  
>  		/* Conditional STIBP enabled? */
> -		if (static_branch_unlikely(&switch_to_cond_stibp))
> +		if (static_branch_likely(&switch_to_cond_stibp))
>  			hostval |= stibp_tif_to_spec_ctrl(ti->flags);
>  
>  		if (hostval != guestval) {
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 7d31192..39a21ae 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -434,7 +434,7 @@ static __always_inline void __speculation_ctrl_update(unsigned long tifp,
>  	 * otherwise avoid the MSR write.
>  	 */
>  	if (IS_ENABLED(CONFIG_SMP) &&
> -	    static_branch_unlikely(&switch_to_cond_stibp)) {
> +	    static_branch_likely(&switch_to_cond_stibp)) {
>  		updmsr |= !!(tif_diff & _TIF_SPEC_IB);
>  		msr |= stibp_tif_to_spec_ctrl(tifn);
>  	}
> -- 
> 1.8.3.1
> 

[tip:x86/pti] x86/speculation: Change misspelled STIPB to STIBP

[tip-bot for Waiman Long]

Commit-ID:  aa77bfb354c495fc4361199e63fc5765b9e1e783
Gitweb:     https://git.kernel.org/tip/aa77bfb354c495fc4361199e63fc5765b9e1e783
Author:     Waiman Long <longman@redhat.com>
AuthorDate: Wed, 5 Dec 2018 14:49:27 -0500
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Thu, 6 Dec 2018 11:49:15 +0100

x86/speculation: Change misspelled STIPB to STIBP

STIBP stands for Single Thread Indirect Branch Predictors. The acronym,
however, can be easily mis-spelled as STIPB. It is perhaps due to the
presence of another related term - IBPB (Indirect Branch Predictor
Barrier).

Fix the mis-spelling in the code.

Signed-off-by: Waiman Long <longman@redhat.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: KarimAllah Ahmed <karahmed@amazon.de>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: x86-ml <x86@kernel.org>
Link: https://lkml.kernel.org/r/1544039368-9009-1-git-send-email-longman@redhat.com
---
 arch/x86/kernel/cpu/bugs.c | 6 +++---
 arch/x86/kernel/process.h  | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 500278f5308e..a68b32cb845a 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -54,7 +54,7 @@ static u64 __ro_after_init x86_spec_ctrl_mask = SPEC_CTRL_IBRS;
 u64 __ro_after_init x86_amd_ls_cfg_base;
 u64 __ro_after_init x86_amd_ls_cfg_ssbd_mask;
 
-/* Control conditional STIPB in switch_to() */
+/* Control conditional STIBP in switch_to() */
 DEFINE_STATIC_KEY_FALSE(switch_to_cond_stibp);
 /* Control conditional IBPB in switch_mm() */
 DEFINE_STATIC_KEY_FALSE(switch_mm_cond_ibpb);
@@ -379,12 +379,12 @@ spectre_v2_user_select_mitigation(enum spectre_v2_mitigation_cmd v2_cmd)
 			"always-on" : "conditional");
 	}
 
-	/* If enhanced IBRS is enabled no STIPB required */
+	/* If enhanced IBRS is enabled no STIBP required */
 	if (spectre_v2_enabled == SPECTRE_V2_IBRS_ENHANCED)
 		return;
 
 	/*
-	 * If SMT is not possible or STIBP is not available clear the STIPB
+	 * If SMT is not possible or STIBP is not available clear the STIBP
 	 * mode.
 	 */
 	if (!smt_possible || !boot_cpu_has(X86_FEATURE_STIBP))
diff --git a/arch/x86/kernel/process.h b/arch/x86/kernel/process.h
index 898e97cf6629..320ab978fb1f 100644
--- a/arch/x86/kernel/process.h
+++ b/arch/x86/kernel/process.h
@@ -19,7 +19,7 @@ static inline void switch_to_extra(struct task_struct *prev,
 	if (IS_ENABLED(CONFIG_SMP)) {
 		/*
 		 * Avoid __switch_to_xtra() invocation when conditional
-		 * STIPB is disabled and the only different bit is
+		 * STIBP is disabled and the only different bit is
 		 * TIF_SPEC_IB. For CONFIG_SMP=n TIF_SPEC_IB is not
 		 * in the TIF_WORK_CTXSW masks.
 		 */

Re: [PATCH 2/2] x86/speculation: switch_to_cond_stibp on is the likely case

[Waiman Long]

On 12/06/2018 03:41 AM, Peter Zijlstra wrote:
> On Wed, Dec 05, 2018 at 02:49:28PM -0500, Waiman Long wrote:
>> Since conditional STIBP is the default, it should be treated as
>> the likely case. Changes the use of static_branch_unlikely() to
>> static_branch_likely() for switch_to_cond_stibp.
> So now you're making kernels on 'fixed' or unaffected hardware slower.

Good point.

The reason I sent out this patch is because of the inconsistency in the
use of likely/unlikely hints.

arch/x86/kernel/cpu/bugs.c:156:        if
(static_branch_unlikely(&switch_to_cond_stibp))
arch/x86/kernel/process.c:440:       
static_branch_unlikely(&switch_to_cond_stibp)) {
arch/x86/kernel/process.h:26:        if
(!static_branch_likely(&switch_to_cond_stibp)) {

So if we are aiming to optimize for "fixed" or unaffected hardware,
maybe we should modify the likely hint to unlikely then.

Cheers,
Longman

Re: [PATCH 2/2] x86/speculation: switch_to_cond_stibp on is the likely case

[Peter Zijlstra]

On Thu, Dec 06, 2018 at 10:38:00AM -0500, Waiman Long wrote:
> On 12/06/2018 03:41 AM, Peter Zijlstra wrote:
> > On Wed, Dec 05, 2018 at 02:49:28PM -0500, Waiman Long wrote:
> >> Since conditional STIBP is the default, it should be treated as
> >> the likely case. Changes the use of static_branch_unlikely() to
> >> static_branch_likely() for switch_to_cond_stibp.
> > So now you're making kernels on 'fixed' or unaffected hardware slower.
> 
> Good point.
> 
> The reason I sent out this patch is because of the inconsistency in the
> use of likely/unlikely hints.
> 
> arch/x86/kernel/cpu/bugs.c:156:        if
> (static_branch_unlikely(&switch_to_cond_stibp))
> arch/x86/kernel/process.c:440:       
> static_branch_unlikely(&switch_to_cond_stibp)) {
> arch/x86/kernel/process.h:26:        if
> (!static_branch_likely(&switch_to_cond_stibp)) {
> 
> So if we are aiming to optimize for "fixed" or unaffected hardware,
> maybe we should modify the likely hint to unlikely then.

Right, I think that makes sense, Thomas?

Re: [PATCH 2/2] x86/speculation: switch_to_cond_stibp on is the likely case

[Thomas Gleixner]

[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]

On Fri, 7 Dec 2018, Peter Zijlstra wrote:
> On Thu, Dec 06, 2018 at 10:38:00AM -0500, Waiman Long wrote:
> > On 12/06/2018 03:41 AM, Peter Zijlstra wrote:
> > > On Wed, Dec 05, 2018 at 02:49:28PM -0500, Waiman Long wrote:
> > >> Since conditional STIBP is the default, it should be treated as
> > >> the likely case. Changes the use of static_branch_unlikely() to
> > >> static_branch_likely() for switch_to_cond_stibp.
> > > So now you're making kernels on 'fixed' or unaffected hardware slower.
> > 
> > Good point.
> > 
> > The reason I sent out this patch is because of the inconsistency in the
> > use of likely/unlikely hints.
> > 
> > arch/x86/kernel/cpu/bugs.c:156:        if
> > (static_branch_unlikely(&switch_to_cond_stibp))
> > arch/x86/kernel/process.c:440:       
> > static_branch_unlikely(&switch_to_cond_stibp)) {
> > arch/x86/kernel/process.h:26:        if
> > (!static_branch_likely(&switch_to_cond_stibp)) {
> > 
> > So if we are aiming to optimize for "fixed" or unaffected hardware,
> > maybe we should modify the likely hint to unlikely then.
> 
> Right, I think that makes sense, Thomas?

Yeah, I probably got that wrong in some places. Let me look.

Thanks,

	Thomas