linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] IBRS patch series
@ 2018-01-10  2:26 Tim Chen
  2018-01-10  2:26 ` [PATCH v3 1/5] x86/feature: Detect the x86 IBRS feature to control Speculation Tim Chen
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Tim Chen @ 2018-01-10  2:26 UTC (permalink / raw)
  To: Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Greg KH
  Cc: Tim Chen, Dave Hansen, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, David Woodhouse, Peter Zijlstra, Dan Williams,
	Paolo Bonzini, Ashok Raj, linux-kernel

Thanks to all the reviewers.  One key feedback I
got was we should make this patch series simple, so we can
put in basic IBRS functionality first.  So I took out
the run time control of IBRS, toggling IBRS to firmware call for retpoline
and recheck of IBRS feature on microcode reload. We will defer dealing
with those complications later.

This patch series is integrated with the retpoline patches
on x86/tip. The user can opt for ibrs by "spectre_v2=ibrs"
instead of "spectre_v2=retpoline" in
boot parameter.  Otherwise retpoline will be used by default
for spectre_v2.

The patchset is applied on top of the latest x86/tip with retpoline patches.

I've tested the patchset mostly on the 4.15-rc6.
As I have just merged the patchset top x86/tip, some bare testing
has been done on the x86/tip. Will be doing more testing there.

Thomas,
I have to switch a check in patch 5 from lockdep_assert_irqs_disabled
to WARN_ON_ONCE as it is not available on x86/tip. We should use
lockdep_assert_irqs_disabled when we merge back to mainline.

+       /* should use lockdep_assert_irqs_disabled() when available */
+       WARN_ON_ONCE(!irqs_disabled());

Thanks.
Tim

v3.
1. Use boot parameter spectre_v2=ibrs to opt in for enabling IBRS.
2. Remove run time control of IBRS usage.
3. Remove the patches for IBRS detection on microcode reload,
enabling of IBRS for firmware call when using retpoline.

v2.
1. Added missing feature enumeration in tools/arch/x86/include/asm/cpufeatures.h  
2. Kernel entry macros label cleanup and move them to calling.h
3. Remove unnecessary irqs_diabled check in the mwait.
4. Don't use a bit field base sys control variable to make ibrs enabling
   simpler and easier to understand.
5. Corrected compile issues for firmware update code.
6. Leave IBPB feature bits out from this patch series and will be added
   in its own set of patches later.

Tim

---patch series details---
This patch series enables the basic detection and usage of x86 indirect
branch speculation feature.  It enables the indirect branch restricted
speculation (IBRS) on kernel entry and disables it on exit.
It enumerates the indirect branch prediction barrier (IBPB).

The x86 IBRS feature requires corresponding microcode support.
It mitigates the variant 2 vulnerability described in
https://googleprojectzero.blogspot.com/2018/01/reading-privileged-memory-with-side.html

If IBRS is set, near returns and near indirect jumps/calls will not
allow their predicted target address to be controlled by code that
executed in a less privileged prediction mode before the IBRS mode was
last written with a value of 1 or on another logical processor so long
as all RSB entries from the previous less privileged prediction mode
are overwritten.

Both retpoline and IBRS provides mitigation against variant 2 attacks,
with IBRS being the most secured method but could incur more performance
overhead compared to retpoline[1].  If you are paranoid, then set
spectre_v2=ibrs as your boot parameter.

See: https://docs.google.com/document/d/e/2PACX-1vSMrwkaoSUBAFc6Fjd19F18c1O9pudkfAY-7lGYGOTN8mc9ul-J6pWadcAaBJZcVA7W_3jlLKRtKRbd/pub

More detailed description of IBRS is described in the first patch.

The patchset is applied on top of the latest x86/tip with retpoline patches.

Tim Chen (5):
  x86/feature: Detect the x86 IBRS feature to control Speculation
  x86/enter: Create macros to set/clear IBRS
  x86/enter: Use IBRS on syscall and interrupts
  x86/ibrs: Create boot option for IBRS
  x86/idle: Disable IBRS entering idle and enable it on wakeup

 Documentation/admin-guide/kernel-parameters.txt |  3 +
 arch/x86/entry/calling.h                        | 73 +++++++++++++++++++++++++
 arch/x86/entry/entry_64.S                       | 23 ++++++++
 arch/x86/entry/entry_64_compat.S                | 14 ++++-
 arch/x86/include/asm/cpufeatures.h              |  2 +
 arch/x86/include/asm/msr-index.h                |  4 ++
 arch/x86/include/asm/mwait.h                    | 13 +++++
 arch/x86/include/asm/spec_ctrl.h                | 22 ++++++++
 arch/x86/kernel/cpu/Makefile                    |  1 +
 arch/x86/kernel/cpu/scattered.c                 |  3 +
 arch/x86/kernel/cpu/spec_ctrl.c                 | 48 ++++++++++++++++
 arch/x86/kernel/process.c                       |  9 ++-
 tools/arch/x86/include/asm/cpufeatures.h        |  2 +
 13 files changed, 214 insertions(+), 3 deletions(-)
 create mode 100644 arch/x86/include/asm/spec_ctrl.h
 create mode 100644 arch/x86/kernel/cpu/spec_ctrl.c

-- 
2.9.4

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

* [PATCH v3 1/5] x86/feature: Detect the x86 IBRS feature to control Speculation
  2018-01-10  2:26 [PATCH v3 0/5] IBRS patch series Tim Chen
@ 2018-01-10  2:26 ` Tim Chen
  2018-01-10  2:26 ` [PATCH v3 2/5] x86/enter: Create macros to set/clear IBRS Tim Chen
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Tim Chen @ 2018-01-10  2:26 UTC (permalink / raw)
  To: Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Greg KH
  Cc: Tim Chen, Dave Hansen, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, David Woodhouse, Peter Zijlstra, Dan Williams,
	Paolo Bonzini, Ashok Raj, linux-kernel

cpuid ax=0x7, return rdx bit 26 to indicate presence of this feature

IA32_SPEC_CTRL (MSR 0x48)
IA32_SPEC_CTRL, bit0 – Indirect Branch Restricted Speculation (IBRS)

If IBRS is set, near returns and near indirect jumps/calls will not allow
their predicted target address to be controlled by code that executed in
a less privileged prediction mode before the IBRS mode was last written
with a value of 1 or on another logical processor so long as all return
stack buffer (RSB) entries from the previous less privileged prediction
mode are overwritten.

* Thus a near indirect jump/call/return may be affected by code in a
less privileged prediction mode that executed AFTER IBRS mode was last
written with a value of 1.

* Note: IBRS is not required in order to isolate branch predictions for
SMM or SGX enclaves.

* Code executed by a sibling logical processor cannot control indirect
jump/call/return predicted target when IBRS is set.

* SMEP will prevent supervisor mode using RSB entries filled by user code;
this can reduce the need for software to overwrite RSB entries.

CPU performance could be reduced when running with IBRS set.

Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
---
 arch/x86/include/asm/cpufeatures.h       | 2 ++
 arch/x86/include/asm/msr-index.h         | 4 ++++
 arch/x86/kernel/cpu/scattered.c          | 1 +
 tools/arch/x86/include/asm/cpufeatures.h | 2 ++
 4 files changed, 9 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index f275447..624b58e 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -211,6 +211,8 @@
 #define X86_FEATURE_AVX512_4FMAPS	( 7*32+17) /* AVX-512 Multiply Accumulation Single precision */
 
 #define X86_FEATURE_MBA			( 7*32+18) /* Memory Bandwidth Allocation */
+#define X86_FEATURE_SPEC_CTRL		( 7*32+19) /* Speculation Control */
+#define X86_FEATURE_SPEC_CTRL_IBRS	( 7*32+20) /* Speculation Control, use 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/msr-index.h b/arch/x86/include/asm/msr-index.h
index fa11fb1..3e1cb18 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -42,6 +42,10 @@
 #define MSR_PPIN_CTL			0x0000004e
 #define MSR_PPIN			0x0000004f
 
+#define MSR_IA32_SPEC_CTRL		0x00000048
+#define SPEC_CTRL_DISABLE_IBRS		(0 << 0)
+#define SPEC_CTRL_ENABLE_IBRS		(1 << 0)
+
 #define MSR_IA32_PERFCTR0		0x000000c1
 #define MSR_IA32_PERFCTR1		0x000000c2
 #define MSR_FSB_FREQ			0x000000cd
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 05459ad..bc50c40 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -24,6 +24,7 @@ static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_INTEL_PT,		CPUID_EBX, 25, 0x00000007, 0 },
 	{ X86_FEATURE_AVX512_4VNNIW,    CPUID_EDX,  2, 0x00000007, 0 },
 	{ X86_FEATURE_AVX512_4FMAPS,    CPUID_EDX,  3, 0x00000007, 0 },
+	{ X86_FEATURE_SPEC_CTRL,	CPUID_EDX, 26, 0x00000007, 0 },
 	{ X86_FEATURE_CAT_L3,		CPUID_EBX,  1, 0x00000010, 0 },
 	{ X86_FEATURE_CAT_L2,		CPUID_EBX,  2, 0x00000010, 0 },
 	{ X86_FEATURE_CDP_L3,		CPUID_ECX,  2, 0x00000010, 1 },
diff --git a/tools/arch/x86/include/asm/cpufeatures.h b/tools/arch/x86/include/asm/cpufeatures.h
index 793690f..995d74b 100644
--- a/tools/arch/x86/include/asm/cpufeatures.h
+++ b/tools/arch/x86/include/asm/cpufeatures.h
@@ -205,6 +205,8 @@
 #define X86_FEATURE_AVX512_4FMAPS (7*32+17) /* AVX-512 Multiply Accumulation Single precision */
 
 #define X86_FEATURE_MBA         ( 7*32+18) /* Memory Bandwidth Allocation */
+#define X86_FEATURE_SPEC_CTRL		( 7*32+19) /* Speculation Control */
+#define X86_FEATURE_SPEC_CTRL_IBRS	( 7*32+20) /* Speculation Control, use IBRS */
 
 /* Virtualization flags: Linux defined, word 8 */
 #define X86_FEATURE_TPR_SHADOW  ( 8*32+ 0) /* Intel TPR Shadow */
-- 
2.9.4

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

* [PATCH v3 2/5] x86/enter: Create macros to set/clear IBRS
  2018-01-10  2:26 [PATCH v3 0/5] IBRS patch series Tim Chen
  2018-01-10  2:26 ` [PATCH v3 1/5] x86/feature: Detect the x86 IBRS feature to control Speculation Tim Chen
@ 2018-01-10  2:26 ` Tim Chen
  2018-01-11 16:04   ` Thomas Gleixner
  2018-01-10  2:26 ` [PATCH v3 3/5] x86/enter: Use IBRS on syscall and interrupts Tim Chen
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Tim Chen @ 2018-01-10  2:26 UTC (permalink / raw)
  To: Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Greg KH
  Cc: Tim Chen, Dave Hansen, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, David Woodhouse, Peter Zijlstra, Dan Williams,
	Paolo Bonzini, Ashok Raj, linux-kernel

Create macros to control IBRS.  Use these macros to enable IBRS on kernel entry
paths and disable IBRS on kernel exit paths.

The registers rax, rcx and rdx are touched when controlling IBRS
so they need to be saved when they can't be clobbered.

Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
---
 arch/x86/entry/calling.h | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index 45a63e0..3b9b238 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -6,6 +6,8 @@
 #include <asm/percpu.h>
 #include <asm/asm-offsets.h>
 #include <asm/processor-flags.h>
+#include <asm/msr-index.h>
+#include <asm/cpufeatures.h>
 
 /*
 
@@ -347,3 +349,74 @@ For 32-bit we have the following conventions - kernel is built with
 .Lafter_call_\@:
 #endif
 .endm
+
+/*
+ * IBRS related macros
+ */
+
+.macro PUSH_MSR_REGS
+	pushq	%rax
+	pushq	%rcx
+	pushq	%rdx
+.endm
+
+.macro POP_MSR_REGS
+	popq	%rdx
+	popq	%rcx
+	popq	%rax
+.endm
+
+.macro WRMSR_ASM msr_nr:req edx_val:req eax_val:req
+	movl	\msr_nr, %ecx
+	movl	\edx_val, %edx
+	movl	\eax_val, %eax
+.endm
+
+.macro ENABLE_IBRS
+	ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL_IBRS
+	PUSH_MSR_REGS
+	WRMSR_ASM $MSR_IA32_SPEC_CTRL, $0, $SPEC_CTRL_ENABLE_IBRS
+	POP_MSR_REGS
+.Lskip_\@:
+.endm
+
+.macro DISABLE_IBRS
+	ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL_IBRS
+	PUSH_MSR_REGS
+	WRMSR_ASM $MSR_IA32_SPEC_CTRL, $0, $SPEC_CTRL_DISABLE_IBRS
+	POP_MSR_REGS
+.Lskip_\@:
+.endm
+
+.macro ENABLE_IBRS_CLOBBER
+	ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL_IBRS
+	WRMSR_ASM $MSR_IA32_SPEC_CTRL, $0, $SPEC_CTRL_ENABLE_IBRS
+.Lskip_\@:
+.endm
+
+.macro DISABLE_IBRS_CLOBBER
+	ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL_IBRS
+	WRMSR_ASM $MSR_IA32_SPEC_CTRL, $0, $SPEC_CTRL_DISABLE_IBRS
+.Lskip_\@:
+.endm
+
+.macro ENABLE_IBRS_SAVE_AND_CLOBBER save_reg:req
+	ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL_IBRS
+	movl	$MSR_IA32_SPEC_CTRL, %ecx
+	rdmsr
+	movl	%eax, \save_reg
+	movl	$0, %edx
+	movl	$SPEC_CTRL_ENABLE_IBRS, %eax
+	wrmsr
+.Lskip_\@:
+.endm
+
+.macro RESTORE_IBRS_CLOBBER save_reg:req
+	ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_SPEC_CTRL_IBRS
+	/* Set IBRS to the value saved in the save_reg */
+	movl    $MSR_IA32_SPEC_CTRL, %ecx
+	movl    $0, %edx
+	movl    \save_reg, %eax
+	wrmsr
+.Lskip_\@:
+.endm
-- 
2.9.4

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

* [PATCH v3 3/5] x86/enter: Use IBRS on syscall and interrupts
  2018-01-10  2:26 [PATCH v3 0/5] IBRS patch series Tim Chen
  2018-01-10  2:26 ` [PATCH v3 1/5] x86/feature: Detect the x86 IBRS feature to control Speculation Tim Chen
  2018-01-10  2:26 ` [PATCH v3 2/5] x86/enter: Create macros to set/clear IBRS Tim Chen
@ 2018-01-10  2:26 ` Tim Chen
  2018-01-10 10:04   ` Peter Zijlstra
  2018-01-11 12:45   ` Woodhouse, David
  2018-01-10  2:26 ` [PATCH v3 4/5] x86/ibrs: Create boot option for IBRS Tim Chen
  2018-01-10  2:26 ` [PATCH v3 5/5] x86/idle: Disable IBRS entering idle and enable it on wakeup Tim Chen
  4 siblings, 2 replies; 16+ messages in thread
From: Tim Chen @ 2018-01-10  2:26 UTC (permalink / raw)
  To: Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Greg KH
  Cc: Tim Chen, Dave Hansen, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, David Woodhouse, Peter Zijlstra, Dan Williams,
	Paolo Bonzini, Ashok Raj, linux-kernel

Set IBRS upon kernel entrance via syscall and interrupts. Clear it
upon exit.  IBRS protects against unsafe indirect branching predictions
in the kernel.

The NMI interrupt save/restore of IBRS state was based on Andrea
Arcangeli's implementation.
Here's an explanation by Dave Hansen on why we save IBRS state for NMI.

The normal interrupt code uses the 'error_entry' path which uses the
Code Segment (CS) of the instruction that was interrupted to tell
whether it interrupted the kernel or userspace and thus has to switch
IBRS, or leave it alone.

The NMI code is different.  It uses 'paranoid_entry' because it can
interrupt the kernel while it is running with a userspace IBRS (and %GS
and CR3) value, but has a kernel CS.  If we used the same approach as
the normal interrupt code, we might do the following;

	SYSENTER_entry
<-------------- NMI HERE
	IBRS=1
		do_something()
	IBRS=0
	SYSRET

The NMI code might notice that we are running in the kernel and decide
that it is OK to skip the IBRS=1.  This would leave it running
unprotected with IBRS=0, which is bad.

However, if we unconditionally set IBRS=1, in the NMI, we might get the
following case:

	SYSENTER_entry
	IBRS=1
		do_something()
	IBRS=0
<-------------- NMI HERE (set IBRS=1)
	SYSRET

and we would return to userspace with IBRS=1.  Userspace would run
slowly until we entered and exited the kernel again.

Instead of those two approaches, we chose a third one where we simply
save the IBRS value in a scratch register (%r13) and then restore that
value, verbatim.

Co-developed-by: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
---
 arch/x86/entry/entry_64.S        | 23 +++++++++++++++++++++++
 arch/x86/entry/entry_64_compat.S | 14 +++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 58dbf7a..0da9d16 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -171,6 +171,8 @@ ENTRY(entry_SYSCALL_64_trampoline)
 
 	/* Load the top of the task stack into RSP */
 	movq	CPU_ENTRY_AREA_tss + TSS_sp1 + CPU_ENTRY_AREA, %rsp
+	/* Stack is usable, use the non-clobbering IBRS enable: */
+	ENABLE_IBRS
 
 	/* Start building the simulated IRET frame. */
 	pushq	$__USER_DS			/* pt_regs->ss */
@@ -214,6 +216,8 @@ ENTRY(entry_SYSCALL_64)
 	 */
 	movq	%rsp, PER_CPU_VAR(rsp_scratch)
 	movq	PER_CPU_VAR(cpu_current_top_of_stack), %rsp
+	/* Stack is usable, use the non-clobbering IBRS enable: */
+	ENABLE_IBRS
 
 	TRACE_IRQS_OFF
 
@@ -413,6 +417,7 @@ syscall_return_via_sysret:
 	 * We are on the trampoline stack.  All regs except RDI are live.
 	 * We can do future final exit work right here.
 	 */
+	DISABLE_IBRS
 	SWITCH_TO_USER_CR3_STACK scratch_reg=%rdi
 
 	popq	%rdi
@@ -768,6 +773,7 @@ GLOBAL(swapgs_restore_regs_and_return_to_usermode)
 	 * We can do future final exit work right here.
 	 */
 
+	DISABLE_IBRS
 	SWITCH_TO_USER_CR3_STACK scratch_reg=%rdi
 
 	/* Restore RDI. */
@@ -855,6 +861,14 @@ native_irq_return_ldt:
 	SWAPGS					/* to kernel GS */
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi	/* to kernel CR3 */
 
+	/*
+	 * Normally we enable IBRS when we switch to kernel's CR3.
+	 * But we are going to switch back to user CR3 immediately
+	 * in this routine after fixing ESPFIX stack.  There is
+	 * no vulnerable code branching for IBRS to protect.
+	 * We don't toggle IBRS to avoid the cost of two MSR writes.
+	 */
+
 	movq	PER_CPU_VAR(espfix_waddr), %rdi
 	movq	%rax, (0*8)(%rdi)		/* user RAX */
 	movq	(1*8)(%rsp), %rax		/* user RIP */
@@ -988,6 +1002,8 @@ ENTRY(switch_to_thread_stack)
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
 	movq	%rsp, %rdi
 	movq	PER_CPU_VAR(cpu_current_top_of_stack), %rsp
+	/* Stack is usable, use the non-clobbering IBRS enable: */
+	ENABLE_IBRS
 	UNWIND_HINT sp_offset=16 sp_reg=ORC_REG_DI
 
 	pushq	7*8(%rdi)		/* regs->ss */
@@ -1288,6 +1304,7 @@ ENTRY(paranoid_entry)
 
 1:
 	SAVE_AND_SWITCH_TO_KERNEL_CR3 scratch_reg=%rax save_reg=%r14
+	ENABLE_IBRS_SAVE_AND_CLOBBER save_reg=%r13d
 
 	ret
 END(paranoid_entry)
@@ -1311,6 +1328,7 @@ ENTRY(paranoid_exit)
 	testl	%ebx, %ebx			/* swapgs needed? */
 	jnz	.Lparanoid_exit_no_swapgs
 	TRACE_IRQS_IRETQ
+	RESTORE_IBRS_CLOBBER save_reg=%r13d
 	RESTORE_CR3	scratch_reg=%rbx save_reg=%r14
 	SWAPGS_UNSAFE_STACK
 	jmp	.Lparanoid_exit_restore
@@ -1341,6 +1359,7 @@ ENTRY(error_entry)
 	SWAPGS
 	/* We have user CR3.  Change to kernel CR3. */
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
+	ENABLE_IBRS_CLOBBER
 
 .Lerror_entry_from_usermode_after_swapgs:
 	/* Put us onto the real thread stack. */
@@ -1388,6 +1407,7 @@ ENTRY(error_entry)
 	 */
 	SWAPGS
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
+	ENABLE_IBRS_CLOBBER
 	jmp .Lerror_entry_done
 
 .Lbstep_iret:
@@ -1402,6 +1422,7 @@ ENTRY(error_entry)
 	 */
 	SWAPGS
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
+	ENABLE_IBRS
 
 	/*
 	 * Pretend that the exception came from user mode: set up pt_regs
@@ -1503,6 +1524,7 @@ ENTRY(nmi)
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rdx
 	movq	%rsp, %rdx
 	movq	PER_CPU_VAR(cpu_current_top_of_stack), %rsp
+	ENABLE_IBRS
 	UNWIND_HINT_IRET_REGS base=%rdx offset=8
 	pushq	5*8(%rdx)	/* pt_regs->ss */
 	pushq	4*8(%rdx)	/* pt_regs->rsp */
@@ -1753,6 +1775,7 @@ end_repeat_nmi:
 	movq	$-1, %rsi
 	call	do_nmi
 
+	RESTORE_IBRS_CLOBBER save_reg=%r13d
 	RESTORE_CR3 scratch_reg=%r15 save_reg=%r14
 
 	testl	%ebx, %ebx			/* swapgs needed? */
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 98d5358..8aef23c 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -54,6 +54,7 @@ ENTRY(entry_SYSENTER_compat)
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rsp
 
 	movq	PER_CPU_VAR(cpu_current_top_of_stack), %rsp
+	ENABLE_IBRS
 
 	/*
 	 * User tracing code (ptrace or signal handlers) might assume that
@@ -225,7 +226,12 @@ GLOBAL(entry_SYSCALL_compat_after_hwframe)
 	pushq   $0			/* pt_regs->r15 = 0 */
 
 	/*
-	 * User mode is traced as though IRQs are on, and SYSENTER
+	 * We just saved %rdi so it is safe to clobber.  It is not
+	 * preserved during the C calls inside TRACE_IRQS_OFF anyway.
+	 */
+	ENABLE_IBRS_CLOBBER /* clobbers %rax, %rcx, %rdx */
+
+	/* User mode is traced as though IRQs are on, and SYSENTER
 	 * turned them off.
 	 */
 	TRACE_IRQS_OFF
@@ -239,6 +245,12 @@ GLOBAL(entry_SYSCALL_compat_after_hwframe)
 	/* Opportunistic SYSRET */
 sysret32_from_system_call:
 	TRACE_IRQS_ON			/* User mode traces as IRQs on. */
+	/*
+	 * Clobber of %rax, %rcx, %rdx is OK before register restoring.
+	 * This is safe to do here because we have no indirect branches
+	 * between here and the return to userspace (sysretl).
+	 */
+	DISABLE_IBRS_CLOBBER
 	movq	RBX(%rsp), %rbx		/* pt_regs->rbx */
 	movq	RBP(%rsp), %rbp		/* pt_regs->rbp */
 	movq	EFLAGS(%rsp), %r11	/* pt_regs->flags (in r11) */
-- 
2.9.4

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

* [PATCH v3 4/5] x86/ibrs: Create boot option for IBRS
  2018-01-10  2:26 [PATCH v3 0/5] IBRS patch series Tim Chen
                   ` (2 preceding siblings ...)
  2018-01-10  2:26 ` [PATCH v3 3/5] x86/enter: Use IBRS on syscall and interrupts Tim Chen
@ 2018-01-10  2:26 ` Tim Chen
  2018-01-10  2:26 ` [PATCH v3 5/5] x86/idle: Disable IBRS entering idle and enable it on wakeup Tim Chen
  4 siblings, 0 replies; 16+ messages in thread
From: Tim Chen @ 2018-01-10  2:26 UTC (permalink / raw)
  To: Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Greg KH
  Cc: Tim Chen, Dave Hansen, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, David Woodhouse, Peter Zijlstra, Dan Williams,
	Paolo Bonzini, Ashok Raj, linux-kernel

When "spectre_v2=ibrs" is set in kernel boot option, the kernel will
enable IBRS on entry to kernel and disable IBRS on kernel exit.

The X86_FEATURE_SPEC_CTRL_IBRS will be set on the cpu
if the user opt in to use IBRS against SPECTRE V2 attack.

It is only for cases where the user require maximum level
of security.

Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  3 +++
 arch/x86/include/asm/spec_ctrl.h                | 22 ++++++++++++++++++++++
 arch/x86/kernel/cpu/Makefile                    |  1 +
 arch/x86/kernel/cpu/scattered.c                 |  2 ++
 arch/x86/kernel/cpu/spec_ctrl.c                 | 23 +++++++++++++++++++++++
 5 files changed, 51 insertions(+)
 create mode 100644 arch/x86/include/asm/spec_ctrl.h
 create mode 100644 arch/x86/kernel/cpu/spec_ctrl.c

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 8122b5f..c611700 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3920,6 +3920,9 @@
 			off  - unconditionally disable
 			auto - kernel detects whether your CPU model is
 			       vulnerable
+			ibrs - Use indirect branch restricted speculation
+			(IBRS) to restrict speculation in kernel.  For
+			use cases requiring maximum security.
 
 			Selecting 'on' will, and 'auto' may, choose a
 			mitigation method at run time according to the
diff --git a/arch/x86/include/asm/spec_ctrl.h b/arch/x86/include/asm/spec_ctrl.h
new file mode 100644
index 0000000..948959b
--- /dev/null
+++ b/arch/x86/include/asm/spec_ctrl.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_X86_SPEC_CTRL_H
+#define _ASM_X86_SPEC_CTRL_H
+
+#include <asm/microcode.h>
+
+void spec_ctrl_scan_feature(struct cpuinfo_x86 *c);
+void spec_ctrl_unprotected_begin(void);
+void spec_ctrl_unprotected_end(void);
+
+static inline void __disable_indirect_speculation(void)
+{
+	native_wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_ENABLE_IBRS);
+}
+
+static inline void __enable_indirect_speculation(void)
+{
+	native_wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_DISABLE_IBRS);
+}
+
+#endif /* _ASM_X86_SPEC_CTRL_H */
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 90cb82d..a25f1ab 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -24,6 +24,7 @@ obj-y			+= match.o
 obj-y			+= bugs.o
 obj-$(CONFIG_CPU_FREQ)	+= aperfmperf.o
 obj-y			+= cpuid-deps.o
+obj-y			+= spec_ctrl.o
 
 obj-$(CONFIG_PROC_FS)	+= proc.o
 obj-$(CONFIG_X86_FEATURE_NAMES) += capflags.o powerflags.o
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index bc50c40..5b01487 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -8,6 +8,7 @@
 #include <asm/processor.h>
 
 #include <asm/apic.h>
+#include <asm/spec_ctrl.h>
 
 struct cpuid_bit {
 	u16 feature;
@@ -57,6 +58,7 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
 		if (regs[cb->reg] & (1 << cb->bit))
 			set_cpu_cap(c, cb->feature);
 	}
+	spec_ctrl_scan_feature(c);
 }
 
 u32 get_scattered_cpuid_leaf(unsigned int level, unsigned int sub_leaf,
diff --git a/arch/x86/kernel/cpu/spec_ctrl.c b/arch/x86/kernel/cpu/spec_ctrl.c
new file mode 100644
index 0000000..2f3d6ca
--- /dev/null
+++ b/arch/x86/kernel/cpu/spec_ctrl.c
@@ -0,0 +1,23 @@
+#include <linux/string.h>
+
+#include <asm/spec_ctrl.h>
+#include <asm/cpufeature.h>
+
+static bool ibrs_admin_enabled;
+
+void spec_ctrl_scan_feature(struct cpuinfo_x86 *c)
+{
+	if (boot_cpu_has(X86_FEATURE_SPEC_CTRL)) {
+		if (ibrs_admin_enabled)
+			set_cpu_cap(c, X86_FEATURE_SPEC_CTRL_IBRS);
+	}
+}
+
+static int __init check_ibrs_param(char *str)
+{
+	if (strcmp(str, "ibrs") == 0)
+		ibrs_admin_enabled = true;
+
+	return 0;
+}
+early_param("spectre_v2", check_ibrs_param);
-- 
2.9.4

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

* [PATCH v3 5/5] x86/idle: Disable IBRS entering idle and enable it on wakeup
  2018-01-10  2:26 [PATCH v3 0/5] IBRS patch series Tim Chen
                   ` (3 preceding siblings ...)
  2018-01-10  2:26 ` [PATCH v3 4/5] x86/ibrs: Create boot option for IBRS Tim Chen
@ 2018-01-10  2:26 ` Tim Chen
  4 siblings, 0 replies; 16+ messages in thread
From: Tim Chen @ 2018-01-10  2:26 UTC (permalink / raw)
  To: Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Greg KH
  Cc: Tim Chen, Dave Hansen, Andrea Arcangeli, Andi Kleen,
	Arjan Van De Ven, David Woodhouse, Peter Zijlstra, Dan Williams,
	Paolo Bonzini, Ashok Raj, linux-kernel

Clear IBRS on idle entry and set it on idle exit into kernel on mwait.
When we are in mwait, we are not running but if we leave IBRS on,
it will affect the performance on the sibling hardware thread.  So
we disable IBRS and reenable it when we wake up.

Thanks to Peter Zijlstra and Andrea Arcangeli's suggestion of using
static key to improve IBRS toggling.

Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
---
 arch/x86/include/asm/mwait.h    | 13 +++++++++++++
 arch/x86/kernel/cpu/spec_ctrl.c | 27 ++++++++++++++++++++++++++-
 arch/x86/kernel/process.c       |  9 +++++++--
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/mwait.h b/arch/x86/include/asm/mwait.h
index 39a2fb2..870299a 100644
--- a/arch/x86/include/asm/mwait.h
+++ b/arch/x86/include/asm/mwait.h
@@ -6,6 +6,7 @@
 #include <linux/sched/idle.h>
 
 #include <asm/cpufeature.h>
+#include <asm/spec_ctrl.h>
 
 #define MWAIT_SUBSTATE_MASK		0xf
 #define MWAIT_CSTATE_MASK		0xf
@@ -106,9 +107,21 @@ static inline void mwait_idle_with_hints(unsigned long eax, unsigned long ecx)
 			mb();
 		}
 
+	       /*
+		* CPUs run faster with speculation protection
+		* disabled.  All CPU threads in a core must
+		* disable speculation protection for it to be
+		* disabled.  Disable it while we are idle so the
+		* other hyperthread can run fast.
+		*
+		* Interrupts have been disabled at this point.
+		*/
+
+		spec_ctrl_unprotected_begin();
 		__monitor((void *)&current_thread_info()->flags, 0, 0);
 		if (!need_resched())
 			__mwait(eax, ecx);
+		spec_ctrl_unprotected_end();
 	}
 	current_clr_polling();
 }
diff --git a/arch/x86/kernel/cpu/spec_ctrl.c b/arch/x86/kernel/cpu/spec_ctrl.c
index 2f3d6ca..843b4e6 100644
--- a/arch/x86/kernel/cpu/spec_ctrl.c
+++ b/arch/x86/kernel/cpu/spec_ctrl.c
@@ -4,12 +4,16 @@
 #include <asm/cpufeature.h>
 
 static bool ibrs_admin_enabled;
+DEFINE_STATIC_KEY_FALSE(spec_ctrl_dynamic_ibrs);
 
 void spec_ctrl_scan_feature(struct cpuinfo_x86 *c)
 {
 	if (boot_cpu_has(X86_FEATURE_SPEC_CTRL)) {
-		if (ibrs_admin_enabled)
+		if (ibrs_admin_enabled) {
 			set_cpu_cap(c, X86_FEATURE_SPEC_CTRL_IBRS);
+			if (!c->cpu_index)
+				static_branch_enable(&spec_ctrl_dynamic_ibrs);
+		}
 	}
 }
 
@@ -21,3 +25,24 @@ static int __init check_ibrs_param(char *str)
 	return 0;
 }
 early_param("spectre_v2", check_ibrs_param);
+
+/*
+ * Interrupts must be disabled to begin unprotected speculation.
+ * Otherwise interrupts could come in and start running in unprotected mode.
+ */
+
+void spec_ctrl_unprotected_begin(void)
+{
+	/* should use lockdep_assert_irqs_disabled() when available */
+	WARN_ON_ONCE(!irqs_disabled());
+	if (static_branch_unlikely(&spec_ctrl_dynamic_ibrs))
+		__enable_indirect_speculation();
+}
+EXPORT_SYMBOL_GPL(spec_ctrl_unprotected_begin);
+
+void spec_ctrl_unprotected_end(void)
+{
+	if (static_branch_unlikely(&spec_ctrl_dynamic_ibrs))
+		__disable_indirect_speculation();
+}
+EXPORT_SYMBOL_GPL(spec_ctrl_unprotected_end);
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 3cb2486..87a1121 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -39,6 +39,7 @@
 #include <asm/switch_to.h>
 #include <asm/desc.h>
 #include <asm/prctl.h>
+#include <asm/spec_ctrl.h>
 
 /*
  * per-CPU TSS segments. Threads are completely 'soft' on Linux,
@@ -461,11 +462,15 @@ static __cpuidle void mwait_idle(void)
 			mb(); /* quirk */
 		}
 
+		spec_ctrl_unprotected_begin();
 		__monitor((void *)&current_thread_info()->flags, 0, 0);
-		if (!need_resched())
+		if (!need_resched()) {
 			__sti_mwait(0, 0);
-		else
+			spec_ctrl_unprotected_end();
+		} else {
+			spec_ctrl_unprotected_end();
 			local_irq_enable();
+		}
 		trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
 	} else {
 		local_irq_enable();
-- 
2.9.4

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

* Re: [PATCH v3 3/5] x86/enter: Use IBRS on syscall and interrupts
  2018-01-10  2:26 ` [PATCH v3 3/5] x86/enter: Use IBRS on syscall and interrupts Tim Chen
@ 2018-01-10 10:04   ` Peter Zijlstra
  2018-01-10 11:27     ` Paolo Bonzini
  2018-01-10 18:16     ` Tim Chen
  2018-01-11 12:45   ` Woodhouse, David
  1 sibling, 2 replies; 16+ messages in thread
From: Peter Zijlstra @ 2018-01-10 10:04 UTC (permalink / raw)
  To: Tim Chen
  Cc: Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Greg KH,
	Dave Hansen, Andrea Arcangeli, Andi Kleen, Arjan Van De Ven,
	David Woodhouse, Dan Williams, Paolo Bonzini, Ashok Raj,
	linux-kernel

On Tue, Jan 09, 2018 at 06:26:47PM -0800, Tim Chen wrote:
> Set IBRS upon kernel entrance via syscall and interrupts. Clear it
> upon exit.  IBRS protects against unsafe indirect branching predictions
> in the kernel.
> 
> The NMI interrupt save/restore of IBRS state was based on Andrea
> Arcangeli's implementation.
> Here's an explanation by Dave Hansen on why we save IBRS state for NMI.
> 
> The normal interrupt code uses the 'error_entry' path which uses the
> Code Segment (CS) of the instruction that was interrupted to tell
> whether it interrupted the kernel or userspace and thus has to switch
> IBRS, or leave it alone.
> 
> The NMI code is different.  It uses 'paranoid_entry' because it can
> interrupt the kernel while it is running with a userspace IBRS (and %GS
> and CR3) value, but has a kernel CS.  If we used the same approach as
> the normal interrupt code, we might do the following;
> 
> 	SYSENTER_entry
> <-------------- NMI HERE
> 	IBRS=1
> 		do_something()
> 	IBRS=0
> 	SYSRET
> 
> The NMI code might notice that we are running in the kernel and decide
> that it is OK to skip the IBRS=1.  This would leave it running
> unprotected with IBRS=0, which is bad.
> 
> However, if we unconditionally set IBRS=1, in the NMI, we might get the
> following case:
> 
> 	SYSENTER_entry
> 	IBRS=1
> 		do_something()
> 	IBRS=0
> <-------------- NMI HERE (set IBRS=1)
> 	SYSRET
> 
> and we would return to userspace with IBRS=1.  Userspace would run
> slowly until we entered and exited the kernel again.
> 
> Instead of those two approaches, we chose a third one where we simply
> save the IBRS value in a scratch register (%r13) and then restore that
> value, verbatim.
> 

What this Changelog fails to address is _WHY_ we need this. What does
this provide that retpoline does not.

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

* Re: [PATCH v3 3/5] x86/enter: Use IBRS on syscall and interrupts
  2018-01-10 10:04   ` Peter Zijlstra
@ 2018-01-10 11:27     ` Paolo Bonzini
  2018-01-10 18:16     ` Tim Chen
  1 sibling, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2018-01-10 11:27 UTC (permalink / raw)
  To: Peter Zijlstra, Tim Chen
  Cc: Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Greg KH,
	Dave Hansen, Andrea Arcangeli, Andi Kleen, Arjan Van De Ven,
	David Woodhouse, Dan Williams, Ashok Raj, linux-kernel

On 10/01/2018 11:04, Peter Zijlstra wrote:
> On Tue, Jan 09, 2018 at 06:26:47PM -0800, Tim Chen wrote:
>> Set IBRS upon kernel entrance via syscall and interrupts. Clear it
>> upon exit.  IBRS protects against unsafe indirect branching predictions
>> in the kernel.
>>
>> The NMI interrupt save/restore of IBRS state was based on Andrea
>> Arcangeli's implementation.
>> Here's an explanation by Dave Hansen on why we save IBRS state for NMI.
>>
>> The normal interrupt code uses the 'error_entry' path which uses the
>> Code Segment (CS) of the instruction that was interrupted to tell
>> whether it interrupted the kernel or userspace and thus has to switch
>> IBRS, or leave it alone.
>>
>> The NMI code is different.  It uses 'paranoid_entry' because it can
>> interrupt the kernel while it is running with a userspace IBRS (and %GS
>> and CR3) value, but has a kernel CS.  If we used the same approach as
>> the normal interrupt code, we might do the following;
>>
>> 	SYSENTER_entry
>> <-------------- NMI HERE
>> 	IBRS=1
>> 		do_something()
>> 	IBRS=0
>> 	SYSRET
>>
>> The NMI code might notice that we are running in the kernel and decide
>> that it is OK to skip the IBRS=1.  This would leave it running
>> unprotected with IBRS=0, which is bad.
>>
>> However, if we unconditionally set IBRS=1, in the NMI, we might get the
>> following case:
>>
>> 	SYSENTER_entry
>> 	IBRS=1
>> 		do_something()
>> 	IBRS=0
>> <-------------- NMI HERE (set IBRS=1)
>> 	SYSRET
>>
>> and we would return to userspace with IBRS=1.  Userspace would run
>> slowly until we entered and exited the kernel again.
>>
>> Instead of those two approaches, we chose a third one where we simply
>> save the IBRS value in a scratch register (%r13) and then restore that
>> value, verbatim.
>>
> 
> What this Changelog fails to address is _WHY_ we need this. What does
> this provide that retpoline does not.

Which, for the record, is just that it works better on Skylake+ CPUs.

Paolo

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

* Re: [PATCH v3 3/5] x86/enter: Use IBRS on syscall and interrupts
  2018-01-10 10:04   ` Peter Zijlstra
  2018-01-10 11:27     ` Paolo Bonzini
@ 2018-01-10 18:16     ` Tim Chen
  2018-01-10 18:28       ` Peter Zijlstra
  1 sibling, 1 reply; 16+ messages in thread
From: Tim Chen @ 2018-01-10 18:16 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Greg KH,
	Dave Hansen, Andrea Arcangeli, Andi Kleen, Arjan Van De Ven,
	David Woodhouse, Dan Williams, Paolo Bonzini, Ashok Raj,
	linux-kernel

On 01/10/2018 02:04 AM, Peter Zijlstra wrote:
> On Tue, Jan 09, 2018 at 06:26:47PM -0800, Tim Chen wrote:
>> Set IBRS upon kernel entrance via syscall and interrupts. Clear it
>> upon exit.  IBRS protects against unsafe indirect branching predictions
>> in the kernel.
>>
>> The NMI interrupt save/restore of IBRS state was based on Andrea
>> Arcangeli's implementation.
>> Here's an explanation by Dave Hansen on why we save IBRS state for NMI.
>>
>> The normal interrupt code uses the 'error_entry' path which uses the
>> Code Segment (CS) of the instruction that was interrupted to tell
>> whether it interrupted the kernel or userspace and thus has to switch
>> IBRS, or leave it alone.
>>
>> The NMI code is different.  It uses 'paranoid_entry' because it can
>> interrupt the kernel while it is running with a userspace IBRS (and %GS
>> and CR3) value, but has a kernel CS.  If we used the same approach as
>> the normal interrupt code, we might do the following;
>>
>> 	SYSENTER_entry
>> <-------------- NMI HERE
>> 	IBRS=1
>> 		do_something()
>> 	IBRS=0
>> 	SYSRET
>>
>> The NMI code might notice that we are running in the kernel and decide
>> that it is OK to skip the IBRS=1.  This would leave it running
>> unprotected with IBRS=0, which is bad.
>>
>> However, if we unconditionally set IBRS=1, in the NMI, we might get the
>> following case:
>>
>> 	SYSENTER_entry
>> 	IBRS=1
>> 		do_something()
>> 	IBRS=0
>> <-------------- NMI HERE (set IBRS=1)
>> 	SYSRET
>>
>> and we would return to userspace with IBRS=1.  Userspace would run
>> slowly until we entered and exited the kernel again.
>>
>> Instead of those two approaches, we chose a third one where we simply
>> save the IBRS value in a scratch register (%r13) and then restore that
>> value, verbatim.
>>
> 
> What this Changelog fails to address is _WHY_ we need this. What does
> this provide that retpoline does not.
> 

Ok. I mentioned that in the cover letter that IBRS is a maximum security
mode in the CPU itself to directly restrict all indirect branches to prevent SPECTRE v2.

I'll also include such comments in the commit log here.

Thanks.

Tim

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

* Re: [PATCH v3 3/5] x86/enter: Use IBRS on syscall and interrupts
  2018-01-10 18:16     ` Tim Chen
@ 2018-01-10 18:28       ` Peter Zijlstra
  2018-01-10 18:44         ` Tim Chen
  2018-01-10 18:47         ` Van De Ven, Arjan
  0 siblings, 2 replies; 16+ messages in thread
From: Peter Zijlstra @ 2018-01-10 18:28 UTC (permalink / raw)
  To: Tim Chen
  Cc: Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Greg KH,
	Dave Hansen, Andrea Arcangeli, Andi Kleen, Arjan Van De Ven,
	David Woodhouse, Dan Williams, Paolo Bonzini, Ashok Raj,
	linux-kernel

On Wed, Jan 10, 2018 at 10:16:20AM -0800, Tim Chen wrote:
> On 01/10/2018 02:04 AM, Peter Zijlstra wrote:

> > What this Changelog fails to address is _WHY_ we need this. What does
> > this provide that retpoline does not.
> > 
> 
> Ok. I mentioned that in the cover letter that IBRS is a maximum security
> mode in the CPU itself to directly restrict all indirect branches to prevent SPECTRE v2.
> 
> I'll also include such comments in the commit log here.

That still doesn't say anything useful. Why and where is it better than
retpoline? Why would I ever want to use IBRS? Those are not questions
that have clear answers here.

>From what I can gather of the discussion earlier today is that pre SKL
IBRS is no better than retpoline and a whole lot slower.

On SKL+ retpoline is mostly there, but has a few dinky holes in and it
_might_ make sense to use IBRS.

But I feel it needs explaining what the exact holes are (pjt and dwmw2
had a fair enumeration IIRC) such that people can judge the risk.

No wishy washy maybe nonsense, clear language.

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

* Re: [PATCH v3 3/5] x86/enter: Use IBRS on syscall and interrupts
  2018-01-10 18:28       ` Peter Zijlstra
@ 2018-01-10 18:44         ` Tim Chen
  2018-01-10 18:47         ` Van De Ven, Arjan
  1 sibling, 0 replies; 16+ messages in thread
From: Tim Chen @ 2018-01-10 18:44 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Greg KH,
	Dave Hansen, Andrea Arcangeli, Andi Kleen, Arjan Van De Ven,
	David Woodhouse, Dan Williams, Paolo Bonzini, Ashok Raj,
	linux-kernel

On 01/10/2018 10:28 AM, Peter Zijlstra wrote:
> On Wed, Jan 10, 2018 at 10:16:20AM -0800, Tim Chen wrote:
>> On 01/10/2018 02:04 AM, Peter Zijlstra wrote:
> 
>>> What this Changelog fails to address is _WHY_ we need this. What does
>>> this provide that retpoline does not.
>>>
>>
>> Ok. I mentioned that in the cover letter that IBRS is a maximum security
>> mode in the CPU itself to directly restrict all indirect branches to prevent SPECTRE v2.
>>
>> I'll also include such comments in the commit log here.
> 
> That still doesn't say anything useful. Why and where is it better than
> retpoline? Why would I ever want to use IBRS? Those are not questions
> that have clear answers here.
> 
> From what I can gather of the discussion earlier today is that pre SKL
> IBRS is no better than retpoline and a whole lot slower.
> 
> On SKL+ retpoline is mostly there, but has a few dinky holes in and it
> _might_ make sense to use IBRS.
> 
> But I feel it needs explaining what the exact holes are (pjt and dwmw2
> had a fair enumeration IIRC) such that people can judge the risk.
> 
> No wishy washy maybe nonsense, clear language.
> 

Retpoline depends on the compiler doing the right thing, finding all
instances of indirect jump/call and patching those places with
the retpoline construct to defend against spectre v2.

For IBRS, the cpu itself restricts all the indirect jumps/calls when
IBRS is set.  So this is inherently a more secure mode than the
retpoline approach.  It also helps people who don't have a gcc
that don't support retpoline.

Tim

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

* RE: [PATCH v3 3/5] x86/enter: Use IBRS on syscall and interrupts
  2018-01-10 18:28       ` Peter Zijlstra
  2018-01-10 18:44         ` Tim Chen
@ 2018-01-10 18:47         ` Van De Ven, Arjan
  1 sibling, 0 replies; 16+ messages in thread
From: Van De Ven, Arjan @ 2018-01-10 18:47 UTC (permalink / raw)
  To: Peter Zijlstra, Tim Chen
  Cc: Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Greg KH,
	Hansen, Dave, Andrea Arcangeli, Andi Kleen, David Woodhouse,
	Williams, Dan J, Paolo Bonzini, Raj, Ashok, linux-kernel

> On SKL+ retpoline is mostly there, but has a few dinky holes in and it
> _might_ make sense to use IBRS.
> 
> But I feel it needs explaining what the exact holes are (pjt and dwmw2
> had a fair enumeration IIRC) such that people can judge the risk.

you are correct to need and want this.

the problem is one of patch sequencing a bit;

as retpoline is now it has more of these dinky holes, in the next few days
patches will flow that will fix a bunch of these.

I would like to suggest we do the documentation part after a few days once 
we know how much of these we can reasonable pre-plug.

but yes we must do this tradeoff documentation.


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

* Re: [PATCH v3 3/5] x86/enter: Use IBRS on syscall and interrupts
  2018-01-10  2:26 ` [PATCH v3 3/5] x86/enter: Use IBRS on syscall and interrupts Tim Chen
  2018-01-10 10:04   ` Peter Zijlstra
@ 2018-01-11 12:45   ` Woodhouse, David
  1 sibling, 0 replies; 16+ messages in thread
From: Woodhouse, David @ 2018-01-11 12:45 UTC (permalink / raw)
  To: Tim Chen, Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Greg KH
  Cc: Dave Hansen, Andrea Arcangeli, Andi Kleen, Arjan Van De Ven,
	Peter Zijlstra, Dan Williams, Paolo Bonzini, Ashok Raj,
	linux-kernel

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

On Tue, 2018-01-09 at 18:26 -0800, Tim Chen wrote:
> Set IBRS upon kernel entrance via syscall and interrupts. Clear it
> upon exit. 

In the former set of sites, you're going to want to stuff the RSB too.
The patch I sent out this morning adds the infrastructure you want for
that; we'll just want to shift it from being dependent on RETPOLINE
(both CONFIG_RETPOLINE and X86_FEATURE_RETPOLINE).

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5210 bytes --]

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

* Re: [PATCH v3 2/5] x86/enter: Create macros to set/clear IBRS
  2018-01-10  2:26 ` [PATCH v3 2/5] x86/enter: Create macros to set/clear IBRS Tim Chen
@ 2018-01-11 16:04   ` Thomas Gleixner
  2018-01-11 21:05     ` Tim Chen
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Gleixner @ 2018-01-11 16:04 UTC (permalink / raw)
  To: Tim Chen
  Cc: Andy Lutomirski, Linus Torvalds, Greg KH, Dave Hansen,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, David Woodhouse,
	Peter Zijlstra, Dan Williams, Paolo Bonzini, Ashok Raj,
	linux-kernel

On Tue, 9 Jan 2018, Tim Chen wrote:
> +
> +.macro WRMSR_ASM msr_nr:req edx_val:req eax_val:req
> +	movl	\msr_nr, %ecx
> +	movl	\edx_val, %edx
> +	movl	\eax_val, %eax
> +.endm

This is the most brilliant piece of useless code I've seen in a long time.


	 tglx

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

* Re: [PATCH v3 2/5] x86/enter: Create macros to set/clear IBRS
  2018-01-11 16:04   ` Thomas Gleixner
@ 2018-01-11 21:05     ` Tim Chen
  2018-01-11 21:24       ` Tim Chen
  0 siblings, 1 reply; 16+ messages in thread
From: Tim Chen @ 2018-01-11 21:05 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Andy Lutomirski, Linus Torvalds, Greg KH, Dave Hansen,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, David Woodhouse,
	Peter Zijlstra, Dan Williams, Paolo Bonzini, Ashok Raj,
	linux-kernel

On 01/11/2018 08:04 AM, Thomas Gleixner wrote:
> On Tue, 9 Jan 2018, Tim Chen wrote:
>> +
>> +.macro WRMSR_ASM msr_nr:req edx_val:req eax_val:req
>> +	movl	\msr_nr, %ecx
>> +	movl	\edx_val, %edx
>> +	movl	\eax_val, %eax
>> +.endm
> 
> This is the most brilliant piece of useless code I've seen in a long time.
> 
> 
> 	 tglx
> 

Sorry that was a major brain fart on my part. 

Tim 

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

* Re: [PATCH v3 2/5] x86/enter: Create macros to set/clear IBRS
  2018-01-11 21:05     ` Tim Chen
@ 2018-01-11 21:24       ` Tim Chen
  0 siblings, 0 replies; 16+ messages in thread
From: Tim Chen @ 2018-01-11 21:24 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Andy Lutomirski, Linus Torvalds, Greg KH, Dave Hansen,
	Andrea Arcangeli, Andi Kleen, Arjan Van De Ven, David Woodhouse,
	Peter Zijlstra, Dan Williams, Paolo Bonzini, Ashok Raj,
	linux-kernel

On 01/11/2018 01:05 PM, Tim Chen wrote:
> On 01/11/2018 08:04 AM, Thomas Gleixner wrote:
>> On Tue, 9 Jan 2018, Tim Chen wrote:
>>> +
>>> +.macro WRMSR_ASM msr_nr:req edx_val:req eax_val:req
>>> +	movl	\msr_nr, %ecx
>>> +	movl	\edx_val, %edx
>>> +	movl	\eax_val, %eax
>>> +.endm
>>
>> This is the most brilliant piece of useless code I've seen in a long time.
>>
>>
>> 	 tglx
>>
> 
> Sorry that was a major brain fart on my part. 
> 

Our internal tester mixed in retpoline with IBRS so we've missed
this.  Will correct our test.

Tim

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

end of thread, other threads:[~2018-01-11 21:24 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-10  2:26 [PATCH v3 0/5] IBRS patch series Tim Chen
2018-01-10  2:26 ` [PATCH v3 1/5] x86/feature: Detect the x86 IBRS feature to control Speculation Tim Chen
2018-01-10  2:26 ` [PATCH v3 2/5] x86/enter: Create macros to set/clear IBRS Tim Chen
2018-01-11 16:04   ` Thomas Gleixner
2018-01-11 21:05     ` Tim Chen
2018-01-11 21:24       ` Tim Chen
2018-01-10  2:26 ` [PATCH v3 3/5] x86/enter: Use IBRS on syscall and interrupts Tim Chen
2018-01-10 10:04   ` Peter Zijlstra
2018-01-10 11:27     ` Paolo Bonzini
2018-01-10 18:16     ` Tim Chen
2018-01-10 18:28       ` Peter Zijlstra
2018-01-10 18:44         ` Tim Chen
2018-01-10 18:47         ` Van De Ven, Arjan
2018-01-11 12:45   ` Woodhouse, David
2018-01-10  2:26 ` [PATCH v3 4/5] x86/ibrs: Create boot option for IBRS Tim Chen
2018-01-10  2:26 ` [PATCH v3 5/5] x86/idle: Disable IBRS entering idle and enable it on wakeup Tim Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).