All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/speculation: Support Enhanced IBRS on future CPUs
@ 2018-11-07  0:42 Sai Praneeth Prakhya
  2018-11-07  9:25 ` Greg KH
  0 siblings, 1 reply; 11+ messages in thread
From: Sai Praneeth Prakhya @ 2018-11-07  0:42 UTC (permalink / raw)
  To: stable
  Cc: Sai Praneeth, Thomas Gleixner, Tim C Chen, Dave Hansen,
	Ravi Shankar, Andi Kleen

From: Sai Praneeth <sai.praneeth.prakhya@intel.com>

[ Upstream commit 706d51681d636a0c4a5ef53395ec3b803e45ed4d ]

Changes from upstream:
----------------------
1. Use bit 30 of word 7 in cpufeatures for X86_FEATURE_IBRS_ENHANCED as bit 29
   is now used by L1TF.
2. Fix some trivial line fuzzing.

Based on kernel version:
------------------------
Linux 4.9.135

Commit message from upstream:
-----------------------------
Future Intel processors will support "Enhanced IBRS" which is an "always
on" mode i.e. IBRS bit in SPEC_CTRL MSR is enabled once and never
disabled.

>From the specification [1]:

 "With enhanced IBRS, the predicted targets of indirect branches
  executed cannot be controlled by software that was executed in a less
  privileged predictor mode or on another logical processor. As a
  result, software operating on a processor with enhanced IBRS need not
  use WRMSR to set IA32_SPEC_CTRL.IBRS after every transition to a more
  privileged predictor mode. Software can isolate predictor modes
  effectively simply by setting the bit once. Software need not disable
  enhanced IBRS prior to entering a sleep state such as MWAIT or HLT."

If Enhanced IBRS is supported by the processor then use it as the
preferred spectre v2 mitigation mechanism instead of Retpoline. Intel's
Retpoline white paper [2] states:

 "Retpoline is known to be an effective branch target injection (Spectre
  variant 2) mitigation on Intel processors belonging to family 6
  (enumerated by the CPUID instruction) that do not have support for
  enhanced IBRS. On processors that support enhanced IBRS, it should be
  used for mitigation instead of retpoline."

The reason why Enhanced IBRS is the recommended mitigation on processors
which support it is that these processors also support CET which
provides a defense against ROP attacks. Retpoline is very similar to ROP
techniques and might trigger false positives in the CET defense.

If Enhanced IBRS is selected as the mitigation technique for spectre v2,
the IBRS bit in SPEC_CTRL MSR is set once at boot time and never
cleared. Kernel also has to make sure that IBRS bit remains set after
VMEXIT because the guest might have cleared the bit. This is already
covered by the existing x86_spec_ctrl_set_guest() and
x86_spec_ctrl_restore_host() speculation control functions.

Enhanced IBRS still requires IBPB for full mitigation.

[1] Speculative-Execution-Side-Channel-Mitigations.pdf
[2] Retpoline-A-Branch-Target-Injection-Mitigation.pdf
Both documents are available at:
https://bugzilla.kernel.org/show_bug.cgi?id=199511

Originally-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Tim C Chen <tim.c.chen@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: <stable@vger.kernel.org>
---
 arch/x86/include/asm/cpufeatures.h   |  1 +
 arch/x86/include/asm/nospec-branch.h |  1 +
 arch/x86/kernel/cpu/bugs.c           | 20 ++++++++++++++++++--
 arch/x86/kernel/cpu/common.c         |  3 +++
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index f6d1bc93589c..c56c24347f15 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -213,6 +213,7 @@
 #define X86_FEATURE_STIBP	( 7*32+27) /* Single Thread Indirect Branch Predictors */
 #define X86_FEATURE_ZEN		( 7*32+28) /* "" CPU is AMD family 0x17 (Zen) */
 #define X86_FEATURE_L1TF_PTEINV	( 7*32+29) /* "" L1TF workaround PTE inversion */
+#define X86_FEATURE_IBRS_ENHANCED	( 7*32+30) /* Enhanced IBRS */
 
 /* Virtualization flags: Linux defined, word 8 */
 #define X86_FEATURE_TPR_SHADOW  ( 8*32+ 0) /* Intel TPR Shadow */
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 8b38df98548e..1b4132161c1f 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -215,6 +215,7 @@ enum spectre_v2_mitigation {
 	SPECTRE_V2_RETPOLINE_GENERIC,
 	SPECTRE_V2_RETPOLINE_AMD,
 	SPECTRE_V2_IBRS,
+	SPECTRE_V2_IBRS_ENHANCED,
 };
 
 /* The Speculative Store Bypass disable variants */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 8103adacbc83..6221166e3fca 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -139,6 +139,7 @@ static const char *spectre_v2_strings[] = {
 	[SPECTRE_V2_RETPOLINE_MINIMAL_AMD]	= "Vulnerable: Minimal AMD ASM retpoline",
 	[SPECTRE_V2_RETPOLINE_GENERIC]		= "Mitigation: Full generic retpoline",
 	[SPECTRE_V2_RETPOLINE_AMD]		= "Mitigation: Full AMD retpoline",
+	[SPECTRE_V2_IBRS_ENHANCED]		= "Mitigation: Enhanced IBRS",
 };
 
 #undef pr_fmt
@@ -340,6 +341,13 @@ static void __init spectre_v2_select_mitigation(void)
 
 	case SPECTRE_V2_CMD_FORCE:
 	case SPECTRE_V2_CMD_AUTO:
+		if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
+			mode = SPECTRE_V2_IBRS_ENHANCED;
+			/* Force it so VMEXIT will restore correctly */
+			x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
+			wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+			goto specv2_set_mode;
+		}
 		if (IS_ENABLED(CONFIG_RETPOLINE))
 			goto retpoline_auto;
 		break;
@@ -377,6 +385,7 @@ static void __init spectre_v2_select_mitigation(void)
 		setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
 	}
 
+specv2_set_mode:
 	spectre_v2_enabled = mode;
 	pr_info("%s\n", spectre_v2_strings[mode]);
 
@@ -399,9 +408,16 @@ static void __init spectre_v2_select_mitigation(void)
 
 	/*
 	 * Retpoline means the kernel is safe because it has no indirect
-	 * branches. But firmware isn't, so use IBRS to protect that.
+	 * branches. Enhanced IBRS protects firmware too, so, enable restricted
+	 * speculation around firmware calls only when Enhanced IBRS isn't
+	 * supported.
+	 *
+	 * Use "mode" to check Enhanced IBRS instead of boot_cpu_has(), because
+	 * the user might select retpoline on the kernel command line and if
+	 * the CPU supports Enhanced IBRS, kernel might un-intentionally not
+	 * enable IBRS around firmware calls.
 	 */
-	if (boot_cpu_has(X86_FEATURE_IBRS)) {
+	if (boot_cpu_has(X86_FEATURE_IBRS) && mode != SPECTRE_V2_IBRS_ENHANCED) {
 		setup_force_cpu_cap(X86_FEATURE_USE_IBRS_FW);
 		pr_info("Enabling Restricted Speculation for firmware calls\n");
 	}
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index dc0850bb74be..3c01610c5ba9 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -959,6 +959,9 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
 	setup_force_cpu_bug(X86_BUG_SPECTRE_V1);
 	setup_force_cpu_bug(X86_BUG_SPECTRE_V2);
 
+	if (ia32_cap & ARCH_CAP_IBRS_ALL)
+		setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
+
 	if (x86_match_cpu(cpu_no_meltdown))
 		return;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] x86/speculation: Support Enhanced IBRS on future CPUs
  2018-11-07  0:42 [PATCH] x86/speculation: Support Enhanced IBRS on future CPUs Sai Praneeth Prakhya
@ 2018-11-07  9:25 ` Greg KH
  2018-11-07 18:05   ` Prakhya, Sai Praneeth
  0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2018-11-07  9:25 UTC (permalink / raw)
  To: Sai Praneeth Prakhya
  Cc: stable, Thomas Gleixner, Tim C Chen, Dave Hansen, Ravi Shankar,
	Andi Kleen

On Tue, Nov 06, 2018 at 04:42:45PM -0800, Sai Praneeth Prakhya wrote:
> From: Sai Praneeth <sai.praneeth.prakhya@intel.com>
> 
> [ Upstream commit 706d51681d636a0c4a5ef53395ec3b803e45ed4d ]
> 
> Changes from upstream:
> ----------------------
> 1. Use bit 30 of word 7 in cpufeatures for X86_FEATURE_IBRS_ENHANCED as bit 29
>    is now used by L1TF.
> 2. Fix some trivial line fuzzing.
> 
> Based on kernel version:
> ------------------------
> Linux 4.9.135

This format is very odd, shouldn't it go all below the original ---
line?

Please fix up and resend properly.  Also, why 4.9?  What is wrong with
4.18.y, 4.14.y, and 4.4.y?  Just picking one random kernel in the middle
of the stable releases seems foolish for those users of newer kernels,
right?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] x86/speculation: Support Enhanced IBRS on future CPUs
  2018-11-07  9:25 ` Greg KH
@ 2018-11-07 18:05   ` Prakhya, Sai Praneeth
  0 siblings, 0 replies; 11+ messages in thread
From: Prakhya, Sai Praneeth @ 2018-11-07 18:05 UTC (permalink / raw)
  To: Greg KH
  Cc: stable, Thomas Gleixner, Chen, Tim C, Hansen, Dave, Shankar,
	Ravi V, Andi Kleen

> > [ Upstream commit 706d51681d636a0c4a5ef53395ec3b803e45ed4d ]
> >
> > Changes from upstream:
> > ----------------------
> > 1. Use bit 30 of word 7 in cpufeatures for X86_FEATURE_IBRS_ENHANCED as
> bit 29
> >    is now used by L1TF.
> > 2. Fix some trivial line fuzzing.
> >
> > Based on kernel version:
> > ------------------------
> > Linux 4.9.135
> 
> This format is very odd, shouldn't it go all below the original --- line?
> 
> Please fix up and resend properly.

Sure! My bad, sorry! As suggested, I will fix this up and will resend a V2.

> Also, why 4.9?  What is wrong with 4.18.y,
> 4.14.y, and 4.4.y?  Just picking one random kernel in the middle of the stable
> releases seems foolish for those users of newer kernels, right?

I have skipped 4.4 because,
1. It's too old
2. x86/kvm/vmx.c doesn't use x86_spec_ctrl_set_guest() and x86_spec_ctrl_restore_host()
before entering guest and after exiting guest and entering host. This is needed because IBRS
bit in SPEC_CTRL register could be modified by guest but host should still have it set always.

Regards,
Sai

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] x86/speculation: Support Enhanced IBRS on future CPUs
  2018-11-07  0:26 Sai Praneeth Prakhya
@ 2018-11-07  9:26 ` Greg KH
  0 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2018-11-07  9:26 UTC (permalink / raw)
  To: Sai Praneeth Prakhya
  Cc: stable, Thomas Gleixner, Tim C Chen, Dave Hansen, Ravi Shankar,
	Andi Kleen

On Tue, Nov 06, 2018 at 04:26:05PM -0800, Sai Praneeth Prakhya wrote:
> From: Sai Praneeth <sai.praneeth.prakhya@intel.com>
> 
> [ Upstream commit 706d51681d636a0c4a5ef53395ec3b803e45ed4d ]
> 
> Changes from upstream:
> ----------------------
> 1. Use bit 30 of word 7 in cpufeatures for X86_FEATURE_IBRS_ENHANCED as bit 29
>    is now used by L1TF.
> 2. Fix some trivial line fuzzing.
> 
> Note: Based on kernel version "Linux 4.18.17" and to be applied on both "Linux
> 4.18.17" and "Linux 4.14.79".

Ah, here's the other patch, please fix it up so I do not have to
hand-edit a file to apply this.

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] x86/speculation: Support Enhanced IBRS on future CPUs
@ 2018-11-07  0:26 Sai Praneeth Prakhya
  2018-11-07  9:26 ` Greg KH
  0 siblings, 1 reply; 11+ messages in thread
From: Sai Praneeth Prakhya @ 2018-11-07  0:26 UTC (permalink / raw)
  To: stable
  Cc: Sai Praneeth, Thomas Gleixner, Tim C Chen, Dave Hansen,
	Ravi Shankar, Andi Kleen

From: Sai Praneeth <sai.praneeth.prakhya@intel.com>

[ Upstream commit 706d51681d636a0c4a5ef53395ec3b803e45ed4d ]

Changes from upstream:
----------------------
1. Use bit 30 of word 7 in cpufeatures for X86_FEATURE_IBRS_ENHANCED as bit 29
   is now used by L1TF.
2. Fix some trivial line fuzzing.

Note: Based on kernel version "Linux 4.18.17" and to be applied on both "Linux
4.18.17" and "Linux 4.14.79".

Commit message from upstream:
-----------------------------
Future Intel processors will support "Enhanced IBRS" which is an "always
on" mode i.e. IBRS bit in SPEC_CTRL MSR is enabled once and never
disabled.

>From the specification [1]:

 "With enhanced IBRS, the predicted targets of indirect branches
  executed cannot be controlled by software that was executed in a less
  privileged predictor mode or on another logical processor. As a
  result, software operating on a processor with enhanced IBRS need not
  use WRMSR to set IA32_SPEC_CTRL.IBRS after every transition to a more
  privileged predictor mode. Software can isolate predictor modes
  effectively simply by setting the bit once. Software need not disable
  enhanced IBRS prior to entering a sleep state such as MWAIT or HLT."

If Enhanced IBRS is supported by the processor then use it as the
preferred spectre v2 mitigation mechanism instead of Retpoline. Intel's
Retpoline white paper [2] states:

 "Retpoline is known to be an effective branch target injection (Spectre
  variant 2) mitigation on Intel processors belonging to family 6
  (enumerated by the CPUID instruction) that do not have support for
  enhanced IBRS. On processors that support enhanced IBRS, it should be
  used for mitigation instead of retpoline."

The reason why Enhanced IBRS is the recommended mitigation on processors
which support it is that these processors also support CET which
provides a defense against ROP attacks. Retpoline is very similar to ROP
techniques and might trigger false positives in the CET defense.

If Enhanced IBRS is selected as the mitigation technique for spectre v2,
the IBRS bit in SPEC_CTRL MSR is set once at boot time and never
cleared. Kernel also has to make sure that IBRS bit remains set after
VMEXIT because the guest might have cleared the bit. This is already
covered by the existing x86_spec_ctrl_set_guest() and
x86_spec_ctrl_restore_host() speculation control functions.

Enhanced IBRS still requires IBPB for full mitigation.

[1] Speculative-Execution-Side-Channel-Mitigations.pdf
[2] Retpoline-A-Branch-Target-Injection-Mitigation.pdf
Both documents are available at:
https://bugzilla.kernel.org/show_bug.cgi?id=199511

Originally-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Tim C Chen <tim.c.chen@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: <stable@vger.kernel.org>
---
 arch/x86/include/asm/cpufeatures.h   |  1 +
 arch/x86/include/asm/nospec-branch.h |  1 +
 arch/x86/kernel/cpu/bugs.c           | 20 ++++++++++++++++++--
 arch/x86/kernel/cpu/common.c         |  3 +++
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 64aaa3f5f36c..c8ac84e90d0f 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -220,6 +220,7 @@
 #define X86_FEATURE_STIBP		( 7*32+27) /* Single Thread Indirect Branch Predictors */
 #define X86_FEATURE_ZEN			( 7*32+28) /* "" CPU is AMD family 0x17 (Zen) */
 #define X86_FEATURE_L1TF_PTEINV		( 7*32+29) /* "" L1TF workaround PTE inversion */
+#define X86_FEATURE_IBRS_ENHANCED	( 7*32+30) /* Enhanced IBRS */
 
 /* Virtualization flags: Linux defined, word 8 */
 #define X86_FEATURE_TPR_SHADOW		( 8*32+ 0) /* Intel TPR Shadow */
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index f6f6c63da62f..e7c8086e570e 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -215,6 +215,7 @@ enum spectre_v2_mitigation {
 	SPECTRE_V2_RETPOLINE_GENERIC,
 	SPECTRE_V2_RETPOLINE_AMD,
 	SPECTRE_V2_IBRS,
+	SPECTRE_V2_IBRS_ENHANCED,
 };
 
 /* The Speculative Store Bypass disable variants */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 4891a621a752..817e57e96d67 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -141,6 +141,7 @@ static const char *spectre_v2_strings[] = {
 	[SPECTRE_V2_RETPOLINE_MINIMAL_AMD]	= "Vulnerable: Minimal AMD ASM retpoline",
 	[SPECTRE_V2_RETPOLINE_GENERIC]		= "Mitigation: Full generic retpoline",
 	[SPECTRE_V2_RETPOLINE_AMD]		= "Mitigation: Full AMD retpoline",
+	[SPECTRE_V2_IBRS_ENHANCED]		= "Mitigation: Enhanced IBRS",
 };
 
 #undef pr_fmt
@@ -343,6 +344,13 @@ static void __init spectre_v2_select_mitigation(void)
 
 	case SPECTRE_V2_CMD_FORCE:
 	case SPECTRE_V2_CMD_AUTO:
+		if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
+			mode = SPECTRE_V2_IBRS_ENHANCED;
+			/* Force it so VMEXIT will restore correctly */
+			x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
+			wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base);
+			goto specv2_set_mode;
+		}
 		if (IS_ENABLED(CONFIG_RETPOLINE))
 			goto retpoline_auto;
 		break;
@@ -380,6 +388,7 @@ static void __init spectre_v2_select_mitigation(void)
 		setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
 	}
 
+specv2_set_mode:
 	spectre_v2_enabled = mode;
 	pr_info("%s\n", spectre_v2_strings[mode]);
 
@@ -402,9 +411,16 @@ static void __init spectre_v2_select_mitigation(void)
 
 	/*
 	 * Retpoline means the kernel is safe because it has no indirect
-	 * branches. But firmware isn't, so use IBRS to protect that.
+	 * branches. Enhanced IBRS protects firmware too, so, enable restricted
+	 * speculation around firmware calls only when Enhanced IBRS isn't
+	 * supported.
+	 *
+	 * Use "mode" to check Enhanced IBRS instead of boot_cpu_has(), because
+	 * the user might select retpoline on the kernel command line and if
+	 * the CPU supports Enhanced IBRS, kernel might un-intentionally not
+	 * enable IBRS around firmware calls.
 	 */
-	if (boot_cpu_has(X86_FEATURE_IBRS)) {
+	if (boot_cpu_has(X86_FEATURE_IBRS) && mode != SPECTRE_V2_IBRS_ENHANCED) {
 		setup_force_cpu_cap(X86_FEATURE_USE_IBRS_FW);
 		pr_info("Enabling Restricted Speculation for firmware calls\n");
 	}
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 1ee8ea36af30..79561bfcfa87 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1015,6 +1015,9 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
 	   !cpu_has(c, X86_FEATURE_AMD_SSB_NO))
 		setup_force_cpu_bug(X86_BUG_SPEC_STORE_BYPASS);
 
+	if (ia32_cap & ARCH_CAP_IBRS_ALL)
+		setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
+
 	if (x86_match_cpu(cpu_no_meltdown))
 		return;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* RE: [PATCH] x86/speculation: Support Enhanced IBRS on future CPUs
  2018-07-30 17:07       ` Prakhya, Sai Praneeth
@ 2018-07-30 17:11         ` Thomas Gleixner
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Gleixner @ 2018-07-30 17:11 UTC (permalink / raw)
  To: Prakhya, Sai Praneeth
  Cc: Hansen, Dave, x86, linux-kernel, Chen, Tim C, Shankar, Ravi V,
	Ingo Molnar

On Mon, 30 Jul 2018, Prakhya, Sai Praneeth wrote:
> > > >> From: Sai Praneeth <sai.praneeth.prakhya@intel.com> Some future
> > > >> Intel processors may support "Enhanced IBRS" which is an "always
> > > >> on" mode i.e. IBRS bit in SPEC_CTRL MSR is enabled once and never
> > > >> disabled. According to specification[1], this should simplify
> > > >> software enabling and improve performance.
> > > > SHOULD is not really helpful. The question is whether it does
> > > > improve performance in practice or not. You really want to add
> > > > numbers comparing retpoutine and enhanced IBRS.
> > >
> > > One thing to remember from Intel's retpoline paper:
> > >
> > > > Retpoline is known to be an effective branch target injection
> > > > (Spectre variant 2) mitigation on Intel processors belonging to
> > > > family 6 (enumerated by the CPUID instruction) that do not have
> > > > support for enhanced IBRS. On processors that support enhanced IBRS,
> > > > it should be used for mitigation instead of retpoline.
> > >
> > > That's both a statement of "Intel would like you to use enhanced IBRS
> > > over retpoline where available" and "retpoline provides less
> > > mitigation on processors with enhanced IBRS compared to those without".
> > >
> > > In other words, we can _do_ performance deltas, but they won't be as
> > > meaningful because they won't really have apples-to-apples mitigation
> > > properties.
> > 
> > Fair enough, but this wants to be spelled out in the change log explicitely instead
> > of unspecific blurbs.
> 
> Sure! Makes sense. Would you like me to send a V2 with updated change log?

Of course.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] x86/speculation: Support Enhanced IBRS on future CPUs
  2018-07-30 16:58     ` Thomas Gleixner
@ 2018-07-30 17:07       ` Prakhya, Sai Praneeth
  2018-07-30 17:11         ` Thomas Gleixner
  0 siblings, 1 reply; 11+ messages in thread
From: Prakhya, Sai Praneeth @ 2018-07-30 17:07 UTC (permalink / raw)
  To: Thomas Gleixner, Hansen, Dave
  Cc: x86, linux-kernel, Chen, Tim C, Shankar, Ravi V, Ingo Molnar

> > >> From: Sai Praneeth <sai.praneeth.prakhya@intel.com> Some future
> > >> Intel processors may support "Enhanced IBRS" which is an "always
> > >> on" mode i.e. IBRS bit in SPEC_CTRL MSR is enabled once and never
> > >> disabled. According to specification[1], this should simplify
> > >> software enabling and improve performance.
> > > SHOULD is not really helpful. The question is whether it does
> > > improve performance in practice or not. You really want to add
> > > numbers comparing retpoutine and enhanced IBRS.
> >
> > One thing to remember from Intel's retpoline paper:
> >
> > > Retpoline is known to be an effective branch target injection
> > > (Spectre variant 2) mitigation on Intel processors belonging to
> > > family 6 (enumerated by the CPUID instruction) that do not have
> > > support for enhanced IBRS. On processors that support enhanced IBRS,
> > > it should be used for mitigation instead of retpoline.
> >
> > That's both a statement of "Intel would like you to use enhanced IBRS
> > over retpoline where available" and "retpoline provides less
> > mitigation on processors with enhanced IBRS compared to those without".
> >
> > In other words, we can _do_ performance deltas, but they won't be as
> > meaningful because they won't really have apples-to-apples mitigation
> > properties.
> 
> Fair enough, but this wants to be spelled out in the change log explicitely instead
> of unspecific blurbs.

Sure! Makes sense. Would you like me to send a V2 with updated change log?

Regards,
Sai

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] x86/speculation: Support Enhanced IBRS on future CPUs
  2018-07-30 14:29   ` Dave Hansen
@ 2018-07-30 16:58     ` Thomas Gleixner
  2018-07-30 17:07       ` Prakhya, Sai Praneeth
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Gleixner @ 2018-07-30 16:58 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Sai Praneeth Prakhya, x86, linux-kernel, Tim C Chen,
	Ravi Shankar, Ingo Molnar

On Mon, 30 Jul 2018, Dave Hansen wrote:
> On 07/30/2018 05:25 AM, Thomas Gleixner wrote:
> > On Tue, 24 Jul 2018, Sai Praneeth Prakhya wrote:
> >> From: Sai Praneeth <sai.praneeth.prakhya@intel.com>
> >> Some future Intel processors may support "Enhanced IBRS" which is an
> >> "always on" mode i.e. IBRS bit in SPEC_CTRL MSR is enabled once and
> >> never disabled. According to specification[1], this should simplify
> >> software enabling and improve performance.
> > SHOULD is not really helpful. The question is whether it does improve
> > performance in practice or not. You really want to add numbers comparing
> > retpoutine and enhanced IBRS.
> 
> One thing to remember from Intel's retpoline paper:
> 
> > Retpoline is known to be an effective branch target injection
> > (Spectre variant 2) mitigation on Intel processors belonging to
> > family 6 (enumerated by the CPUID instruction) that do not have
> > support for enhanced IBRS. On processors that support enhanced IBRS,
> > it should be used for mitigation instead of retpoline.
> 
> That's both a statement of "Intel would like you to use enhanced IBRS
> over retpoline where available" and "retpoline provides less mitigation
> on processors with enhanced IBRS compared to those without".
> 
> In other words, we can _do_ performance deltas, but they won't be as
> meaningful because they won't really have apples-to-apples mitigation
> properties.

Fair enough, but this wants to be spelled out in the change log explicitely
instead of unspecific blurbs.

Thanks,

	tglx


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] x86/speculation: Support Enhanced IBRS on future CPUs
  2018-07-30 12:25 ` Thomas Gleixner
@ 2018-07-30 14:29   ` Dave Hansen
  2018-07-30 16:58     ` Thomas Gleixner
  0 siblings, 1 reply; 11+ messages in thread
From: Dave Hansen @ 2018-07-30 14:29 UTC (permalink / raw)
  To: Thomas Gleixner, Sai Praneeth Prakhya
  Cc: x86, linux-kernel, Tim C Chen, Ravi Shankar, Ingo Molnar

On 07/30/2018 05:25 AM, Thomas Gleixner wrote:
> On Tue, 24 Jul 2018, Sai Praneeth Prakhya wrote:
>> From: Sai Praneeth <sai.praneeth.prakhya@intel.com>
>> Some future Intel processors may support "Enhanced IBRS" which is an
>> "always on" mode i.e. IBRS bit in SPEC_CTRL MSR is enabled once and
>> never disabled. According to specification[1], this should simplify
>> software enabling and improve performance.
> SHOULD is not really helpful. The question is whether it does improve
> performance in practice or not. You really want to add numbers comparing
> retpoutine and enhanced IBRS.

One thing to remember from Intel's retpoline paper:

> Retpoline is known to be an effective branch target injection
> (Spectre variant 2) mitigation on Intel processors belonging to
> family 6 (enumerated by the CPUID instruction) that do not have
> support for enhanced IBRS. On processors that support enhanced IBRS,
> it should be used for mitigation instead of retpoline.

That's both a statement of "Intel would like you to use enhanced IBRS
over retpoline where available" and "retpoline provides less mitigation
on processors with enhanced IBRS compared to those without".

In other words, we can _do_ performance deltas, but they won't be as
meaningful because they won't really have apples-to-apples mitigation
properties.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] x86/speculation: Support Enhanced IBRS on future CPUs
  2018-07-24 20:53 Sai Praneeth Prakhya
@ 2018-07-30 12:25 ` Thomas Gleixner
  2018-07-30 14:29   ` Dave Hansen
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Gleixner @ 2018-07-30 12:25 UTC (permalink / raw)
  To: Sai Praneeth Prakhya
  Cc: x86, linux-kernel, Tim C Chen, Dave Hansen, Ravi Shankar, Ingo Molnar

On Tue, 24 Jul 2018, Sai Praneeth Prakhya wrote:

> From: Sai Praneeth <sai.praneeth.prakhya@intel.com>
> 
> Some future Intel processors may support "Enhanced IBRS" which is an
> "always on" mode i.e. IBRS bit in SPEC_CTRL MSR is enabled once and
> never disabled. According to specification[1], this should simplify
> software enabling and improve performance.

SHOULD is not really helpful. The question is whether it does improve
performance in practice or not. You really want to add numbers comparing
retpoutine and enhanced IBRS.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] x86/speculation: Support Enhanced IBRS on future CPUs
@ 2018-07-24 20:53 Sai Praneeth Prakhya
  2018-07-30 12:25 ` Thomas Gleixner
  0 siblings, 1 reply; 11+ messages in thread
From: Sai Praneeth Prakhya @ 2018-07-24 20:53 UTC (permalink / raw)
  To: x86, linux-kernel
  Cc: Sai Praneeth, Tim C Chen, Dave Hansen, Thomas Gleixner,
	Ravi Shankar, Ingo Molnar

From: Sai Praneeth <sai.praneeth.prakhya@intel.com>

Some future Intel processors may support "Enhanced IBRS" which is an
"always on" mode i.e. IBRS bit in SPEC_CTRL MSR is enabled once and
never disabled. According to specification[1], this should simplify
software enabling and improve performance.

[With enhanced IBRS, the predicted targets of indirect branches executed
cannot be controlled by software that was executed in a less privileged
predictor mode or on another logical processor. As a result, software
operating on a processor with enhanced IBRS need not use WRMSR to set
IA32_SPEC_CTRL.IBRS after every transition to a more privileged
predictor mode. Software can isolate predictor modes effectively simply
by setting the bit once. Software need not disable enhanced IBRS prior
to entering a sleep state such as MWAIT or HLT.] - Specification

Even with enhanced IBRS, we still need to make sure that IBRS bit in
SPEC_CTRL MSR is always set i.e. while booting, if we detect support for
Enhanced IBRS, we enable IBRS bit in SPEC_CTRL MSR and we should also
make sure that it remains set always. In other words, if the guest has
cleared IBRS bit, upon VMEXIT the bit should still be set.

Fortunately, kernel already has the infrastructure ready. kvm/vmx.c does
x86_spec_ctrl_set_guest() before entering guest and
x86_spec_ctrl_restore_host() after leaving guest. So, the guest view of
SPEC_CTRL MSR is restored before entering guest and the host view of
SPEC_CTRL MSR is restored before entering host and hence IBRS will be
set after VMEXIT.

For Intel CPUs that support Enhanced IBRS, this patch also makes
Enhanced IBRS as the default Spectre V2 mitigation technique instead of
retpoline. Also, we still need IBPB even with enhanced IBRS.

[1] https://software.intel.com/sites/default/files/managed/c5/63/336996-Speculative-Execution-Side-Channel-Mitigations.pdf

Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Originally-by: David Woodhouse <dwmw@amazon.co.uk>
Cc: Tim C Chen <tim.c.chen@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ravi Shankar <ravi.v.shankar@intel.com>
Cc: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/cpufeatures.h   |  1 +
 arch/x86/include/asm/nospec-branch.h |  2 +-
 arch/x86/kernel/cpu/bugs.c           | 29 +++++++++++++++++++++++++++--
 arch/x86/kernel/cpu/common.c         |  3 +++
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 5701f5cecd31..f75815b1dbee 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -219,6 +219,7 @@
 #define X86_FEATURE_IBPB		( 7*32+26) /* Indirect Branch Prediction Barrier */
 #define X86_FEATURE_STIBP		( 7*32+27) /* Single Thread Indirect Branch Predictors */
 #define X86_FEATURE_ZEN			( 7*32+28) /* "" CPU is AMD family 0x17 (Zen) */
+#define X86_FEATURE_IBRS_ENHANCED		( 7*32+29) /* "ibrs_enhanced" Use Enhanced IBRS in kernel */
 
 /* Virtualization flags: Linux defined, word 8 */
 #define X86_FEATURE_TPR_SHADOW		( 8*32+ 0) /* Intel TPR Shadow */
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index f6f6c63da62f..fd2a8c1b88bc 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -214,7 +214,7 @@ enum spectre_v2_mitigation {
 	SPECTRE_V2_RETPOLINE_MINIMAL_AMD,
 	SPECTRE_V2_RETPOLINE_GENERIC,
 	SPECTRE_V2_RETPOLINE_AMD,
-	SPECTRE_V2_IBRS,
+	SPECTRE_V2_IBRS_ENHANCED,
 };
 
 /* The Speculative Store Bypass disable variants */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 5c0ea39311fe..a66517de1301 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -130,6 +130,7 @@ static const char *spectre_v2_strings[] = {
 	[SPECTRE_V2_RETPOLINE_MINIMAL_AMD]	= "Vulnerable: Minimal AMD ASM retpoline",
 	[SPECTRE_V2_RETPOLINE_GENERIC]		= "Mitigation: Full generic retpoline",
 	[SPECTRE_V2_RETPOLINE_AMD]		= "Mitigation: Full AMD retpoline",
+	[SPECTRE_V2_IBRS_ENHANCED]		= "Mitigation: Enhanced IBRS",
 };
 
 #undef pr_fmt
@@ -349,6 +350,8 @@ static void __init spectre_v2_select_mitigation(void)
 
 	case SPECTRE_V2_CMD_FORCE:
 	case SPECTRE_V2_CMD_AUTO:
+		if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED))
+			goto skip_retpoline_enable_ibrs;
 		if (IS_ENABLED(CONFIG_RETPOLINE))
 			goto retpoline_auto;
 		break;
@@ -385,7 +388,22 @@ static void __init spectre_v2_select_mitigation(void)
 					 SPECTRE_V2_RETPOLINE_MINIMAL;
 		setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
 	}
+	goto enable_other_mitigations;
 
+skip_retpoline_enable_ibrs:
+	mode = SPECTRE_V2_IBRS_ENHANCED;
+
+	/*
+	 * As we don't use IBRS in kernel, nobody should have set
+	 * SPEC_CTRL_IBRS until now. Shout loud if somebody did enable
+	 * SPEC_CTRL_IBRS before us.
+	 */
+	WARN_ON_ONCE(x86_spec_ctrl_base & SPEC_CTRL_IBRS);
+
+	/* Ensure SPEC_CTRL_IBRS is set after VMEXIT from a guest */
+	x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
+
+enable_other_mitigations:
 	spectre_v2_enabled = mode;
 	pr_info("%s\n", spectre_v2_strings[mode]);
 
@@ -415,9 +433,16 @@ static void __init spectre_v2_select_mitigation(void)
 
 	/*
 	 * Retpoline means the kernel is safe because it has no indirect
-	 * branches. But firmware isn't, so use IBRS to protect that.
+	 * branches. Enhanced IBRS protects firmware too, so, enable restricted
+	 * speculation around firmware calls only when Enhanced IBRS isn't
+	 * supported.
+	 *
+	 * Use "mode" to check Enhanced IBRS instead of boot_cpu_has(), because
+	 * user might select retpoline on command line and if CPU supports
+	 * Enhanced IBRS, we might un-intentionally not enable IBRS around
+	 * firmware calls.
 	 */
-	if (boot_cpu_has(X86_FEATURE_IBRS)) {
+	if (boot_cpu_has(X86_FEATURE_IBRS) && mode != SPECTRE_V2_IBRS_ENHANCED) {
 		setup_force_cpu_cap(X86_FEATURE_USE_IBRS_FW);
 		pr_info("Enabling Restricted Speculation for firmware calls\n");
 	}
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index eb4cb3efd20e..8ed73a46511f 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1005,6 +1005,9 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)
 	   !cpu_has(c, X86_FEATURE_AMD_SSB_NO))
 		setup_force_cpu_bug(X86_BUG_SPEC_STORE_BYPASS);
 
+	if (ia32_cap & ARCH_CAP_IBRS_ALL)
+		setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
+
 	if (x86_match_cpu(cpu_no_meltdown))
 		return;
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-11-08  3:37 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-07  0:42 [PATCH] x86/speculation: Support Enhanced IBRS on future CPUs Sai Praneeth Prakhya
2018-11-07  9:25 ` Greg KH
2018-11-07 18:05   ` Prakhya, Sai Praneeth
  -- strict thread matches above, loose matches on Subject: below --
2018-11-07  0:26 Sai Praneeth Prakhya
2018-11-07  9:26 ` Greg KH
2018-07-24 20:53 Sai Praneeth Prakhya
2018-07-30 12:25 ` Thomas Gleixner
2018-07-30 14:29   ` Dave Hansen
2018-07-30 16:58     ` Thomas Gleixner
2018-07-30 17:07       ` Prakhya, Sai Praneeth
2018-07-30 17:11         ` Thomas Gleixner

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.