Messages in this thread | | | Date | Thu, 22 Nov 2018 08:57:09 +0100 | From | Ingo Molnar <> | Subject | Re: [patch 16/24] x86/speculation: Prepare for per task indirect branch speculation control |
| |
* Thomas Gleixner <tglx@linutronix.de> wrote:
> From: Tim Chen <tim.c.chen@linux.intel.com> > > To avoid the overhead of STIBP always on, it's necessary to allow per task > control of STIBP. > > Add a new task flag TIF_SPEC_IB and evaluate it during context switch if > SMT is active and flag evaluation is enabled by the speculation control > code. Add the conditional evaluation to x86_virt_spec_ctrl() as well so the > guest/host switch works properly. > > This has no effect because TIF_SPEC_IB cannot be set yet and the static key > which controls evaluation is off. Preparatory patch for adding the control > code. > > [ tglx: Simplify the context switch logic and make the TIF evaluation > depend on SMP=y and on the static key controlling the conditional > update. Rename it to TIF_SPEC_IB because it controls both STIBP and > IBPB ] > > Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > > --- > arch/x86/include/asm/msr-index.h | 5 +++-- > arch/x86/include/asm/spec-ctrl.h | 12 ++++++++++++ > arch/x86/include/asm/thread_info.h | 5 ++++- > arch/x86/kernel/cpu/bugs.c | 4 ++++ > arch/x86/kernel/process.c | 24 ++++++++++++++++++++++-- > 5 files changed, 45 insertions(+), 5 deletions(-) > > --- a/arch/x86/include/asm/msr-index.h > +++ b/arch/x86/include/asm/msr-index.h > @@ -41,9 +41,10 @@ > > #define MSR_IA32_SPEC_CTRL 0x00000048 /* 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 */ > +#define SPEC_CTRL_SSBD (1 << SPEC_CTRL_SSBD_SHIFT) /* Speculative Store Bypass Disable */ > > #define MSR_IA32_PRED_CMD 0x00000049 /* Prediction Command */ > #define PRED_CMD_IBPB (1 << 0) /* Indirect Branch Prediction Barrier */ > --- 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( > 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_IB < SPEC_CTRL_STIBP_SHIFT); > + return (tifn & _TIF_SPEC_IB) >> (TIF_SPEC_IB - 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_IB < SPEC_CTRL_STIBP_SHIFT); > + return (spec_ctrl & SPEC_CTRL_STIBP) << (TIF_SPEC_IB - 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; > --- 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_EMU 6 /* syscall emulation active */ > #define TIF_SYSCALL_AUDIT 7 /* syscall auditing active */ > #define TIF_SECCOMP 8 /* secure computing */ > +#define TIF_SPEC_IB 9 /* Indirect branch speculation mitigation */ > #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_IB (1 << TIF_SPEC_IB) > #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_IB) > > #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY) > #define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW) > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -148,6 +148,10 @@ x86_virt_spec_ctrl(u64 guest_spec_ctrl, > static_cpu_has(X86_FEATURE_AMD_SSBD)) > hostval |= ssbd_tif_to_spec_ctrl(ti->flags); > > + /* Check whether dynamic indirect branch control is on */ > + if (static_branch_unlikely(&switch_to_cond_stibp)) > + hostval |= stibp_tif_to_spec_ctrl(ti->flags); > + > if (hostval != guestval) { > msrval = setguest ? guestval : hostval; > wrmsrl(MSR_IA32_SPEC_CTRL, msrval); > --- a/arch/x86/kernel/process.c > +++ b/arch/x86/kernel/process.c > @@ -12,6 +12,7 @@ > #include <linux/sched/debug.h> > #include <linux/sched/task.h> > #include <linux/sched/task_stack.h> > +#include <linux/sched/topology.h> > #include <linux/init.h> > #include <linux/export.h> > #include <linux/pm.h> > @@ -406,6 +407,11 @@ static __always_inline void spec_ctrl_up > if (static_cpu_has(X86_FEATURE_SSBD)) > msr |= ssbd_tif_to_spec_ctrl(tifn); > > + /* Only evaluate STIBP if dynamic control is enabled */ > + if (IS_ENABLED(CONFIG_SMP) && > + static_branch_unlikely(&switch_to_cond_stibp)) > + msr |= stibp_tif_to_spec_ctrl(tifn);
> + /* > + * Only evaluate TIF_SPEC_IB if dynamic control is > + * enabled, otherwise avoid the MSR write > + */ > + if (IS_ENABLED(CONFIG_SMP) && > + static_branch_unlikely(&switch_to_cond_stibp)) > + updmsr |= !!(tif_diff & _TIF_SPEC_IB);
Small nit:
we use several terms here in an interchangeable fashion:
- 'dynamic control' - 'conditional STIBP'
The in-code variable naming follows the second nomenclature, while we often mention 'dynamic control' - and the relationship is not always obvious immediately.
It might make sense to pick one of these - for example if we pick 'conditional STIBP' then the second comment would become:
/* * Only evaluate TIF_SPEC_IB if conditional STIBP is * enabled, otherwise avoid the MSR write */
etc.
Thanks,
ngo
| |