[PATCH 00/35] jump_label, objtool, IBRS and IBPB

[PATCH 00/35] jump_label, objtool, IBRS and IBPB

[Peter Zijlstra, Peter Zijlstra]

Lots of patches..

They include:

 - objtool validation of jump_label/static_cpu_has

   Allows asserting that the code block following a jump_label/static_cpu_has
   is indeed unconditional. Ensures GCC doesn't generate particularly stupid
   code which would re-insert a dynamic test.

 - objtool validation of retpoline

   Looks for indirect JMP/CALL sites when build with a retpoline enabled
   compiler. Has already spotted a bunch of sites that need fixing, see
   below.

 - makes x86 hard rely on asm-goto to ensure we can indeed use static_cpu_has
   to avoid dynamic branches (and thus speculation).

 - The IBRS/IBPB patches from Thomas that use static_cpu_has()

   These hard rely on the above; we must not speculate across the IBRS/IBPB
   MSR writes otherwise that would totally defeat the point. Prior patches had
   LFENCE crud in the else-clause, which then makes the primitives
   unconditionally expensive.

 - Rebased the IBRS/IBPB-KVM patches from Ashok on top

 - Random odd fixes for various things encountered while doing the above.


Please have a look and sorry for this many patches.

---

Output of a x86_64-allmodconfig -KCOV -KASAN build:

arch/x86/entry/.tmp_entry_64.o: warning: objtool: .entry.text+0x1cb2: indirect call found in RETPOLINE build
arch/x86/entry/.tmp_entry_64.o: warning: objtool: .entry.text+0x1cc7: indirect call found in RETPOLINE build
arch/x86/hyperv/.tmp_mmu.o: warning: objtool: hyperv_flush_tlb_others()+0x30c: indirect call found in RETPOLINE build
arch/x86/hyperv/.tmp_mmu.o: warning: objtool: hyperv_flush_tlb_others()+0x3b0: indirect call found in RETPOLINE build
arch/x86/hyperv/.tmp_mmu.o: warning: objtool: hyperv_flush_tlb_others_ex()+0x3a1: indirect call found in RETPOLINE build
arch/x86/hyperv/.tmp_mmu.o: warning: objtool: hyperv_flush_tlb_others_ex()+0x45c: indirect call found in RETPOLINE build
arch/x86/xen/.tmp_multicalls.o: warning: objtool: xen_mc_flush()+0x1da: indirect call found in RETPOLINE build
arch/x86/kvm/.tmp_emulate.o: warning: objtool: fastop()+0x54: indirect call found in RETPOLINE build
arch/x86/kvm/.tmp_emulate.o: warning: objtool: em_loop()+0xcc: indirect call found in RETPOLINE build
arch/x86/kvm/.tmp_emulate.o: warning: objtool: x86_emulate_insn()+0xbd6: indirect call found in RETPOLINE build
arch/x86/kvm/.tmp_emulate.o: warning: objtool: x86_emulate_insn()+0xc1a: indirect call found in RETPOLINE build
arch/x86/kvm/.tmp_emulate.o: warning: objtool: x86_emulate_insn()+0xc66: indirect call found in RETPOLINE build
arch/x86/kvm/.tmp_vmx.o: warning: objtool: vmx_handle_external_intr()+0x50: indirect call found in RETPOLINE build
arch/x86/mm/.tmp_mem_encrypt_boot.o: warning: objtool: sme_encrypt_execute()+0x48: indirect call found in RETPOLINE build
drivers/hv/.tmp_hv.o: warning: objtool: hv_post_message()+0x72: indirect call found in RETPOLINE build
drivers/hv/.tmp_connection.o: warning: objtool: vmbus_set_event()+0x33: indirect call found in RETPOLINE build
drivers/pci/host/.tmp_pci-hyperv.o: warning: objtool: hv_irq_unmask()+0x22b: indirect call found in RETPOLINE build
drivers/xen/.tmp_privcmd.o: warning: objtool: privcmd_ioctl()+0xcf: indirect call found in RETPOLINE build
drivers/watchdog/.tmp_hpwdt.o: warning: objtool: .text+0x24: indirect call found in RETPOLINE build

[PATCH 01/35] jump_label: Add branch hints to static_branch_{un,}likely()

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-jump_label-fix.patch --]
[-- Type: text/plain, Size: 935 bytes --]

For some reason these were missing, I've not observed this patch
making a difference in the few code locations I checked, but this
makes sense.

Cc: Jason Baron <jbaron@akamai.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/linux/jump_label.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -388,7 +388,7 @@ extern bool ____wrong_branch_error(void)
 		branch = !arch_static_branch_jump(&(x)->key, true);		\
 	else									\
 		branch = ____wrong_branch_error();				\
-	branch;									\
+	likely(branch);								\
 })
 
 #define static_branch_unlikely(x)						\
@@ -400,7 +400,7 @@ extern bool ____wrong_branch_error(void)
 		branch = arch_static_branch(&(x)->key, false);			\
 	else									\
 		branch = ____wrong_branch_error();				\
-	branch;									\
+	unlikely(branch);							\
 })
 
 #else /* !HAVE_JUMP_LABEL */

[PATCH 02/35] sched: Optimize ttwu_stat()

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-sched-opt-ttwu_stat.patch --]
[-- Type: text/plain, Size: 2601 bytes --]

The whole of ttwu_stat() is guarded by a single schedstat_enabled(),
there is absolutely no point in then issuing another static_branch for
every single schedstat_inc() in there.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/core.c  |   16 ++++++++--------
 kernel/sched/stats.h |    2 ++
 2 files changed, 10 insertions(+), 8 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1630,16 +1630,16 @@ ttwu_stat(struct task_struct *p, int cpu
 
 #ifdef CONFIG_SMP
 	if (cpu == rq->cpu) {
-		schedstat_inc(rq->ttwu_local);
-		schedstat_inc(p->se.statistics.nr_wakeups_local);
+		__schedstat_inc(rq->ttwu_local);
+		__schedstat_inc(p->se.statistics.nr_wakeups_local);
 	} else {
 		struct sched_domain *sd;
 
-		schedstat_inc(p->se.statistics.nr_wakeups_remote);
+		__schedstat_inc(p->se.statistics.nr_wakeups_remote);
 		rcu_read_lock();
 		for_each_domain(rq->cpu, sd) {
 			if (cpumask_test_cpu(cpu, sched_domain_span(sd))) {
-				schedstat_inc(sd->ttwu_wake_remote);
+				__schedstat_inc(sd->ttwu_wake_remote);
 				break;
 			}
 		}
@@ -1647,14 +1647,14 @@ ttwu_stat(struct task_struct *p, int cpu
 	}
 
 	if (wake_flags & WF_MIGRATED)
-		schedstat_inc(p->se.statistics.nr_wakeups_migrate);
+		__schedstat_inc(p->se.statistics.nr_wakeups_migrate);
 #endif /* CONFIG_SMP */
 
-	schedstat_inc(rq->ttwu_count);
-	schedstat_inc(p->se.statistics.nr_wakeups);
+	__schedstat_inc(rq->ttwu_count);
+	__schedstat_inc(p->se.statistics.nr_wakeups);
 
 	if (wake_flags & WF_SYNC)
-		schedstat_inc(p->se.statistics.nr_wakeups_sync);
+		__schedstat_inc(p->se.statistics.nr_wakeups_sync);
 }
 
 static inline void ttwu_activate(struct rq *rq, struct task_struct *p, int en_flags)
--- a/kernel/sched/stats.h
+++ b/kernel/sched/stats.h
@@ -31,6 +31,7 @@ rq_sched_info_dequeued(struct rq *rq, un
 		rq->rq_sched_info.run_delay += delta;
 }
 #define schedstat_enabled()		static_branch_unlikely(&sched_schedstats)
+#define __schedstat_inc(var)		do { var++; } while (0)
 #define schedstat_inc(var)		do { if (schedstat_enabled()) { var++; } } while (0)
 #define schedstat_add(var, amt)		do { if (schedstat_enabled()) { var += (amt); } } while (0)
 #define schedstat_set(var, val)		do { if (schedstat_enabled()) { var = (val); } } while (0)
@@ -48,6 +49,7 @@ static inline void
 rq_sched_info_depart(struct rq *rq, unsigned long long delta)
 {}
 #define schedstat_enabled()		0
+#define __schedstat_inc(var)		do { } while (0)
 #define schedstat_inc(var)		do { } while (0)
 #define schedstat_add(var, amt)		do { } while (0)
 #define schedstat_set(var, val)		do { } while (0)

[PATCH 03/35] x86: Reindent _static_cpu_has

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-fix-static_cpu_has.patch --]
[-- Type: text/plain, Size: 3020 bytes --]

Because its daft..

Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/cpufeature.h |   78 +++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 39 deletions(-)

--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -145,45 +145,45 @@ extern void clear_cpu_cap(struct cpuinfo
  */
 static __always_inline __pure bool _static_cpu_has(u16 bit)
 {
-		asm_volatile_goto("1: jmp 6f\n"
-			 "2:\n"
-			 ".skip -(((5f-4f) - (2b-1b)) > 0) * "
-			         "((5f-4f) - (2b-1b)),0x90\n"
-			 "3:\n"
-			 ".section .altinstructions,\"a\"\n"
-			 " .long 1b - .\n"		/* src offset */
-			 " .long 4f - .\n"		/* repl offset */
-			 " .word %P1\n"			/* always replace */
-			 " .byte 3b - 1b\n"		/* src len */
-			 " .byte 5f - 4f\n"		/* repl len */
-			 " .byte 3b - 2b\n"		/* pad len */
-			 ".previous\n"
-			 ".section .altinstr_replacement,\"ax\"\n"
-			 "4: jmp %l[t_no]\n"
-			 "5:\n"
-			 ".previous\n"
-			 ".section .altinstructions,\"a\"\n"
-			 " .long 1b - .\n"		/* src offset */
-			 " .long 0\n"			/* no replacement */
-			 " .word %P0\n"			/* feature bit */
-			 " .byte 3b - 1b\n"		/* src len */
-			 " .byte 0\n"			/* repl len */
-			 " .byte 0\n"			/* pad len */
-			 ".previous\n"
-			 ".section .altinstr_aux,\"ax\"\n"
-			 "6:\n"
-			 " testb %[bitnum],%[cap_byte]\n"
-			 " jnz %l[t_yes]\n"
-			 " jmp %l[t_no]\n"
-			 ".previous\n"
-			 : : "i" (bit), "i" (X86_FEATURE_ALWAYS),
-			     [bitnum] "i" (1 << (bit & 7)),
-			     [cap_byte] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
-			 : : t_yes, t_no);
-	t_yes:
-		return true;
-	t_no:
-		return false;
+	asm_volatile_goto("1: jmp 6f\n"
+		 "2:\n"
+		 ".skip -(((5f-4f) - (2b-1b)) > 0) * "
+			 "((5f-4f) - (2b-1b)),0x90\n"
+		 "3:\n"
+		 ".section .altinstructions,\"a\"\n"
+		 " .long 1b - .\n"		/* src offset */
+		 " .long 4f - .\n"		/* repl offset */
+		 " .word %P1\n"			/* always replace */
+		 " .byte 3b - 1b\n"		/* src len */
+		 " .byte 5f - 4f\n"		/* repl len */
+		 " .byte 3b - 2b\n"		/* pad len */
+		 ".previous\n"
+		 ".section .altinstr_replacement,\"ax\"\n"
+		 "4: jmp %l[t_no]\n"
+		 "5:\n"
+		 ".previous\n"
+		 ".section .altinstructions,\"a\"\n"
+		 " .long 1b - .\n"		/* src offset */
+		 " .long 0\n"			/* no replacement */
+		 " .word %P0\n"			/* feature bit */
+		 " .byte 3b - 1b\n"		/* src len */
+		 " .byte 0\n"			/* repl len */
+		 " .byte 0\n"			/* pad len */
+		 ".previous\n"
+		 ".section .altinstr_aux,\"ax\"\n"
+		 "6:\n"
+		 " testb %[bitnum],%[cap_byte]\n"
+		 " jnz %l[t_yes]\n"
+		 " jmp %l[t_no]\n"
+		 ".previous\n"
+		 : : "i" (bit), "i" (X86_FEATURE_ALWAYS),
+		     [bitnum] "i" (1 << (bit & 7)),
+		     [cap_byte] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
+		 : : t_yes, t_no);
+t_yes:
+	return true;
+t_no:
+	return false;
 }
 
 #define static_cpu_has(bit)					\

[PATCH 04/35] x86: Update _static_cpu_has to use all named variables

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-fix-static_cpu_has_names.patch --]
[-- Type: text/plain, Size: 1491 bytes --]


Requested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/cpufeature.h |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -153,7 +153,7 @@ static __always_inline __pure bool _stat
 		 ".section .altinstructions,\"a\"\n"
 		 " .long 1b - .\n"		/* src offset */
 		 " .long 4f - .\n"		/* repl offset */
-		 " .word %P1\n"			/* always replace */
+		 " .word %P[always]\n"		/* always replace */
 		 " .byte 3b - 1b\n"		/* src len */
 		 " .byte 5f - 4f\n"		/* repl len */
 		 " .byte 3b - 2b\n"		/* pad len */
@@ -165,7 +165,7 @@ static __always_inline __pure bool _stat
 		 ".section .altinstructions,\"a\"\n"
 		 " .long 1b - .\n"		/* src offset */
 		 " .long 0\n"			/* no replacement */
-		 " .word %P0\n"			/* feature bit */
+		 " .word %P[feature]\n"		/* feature bit */
 		 " .byte 3b - 1b\n"		/* src len */
 		 " .byte 0\n"			/* repl len */
 		 " .byte 0\n"			/* pad len */
@@ -176,8 +176,9 @@ static __always_inline __pure bool _stat
 		 " jnz %l[t_yes]\n"
 		 " jmp %l[t_no]\n"
 		 ".previous\n"
-		 : : "i" (bit), "i" (X86_FEATURE_ALWAYS),
-		     [bitnum] "i" (1 << (bit & 7)),
+		 : : [feature]  "i" (bit),
+		     [always]   "i" (X86_FEATURE_ALWAYS),
+		     [bitnum]   "i" (1 << (bit & 7)),
 		     [cap_byte] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
 		 : : t_yes, t_no);
 t_yes:

[PATCH 05/35] x86: Add a type field to alt_instr

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-alternative-type.patch --]
[-- Type: text/plain, Size: 3057 bytes --]

Add a type field to the alternative description. For now this will be
used to annotate static_cpu_has() but possible future uses include
using it to implement alternative alignment and special NOP handling.

Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/alternative-asm.h |    3 ++-
 arch/x86/include/asm/alternative.h     |    6 +++++-
 arch/x86/include/asm/cpufeature.h      |    2 ++
 tools/objtool/special.c                |    2 +-
 4 files changed, 10 insertions(+), 3 deletions(-)

--- a/arch/x86/include/asm/alternative-asm.h
+++ b/arch/x86/include/asm/alternative-asm.h
@@ -25,13 +25,14 @@
  * enough information for the alternatives patching code to patch an
  * instruction. See apply_alternatives().
  */
-.macro altinstruction_entry orig alt feature orig_len alt_len pad_len
+.macro altinstruction_entry orig alt feature orig_len alt_len pad_len type=0
 	.long \orig - .
 	.long \alt - .
 	.word \feature
 	.byte \orig_len
 	.byte \alt_len
 	.byte \pad_len
+	.byte \type
 .endm
 
 /*
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -45,6 +45,8 @@
 #define LOCK_PREFIX ""
 #endif
 
+#define ALT_TYPE_DEFAULT	0
+
 struct alt_instr {
 	s32 instr_offset;	/* original instruction */
 	s32 repl_offset;	/* offset to replacement instruction */
@@ -52,6 +54,7 @@ struct alt_instr {
 	u8  instrlen;		/* length of original instruction */
 	u8  replacementlen;	/* length of new instruction */
 	u8  padlen;		/* length of build-time padding */
+	u8  type;		/* type of alternative */
 } __packed;
 
 /*
@@ -127,7 +130,8 @@ static inline int alternatives_text_rese
 	" .word " __stringify(feature) "\n"		/* feature bit     */ \
 	" .byte " alt_total_slen "\n"			/* source len      */ \
 	" .byte " alt_rlen(num) "\n"			/* replacement len */ \
-	" .byte " alt_pad_len "\n"			/* pad len */
+	" .byte " alt_pad_len "\n"			/* pad len */	      \
+	" .byte 0 \n"					/* type */
 
 #define ALTINSTR_REPLACEMENT(newinstr, feature, num)	/* replacement */     \
 	b_replacement(num)":\n\t" newinstr "\n" e_replacement(num) ":\n\t"
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -157,6 +157,7 @@ static __always_inline __pure bool _stat
 		 " .byte 3b - 1b\n"		/* src len */
 		 " .byte 5f - 4f\n"		/* repl len */
 		 " .byte 3b - 2b\n"		/* pad len */
+		 " .byte 0\n"			/* type */
 		 ".previous\n"
 		 ".section .altinstr_replacement,\"ax\"\n"
 		 "4: jmp %l[t_no]\n"
@@ -169,6 +170,7 @@ static __always_inline __pure bool _stat
 		 " .byte 3b - 1b\n"		/* src len */
 		 " .byte 0\n"			/* repl len */
 		 " .byte 0\n"			/* pad len */
+		 " .byte 0\n"			/* type */
 		 ".previous\n"
 		 ".section .altinstr_aux,\"ax\"\n"
 		 "6:\n"
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -34,7 +34,7 @@
 #define JUMP_ORIG_OFFSET	0
 #define JUMP_NEW_OFFSET		8
 
-#define ALT_ENTRY_SIZE		13
+#define ALT_ENTRY_SIZE		14
 #define ALT_ORIG_OFFSET		0
 #define ALT_NEW_OFFSET		4
 #define ALT_FEATURE_OFFSET	8

[PATCH 06/35] objtool: Implement base jump_assert support

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peter_zijlstra-q-objtool_vs_jump_label.patch --]
[-- Type: text/plain, Size: 4017 bytes --]

Implement a jump_label assertion that asserts that the code location
is indeed only reachable through a static_branch. Because if GCC is
absolutely retaded it could generate code like:

	xor rax,rax
	NOP/JMP 1f
	mov $1, rax
1:
	test rax,rax
	jz 2f
	<do-code>
2:

instead of the sensible:

	NOP/JMP 1f
	<do-code>
1:

This implements objtool infrastructure for ensuring the code ends up
sane, since we'll rely on that for correctness and security.

We tag the instructions after the static branch with static_jump_dest=true;
that is the instruction after the NOP and the instruction at the
JMP+disp site.

Then, when we read the .discard.jump_assert section, we assert that
each entry points to an instruction that has static_jump_dest set.

With this we can assert that the code emitted for the if statement
ends up at the static jump location and nothing untowards happened.

Cc: Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c |   70 ++++++++++++++++++++++++++++++++++++++++++++++++--
 tools/objtool/check.h |    1 
 2 files changed, 69 insertions(+), 2 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -687,8 +687,17 @@ static int handle_jump_alt(struct objtoo
 			   struct instruction *orig_insn,
 			   struct instruction **new_insn)
 {
-	if (orig_insn->type == INSN_NOP)
+	struct instruction *next_insn = list_next_entry(orig_insn, list);
+
+	if (orig_insn->type == INSN_NOP) {
+		/*
+		 * If orig_insn is a NOP, then new_insn is the branch target
+		 * for when it would've been a JMP.
+		 */
+		next_insn->static_jump_dest = true;
+		(*new_insn)->static_jump_dest = true;
 		return 0;
+	}
 
 	if (orig_insn->type != INSN_JUMP_UNCONDITIONAL) {
 		WARN_FUNC("unsupported instruction at jump label",
@@ -696,7 +705,16 @@ static int handle_jump_alt(struct objtoo
 		return -1;
 	}
 
-	*new_insn = list_next_entry(orig_insn, list);
+	/*
+	 * Otherwise, orig_insn is a JMP and it will have orig_insn->jump_dest.
+	 * In this case we'll effectively NOP the alt by pointing new_insn at
+	 * next_insn.
+	 */
+	orig_insn->jump_dest->static_jump_dest = true;
+	next_insn->static_jump_dest = true;
+
+	*new_insn = next_insn;
+
 	return 0;
 }
 
@@ -1067,6 +1085,50 @@ static int read_unwind_hints(struct objt
 	return 0;
 }
 
+static int assert_static_jumps(struct objtool_file *file)
+{
+	struct section *sec, *relasec;
+	struct instruction *insn;
+	struct rela *rela;
+	int i;
+
+	sec = find_section_by_name(file->elf, ".discard.jump_assert");
+	if (!sec)
+		return 0;
+
+	relasec = sec->rela;
+	if (!relasec) {
+		WARN("missing .rela.discard.jump_assert section");
+		return -1;
+	}
+
+	if (sec->len % sizeof(unsigned long)) {
+		WARN("jump_assert size mismatch: %d %ld", sec->len, sizeof(unsigned long));
+		return -1;
+	}
+
+	for (i = 0; i < sec->len / sizeof(unsigned long); i++) {
+		rela = find_rela_by_dest(sec, i * sizeof(unsigned long));
+		if (!rela) {
+			WARN("can't find rela for jump_assert[%d]", i);
+			return -1;
+		}
+
+		insn = find_insn(file, rela->sym->sec, rela->addend);
+		if (!insn) {
+			WARN("can't find insn for jump_assert[%d]", i);
+			return -1;
+		}
+
+		if (!insn->static_jump_dest) {
+			WARN_FUNC("static assert FAIL", insn->sec, insn->offset);
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
 static int decode_sections(struct objtool_file *file)
 {
 	int ret;
@@ -2000,6 +2062,10 @@ int check(const char *_objname, bool _no
 		goto out;
 	warnings += ret;
 
+	ret = assert_static_jumps(&file);
+	if (ret < 0)
+		return ret;
+
 	if (list_empty(&file.insn_list))
 		goto out;
 
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -45,6 +45,7 @@ struct instruction {
 	unsigned char type;
 	unsigned long immediate;
 	bool alt_group, visited, dead_end, ignore, hint, save, restore, ignore_alts;
+	bool static_jump_dest;
 	struct symbol *call_dest;
 	struct instruction *jump_dest;
 	struct list_head alts;

[PATCH 07/35] x86: Annotate static_cpu_has alternative

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-x86-static_cpu_has-annotate.patch --]
[-- Type: text/plain, Size: 1696 bytes --]

In order to recognise static_cpu_has() alternatives from any other
alternative without dodgy heuristics, we need to explicitly mark them.
Use the new type field for this.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/alternative.h |    1 +
 arch/x86/include/asm/cpufeature.h  |    5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -46,6 +46,7 @@
 #endif
 
 #define ALT_TYPE_DEFAULT	0
+#define ALT_TYPE_STATIC_CPU_HAS	1 /* objtool, static_cpu_has */
 
 struct alt_instr {
 	s32 instr_offset;	/* original instruction */
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -157,7 +157,7 @@ static __always_inline __pure bool _stat
 		 " .byte 3b - 1b\n"		/* src len */
 		 " .byte 5f - 4f\n"		/* repl len */
 		 " .byte 3b - 2b\n"		/* pad len */
-		 " .byte 0\n"			/* type */
+		 " .byte %P[type]\n"		/* type */
 		 ".previous\n"
 		 ".section .altinstr_replacement,\"ax\"\n"
 		 "4: jmp %l[t_no]\n"
@@ -170,7 +170,7 @@ static __always_inline __pure bool _stat
 		 " .byte 3b - 1b\n"		/* src len */
 		 " .byte 0\n"			/* repl len */
 		 " .byte 0\n"			/* pad len */
-		 " .byte 0\n"			/* type */
+		 " .byte %P[type]\n"		/* type */
 		 ".previous\n"
 		 ".section .altinstr_aux,\"ax\"\n"
 		 "6:\n"
@@ -181,6 +181,7 @@ static __always_inline __pure bool _stat
 		 : : [feature]  "i" (bit),
 		     [always]   "i" (X86_FEATURE_ALWAYS),
 		     [bitnum]   "i" (1 << (bit & 7)),
+		     [type]	"i" (ALT_TYPE_STATIC_CPU_HAS),
 		     [cap_byte] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
 		 : : t_yes, t_no);
 t_yes:

[PATCH 08/35] objtool: Implement jump_assert for _static_cpu_has()

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-objtool-static_cpu_has.patch --]
[-- Type: text/plain, Size: 3885 bytes --]

Unlike the jump_label bits, static_cpu_has is implemented with
alternatives. We use the new type field to distinguish them from any
other alternatives

Like jump_labels, make static_cpu_has set static_jump_dest on the
instructions after the static branch such that we can assert on it.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c   |   21 +++++++++++++++++++++
 tools/objtool/special.c |   26 +++++++++++++++-----------
 tools/objtool/special.h |    1 +
 3 files changed, 37 insertions(+), 11 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -636,6 +636,12 @@ static int handle_group_alt(struct objto
 	fake_jump->ignore = true;
 
 	if (!special_alt->new_len) {
+		/*
+		 * The NOP case for _static_cpu_has()
+		 */
+		if (special_alt->static_cpu_has)
+			fake_jump->jump_dest->static_jump_dest = true;
+
 		*new_insn = fake_jump;
 		return 0;
 	}
@@ -664,6 +670,21 @@ static int handle_group_alt(struct objto
 				  insn->sec, insn->offset);
 			return -1;
 		}
+
+		if (special_alt->static_cpu_has) {
+			if (insn->type != INSN_JUMP_UNCONDITIONAL) {
+				WARN_FUNC("not an unconditional jump in _static_cpu_has()",
+					  insn->sec, insn->offset);
+			}
+			if (insn->jump_dest == fake_jump) {
+				WARN_FUNC("jump inside alternative for _static_cpu_has()",
+					  insn->sec, insn->offset);
+			}
+			/*
+			 * The JMP+disp case for _static_cpu_has()
+			 */
+			insn->jump_dest->static_jump_dest = true;
+		}
 	}
 
 	if (!last_new_insn) {
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -40,6 +40,10 @@
 #define ALT_FEATURE_OFFSET	8
 #define ALT_ORIG_LEN_OFFSET	10
 #define ALT_NEW_LEN_OFFSET	11
+#define ALT_TYPE_OFFSET		13
+
+#define ALT_TYPE_DEFAULT	0
+#define ALT_TYPE_STATIC_CPU_HAS	1
 
 #define X86_FEATURE_POPCNT (4*32+23)
 
@@ -48,7 +52,6 @@ struct special_entry {
 	bool group, jump_or_nop;
 	unsigned char size, orig, new;
 	unsigned char orig_len, new_len; /* group only */
-	unsigned char feature; /* ALTERNATIVE macro CPU feature */
 };
 
 struct special_entry entries[] = {
@@ -60,7 +63,6 @@ struct special_entry entries[] = {
 		.orig_len = ALT_ORIG_LEN_OFFSET,
 		.new = ALT_NEW_OFFSET,
 		.new_len = ALT_NEW_LEN_OFFSET,
-		.feature = ALT_FEATURE_OFFSET,
 	},
 	{
 		.sec = "__jump_table",
@@ -84,24 +86,23 @@ static int get_alt_entry(struct elf *elf
 {
 	struct rela *orig_rela, *new_rela;
 	unsigned long offset;
+	void *data;
 
 	offset = idx * entry->size;
+	data = sec->data->d_buf + offset;
 
 	alt->group = entry->group;
 	alt->jump_or_nop = entry->jump_or_nop;
 
 	if (alt->group) {
-		alt->orig_len = *(unsigned char *)(sec->data->d_buf + offset +
-						   entry->orig_len);
-		alt->new_len = *(unsigned char *)(sec->data->d_buf + offset +
-						  entry->new_len);
-	}
-
-	if (entry->feature) {
 		unsigned short feature;
+		unsigned char type;
 
-		feature = *(unsigned short *)(sec->data->d_buf + offset +
-					      entry->feature);
+		alt->orig_len = *(unsigned char *)(data + entry->orig_len);
+		alt->new_len = *(unsigned char *)(data + entry->new_len);
+
+		feature = *(unsigned short *)(data + ALT_FEATURE_OFFSET);
+		type = *(unsigned char *)(data + ALT_TYPE_OFFSET);
 
 		/*
 		 * It has been requested that we don't validate the !POPCNT
@@ -110,6 +111,9 @@ static int get_alt_entry(struct elf *elf
 		 */
 		if (feature == X86_FEATURE_POPCNT)
 			alt->skip_orig = true;
+
+		if (type == ALT_TYPE_STATIC_CPU_HAS)
+			alt->static_cpu_has = true;
 	}
 
 	orig_rela = find_rela_by_dest(sec, offset + entry->orig);
--- a/tools/objtool/special.h
+++ b/tools/objtool/special.h
@@ -27,6 +27,7 @@ struct special_alt {
 	bool group;
 	bool skip_orig;
 	bool jump_or_nop;
+	bool static_cpu_has;
 
 	struct section *orig_sec;
 	unsigned long orig_off;

[PATCH 09/35] objtool: Introduce special_type

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-objtool-special-type.patch --]
[-- Type: text/plain, Size: 3157 bytes --]

Use an enum for the special_alt entries instead of a collection of
booleans.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c   |   14 +++++++++++---
 tools/objtool/special.c |   14 +++++++-------
 tools/objtool/special.h |   10 ++++++++--
 3 files changed, 26 insertions(+), 12 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -773,7 +773,7 @@ static int add_special_section_alts(stru
 			continue;
 
 		new_insn = NULL;
-		if (!special_alt->group || special_alt->new_len) {
+		if (special_alt->type != alternative || special_alt->new_len) {
 			new_insn = find_insn(file, special_alt->new_sec,
 					     special_alt->new_off);
 			if (!new_insn) {
@@ -785,16 +785,24 @@ static int add_special_section_alts(stru
 			}
 		}
 
-		if (special_alt->group) {
+		switch (special_alt->type) {
+		case alternative:
 			ret = handle_group_alt(file, special_alt, orig_insn,
 					       &new_insn);
 			if (ret)
 				goto out;
-		} else if (special_alt->jump_or_nop) {
+			break;
+
+		case jump_label:
 			ret = handle_jump_alt(file, special_alt, orig_insn,
 					      &new_insn);
 			if (ret)
 				goto out;
+
+			break;
+
+		default:
+			break;
 		}
 
 		alt = malloc(sizeof(*alt));
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -49,7 +49,7 @@
 
 struct special_entry {
 	const char *sec;
-	bool group, jump_or_nop;
+	enum special_type type;
 	unsigned char size, orig, new;
 	unsigned char orig_len, new_len; /* group only */
 };
@@ -57,7 +57,7 @@ struct special_entry {
 struct special_entry entries[] = {
 	{
 		.sec = ".altinstructions",
-		.group = true,
+		.type = alternative,
 		.size = ALT_ENTRY_SIZE,
 		.orig = ALT_ORIG_OFFSET,
 		.orig_len = ALT_ORIG_LEN_OFFSET,
@@ -66,13 +66,14 @@ struct special_entry entries[] = {
 	},
 	{
 		.sec = "__jump_table",
-		.jump_or_nop = true,
+		.type = jump_label,
 		.size = JUMP_ENTRY_SIZE,
 		.orig = JUMP_ORIG_OFFSET,
 		.new = JUMP_NEW_OFFSET,
 	},
 	{
 		.sec = "__ex_table",
+		.type = exception,
 		.size = EX_ENTRY_SIZE,
 		.orig = EX_ORIG_OFFSET,
 		.new = EX_NEW_OFFSET,
@@ -91,10 +92,9 @@ static int get_alt_entry(struct elf *elf
 	offset = idx * entry->size;
 	data = sec->data->d_buf + offset;
 
-	alt->group = entry->group;
-	alt->jump_or_nop = entry->jump_or_nop;
+	alt->type = entry->type;
 
-	if (alt->group) {
+	if (alt->type == alternative) {
 		unsigned short feature;
 		unsigned char type;
 
@@ -130,7 +130,7 @@ static int get_alt_entry(struct elf *elf
 	alt->orig_sec = orig_rela->sym->sec;
 	alt->orig_off = orig_rela->addend;
 
-	if (!entry->group || alt->new_len) {
+	if (entry->type != alternative || alt->new_len) {
 		new_rela = find_rela_by_dest(sec, offset + entry->new);
 		if (!new_rela) {
 			WARN_FUNC("can't find new rela",
--- a/tools/objtool/special.h
+++ b/tools/objtool/special.h
@@ -21,12 +21,18 @@
 #include <stdbool.h>
 #include "elf.h"
 
+enum special_type {
+	alternative,
+	jump_label,
+	exception,
+};
+
 struct special_alt {
 	struct list_head list;
 
-	bool group;
+	enum special_type type;
+
 	bool skip_orig;
-	bool jump_or_nop;
 	bool static_cpu_has;
 
 	struct section *orig_sec;

[PATCH 10/35] x86/jump_label: Implement arch_static_assert()

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-x86-asm-goto-assert.patch --]
[-- Type: text/plain, Size: 1422 bytes --]

Implement the static (branch) assertion. It simply emits the address
of the next instruction into a special section which objtool will read
and validate against either __jump_table or .altinstructions.

Use like:

	if (static_branch_likely(_key)) {
		arch_static_assert();
		/* do stuff */
	}

Or

	if (static_cpu_has(_feat)) {
		arch_static_assert();
		/* do stuff */
	}

Cc: Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/jump_label.h |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -62,6 +62,29 @@ static __always_inline bool arch_static_
 	return true;
 }
 
+/*
+ * Annotation for objtool; asserts that the previous instruction is the
+ * jump_label patch site. Or rather, that the next instruction is a static
+ * branch target.
+ *
+ * Use like:
+ *
+ *	if (static_branch_likely(key)) {
+ *		arch_static_assert();
+ *		do_code();
+ *	}
+ *
+ * Also works with static_cpu_has().
+ */
+static __always_inline void arch_static_assert(void)
+{
+	asm volatile ("1:\n\t"
+		      ".pushsection .discard.jump_assert \n\t"
+		      _ASM_ALIGN  "\n\t"
+		      _ASM_PTR "1b \n\t"
+		      ".popsection \n\t");
+}
+
 #ifdef CONFIG_X86_64
 typedef u64 jump_label_t;
 #else

[PATCH 11/35] objtool: Add retpoline validation

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-objtool-indirect.patch --]
[-- Type: text/plain, Size: 6989 bytes --]

David requested a objtool validation pass for RETPOLINE enabled
builds, where it validates no unannotated indirect  jumps or calls are
left.

Add an additional .discard.retpoline_safe section to allow annotating
the few indirect sites that are required and safe.

Requested-by: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 scripts/Makefile.build        |    6 ++
 tools/objtool/builtin-check.c |    5 +-
 tools/objtool/builtin-orc.c   |    4 -
 tools/objtool/check.c         |   92 +++++++++++++++++++++++++++++++++++++++---
 tools/objtool/check.h         |    4 -
 5 files changed, 100 insertions(+), 11 deletions(-)

--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -269,6 +269,10 @@ objtool_args += --no-unreachable
 else
 objtool_args += $(call cc-ifversion, -lt, 0405, --no-unreachable)
 endif
+ifdef CONFIG_RETPOLINE
+  objtool_args += --retpoline
+endif
+
 
 ifdef CONFIG_MODVERSIONS
 objtool_o = $(@D)/.tmp_$(@F)
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -29,7 +29,7 @@
 #include "builtin.h"
 #include "check.h"
 
-bool no_fp, no_unreachable;
+bool no_fp, no_unreachable, retpoline;
 
 static const char * const check_usage[] = {
 	"objtool check [<options>] file.o",
@@ -39,6 +39,7 @@ static const char * const check_usage[]
 const struct option check_options[] = {
 	OPT_BOOLEAN('f', "no-fp", &no_fp, "Skip frame pointer validation"),
 	OPT_BOOLEAN('u', "no-unreachable", &no_unreachable, "Skip 'unreachable instruction' warnings"),
+	OPT_BOOLEAN('r', "retpoline", &retpoline, "Validate retpoline assumptions"),
 	OPT_END(),
 };
 
@@ -53,5 +54,5 @@ int cmd_check(int argc, const char **arg
 
 	objname = argv[0];
 
-	return check(objname, no_fp, no_unreachable, false);
+	return check(objname, no_fp, no_unreachable, retpoline, false);
 }
--- a/tools/objtool/builtin-orc.c
+++ b/tools/objtool/builtin-orc.c
@@ -37,7 +37,7 @@ static const char *orc_usage[] = {
 };
 
 extern const struct option check_options[];
-extern bool no_fp, no_unreachable;
+extern bool no_fp, no_unreachable, retpoline;
 
 int cmd_orc(int argc, const char **argv)
 {
@@ -51,7 +51,7 @@ int cmd_orc(int argc, const char **argv)
 
 		objname = argv[0];
 
-		return check(objname, no_fp, no_unreachable, true);
+		return check(objname, no_fp, no_unreachable, retpoline, true);
 
 	}
 
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -33,7 +33,7 @@ struct alternative {
 };
 
 const char *objname;
-static bool no_fp;
+static bool no_fp, retpoline;
 struct cfi_state initial_func_cfi;
 
 struct instruction *find_insn(struct objtool_file *file,
@@ -496,6 +496,7 @@ static int add_jump_destinations(struct
 			 * disguise, so convert them accordingly.
 			 */
 			insn->type = INSN_JUMP_DYNAMIC;
+			insn->retpoline_safe = true;
 			continue;
 		} else {
 			/* sibling call */
@@ -548,13 +549,11 @@ static int add_call_destinations(struct
 			 * normal for a function to call within itself.  So
 			 * disable this warning for now.
 			 */
-#if 0
-			if (!insn->call_dest) {
+			if (!retpoline && !insn->call_dest) {
 				WARN_FUNC("can't find call dest symbol at offset 0x%lx",
 					  insn->sec, insn->offset, dest_off);
 				return -1;
 			}
-#endif
 		} else if (rela->sym->type == STT_SECTION) {
 			insn->call_dest = find_symbol_by_offset(rela->sym->sec,
 								rela->addend+4);
@@ -1158,6 +1157,54 @@ static int assert_static_jumps(struct ob
 	return 0;
 }
 
+static int read_retpoline_hints(struct objtool_file *file)
+{
+	struct section *sec, *relasec;
+	struct instruction *insn;
+	struct rela *rela;
+	int i;
+
+	sec = find_section_by_name(file->elf, ".discard.retpoline_safe");
+	if (!sec)
+		return 0;
+
+	relasec = sec->rela;
+	if (!relasec) {
+		WARN("missing .rela.discard.retpoline_safe section");
+		return -1;
+	}
+
+	if (sec->len % sizeof(unsigned long)) {
+		WARN("retpoline_safe size mismatch: %d %ld", sec->len, sizeof(unsigned long));
+		return -1;
+	}
+
+	for (i = 0; i < sec->len / sizeof(unsigned long); i++) {
+		rela = find_rela_by_dest(sec, i * sizeof(unsigned long));
+		if (!rela) {
+			WARN("can't find rela for retpoline_safe[%d]", i);
+			return -1;
+		}
+
+		insn = find_insn(file, rela->sym->sec, rela->addend);
+		if (!insn) {
+			WARN("can't find insn for retpoline_safe[%d]", i);
+			return -1;
+		}
+
+		if (insn->type != INSN_JUMP_DYNAMIC &&
+		    insn->type != INSN_CALL_DYNAMIC) {
+			WARN_FUNC("retpoline_safe hint not a indirect jump/call",
+				  insn->sec, insn->offset);
+			return -1;
+		}
+
+		insn->retpoline_safe = true;
+	}
+
+	return 0;
+}
+
 static int decode_sections(struct objtool_file *file)
 {
 	int ret;
@@ -1196,6 +1243,10 @@ static int decode_sections(struct objtoo
 	if (ret)
 		return ret;
 
+	ret = read_retpoline_hints(file);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
@@ -1939,6 +1990,29 @@ static int validate_unwind_hints(struct
 	return warnings;
 }
 
+static int validate_retpoline(struct objtool_file *file)
+{
+	struct instruction *insn;
+	int warnings = 0;
+
+	for_each_insn(file, insn) {
+		if (insn->type != INSN_JUMP_DYNAMIC &&
+		    insn->type != INSN_CALL_DYNAMIC)
+			continue;
+
+		if (insn->retpoline_safe)
+			continue;
+
+		WARN_FUNC("indirect %s found in RETPOLINE build",
+			  insn->sec, insn->offset,
+			  insn->type == INSN_JUMP_DYNAMIC ? "jump" : "call");
+
+		warnings++;
+	}
+
+	return warnings;
+}
+
 static bool is_kasan_insn(struct instruction *insn)
 {
 	return (insn->type == INSN_CALL &&
@@ -2064,13 +2138,14 @@ static void cleanup(struct objtool_file
 	elf_close(file->elf);
 }
 
-int check(const char *_objname, bool _no_fp, bool no_unreachable, bool orc)
+int check(const char *_objname, bool _no_fp, bool no_unreachable, bool _retpoline, bool orc)
 {
 	struct objtool_file file;
 	int ret, warnings = 0;
 
 	objname = _objname;
 	no_fp = _no_fp;
+	retpoline = _retpoline;
 
 	file.elf = elf_open(objname, orc ? O_RDWR : O_RDONLY);
 	if (!file.elf)
@@ -2098,6 +2173,13 @@ int check(const char *_objname, bool _no
 	if (list_empty(&file.insn_list))
 		goto out;
 
+	if (retpoline) {
+		ret = validate_retpoline(&file);
+		if (ret < 0)
+			return ret;
+		warnings += ret;
+	}
+
 	ret = validate_functions(&file);
 	if (ret < 0)
 		goto out;
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -45,7 +45,7 @@ struct instruction {
 	unsigned char type;
 	unsigned long immediate;
 	bool alt_group, visited, dead_end, ignore, hint, save, restore, ignore_alts;
-	bool static_jump_dest;
+	bool static_jump_dest, retpoline_safe;
 	struct symbol *call_dest;
 	struct instruction *jump_dest;
 	struct list_head alts;
@@ -63,7 +63,7 @@ struct objtool_file {
 	bool ignore_unreachables, c_file, hints;
 };
 
-int check(const char *objname, bool no_fp, bool no_unreachable, bool orc);
+int check(const char *objname, bool no_fp, bool no_unreachable, bool retpoline, bool orc);
 
 struct instruction *find_insn(struct objtool_file *file,
 			      struct section *sec, unsigned long offset);

[PATCH 12/35] x86/paravirt: Annotate indirect calls

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-retpoline-annotate-pv.patch --]
[-- Type: text/plain, Size: 3584 bytes --]

Paravirt emits indirect calls which get flagged by objtool retpoline
checks, annotate it away because all these indirect calls will be
patched out before we start userspace.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/paravirt.h       |   22 ++++++++++++++++++----
 arch/x86/include/asm/paravirt_types.h |    7 ++++++-
 2 files changed, 24 insertions(+), 5 deletions(-)

--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -828,6 +828,12 @@ extern void default_banner(void);
 	 .short clobbers;			\
 	.popsection
 
+#define PARA_RETPOLINE_SAFE				\
+	773:;						\
+	.pushsection .discard.retpoline_safe;		\
+	_ASM_PTR 773b;					\
+	.popsection
+
 
 #define COND_PUSH(set, mask, reg)			\
 	.if ((~(set)) & mask); push %reg; .endif
@@ -879,23 +885,27 @@ extern void default_banner(void);
 
 #define INTERRUPT_RETURN						\
 	PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_iret), CLBR_NONE,	\
-		  jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_iret))
+		  PARA_RETPOLINE_SAFE;					\
+		  jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_iret);)
 
 #define DISABLE_INTERRUPTS(clobbers)					\
 	PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_irq_disable), clobbers, \
 		  PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE);		\
+		  PARA_RETPOLINE_SAFE;					\
 		  call PARA_INDIRECT(pv_irq_ops+PV_IRQ_irq_disable);	\
 		  PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);)
 
 #define ENABLE_INTERRUPTS(clobbers)					\
 	PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_irq_enable), clobbers,	\
 		  PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE);		\
+		  PARA_RETPOLINE_SAFE;					\
 		  call PARA_INDIRECT(pv_irq_ops+PV_IRQ_irq_enable);	\
 		  PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);)
 
 #ifdef CONFIG_X86_32
 #define GET_CR0_INTO_EAX				\
 	push %ecx; push %edx;				\
+	PARA_RETPOLINE_SAFE;				\
 	call PARA_INDIRECT(pv_cpu_ops+PV_CPU_read_cr0);	\
 	pop %edx; pop %ecx
 #else	/* !CONFIG_X86_32 */
@@ -917,21 +927,25 @@ extern void default_banner(void);
  */
 #define SWAPGS								\
 	PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_swapgs), CLBR_NONE,	\
-		  call PARA_INDIRECT(pv_cpu_ops+PV_CPU_swapgs)		\
+		  PARA_RETPOLINE_SAFE;					\
+		  call PARA_INDIRECT(pv_cpu_ops+PV_CPU_swapgs);		\
 		 )
 
 #define GET_CR2_INTO_RAX				\
-	call PARA_INDIRECT(pv_mmu_ops+PV_MMU_read_cr2)
+	PARA_RETPOLINE_SAFE;				\
+	call PARA_INDIRECT(pv_mmu_ops+PV_MMU_read_cr2);
 
 #define USERGS_SYSRET64							\
 	PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_usergs_sysret64),	\
 		  CLBR_NONE,						\
-		  jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_usergs_sysret64))
+		  PARA_RETPOLINE_SAFE;					\
+		  jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_usergs_sysret64);)
 
 #ifdef CONFIG_DEBUG_ENTRY
 #define SAVE_FLAGS(clobbers)                                        \
 	PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_save_fl), clobbers, \
 		  PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE);        \
+		  PARA_RETPOLINE_SAFE;				    \
 		  call PARA_INDIRECT(pv_irq_ops+PV_IRQ_save_fl);    \
 		  PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);)
 #endif
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -392,7 +392,12 @@ int paravirt_disable_iospace(void);
  * offset into the paravirt_patch_template structure, and can therefore be
  * freely converted back into a structure offset.
  */
-#define PARAVIRT_CALL	"call *%c[paravirt_opptr];"
+#define PARAVIRT_CALL					\
+	"773:;\n"					\
+	".pushsection .discard.retpoline_safe\n"	\
+	_ASM_PTR " 773b\n"				\
+	".popsection\n"					\
+	"call *%c[paravirt_opptr];"
 
 /*
  * These macros are intended to wrap calls through one of the paravirt

[PATCH 13/35] x86,nospec: Annotate indirect calls/jumps

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-retpoline-annotate-nospec.patch --]
[-- Type: text/plain, Size: 1766 bytes --]

Annotate the indirect calls/jumps in the CALL_NOSPEC/JUMP_NOSPEC
alternatives.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/nospec-branch.h |   20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -67,6 +67,18 @@
 .endm
 
 /*
+ * This should be used immediately before an indirect jump/call. It tells
+ * objtool the subsequent indirect jump/call is vouched safe for retpoline
+ * builds.
+ */
+.macro ANNOTATE_RETPOLINE_SAFE
+	.Lannotate_\@:
+	.pushsection .discard.retpoline_safe
+	_ASM_PTR .Lannotate_\@
+	.popsection
+.endm
+
+/*
  * These are the bare retpoline primitives for indirect jmp and call.
  * Do not use these directly; they only exist to make the ALTERNATIVE
  * invocation below less ugly.
@@ -102,9 +114,9 @@
 .macro JMP_NOSPEC reg:req
 #ifdef CONFIG_RETPOLINE
 	ANNOTATE_NOSPEC_ALTERNATIVE
-	ALTERNATIVE_2 __stringify(jmp *\reg),				\
+	ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; jmp *\reg),	\
 		__stringify(RETPOLINE_JMP \reg), X86_FEATURE_RETPOLINE,	\
-		__stringify(lfence; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
+		__stringify(lfence; ANNOTATE_RETPOLINE_SAFE; jmp *\reg), X86_FEATURE_RETPOLINE_AMD
 #else
 	jmp	*\reg
 #endif
@@ -113,9 +125,9 @@
 .macro CALL_NOSPEC reg:req
 #ifdef CONFIG_RETPOLINE
 	ANNOTATE_NOSPEC_ALTERNATIVE
-	ALTERNATIVE_2 __stringify(call *\reg),				\
+	ALTERNATIVE_2 __stringify(ANNOTATE_RETPOLINE_SAFE; call *\reg),	\
 		__stringify(RETPOLINE_CALL \reg), X86_FEATURE_RETPOLINE,\
-		__stringify(lfence; call *\reg), X86_FEATURE_RETPOLINE_AMD
+		__stringify(lfence; ANNOTATE_RETPOLINE_SAFE; call *\reg), X86_FEATURE_RETPOLINE_AMD
 #else
 	call	*\reg
 #endif

[PATCH 14/35] x86: Annotate indirect jump in head_64.S

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-objtool-retpoline-annotate.patch --]
[-- Type: text/plain, Size: 713 bytes --]

The objtool retpoline validation found this indirect jump. Seeing how
its on CPU bringup before we run userspace it should be safe, annotate
it.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kernel/head_64.S |    2 ++
 1 file changed, 2 insertions(+)

--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -23,6 +23,7 @@
 #include <asm/nops.h>
 #include "../entry/calling.h"
 #include <asm/export.h>
+#include <asm/nospec-branch.h>
 
 #ifdef CONFIG_PARAVIRT
 #include <asm/asm-offsets.h>
@@ -134,6 +135,7 @@ ENTRY(secondary_startup_64)
 
 	/* Ensure I am executing from virtual addresses */
 	movq	$1f, %rax
+	ANNOTATE_RETPOLINE_SAFE
 	jmp	*%rax
 1:
 	UNWIND_HINT_EMPTY

[PATCH 15/35] objtool: More complex static jump implementation

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-objtool-fancy.patch --]
[-- Type: text/plain, Size: 3333 bytes --]

When using something like:

  -#define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]))
  +#define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]) && \
  +			(arch_static_assert(), true))

we get an objtool assertion fail like:

kernel/sched/fair.o: warning: objtool: hrtick_update()+0xd: static assert FAIL

where:

0000000000001140 <hrtick_update>:
    1140:       0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)
    1145:       c3                      retq
    1146:       48 8b b7 30 09 00 00    mov    0x930(%rdi),%rsi
    114d:       8b 87 d8 09 00 00       mov    0x9d8(%rdi),%eax
    1153:       48 0f a3 05 00 00 00    bt     %rax,0x0(%rip)        # 115b <hrtick_update+0x1b>
    115a:       00
                        1157: R_X86_64_PC32     __cpu_active_mask-0x4

and:

RELOCATION RECORDS FOR [__jump_table]:
0000000000000150 R_X86_64_64       .text+0x0000000000001140
0000000000000158 R_X86_64_64       .text+0x0000000000001146

RELOCATION RECORDS FOR [.discard.jump_assert]:
0000000000000028 R_X86_64_64       .text+0x000000000000114d

IOW, GCC managed to place the assertion 1 instruction _after_ the
static jump target (it lifted a load over it).

So while the code generation is fine, the assertion gets placed wrong.
We can 'fix' this by not only considering the immediate static jump
locations but also all the unconditional code after it, terminating
the basic block on any unconditional instruction or branch entry
point.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c |   41 +++++++++++++++++++++++++++++++++++++++++
 tools/objtool/check.h |    2 +-
 2 files changed, 42 insertions(+), 1 deletion(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -520,6 +520,8 @@ static int add_jump_destinations(struct
 				  dest_off);
 			return -1;
 		}
+
+		insn->jump_dest->branch_target = true;
 	}
 
 	return 0;
@@ -1197,6 +1199,41 @@ static int read_retpoline_hints(struct o
 	return 0;
 }
 
+static int grow_static_blocks(struct objtool_file *file)
+{
+	struct instruction *insn;
+	bool static_block = false;
+
+	for_each_insn(file, insn) {
+		if (!static_block && !insn->static_jump_dest)
+			continue;
+
+		if (insn->static_jump_dest) {
+			static_block = true;
+			continue;
+		}
+
+		if (insn->branch_target) {
+			static_block = false;
+			continue;
+		} else switch (insn->type) {
+		case INSN_JUMP_CONDITIONAL:
+		case INSN_JUMP_UNCONDITIONAL:
+		case INSN_JUMP_DYNAMIC:
+		case INSN_CALL:
+		case INSN_CALL_DYNAMIC:
+		case INSN_RETURN:
+		case INSN_BUG:
+			static_block = false;
+			continue;
+		}
+
+		insn->static_jump_dest = static_block;
+	}
+
+	return 0;
+}
+
 static int decode_sections(struct objtool_file *file)
 {
 	int ret;
@@ -1239,6 +1276,10 @@ static int decode_sections(struct objtoo
 	if (ret)
 		return ret;
 
+	ret = grow_static_blocks(file);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -45,7 +45,7 @@ struct instruction {
 	unsigned char type;
 	unsigned long immediate;
 	bool alt_group, visited, dead_end, ignore, hint, save, restore, ignore_alts;
-	bool static_jump_dest, retpoline_safe;
+	bool static_jump_dest, retpoline_safe, branch_target;
 	struct symbol *call_dest;
 	struct instruction *jump_dest;
 	struct list_head alts;

[PATCH 16/35] objtool: Use existing global variables for options

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-objtool-unclutter.patch --]
[-- Type: text/plain, Size: 2885 bytes --]

Use the existing global variables instead of passing them around and
creating duplicate global variables.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/builtin-check.c |    2 +-
 tools/objtool/builtin-orc.c   |    6 +-----
 tools/objtool/builtin.h       |    5 +++++
 tools/objtool/check.c         |    6 ++----
 tools/objtool/check.h         |    2 +-
 5 files changed, 10 insertions(+), 11 deletions(-)

--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -54,5 +54,5 @@ int cmd_check(int argc, const char **arg
 
 	objname = argv[0];
 
-	return check(objname, no_fp, no_unreachable, retpoline, false);
+	return check(objname, false);
 }
--- a/tools/objtool/builtin-orc.c
+++ b/tools/objtool/builtin-orc.c
@@ -25,7 +25,6 @@
  */
 
 #include <string.h>
-#include <subcmd/parse-options.h>
 #include "builtin.h"
 #include "check.h"
 
@@ -36,9 +35,6 @@ static const char *orc_usage[] = {
 	NULL,
 };
 
-extern const struct option check_options[];
-extern bool no_fp, no_unreachable, retpoline;
-
 int cmd_orc(int argc, const char **argv)
 {
 	const char *objname;
@@ -51,7 +47,7 @@ int cmd_orc(int argc, const char **argv)
 
 		objname = argv[0];
 
-		return check(objname, no_fp, no_unreachable, retpoline, true);
+		return check(objname, true);
 
 	}
 
--- a/tools/objtool/builtin.h
+++ b/tools/objtool/builtin.h
@@ -17,6 +17,11 @@
 #ifndef _BUILTIN_H
 #define _BUILTIN_H
 
+#include <subcmd/parse-options.h>
+
+extern const struct option check_options[];
+extern bool no_fp, no_unreachable, retpoline;
+
 extern int cmd_check(int argc, const char **argv);
 extern int cmd_orc(int argc, const char **argv);
 
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -18,6 +18,7 @@
 #include <string.h>
 #include <stdlib.h>
 
+#include "builtin.h"
 #include "check.h"
 #include "elf.h"
 #include "special.h"
@@ -33,7 +34,6 @@ struct alternative {
 };
 
 const char *objname;
-static bool no_fp, retpoline;
 struct cfi_state initial_func_cfi;
 
 struct instruction *find_insn(struct objtool_file *file,
@@ -2179,14 +2179,12 @@ static void cleanup(struct objtool_file
 	elf_close(file->elf);
 }
 
-int check(const char *_objname, bool _no_fp, bool no_unreachable, bool _retpoline, bool orc)
+int check(const char *_objname, bool orc)
 {
 	struct objtool_file file;
 	int ret, warnings = 0;
 
 	objname = _objname;
-	no_fp = _no_fp;
-	retpoline = _retpoline;
 
 	file.elf = elf_open(objname, orc ? O_RDWR : O_RDONLY);
 	if (!file.elf)
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -63,7 +63,7 @@ struct objtool_file {
 	bool ignore_unreachables, c_file, hints;
 };
 
-int check(const char *objname, bool no_fp, bool no_unreachable, bool retpoline, bool orc);
+int check(const char *objname, bool orc);
 
 struct instruction *find_insn(struct objtool_file *file,
 			      struct section *sec, unsigned long offset);

[PATCH 17/35] objtool: Even more complex static block checks

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-objtool-more-clever.patch --]
[-- Type: text/plain, Size: 3465 bytes --]

I've observed GCC transform:

  f()
  {
	  if (!static_branch_unlikely())
		  return;

	  static_assert();
	  A;
  }

  g()
  {
	  f();
  }

Into:

  f()
  {
	  static_assert();
	  A;
  }

  g()
  {
	  if (static_branch_unlikely())
		  f();
  }

Which results in the assertion landing at f+0. The transformation is
valid and useful; it avoids a pointless CALL+RET sequence, so we'll
have to teach objtool how to deal with this.

Do this by marking all CALL destinations with static_call when called
from a static_block and non_static_call when called outside a
static_block. This allows us to identify functions called exclusively
from a static_block and start them with a static_block.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c |   81 +++++++++++++++++++++++++++++++++++++-------------
 tools/objtool/elf.h   |    1 
 2 files changed, 61 insertions(+), 21 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1207,36 +1207,75 @@ static int read_retpoline_hints(struct o
 	return 0;
 }
 
+static bool __grow_static_block(struct objtool_file *file,
+				struct instruction *insn, bool ign_bt)
+{
+	/* if a jump can come in here, terminate */
+	if (insn->branch_target && !ign_bt)
+		return false;
+
+	switch (insn->type) {
+	case INSN_JUMP_UNCONDITIONAL:
+		/* mark this instruction, terminate this section */
+		insn->static_jump_dest = true;
+		return false;
+
+	/* these disturb unconditional code flow, terminate */
+	case INSN_JUMP_CONDITIONAL:
+	case INSN_JUMP_DYNAMIC:
+	case INSN_RETURN:
+	case INSN_BUG:
+		return false;
+
+	/* these return right back and don't disturb the code flow */
+	case INSN_CALL:
+	case INSN_CALL_DYNAMIC:
+		break;
+	}
+
+	/* mark this insn, and continue the section */
+	insn->static_jump_dest = true;
+	return true;
+}
+
 static int grow_static_blocks(struct objtool_file *file)
 {
-	struct instruction *insn;
 	bool static_block = false;
+	struct symbol *func, *tmp;
+	struct instruction *insn;
+	struct section *sec;
 
 	for_each_insn(file, insn) {
-		if (!static_block && !insn->static_jump_dest)
-			continue;
+		if (static_block || insn->static_jump_dest)
+			static_block = __grow_static_block(file, insn, !static_block);
 
-		if (insn->static_jump_dest) {
-			static_block = true;
-			continue;
+		if (insn->type == INSN_CALL) {
+			func = insn->call_dest;
+			if (!func)
+				continue;
+
+			if (static_block)
+				func->static_call = true;
+			else
+				func->non_static_call = true;
 		}
+	}
 
-		if (insn->branch_target) {
-			static_block = false;
-			continue;
-		} else switch (insn->type) {
-		case INSN_JUMP_CONDITIONAL:
-		case INSN_JUMP_UNCONDITIONAL:
-		case INSN_JUMP_DYNAMIC:
-		case INSN_CALL:
-		case INSN_CALL_DYNAMIC:
-		case INSN_RETURN:
-		case INSN_BUG:
-			static_block = false;
-			continue;
+	for_each_sec(file, sec) {
+		list_for_each_entry_safe(func, tmp, &sec->symbol_list, list) {
+			if (!func->static_call)
+				continue;
+
+			if (func->non_static_call)
+				continue;
+
+			/* static && !non_static -- only static callers */
+
+			func_for_each_insn(file, func, insn) {
+				if (!__grow_static_block(file, insn, false))
+					break;
+			}
 		}
-
-		insn->static_jump_dest = static_block;
 	}
 
 	return 0;
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -61,6 +61,7 @@ struct symbol {
 	unsigned char bind, type;
 	unsigned long offset;
 	unsigned int len;
+	bool static_call, non_static_call;
 };
 
 struct rela {

[PATCH 18/35] objtool: Another static block fail

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-objtool-more-clever-3.patch --]
[-- Type: text/plain, Size: 2621 bytes --]

I've observed GCC generate:

  sym:
     NOP/JMP 1f	(static_branch)
     JMP 2f
  1: /* crud */
     JMP 3f
  2: /* other crud */

  3: RETQ


This means we need to follow unconditional jumps; be conservative and
only follow if its a unique jump.

(I've not yet figured out which CONFIG option is responsible for this,
 a normal defconfig build does not generate crap like this)

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c |   33 +++++++++++++++++++++++++++++++--
 tools/objtool/check.h |    3 ++-
 2 files changed, 33 insertions(+), 3 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -521,7 +521,7 @@ static int add_jump_destinations(struct
 			return -1;
 		}
 
-		insn->jump_dest->branch_target = true;
+		insn->jump_dest->branch_target++;
 	}
 
 	return 0;
@@ -1207,6 +1207,8 @@ static int read_retpoline_hints(struct o
 	return 0;
 }
 
+static void jmp_grow_static_block(struct objtool_file *file, struct instruction *insn);
+
 static bool __grow_static_block(struct objtool_file *file,
 				struct instruction *insn, bool ign_bt)
 {
@@ -1216,7 +1218,8 @@ static bool __grow_static_block(struct o
 
 	switch (insn->type) {
 	case INSN_JUMP_UNCONDITIONAL:
-		/* mark this instruction, terminate this section */
+		/* follow the jump, mark this instruction, terminate this section */
+		jmp_grow_static_block(file, insn->jump_dest);
 		insn->static_jump_dest = true;
 		return false;
 
@@ -1238,6 +1241,32 @@ static bool __grow_static_block(struct o
 	return true;
 }
 
+static void jmp_grow_static_block(struct objtool_file *file, struct instruction *insn)
+{
+	bool ignore = true;
+
+	/* !jump_dest */
+	if (!insn)
+		return;
+
+	/* more than a single site jumps here, can't be certain */
+	if (insn->branch_target > 1)
+		return;
+
+	for (; &insn->list != &file->insn_list;
+	     insn = list_next_entry(insn, list)) {
+
+		/*
+		 * Per definition the first insn of a jump is a branch target,
+		 * don't terminate because of that.
+		 */
+		if (!__grow_static_block(file, insn, ignore))
+			break;
+
+		ignore = false;
+	}
+}
+
 static int grow_static_blocks(struct objtool_file *file)
 {
 	bool static_block = false;
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -45,7 +45,8 @@ struct instruction {
 	unsigned char type;
 	unsigned long immediate;
 	bool alt_group, visited, dead_end, ignore, hint, save, restore, ignore_alts;
-	bool static_jump_dest, retpoline_safe, branch_target;
+	bool static_jump_dest, retpoline_safe;
+	int branch_target;
 	struct symbol *call_dest;
 	struct instruction *jump_dest;
 	struct list_head alts;

Re: [PATCH 18/35] objtool: Another static block fail

[Peter Zijlstra]

On Thu, Jan 18, 2018 at 02:48:18PM +0100, Peter Zijlstra wrote:
> @@ -1216,7 +1218,8 @@ static bool __grow_static_block(struct o
>  
>  	switch (insn->type) {
>  	case INSN_JUMP_UNCONDITIONAL:
> -		/* mark this instruction, terminate this section */
> +		/* follow the jump, mark this instruction, terminate this section */
> +		jmp_grow_static_block(file, insn->jump_dest);

Hmm we should only do this when the jump is backwards, if its forwards
we should just mark the destination instruction and let the main
iteration loop take it.

>  		insn->static_jump_dest = true;
>  		return false;
>  

something like the below merged in maybe; I'm going to look at this
again later, my brain is completely useless today :/

Josh, how would you feel about adding the bits required to make objtool
a full disassembler? That would make it far easier to visualse the state
and I don't think its _that_ much more, all the difficult bits are
already done afaict, all we need is infrastructure to print the already
fully decoded instruction.


---
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1210,16 +1210,16 @@ static int read_retpoline_hints(struct o
 static void jmp_grow_static_block(struct objtool_file *file, struct instruction *insn);
 
 static bool __grow_static_block(struct objtool_file *file,
-				struct instruction *insn, bool ign_bt)
+				struct instruction *insn)
 {
-	/* if a jump can come in here, terminate */
-	if (insn->branch_target && !ign_bt)
+	/* if a !static jump can come in here, terminate */
+	if (insn->branch_target && !insn->static_jump_dest)
 		return false;
 
 	switch (insn->type) {
 	case INSN_JUMP_UNCONDITIONAL:
 		/* follow the jump, mark this instruction, terminate this section */
-		jmp_grow_static_block(file, insn->jump_dest);
+		jmp_grow_static_block(file, insn);
 		insn->static_jump_dest = true;
 		return false;
 
@@ -1243,27 +1243,43 @@ static bool __grow_static_block(struct o
 
 static void jmp_grow_static_block(struct objtool_file *file, struct instruction *insn)
 {
-	bool ignore = true;
+	struct instruction *dest = insn->jump_dest;
+	bool static_block = true;
 
-	/* !jump_dest */
-	if (!insn)
+	if (!dest)
 		return;
 
 	/* more than a single site jumps here, can't be certain */
-	if (insn->branch_target > 1)
+	if (dest->branch_target > 1)
 		return;
 
-	for (; &insn->list != &file->insn_list;
-	     insn = list_next_entry(insn, list)) {
-
+	if (dest->offset > insn->offset) {
 		/*
-		 * Per definition the first insn of a jump is a branch target,
-		 * don't terminate because of that.
+		 * fwd jump, the main iteration will still get there.
+		 * make sure it continues the section there.
 		 */
-		if (!__grow_static_block(file, insn, ignore))
-			break;
+		dest->static_jump_dest = true;
+		return;
+	}
 
-		ignore = false;
+	/* backwards jump, we need to revisit the instructions */
+	for (; static_block && dest != insn; dest = list_next_entry(dest, list)) {
+
+		static_block = __grow_static_block(file, dest);
+
+		if (insn->type == INSN_CALL) {
+			struct symbol *func = insn->call_dest;
+			if (!func)
+				continue;
+
+			/*
+			 * we flipped the call to static, update the stats.
+			 */
+			if (insn->static_jump_dest) {
+				func->non_static_call--;
+				func->static_call++;
+			}
+		}
 	}
 }
 
@@ -1276,7 +1292,7 @@ static int grow_static_blocks(struct obj
 
 	for_each_insn(file, insn) {
 		if (static_block || insn->static_jump_dest)
-			static_block = __grow_static_block(file, insn, !static_block);
+			static_block = __grow_static_block(file, insn);
 
 		if (insn->type == INSN_CALL) {
 			func = insn->call_dest;
@@ -1284,9 +1300,9 @@ static int grow_static_blocks(struct obj
 				continue;
 
 			if (static_block)
-				func->static_call = true;
+				func->static_call++;
 			else
-				func->non_static_call = true;
+				func->non_static_call++;
 		}
 	}
 
@@ -1301,7 +1317,7 @@ static int grow_static_blocks(struct obj
 			/* static && !non_static -- only static callers */
 
 			func_for_each_insn(file, func, insn) {
-				if (!__grow_static_block(file, insn, false))
+				if (!__grow_static_block(file, insn))
 					break;
 			}
 		}
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -61,7 +61,7 @@ struct symbol {
 	unsigned char bind, type;
 	unsigned long offset;
 	unsigned int len;
-	bool static_call, non_static_call;
+	unsigned int static_call, non_static_call;
 };
 
 struct rela {

Re: [PATCH 18/35] objtool: Another static block fail

[Josh Poimboeuf]

On Fri, Jan 19, 2018 at 05:42:29PM +0100, Peter Zijlstra wrote:
> Josh, how would you feel about adding the bits required to make objtool
> a full disassembler? That would make it far easier to visualse the state
> and I don't think its _that_ much more, all the difficult bits are
> already done afaict, all we need is infrastructure to print the already
> fully decoded instruction.

It sounds interesting, but I have no idea what you mean :-)  Can you
elaborate?

-- 
Josh

Re: [PATCH 18/35] objtool: Another static block fail

[Peter Zijlstra]

On Mon, Jan 29, 2018 at 12:01:05PM -0600, Josh Poimboeuf wrote:
> On Fri, Jan 19, 2018 at 05:42:29PM +0100, Peter Zijlstra wrote:
> > Josh, how would you feel about adding the bits required to make objtool
> > a full disassembler? That would make it far easier to visualse the state
> > and I don't think its _that_ much more, all the difficult bits are
> > already done afaict, all we need is infrastructure to print the already
> > fully decoded instruction.
> 
> It sounds interesting, but I have no idea what you mean :-)  Can you
> elaborate?

Well, having had to debug the objtool state, I end up printing
sec+offset and then matching up with objdump output. It might be easier
to just print annotated assembly in one go.

Of course, to make that so, we have this small matter of actually
implementing the bits that print the instructions first ;-)

[PATCH 19/35] objtool: Skip static assert when KCOV/KASAN

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-objtool-no-assert.patch --]
[-- Type: text/plain, Size: 2057 bytes --]

They make an absolutely horrid mess of things.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 scripts/Makefile.build        |    9 +++++++++
 tools/objtool/builtin-check.c |    3 ++-
 tools/objtool/builtin.h       |    2 +-
 tools/objtool/check.c         |    3 +++
 4 files changed, 15 insertions(+), 2 deletions(-)

--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -274,6 +274,15 @@ ifneq ($(RETPOLINE_CFLAGS),)
   objtool_args += --retpoline
 endif
 endif
+ifdef CONFIG_KCOV
+  objtool_no_assert := 1
+endif
+ifdef CONFIG_KASAN
+  objtool_no_assert := 1
+endif
+ifdef objtool_no_assert
+  objtool_args += --no-static-assert
+endif
 
 
 ifdef CONFIG_MODVERSIONS
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -29,7 +29,7 @@
 #include "builtin.h"
 #include "check.h"
 
-bool no_fp, no_unreachable, retpoline;
+bool no_fp, no_unreachable, retpoline, no_assert;
 
 static const char * const check_usage[] = {
 	"objtool check [<options>] file.o",
@@ -40,6 +40,7 @@ const struct option check_options[] = {
 	OPT_BOOLEAN('f', "no-fp", &no_fp, "Skip frame pointer validation"),
 	OPT_BOOLEAN('u', "no-unreachable", &no_unreachable, "Skip 'unreachable instruction' warnings"),
 	OPT_BOOLEAN('r', "retpoline", &retpoline, "Validate retpoline assumptions"),
+	OPT_BOOLEAN('s', "no-static-assert", &no_assert, "Skip static branch validation"),
 	OPT_END(),
 };
 
--- a/tools/objtool/builtin.h
+++ b/tools/objtool/builtin.h
@@ -20,7 +20,7 @@
 #include <subcmd/parse-options.h>
 
 extern const struct option check_options[];
-extern bool no_fp, no_unreachable, retpoline;
+extern bool no_fp, no_unreachable, retpoline, no_assert;
 
 extern int cmd_check(int argc, const char **argv);
 extern int cmd_orc(int argc, const char **argv);
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1122,6 +1122,9 @@ static int assert_static_jumps(struct ob
 	struct rela *rela;
 	int i;
 
+	if (no_assert)
+		return 0;
+
 	sec = find_section_by_name(file->elf, ".discard.jump_assert");
 	if (!sec)
 		return 0;

[PATCH 20/35] x86: Force asm-goto

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-x86-force-asm-goto.patch --]
[-- Type: text/plain, Size: 1867 bytes --]

Now that we have objtool to validate the correctness of asm-goto
constructs we can start using it to guarantee the absence of dynamic
branches (and thus speculation).

A primary prerequisite for this is of course that the compiler
supports asm-goto. This effecively lifts the minimum GCC version to
build an x86 kernel to gcc-4.5.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 Makefile          |   13 +++++++------
 arch/x86/Makefile |    4 ++++
 2 files changed, 11 insertions(+), 6 deletions(-)

--- a/Makefile
+++ b/Makefile
@@ -513,6 +513,13 @@ ifneq ($(filter install,$(MAKECMDGOALS))
         endif
 endif
 
+# check for 'asm goto'
+ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $(KBUILD_CFLAGS)), y)
+  CC_HAVE_ASM_GOTO := 1
+  KBUILD_CFLAGS += -DCC_HAVE_ASM_GOTO
+  KBUILD_AFLAGS += -DCC_HAVE_ASM_GOTO
+endif
+
 ifeq ($(mixed-targets),1)
 # ===========================================================================
 # We're called with mixed targets (*config and build targets).
@@ -652,12 +659,6 @@ KBUILD_CFLAGS += $(call cc-ifversion, -l
 # Tell gcc to never replace conditional load with a non-conditional one
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
 
-# check for 'asm goto'
-ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $(KBUILD_CFLAGS)), y)
-	KBUILD_CFLAGS += -DCC_HAVE_ASM_GOTO
-	KBUILD_AFLAGS += -DCC_HAVE_ASM_GOTO
-endif
-
 include scripts/Makefile.gcc-plugins
 
 ifdef CONFIG_READABLE_ASM
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -186,6 +186,10 @@ ifdef CONFIG_FUNCTION_GRAPH_TRACER
   endif
 endif
 
+ifndef CC_HAVE_ASM_GOTO
+  $(error Compiler lacks asm-goto support.)
+endif
+
 #
 # Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent a
 # GCC bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226).  There's no way

Re: [PATCH 20/35] x86: Force asm-goto

[David Woodhouse]

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

On Thu, 2018-01-18 at 14:48 +0100, Peter Zijlstra wrote:
> Now that we have objtool to validate the correctness of asm-goto
> constructs we can start using it to guarantee the absence of dynamic
> branches (and thus speculation).
> 
> A primary prerequisite for this is of course that the compiler
> supports asm-goto. This effecively lifts the minimum GCC version to
> build an x86 kernel to gcc-4.5.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

This is cute, but it's the wrong way round. We *know* people will be
immediately backporting this to every stable kernel they're still
using.

Let's put it in using alternatives (or the 'else lfence' version,
even), and then clean it up with static_cpu_has() on *top* of that, so
that the latter doesn't have to be backported.

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

[PATCH 21/35] x86: Remove FAST_FEATURE_TESTS

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-x86-remove-fast-feature.patch --]
[-- Type: text/plain, Size: 1861 bytes --]

Since we want to rely on static branches to avoid speculation, remove
any possible fallback code for static_cpu_has.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/Kconfig                  |   11 -----------
 arch/x86/include/asm/cpufeature.h |    8 --------
 2 files changed, 19 deletions(-)

--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -389,17 +389,6 @@ config X86_FEATURE_NAMES
 
 	  If in doubt, say Y.
 
-config X86_FAST_FEATURE_TESTS
-	bool "Fast CPU feature tests" if EMBEDDED
-	default y
-	---help---
-	  Some fast-paths in the kernel depend on the capabilities of the CPU.
-	  Say Y here for the kernel to patch in the appropriate code at runtime
-	  based on the capabilities of the CPU. The infrastructure for patching
-	  code at runtime takes up some additional space; space-constrained
-	  embedded systems may wish to say N here to produce smaller, slightly
-	  slower code.
-
 config X86_X2APIC
 	bool "Support x2apic"
 	depends on X86_LOCAL_APIC && X86_64 && (IRQ_REMAP || HYPERVISOR_GUEST)
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -137,7 +137,6 @@ extern void clear_cpu_cap(struct cpuinfo
 
 #define setup_force_cpu_bug(bit) setup_force_cpu_cap(bit)
 
-#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_X86_FAST_FEATURE_TESTS)
 /*
  * Static testing of CPU features.  Used the same as boot_cpu_has().
  * These will statically patch the target code for additional
@@ -196,13 +195,6 @@ static __always_inline __pure bool _stat
 		boot_cpu_has(bit) :				\
 		_static_cpu_has(bit)				\
 )
-#else
-/*
- * Fall back to dynamic for gcc versions which don't support asm goto. Should be
- * a minority now anyway.
- */
-#define static_cpu_has(bit)		boot_cpu_has(bit)
-#endif
 
 #define cpu_has_bug(c, bit)		cpu_has(c, (bit))
 #define set_cpu_bug(c, bit)		set_cpu_cap(c, (bit))

[PATCH 22/35] x86/cpufeatures: Detect Speculation control feature

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: x86-feature--Detect_the_x86_IBRS_feature_to_control_Speculation.patch --]
[-- Type: text/plain, Size: 3618 bytes --]

From: Tim Chen <tim.c.chen@linux.intel.com>

CPUs can expose a MSR to control speculation. The initial function of this
MSR is to control Indirect Branch Speculation, which is required to
mitigate the Spectre_V2 attack on certain CPU generations.

If CPUID(7).RDX[26] is set then MSR_IA32_SPEC_CTRL (0x48) is available and
bit 0 of that MSR controls whether Indirect Branch Speculation is
restricted or not. The control bit is named IBRS (Indirect Branch
Restricted Speculation). The IBSR bit can be unconditionally set to 1
without clearing it before.

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.

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

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

Enabling IBRS can cause a measurable and depending on the workload
significant CPU performance penalty.

[ tglx: Steam blastered changelog ]

Cc: Arjan Van De Ven <arjan.van.de.ven@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Greg KH <gregkh@linuxfoundation.org>
Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/cpufeatures.h |    2 ++
 arch/x86/include/asm/msr-index.h   |    4 ++++
 arch/x86/kernel/cpu/scattered.c    |    1 +
 3 files changed, 7 insertions(+)

--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -211,6 +211,8 @@
 
 #define X86_FEATURE_MBA			( 7*32+18) /* Memory Bandwidth Allocation */
 #define X86_FEATURE_RSB_CTXSW		( 7*32+19) /* Fill RSB on context switches */
+#define X86_FEATURE_SPEC_CTRL		( 7*32+20) /* Speculation Control */
+#define X86_FEATURE_IBRS		( 7*32+21) /* Indirect Branch Restricted Speculation */
 
 /* Virtualization flags: Linux defined, word 8 */
 #define X86_FEATURE_TPR_SHADOW		( 8*32+ 0) /* Intel TPR Shadow */
--- 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
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -23,6 +23,7 @@ static const struct cpuid_bit cpuid_bits
 	{ X86_FEATURE_EPB,		CPUID_ECX,  3, 0x00000006, 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 },

[PATCH 23/35] x86/speculation: Add basic speculation control code

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: x86-speculation--Add-basic-speculation-control-code.patch --]
[-- Type: text/plain, Size: 5641 bytes --]

From: Thomas Gleixner <tglx@linutronix.de>

Add the minimal infrastructure to control the speculation control feature.

 - Integrate it into the spectre_v2 coammand line parser and the mitigation
   selector function. The conditional selector function is a placeholder
   right now, which needs to be expanded with CPU specific decision
   functions.

 - Provide a static key for the actual code control.

 - Provide a init function which is called after jump label patching is
   functional.

 - Provide an interface for the late micro code loader to allow late
   discovery of the IBRS support. Not yet functional.

[peterz: fixed Makefile]

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 Documentation/admin-guide/kernel-parameters.txt |    1 
 arch/x86/include/asm/nospec-branch.h            |    5 +++
 arch/x86/kernel/cpu/Makefile                    |    1 
 arch/x86/kernel/cpu/bugs.c                      |   26 +++++++++++++++++-
 arch/x86/kernel/cpu/specctrl.c                  |   33 ++++++++++++++++++++++++
 5 files changed, 64 insertions(+), 2 deletions(-)

--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3932,6 +3932,7 @@
 			retpoline	  - replace indirect branches
 			retpoline,generic - google's original retpoline
 			retpoline,amd     - AMD-specific minimal thunk
+			ibrs		  - Intel: Indirect Branch Restricted Speculation
 
 			Not specifying this option is equivalent to
 			spectre_v2=auto.
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -214,5 +214,10 @@ static inline void vmexit_fill_RSB(void)
 		      : "r" (loops) : "memory" );
 #endif
 }
+
+bool specctrl_force_enable_ibrs(void);
+bool specctrl_cond_enable_ibrs(bool full_retpoline);
+bool is_skylake_era(void);
+
 #endif /* __ASSEMBLY__ */
 #endif /* __NOSPEC_BRANCH_H__ */
--- 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			+= specctrl.o
 
 obj-$(CONFIG_PROC_FS)	+= proc.o
 obj-$(CONFIG_X86_FEATURE_NAMES) += capflags.o powerflags.o
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -79,6 +79,7 @@ enum spectre_v2_mitigation_cmd {
 	SPECTRE_V2_CMD_RETPOLINE,
 	SPECTRE_V2_CMD_RETPOLINE_GENERIC,
 	SPECTRE_V2_CMD_RETPOLINE_AMD,
+	SPECTRE_V2_CMD_IBRS,
 };
 
 static const char *spectre_v2_strings[] = {
@@ -87,6 +88,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]			= "Mitigation: Indirect Branch Restricted Speculation",
 };
 
 #undef pr_fmt
@@ -144,6 +146,8 @@ static enum spectre_v2_mitigation_cmd __
 		} else if (match_option(arg, ret, "retpoline,generic")) {
 			spec2_print_if_insecure("generic retpoline selected on command line.");
 			return SPECTRE_V2_CMD_RETPOLINE_GENERIC;
+		} else if (match_option(arg, ret, "ibrs")) {
+			return SPECTRE_V2_CMD_IBRS;
 		} else if (match_option(arg, ret, "auto")) {
 			return SPECTRE_V2_CMD_AUTO;
 		}
@@ -156,8 +160,8 @@ static enum spectre_v2_mitigation_cmd __
 	return SPECTRE_V2_CMD_NONE;
 }
 
-/* Check for Skylake-like CPUs (for RSB handling) */
-static bool __init is_skylake_era(void)
+/* Check for Skylake-like CPUs (for RSB and IBRS handling) */
+bool __init is_skylake_era(void)
 {
 	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
 	    boot_cpu_data.x86 == 6) {
@@ -175,6 +179,7 @@ static bool __init is_skylake_era(void)
 
 static void __init spectre_v2_select_mitigation(void)
 {
+	bool full_retpoline = IS_ENABLED(CONFIG_RETPOLINE) && retp_compiler();
 	enum spectre_v2_mitigation_cmd cmd = spectre_v2_parse_cmdline();
 	enum spectre_v2_mitigation mode = SPECTRE_V2_NONE;
 
@@ -190,9 +195,25 @@ static void __init spectre_v2_select_mit
 	case SPECTRE_V2_CMD_NONE:
 		return;
 
+	case SPECTRE_V2_CMD_IBRS:
+		/* Command line requested IBRS. Try to enable it */
+		if (specctrl_force_enable_ibrs()) {
+			mode = SPECTRE_V2_IBRS;
+			goto set_mode;
+		}
+		/* FALLTRHU */
+
 	case SPECTRE_V2_CMD_FORCE:
 		/* FALLTRHU */
 	case SPECTRE_V2_CMD_AUTO:
+		/*
+		 * Check whether the CPU prefers to have IBRS or IBRS is
+		 * the only available mitigation.
+		 */
+		if (specctrl_cond_enable_ibrs(full_retpoline)) {
+			mode = SPECTRE_V2_IBRS;
+			goto set_mode;
+		}
 		goto retpoline_auto;
 
 	case SPECTRE_V2_CMD_RETPOLINE_AMD:
@@ -229,6 +250,7 @@ static void __init spectre_v2_select_mit
 		setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
 	}
 
+set_mode:
 	spectre_v2_enabled = mode;
 	pr_info("%s\n", spectre_v2_strings[mode]);
 
--- /dev/null
+++ b/arch/x86/kernel/cpu/specctrl.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
+#include <asm/nospec-branch.h>
+
+static inline void specctrl_enable_ibrs(void)
+{
+	setup_force_cpu_cap(X86_FEATURE_IBRS);
+}
+
+bool __init specctrl_force_enable_ibrs(void)
+{
+	if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL))
+		return false;
+	specctrl_enable_ibrs();
+	return true;
+}
+
+bool __init specctrl_cond_enable_ibrs(bool full_retpoline)
+{
+	if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL))
+		return false;
+	/*
+	 * IBRS is only required by SKL or as fallback if retpoline is not
+	 * fully supported.
+	 */
+	if (!is_skylake_era() && full_retpoline)
+		return false;
+
+	specctrl_enable_ibrs();
+	return true;
+}

Re: [PATCH 23/35] x86/speculation: Add basic speculation control code

[Josh Poimboeuf]

On Thu, Jan 18, 2018 at 02:48:23PM +0100, Peter Zijlstra wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Add the minimal infrastructure to control the speculation control feature.
> 
>  - Integrate it into the spectre_v2 coammand line parser and the mitigation
>    selector function. The conditional selector function is a placeholder
>    right now, which needs to be expanded with CPU specific decision
>    functions.
> 
>  - Provide a static key for the actual code control.
> 
>  - Provide a init function which is called after jump label patching is
>    functional.
> 
>  - Provide an interface for the late micro code loader to allow late
>    discovery of the IBRS support. Not yet functional.
> 
> [peterz: fixed Makefile]
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |    1 
>  arch/x86/include/asm/nospec-branch.h            |    5 +++
>  arch/x86/kernel/cpu/Makefile                    |    1 
>  arch/x86/kernel/cpu/bugs.c                      |   26 +++++++++++++++++-
>  arch/x86/kernel/cpu/specctrl.c                  |   33 ++++++++++++++++++++++++
>  5 files changed, 64 insertions(+), 2 deletions(-)
> 
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3932,6 +3932,7 @@
>  			retpoline	  - replace indirect branches
>  			retpoline,generic - google's original retpoline
>  			retpoline,amd     - AMD-specific minimal thunk
> +			ibrs		  - Intel: Indirect Branch Restricted Speculation

Are there plans to add spectre_v2=ibrs_always to prevent SMT-based
attacks?

> --- /dev/null
> +++ b/arch/x86/kernel/cpu/specctrl.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <asm/cpufeature.h>
> +#include <asm/cpufeatures.h>
> +#include <asm/nospec-branch.h>
> +
> +static inline void specctrl_enable_ibrs(void)
> +{
> +	setup_force_cpu_cap(X86_FEATURE_IBRS);
> +}

"spec_ctrl" seems much more readable than specctrl (for both function
and file names).  And also more consistent with the SPEC_CTRL MSR and
FEATURE names.

-- 
Josh

Re: [PATCH 23/35] x86/speculation: Add basic speculation control code

[Dave Hansen]

On 01/18/2018 08:37 AM, Josh Poimboeuf wrote:
>>
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -3932,6 +3932,7 @@
>>  			retpoline	  - replace indirect branches
>>  			retpoline,generic - google's original retpoline
>>  			retpoline,amd     - AMD-specific minimal thunk
>> +			ibrs		  - Intel: Indirect Branch Restricted Speculation
> Are there plans to add spectre_v2=ibrs_always to prevent SMT-based
> attacks?

What does "ibrs_always" mean to you?

There is a second bit in the MSR (STIBP) that is intended to keep
hyperthreads from influencing each-other.  That is behavior is implicit
when IBRS is enabled.

I think ibrs_always *should* probably be kept to refer to the future
CPUs that can safely leave IBRS enabled all the time.

Re: [PATCH 23/35] x86/speculation: Add basic speculation control code

[Paolo Bonzini]

On 18/01/2018 18:08, Dave Hansen wrote:
> On 01/18/2018 08:37 AM, Josh Poimboeuf wrote:
>>>
>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>> @@ -3932,6 +3932,7 @@
>>>  			retpoline	  - replace indirect branches
>>>  			retpoline,generic - google's original retpoline
>>>  			retpoline,amd     - AMD-specific minimal thunk
>>> +			ibrs		  - Intel: Indirect Branch Restricted Speculation
>> Are there plans to add spectre_v2=ibrs_always to prevent SMT-based
>> attacks?
> 
> What does "ibrs_always" mean to you?
> 
> There is a second bit in the MSR (STIBP) that is intended to keep
> hyperthreads from influencing each-other.  That is behavior is implicit
> when IBRS is enabled.

Yeah, I think we should have a mode to always leave that enabled, or
always set IBRS=1.

> I think ibrs_always *should* probably be kept to refer to the future
> CPUs that can safely leave IBRS enabled all the time.

Is that "safely" or "without throwing performance down the drain"?

Does "always IBRS=1" *hinder* the mitigation on existing processor, as
long as you reset IBRS=1 on kernel entry and vmexit?  Or is it just slow?

Paolo

Re: [PATCH 23/35] x86/speculation: Add basic speculation control code

[Josh Poimboeuf]

On Thu, Jan 18, 2018 at 06:12:36PM +0100, Paolo Bonzini wrote:
> On 18/01/2018 18:08, Dave Hansen wrote:
> > On 01/18/2018 08:37 AM, Josh Poimboeuf wrote:
> >>>
> >>> --- a/Documentation/admin-guide/kernel-parameters.txt
> >>> +++ b/Documentation/admin-guide/kernel-parameters.txt
> >>> @@ -3932,6 +3932,7 @@
> >>>  			retpoline	  - replace indirect branches
> >>>  			retpoline,generic - google's original retpoline
> >>>  			retpoline,amd     - AMD-specific minimal thunk
> >>> +			ibrs		  - Intel: Indirect Branch Restricted Speculation
> >> Are there plans to add spectre_v2=ibrs_always to prevent SMT-based
> >> attacks?
> > 
> > What does "ibrs_always" mean to you?

Maybe ibrs_always isn't the best name.  Basically we need an option to
protect user-user attacks via SMT.

It could be implemented with IBRS=1, or STIBP, or as part of the
mythical IBRS_ATT.

Maybe a 'user_smt' option, which could be appended to existing
'retpoline' or 'ibrs' options?  Like spectre_v2=retpoline,user_smt or
spectre_v2=ibrs,user_smt?

> > There is a second bit in the MSR (STIBP) that is intended to keep
> > hyperthreads from influencing each-other.  That is behavior is implicit
> > when IBRS is enabled.

Does this bit exist yet?  I've never seen any patches for it.

> Yeah, I think we should have a mode to always leave that enabled, or
> always set IBRS=1.
> 
> > I think ibrs_always *should* probably be kept to refer to the future
> > CPUs that can safely leave IBRS enabled all the time.
> 
> Is that "safely" or "without throwing performance down the drain"?
> 
> Does "always IBRS=1" *hinder* the mitigation on existing processor, as
> long as you reset IBRS=1 on kernel entry and vmexit?  Or is it just slow?

Yes, enquiring minds want to know...

-- 
Josh

Re: [PATCH 23/35] x86/speculation: Add basic speculation control code

[Andrea Arcangeli]

On Thu, Jan 18, 2018 at 12:24:31PM -0600, Josh Poimboeuf wrote:
> On Thu, Jan 18, 2018 at 06:12:36PM +0100, Paolo Bonzini wrote:
> > On 18/01/2018 18:08, Dave Hansen wrote:
> > > On 01/18/2018 08:37 AM, Josh Poimboeuf wrote:
> > >>>
> > >>> --- a/Documentation/admin-guide/kernel-parameters.txt
> > >>> +++ b/Documentation/admin-guide/kernel-parameters.txt
> > >>> @@ -3932,6 +3932,7 @@
> > >>>  			retpoline	  - replace indirect branches
> > >>>  			retpoline,generic - google's original retpoline
> > >>>  			retpoline,amd     - AMD-specific minimal thunk
> > >>> +			ibrs		  - Intel: Indirect Branch Restricted Speculation
> > >> Are there plans to add spectre_v2=ibrs_always to prevent SMT-based
> > >> attacks?
> > > 
> > > What does "ibrs_always" mean to you?
> 
> Maybe ibrs_always isn't the best name.  Basically we need an option to
> protect user-user attacks via SMT.
> 
> It could be implemented with IBRS=1, or STIBP, or as part of the
> mythical IBRS_ATT.

User stibp or user ibrs would be different things, both would be valid
for different use cases, and the user stibp should perform better.

Leaving ibrs on when returning from kernel to userland (or setting
ibrs if kernel used retpolines instead of ibrs) achieves stronger
semantics than just setting SPEC_CTRL with stibp when returning to
userland.

That is true no matter if kernel is using retpolines or ibrs.

IBRS is semantically equivalent to "STIBP; IBPB", so user_ibrs is
always inclusive of user_stibp.

Said that the CPU should better achieve such semantics without really
internally issuing an IBPB of course, but you can think at the current
IBRS as "STIBP; IBPB". That IBPB immediately after the STIBP makes a
difference to the non HT attacks possible on host userland.

user_smt wouldn't solve all cases that user_ibrs solves, but it'd be
ideal if critical user apps are built with retpolines and the only
concern left is a HT/SMT attack on those only need to care about
HT/SMT.

To begin with, user_ibrs would be more important than user_stibp.

On a side note: stibp isn't always available, it requires a new cpuid
check on bit 27 too, you can still write to it but it won't #gp, on
some CPUs it's simply implicit and you can write to it, but it's a
noop. I haven't figured exactly to differentiate when it's disabled or
implicitly enabled when not enumerated in cpuid.

Re: [PATCH 23/35] x86/speculation: Add basic speculation control code

[Andy Lutomirski]

On Thu, Jan 18, 2018 at 11:08 AM, Andrea Arcangeli <aarcange@redhat.com> wrote:
> On Thu, Jan 18, 2018 at 12:24:31PM -0600, Josh Poimboeuf wrote:
>> On Thu, Jan 18, 2018 at 06:12:36PM +0100, Paolo Bonzini wrote:
>> > On 18/01/2018 18:08, Dave Hansen wrote:
>> > > On 01/18/2018 08:37 AM, Josh Poimboeuf wrote:
>> > >>>
>> > >>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> > >>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> > >>> @@ -3932,6 +3932,7 @@
>> > >>>                         retpoline         - replace indirect branches
>> > >>>                         retpoline,generic - google's original retpoline
>> > >>>                         retpoline,amd     - AMD-specific minimal thunk
>> > >>> +                       ibrs              - Intel: Indirect Branch Restricted Speculation
>> > >> Are there plans to add spectre_v2=ibrs_always to prevent SMT-based
>> > >> attacks?
>> > >
>> > > What does "ibrs_always" mean to you?
>>
>> Maybe ibrs_always isn't the best name.  Basically we need an option to
>> protect user-user attacks via SMT.
>>
>> It could be implemented with IBRS=1, or STIBP, or as part of the
>> mythical IBRS_ATT.
>
> User stibp or user ibrs would be different things, both would be valid
> for different use cases, and the user stibp should perform better.
>
> Leaving ibrs on when returning from kernel to userland (or setting
> ibrs if kernel used retpolines instead of ibrs) achieves stronger
> semantics than just setting SPEC_CTRL with stibp when returning to
> userland.

I read the whitepaper that documented the new MSRs a couple days ago
and I'm now completely unable to find it.  If anyone could send the
link, that would be great.

>From memory, however, the docs were quite clear that setting leaving
IBRS set when entering user mode or guest mode does not guarantee any
particular protection unless an additional CPUID bit (the name of
which I forget) is set, and that current CPUs will *not* get that bit
set by microcode update.  IOW the protection given is that, if you set
IBRS bit zero after entry to kernel mode, you are protected until you
re-enter user mode.  When you're in user mode, you're not protected.
When you return back to kernel mode, you *still* aren't protected no
matter what value is in the MSR until you write 1 again.

>
> That is true no matter if kernel is using retpolines or ibrs.
>
> IBRS is semantically equivalent to "STIBP; IBPB", so user_ibrs is
> always inclusive of user_stibp.

Are you quite sure?  I had the impression that IBPB was much slower
than writing 1 to IBRS and that writing 1 to IBRS has much more
limited effects.

Re: [PATCH 23/35] x86/speculation: Add basic speculation control code

[Andrew Cooper]

On 18/01/2018 23:25, Andy Lutomirski wrote:
> On Thu, Jan 18, 2018 at 11:08 AM, Andrea Arcangeli <aarcange@redhat.com> wrote:
>> On Thu, Jan 18, 2018 at 12:24:31PM -0600, Josh Poimboeuf wrote:
>>> On Thu, Jan 18, 2018 at 06:12:36PM +0100, Paolo Bonzini wrote:
>>>> On 18/01/2018 18:08, Dave Hansen wrote:
>>>>> On 01/18/2018 08:37 AM, Josh Poimboeuf wrote:
>>>>>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>>>>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>>>>>> @@ -3932,6 +3932,7 @@
>>>>>>>                         retpoline         - replace indirect branches
>>>>>>>                         retpoline,generic - google's original retpoline
>>>>>>>                         retpoline,amd     - AMD-specific minimal thunk
>>>>>>> +                       ibrs              - Intel: Indirect Branch Restricted Speculation
>>>>>> Are there plans to add spectre_v2=ibrs_always to prevent SMT-based
>>>>>> attacks?
>>>>> What does "ibrs_always" mean to you?
>>> Maybe ibrs_always isn't the best name.  Basically we need an option to
>>> protect user-user attacks via SMT.
>>>
>>> It could be implemented with IBRS=1, or STIBP, or as part of the
>>> mythical IBRS_ATT.
>> User stibp or user ibrs would be different things, both would be valid
>> for different use cases, and the user stibp should perform better.
>>
>> Leaving ibrs on when returning from kernel to userland (or setting
>> ibrs if kernel used retpolines instead of ibrs) achieves stronger
>> semantics than just setting SPEC_CTRL with stibp when returning to
>> userland.
> I read the whitepaper that documented the new MSRs a couple days ago
> and I'm now completely unable to find it.  If anyone could send the
> link, that would be great.

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

~Andrew

Re: [PATCH 23/35] x86/speculation: Add basic speculation control code

[Andrea Arcangeli]

Hello,

On Thu, Jan 18, 2018 at 03:25:25PM -0800, Andy Lutomirski wrote:
> I read the whitepaper that documented the new MSRs a couple days ago
> and I'm now completely unable to find it.  If anyone could send the
> link, that would be great.

I see Andrew posted a link.

> From memory, however, the docs were quite clear that setting leaving
> IBRS set when entering user mode or guest mode does not guarantee any
> particular protection unless an additional CPUID bit (the name of
> which I forget) is set, and that current CPUs will *not* get that bit

My current understanding is that with SPEC_CTRL alone set in cpuid,
IBRS is meaningful, other bits don't matter.

> set by microcode update.  IOW the protection given is that, if you set
> IBRS bit zero after entry to kernel mode, you are protected until you
> re-enter user mode.  When you're in user mode, you're not protected.

If you leave IBRS set while in user mode, userland is protected as
strong as kernel mode.

Of course kernel can attack usermode through spectre variant#2 if it
wants :), but usermode has to trust the kernel to begin with, just
like guest mode has to trust the host kernel.

If you leave IBRS set, guest mode (including guest usermode) cannot
attack host userland, host userland cannot attack other host userland
and no HT is possible either.

Note guest kernel attack on host userland is not as concerning but
it's possible too, as long as you use host dm-crypt instead of qcow2
encryption on host, it's not a major concern. guest userland attack on
host userland is more of a concern because if it can be mounted
successfully, it would dump all guest memory making it impossible for
the guest to defend itself from its own userland (unless it uses ibpb
2 of course which calls IBPB at guest user to kernel entry instead of
IBRS). IBRS isn't enough in guest because like retpoline it doesn't
guarantee an IBPB.

Because these are only theoretical and there's no PoC IBRS is not
enabled by default in userland of course. However retpolining qemu
would be a low overhead sure solution to this that would avoid having
to set ibrs in userland to achieve the same.

> When you return back to kernel mode, you *still* aren't protected no
> matter what value is in the MSR until you write 1 again.

When you return to kernel mode you've to call IBRS again even if it
was left set, because there's a higher-privilege mode change. That's
equivalent to calling only IBPB and leaving STIBP set (only way to
understand the locations where IBRS has to be set is to imagine IBRS
as a strict "STIBP; IBPB").

specs are very explicit that it's very meaningful to set IBRS even if
already set.

> > That is true no matter if kernel is using retpolines or ibrs.
> >
> > IBRS is semantically equivalent to "STIBP; IBPB", so user_ibrs is
> > always inclusive of user_stibp.
> 
> Are you quite sure?  I had the impression that IBPB was much slower

Nothing in the specs says that IBRS is a "STIBP; IBPB" equivalent, but
if you want to find where to set IBRS, yes, I'm quite sure, you've to
think it like it.

> than writing 1 to IBRS and that writing 1 to IBRS has much more
> limited effects.

I think I already partly answered it already in the sentence that
followed the one you quoted, but I should elaborate it.

The semantics to use depending what you're trying to solve because it
can have both.

"STIBP; IBPB" could be called the coarse semantics and you have to
imagine IBRS as "STIBP; IBPB" whenever you are checking where to write
IBRS, or you risk missing places where you have to write IBRS even if
it is already set.

As opposed when you check when you need to leave IBRS set (note: after
it was already set in all places found with the coarse semantics),
or where you need to call IBPB, you can't rely on the coarse
semantics because of course the IBPB may not have really
happened... and so you've to imagine IBRS with finegrined semantics as
"temporary immunization from the poison including HT/SMT poison and
all poison is still left in the CPU and will return to affect the
runtime as soon as IBRS is cleared".

IBRS Q/A:

1) When to write IBRS in SPEC_CTRL? -> imagine it as "STIBP; IBPB"

2) When to leave IBRS set or when to call IBPB -> imagine the previous
setting of IBRS as temporarily disabling indirect branch prediction
without an IBPB implicit in IBRS

If you think it only like 1) you risk missing some places where you've
to write IBRS even if it was already set.

If you think it only like 2) you risk clearing it too early or you
risk missing a necessary IBPB.

It has to be thought simultaneously in both ways.

The sure thing I get from the specs is IBRS always implies STIBP (even
when STIBP is a noop), specs are pretty explicit about that.

So following the above two rules, assume you've retpolined host kernel
and you want to protect host userland from guest userland, IBRS set in
the host kernel to host user transition (user_ibrs) will achieve it
fine. Setting STIBP in the kernel (user_stibp) to user transition
won't help at all with this scenario.

If the kernel in host was using IBRS instead of retpolines, it already
had to set IBRS in vmexit to satisfy the coarse semantics and it would
just need to leave IBRS set in the kernel to user transition (it would
then need to set IBRS again in the user to kernel transition that
follows even if it was already set).

Thanks,
Andrea

Re: [PATCH 23/35] x86/speculation: Add basic speculation control code

[Andy Lutomirski]

On Thu, Jan 18, 2018 at 5:41 PM, Andrea Arcangeli <aarcange@redhat.com> wrote:
> Hello,
>
> On Thu, Jan 18, 2018 at 03:25:25PM -0800, Andy Lutomirski wrote:
>> I read the whitepaper that documented the new MSRs a couple days ago
>> and I'm now completely unable to find it.  If anyone could send the
>> link, that would be great.
>
> I see Andrew posted a link.
>
>> From memory, however, the docs were quite clear that setting leaving
>> IBRS set when entering user mode or guest mode does not guarantee any
>> particular protection unless an additional CPUID bit (the name of
>> which I forget) is set, and that current CPUs will *not* get that bit
>
> My current understanding is that with SPEC_CTRL alone set in cpuid,
> IBRS is meaningful, other bits don't matter.
>
>> set by microcode update.  IOW the protection given is that, if you set
>> IBRS bit zero after entry to kernel mode, you are protected until you
>> re-enter user mode.  When you're in user mode, you're not protected.
>
> If you leave IBRS set while in user mode, userland is protected as
> strong as kernel mode.

The link that Andrew posted says:

If software sets IA32_SPEC_CTRL.IBRS to 1 after a transition to a more
privileged predictor mode, predicted targets of indirect branches
executed in that predictor mode with IA32_SPEC_CTRL.IBRS = 1 cannot be
controlled by software that was executed in a less privileged
predictor mode or on another logical processor.

...

Enabling IBRS does not prevent software from controlling the predicted
targets of indirect branches of unrelated software executed later at
the same predictor mode (for example, between two different user
applications, or two different virtual machines). Such isolation can
be ensured through use of the IBPB command, described in Section
2.5.3, “Indirect Branch Predictor Barrier (IBPB)”.

So maybe it's poorly written, but I see nothing in that language that
suggests that IBRS=1 (on a CPU without enhanced IBRS) provides any
guarantees at all about who can or cannot control speculation of
indirect branches in user mode.

>
> When you return to kernel mode you've to call IBRS again even if it
> was left set, because there's a higher-privilege mode change. That's
> equivalent to calling only IBPB and leaving STIBP set (only way to
> understand the locations where IBRS has to be set is to imagine IBRS
> as a strict "STIBP; IBPB").

Then Intel should improve their spec to say so.

> IBRS Q/A:
>
> 1) When to write IBRS in SPEC_CTRL? -> imagine it as "STIBP; IBPB"
>
> 2) When to leave IBRS set or when to call IBPB -> imagine the previous
> setting of IBRS as temporarily disabling indirect branch prediction
> without an IBPB implicit in IBRS
>
> If you think it only like 1) you risk missing some places where you've
> to write IBRS even if it was already set.
>
> If you think it only like 2) you risk clearing it too early or you
> risk missing a necessary IBPB.
>
> It has to be thought simultaneously in both ways.

>From your description, it sounds like what's actually happening is:

When IBRS is enabled, indirect branch speculation targets cannot be
controlled by code that executed on a different logical processor on
by code that executed prior to the most recent time that IBRS was set
to 1.  Additionally, if the CPU supports enhanced IBRS, then indirect
branch speculation targets cannot be controlled by code that executed
at a less privileged predictor mode.

Is that actually correct?  If so, could Intel please *say* so?
Because writing voodoo magic security code seriously sucks.

>
> The sure thing I get from the specs is IBRS always implies STIBP (even
> when STIBP is a noop), specs are pretty explicit about that.

Ah, it says:

As noted in Section 2.5.1, “Indirect Branch Restricted Speculation
(IBRS)”, enabling IBRS prevents software operating on one logical
processor from controlling the predicted targets of indirect branches
executed on another logical processor. For that reason, it is not
necessary to enable STIBP when IBRS is  enabled.

So I guess we can write 1 when we enter the kernel, but we probably
want to write 2 instead of 0 when we exit.

RE: [PATCH 23/35] x86/speculation: Add basic speculation control code

[Van De Ven, Arjan]

> Enabling IBRS does not prevent software from controlling the predicted
> targets of indirect branches of unrelated software executed later at
> the same predictor mode (for example, between two different user
> applications, or two different virtual machines). Such isolation can
> be ensured through use of the IBPB command, described in Section
> 2.5.3, “Indirect Branch Predictor Barrier (IBPB)”.
> 
> So maybe it's poorly written, but I see nothing in that language that
> suggests that IBRS=1 (on a CPU without enhanced IBRS) provides any
> guarantees at all about who can or cannot control speculation of
> indirect branches in user mode.

there is no such guarantee. Some of the IBRS implementations will actually flush rather than disable, or flush parts and disable other parts.

yes the wording is a bit cryptic, but it's also very explicit about what it covers (and the rest is not covered!) and had to allow a few different implementations unfortunately.

Re: [PATCH 23/35] x86/speculation: Add basic speculation control code

[Andrea Arcangeli]

On Fri, Jan 19, 2018 at 04:15:33AM +0000, Van De Ven, Arjan wrote:
> there is no such guarantee. Some of the IBRS implementations will
> actually flush rather than disable, or flush parts and disable other
> parts.

To me it helps in order to memorize the spec to understand why the
spec is the way it is.

I tried to help explaining some of that, but I notice that I created
more confusion... I never intended IBPB can be skipped in user to user
switches if leaving IBRS set in userland, that's not what we do and it
wouldn't be ok with certain smarter CPUs.

> yes the wording is a bit cryptic, but it's also very explicit about
> what it covers (and the rest is not covered!) and had to allow a few
> different implementations unfortunately.

We already follow the spec to the letter and we only depend on what is
covered there.

Surely the specs already explain everything better than I could ever
do, so if anything wasn't clear in the two previous emails where I
failed to explain the difference between setting or leaving IBRS set
in userland (ibrs_user) and setting or leaving STIBP set in userland
(stibp_user) you'll find all answers in the very explicit spec per
above quote.

Thanks,
Andrea

[PATCH 24/35] x86/msr: Move native_*msr macros out of microcode.h

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: x86-msr--Move-native_msr-macros-out-of-microcode.h.patch --]
[-- Type: text/plain, Size: 1743 bytes --]

From: Thomas Gleixner <tglx@linutronix.de>

[peterz: added native_rdmsrl]

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/microcode.h |   14 --------------
 arch/x86/include/asm/msr.h       |   18 ++++++++++++++++++
 2 files changed, 18 insertions(+), 14 deletions(-)

--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -6,20 +6,6 @@
 #include <linux/earlycpio.h>
 #include <linux/initrd.h>
 
-#define native_rdmsr(msr, val1, val2)			\
-do {							\
-	u64 __val = __rdmsr((msr));			\
-	(void)((val1) = (u32)__val);			\
-	(void)((val2) = (u32)(__val >> 32));		\
-} while (0)
-
-#define native_wrmsr(msr, low, high)			\
-	__wrmsr(msr, low, high)
-
-#define native_wrmsrl(msr, val)				\
-	__wrmsr((msr), (u32)((u64)(val)),		\
-		       (u32)((u64)(val) >> 32))
-
 struct ucode_patch {
 	struct list_head plist;
 	void *data;		/* Intel uses only this one */
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -175,6 +175,24 @@ native_write_msr_safe(unsigned int msr,
 extern int rdmsr_safe_regs(u32 regs[8]);
 extern int wrmsr_safe_regs(u32 regs[8]);
 
+/* Simple wrappers for microcode and speculation control */
+#define native_rdmsr(msr, val1, val2)			\
+do {							\
+	u64 __val = __rdmsr((msr));			\
+	(void)((val1) = (u32)__val);			\
+	(void)((val2) = (u32)(__val >> 32));		\
+} while (0)
+
+#define native_rdmsrl(msr)				\
+	__rdmsr(msr)
+
+#define native_wrmsr(msr, low, high)			\
+	__wrmsr(msr, low, high)
+
+#define native_wrmsrl(msr, val)				\
+	__wrmsr((msr), (u32)((u64)(val)),		\
+		       (u32)((u64)(val) >> 32))
+
 /**
  * rdtsc() - returns the current TSC without ordering constraints
  *

[PATCH 25/35] x86/speculation: Add inlines to control Indirect Branch Speculation

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: x86-specctrl--Add-inlines-to-control-Indirect-Branch-Speculation.patch --]
[-- Type: text/plain, Size: 998 bytes --]

From: Thomas Gleixner <tglx@linutronix.de>

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/include/asm/nospec-branch.h |   12 ++++++++++++
 1 file changed, 12 insertions(+)

--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -6,6 +6,7 @@
 #include <asm/alternative.h>
 #include <asm/alternative-asm.h>
 #include <asm/cpufeatures.h>
+#include <asm/msr.h>
 
 /*
  * Fill the CPU return stack buffer.
@@ -219,5 +220,16 @@ bool specctrl_force_enable_ibrs(void);
 bool specctrl_cond_enable_ibrs(bool full_retpoline);
 bool is_skylake_era(void);
 
+static inline void stop_indirect_branch_speculation(void)
+{
+	if (static_cpu_has(X86_FEATURE_IBRS))
+		native_wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_ENABLE_IBRS);
+}
+
+static inline void restart_indirect_branch_speculation(void)
+{
+	if (static_cpu_has(X86_FEATURE_IBRS))
+		native_wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_DISABLE_IBRS);
+}
 #endif /* __ASSEMBLY__ */
 #endif /* __NOSPEC_BRANCH_H__ */

[PATCH 26/35] x86/enter: Create macros to stop/restart Indirect Branch Speculation

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: x86-enter--Create_macros_to_set-clear_IBRS.patch --]
[-- Type: text/plain, Size: 2877 bytes --]

From: Tim Chen <tim.c.chen@linux.intel.com>

Create macros to control Indirect Branch Speculation.

Name them so they reflect what they are actually doing. The Intel supplied
names are suggesting that they 'enable' something while in reality they
disable.

[ tglx: Changed macro names and rewrote changelog ]

Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Arjan Van De Ven <arjan.van.de.ven@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: Ashok Raj <ashok.raj@intel.com>
---
 arch/x86/entry/calling.h |   73 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

--- 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>
 
 /*
 
@@ -349,3 +351,74 @@ For 32-bit we have the following convent
 .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
+	wrmsr
+.endm
+
+.macro STOP_IB_SPEC
+	ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_IBRS
+	PUSH_MSR_REGS
+	WRMSR_ASM $MSR_IA32_SPEC_CTRL, $0, $SPEC_CTRL_ENABLE_IBRS
+	POP_MSR_REGS
+.Lskip_\@:
+.endm
+
+.macro RESTART_IB_SPEC
+	ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_IBRS
+	PUSH_MSR_REGS
+	WRMSR_ASM $MSR_IA32_SPEC_CTRL, $0, $SPEC_CTRL_DISABLE_IBRS
+	POP_MSR_REGS
+.Lskip_\@:
+.endm
+
+.macro STOP_IB_SPEC_CLOBBER
+	ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_IBRS
+	WRMSR_ASM $MSR_IA32_SPEC_CTRL, $0, $SPEC_CTRL_ENABLE_IBRS
+.Lskip_\@:
+.endm
+
+.macro RESTART_IB_SPEC_CLOBBER
+	ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_IBRS
+	WRMSR_ASM $MSR_IA32_SPEC_CTRL, $0, $SPEC_CTRL_DISABLE_IBRS
+.Lskip_\@:
+.endm
+
+.macro STOP_IB_SPEC_SAVE_AND_CLOBBER save_reg:req
+	ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_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_IB_SPEC_CLOBBER save_reg:req
+	ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_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

Re: [PATCH 26/35] x86/enter: Create macros to stop/restart Indirect Branch Speculation

[Tim Chen]

On Thu, Jan 18, 2018 at 02:48:26PM +0100, Peter Zijlstra wrote:
> From: Tim Chen <tim.c.chen@linux.intel.com>
> 
> +
> +.macro STOP_IB_SPEC
> +	ALTERNATIVE "jmp .Lskip_\@", "", X86_FEATURE_IBRS

Peter,

Will it be possible to use static key here like
	STATIC_JUMP_IF_FALSE .Lskip_\@, specctrl_ibrs, def=0

In Thomas' original patchset, there is a specctrl_ibrs static key.

That will make a switch of static branch here easier if we want
to change it later in boot or at run time.

Tim 

> +	PUSH_MSR_REGS
> +	WRMSR_ASM $MSR_IA32_SPEC_CTRL, $0, $SPEC_CTRL_ENABLE_IBRS
> +	POP_MSR_REGS
> +.Lskip_\@:
> +.endm
> +

[PATCH 27/35] x86/enter: Use IBRS on syscall and interrupts

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: x86-enter--Use_IBRS_on_syscall_and_interrupts.patch --]
[-- Type: text/plain, Size: 7948 bytes --]

From: Tim Chen <tim.c.chen@linux.intel.com>

Stop Indirect Branch Speculation on every user space to kernel space
transition and reenable it when returning to user space.

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>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Arjan Van De Ven <arjan.van.de.ven@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: Ashok Raj <ashok.raj@intel.com>
---
 arch/x86/entry/entry_64.S        |   35 ++++++++++++++++++++++++++++++++++-
 arch/x86/entry/entry_64_compat.S |   21 +++++++++++++++++++--
 2 files changed, 53 insertions(+), 3 deletions(-)

--- 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
+	/* Stop indirect branch speculation */
+	STOP_IB_SPEC
 
 	/* 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
+	/* Stop Indirect Branch Speculation */
+	STOP_IB_SPEC
 
 	TRACE_IRQS_OFF
 
@@ -409,6 +413,8 @@ GLOBAL(entry_SYSCALL_64_after_hwframe)
 	pushq	RSP-RDI(%rdi)	/* RSP */
 	pushq	(%rdi)		/* RDI */
 
+	/* Restart Indirect Branch Speculation */
+	RESTART_IB_SPEC
 	/*
 	 * We are on the trampoline stack.  All regs except RDI are live.
 	 * We can do future final exit work right here.
@@ -757,11 +763,12 @@ GLOBAL(swapgs_restore_regs_and_return_to
 	/* Push user RDI on the trampoline stack. */
 	pushq	(%rdi)
 
+	/* Restart Indirect Branch Speculation */
+	RESTART_IB_SPEC
 	/*
 	 * We are on the trampoline stack.  All regs except RDI are live.
 	 * We can do future final exit work right here.
 	 */
-
 	SWITCH_TO_USER_CR3_STACK scratch_reg=%rdi
 
 	/* Restore RDI. */
@@ -849,6 +856,13 @@ ENTRY(native_iret)
 	SWAPGS					/* to kernel GS */
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi	/* to kernel CR3 */
 
+	/*
+	 * There is no point in disabling Indirect Branch Speculation
+	 * here as this is going to return to user space immediately
+	 * after fixing ESPFIX stack.  There is no vulnerable code
+	 * to protect so spare two MSR writes.
+	 */
+
 	movq	PER_CPU_VAR(espfix_waddr), %rdi
 	movq	%rax, (0*8)(%rdi)		/* user RAX */
 	movq	(1*8)(%rsp), %rax		/* user RIP */
@@ -982,6 +996,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
+	/* Stop Indirect Branch Speculation */
+	STOP_IB_SPEC
 	UNWIND_HINT sp_offset=16 sp_reg=ORC_REG_DI
 
 	pushq	7*8(%rdi)		/* regs->ss */
@@ -1282,6 +1298,8 @@ ENTRY(paranoid_entry)
 
 1:
 	SAVE_AND_SWITCH_TO_KERNEL_CR3 scratch_reg=%rax save_reg=%r14
+	/* Stop Indirect Branch speculation */
+	STOP_IB_SPEC_SAVE_AND_CLOBBER save_reg=%r13d
 
 	ret
 END(paranoid_entry)
@@ -1305,6 +1323,8 @@ ENTRY(paranoid_exit)
 	testl	%ebx, %ebx			/* swapgs needed? */
 	jnz	.Lparanoid_exit_no_swapgs
 	TRACE_IRQS_IRETQ
+	/* Restore Indirect Branch Speculation to the previous state */
+	RESTORE_IB_SPEC_CLOBBER save_reg=%r13d
 	RESTORE_CR3	scratch_reg=%rbx save_reg=%r14
 	SWAPGS_UNSAFE_STACK
 	jmp	.Lparanoid_exit_restore
@@ -1335,6 +1355,8 @@ ENTRY(error_entry)
 	SWAPGS
 	/* We have user CR3.  Change to kernel CR3. */
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
+	/* Stop Indirect Branch Speculation */
+	STOP_IB_SPEC_CLOBBER
 
 .Lerror_entry_from_usermode_after_swapgs:
 	/* Put us onto the real thread stack. */
@@ -1382,6 +1404,8 @@ ENTRY(error_entry)
 	 */
 	SWAPGS
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
+	/* Stop Indirect Branch Speculation */
+	STOP_IB_SPEC_CLOBBER
 	jmp .Lerror_entry_done
 
 .Lbstep_iret:
@@ -1396,6 +1420,8 @@ ENTRY(error_entry)
 	 */
 	SWAPGS
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
+	/* Stop Indirect Branch Speculation */
+	STOP_IB_SPEC
 
 	/*
 	 * Pretend that the exception came from user mode: set up pt_regs
@@ -1497,6 +1523,10 @@ ENTRY(nmi)
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rdx
 	movq	%rsp, %rdx
 	movq	PER_CPU_VAR(cpu_current_top_of_stack), %rsp
+
+	/* Stop Indirect Branch Speculation */
+	STOP_IB_SPEC
+
 	UNWIND_HINT_IRET_REGS base=%rdx offset=8
 	pushq	5*8(%rdx)	/* pt_regs->ss */
 	pushq	4*8(%rdx)	/* pt_regs->rsp */
@@ -1747,6 +1777,9 @@ ENTRY(nmi)
 	movq	$-1, %rsi
 	call	do_nmi
 
+	/* Restore Indirect Branch speculation to the previous state */
+	RESTORE_IB_SPEC_CLOBBER save_reg=%r13d
+
 	RESTORE_CR3 scratch_reg=%r15 save_reg=%r14
 
 	testl	%ebx, %ebx			/* swapgs needed? */
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -54,6 +54,8 @@ ENTRY(entry_SYSENTER_compat)
 	SWITCH_TO_KERNEL_CR3 scratch_reg=%rsp
 
 	movq	PER_CPU_VAR(cpu_current_top_of_stack), %rsp
+	/* Stop Indirect Branch Speculation */
+	STOP_IB_SPEC
 
 	/*
 	 * User tracing code (ptrace or signal handlers) might assume that
@@ -224,12 +226,18 @@ GLOBAL(entry_SYSCALL_compat_after_hwfram
 	pushq   $0			/* pt_regs->r14 = 0 */
 	pushq   $0			/* pt_regs->r15 = 0 */
 
-	/*
-	 * User mode is traced as though IRQs are on, and SYSENTER
+	/* Stop Indirect Branch Speculation. All registers are saved already */
+	STOP_IB_SPEC_CLOBBER
+
+	/* User mode is traced as though IRQs are on, and SYSENTER
 	 * turned them off.
 	 */
 	TRACE_IRQS_OFF
 
+	/*
+	 * We just saved %rdi so it is safe to clobber.  It is not
+	 * preserved during the C calls inside TRACE_IRQS_OFF anyway.
+	 */
 	movq	%rsp, %rdi
 	call	do_fast_syscall_32
 	/* XEN PV guests always use IRET path */
@@ -239,6 +247,15 @@ GLOBAL(entry_SYSCALL_compat_after_hwfram
 	/* Opportunistic SYSRET */
 sysret32_from_system_call:
 	TRACE_IRQS_ON			/* User mode traces as IRQs on. */
+
+	/*
+	 * Restart Indirect Branch Speculation. This is safe to do here
+	 * because there are no indirect branches between here and the
+	 * return to userspace (sysretl).
+	 * Clobber of %rax, %rcx, %rdx is OK before register restoring.
+	 */
+	RESTART_IB_SPEC_CLOBBER
+
 	movq	RBX(%rsp), %rbx		/* pt_regs->rbx */
 	movq	RBP(%rsp), %rbp		/* pt_regs->rbp */
 	movq	EFLAGS(%rsp), %r11	/* pt_regs->flags (in r11) */

[PATCH 28/35] x86/idle: Control Indirect Branch Speculation in idle

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: x86-idle--Control-Indirect-Branch-Speculation-in-idle.patch --]
[-- Type: text/plain, Size: 2799 bytes --]

From: Thomas Gleixner <tglx@linutronix.de>

Indirect Branch Speculation (IBS) is controlled per physical core. If one
thread disables it then it's disabled for the core. If a thread enters idle
it makes sense to reenable IBS so the sibling thread can run with full
speculation enabled in user space.

This makes only sense in mwait_idle_with_hints() because mwait_idle() can
serve an interrupt immediately before speculation can be stopped again. SKL
which requires IBRS should use mwait_idle_with_hints() so this is a non
issue and in the worst case a missed optimization.

[peterz: fixed stop_indirect_branch_speculation placement]

Originally-by: Tim Chen <tim.c.chen@linux.intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/mwait.h |   14 ++++++++++++++
 arch/x86/kernel/process.c    |   14 ++++++++++++++
 2 files changed, 28 insertions(+)

--- 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/nospec-branch.h>
 
 #define MWAIT_SUBSTATE_MASK		0xf
 #define MWAIT_CSTATE_MASK		0xf
@@ -106,9 +107,22 @@ static inline void mwait_idle_with_hints
 			mb();
 		}
 
+		/*
+		 * Indirect Branch Speculation (IBS) is controlled per
+		 * physical core. If one thread disables it, then it's
+		 * disabled on all threads of the core. The kernel disables
+		 * it on entry from user space. Reenable it on the thread
+		 * which goes idle so the other thread has a chance to run
+		 * with full speculation enabled in userspace.
+		 */
+		restart_indirect_branch_speculation();
 		__monitor((void *)&current_thread_info()->flags, 0, 0);
 		if (!need_resched())
 			__mwait(eax, ecx);
+		/*
+		 * Stop IBS again to protect kernel execution.
+		 */
+		stop_indirect_branch_speculation();
 	}
 	current_clr_polling();
 }
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -461,6 +461,20 @@ static __cpuidle void mwait_idle(void)
 			mb(); /* quirk */
 		}
 
+		/*
+		 * Indirect Branch Speculation (IBS) is controlled per
+		 * physical core. If one thread disables it, then it's
+		 * disabled on all threads of the core. The kernel disables
+		 * it on entry from user space. For __sti_mwait() it's
+		 * wrong to reenable it because an interrupt can be served
+		 * before speculation can be stopped again.
+		 *
+		 * To plug that hole the interrupt entry code would need to
+		 * save current state and restore. Not worth the trouble as
+		 * SKL should not use mwait_idle(). It should use
+		 * mwait_idle_with_hints() which can do speculation control
+		 * safely.
+		 */
 		__monitor((void *)&current_thread_info()->flags, 0, 0);
 		if (!need_resched())
 			__sti_mwait(0, 0);

Re: [PATCH 28/35] x86/idle: Control Indirect Branch Speculation in idle

[Andrew Cooper]

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

On 18/01/18 13:48, Peter Zijlstra wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> Indirect Branch Speculation (IBS) is controlled per physical core. If one
> thread disables it then it's disabled for the core. If a thread enters idle
> it makes sense to reenable IBS so the sibling thread can run with full
> speculation enabled in user space.
>
> This makes only sense in mwait_idle_with_hints() because mwait_idle() can
> serve an interrupt immediately before speculation can be stopped again. SKL
> which requires IBRS should use mwait_idle_with_hints() so this is a non
> issue and in the worst case a missed optimization.
>
> [peterz: fixed stop_indirect_branch_speculation placement]
>
> Originally-by: Tim Chen <tim.c.chen@linux.intel.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/include/asm/mwait.h |   14 ++++++++++++++
>  arch/x86/kernel/process.c    |   14 ++++++++++++++
>  2 files changed, 28 insertions(+)
>
> --- 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/nospec-branch.h>
>  
>  #define MWAIT_SUBSTATE_MASK		0xf
>  #define MWAIT_CSTATE_MASK		0xf
> @@ -106,9 +107,22 @@ static inline void mwait_idle_with_hints
>  			mb();
>  		}
>  
> +		/*
> +		 * Indirect Branch Speculation (IBS) is controlled per
> +		 * physical core. If one thread disables it, then it's
> +		 * disabled on all threads of the core. The kernel disables
> +		 * it on entry from user space. Reenable it on the thread
> +		 * which goes idle so the other thread has a chance to run
> +		 * with full speculation enabled in userspace.
> +		 */
> +		restart_indirect_branch_speculation();
>  		__monitor((void *)&current_thread_info()->flags, 0, 0);
>  		if (!need_resched())
>  			__mwait(eax, ecx);
> +		/*
> +		 * Stop IBS again to protect kernel execution.
> +		 */
> +		stop_indirect_branch_speculation();

Be very careful.  The safety of this on Skylake+ depends on there not
being a ret instruction which can be speculatively reached between the
two WRMSRs used to play with MSR_SPEC_CTRL.

You need to guarantee that the net call tree of these five functions is
forced always inline.  A plain "static inline" function is not good
enough (and there are several here), if the compiler chooses to
out-of-line it, and a real function call is definitely a problem.

I accidentally introduced a vulnerability into Xen in my first attempt
at this.  (I'm still trying to decide whether reimplementing it in asm
would be better long term).

~Andrew

[-- Attachment #2: Type: text/html, Size: 3599 bytes --]

[PATCH 29/35] x86/speculation: Add IPBP support

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: x86-speculation--Add-IPBP-support.patch --]
[-- Type: text/plain, Size: 3227 bytes --]

From: Thomas Gleixner <tglx@linutronix.de>

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/cpufeatures.h   |    4 +++-
 arch/x86/include/asm/msr-index.h     |    3 +++
 arch/x86/include/asm/nospec-branch.h |    9 +++++++++
 arch/x86/kernel/cpu/bugs.c           |    2 ++
 arch/x86/kernel/cpu/specctrl.c       |    9 +++++++++
 5 files changed, 26 insertions(+), 1 deletion(-)

--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -212,8 +212,9 @@
 
 #define X86_FEATURE_MBA			( 7*32+18) /* Memory Bandwidth Allocation */
 #define X86_FEATURE_RSB_CTXSW		( 7*32+19) /* Fill RSB on context switches */
-#define X86_FEATURE_SPEC_CTRL		( 7*32+20) /* Speculation Control */
+#define X86_FEATURE_SPEC_CTRL		( 7*32+20) /* Speculation Control - Intel only */
 #define X86_FEATURE_IBRS		( 7*32+21) /* Indirect Branch Restricted Speculation */
+#define X86_FEATURE_IBPB		( 7*32+22) /* Indirect Branch Prediction Barrier */
 
 /* Virtualization flags: Linux defined, word 8 */
 #define X86_FEATURE_TPR_SHADOW		( 8*32+ 0) /* Intel TPR Shadow */
@@ -273,6 +274,7 @@
 #define X86_FEATURE_CLZERO		(13*32+ 0) /* CLZERO instruction */
 #define X86_FEATURE_IRPERF		(13*32+ 1) /* Instructions Retired Count */
 #define X86_FEATURE_XSAVEERPTR		(13*32+ 2) /* Always save/restore FP error pointers */
+#define X86_FEATURE_AMD_IBPB		(13*32+12) /* Indirect Branch Prediction Barrier support */
 
 /* Thermal and Power Management Leaf, CPUID level 0x00000006 (EAX), word 14 */
 #define X86_FEATURE_DTHERM		(14*32+ 0) /* Digital Thermal Sensor */
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -46,6 +46,9 @@
 #define SPEC_CTRL_DISABLE_IBRS		(0 << 0)
 #define SPEC_CTRL_ENABLE_IBRS		(1 << 0)
 
+#define MSR_IA32_PRED_CMD		0x00000049
+#define PRED_CMD_IBPB			(1 << 0)
+
 #define MSR_IA32_PERFCTR0		0x000000c1
 #define MSR_IA32_PERFCTR1		0x000000c2
 #define MSR_FSB_FREQ			0x000000cd
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -231,5 +231,14 @@ static inline void restart_indirect_bran
 	if (static_cpu_has(X86_FEATURE_IBRS))
 		native_wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_DISABLE_IBRS);
 }
+
+void specctrl_init_ibpb(void);
+
+static inline void indirect_branch_prediction_barrier(void)
+{
+	if (static_cpu_has(X86_FEATURE_IBPB))
+		native_wrmsrl(MSR_IA32_PRED_CMD, PRED_CMD_IBPB);
+}
+
 #endif /* __ASSEMBLY__ */
 #endif /* __NOSPEC_BRANCH_H__ */
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -271,6 +271,8 @@ static void __init spectre_v2_select_mit
 		setup_force_cpu_cap(X86_FEATURE_RSB_CTXSW);
 		pr_info("Filling RSB on context switch\n");
 	}
+	/* Initialize Indirect Branch Prediction Barrier if supported */
+	specctrl_init_ibpb();
 }
 
 #undef pr_fmt
--- a/arch/x86/kernel/cpu/specctrl.c
+++ b/arch/x86/kernel/cpu/specctrl.c
@@ -30,3 +30,12 @@ bool __init specctrl_cond_enable_ibrs(bo
 	specctrl_enable_ibrs();
 	return true;
 }
+
+void __init specctrl_init_ibpb(void)
+{
+	if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL) &&
+	    !boot_cpu_has(X86_FEATURE_AMD_IBPB))
+		return;
+
+	setup_force_cpu_cap(X86_FEATURE_IBPB);
+}

Re: [PATCH 29/35] x86/speculation: Add IPBP support

[Josh Poimboeuf]

On Thu, Jan 18, 2018 at 02:48:29PM +0100, Peter Zijlstra wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

s/IPBP/IBPB/ in subject.

-- 
Josh

Re: [PATCH 29/35] x86/speculation: Add IPBP support

[Borislav Petkov]

On Thu, Jan 18, 2018 at 02:48:29PM +0100, Peter Zijlstra wrote:
> From: Thomas Gleixner <tglx@linutronix.de>

<--- Add commit message here.

> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/include/asm/cpufeatures.h   |    4 +++-
>  arch/x86/include/asm/msr-index.h     |    3 +++
>  arch/x86/include/asm/nospec-branch.h |    9 +++++++++
>  arch/x86/kernel/cpu/bugs.c           |    2 ++
>  arch/x86/kernel/cpu/specctrl.c       |    9 +++++++++
>  5 files changed, 26 insertions(+), 1 deletion(-)
> 
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -212,8 +212,9 @@
>  
>  #define X86_FEATURE_MBA			( 7*32+18) /* Memory Bandwidth Allocation */
>  #define X86_FEATURE_RSB_CTXSW		( 7*32+19) /* Fill RSB on context switches */
> -#define X86_FEATURE_SPEC_CTRL		( 7*32+20) /* Speculation Control */
> +#define X86_FEATURE_SPEC_CTRL		( 7*32+20) /* Speculation Control - Intel only */
>  #define X86_FEATURE_IBRS		( 7*32+21) /* Indirect Branch Restricted Speculation */
> +#define X86_FEATURE_IBPB		( 7*32+22) /* Indirect Branch Prediction Barrier */
>  
>  /* Virtualization flags: Linux defined, word 8 */
>  #define X86_FEATURE_TPR_SHADOW		( 8*32+ 0) /* Intel TPR Shadow */
> @@ -273,6 +274,7 @@
>  #define X86_FEATURE_CLZERO		(13*32+ 0) /* CLZERO instruction */
>  #define X86_FEATURE_IRPERF		(13*32+ 1) /* Instructions Retired Count */
>  #define X86_FEATURE_XSAVEERPTR		(13*32+ 2) /* Always save/restore FP error pointers */
> +#define X86_FEATURE_AMD_IBPB		(13*32+12) /* Indirect Branch Prediction Barrier support */

I guess you could make that

#define X86_FEATURE_AMD_IBPB		(13*32+12) /* "" Indirect Branch Prediction Barrier support */

(note the "" in the comment)

so that it doesn't appear in /proc/cpuinfo as those two flags denote the
same thing.

>  /* Thermal and Power Management Leaf, CPUID level 0x00000006 (EAX), word 14 */
>  #define X86_FEATURE_DTHERM		(14*32+ 0) /* Digital Thermal Sensor */
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -46,6 +46,9 @@
>  #define SPEC_CTRL_DISABLE_IBRS		(0 << 0)
>  #define SPEC_CTRL_ENABLE_IBRS		(1 << 0)
>  
> +#define MSR_IA32_PRED_CMD		0x00000049
> +#define PRED_CMD_IBPB			(1 << 0)
> +
>  #define MSR_IA32_PERFCTR0		0x000000c1
>  #define MSR_IA32_PERFCTR1		0x000000c2
>  #define MSR_FSB_FREQ			0x000000cd
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -231,5 +231,14 @@ static inline void restart_indirect_bran
>  	if (static_cpu_has(X86_FEATURE_IBRS))
>  		native_wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_DISABLE_IBRS);
>  }
> +
> +void specctrl_init_ibpb(void);
> +
> +static inline void indirect_branch_prediction_barrier(void)

That's a long name: ibp_barrier() is what I had called it, with a
comment above it.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 29/35] x86/speculation: Add IPBP support

[Josh Poimboeuf]

On Thu, Jan 18, 2018 at 07:31:16PM +0100, Borislav Petkov wrote:
> On Thu, Jan 18, 2018 at 02:48:29PM +0100, Peter Zijlstra wrote:
> > From: Thomas Gleixner <tglx@linutronix.de>
> 
> <--- Add commit message here.
> 
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > ---
> >  arch/x86/include/asm/cpufeatures.h   |    4 +++-
> >  arch/x86/include/asm/msr-index.h     |    3 +++
> >  arch/x86/include/asm/nospec-branch.h |    9 +++++++++
> >  arch/x86/kernel/cpu/bugs.c           |    2 ++
> >  arch/x86/kernel/cpu/specctrl.c       |    9 +++++++++
> >  5 files changed, 26 insertions(+), 1 deletion(-)
> > 
> > --- a/arch/x86/include/asm/cpufeatures.h
> > +++ b/arch/x86/include/asm/cpufeatures.h
> > @@ -212,8 +212,9 @@
> >  
> >  #define X86_FEATURE_MBA			( 7*32+18) /* Memory Bandwidth Allocation */
> >  #define X86_FEATURE_RSB_CTXSW		( 7*32+19) /* Fill RSB on context switches */
> > -#define X86_FEATURE_SPEC_CTRL		( 7*32+20) /* Speculation Control */
> > +#define X86_FEATURE_SPEC_CTRL		( 7*32+20) /* Speculation Control - Intel only */
> >  #define X86_FEATURE_IBRS		( 7*32+21) /* Indirect Branch Restricted Speculation */
> > +#define X86_FEATURE_IBPB		( 7*32+22) /* Indirect Branch Prediction Barrier */
> >  
> >  /* Virtualization flags: Linux defined, word 8 */
> >  #define X86_FEATURE_TPR_SHADOW		( 8*32+ 0) /* Intel TPR Shadow */
> > @@ -273,6 +274,7 @@
> >  #define X86_FEATURE_CLZERO		(13*32+ 0) /* CLZERO instruction */
> >  #define X86_FEATURE_IRPERF		(13*32+ 1) /* Instructions Retired Count */
> >  #define X86_FEATURE_XSAVEERPTR		(13*32+ 2) /* Always save/restore FP error pointers */
> > +#define X86_FEATURE_AMD_IBPB		(13*32+12) /* Indirect Branch Prediction Barrier support */
> 
> I guess you could make that
> 
> #define X86_FEATURE_AMD_IBPB		(13*32+12) /* "" Indirect Branch Prediction Barrier support */
> 
> (note the "" in the comment)
> 
> so that it doesn't appear in /proc/cpuinfo as those two flags denote the
> same thing.

Maybe I missed the memo, why do we need both X86_FEATURE_IBPB and
X86_FEATURE_AMD_IBPB?

-- 
Josh

Re: [PATCH 29/35] x86/speculation: Add IPBP support

[Borislav Petkov]

On Thu, Jan 18, 2018 at 12:35:23PM -0600, Josh Poimboeuf wrote:
> Maybe I missed the memo, why do we need both X86_FEATURE_IBPB and
> X86_FEATURE_AMD_IBPB?

So AMD_IBPB is a different CPUID bit in a different CPUID function
and on Intel, IBPB is set only when X86_FEATURE_SPEC_CTRL - see
specctrl_init_ibpb() in that same patch.

IOW, CPUID(7).EDAC[26] means both IBRS and IBPB on Intel.

So, in the end of the day, X86_FEATURE_IBPB is the feature bit we'll be
testing and X86_FEATURE_AMD_IBPB is used only to set X86_FEATURE_IBPB.

:-)

More confused?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[PATCH 30/35] x86/speculation: Use Indirect Branch Prediction Barrier in context switch

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: x86-speculation--Use-Indirect-Branch-Prediction-Barrier-in-context-switch.patch --]
[-- Type: text/plain, Size: 1130 bytes --]

From: Thomas Gleixner <tglx@linutronix.de>

[peterz: comment]

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/mm/tlb.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -6,13 +6,14 @@
 #include <linux/interrupt.h>
 #include <linux/export.h>
 #include <linux/cpu.h>
+#include <linux/debugfs.h>
 
 #include <asm/tlbflush.h>
 #include <asm/mmu_context.h>
+#include <asm/nospec-branch.h>
 #include <asm/cache.h>
 #include <asm/apic.h>
 #include <asm/uv/uv.h>
-#include <linux/debugfs.h>
 
 /*
  *	TLB flushing, formerly SMP-only
@@ -220,6 +221,13 @@ void switch_mm_irqs_off(struct mm_struct
 		u16 new_asid;
 		bool need_flush;
 
+		/*
+		 * Avoid user/user BTB poisoning by flushing the branch predictor
+		 * when switching between processes. This stops one process from
+		 * doing spectre-v2 attacks on another process's data.
+		 */
+		indirect_branch_prediction_barrier();
+
 		if (IS_ENABLED(CONFIG_VMAP_STACK)) {
 			/*
 			 * If our current stack is in vmalloc space and isn't

Re: [PATCH 30/35] x86/speculation: Use Indirect Branch Prediction Barrier in context switch

[Tim Chen]

On 01/18/2018 05:48 AM, Peter Zijlstra wrote:
>
>+		/*
>+		 * Avoid user/user BTB poisoning by flushing the branch predictor
>+		 * when switching between processes. This stops one process from
>+		 * doing spectre-v2 attacks on another process's data.
>+		 */
>+		indirect_branch_prediction_barrier();
>+

Some optimizations can be done here to avoid overhead in barrier call.

For example, don't do the barrier if prev and next mm are
the same.  If the two process trust each other, or the new process
already have rights to look into the previous process,
the barrier could be skipped.

Tim

Re: [PATCH 30/35] x86/speculation: Use Indirect Branch Prediction Barrier in context switch

[Kevin Easton]

On Thu, Jan 18, 2018 at 04:38:32PM -0800, Tim Chen wrote:
> On 01/18/2018 05:48 AM, Peter Zijlstra wrote:
> >
> >+		/*
> >+		 * Avoid user/user BTB poisoning by flushing the branch predictor
> >+		 * when switching between processes. This stops one process from
> >+		 * doing spectre-v2 attacks on another process's data.
> >+		 */
> >+		indirect_branch_prediction_barrier();
> >+
> 
> Some optimizations can be done here to avoid overhead in barrier call.
> 
> For example, don't do the barrier if prev and next mm are
> the same.  If the two process trust each other, or the new process
> already have rights to look into the previous process,
> the barrier could be skipped.

Isn't it the other way around with the BTB poisoning? previous is
potentially attacking next, so the barrier can be avoided only if previous
is allowed to ptrace next?

    - Kevin

Re: [PATCH 30/35] x86/speculation: Use Indirect Branch Prediction Barrier in context switch

[Tim Chen]

On 01/18/2018 08:03 PM, Kevin Easton wrote:
> On Thu, Jan 18, 2018 at 04:38:32PM -0800, Tim Chen wrote:
>> On 01/18/2018 05:48 AM, Peter Zijlstra wrote:
>>>
>>> +		/*
>>> +		 * Avoid user/user BTB poisoning by flushing the branch predictor
>>> +		 * when switching between processes. This stops one process from
>>> +		 * doing spectre-v2 attacks on another process's data.
>>> +		 */
>>> +		indirect_branch_prediction_barrier();
>>> +
>>
>> Some optimizations can be done here to avoid overhead in barrier call.
>>
>> For example, don't do the barrier if prev and next mm are
>> the same.  If the two process trust each other, or the new process
>> already have rights to look into the previous process,
>> the barrier could be skipped.
> 
> Isn't it the other way around with the BTB poisoning? previous is
> potentially attacking next, so the barrier can be avoided only if previous
> is allowed to ptrace next?
> 

Yes, if the next process don't trust the previous process, then
doing a prediction barrier before the context switch makes sense.

Tim

[PATCH 31/35] x86/ibrs: Add new helper macros to save/restore MSR_IA32_SPEC_CTRL

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: ashok_raj-x86_ibrs-add_new_helper_macros_to_save_restore_msr_ia32_spec_ctrl.patch --]
[-- Type: text/plain, Size: 1867 bytes --]

From: Ashok Raj <ashok.raj@intel.com>

Add some helper macros to save/restore MSR_IA32_SPEC_CTRL.

  stop_indirect_branch_speculation_and_save() - saves the current
	state and enables IBRS.

  restore_indirect_branch_speculation() - restores the previously
	saved IBRS state.

[peterz: renaming and folding of logic from the kvm patches]

Cc: Asit Mallick <asit.k.mallick@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Arjan Van De Ven <arjan.van.de.ven@intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/nospec-branch.h |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -232,6 +232,24 @@ static inline void restart_indirect_bran
 		native_wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_DISABLE_IBRS);
 }
 
+static inline u64 stop_indirect_branch_speculation_and_save(void)
+{
+	u64 val = 0;
+
+	if (static_cpu_has(X86_FEATURE_IBRS)) {
+		val = native_rdmsrl(MSR_IA32_SPEC_CTRL);
+		native_wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_ENABLE_IBRS);
+	}
+
+	return val;
+}
+
+static inline void restore_indirect_branch_speculation(u64 val)
+{
+	if (static_cpu_has(X86_FEATURE_IBRS))
+		native_wrmsrl(MSR_IA32_SPEC_CTRL, val);
+}
+
 void specctrl_init_ibpb(void);
 
 static inline void indirect_branch_prediction_barrier(void)

[PATCH 32/35] x86/vmx: Direct access to MSR_IA32_SPEC_CTRL

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: ashok_raj-x86_ibrs-add_direct_access_support_for_msr_ia32_spec_ctrl.patch --]
[-- Type: text/plain, Size: 4337 bytes --]

From: Ashok Raj <ashok.raj@intel.com>

Add direct access to MSR_IA32_SPEC_CTRL from a guest. Also save/restore
IBRS values during exits and guest resume path.

[peterz: rebased]

Cc: Asit Mallick <asit.k.mallick@intel.com>
Cc: Arjan Van De Ven <arjan.van.de.ven@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kvm/cpuid.c |    3 ++-
 arch/x86/kvm/vmx.c   |   25 +++++++++++++++++++++++++
 arch/x86/kvm/x86.c   |    1 +
 3 files changed, 28 insertions(+), 1 deletion(-)

--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -70,6 +70,7 @@ u64 kvm_supported_xcr0(void)
 /* These are scattered features in cpufeatures.h. */
 #define KVM_CPUID_BIT_AVX512_4VNNIW     2
 #define KVM_CPUID_BIT_AVX512_4FMAPS     3
+#define KVM_CPUID_BIT_SPEC_CTRL        26
 #define KF(x) bit(KVM_CPUID_BIT_##x)
 
 int kvm_update_cpuid(struct kvm_vcpu *vcpu)
@@ -392,7 +393,7 @@ static inline int __do_cpuid_ent(struct
 
 	/* cpuid 7.0.edx*/
 	const u32 kvm_cpuid_7_0_edx_x86_features =
-		KF(AVX512_4VNNIW) | KF(AVX512_4FMAPS);
+		KF(AVX512_4VNNIW) | KF(AVX512_4FMAPS) | KF(SPEC_CTRL);
 
 	/* all calls to cpuid_count() should be made on the same cpu */
 	get_cpu();
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -580,6 +580,7 @@ struct vcpu_vmx {
 	u32 vm_entry_controls_shadow;
 	u32 vm_exit_controls_shadow;
 	u32 secondary_exec_control;
+	u64 spec_ctrl;
 
 	/*
 	 * loaded_vmcs points to the VMCS currently used in this vcpu. For a
@@ -3260,6 +3261,9 @@ static int vmx_get_msr(struct kvm_vcpu *
 	case MSR_IA32_TSC:
 		msr_info->data = guest_read_tsc(vcpu);
 		break;
+	case MSR_IA32_SPEC_CTRL:
+		msr_info->data = to_vmx(vcpu)->spec_ctrl;
+		break;
 	case MSR_IA32_SYSENTER_CS:
 		msr_info->data = vmcs_read32(GUEST_SYSENTER_CS);
 		break;
@@ -3367,6 +3371,9 @@ static int vmx_set_msr(struct kvm_vcpu *
 	case MSR_IA32_TSC:
 		kvm_write_tsc(vcpu, msr_info);
 		break;
+	case MSR_IA32_SPEC_CTRL:
+		to_vmx(vcpu)->spec_ctrl = msr_info->data;
+		break;
 	case MSR_IA32_CR_PAT:
 		if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT) {
 			if (!kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, data))
@@ -6791,6 +6798,13 @@ static __init int hardware_setup(void)
 		kvm_tsc_scaling_ratio_frac_bits = 48;
 	}
 
+	/*
+	 * If feature is available then setup MSR_IA32_SPEC_CTRL to be in
+	 * passthrough mode for the guest.
+	 */
+	if (boot_cpu_has(X86_FEATURE_SPEC_CTRL))
+		vmx_disable_intercept_for_msr(MSR_IA32_SPEC_CTRL, false);
+
 	vmx_disable_intercept_for_msr(MSR_FS_BASE, false);
 	vmx_disable_intercept_for_msr(MSR_GS_BASE, false);
 	vmx_disable_intercept_for_msr(MSR_KERNEL_GS_BASE, true);
@@ -9299,6 +9313,15 @@ static void __noclone vmx_vcpu_run(struc
 	vmx_arm_hv_timer(vcpu);
 
 	vmx->__launched = vmx->loaded_vmcs->launched;
+
+	/*
+	 * Just update whatever the value was set for the MSR in guest.
+	 * If this is unlaunched: Assume that initialized value is 0.
+	 * IRQ's also need to be disabled. If guest value is 0, an interrupt
+	 * could start running in unprotected mode (i.e with IBRS=0).
+	 */
+	restore_indirect_branch_speculation(vmx->spec_ctrl);
+
 	asm(
 		/* Store host registers */
 		"push %%" _ASM_DX "; push %%" _ASM_BP ";"
@@ -9407,6 +9430,8 @@ static void __noclone vmx_vcpu_run(struc
 	/* Eliminate branch target predictions from guest mode */
 	vmexit_fill_RSB();
 
+	vmx->spec_ctrl = stop_indirect_branch_speculation_and_save();
+
 	/* MSR_IA32_DEBUGCTLMSR is zeroed on vmexit. Restore it if needed */
 	if (debugctlmsr)
 		update_debugctlmsr(debugctlmsr);
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1006,6 +1006,7 @@ static u32 msrs_to_save[] = {
 #endif
 	MSR_IA32_TSC, MSR_IA32_CR_PAT, MSR_VM_HSAVE_PA,
 	MSR_IA32_FEATURE_CONTROL, MSR_IA32_BNDCFGS, MSR_TSC_AUX,
+	MSR_IA32_SPEC_CTRL,
 };
 
 static unsigned num_msrs_to_save;

[PATCH 33/35] x86/svm: Direct access to MSR_IA32_SPEC_CTRL

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: paolo_bonzini-x86_svm-direct_access_to_msr_ia32_spec_ctrl.patch --]
[-- Type: text/plain, Size: 2812 bytes --]

From: Paolo Bonzini <pbonzini@redhat.com>

Direct access to MSR_IA32_SPEC_CTRL is important
for performance.  Allow load/store of MSR_IA32_SPEC_CTRL, restore guest
IBRS on VM entry and set restore host values on VM exit.
it yet).

TBD: need to check msr's can be passed through even if feature is not
emuerated by the CPU.

[peterz: rebased, folded feedback from Tom]

Cc: Asit Mallick <asit.k.mallick@intel.com>
Cc: Arjan Van De Ven <arjan.van.de.ven@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kvm/svm.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -184,6 +184,8 @@ struct vcpu_svm {
 		u64 gs_base;
 	} host;
 
+	u64 spec_ctrl;
+
 	u32 *msrpm;
 
 	ulong nmi_iret_rip;
@@ -249,6 +251,7 @@ static const struct svm_direct_access_ms
 	{ .index = MSR_CSTAR,				.always = true  },
 	{ .index = MSR_SYSCALL_MASK,			.always = true  },
 #endif
+	{ .index = MSR_IA32_SPEC_CTRL,          .always = true  },
 	{ .index = MSR_IA32_LASTBRANCHFROMIP,		.always = false },
 	{ .index = MSR_IA32_LASTBRANCHTOIP,		.always = false },
 	{ .index = MSR_IA32_LASTINTFROMIP,		.always = false },
@@ -3577,6 +3580,9 @@ static int svm_get_msr(struct kvm_vcpu *
 	case MSR_VM_CR:
 		msr_info->data = svm->nested.vm_cr_msr;
 		break;
+	case MSR_IA32_SPEC_CTRL:
+		msr_info->data = svm->spec_ctrl;
+		break;
 	case MSR_IA32_UCODE_REV:
 		msr_info->data = 0x01000065;
 		break;
@@ -3725,6 +3731,9 @@ static int svm_set_msr(struct kvm_vcpu *
 	case MSR_VM_IGNNE:
 		vcpu_unimpl(vcpu, "unimplemented wrmsr: 0x%x data 0x%llx\n", ecx, data);
 		break;
+	case MSR_IA32_SPEC_CTRL:
+		svm->spec_ctrl = data;
+		break;
 	case MSR_IA32_APICBASE:
 		if (kvm_vcpu_apicv_active(vcpu))
 			avic_update_vapic_bar(to_svm(vcpu), data);
@@ -4911,6 +4920,8 @@ static void svm_vcpu_run(struct kvm_vcpu
 
 	clgi();
 
+	restore_indirect_branch_speculation(svm->spec_ctrl);
+
 	local_irq_enable();
 
 	asm volatile (
@@ -4989,6 +5000,8 @@ static void svm_vcpu_run(struct kvm_vcpu
 	/* Eliminate branch target predictions from guest mode */
 	vmexit_fill_RSB();
 
+	svm->spec_ctrl = stop_indirect_branch_speculation_and_save();
+
 #ifdef CONFIG_X86_64
 	wrmsrl(MSR_GS_BASE, svm->host.gs_base);
 #else

[PATCH 34/35] x86/kvm: Add IBPB support

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: ashok_raj-x86_feature-detect_the_x86_feature_indirect_branch_prediction_barrier.patch --]
[-- Type: text/plain, Size: 3522 bytes --]

From: Ashok Raj <ashok.raj@intel.com>

Add MSR passthrough for MSR_IA32_PRED_CMD and place branch predictor
barriers on switching between VMs to avoid inter VM specte-v2 attacks.

[peterz: rebase and changelog rewrite]

Cc: Asit Mallick <asit.k.mallick@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Arjan Van De Ven <arjan.van.de.ven@intel.com>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kvm/svm.c |    8 ++++++++
 arch/x86/kvm/vmx.c |    8 ++++++++
 2 files changed, 16 insertions(+)

--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -252,6 +252,7 @@ static const struct svm_direct_access_ms
 	{ .index = MSR_SYSCALL_MASK,			.always = true  },
 #endif
 	{ .index = MSR_IA32_SPEC_CTRL,          .always = true  },
+	{ .index = MSR_IA32_PRED_CMD,           .always = true },
 	{ .index = MSR_IA32_LASTBRANCHFROMIP,		.always = false },
 	{ .index = MSR_IA32_LASTBRANCHTOIP,		.always = false },
 	{ .index = MSR_IA32_LASTINTFROMIP,		.always = false },
@@ -532,6 +533,7 @@ struct svm_cpu_data {
 	struct kvm_ldttss_desc *tss_desc;
 
 	struct page *save_area;
+	struct vmcb *current_vmcb;
 };
 
 static DEFINE_PER_CPU(struct svm_cpu_data *, svm_data);
@@ -1709,11 +1711,13 @@ static void svm_free_vcpu(struct kvm_vcp
 	__free_pages(virt_to_page(svm->nested.msrpm), MSRPM_ALLOC_ORDER);
 	kvm_vcpu_uninit(vcpu);
 	kmem_cache_free(kvm_vcpu_cache, svm);
+	indirect_branch_prediction_barrier();
 }
 
 static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
+	struct svm_cpu_data *sd = per_cpu(svm_data, cpu);
 	int i;
 
 	if (unlikely(cpu != vcpu->cpu)) {
@@ -1742,6 +1746,10 @@ static void svm_vcpu_load(struct kvm_vcp
 	if (static_cpu_has(X86_FEATURE_RDTSCP))
 		wrmsrl(MSR_TSC_AUX, svm->tsc_aux);
 
+	if (sd->current_vmcb != svm->vmcb) {
+		sd->current_vmcb = svm->vmcb;
+		indirect_branch_prediction_barrier();
+	}
 	avic_vcpu_load(vcpu, cpu);
 }
 
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2280,6 +2280,7 @@ static void vmx_vcpu_load(struct kvm_vcp
 	if (per_cpu(current_vmcs, cpu) != vmx->loaded_vmcs->vmcs) {
 		per_cpu(current_vmcs, cpu) = vmx->loaded_vmcs->vmcs;
 		vmcs_load(vmx->loaded_vmcs->vmcs);
+		indirect_branch_prediction_barrier();
 	}
 
 	if (!already_loaded) {
@@ -3837,6 +3838,11 @@ static void free_loaded_vmcs(struct load
 	free_vmcs(loaded_vmcs->vmcs);
 	loaded_vmcs->vmcs = NULL;
 	WARN_ON(loaded_vmcs->shadow_vmcs != NULL);
+	/*
+	 * The VMCS could be recycled, causing a false negative in vmx_vcpu_load
+	 * block speculative execution.
+	 */
+	indirect_branch_prediction_barrier();
 }
 
 static void free_kvm_area(void)
@@ -6804,6 +6810,8 @@ static __init int hardware_setup(void)
 	 */
 	if (boot_cpu_has(X86_FEATURE_SPEC_CTRL))
 		vmx_disable_intercept_for_msr(MSR_IA32_SPEC_CTRL, false);
+	if (boot_cpu_has(X86_FEATURE_IBPB))
+		vmx_disable_intercept_for_msr(MSR_IA32_PRED_CMD, false);
 
 	vmx_disable_intercept_for_msr(MSR_FS_BASE, false);
 	vmx_disable_intercept_for_msr(MSR_GS_BASE, false);

Re: [PATCH 34/35] x86/kvm: Add IBPB support

[Paolo Bonzini]

On 18/01/2018 14:48, Peter Zijlstra wrote:
> From: Ashok Raj <ashok.raj@intel.com>
> 
> Add MSR passthrough for MSR_IA32_PRED_CMD and place branch predictor
> barriers on switching between VMs to avoid inter VM specte-v2 attacks.
> 
> [peterz: rebase and changelog rewrite]
> 
> Cc: Asit Mallick <asit.k.mallick@intel.com>
> Cc: Dave Hansen <dave.hansen@intel.com>
> Cc: Arjan Van De Ven <arjan.van.de.ven@intel.com>
> Cc: Tim Chen <tim.c.chen@linux.intel.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Jun Nakajima <jun.nakajima@intel.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/kvm/svm.c |    8 ++++++++
>  arch/x86/kvm/vmx.c |    8 ++++++++
>  2 files changed, 16 insertions(+)

This patch is missing the AMD-specific CPUID bit.

In addition, it is not bisectable because guests will see the SPEC_CTRL CPUID
bit after patch 32 even if PRED_CMD is not supported yet.

The simplest solutions are:

1) add the indirect_branch_prediction_barrier() calls first, and squash
everything else including the AMD CPUID bit into a single patch.  

2) place the IBPB in this series, and only add stop/restart_indirect_branch_
speculation() to the vmexit and vmentry paths.  We will do the guest enabling
in kvm.git after this is ready, it should only take a week and we'll ensure
it is backportable to 4.14 and 4.15.

3) same as (2) except we'll send the guest enabling through TIP.

Paolo

> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -252,6 +252,7 @@ static const struct svm_direct_access_ms
>  	{ .index = MSR_SYSCALL_MASK,			.always = true  },
>  #endif
>  	{ .index = MSR_IA32_SPEC_CTRL,          .always = true  },
> +	{ .index = MSR_IA32_PRED_CMD,           .always = true },
>  	{ .index = MSR_IA32_LASTBRANCHFROMIP,		.always = false },
>  	{ .index = MSR_IA32_LASTBRANCHTOIP,		.always = false },
>  	{ .index = MSR_IA32_LASTINTFROMIP,		.always = false },
> @@ -532,6 +533,7 @@ struct svm_cpu_data {
>  	struct kvm_ldttss_desc *tss_desc;
>  
>  	struct page *save_area;
> +	struct vmcb *current_vmcb;
>  };
>  
>  static DEFINE_PER_CPU(struct svm_cpu_data *, svm_data);
> @@ -1709,11 +1711,13 @@ static void svm_free_vcpu(struct kvm_vcp
>  	__free_pages(virt_to_page(svm->nested.msrpm), MSRPM_ALLOC_ORDER);
>  	kvm_vcpu_uninit(vcpu);
>  	kmem_cache_free(kvm_vcpu_cache, svm);
> +	indirect_branch_prediction_barrier();
>  }
>  
>  static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
> +	struct svm_cpu_data *sd = per_cpu(svm_data, cpu);
>  	int i;
>  
>  	if (unlikely(cpu != vcpu->cpu)) {
> @@ -1742,6 +1746,10 @@ static void svm_vcpu_load(struct kvm_vcp
>  	if (static_cpu_has(X86_FEATURE_RDTSCP))
>  		wrmsrl(MSR_TSC_AUX, svm->tsc_aux);
>  
> +	if (sd->current_vmcb != svm->vmcb) {
> +		sd->current_vmcb = svm->vmcb;
> +		indirect_branch_prediction_barrier();
> +	}
>  	avic_vcpu_load(vcpu, cpu);
>  }
>  
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2280,6 +2280,7 @@ static void vmx_vcpu_load(struct kvm_vcp
>  	if (per_cpu(current_vmcs, cpu) != vmx->loaded_vmcs->vmcs) {
>  		per_cpu(current_vmcs, cpu) = vmx->loaded_vmcs->vmcs;
>  		vmcs_load(vmx->loaded_vmcs->vmcs);
> +		indirect_branch_prediction_barrier();
>  	}
>  
>  	if (!already_loaded) {
> @@ -3837,6 +3838,11 @@ static void free_loaded_vmcs(struct load
>  	free_vmcs(loaded_vmcs->vmcs);
>  	loaded_vmcs->vmcs = NULL;
>  	WARN_ON(loaded_vmcs->shadow_vmcs != NULL);
> +	/*
> +	 * The VMCS could be recycled, causing a false negative in vmx_vcpu_load
> +	 * block speculative execution.
> +	 */
> +	indirect_branch_prediction_barrier();

This IBPB is not needed, as loaded_vmcs->NULL is now NULL and there will be a
barrier the next time vmx_vcpu_load is called on this CPU.

Thanks,

Paolo

>  }
>  
>  static void free_kvm_area(void)
> @@ -6804,6 +6810,8 @@ static __init int hardware_setup(void)
>  	 */
>  	if (boot_cpu_has(X86_FEATURE_SPEC_CTRL))
>  		vmx_disable_intercept_for_msr(MSR_IA32_SPEC_CTRL, false);
> +	if (boot_cpu_has(X86_FEATURE_IBPB))
> +		vmx_disable_intercept_for_msr(MSR_IA32_PRED_CMD, false);
>  
>  	vmx_disable_intercept_for_msr(MSR_FS_BASE, false);
>  	vmx_disable_intercept_for_msr(MSR_GS_BASE, false);
> 
> 

Re: [PATCH 34/35] x86/kvm: Add IBPB support

[Paolo Bonzini]

On 18/01/2018 16:32, Paolo Bonzini wrote:
> On 18/01/2018 14:48, Peter Zijlstra wrote:
>> From: Ashok Raj <ashok.raj@intel.com>
>>
>> Add MSR passthrough for MSR_IA32_PRED_CMD and place branch predictor
>> barriers on switching between VMs to avoid inter VM specte-v2 attacks.
>>
>> [peterz: rebase and changelog rewrite]
>>
>> Cc: Asit Mallick <asit.k.mallick@intel.com>
>> Cc: Dave Hansen <dave.hansen@intel.com>
>> Cc: Arjan Van De Ven <arjan.van.de.ven@intel.com>
>> Cc: Tim Chen <tim.c.chen@linux.intel.com>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Cc: Andrea Arcangeli <aarcange@redhat.com>
>> Cc: Andi Kleen <ak@linux.intel.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> Cc: Jun Nakajima <jun.nakajima@intel.com>
>> Cc: Andy Lutomirski <luto@kernel.org>
>> Cc: Greg KH <gregkh@linuxfoundation.org>
>> Cc: David Woodhouse <dwmw@amazon.co.uk>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>> ---
>>  arch/x86/kvm/svm.c |    8 ++++++++
>>  arch/x86/kvm/vmx.c |    8 ++++++++
>>  2 files changed, 16 insertions(+)
> 
> This patch is missing the AMD-specific CPUID bit.
> 
> In addition, it is not bisectable because guests will see the SPEC_CTRL CPUID
> bit after patch 32 even if PRED_CMD is not supported yet.
> 
> The simplest solutions are:
> 
> 1) add the indirect_branch_prediction_barrier() calls first, and squash
> everything else including the AMD CPUID bit into a single patch.  
> 
> 2) place the IBPB in this series, and only add stop/restart_indirect_branch_
> speculation() to the vmexit and vmentry paths.  We will do the guest enabling
> in kvm.git after this is ready, it should only take a week and we'll ensure
> it is backportable to 4.14 and 4.15.
> 
> 3) same as (2) except we'll send the guest enabling through TIP.
> 
> Paolo
> 
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -252,6 +252,7 @@ static const struct svm_direct_access_ms
>>  	{ .index = MSR_SYSCALL_MASK,			.always = true  },
>>  #endif
>>  	{ .index = MSR_IA32_SPEC_CTRL,          .always = true  },
>> +	{ .index = MSR_IA32_PRED_CMD,           .always = true },
>>  	{ .index = MSR_IA32_LASTBRANCHFROMIP,		.always = false },
>>  	{ .index = MSR_IA32_LASTBRANCHTOIP,		.always = false },
>>  	{ .index = MSR_IA32_LASTINTFROMIP,		.always = false },
>> @@ -532,6 +533,7 @@ struct svm_cpu_data {
>>  	struct kvm_ldttss_desc *tss_desc;
>>  
>>  	struct page *save_area;
>> +	struct vmcb *current_vmcb;
>>  };
>>  
>>  static DEFINE_PER_CPU(struct svm_cpu_data *, svm_data);
>> @@ -1709,11 +1711,13 @@ static void svm_free_vcpu(struct kvm_vcp
>>  	__free_pages(virt_to_page(svm->nested.msrpm), MSRPM_ALLOC_ORDER);
>>  	kvm_vcpu_uninit(vcpu);
>>  	kmem_cache_free(kvm_vcpu_cache, svm);
>> +	indirect_branch_prediction_barrier();
>>  }
>>  
>>  static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>>  {
>>  	struct vcpu_svm *svm = to_svm(vcpu);
>> +	struct svm_cpu_data *sd = per_cpu(svm_data, cpu);
>>  	int i;
>>  
>>  	if (unlikely(cpu != vcpu->cpu)) {
>> @@ -1742,6 +1746,10 @@ static void svm_vcpu_load(struct kvm_vcp
>>  	if (static_cpu_has(X86_FEATURE_RDTSCP))
>>  		wrmsrl(MSR_TSC_AUX, svm->tsc_aux);
>>  
>> +	if (sd->current_vmcb != svm->vmcb) {
>> +		sd->current_vmcb = svm->vmcb;
>> +		indirect_branch_prediction_barrier();
>> +	}
>>  	avic_vcpu_load(vcpu, cpu);
>>  }
>>  
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -2280,6 +2280,7 @@ static void vmx_vcpu_load(struct kvm_vcp
>>  	if (per_cpu(current_vmcs, cpu) != vmx->loaded_vmcs->vmcs) {
>>  		per_cpu(current_vmcs, cpu) = vmx->loaded_vmcs->vmcs;
>>  		vmcs_load(vmx->loaded_vmcs->vmcs);
>> +		indirect_branch_prediction_barrier();
>>  	}
>>  
>>  	if (!already_loaded) {
>> @@ -3837,6 +3838,11 @@ static void free_loaded_vmcs(struct load
>>  	free_vmcs(loaded_vmcs->vmcs);
>>  	loaded_vmcs->vmcs = NULL;
>>  	WARN_ON(loaded_vmcs->shadow_vmcs != NULL);
>> +	/*
>> +	 * The VMCS could be recycled, causing a false negative in vmx_vcpu_load
>> +	 * block speculative execution.
>> +	 */
>> +	indirect_branch_prediction_barrier();
> 
> This IBPB is not needed, as loaded_vmcs->NULL is now NULL and there will be a
> barrier the next time vmx_vcpu_load is called on this CPU.

Without retpolines, KVM userspace is not protected from the guest
poisoning the BTB, because there is no IBRS-barrier on the vmexit path.
So there are two more IBPBs that are needed if retpolines are enabled:

1) in kvm_sched_out

2) at the end of vcpu_run

Thanks,

Paolo

Re: [PATCH 34/35] x86/kvm: Add IBPB support

[David Woodhouse]

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

On Fri, 2018-01-19 at 16:25 +0100, Paolo Bonzini wrote:
> Without retpolines, KVM userspace is not protected from the guest
> poisoning the BTB, because there is no IBRS-barrier on the vmexit
> path.
> So there are two more IBPBs that are needed if retpolines are
> enabled:
> 
> 1) in kvm_sched_out
> 
> 2) at the end of vcpu_run

Hm, yes. That does seem reasonable. Can we make it conditional so it
only happens *if* we end up back in userspace, and not for a VM-
>kernel->VM transition?

And can I have a patch against
http://git.infradead.org/users/dwmw2/linux-retpoline.git/shortlog/refs/heads/ibpb-upstream
please (see the XX in that top commit too).

I'm still putting that together, and the IBRS bits on top; getting on
an airplane to spend some more quality time with it now...

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

Re: [PATCH 34/35] x86/kvm: Add IBPB support

[Andy Lutomirski]

On Fri, Jan 19, 2018 at 8:08 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Fri, 2018-01-19 at 16:25 +0100, Paolo Bonzini wrote:
>> Without retpolines, KVM userspace is not protected from the guest
>> poisoning the BTB, because there is no IBRS-barrier on the vmexit
>> path.
>> So there are two more IBPBs that are needed if retpolines are
>> enabled:
>>
>> 1) in kvm_sched_out
>>
>> 2) at the end of vcpu_run
>
> Hm, yes. That does seem reasonable. Can we make it conditional so it
> only happens *if* we end up back in userspace, and not for a VM-
>>kernel->VM transition?

kvm_on_user_return(), perhaps?

Re: [PATCH 34/35] x86/kvm: Add IBPB support

[Paolo Bonzini]

On 19/01/2018 17:08, David Woodhouse wrote:
> On Fri, 2018-01-19 at 16:25 +0100, Paolo Bonzini wrote:
>> Without retpolines, KVM userspace is not protected from the guest
>> poisoning the BTB, because there is no IBRS-barrier on the vmexit
>> path.
>> So there are two more IBPBs that are needed if retpolines are
>> enabled:
>>
>> 1) in kvm_sched_out
>>
>> 2) at the end of vcpu_run
> 
> Hm, yes. That does seem reasonable. Can we make it conditional so it
> only happens *if* we end up back in userspace, and not for a VM->
> kernel->VM transition?

We can/want/must.  It's actually kvm_arch_vcpu_ioctl_run, not vcpu_run.
Sorry.

> And can I have a patch against
> http://git.infradead.org/users/dwmw2/linux-retpoline.git/shortlog/refs/heads/ibpb-upstream
> please (see the XX in that top commit too).
> 
> I'm still putting that together, and the IBRS bits on top; getting on
> an airplane to spend some more quality time with it now...

I'm completely lost with all the branches...

Paolo

[PATCH 35/35] x86/nospec: Add static assertions

[Peter Zijlstra, Peter Zijlstra]

[-- Attachment #1: peterz-assert-ibrs-ibpb.patch --]
[-- Type: text/plain, Size: 2038 bytes --]

These sites must not end up under dynamic conditions because then it
could be speculated across. Ensure objtools verifies this.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/nospec-branch.h |   19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -3,6 +3,8 @@
 #ifndef __NOSPEC_BRANCH_H__
 #define __NOSPEC_BRANCH_H__
 
+#include <linux/jump_label.h>
+
 #include <asm/alternative.h>
 #include <asm/alternative-asm.h>
 #include <asm/cpufeatures.h>
@@ -222,14 +224,18 @@ bool is_skylake_era(void);
 
 static inline void stop_indirect_branch_speculation(void)
 {
-	if (static_cpu_has(X86_FEATURE_IBRS))
+	if (static_cpu_has(X86_FEATURE_IBRS)) {
+		arch_static_assert();
 		native_wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_ENABLE_IBRS);
+	}
 }
 
 static inline void restart_indirect_branch_speculation(void)
 {
-	if (static_cpu_has(X86_FEATURE_IBRS))
+	if (static_cpu_has(X86_FEATURE_IBRS)) {
+		arch_static_assert();
 		native_wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_DISABLE_IBRS);
+	}
 }
 
 static inline u64 stop_indirect_branch_speculation_and_save(void)
@@ -237,6 +243,7 @@ static inline u64 stop_indirect_branch_s
 	u64 val = 0;
 
 	if (static_cpu_has(X86_FEATURE_IBRS)) {
+		arch_static_assert();
 		val = native_rdmsrl(MSR_IA32_SPEC_CTRL);
 		native_wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_ENABLE_IBRS);
 	}
@@ -246,16 +253,20 @@ static inline u64 stop_indirect_branch_s
 
 static inline void restore_indirect_branch_speculation(u64 val)
 {
-	if (static_cpu_has(X86_FEATURE_IBRS))
+	if (static_cpu_has(X86_FEATURE_IBRS)) {
+		arch_static_assert();
 		native_wrmsrl(MSR_IA32_SPEC_CTRL, val);
+	}
 }
 
 void specctrl_init_ibpb(void);
 
 static inline void indirect_branch_prediction_barrier(void)
 {
-	if (static_cpu_has(X86_FEATURE_IBPB))
+	if (static_cpu_has(X86_FEATURE_IBPB)) {
+		arch_static_assert();
 		native_wrmsrl(MSR_IA32_PRED_CMD, PRED_CMD_IBPB);
+	}
 }
 
 #endif /* __ASSEMBLY__ */