Re: [Patch v7 10/18] x86/speculation: Turn on or off STIBP according to a task's TIF_STIBP

2018-11-21 Thread Thomas Gleixner
On Tue, 20 Nov 2018, Tim Chen wrote:
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 74bef48..48fcd46 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -406,6 +406,8 @@ static __always_inline void spec_ctrl_update_msr(unsigned 
> long tifn)
>   if (static_cpu_has(X86_FEATURE_SSBD))
>   msr |= ssbd_tif_to_spec_ctrl(tifn);
>  
> + msr |= stibp_tif_to_spec_ctrl(tifn);

This is wrong. If STIBP is disabled, but the TIF flag is set to control
IBPB and this path is entered to handle a SSBD change, then this will set
STIBP as well.

> +
>   wrmsrl(MSR_IA32_SPEC_CTRL, msr);
>  }
>  
> @@ -418,7 +420,17 @@ static __always_inline void 
> spec_ctrl_update_msr(unsigned long tifn)
>  static __always_inline void __speculation_ctrl_update(unsigned long tifp,
> unsigned long tifn)
>  {
> - bool updmsr = false;
> + bool updmsr;
> +
> + /*
> +  * Need STIBP defense against Spectre v2 attack
> +  * if SMT is in use and enhanced IBRS is unsupported.
> +  */
> + if (static_cpu_has(X86_FEATURE_STIBP) && cpu_use_smt_and_hotplug &&
> + !static_cpu_has(X86_FEATURE_USE_IBRS_ENHANCED))
> + updmsr = !!((tifp ^ tifn) & _TIF_SPEC_INDIR_BRANCH);

At the end of the series this has then two alternative patch bits and two
static keys. This can be done with a single static key which is controlled
from the bug code.

I'll send out a cleaned up version of that stuff later today.

Thanks,

tglx


Re: [Patch v7 10/18] x86/speculation: Turn on or off STIBP according to a task's TIF_STIBP

2018-11-21 Thread Thomas Gleixner
On Tue, 20 Nov 2018, Tim Chen wrote:
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 74bef48..48fcd46 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -406,6 +406,8 @@ static __always_inline void spec_ctrl_update_msr(unsigned 
> long tifn)
>   if (static_cpu_has(X86_FEATURE_SSBD))
>   msr |= ssbd_tif_to_spec_ctrl(tifn);
>  
> + msr |= stibp_tif_to_spec_ctrl(tifn);

This is wrong. If STIBP is disabled, but the TIF flag is set to control
IBPB and this path is entered to handle a SSBD change, then this will set
STIBP as well.

> +
>   wrmsrl(MSR_IA32_SPEC_CTRL, msr);
>  }
>  
> @@ -418,7 +420,17 @@ static __always_inline void 
> spec_ctrl_update_msr(unsigned long tifn)
>  static __always_inline void __speculation_ctrl_update(unsigned long tifp,
> unsigned long tifn)
>  {
> - bool updmsr = false;
> + bool updmsr;
> +
> + /*
> +  * Need STIBP defense against Spectre v2 attack
> +  * if SMT is in use and enhanced IBRS is unsupported.
> +  */
> + if (static_cpu_has(X86_FEATURE_STIBP) && cpu_use_smt_and_hotplug &&
> + !static_cpu_has(X86_FEATURE_USE_IBRS_ENHANCED))
> + updmsr = !!((tifp ^ tifn) & _TIF_SPEC_INDIR_BRANCH);

At the end of the series this has then two alternative patch bits and two
static keys. This can be done with a single static key which is controlled
from the bug code.

I'll send out a cleaned up version of that stuff later today.

Thanks,

tglx


[Patch v7 10/18] x86/speculation: Turn on or off STIBP according to a task's TIF_STIBP

2018-11-20 Thread Tim Chen
If STIBP is used all the time, tasks that do not need STIBP
protection will get unnecessarily slowed down by STIBP.

To apply STIBP only to tasks that need it, a new task TIF_STIBP flag is
created. A x86 CPU uses STIBP only for tasks labeled with TIF_STIBP.

During context switch, this flag is checked and the STIBP bit in SPEC_CTRL
MSR is updated according to changes in this flag between previous and
next task.

Signed-off-by: Tim Chen 
---
 arch/x86/include/asm/msr-index.h   |  6 +-
 arch/x86/include/asm/spec-ctrl.h   | 12 
 arch/x86/include/asm/thread_info.h |  5 -
 arch/x86/kernel/process.c  | 14 +-
 4 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 80f4a4f..501c9d3 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -41,7 +41,11 @@
 
 #define MSR_IA32_SPEC_CTRL 0x0048 /* 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_STIBP_SHIFT  1  /*
+   * Single Thread Indirect 
Branch
+   * Predictor (STIBP) bit
+   */
+#define SPEC_CTRL_STIBP(1 << SPEC_CTRL_STIBP_SHIFT) /* 
STIBP mask */
 #define SPEC_CTRL_SSBD_SHIFT   2  /* Speculative Store Bypass 
Disable bit */
 #define SPEC_CTRL_SSBD (1 << SPEC_CTRL_SSBD_SHIFT)   /* 
Speculative Store Bypass Disable */
 
diff --git a/arch/x86/include/asm/spec-ctrl.h b/arch/x86/include/asm/spec-ctrl.h
index 8e2f841..41b993e 100644
--- a/arch/x86/include/asm/spec-ctrl.h
+++ b/arch/x86/include/asm/spec-ctrl.h
@@ -53,12 +53,24 @@ static inline u64 ssbd_tif_to_spec_ctrl(u64 tifn)
return (tifn & _TIF_SSBD) >> (TIF_SSBD - SPEC_CTRL_SSBD_SHIFT);
 }
 
+static inline u64 stibp_tif_to_spec_ctrl(u64 tifn)
+{
+   BUILD_BUG_ON(TIF_SPEC_INDIR_BRANCH < SPEC_CTRL_STIBP_SHIFT);
+   return (tifn & _TIF_SPEC_INDIR_BRANCH) >> (TIF_SPEC_INDIR_BRANCH - 
SPEC_CTRL_STIBP_SHIFT);
+}
+
 static inline unsigned long ssbd_spec_ctrl_to_tif(u64 spec_ctrl)
 {
BUILD_BUG_ON(TIF_SSBD < SPEC_CTRL_SSBD_SHIFT);
return (spec_ctrl & SPEC_CTRL_SSBD) << (TIF_SSBD - 
SPEC_CTRL_SSBD_SHIFT);
 }
 
+static inline unsigned long stibp_spec_ctrl_to_tif(u64 spec_ctrl)
+{
+   BUILD_BUG_ON(TIF_SPEC_INDIR_BRANCH < SPEC_CTRL_STIBP_SHIFT);
+   return (spec_ctrl & SPEC_CTRL_STIBP) << (TIF_SPEC_INDIR_BRANCH - 
SPEC_CTRL_STIBP_SHIFT);
+}
+
 static inline u64 ssbd_tif_to_amd_ls_cfg(u64 tifn)
 {
return (tifn & _TIF_SSBD) ? x86_amd_ls_cfg_ssbd_mask : 0ULL;
diff --git a/arch/x86/include/asm/thread_info.h 
b/arch/x86/include/asm/thread_info.h
index 2ff2a30..b3032c7 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -83,6 +83,7 @@ struct thread_info {
 #define TIF_SYSCALL_EMU6   /* syscall emulation active */
 #define TIF_SYSCALL_AUDIT  7   /* syscall auditing active */
 #define TIF_SECCOMP8   /* secure computing */
+#define TIF_SPEC_INDIR_BRANCH  9   /* Indirect branch speculation 
restricted */
 #define TIF_USER_RETURN_NOTIFY 11  /* notify kernel of userspace return */
 #define TIF_UPROBE 12  /* breakpointed or singlestepping */
 #define TIF_PATCH_PENDING  13  /* pending live patching update */
@@ -110,6 +111,7 @@ struct thread_info {
 #define _TIF_SYSCALL_EMU   (1 << TIF_SYSCALL_EMU)
 #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)
 #define _TIF_SECCOMP   (1 << TIF_SECCOMP)
+#define _TIF_SPEC_INDIR_BRANCH (1 << TIF_SPEC_INDIR_BRANCH)
 #define _TIF_USER_RETURN_NOTIFY(1 << TIF_USER_RETURN_NOTIFY)
 #define _TIF_UPROBE(1 << TIF_UPROBE)
 #define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING)
@@ -146,7 +148,8 @@ struct thread_info {
 
 /* flags to check in __switch_to() */
 #define _TIF_WORK_CTXSW
\
-   (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP|_TIF_SSBD)
+   (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP| \
+_TIF_SSBD|_TIF_SPEC_INDIR_BRANCH)
 
 #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)
 #define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 74bef48..48fcd46 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -406,6 +406,8 @@ static __always_inline void spec_ctrl_update_msr(unsigned 
long tifn)
if (static_cpu_has(X86_FEATURE_SSBD))
msr |= ssbd_tif_to_spec_ctrl(tifn);
 
+   msr |= stibp_tif_to_spec_ctrl(tifn);
+
  

[Patch v7 10/18] x86/speculation: Turn on or off STIBP according to a task's TIF_STIBP

2018-11-20 Thread Tim Chen
If STIBP is used all the time, tasks that do not need STIBP
protection will get unnecessarily slowed down by STIBP.

To apply STIBP only to tasks that need it, a new task TIF_STIBP flag is
created. A x86 CPU uses STIBP only for tasks labeled with TIF_STIBP.

During context switch, this flag is checked and the STIBP bit in SPEC_CTRL
MSR is updated according to changes in this flag between previous and
next task.

Signed-off-by: Tim Chen 
---
 arch/x86/include/asm/msr-index.h   |  6 +-
 arch/x86/include/asm/spec-ctrl.h   | 12 
 arch/x86/include/asm/thread_info.h |  5 -
 arch/x86/kernel/process.c  | 14 +-
 4 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 80f4a4f..501c9d3 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -41,7 +41,11 @@
 
 #define MSR_IA32_SPEC_CTRL 0x0048 /* 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_STIBP_SHIFT  1  /*
+   * Single Thread Indirect 
Branch
+   * Predictor (STIBP) bit
+   */
+#define SPEC_CTRL_STIBP(1 << SPEC_CTRL_STIBP_SHIFT) /* 
STIBP mask */
 #define SPEC_CTRL_SSBD_SHIFT   2  /* Speculative Store Bypass 
Disable bit */
 #define SPEC_CTRL_SSBD (1 << SPEC_CTRL_SSBD_SHIFT)   /* 
Speculative Store Bypass Disable */
 
diff --git a/arch/x86/include/asm/spec-ctrl.h b/arch/x86/include/asm/spec-ctrl.h
index 8e2f841..41b993e 100644
--- a/arch/x86/include/asm/spec-ctrl.h
+++ b/arch/x86/include/asm/spec-ctrl.h
@@ -53,12 +53,24 @@ static inline u64 ssbd_tif_to_spec_ctrl(u64 tifn)
return (tifn & _TIF_SSBD) >> (TIF_SSBD - SPEC_CTRL_SSBD_SHIFT);
 }
 
+static inline u64 stibp_tif_to_spec_ctrl(u64 tifn)
+{
+   BUILD_BUG_ON(TIF_SPEC_INDIR_BRANCH < SPEC_CTRL_STIBP_SHIFT);
+   return (tifn & _TIF_SPEC_INDIR_BRANCH) >> (TIF_SPEC_INDIR_BRANCH - 
SPEC_CTRL_STIBP_SHIFT);
+}
+
 static inline unsigned long ssbd_spec_ctrl_to_tif(u64 spec_ctrl)
 {
BUILD_BUG_ON(TIF_SSBD < SPEC_CTRL_SSBD_SHIFT);
return (spec_ctrl & SPEC_CTRL_SSBD) << (TIF_SSBD - 
SPEC_CTRL_SSBD_SHIFT);
 }
 
+static inline unsigned long stibp_spec_ctrl_to_tif(u64 spec_ctrl)
+{
+   BUILD_BUG_ON(TIF_SPEC_INDIR_BRANCH < SPEC_CTRL_STIBP_SHIFT);
+   return (spec_ctrl & SPEC_CTRL_STIBP) << (TIF_SPEC_INDIR_BRANCH - 
SPEC_CTRL_STIBP_SHIFT);
+}
+
 static inline u64 ssbd_tif_to_amd_ls_cfg(u64 tifn)
 {
return (tifn & _TIF_SSBD) ? x86_amd_ls_cfg_ssbd_mask : 0ULL;
diff --git a/arch/x86/include/asm/thread_info.h 
b/arch/x86/include/asm/thread_info.h
index 2ff2a30..b3032c7 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -83,6 +83,7 @@ struct thread_info {
 #define TIF_SYSCALL_EMU6   /* syscall emulation active */
 #define TIF_SYSCALL_AUDIT  7   /* syscall auditing active */
 #define TIF_SECCOMP8   /* secure computing */
+#define TIF_SPEC_INDIR_BRANCH  9   /* Indirect branch speculation 
restricted */
 #define TIF_USER_RETURN_NOTIFY 11  /* notify kernel of userspace return */
 #define TIF_UPROBE 12  /* breakpointed or singlestepping */
 #define TIF_PATCH_PENDING  13  /* pending live patching update */
@@ -110,6 +111,7 @@ struct thread_info {
 #define _TIF_SYSCALL_EMU   (1 << TIF_SYSCALL_EMU)
 #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)
 #define _TIF_SECCOMP   (1 << TIF_SECCOMP)
+#define _TIF_SPEC_INDIR_BRANCH (1 << TIF_SPEC_INDIR_BRANCH)
 #define _TIF_USER_RETURN_NOTIFY(1 << TIF_USER_RETURN_NOTIFY)
 #define _TIF_UPROBE(1 << TIF_UPROBE)
 #define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING)
@@ -146,7 +148,8 @@ struct thread_info {
 
 /* flags to check in __switch_to() */
 #define _TIF_WORK_CTXSW
\
-   (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP|_TIF_SSBD)
+   (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP| \
+_TIF_SSBD|_TIF_SPEC_INDIR_BRANCH)
 
 #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)
 #define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 74bef48..48fcd46 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -406,6 +406,8 @@ static __always_inline void spec_ctrl_update_msr(unsigned 
long tifn)
if (static_cpu_has(X86_FEATURE_SSBD))
msr |= ssbd_tif_to_spec_ctrl(tifn);
 
+   msr |= stibp_tif_to_spec_ctrl(tifn);
+