All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Lendacky, Thomas" <Thomas.Lendacky@amd.com>
To: Thomas Gleixner <tglx@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>
Cc: "x86@kernel.org" <x86@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Jiri Kosina <jkosina@suse.cz>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	David Woodhouse <dwmw@amazon.co.uk>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	Andi Kleen <ak@linux.intel.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Casey Schaufler <casey.schaufler@intel.com>,
	Asit Mallick <asit.k.mallick@intel.com>,
	Arjan van de Ven <arjan@linux.intel.com>,
	Jon Masters <jcm@redhat.com>, Waiman Long <longman9394@gmail.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Dave Stewart <david.c.stewart@intel.com>,
	Kees Cook <keescook@chromium.org>
Subject: Re: [patch V2 18/28] x86/speculation: Prepare for per task indirect branch speculation control
Date: Tue, 27 Nov 2018 17:25:08 +0000	[thread overview]
Message-ID: <7ec59a1a-4caf-24f6-3466-ee1d01594861@amd.com> (raw)
In-Reply-To: <20181125185005.176917199@linutronix.de>

On 11/25/2018 12:33 PM, Thomas Gleixner wrote:
> 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>
> 
> ---
> 
> v1 -> v2: Remove pointless include. Use consistent comments.
> 
> ---
>  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          |   23 +++++++++++++++++++++--
>  5 files changed, 44 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);
>  
> +		/* Conditional STIBP enabled? */
> +		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
> @@ -406,6 +406,11 @@ static __always_inline void spec_ctrl_up
>  	if (static_cpu_has(X86_FEATURE_SSBD))
>  		msr |= ssbd_tif_to_spec_ctrl(tifn);

I did some quick testing and found my original logic was flawed. Since
spec_ctrl_update_msr() can now be called for STIBP, an additional check
is needed to set the SSBD MSR bit.

Both X86_FEATURE_VIRT_SSBD and X86_FEATURE_LS_CFG_SSBD cause
X86_FEATURE_SSBD to be set. Before this patch, spec_ctrl_update_msr() was
only called if X86_FEATURE_SSBD was set and one of the other SSBD features
wasn't set. But now, STIBP can cause spec_ctrl_update_msr() to get called
and cause the SSBD MSR bit to be set when it shouldn't (could result in
a GP fault).

Thanks,
Tom

>  
> +	/* Only evaluate if conditional STIBP is enabled */
> +	if (IS_ENABLED(CONFIG_SMP) &&
> +	    static_branch_unlikely(&switch_to_cond_stibp))
> +		msr |= stibp_tif_to_spec_ctrl(tifn);
> +
>  	wrmsrl(MSR_IA32_SPEC_CTRL, msr);
>  }
>  
> @@ -418,10 +423,16 @@ static __always_inline void spec_ctrl_up
>  static __always_inline void __speculation_ctrl_update(unsigned long tifp,
>  						      unsigned long tifn)
>  {
> +	unsigned long tif_diff = tifp ^ tifn;
>  	bool updmsr = false;
>  
> -	/* If TIF_SSBD is different, select the proper mitigation method */
> -	if ((tifp ^ tifn) & _TIF_SSBD) {
> +	/*
> +	 * If TIF_SSBD is different, select the proper mitigation
> +	 * method. Note that if SSBD mitigation is disabled or permanentely
> +	 * enabled this branch can't be taken because nothing can set
> +	 * TIF_SSBD.
> +	 */
> +	if (tif_diff & _TIF_SSBD) {
>  		if (static_cpu_has(X86_FEATURE_VIRT_SSBD))
>  			amd_set_ssb_virt_state(tifn);
>  		else if (static_cpu_has(X86_FEATURE_LS_CFG_SSBD))
> @@ -430,6 +441,14 @@ static __always_inline void __speculatio
>  			updmsr  = true;
>  	}
>  
> +	/*
> +	 * Only evaluate TIF_SPEC_IB if conditional STIBP 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);
> +
>  	if (updmsr)
>  		spec_ctrl_update_msr(tifn);
>  }
> 
> 

  reply	other threads:[~2018-11-27 17:25 UTC|newest]

Thread overview: 112+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-25 18:33 [patch V2 00/28] x86/speculation: Remedy the STIBP/IBPB overhead Thomas Gleixner
2018-11-25 18:33 ` [patch V2 01/28] x86/speculation: Update the TIF_SSBD comment Thomas Gleixner
2018-11-28 14:20   ` [tip:x86/pti] " tip-bot for Tim Chen
2018-11-29 14:27   ` [patch V2 01/28] " Konrad Rzeszutek Wilk
2018-11-25 18:33 ` [patch V2 02/28] x86/speculation: Clean up spectre_v2_parse_cmdline() Thomas Gleixner
2018-11-28 14:20   ` [tip:x86/pti] " tip-bot for Tim Chen
2018-11-29 14:28   ` [patch V2 02/28] " Konrad Rzeszutek Wilk
2018-11-25 18:33 ` [patch V2 03/28] x86/speculation: Remove unnecessary ret variable in cpu_show_common() Thomas Gleixner
2018-11-28 14:21   ` [tip:x86/pti] " tip-bot for Tim Chen
2018-11-29 14:28   ` [patch V2 03/28] " Konrad Rzeszutek Wilk
2018-11-25 18:33 ` [patch V2 04/28] x86/speculation: Reorganize cpu_show_common() Thomas Gleixner
2018-11-26 15:08   ` Borislav Petkov
2018-11-28 14:22   ` [tip:x86/pti] x86/speculation: Move STIPB/IBPB string conditionals out of cpu_show_common() tip-bot for Tim Chen
2018-11-29 14:29   ` [patch V2 04/28] x86/speculation: Reorganize cpu_show_common() Konrad Rzeszutek Wilk
2018-11-25 18:33 ` [patch V2 05/28] x86/speculation: Disable STIBP when enhanced IBRS is in use Thomas Gleixner
2018-11-28 14:22   ` [tip:x86/pti] " tip-bot for Tim Chen
2018-11-29 14:35   ` [patch V2 05/28] " Konrad Rzeszutek Wilk
2018-11-25 18:33 ` [patch V2 06/28] x86/speculation: Rename SSBD update functions Thomas Gleixner
2018-11-26 15:24   ` Borislav Petkov
2018-11-28 14:23   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-29 14:37   ` [patch V2 06/28] " Konrad Rzeszutek Wilk
2018-11-25 18:33 ` [patch V2 07/28] x86/speculation: Reorganize speculation control MSRs update Thomas Gleixner
2018-11-26 15:47   ` Borislav Petkov
2018-11-28 14:23   ` [tip:x86/pti] " tip-bot for Tim Chen
2018-11-29 14:41   ` [patch V2 07/28] " Konrad Rzeszutek Wilk
2018-11-25 18:33 ` [patch V2 08/28] sched/smt: Make sched_smt_present track topology Thomas Gleixner
2018-11-28 14:24   ` [tip:x86/pti] " tip-bot for Peter Zijlstra (Intel)
2018-11-29 14:42   ` [patch V2 08/28] " Konrad Rzeszutek Wilk
2018-11-29 14:50     ` Konrad Rzeszutek Wilk
2018-11-29 15:48       ` Peter Zijlstra
2018-11-25 18:33 ` [patch V2 09/28] x86/Kconfig: Select SCHED_SMT if SMP enabled Thomas Gleixner
2018-11-28 14:24   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-29 14:44   ` [patch V2 09/28] " Konrad Rzeszutek Wilk
2018-11-25 18:33 ` [patch V2 10/28] sched/smt: Expose sched_smt_present static key Thomas Gleixner
2018-11-28 14:25   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-29 14:44   ` [patch V2 10/28] " Konrad Rzeszutek Wilk
2018-11-25 18:33 ` [patch V2 11/28] x86/speculation: Rework SMT state change Thomas Gleixner
2018-11-28 14:26   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 12/28] x86/l1tf: Show actual SMT state Thomas Gleixner
2018-11-28 14:26   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 13/28] x86/speculation: Reorder the spec_v2 code Thomas Gleixner
2018-11-26 22:21   ` Borislav Petkov
2018-11-28 14:27   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 14/28] x86/speculation: Mark string arrays const correctly Thomas Gleixner
2018-11-28 14:27   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 15/28] x86/speculataion: Mark command line parser data __initdata Thomas Gleixner
2018-11-28 14:28   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 16/28] x86/speculation: Unify conditional spectre v2 print functions Thomas Gleixner
2018-11-28 14:29   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 17/28] x86/speculation: Add command line control for indirect branch speculation Thomas Gleixner
2018-11-28 14:29   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 18/28] x86/speculation: Prepare for per task indirect branch speculation control Thomas Gleixner
2018-11-27 17:25   ` Lendacky, Thomas [this message]
2018-11-27 19:51     ` Tim Chen
2018-11-28  9:39       ` Thomas Gleixner
2018-11-27 20:39     ` Thomas Gleixner
2018-11-27 20:42       ` Thomas Gleixner
2018-11-27 21:52         ` Lendacky, Thomas
2018-11-28 14:30   ` [tip:x86/pti] " tip-bot for Tim Chen
2018-11-25 18:33 ` [patch V2 19/28] x86/process: Consolidate and simplify switch_to_xtra() code Thomas Gleixner
2018-11-26 18:30   ` Borislav Petkov
2018-11-28 14:30   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 20/28] x86/speculation: Avoid __switch_to_xtra() calls Thomas Gleixner
2018-11-28 14:31   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 21/28] x86/speculation: Prepare for conditional IBPB in switch_mm() Thomas Gleixner
2018-11-25 19:11   ` Thomas Gleixner
2018-11-25 20:53   ` Andi Kleen
2018-11-25 22:20     ` Thomas Gleixner
2018-11-25 23:04       ` Andy Lutomirski
2018-11-26  7:10         ` Thomas Gleixner
2018-11-26 13:36           ` Ingo Molnar
2018-11-26  3:07       ` Andi Kleen
2018-11-26  6:50         ` Thomas Gleixner
2018-11-28 14:31   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 22/28] ptrace: Remove unused ptrace_may_access_sched() and MODE_IBRS Thomas Gleixner
2018-11-28 14:32   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 23/28] x86/speculation: Split out TIF update Thomas Gleixner
2018-11-28 14:33   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 24/28] x86/speculation: Prepare arch_smt_update() for PRCTL mode Thomas Gleixner
2018-11-27 20:18   ` Lendacky, Thomas
2018-11-27 20:30     ` Thomas Gleixner
2018-11-27 21:20       ` Lendacky, Thomas
2018-11-28 14:34   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 25/28] x86/speculation: Add prctl() control for indirect branch speculation Thomas Gleixner
2018-11-28 14:34   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 26/28] x86/speculation: Enable prctl mode for spectre_v2_user Thomas Gleixner
2018-11-26  7:56   ` Dominik Brodowski
2018-11-28 14:35   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-25 18:33 ` [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode Thomas Gleixner
2018-11-25 19:35   ` Randy Dunlap
2018-11-25 20:40   ` Linus Torvalds
2018-11-25 20:52     ` Jiri Kosina
2018-11-25 22:28     ` Thomas Gleixner
2018-11-26 13:30       ` Ingo Molnar
2018-11-26 20:48       ` Andrea Arcangeli
2018-11-26 20:58         ` Thomas Gleixner
2018-11-26 21:52           ` Lendacky, Thomas
2018-11-27  0:37             ` Tim Chen
2018-12-04  1:38     ` Tim Chen
2018-12-04  8:39       ` Jiri Kosina
2018-12-04  9:43         ` Arjan van de Ven
2018-12-04  9:46         ` Arjan van de Ven
2018-12-04 17:20       ` Linus Torvalds
2018-12-04 18:58         ` Tim Chen
2018-11-28 14:35   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-12-04 18:45   ` [patch V2 27/28] " Dave Hansen
2018-11-25 18:33 ` [patch V2 28/28] x86/speculation: Provide IBPB always command line options Thomas Gleixner
2018-11-28 14:36   ` [tip:x86/pti] " tip-bot for Thomas Gleixner
2018-11-26 13:37 ` [patch V2 00/28] x86/speculation: Remedy the STIBP/IBPB overhead Ingo Molnar
2018-11-28 14:24 ` Thomas Gleixner
2018-11-29 19:02   ` Tim Chen
2018-12-10 23:43 ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7ec59a1a-4caf-24f6-3466-ee1d01594861@amd.com \
    --to=thomas.lendacky@amd.com \
    --cc=aarcange@redhat.com \
    --cc=ak@linux.intel.com \
    --cc=arjan@linux.intel.com \
    --cc=asit.k.mallick@intel.com \
    --cc=casey.schaufler@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=david.c.stewart@intel.com \
    --cc=dwmw@amazon.co.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=jcm@redhat.com \
    --cc=jkosina@suse.cz \
    --cc=jpoimboe@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman9394@gmail.com \
    --cc=luto@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=tim.c.chen@linux.intel.com \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.