x86: static_cpu_has_safe: discard dynamic check after init

Move the code to do the dynamic check to the init text section so that it is discarded after alternatives have run and a static branch has been chosen.

A new section is defined to avoid warnings with modpost due to references to init text from main text, which in this case is legitimate. All such references are patched out before init mem is discarded.

---
 arch/x86/include/asm/cpufeature.h | 33 ++++++++++++++++++++++-----------
 arch/x86/kernel/cpu/common.c      |  6 ------
 arch/x86/kernel/vmlinux.lds.S     |  6 ++++++
 3 files changed, 28 insertions(+), 17 deletions(-)

index 7ad8c94..2efbd83 100644
@@ -412,7 +412,6 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
 
 #if __GNUC__ >= 4 && defined(CONFIG_X86_FAST_FEATURE_TESTS)
 extern void warn_pre_alternatives(void);
-extern bool __static_cpu_has_safe(u16 bit);
 
 /*
  * Static testing of CPU features.  Used the same as boot_cpu_has().

@@ -502,10 +501,10 @@ static __always_inline __pure bool __static_cpu_has(u16 bit)
 		boot_cpu_has(bit)				\
 )
 
-static __always_inline __pure bool _static_cpu_has_safe(u16 bit)
+static __always_inline __pure bool _static_cpu_has_safe(u16 bit, __u32 *caps)
 {
 #ifdef CC_HAVE_ASM_GOTO
-		asm_volatile_goto("1: jmp %l[t_dynamic]\n"
+		asm_volatile_goto("1: jmp 6f\n"
 			 "2:\n"
 			 ".skip -(((5f-4f) - (2b-1b)) > 0) * "
 			         "((5f-4f) - (2b-1b)),0x90\n"

@@ -530,17 +529,22 @@ static __always_inline __pure bool _static_cpu_has_safe(u16 bit)
 			 " .byte 0\n"			/* repl len */
 			 " .byte 0\n"			/* pad len */
 			 ".previous\n"
-			 : : "i" (bit), "i" (X86_FEATURE_ALWAYS)
-			 : : t_dynamic, t_no);
+			 ".section .static_cpu_has,\"ax\"\n"
+			 "6: testl %2,%3\n"
+			 "   jnz %l[t_yes]\n"
+			 "   jmp %l[t_no]\n"
+			 ".previous\n"
+			 : : "i" (bit), "i" (X86_FEATURE_ALWAYS),
+			     "i" (1 << (bit & 31)), "m" (caps[bit/32])
+			 : : t_yes, t_no);
+	t_yes:
 		return true;
 	t_no:
 		return false;
-	t_dynamic:
-		return __static_cpu_has_safe(bit);
 #else
 		u8 flag;
 		/* Open-coded due to __stringify() in ALTERNATIVE() */
-		asm volatile("1: movb $2,%0\n"
+		asm volatile("1: jmp 7f\n"
 			     "2:\n"
 			     ".section .altinstructions,\"a\"\n"
 			     " .long 1b - .\n"		/* src offset */

@@ -572,9 +576,15 @@ static __always_inline __pure bool _static_cpu_has_safe(u16 bit)
 			     "5: movb $1,%0\n"
 			     "6:\n"
 			     ".previous\n"
+			     ".section .static_cpu_has,\"ax\"\n"
+			     "7: testl %3,%4\n"
+			     "   setnz %0\n"
+			     "   jmp 2b\n"
+			     ".previous\n"
 			     : "=qm" (flag)
-			     : "i" (bit), "i" (X86_FEATURE_ALWAYS));
-		return (flag == 2 ? __static_cpu_has_safe(bit) : flag);
+			     : "i" (bit), "i" (X86_FEATURE_ALWAYS),
+			       "i" (1 << (bit & 31)), "m" (caps[bit/32]));
+		return (flag != 0);
 #endif /* CC_HAVE_ASM_GOTO */
 }
 

@@ -582,7 +592,8 @@ static __always_inline __pure bool _static_cpu_has_safe(u16 bit)
 (								\
 	__builtin_constant_p(boot_cpu_has(bit)) ?		\
 		boot_cpu_has(bit) :				\
-		_static_cpu_has_safe(bit)			\
+		_static_cpu_has_safe(bit,			\
+			 &boot_cpu_data.x86_capability[0])	\
 )
 #else
 /*

index 37830de..897c65b 100644
@@ -1483,12 +1483,6 @@ void warn_pre_alternatives(void)
 EXPORT_SYMBOL_GPL(warn_pre_alternatives);
 #endif
 
-inline bool __static_cpu_has_safe(u16 bit)
-{
-	return boot_cpu_has(bit);
-}
-EXPORT_SYMBOL_GPL(__static_cpu_has_safe);
-
 static void bsp_resume(void)
 {
 	if (this_cpu->c_bsp_resume)

index 4f19942..4df1467 100644
@@ -195,6 +195,12 @@ SECTIONS
 	:init
 #endif
 
+	.static_cpu_has : AT(ADDR(.static_cpu_has) - LOAD_OFFSET) {
+		__static_cpu_has_start = .;
+		*(.static_cpu_has)
+		__static_cpu_has_end = .;
+	}
+
 	INIT_DATA_SECTION(16)
 
 	.x86_cpu_dev.init : AT(ADDR(.x86_cpu_dev.init) - LOAD_OFFSET) {

2.5.0

And we're doing this because...? Space savings? How much are we talkin'?

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

It saves space. The only thing left in main text is one jump instruction (5 bytes).

The old t_dynamic branch looked something like this (26 bytes in .text, not discarded):

     553:       bf 7d 00 00 00          mov    $0x7d,%edi
     558:       48 89 4d c8             mov    %rcx,-0x38(%rbp)
     55c:       e8 00 00 00 00          callq  561 <__switch_to+0xf1>
                        55d: R_X86_64_PC32      __static_cpu_has_safe-0x4
     561:       84 c0                   test   %al,%al
     563:       48 8b 4d c8             mov    -0x38(%rbp),%rcx
     567:       0f 85 77 01 00 00       jne    6e4 <__switch_to+0x274>

New (21 bytes, discarded after init):
0: f7 05 00 00 00 00 00 testl $0x20000000,0x0(%rip) #
a <.static_cpu_has+0xa>
7: 00 00 20

                        2: R_X86_64_PC32        boot_cpu_data+0x18
   a:   0f 85 00 00 00 00       jne    10 <.static_cpu_has+0x10>
                        c: R_X86_64_PC32        .text+0x4b9
  10:   e9 00 00 00 00          jmpq   15 <.static_cpu_has+0x15>
                        11: R_X86_64_PC32       .text+0x529

--
Brian Gerst

How do I measure this?

Because with my tailored config here, the only thing I'm seeing is a growth of text by 730 bytes:

before:
text data bss dec hex filename
10926552 3598944 16642048 31167544 1db9438 vmlinux

[    0.056552] Freeing SMP alternatives memory: 24K (ffffffff81dd7000 - ffffffff81ddd000)
[    2.883728] Freeing unused kernel memory: 2904K (ffffffff81b01000 - ffffffff81dd7000)

after:
text data bss dec hex filename
10927282 3598944 16642048 31168274 1db9712 vmlinux

[    0.052559] Freeing SMP alternatives memory: 24K (ffffffff81dd7000 - ffffffff81ddd000)
[    3.225318] Freeing unused kernel memory: 2904K (ffffffff81b01000 - ffffffff81dd7000)

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

It is due to page alignment padding. It was not enough to lose a whole page from .text in your case.

The size command includes any section that is marked executable in the text count, including init text. If you use readelf -S vmlinux.o instead you will notice that .text is the same size or smaller, and .static_cpu_has (which is freed after boot) is the difference.

--
Brian Gerst

If I'm understanding this correctly, the total non-init overhead from static_cpu_has_safe with your patch is five bytes. I'd imagine that the short jmp optimization in regular static_cpu_has essentially never works, which means it will also use five bytes of text, which makes me wonder whether we should just make static_cpu_has safe and remove the distinction.

Also, someone should write a little script to measure kernel size minus .init stuff.

--Andy

So nothing more got freed.

So we're talking about less than a page here?

[19] .static_cpu_has PROGBITS ffffffff81d335d3 011335d3

       00000000000002df  0000000000000000  AX       0     0     1

That's 479 bytes. Meh, it doesn't look like it is worth the trouble.

Maybe I should build an allyesconfig.

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

I think that, if we can make static_cpu_has be unconditionally safe as a result and get rid of warn_pre_alternatives, then it is worth the trouble.

--Andy

Problem with this is the additional .altinstructions entry for X86_FEATURE_ALWAYS. And sometimes you don't really need to use the _safe variant when you know you're safe.

That's off by default, behind CONFIG_X86_DEBUG_STATIC_CPU_HAS.

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

I think the two-byte optimization is the real issue if there is one at all. I don't care about the inittext, and unless I'm misremembering completely altinstructions also get ejected.

So I don't personally object to killing off the unsafe variant.

-hpa

Well, we do optimize the jumps for a couple releases now, see recompute_jump(). So we would always get the short JMP, if made possible by how gcc lays out the code so that the target is not far away. If the JMP to the dynamic branch doesn't get nopped out anyway, that is.

We have "debug-alternative" to dump exactly what happens during patching.

Ah yes, they do.

Ok.

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

The optimization has always been there, the question is how often it actually kicks in.

Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

Luckily, I have this disassembler tool which dumps the alternatives sections in a more readable format. I can dump all the static_cpu_has() call sites tomorrow and we can see what gcc generates.

;-}

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Ok, below is the list of all 12(!) places where static_cpu_has() has generated a 2-byte JMP. Which has saved us a whopping 36 bytes! On an x86_64 allyesconfig!

Patch removing it below. I better look at a 32-bit allyesconfig too first, though.

old insn VA: 0xffffffff8146b5ac, CPU feat: X86_FEATURE_PCOMMIT, size: 2, padlen: 0 wmb_pmem:
ffffffff8146b5ac: eb 02 jmp ffffffff8146b5b0
repl insn: 0xffffffff8ca7c651, size: 0

old insn VA: 0xffffffff8200dcd6, CPU feat: X86_FEATURE_PCOMMIT, size: 2, padlen: 0 arch_has_wmb_pmem:
ffffffff8200dcd6: eb 02 jmp ffffffff8200dcda
repl insn: 0xffffffff8ca7ebd8, size: 0

old insn VA: 0xffffffff828afe51, CPU feat: X86_FEATURE_PCOMMIT, size: 2, padlen: 0 arch_has_wmb_pmem:
ffffffff828afe51: eb 02 jmp ffffffff828afe55
repl insn: 0xffffffff8ca80f98, size: 0

old insn VA: 0xffffffff81072f77, CPU feat: X86_FEATURE_NRIPS, size: 2, padlen: 0 rdpmc_interception:
ffffffff81072f77: eb 4d jmp ffffffff81072fc6
repl insn: 0xffffffff8ca79f6b, size: 0

old insn VA: 0xffffffff8107437d, CPU feat: X86_FEATURE_NRIPS, size: 2, padlen: 0 svm_queue_exception:
ffffffff8107437d: eb 6f jmp ffffffff810743ee
repl insn: 0xffffffff8ca79f9f, size: 0

old insn VA: 0xffffffff8107741b, CPU feat: X86_FEATURE_NRIPS, size: 2, padlen: 0 svm_check_intercept:
ffffffff8107741b: eb 67 jmp ffffffff81077484
repl insn: 0xffffffff8ca79fed, size: 0

old insn VA: 0xffffffff8107741b, CPU feat: X86_FEATURE_NRIPS, size: 2, padlen: 0 svm_check_intercept:
ffffffff8107741b: eb 67 jmp ffffffff81077484
repl insn: 0xffffffff8ca79fed, size: 0

old insn VA: 0xffffffff81075c4f, CPU feat: X86_FEATURE_TSCRATEMSR, size: 2, padlen: 0 svm_hardware_enable:
ffffffff81075c4f: eb 57 jmp ffffffff81075ca8
repl insn: 0xffffffff8ca79fb9, size: 0

old insn VA: 0xffffffff81072c00, CPU feat: X86_FEATURE_DECODEASSISTS, size: 2, padlen: 0 invlpg_interception:
ffffffff81072c00: eb 55 jmp ffffffff81072c57
repl insn: 0xffffffff8ca79f51, size: 0

old insn VA: 0xffffffff8107097c, CPU feat: X86_FEATURE_FLUSHBYASID, size: 2, padlen: 0 svm_flush_tlb:
ffffffff8107097c: eb 35 jmp ffffffff810709b3
repl insn: 0xffffffff8ca79ee9, size: 0

old insn VA: 0xffffffff8108c0d0, CPU feat: X86_BUG_SYSRET_SS_ATTRS, size: 2, padlen: 0 __switch_to:
ffffffff8108c0d0: eb 70 jmp ffffffff8108c142
repl insn: 0xffffffff8ca7a1a7, size: 0

old insn VA: 0xffffffff81075d90, CPU feat: X86_BUG_AMD_TLB_MMATCH, size: 2, padlen: 0 svm_hardware_enable:
ffffffff81075d90: eb 7c jmp ffffffff81075e0e
repl insn: 0xffffffff8ca79fd3, size: 0

---

index 9b18ed9..68a2d1f 100644
@@ -350,16 +350,6 @@ config DEBUG_IMR_SELFTEST
 
 	  If unsure say N here.
 
-config X86_DEBUG_STATIC_CPU_HAS
-	bool "Debug alternatives"
-	depends on DEBUG_KERNEL
-	---help---
-	  This option causes additional code to be generated which
-	  fails if static_cpu_has() is used before alternatives have
-	  run.
-
-	  If unsure, say N.
-
 config X86_DEBUG_FPU
 	bool "Debug the x86 FPU code"
 	depends on DEBUG_KERNEL

index 0366374..c2d7a97 100644
@@ -477,7 +477,7 @@ __visible long do_fast_syscall_32(struct pt_regs *regs)
 	 * We don't allow syscalls at all from VM86 mode, but we still
 	 * need to check VM, because we might be returning from sys_vm86.
 	 */
-	return static_cpu_has(X86_FEATURE_SEP) &&
+	return static_cpu_has_safe(X86_FEATURE_SEP) &&
 		regs->cs == __USER_CS && regs->ss == __USER_DS &&
 		regs->ip == landing_pad &&
 		(regs->flags & (X86_EFLAGS_RF | X86_EFLAGS_TF | X86_EFLAGS_VM)) == 0;

index 69f1366..2fb511b 100644
@@ -114,8 +114,8 @@ GET_SEED(arch_get_random_seed_int, unsigned int, RDSEED_INT, ASM_NOP4);
 
 #endif /* CONFIG_X86_64 */
 
-#define arch_has_random()	static_cpu_has(X86_FEATURE_RDRAND)
-#define arch_has_random_seed()	static_cpu_has(X86_FEATURE_RDSEED)
+#define arch_has_random()	static_cpu_has_safe(X86_FEATURE_RDRAND)
+#define arch_has_random_seed()	static_cpu_has_safe(X86_FEATURE_RDSEED)
 
 #else
 

index c70689b..2bd6e47 100644
@@ -96,7 +96,7 @@ static inline void __sti_mwait(unsigned long eax, unsigned long ecx)
 static inline void mwait_idle_with_hints(unsigned long eax, unsigned long ecx)
 {
 	if (!current_set_polling_and_test()) {
-		if (static_cpu_has_bug(X86_BUG_CLFLUSH_MONITOR)) {
+		if (static_cpu_has_bug_safe(X86_BUG_CLFLUSH_MONITOR)) {
 			mb();
 			clflush((void *)&current_thread_info()->flags);
 			mb();

index 1544fab..5d7abb4 100644
@@ -142,7 +142,7 @@ static inline bool __arch_has_wmb_pmem(void)
 	 * We require that wmb() be an 'sfence', that is only guaranteed on
 	 * 64-bit builds
 	 */
-	return static_cpu_has(X86_FEATURE_PCOMMIT);
+	return static_cpu_has_safe(X86_FEATURE_PCOMMIT);
 }
 #endif /* CONFIG_ARCH_HAS_PMEM_API */
 #endif /* __ASM_X86_PMEM_H__ */

index eaba080..f456616 100644
@@ -43,7 +43,7 @@ static inline void queued_spin_unlock(struct qspinlock *lock)
 #define virt_spin_lock virt_spin_lock
 static inline bool virt_spin_lock(struct qspinlock *lock)
 {
-	if (!static_cpu_has(X86_FEATURE_HYPERVISOR))
+	if (!static_cpu_has_safe(X86_FEATURE_HYPERVISOR))
 		return false;
 
 	/*

index 18ca99f..f8a6cfb 100644
@@ -32,11 +32,11 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
 		   "fpu_exception\t: %s\n"
 		   "cpuid level\t: %d\n"
 		   "wp\t\t: %s\n",
-		   static_cpu_has_bug(X86_BUG_FDIV) ? "yes" : "no",
-		   static_cpu_has_bug(X86_BUG_F00F) ? "yes" : "no",
-		   static_cpu_has_bug(X86_BUG_COMA) ? "yes" : "no",
-		   static_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
-		   static_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
+		   static_cpu_has_bug_safe(X86_BUG_FDIV) ? "yes" : "no",
+		   static_cpu_has_bug_safe(X86_BUG_F00F) ? "yes" : "no",
+		   static_cpu_has_bug_safe(X86_BUG_COMA) ? "yes" : "no",
+		   static_cpu_has_safe(X86_FEATURE_FPU) ? "yes" : "no",
+		   static_cpu_has_safe(X86_FEATURE_FPU) ? "yes" : "no",
 		   c->cpuid_level,
 		   c->wp_works_ok ? "yes" : "no");
 }

index 0bc3490..9b0163c 100644
@@ -275,7 +275,7 @@ int fpregs_get(struct task_struct *target, const struct user_regset *regset,
 
 	fpu__activate_fpstate_read(fpu);
 
-	if (!static_cpu_has(X86_FEATURE_FPU))
+	if (!static_cpu_has_safe(X86_FEATURE_FPU))
 		return fpregs_soft_get(target, regset, pos, count, kbuf, ubuf);
 
 	if (!cpu_has_fxsr)

@@ -306,7 +306,7 @@ int fpregs_set(struct task_struct *target, const struct user_regset *regset,
 	fpu__activate_fpstate_write(fpu);
 	fpstate_sanitize_xstate(fpu);
 
-	if (!static_cpu_has(X86_FEATURE_FPU))
+	if (!static_cpu_has_safe(X86_FEATURE_FPU))
 		return fpregs_soft_set(target, regset, pos, count, kbuf, ubuf);
 
 	if (!cpu_has_fxsr)

index 31c6a60..05b6ede 100644
@@ -162,7 +162,7 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
 	if (!access_ok(VERIFY_WRITE, buf, size))
 		return -EACCES;
 
-	if (!static_cpu_has(X86_FEATURE_FPU))
+	if (!static_cpu_has_safe(X86_FEATURE_FPU))
 		return fpregs_soft_get(current, NULL, 0,
 			sizeof(struct user_i387_ia32_struct), NULL,
 			(struct _fpstate_32 __user *) buf) ? -1 : 1;

@@ -267,7 +267,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 
 	fpu__activate_curr(fpu);
 
-	if (!static_cpu_has(X86_FEATURE_FPU))
+	if (!static_cpu_has_safe(X86_FEATURE_FPU))
 		return fpregs_soft_set(current, NULL,
 				       0, sizeof(struct user_i387_ia32_struct),
 				       NULL, buf) != 0;

index b9d99e0..c49a284 100644
@@ -411,7 +411,7 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
 		     task_thread_info(prev_p)->flags & _TIF_WORK_CTXSW_PREV))
 		__switch_to_xtra(prev_p, next_p, tss);
 
-	if (static_cpu_has_bug(X86_BUG_SYSRET_SS_ATTRS)) {
+	if (static_cpu_has_bug_safe(X86_BUG_SYSRET_SS_ATTRS)) {
 		/*
 		 * AMD CPUs have a misfeature: SYSRET sets the SS selector but
 		 * does not update the cached descriptor.  As a result, if we

index c8eda14..89d5ad7 100644
@@ -32,7 +32,7 @@ static inline bool guest_cpuid_has_xsave(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpuid_entry2 *best;
 
-	if (!static_cpu_has(X86_FEATURE_XSAVE))
+	if (!static_cpu_has_safe(X86_FEATURE_XSAVE))
 		return false;
 
 	best = kvm_find_cpuid_entry(vcpu, 1, 0);

index c13a64b..1892bdd 100644
@@ -516,7 +516,7 @@ static void skip_emulated_instruction(struct kvm_vcpu *vcpu)
 	struct vcpu_svm *svm = to_svm(vcpu);
 
 	if (svm->vmcb->control.next_rip != 0) {
-		WARN_ON_ONCE(!static_cpu_has(X86_FEATURE_NRIPS));
+		WARN_ON_ONCE(!static_cpu_has_safe(X86_FEATURE_NRIPS));
 		svm->next_rip = svm->vmcb->control.next_rip;
 	}
 

@@ -548,7 +548,7 @@ static void svm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr,
 	    nested_svm_check_exception(svm, nr, has_error_code, error_code))
 		return;
 
-	if (nr == BP_VECTOR && !static_cpu_has(X86_FEATURE_NRIPS)) {
+	if (nr == BP_VECTOR && !static_cpu_has_safe(X86_FEATURE_NRIPS)) {
 		unsigned long rip, old_rip = kvm_rip_read(&svm->vcpu);
 
 		/*

@@ -577,7 +577,7 @@ static void svm_init_erratum_383(void)
 	int err;
 	u64 val;
 
-	if (!static_cpu_has_bug(X86_BUG_AMD_TLB_MMATCH))
+	if (!static_cpu_has_bug_safe(X86_BUG_AMD_TLB_MMATCH))
 		return;
 
 	/* Use _safe variants to not break nested virtualization */

@@ -631,7 +631,7 @@ static int has_svm(void)
 static void svm_hardware_disable(void)
 {
 	/* Make sure we clean up behind us */
-	if (static_cpu_has(X86_FEATURE_TSCRATEMSR))
+	if (static_cpu_has_safe(X86_FEATURE_TSCRATEMSR))
 		wrmsrl(MSR_AMD64_TSC_RATIO, TSC_RATIO_DEFAULT);
 
 	cpu_svm_disable();

@@ -674,7 +674,7 @@ static int svm_hardware_enable(void)
 
 	wrmsrl(MSR_VM_HSAVE_PA, page_to_pfn(sd->save_area) << PAGE_SHIFT);
 
-	if (static_cpu_has(X86_FEATURE_TSCRATEMSR)) {
+	if (static_cpu_has_safe(X86_FEATURE_TSCRATEMSR)) {
 		wrmsrl(MSR_AMD64_TSC_RATIO, TSC_RATIO_DEFAULT);
 		__this_cpu_write(current_tsc_ratio, TSC_RATIO_DEFAULT);
 	}

@@ -1233,7 +1233,7 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
 		rdmsrl(host_save_user_msrs[i], svm->host_user_msrs[i]);
 
-	if (static_cpu_has(X86_FEATURE_TSCRATEMSR)) {
+	if (static_cpu_has_safe(X86_FEATURE_TSCRATEMSR)) {
 		u64 tsc_ratio = vcpu->arch.tsc_scaling_ratio;
 		if (tsc_ratio != __this_cpu_read(current_tsc_ratio)) {
 			__this_cpu_write(current_tsc_ratio, tsc_ratio);

@@ -1241,7 +1241,7 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 		}
 	}
 	/* This assumes that the kernel never uses MSR_TSC_AUX */
-	if (static_cpu_has(X86_FEATURE_RDTSCP))
+	if (static_cpu_has_safe(X86_FEATURE_RDTSCP))
 		wrmsrl(MSR_TSC_AUX, svm->tsc_aux);
 }
 

@@ -2806,7 +2806,7 @@ static int iret_interception(struct vcpu_svm *svm)
 
 static int invlpg_interception(struct vcpu_svm *svm)
 {
-	if (!static_cpu_has(X86_FEATURE_DECODEASSISTS))
+	if (!static_cpu_has_safe(X86_FEATURE_DECODEASSISTS))
 		return emulate_instruction(&svm->vcpu, 0) == EMULATE_DONE;
 
 	kvm_mmu_invlpg(&svm->vcpu, svm->vmcb->control.exit_info_1);

@@ -2823,7 +2823,7 @@ static int rdpmc_interception(struct vcpu_svm *svm)
 {
 	int err;
 
-	if (!static_cpu_has(X86_FEATURE_NRIPS))
+	if (!static_cpu_has_safe(X86_FEATURE_NRIPS))
 		return emulate_on_interception(svm);
 
 	err = kvm_rdpmc(&svm->vcpu);

@@ -2864,7 +2864,7 @@ static int cr_interception(struct vcpu_svm *svm)
 	unsigned long val;
 	int err;
 
-	if (!static_cpu_has(X86_FEATURE_DECODEASSISTS))
+	if (!static_cpu_has_safe(X86_FEATURE_DECODEASSISTS))
 		return emulate_on_interception(svm);
 
 	if (unlikely((svm->vmcb->control.exit_info_1 & CR_VALID) == 0))

@@ -3710,7 +3710,7 @@ static void svm_flush_tlb(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
-	if (static_cpu_has(X86_FEATURE_FLUSHBYASID))
+	if (static_cpu_has_safe(X86_FEATURE_FLUSHBYASID))
 		svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ASID;
 	else
 		svm->asid_generation--;

@@ -4282,7 +4282,7 @@ static int svm_check_intercept(struct kvm_vcpu *vcpu,
 	}
 
 	/* TODO: Advertise NRIPS to guest hypervisor unconditionally */
-	if (static_cpu_has(X86_FEATURE_NRIPS))
+	if (static_cpu_has_safe(X86_FEATURE_NRIPS))
 		vmcb->control.next_rip  = info->next_rip;
 	vmcb->control.exit_code = icpt_info.exit_code;
 	vmexit = nested_svm_exit_handled(svm);

index e2951b6..4bef603 100644
@@ -8993,7 +8993,7 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 	if (cpu_has_secondary_exec_ctrls())
 		vmcs_set_secondary_exec_control(secondary_exec_ctl);
 
-	if (static_cpu_has(X86_FEATURE_PCOMMIT) && nested) {
+	if (static_cpu_has_safe(X86_FEATURE_PCOMMIT) && nested) {
 		if (guest_cpuid_has_pcommit(vcpu))
 			vmx->nested.nested_vmx_secondary_ctls_high |=
 				SECONDARY_EXEC_PCOMMIT;

index eef44d9..711e9bc 100644
@@ -1033,7 +1033,7 @@ static inline bool smap_violation(int error_code, struct pt_regs *regs)
 	if (!IS_ENABLED(CONFIG_X86_SMAP))
 		return false;
 
-	if (!static_cpu_has(X86_FEATURE_SMAP))
+	if (!static_cpu_has_safe(X86_FEATURE_SMAP))
 		return false;
 
 	if (error_code & PF_USER)

index 55d38cf..191650f 100644
@@ -275,7 +275,7 @@ static void do_inject(void)
 	 * only on the node base core. Refer to D18F3x44[NbMcaToMstCpuEn] for
 	 * Fam10h and later BKDGs.
 	 */
-	if (static_cpu_has(X86_FEATURE_AMD_DCM) && b == 4) {
+	if (static_cpu_has_safe(X86_FEATURE_AMD_DCM) && b == 4) {
 		toggle_nb_mca_mst_cpu(amd_get_nb_id(cpu));
 		cpu = get_nbc_for_node(amd_get_nb_id(cpu));
 	}

index f6b79ab..87b0cf4 100644
@@ -115,7 +115,7 @@ static int __init amd_freq_sensitivity_init(void)
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
 		return -ENODEV;
 
-	if (!static_cpu_has(X86_FEATURE_PROC_FEEDBACK))
+	if (!static_cpu_has_safe(X86_FEATURE_PROC_FEEDBACK))
 		return -ENODEV;
 
 	if (rdmsrl_safe(MSR_AMD64_FREQ_SENSITIVITY_ACTUAL, &val))

index 0b5bf13..aa71612 100644
@@ -1193,7 +1193,7 @@ static int powernowk8_init(void)
 	unsigned int i, supported_cpus = 0;
 	int ret;
 
-	if (static_cpu_has(X86_FEATURE_HW_PSTATE)) {
+	if (static_cpu_has_safe(X86_FEATURE_HW_PSTATE)) {
 		__request_acpi_cpufreq();
 		return -ENODEV;
 	}

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Yap, no 2-byte jumps in the static_cpu_has()-generated code on an 32-bit allyesconfig build.

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Andy has a point: I should simply drop static_cpu_has and kill the "_safe" suffix of the remaining variant:

---
 arch/x86/Kconfig.debug               | 10 ----
 arch/x86/include/asm/cpufeature.h    | 99 +++---------------------------------
 arch/x86/include/asm/fpu/internal.h  | 14 ++---
 arch/x86/kernel/apic/apic_numachip.c |  4 +-
 arch/x86/kernel/cpu/common.c         |  4 +-
 arch/x86/kernel/vm86_32.c            |  2 +-
 drivers/cpufreq/intel_pstate.c       |  2 +-
 fs/btrfs/disk-io.c                   |  2 +-
 8 files changed, 21 insertions(+), 116 deletions(-)

index 9b18ed9..68a2d1f 100644
@@ -350,16 +350,6 @@ config DEBUG_IMR_SELFTEST
 
 	  If unsure say N here.
 
-config X86_DEBUG_STATIC_CPU_HAS
-	bool "Debug alternatives"
-	depends on DEBUG_KERNEL
-	---help---
-	  This option causes additional code to be generated which
-	  fails if static_cpu_has() is used before alternatives have
-	  run.
-
-	  If unsure, say N.
-
 config X86_DEBUG_FPU
 	bool "Debug the x86 FPU code"
 	depends on DEBUG_KERNEL

index 0fd440d..97022dd 100644
@@ -58,22 +58,22 @@ extern u64 fpu__get_supported_xfeatures_mask(void);
  */
 static __always_inline __pure bool use_eager_fpu(void)
 {
-	return static_cpu_has_safe(X86_FEATURE_EAGER_FPU);
+	return static_cpu_has(X86_FEATURE_EAGER_FPU);
 }
 
 static __always_inline __pure bool use_xsaveopt(void)
 {
-	return static_cpu_has_safe(X86_FEATURE_XSAVEOPT);
+	return static_cpu_has(X86_FEATURE_XSAVEOPT);
 }
 
 static __always_inline __pure bool use_xsave(void)
 {
-	return static_cpu_has_safe(X86_FEATURE_XSAVE);
+	return static_cpu_has(X86_FEATURE_XSAVE);
 }
 
 static __always_inline __pure bool use_fxsr(void)
 {
-	return static_cpu_has_safe(X86_FEATURE_FXSR);
+	return static_cpu_has(X86_FEATURE_FXSR);
 }
 
 /*

@@ -300,7 +300,7 @@ static inline void copy_xregs_to_kernel_booting(struct xregs_state *xstate)
 
 	WARN_ON(system_state != SYSTEM_BOOTING);
 
-	if (static_cpu_has_safe(X86_FEATURE_XSAVES))
+	if (static_cpu_has(X86_FEATURE_XSAVES))
 		XSTATE_OP(XSAVES, xstate, lmask, hmask, err);
 	else
 		XSTATE_OP(XSAVE, xstate, lmask, hmask, err);

@@ -322,7 +322,7 @@ static inline void copy_kernel_to_xregs_booting(struct xregs_state *xstate)
 
 	WARN_ON(system_state != SYSTEM_BOOTING);
 
-	if (static_cpu_has_safe(X86_FEATURE_XSAVES))
+	if (static_cpu_has(X86_FEATURE_XSAVES))
 		XSTATE_OP(XRSTORS, xstate, lmask, hmask, err);
 	else
 		XSTATE_OP(XRSTOR, xstate, lmask, hmask, err);

@@ -460,7 +460,7 @@ static inline void copy_kernel_to_fpregs(union fpregs_state *fpstate)
 	 * pending. Clear the x87 state here by setting it to fixed values.
 	 * "m" is a random variable that should be in L1.
 	 */
-	if (unlikely(static_cpu_has_bug_safe(X86_BUG_FXSAVE_LEAK))) {
+	if (unlikely(static_cpu_has_bug(X86_BUG_FXSAVE_LEAK))) {
 		asm volatile(
 			"fnclex\n\t"
 			"emms\n\t"

index c80c02c..ab5c2c6 100644
@@ -30,7 +30,7 @@ static unsigned int numachip1_get_apic_id(unsigned long x)
 	unsigned long value;
 	unsigned int id = (x >> 24) & 0xff;
 
-	if (static_cpu_has_safe(X86_FEATURE_NODEID_MSR)) {
+	if (static_cpu_has(X86_FEATURE_NODEID_MSR)) {
 		rdmsrl(MSR_FAM10H_NODE_ID, value);
 		id |= (value << 2) & 0xff00;
 	}

@@ -178,7 +178,7 @@ static void fixup_cpu_id(struct cpuinfo_x86 *c, int node)
 	this_cpu_write(cpu_llc_id, node);
 
 	/* Account for nodes per socket in multi-core-module processors */
-	if (static_cpu_has_safe(X86_FEATURE_NODEID_MSR)) {
+	if (static_cpu_has(X86_FEATURE_NODEID_MSR)) {
 		rdmsrl(MSR_FAM10H_NODE_ID, val);
 		nodes = ((val >> 3) & 7) + 1;
 	}

index 37830de..a57ec0d 100644
@@ -1483,11 +1483,11 @@ void warn_pre_alternatives(void)
 EXPORT_SYMBOL_GPL(warn_pre_alternatives);
 #endif
 
-inline bool __static_cpu_has_safe(u16 bit)
+inline bool __static_cpu_has(u16 bit)
 {
 	return boot_cpu_has(bit);
 }
-EXPORT_SYMBOL_GPL(__static_cpu_has_safe);
+EXPORT_SYMBOL_GPL(__static_cpu_has);
 
 static void bsp_resume(void)
 {

index e574b85..3dce1ca 100644
@@ -362,7 +362,7 @@ static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus)
 	/* make room for real-mode segments */
 	tsk->thread.sp0 += 16;
 
-	if (static_cpu_has_safe(X86_FEATURE_SEP))
+	if (static_cpu_has(X86_FEATURE_SEP))
 		tsk->thread.sysenter_cs = 0;
 
 	load_sp0(tss, &tsk->thread);

index cd83d47..3a4b39a 100644
@@ -1431,7 +1431,7 @@ static int __init intel_pstate_init(void)
 	if (!all_cpu_data)
 		return -ENOMEM;
 
-	if (static_cpu_has_safe(X86_FEATURE_HWP) && !no_hwp) {
+	if (static_cpu_has(X86_FEATURE_HWP) && !no_hwp) {
 		pr_info("intel_pstate: HWP enabled\n");
 		hwp_active++;
 	}

index e99ccd6..87ce612 100644
@@ -924,7 +924,7 @@ static int check_async_write(struct inode *inode, unsigned long bio_flags)
 	if (bio_flags & EXTENT_BIO_TREE_LOG)
 		return 0;
 #ifdef CONFIG_X86
-	if (static_cpu_has_safe(X86_FEATURE_XMM4_2))
+	if (static_cpu_has(X86_FEATURE_XMM4_2))
 		return 0;
 #endif
 	return 1;

1.8.5.6

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Is this on top of Brian's patch? I think we should do both or neither.

--Andy

Of course. I'll prep a branch tomorrow for the build bot to smoke-test.

Sent from a small device: formatting sux and brevity is inevitable. 

This is the only one I could possibly imagine mattering. Would it be possible to get the disassembly here?

-hpa

Sure, I've kept the vmlinux, see below:

ffffffff8108c0d0: eb 70 jmp ffffffff8108c142 <__switch_to+0x838>

jumps to:

ffffffff8108c142: 48 ff 05 b7 08 aa 0b incq 0xbaa08b7(%rip)

which is something-gcov. In any case, it jumps over the SS fixup code:

        if (static_cpu_has_bug(X86_BUG_SYSRET_SS_ATTRS)) {

		...

                unsigned short ss_sel;
                savesegment(ss, ss_sel);
                if (ss_sel != __KERNEL_DS)
                        loadsegment(ss, __KERNEL_DS);
        }

I guess we could fix that with an alternative_call() where @oldinstr is empty so that on !X86_BUG_SYSRET_SS_ATTRS machines we simply return and on the others, we do a CALL fixup_ss or so.

We have to pay attention to clobbers, though, similar to the POPCNT fun in __arch_hweight32().

ffffffff8108b90a <__switch_to>:

ffffffff8108b90a:       55                      push   %rbp
ffffffff8108b90b:       48 8d 87 c0 2d 00 00    lea    0x2dc0(%rdi),%rax
ffffffff8108b912:       48 c7 c1 80 f6 1d 00    mov    $0x1df680,%rcx
ffffffff8108b919:       48 89 e5                mov    %rsp,%rbp
ffffffff8108b91c:       41 57                   push   %r15
ffffffff8108b91e:       45 31 ff                xor    %r15d,%r15d
ffffffff8108b921:       41 56                   push   %r14

...

ffffffff8108c0c2:       48 ff 05 07 09 aa 0b    incq   0xbaa0907(%rip)        # ffffffff8cb2c9d0 <__gcov0.__switch_to+0xc0>
ffffffff8108c0c9:       48 ff 05 08 09 aa 0b    incq   0xbaa0908(%rip)        # ffffffff8cb2c9d8 <__gcov0.__switch_to+0xc8>
ffffffff8108c0d0:       eb 70                   jmp    ffffffff8108c142 <__switch_to+0x838>
ffffffff8108c0d2:       48 ff 05 07 09 aa 0b    incq   0xbaa0907(%rip)        # ffffffff8cb2c9e0 <__gcov0.__switch_to+0xd0>
ffffffff8108c0d9:       48 ff 05 08 09 aa 0b    incq   0xbaa0908(%rip)        # ffffffff8cb2c9e8 <__gcov0.__switch_to+0xd8>
ffffffff8108c0e0:       48 ff 05 11 09 aa 0b    incq   0xbaa0911(%rip)        # ffffffff8cb2c9f8 <__gcov0.__switch_to+0xe8>
ffffffff8108c0e7:       66 8c d0                mov    %ss,%ax
ffffffff8108c0ea:       66 83 f8 18             cmp    $0x18,%ax
ffffffff8108c0ee:       75 17                   jne    ffffffff8108c107 <__switch_to+0x7fd>
ffffffff8108c0f0:       eb 57                   jmp    ffffffff8108c149 <__switch_to+0x83f>
ffffffff8108c0f2:       48 ff 05 cf 08 aa 0b    incq   0xbaa08cf(%rip)        # ffffffff8cb2c9c8 <__gcov0.__switch_to+0xb8>
ffffffff8108c0f9:       e8 db b0 01 00          callq  ffffffff810a71d9 <warn_pre_alternatives>
ffffffff8108c0fe:       48 ff 05 eb 08 aa 0b    incq   0xbaa08eb(%rip)        # ffffffff8cb2c9f0 <__gcov0.__switch_to+0xe0>
ffffffff8108c105:       eb 3b                   jmp    ffffffff8108c142 <__switch_to+0x838>
ffffffff8108c107:       48 ff 05 fa 08 aa 0b    incq   0xbaa08fa(%rip)        # ffffffff8cb2ca08 <__gcov0.__switch_to+0xf8>
ffffffff8108c10e:       b8 18 00 00 00          mov    $0x18,%eax
ffffffff8108c113:       8e d0                   mov    %eax,%ss
ffffffff8108c115:       48 ff 05 f4 08 aa 0b    incq   0xbaa08f4(%rip)        # ffffffff8cb2ca10 <__gcov0.__switch_to+0x100>
ffffffff8108c11c:       eb 2b                   jmp    ffffffff8108c149 <__switch_to+0x83f>
ffffffff8108c11e:       48 ff 05 9b 02 aa 0b    incq   0xbaa029b(%rip)        # ffffffff8cb2c3c0 <__gcov0.copy_xregs_to_kernel+0x30>
ffffffff8108c125:       e9 91 f9 ff ff          jmpq   ffffffff8108babb <__switch_to+0x1b1>
ffffffff8108c12a:       48 ff 05 bf 02 aa 0b    incq   0xbaa02bf(%rip)        # ffffffff8cb2c3f0 <__gcov0.copy_kernel_to_xregs+0x20>
ffffffff8108c131:       e9 84 fe ff ff          jmpq   ffffffff8108bfba <__switch_to+0x6b0>
ffffffff8108c136:       48 ff 05 43 02 aa 0b    incq   0xbaa0243(%rip)        # ffffffff8cb2c380 <__gcov0.copy_kernel_to_fxregs+0x20>
ffffffff8108c13d:       e9 db fe ff ff          jmpq   ffffffff8108c01d <__switch_to+0x713>
ffffffff8108c142:       48 ff 05 b7 08 aa 0b    incq   0xbaa08b7(%rip)        # ffffffff8cb2ca00 <__gcov0.__switch_to+0xf0>
ffffffff8108c149:       48 83 c4 28             add    $0x28,%rsp
ffffffff8108c14d:       4c 89 e0                mov    %r12,%rax
ffffffff8108c150:       5b                      pop    %rbx
ffffffff8108c151:       41 5c                   pop    %r12
ffffffff8108c153:       41 5d                   pop    %r13
ffffffff8108c155:       41 5e                   pop    %r14
ffffffff8108c157:       41 5f                   pop    %r15
ffffffff8108c159:       5d                      pop    %rbp
ffffffff8108c15a:       c3                      retq

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Ah. What would be even more of a win would be to rebias static_cpu_has_bug() so that the fallthrough case is the functional one. Easily done by reversing the labels.

Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

By reversing you mean this:

---

index 77c51f4c15b7..49fa56f2b083 100644
@@ -174,10 +174,10 @@ static __always_inline __pure bool _static_cpu_has(u16 bit)
                             [bitnum] "i" (1 << (bit & 7)),
                             [cap_word] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
                         : : t_yes, t_no);
-       t_yes:
-               return true;
        t_no:
                return false;
+       t_yes:
+               return true;
 #else
                return boot_cpu_has(bit);
 #endif /* CC_HAVE_ASM_GOTO */

---

?

In any case, here's what happens with the current patchset:

vmlinux:

ffffffff8100472a:       e9 50 0e de 00          jmpq   ffffffff81de557f <__alt_instructions_end+0x7aa>
ffffffff8100472f:       66 8c d0                mov    %ss,%ax
ffffffff81004732:       66 83 f8 18             cmp    $0x18,%ax
ffffffff81004736:       74 07                   je     ffffffff8100473f <__switch_to+0x2ef>
ffffffff81004738:       b8 18 00 00 00          mov    $0x18,%eax
ffffffff8100473d:       8e d0                   mov    %eax,%ss
ffffffff8100473f:       48 83 c4 18             add    $0x18,%rsp
ffffffff81004743:       4c 89 e0                mov    %r12,%rax
ffffffff81004746:       5b                      pop    %rbx
ffffffff81004747:       41 5c                   pop    %r12
ffffffff81004749:       41 5d                   pop    %r13
ffffffff8100474b:       41 5e                   pop    %r14
ffffffff8100474d:       41 5f                   pop    %r15
ffffffff8100474f:       5d                      pop    %rbp
ffffffff81004750:       c3                      retq

That first JMP above sends us to the dynamic section which is in asm now:

ffffffff81de557f:       f6 05 8f de d1 ff 01    testb  $0x1,-0x2e2171(%rip)        # ffffffff81b03415 <boot_cpu_data+0x55>
ffffffff81de5586:       0f 85 a3 f1 21 ff       jne    ffffffff8100472f <__switch_to+0x2df>
ffffffff81de558c:       e9 ae f1 21 ff          jmpq   ffffffff8100473f <__switch_to+0x2ef>

After X86_FEATURE_ALWAYS patching, that first JMP has become a 2-byte JMP:

[    0.306333] apply_alternatives: feat: 3*32+21, old: (ffffffff8100472a, len: 5), repl: (ffffffff81de4e12, len: 5), pad: 0
[    0.308005] ffffffff8100472a: old_insn: e9 50 0e de 00
[    0.312012] ffffffff81de4e12: rpl_insn: e9 28 f9 21 ff
[    0.318201] recompute_jump: target RIP: ffffffff8100473f, new_displ: 0x15
[    0.320007] recompute_jump: final displ: 0x00000013, JMP 0xffffffff8100473f
[    0.324005] ffffffff8100472a: final_insn: eb 13 0f 1f 00

so basically we jump over the %ss fixup:

ffffffff8100472a: eb 13 0f 1f 00 jmp ffffffff8100473f

ffffffff8100472f:       66 8c d0                mov    %ss,%ax
ffffffff81004732:       66 83 f8 18             cmp    $0x18,%ax
ffffffff81004736:       74 07                   je     ffffffff8100473f <__switch_to+0x2ef>
ffffffff81004738:       b8 18 00 00 00          mov    $0x18,%eax
ffffffff8100473d:       8e d0                   mov    %eax,%ss
ffffffff8100473f:       48 83 c4 18             add    $0x18,%rsp		<----
ffffffff81004743:       4c 89 e0                mov    %r12,%rax
ffffffff81004746:       5b                      pop    %rbx
ffffffff81004747:       41 5c                   pop    %r12
ffffffff81004749:       41 5d                   pop    %r13
ffffffff8100474b:       41 5e                   pop    %r14
ffffffff8100474d:       41 5f                   pop    %r15
ffffffff8100474f:       5d                      pop    %rbp
ffffffff81004750:       c3                      retq

After X86_BUG_SYSRET_SS_ATTRS patching:

[    0.330367] apply_alternatives: feat: 16*32+8, old: (ffffffff8100472a, len: 5), repl: (ffffffff81de3996, len: 0), pad: 0
[    0.332005] ffffffff8100472a: old_insn: eb 13 0f 1f 00
[    0.338332] ffffffff8100472a: final_insn: 0f 1f 44 00 00

ffffffff8100472a: 0f 1f 44 00 00 nop

ffffffff8100472f:       66 8c d0                mov    %ss,%ax
ffffffff81004732:       66 83 f8 18             cmp    $0x18,%ax
ffffffff81004736:       74 07                   je     ffffffff8100473f <__switch_to+0x2ef>
ffffffff81004738:       b8 18 00 00 00          mov    $0x18,%eax
ffffffff8100473d:       8e d0                   mov    %eax,%ss
ffffffff8100473f:       48 83 c4 18             add    $0x18,%rsp
ffffffff81004743:       4c 89 e0                mov    %r12,%rax
ffffffff81004746:       5b                      pop    %rbx
ffffffff81004747:       41 5c                   pop    %r12
ffffffff81004749:       41 5d                   pop    %r13
ffffffff8100474b:       41 5e                   pop    %r14
ffffffff8100474d:       41 5f                   pop    %r15
ffffffff8100474f:       5d                      pop    %rbp
ffffffff81004750:       c3                      retq

So the penalty for the !X86_BUG_SYSRET_SS_ATTRS CPUs is a 2-byte JMP. Do we care?

In the case we do, we could do this:

JMP ss_fixup
ret:

	RET return prev_p;

ss_fixup:
<fixup SS>
jmp ret

and the !X86_BUG_SYSRET_SS_ATTRS CPUs would overwrite that "JMP ss_fixup" with a NOP and they're fine. However, the X86_BUG_SYSRET_SS_ATTRS CPUs will have to do two jumps, one to the fixup code and one back to RET.

Now, how about I convert

                unsigned short ss_sel;
                savesegment(ss, ss_sel);
                if (ss_sel != __KERNEL_DS)
                        loadsegment(ss, __KERNEL_DS);

into asm and into an alternative()?

Then, the !X86_BUG_SYSRET_SS_ATTRS CPUs will trade off that JMP with a bunch of NOPs which will pollute I$.

Hmmm.

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Yes, having t_no as the fallthrough case ought to move the yes code out of line.

The current code probably pollutes the i$ too.

Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

Dunno, maybe I'm doing something wrong:

I have this change:

---

index 7f09de998c93..f9833fcb8fcb 100644
@@ -175,10 +175,10 @@ static __always_inline __pure bool _static_cpu_has(u16 bit)
                             [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;
+       t_yes:
+               return true;
 }
 
 #define static_cpu_has(bit)                                    \

---

and the resulting code looks even wrong (or my brain is fried for today
- one of the two).

vmlinux:

ffffffff810046ae:       e9 cc 0e de 00          jmpq   ffffffff81de557f <__alt_instructions_end+0x7aa>
ffffffff810046b3:       48 83 c4 18             add    $0x18,%rsp
ffffffff810046b7:       4c 89 e0                mov    %r12,%rax
ffffffff810046ba:       5b                      pop    %rbx
ffffffff810046bb:       41 5c                   pop    %r12
ffffffff810046bd:       41 5d                   pop    %r13
ffffffff810046bf:       41 5e                   pop    %r14
ffffffff810046c1:       41 5f                   pop    %r15
ffffffff810046c3:       5d                      pop    %rbp
ffffffff810046c4:       c3                      retq

dynamic branch:

ffffffff81de557f:       f6 05 8f de d1 ff 01    testb  $0x1,-0x2e2171(%rip)        # ffffffff81b03415 <boot_cpu_data+0x55>
ffffffff81de5586:       0f 85 f6 f1 21 ff       jne    ffffffff81004782 <__switch_to+0x332>
ffffffff81de558c:       e9 22 f1 21 ff          jmpq   ffffffff810046b3 <__switch_to+0x263>

after X86_FEATURE_ALWAYS patching:

[    0.288007] apply_alternatives: feat: 3*32+21, old: (ffffffff810046ae, len: 5), repl: (ffffffff81de4dff, len: 5), pad: 0
[    0.292004] ffffffff810046ae: old_insn: e9 cc 0e de 00
[    0.300013] ffffffff81de4dff: rpl_insn: e9 af f8 21 ff
[    0.308006] recompute_jump: target RIP: ffffffff810046b3, new_displ: 0x5
[    0.312006] recompute_jump: final displ: 0x00000003, JMP 0xffffffff810046b3
[    0.316006] ffffffff810046ae: final_insn: eb 03 0f 1f 00

ffffffff810046ae:       eb 03 0f 1f 00		jmp    ffffffff810046b3 ---
ffffffff810046b3:       48 83 c4 18             add    $0x18,%rsp	<--
ffffffff810046b7:       4c 89 e0                mov    %r12,%rax
ffffffff810046ba:       5b                      pop    %rbx
ffffffff810046bb:       41 5c                   pop    %r12
ffffffff810046bd:       41 5d                   pop    %r13
ffffffff810046bf:       41 5e                   pop    %r14
ffffffff810046c1:       41 5f                   pop    %r15
ffffffff810046c3:       5d                      pop    %rbp
ffffffff810046c4:       c3                      retq

so this is silly: we're basically jumping after the JMP instruction itself. So that will be the case on !X86_BUG_SYSRET_SS_ATTRS CPUs. Still a two-byte and now even a useless JMP.

The right thing to do would be to generate a NOP simply.

On X86_BUG_SYSRET_SS_ATTRS CPUs:

[    0.322014] apply_alternatives: feat: 16*32+8, old: (ffffffff810046ae, len: 5), repl: (ffffffff81de3962, len: 0), pad: 0
[    0.324005] ffffffff810046ae: old_insn: eb 03 0f 1f 00
[    0.332006] ffffffff810046ae: final_insn: 0f 1f 44 00 00

ffffffff810046ae:       0f 1f 44 00 00		nop
ffffffff810046b3:       48 83 c4 18             add    $0x18,%rsp
ffffffff810046b7:       4c 89 e0                mov    %r12,%rax
ffffffff810046ba:       5b                      pop    %rbx
ffffffff810046bb:       41 5c                   pop    %r12
ffffffff810046bd:       41 5d                   pop    %r13
ffffffff810046bf:       41 5e                   pop    %r14
ffffffff810046c1:       41 5f                   pop    %r15
ffffffff810046c3:       5d                      pop    %rbp
ffffffff810046c4:       c3                      retq

which is actually even wrong!

What it should've done is

	jne    ffffffff81004782

as the dynamic code did. At that address we have the ss fixup:

ffffffff81004782:       66 8c d0                mov    %ss,%ax
ffffffff81004785:       66 83 f8 18             cmp    $0x18,%ax
ffffffff81004789:       0f 84 24 ff ff ff       je     ffffffff810046b3 <__switch_to+0x263>
ffffffff8100478f:       b8 18 00 00 00          mov    $0x18,%eax
ffffffff81004794:       8e d0                   mov    %eax,%ss
ffffffff81004796:       e9 18 ff ff ff          jmpq   ffffffff810046b3 <__switch_to+0x263>

with the jump back to the ret code. Which means, !X86_BUG_SYSRET_SS_ATTRS CPUs get to do a forward and a backward JMP. So even if it did the right thing, it would be two JMPs.

Meh.

I need to think about something better.

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

OK, so gcc isn't as clever as I thought.

-hpa

Maybe a label attribute would help, I don't know.

-hpa

Here's another version which works, not really better though:

Change is this:

---
+ asm_volatile_goto(ALTERNATIVE("", "jmp %l[t_fixup_ss]",
+ X86_BUG_SYSRET_SS_ATTRS)
+ : : : : t_fixup_ss);
+
+ return prev_p;
+
+t_fixup_ss:

<snip comment>

+ savesegment(ss, ss_sel);
+ if (ss_sel != __KERNEL_DS)
+ loadsegment(ss, __KERNEL_DS);

        return prev_p;

---

with two "return prev_p" with the hope that gcc won't generate a second JMP back to the frame restore and ret code. But, nah, it does.

vmlinux:

ffffffff8100472a:       90                      nop
ffffffff8100472b:       90                      nop
ffffffff8100472c:       90                      nop
ffffffff8100472d:       90                      nop
ffffffff8100472e:       90                      nop
ffffffff8100472f:       48 83 c4 18             add    $0x18,%rsp
ffffffff81004733:       4c 89 e0                mov    %r12,%rax
ffffffff81004736:       5b                      pop    %rbx
ffffffff81004737:       41 5c                   pop    %r12
ffffffff81004739:       41 5d                   pop    %r13
ffffffff8100473b:       41 5e                   pop    %r14
ffffffff8100473d:       41 5f                   pop    %r15
ffffffff8100473f:       5d                      pop    %rbp
ffffffff81004740:       c3                      retq

after patching on an X86_BUG_SYSRET_SS_ATTRS CPU:

[    0.264007] apply_alternatives: feat: 16*32+8, old: (ffffffff8100472a, len: 5), repl: (ffffffff81de4e05, len: 5), pad: 5
[    0.268005] ffffffff8100472a: old_insn: 90 90 90 90 90
[    0.273510] ffffffff81de4e05: rpl_insn: e9 68 f9 21 ff
[    0.277496] recompute_jump: target RIP: ffffffff81004772, new_displ: 0x48
[    0.280005] recompute_jump: final displ: 0x00000046, JMP 0xffffffff81004772
[    0.283159] ffffffff8100472a: final_insn: eb 46 0f 1f 00

ffffffff8100472a:       eb 46 0f 1f 00		jmp ffffffff81004772
ffffffff8100472f:       48 83 c4 18             add    $0x18,%rsp
ffffffff81004733:       4c 89 e0                mov    %r12,%rax
ffffffff81004736:       5b                      pop    %rbx
ffffffff81004737:       41 5c                   pop    %r12
ffffffff81004739:       41 5d                   pop    %r13
ffffffff8100473b:       41 5e                   pop    %r14
ffffffff8100473d:       41 5f                   pop    %r15
ffffffff8100473f:       5d                      pop    %rbp
ffffffff81004740:       c3                      retq

so a two-byte JMP jumping to:

ffffffff81004772:       66 8c d0                mov    %ss,%ax
ffffffff81004775:       66 83 f8 18             cmp    $0x18,%ax
ffffffff81004779:       74 b4                   je     ffffffff8100472f <__switch_to+0x2df>
ffffffff8100477b:       b8 18 00 00 00          mov    $0x18,%eax
ffffffff81004780:       8e d0                   mov    %eax,%ss
ffffffff81004782:       eb ab                   jmp    ffffffff8100472f <__switch_to+0x2df>

which does the fixup and jumps back to ...472f which restores the frame and returns.

I wish I could be able to tell gcc to not jump back but add the function return here too as we don't care about code size in that case.

And it's not like it is really better on !X86_BUG_SYSRET_SS_ATTRS CPUs - there we have the 5-byte padding NOP being converted to 5-byte one:

[    0.293164] ffffffff8100472a: [0:5) optimized NOPs: 0f 1f 44 00 00

I need to talk to my gcc guy... :)

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

[45] .static_cpu_has PROGBITS ffffffff97aa655b 16ea655b

       00000000000002df  0000000000000000  AX       0     0     1

Same.

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Ok, staring at this section was wrong. I went and looked at the .s file and now it clicked: gcc inlines those calls to __static_cpu_has_safe of the dynamic jump target:

        .loc 4 538 0
        movl    $125, %edi      #,
        call    __static_cpu_has_safe   #
.LBE885:
.LBE886:
.LBE914:
        .loc 1 240 0
        testb   %al, %al        # D.30157
        je      .L150   #,
        jmp     .L151   #

.L152:
.LBB915:
.LBB909:
.LBB905:
.LBB893:
.LBB892:

        .loc 4 538 0
        movl    $154, %edi      #,
        call    __static_cpu_has_safe   #
.LBE892:
.LBE893:
.LBE905:
        .loc 7 431 0
        testb   %al, %al        # D.30157
        jne     .L154   #,

which turn into:

 751:   bf 7d 00 00 00          mov    $0x7d,%edi
 756:   e8 00 00 00 00          callq  75b <fpu__copy+0xab>
 75b:   84 c0                   test   %al,%al
 75d:   74 a3                   je     702 <fpu__copy+0x52>
 75f:   eb 90                   jmp    6f1 <fpu__copy+0x41>
 761:   bf 9a 00 00 00          mov    $0x9a,%edi
 766:   e8 00 00 00 00          callq  76b <fpu__copy+0xbb>
 76b:   84 c0                   test   %al,%al

That's like 28 bytes in this particular case, which is 14 per call site, on average.

So grepping through my tailored vmlinux, it has 35 entries with X86_FEATURE_ALWAYS in .altinstructions X 14 = 490 bytes.

So yeah, we probably should do this, allyesconfig should give more savings.

One thing I'd probably do differently is not call the throwaway section .static_cpu_has but something like .altinstr_temporary or so and put it after the replacement insns:

        .altinstr_replacement : AT(ADDR(.altinstr_replacement) - LOAD_OFFSET) {
                *(.altinstr_replacement)
		*(.altinstr_temporary)
        }

so that we know those instructions belong to the alternatives mechanism. They'll get discared too, of course.

I could just as well be talking a lot of crap, it is waay too late here.

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Why the f do we call a subroutine for what amounts to a single bt or test instruction?

Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

No real reason. You can kick me when you see me next time:

4a90a99c4f80 ("x86: Add a static_cpu_has_safe variant")

:-)

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

So, here is my suggestion:

1. Just get rid of static_cpu_has_safe() and make static_cpu_has() safe.

2. Get rid of the non-asm goto variant and just fall back to dynamic if asm goto is unavailable. It doesn't make any sense, really, if it is supposed to be safe, and by now the asm goto-capable gcc is in more wide use. (Originally the gcc 3.x fallback to pure dynamic didn't exist, either.)

3. Put the dynamic test in the .init.text section and inline it:

	.section .init.text,"ax"
	testb %2,%3
	jnz %[t_yes]
	jmp %[t_no]
	.previous

	... "i" (1 << (bit & 7)),
	    "m" (((const char *)boot_cpu_data->x86_capability)[bit >> 3]) ...

(The code would be slightly simpler/cleaner with testl, but that would unnecessarily create a long immediate, or with btl, but that would be slower. We could use CONST_MASK_ADDR() and CONST_MASK() from asm/bitops.h, but I'm slightly uncomfortable with the idea of leveraging an interface which is ultimately an internal implementation detail of bitops.h that might change in the future without people realizing its implications.)

The only thing we lose is the 2-byte optimization in case we have the good luck for it to actually work. At this point I'm thinking it isn't worth it, and instead that safety trumps it. It *would* be interesting to see if there are any call sites where it would actually kick in.

-hpa

Can't put it in .init.text or else you get:
WARNING: arch/x86/kernel/built-in.o(.text+0x4b9): Section mismatch in reference from the function __switch_to() to the (unknown reference) .init.text:(unknown)
The function __switch_to() references
the (unknown reference) __init (unknown).
This is often because __switch_to lacks a __init annotation or the annotation of (unknown) is wrong.

We want to override that because we know that the reference will be removed after alternatives run. That's why I created a new section.

--
Brian Gerst

Right. I wish we could tag reference call sites as clean, not sources or targets. Sigh.

However, that's not too much of an issue.

-hpa

I was thinking about something like this:

From: Borislav Petkov <[email protected]>
Date: Wed, 20 Jan 2016 11:45:36 +0100
Subject: [PATCH] x86/alternatives: Add an auxilary section

Add .altinstr_aux for additional instructions which will be used before and/or during patching. All stuff which needs more sophisticated patching should go there. See next patch.

---
 arch/x86/kernel/vmlinux.lds.S | 5 +++++
 1 file changed, 5 insertions(+)

index 74e4bf11f562..35868bf529b9 100644
@@ -244,6 +244,11 @@ SECTIONS
 	 */
 	.altinstr_replacement : AT(ADDR(.altinstr_replacement) - LOAD_OFFSET) {
 		*(.altinstr_replacement)
+		/*
+		 * Auxiliary section for misc instruction patching tasks. See
+		 * static_cpu_has(), for an example.
+		 */
+		*(.altinstr_aux)
 	}
 
 	/*

2.3.5

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

How about:

section for code used exclusively before alternatives are run. All references to such code must be patched out by alternatives, normally by using a patch with X86_FEATURE_ALWAYS.

See static_cpu_has() for an example.

Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

Sure.

My thinking was to make it a bit more generic so that if we decide to do some different monkey business with the alternatives, to put stuff in there too.

But we can always change that later - it's not like it is user-visible.

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

I don't think the verbiage I suggested in any way disagrees with that notion.

Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

Nice!

I was going to do:

		"i" (1 << (bit & 31)),
                "m" (((boot_cpu_data->x86_capability)[bit >> 5]))

:-)

Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

But then you're using testl and get long immediates.

(And the parentheses around boot_cpu_data->x86_capability are redundant.)

-hpa

Right.

Ok, below is what builds here. So no SOBs etc.

All this include hell wankery is so that we can use boot_cpu_data in cpufeature.h. And that's not simple because boot/mkcpustr.c includes it too so if I carve out struct cpuinfo_x86 to a separate asm/cpuinfo.h header, it complains because it doesn't see it.

Thus this _ASM_BOOT_MKCPUSTR_ yucky marker to stop arch/x86/boot from including it.

I'm very open to better ideas. :-)

Other than that, we do:

+                        "6: testb %[bitnum],%[cap_word]\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_word] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])

and that has the TESTB.

Thoughts?

---
 arch/x86/boot/mkcpustr.c          |  5 +++
 arch/x86/include/asm/cpufeature.h | 34 +++++++++++++-----
 arch/x86/include/asm/cpuinfo.h    | 73 +++++++++++++++++++++++++++++++++++++++
 arch/x86/include/asm/processor.h  | 65 ----------------------------------
 arch/x86/kernel/cpu/common.c      |  6 ----
 arch/x86/kernel/vmlinux.lds.S     |  9 +++--
 6 files changed, 110 insertions(+), 82 deletions(-)
 create mode 100644 arch/x86/include/asm/cpuinfo.h

index 637097e66a62..c32113a0e3d4 100644
@@ -17,6 +17,11 @@
 
 #include "../include/asm/required-features.h"
 #include "../include/asm/disabled-features.h"
+
+/*
+ * Stop cpufeature.h from including cpuinfo.h in kernel proper.
+ */
+#define _ASM_BOOT_MKCPUSTR_
 #include "../include/asm/cpufeature.h"
 #include "../kernel/cpu/capflags.c"
 

index 7ad8c9464297..a16cee2376c4 100644
@@ -15,6 +15,10 @@
 #define NCAPINTS	16	/* N 32-bit words worth of info */
 #define NBUGINTS	1	/* N 32-bit bug flags */
 
+#ifndef _ASM_BOOT_MKCPUSTR_
+#include <asm/cpuinfo.h>
+#endif
+
 /*
  * Note: If the comment begins with a quoted string, that string is used
  * in /proc/cpuinfo instead of the macro name.  If the string is "",

@@ -412,7 +416,6 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
 
 #if __GNUC__ >= 4 && defined(CONFIG_X86_FAST_FEATURE_TESTS)
 extern void warn_pre_alternatives(void);
-extern bool __static_cpu_has_safe(u16 bit);
 
 /*
  * Static testing of CPU features.  Used the same as boot_cpu_has().

@@ -505,7 +508,7 @@ static __always_inline __pure bool __static_cpu_has(u16 bit)
 static __always_inline __pure bool _static_cpu_has_safe(u16 bit)
 {
 #ifdef CC_HAVE_ASM_GOTO
-		asm_volatile_goto("1: jmp %l[t_dynamic]\n"
+		asm_volatile_goto("1: jmp 6f\n"
 			 "2:\n"
 			 ".skip -(((5f-4f) - (2b-1b)) > 0) * "
 			         "((5f-4f) - (2b-1b)),0x90\n"

@@ -530,17 +533,23 @@ static __always_inline __pure bool _static_cpu_has_safe(u16 bit)
 			 " .byte 0\n"			/* repl len */
 			 " .byte 0\n"			/* pad len */
 			 ".previous\n"
-			 : : "i" (bit), "i" (X86_FEATURE_ALWAYS)
-			 : : t_dynamic, t_no);
+			 ".section .altinstr_aux,\"ax\"\n"
+			 "6: testb %[bitnum],%[cap_word]\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_word] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3])
+			 : : t_yes, t_no);
+	t_yes:
 		return true;
 	t_no:
 		return false;
-	t_dynamic:
-		return __static_cpu_has_safe(bit);
 #else
 		u8 flag;
 		/* Open-coded due to __stringify() in ALTERNATIVE() */
-		asm volatile("1: movb $2,%0\n"
+		asm volatile("1: jmp 7f\n"
 			     "2:\n"
 			     ".section .altinstructions,\"a\"\n"
 			     " .long 1b - .\n"		/* src offset */

@@ -572,9 +581,16 @@ static __always_inline __pure bool _static_cpu_has_safe(u16 bit)
 			     "5: movb $1,%0\n"
 			     "6:\n"
 			     ".previous\n"
+			     ".section .altinstr_aux,\"ax\"\n"
+			     "7: testb %[bitnum],%[cap_word]\n"
+			     "   setnz %0\n"
+			     "   jmp 2b\n"
+			     ".previous\n"
 			     : "=qm" (flag)
-			     : "i" (bit), "i" (X86_FEATURE_ALWAYS));
-		return (flag == 2 ? __static_cpu_has_safe(bit) : flag);
+			     : "i" (bit), "i" (X86_FEATURE_ALWAYS),
+			       [bitnum] "i" (1 << (bit & 7)),
+			       [cap_word] "m" (((const char *)boot_cpu_data.x86_capability)[bit >> 3]));
+		return (flag != 0);
 #endif /* CC_HAVE_ASM_GOTO */
 }
 

index 37830de8f60a..897c65bd3faa 100644
@@ -1483,12 +1483,6 @@ void warn_pre_alternatives(void)
 EXPORT_SYMBOL_GPL(warn_pre_alternatives);
 #endif
 
-inline bool __static_cpu_has_safe(u16 bit)
-{
-	return boot_cpu_has(bit);
-}
-EXPORT_SYMBOL_GPL(__static_cpu_has_safe);
-
 static void bsp_resume(void)
 {
 	if (this_cpu->c_bsp_resume)

index 35868bf529b9..486dc0e60599 100644
@@ -244,9 +244,14 @@ SECTIONS
 	 */
 	.altinstr_replacement : AT(ADDR(.altinstr_replacement) - LOAD_OFFSET) {
 		*(.altinstr_replacement)
+
 		/*
-		 * Auxiliary section for misc instruction patching tasks. See
-		 * static_cpu_has(), for an example.
+		 * Section for code used exclusively before alternatives are
+		 * run. All references to such code must be patched out by
+		 * alternatives, normally by using a patch with
+		 * X86_FEATURE_ALWAYS.
+		 *
+		 * See static_cpu_has() for an example.
 		 */
 		*(.altinstr_aux)
 	}

2.3.5

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

I think the right answer is to split the macros that define specific CPU features into a separate file. It would also make it assembly-safe which would be useful in some other places.

However, I do also want to observe that we already do have this specific marker... it is called __KERNEL__.

-hpa

We are using temporary %rax register during checking of kernel address alignment. We can ged rid of it since testl instruction is safe and does not change value of the rbp register.

---
 arch/x86/kernel/head_64.S | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

index ffdc0e8..7c21029 100644
@@ -76,9 +76,7 @@ startup_64:
 	subq	$_text - __START_KERNEL_map, %rbp
 
 	/* Is the address not 2M aligned? */
-	movq	%rbp, %rax
-	andl	$~PMD_PAGE_MASK, %eax
-	testl	%eax, %eax
+	testl	$~PMD_PAGE_MASK, %ebp
 	jnz	bad_address
 
 	/*

2.7.0.25.gfc10eb5

From: Borislav Petkov <[email protected]>

Hi,

this is the rest of the series today. Main one is the cpufeature cleanup. I merged the AVIC patch from tip:x86/cpu so that the rest applies cleanly. The rest is simplifications/cleanups.

Please apply,
thanks.

Alexander Kuleshov (1):
x86/head_64: Simplify kernel load address alignment check

Borislav Petkov (5):
x86/cpufeature: Carve out X86_FEATURE_*
x86/cpufeature: Replace the old static_cpu_has() with safe variant x86/cpufeature: Get rid of the non-asm goto variant x86/alternatives: Add an auxilary section
x86/vdso: Use static_cpu_has()

Brian Gerst (1):
x86/alternatives: Discard dynamic check after init

Michael S. Tsirkin (3):
x86/asm: Add condition codes clobber to memory barrier macros x86/asm: Drop a comment left over from X86_OOSTORE x86/asm: Tweak the comment about wmb() use for IO

From: "Michael S. Tsirkin" <[email protected]>

ADDL clobbers flags (such as CF) but barrier.h didn't tell this to gcc. Historically, gcc doesn't need them on x86, and always considers flags clobbered. We are probably missing the cc clobber in a *lot* of places for this reason.

But even if not necessary, it's probably a good thing to add for documentation, and in case gcc semantics ever change.

---
 arch/x86/include/asm/barrier.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

index 0681d2532527..5bce7865b623 100644
@@ -15,9 +15,12 @@
  * Some non-Intel clones support out of order store. wmb() ceases to be a
  * nop for these.
  */
-#define mb() alternative("lock; addl $0,0(%%esp)", "mfence", X86_FEATURE_XMM2)
-#define rmb() alternative("lock; addl $0,0(%%esp)", "lfence", X86_FEATURE_XMM2)
-#define wmb() alternative("lock; addl $0,0(%%esp)", "sfence", X86_FEATURE_XMM)
+#define mb() asm volatile(ALTERNATIVE("lock; addl $0,0(%%esp)", "mfence", \
+				      X86_FEATURE_XMM2) ::: "memory", "cc")
+#define rmb() asm volatile(ALTERNATIVE("lock; addl $0,0(%%esp)", "lfence", \
+				       X86_FEATURE_XMM2) ::: "memory", "cc")
+#define wmb() asm volatile(ALTERNATIVE("lock; addl $0,0(%%esp)", "sfence", \
+				       X86_FEATURE_XMM2) ::: "memory", "cc")
 #else
 #define mb() 	asm volatile("mfence":::"memory")
 #define rmb()	asm volatile("lfence":::"memory")

2.3.5

From: "Michael S. Tsirkin" <[email protected]>

The comment about wmb() being non-nop to deal with non-intel CPUs is a left over from before commit

09df7c4c8097 ("x86: Remove CONFIG_X86_OOSTORE").

It makes no sense now: in particular, wmb() is not a nop even for regular intel CPUs because of weird use-cases e.g. dealing with WC memory.

Drop this comment.

---
 arch/x86/include/asm/barrier.h | 4 ----
 1 file changed, 4 deletions(-)

index 5bce7865b623..d2aa66a3a4b5 100644
@@ -11,10 +11,6 @@
  */
 
 #ifdef CONFIG_X86_32
-/*
- * Some non-Intel clones support out of order store. wmb() ceases to be a
- * nop for these.
- */
 #define mb() asm volatile(ALTERNATIVE("lock; addl $0,0(%%esp)", "mfence", \
 				      X86_FEATURE_XMM2) ::: "memory", "cc")
 #define rmb() asm volatile(ALTERNATIVE("lock; addl $0,0(%%esp)", "lfence", \

2.3.5

From: "Michael S. Tsirkin" <[email protected]>

On x86, we *do* still use the non-nop rmb()/wmb() for IO barriers, but even that is generally questionable.

Leave them around for historical reasons, unless somebody can point to a case where they care about the performance. Tweak the comment so people don't think they are strictly required in all cases.

---
 arch/x86/include/asm/barrier.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index d2aa66a3a4b5..4f95b2affd88 100644
@@ -6,8 +6,8 @@
 
 /*
  * Force strict CPU ordering.
- * And yes, this is required on UP too when we're talking
- * to devices.
+ *
+ * And yes, this might be required on UP too when we're talking to devices.
  */
 
 #ifdef CONFIG_X86_32

2.3.5

From: Borislav Petkov <[email protected]>

Move them to a separate header and have the following dependency:

x86/cpufeatures.h <- x86/processor.h <- x86/cpufeature.h

This makes it easier to use the header in asm code and not include the whole cpufeature.h and add guards for asm.

---
 Documentation/kernel-parameters.txt      |   2 +-
 arch/x86/boot/cpuflags.h                 |   2 +-
 arch/x86/boot/mkcpustr.c                 |   2 +-
 arch/x86/crypto/crc32-pclmul_glue.c      |   2 +-
 arch/x86/crypto/crc32c-intel_glue.c      |   2 +-
 arch/x86/crypto/crct10dif-pclmul_glue.c  |   2 +-
 arch/x86/entry/common.c                  |   1 +
 arch/x86/entry/entry_32.S                |   2 +-
 arch/x86/entry/vdso/vdso32-setup.c       |   1 -
 arch/x86/entry/vdso/vdso32/system_call.S |   2 +-
 arch/x86/entry/vdso/vma.c                |   1 +
 arch/x86/include/asm/alternative.h       |   6 -
 arch/x86/include/asm/apic.h              |   1 -
 arch/x86/include/asm/arch_hweight.h      |   2 +
 arch/x86/include/asm/cmpxchg.h           |   1 +
 arch/x86/include/asm/cpufeature.h        | 284 +-----------------------------
 arch/x86/include/asm/cpufeatures.h       | 288 +++++++++++++++++++++++++++++++
 arch/x86/include/asm/fpu/internal.h      |   1 +
 arch/x86/include/asm/irq_work.h          |   2 +-
 arch/x86/include/asm/mwait.h             |   2 +
 arch/x86/include/asm/processor.h         |   3 +-
 arch/x86/include/asm/smap.h              |   2 +-
 arch/x86/include/asm/smp.h               |   1 -
 arch/x86/include/asm/thread_info.h       |   2 +-
 arch/x86/include/asm/tlbflush.h          |   1 +
 arch/x86/include/asm/uaccess_64.h        |   2 +-
 arch/x86/kernel/cpu/Makefile             |   2 +-
 arch/x86/kernel/cpu/centaur.c            |   2 +-
 arch/x86/kernel/cpu/cyrix.c              |   1 +
 arch/x86/kernel/cpu/intel.c              |   2 +-
 arch/x86/kernel/cpu/intel_cacheinfo.c    |   2 +-
 arch/x86/kernel/cpu/match.c              |   2 +-
 arch/x86/kernel/cpu/mkcapflags.sh        |   6 +-
 arch/x86/kernel/cpu/mtrr/main.c          |   2 +-
 arch/x86/kernel/cpu/transmeta.c          |   2 +-
 arch/x86/kernel/e820.c                   |   1 +
 arch/x86/kernel/head_32.S                |   2 +-
 arch/x86/kernel/hpet.c                   |   1 +
 arch/x86/kernel/msr.c                    |   2 +-
 arch/x86/kernel/verify_cpu.S             |   2 +-
 arch/x86/lib/clear_page_64.S             |   2 +-
 arch/x86/lib/copy_page_64.S              |   2 +-
 arch/x86/lib/copy_user_64.S              |   2 +-
 arch/x86/lib/memcpy_64.S                 |   2 +-
 arch/x86/lib/memmove_64.S                |   2 +-
 arch/x86/lib/memset_64.S                 |   2 +-
 arch/x86/mm/setup_nx.c                   |   1 +
 arch/x86/oprofile/op_model_amd.c         |   1 -
 arch/x86/um/asm/barrier.h                |   2 +-
 lib/atomic64_test.c                      |   2 +-
 50 files changed, 336 insertions(+), 328 deletions(-)
 create mode 100644 arch/x86/include/asm/cpufeatures.h

index b7d44871effc..07c661ae8596 100644
@@ -661,7 +661,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 
 	clearcpuid=BITNUM [X86]
 			Disable CPUID feature X for the kernel. See
-			arch/x86/include/asm/cpufeature.h for the valid bit
+			arch/x86/include/asm/cpufeatures.h for the valid bit
 			numbers. Note the Linux specific bits are not necessarily
 			stable over kernel options, but the vendor specific
 			ones should be.

index ea97697e51e4..4cb404fd45ce 100644
@@ -1,7 +1,7 @@
 #ifndef BOOT_CPUFLAGS_H
 #define BOOT_CPUFLAGS_H
 
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/processor-flags.h>
 
 struct cpu_features {

index 637097e66a62..f72498dc90d2 100644
@@ -17,7 +17,7 @@
 
 #include "../include/asm/required-features.h"
 #include "../include/asm/disabled-features.h"
-#include "../include/asm/cpufeature.h"
+#include "../include/asm/cpufeatures.h"
 #include "../kernel/cpu/capflags.c"
 
 int main(void)

index 07d2c6c86a54..27226df3f7d8 100644
@@ -33,7 +33,7 @@
 #include <linux/crc32.h>
 #include <crypto/internal/hash.h>
 
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/cpu_device_id.h>
 #include <asm/fpu/api.h>
 

index 0e9871693f24..0857b1a1de3b 100644
@@ -30,7 +30,7 @@
 #include <linux/kernel.h>
 #include <crypto/internal/hash.h>
 
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/cpu_device_id.h>
 #include <asm/fpu/internal.h>
 

index a3fcfc97a311..cd4df9322501 100644
@@ -30,7 +30,7 @@
 #include <linux/string.h>
 #include <linux/kernel.h>
 #include <asm/fpu/api.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/cpu_device_id.h>
 
 asmlinkage __u16 crc_t10dif_pcl(__u16 crc, const unsigned char *buf,

index 03663740c866..02e3d89bb137 100644
@@ -26,6 +26,7 @@
 #include <asm/traps.h>
 #include <asm/vdso.h>
 #include <asm/uaccess.h>
+#include <asm/cpufeature.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/syscalls.h>

index 77d8c5112900..4c5228352744 100644
@@ -40,7 +40,7 @@
 #include <asm/processor-flags.h>
 #include <asm/ftrace.h>
 #include <asm/irq_vectors.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/alternative-asm.h>
 #include <asm/asm.h>
 #include <asm/smap.h>

index 08a317a9ae4b..7853b53959cd 100644
@@ -11,7 +11,6 @@
 #include <linux/kernel.h>
 #include <linux/mm_types.h>
 
-#include <asm/cpufeature.h>
 #include <asm/processor.h>
 #include <asm/vdso.h>
 

index 3a1d9297074b..0109ac6cb79c 100644
@@ -3,7 +3,7 @@
 */
 
 #include <asm/dwarf2.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/alternative-asm.h>
 
 /*

index b8f69e264ac4..5471ac362147 100644
@@ -20,6 +20,7 @@
 #include <asm/page.h>
 #include <asm/hpet.h>
 #include <asm/desc.h>
+#include <asm/cpufeature.h>
 
 #if defined(CONFIG_X86_64)
 unsigned int __read_mostly vdso64_enabled = 1;

index 7bfc85bbb8ff..99afb665a004 100644
@@ -152,12 +152,6 @@ static inline int alternatives_text_reserved(void *start, void *end)
 	".popsection"
 
 /*
- * This must be included *after* the definition of ALTERNATIVE due to
- * <asm/arch_hweight.h>
- */
-#include <asm/cpufeature.h>
-
-/*
  * Alternative instructions for different CPU types or capabilities.
  *
  * This allows to use optimized instructions even on generic binary

index c80f6b6f3da2..0899cfc8dfe8 100644
@@ -6,7 +6,6 @@
 
 #include <asm/alternative.h>
 #include <asm/cpufeature.h>
-#include <asm/processor.h>
 #include <asm/apicdef.h>
 #include <linux/atomic.h>
 #include <asm/fixmap.h>

index 259a7c1ef709..02e799fa43d1 100644
@@ -1,6 +1,8 @@
 #ifndef _ASM_X86_HWEIGHT_H
 #define _ASM_X86_HWEIGHT_H
 
+#include <asm/cpufeatures.h>
+
 #ifdef CONFIG_64BIT
 /* popcnt %edi, %eax -- redundant REX prefix for alignment */
 #define POPCNT32 ".byte 0xf3,0x40,0x0f,0xb8,0xc7"

index ad19841eddfe..9733361fed6f 100644
@@ -2,6 +2,7 @@
 #define ASM_X86_CMPXCHG_H
 
 #include <linux/compiler.h>
+#include <asm/cpufeatures.h>
 #include <asm/alternative.h> /* Provides LOCK_PREFIX */
 
 /*

index 0fd440df63f1..d01199def781 100644
@@ -17,6 +17,7 @@
 #include <asm/user.h>
 #include <asm/fpu/api.h>
 #include <asm/fpu/xstate.h>
+#include <asm/cpufeature.h>
 
 /*
  * High level FPU state handling functions:

index 78162f8e248b..d0afb05c84fc 100644
@@ -1,7 +1,7 @@
 #ifndef _ASM_IRQ_WORK_H
 #define _ASM_IRQ_WORK_H
 
-#include <asm/processor.h>
+#include <asm/cpufeature.h>
 
 static inline bool arch_irq_work_has_interrupt(void)
 {

index c70689b5e5aa..0deeb2d26df7 100644
@@ -3,6 +3,8 @@
 
 #include <linux/sched.h>
 
+#include <asm/cpufeature.h>
+
 #define MWAIT_SUBSTATE_MASK		0xf
 #define MWAIT_CSTATE_MASK		0xf
 #define MWAIT_SUBSTATE_SIZE		4

index 2d5a50cb61a2..491a3d9dbb15 100644
@@ -13,7 +13,7 @@ struct vm86;
 #include <asm/types.h>
 #include <uapi/asm/sigcontext.h>
 #include <asm/current.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/page.h>
 #include <asm/pgtable_types.h>
 #include <asm/percpu.h>

@@ -24,7 +24,6 @@ struct vm86;
 #include <asm/fpu/types.h>
 
 #include <linux/personality.h>
-#include <linux/cpumask.h>
 #include <linux/cache.h>
 #include <linux/threads.h>
 #include <linux/math64.h>

index ba665ebd17bb..db333300bd4b 100644
@@ -15,7 +15,7 @@
 
 #include <linux/stringify.h>
 #include <asm/nops.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 
 /* "Raw" instruction opcodes */
 #define __ASM_CLAC	.byte 0x0f,0x01,0xca

index dfcf0727623b..20a3de5cb3b0 100644
@@ -16,7 +16,6 @@
 #endif
 #include <asm/thread_info.h>
 #include <asm/cpumask.h>
-#include <asm/cpufeature.h>
 
 extern int smp_num_siblings;
 extern unsigned int num_processors;

index c7b551028740..c0778fcab06d 100644
@@ -49,7 +49,7 @@
  */
 #ifndef __ASSEMBLY__
 struct task_struct;
-#include <asm/processor.h>
+#include <asm/cpufeature.h>
 #include <linux/atomic.h>
 
 struct thread_info {

index 6df2029405a3..0bb31cb8c73b 100644
@@ -5,6 +5,7 @@
 #include <linux/sched.h>
 
 #include <asm/processor.h>
+#include <asm/cpufeature.h>
 #include <asm/special_insns.h>
 
 #ifdef CONFIG_PARAVIRT

index f2f9b39b274a..d83a55b95a48 100644
@@ -8,7 +8,7 @@
 #include <linux/errno.h>
 #include <linux/lockdep.h>
 #include <asm/alternative.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/page.h>
 
 /*

index 58031303e304..faa7b5204129 100644
@@ -64,7 +64,7 @@ ifdef CONFIG_X86_FEATURE_NAMES
 quiet_cmd_mkcapflags = MKCAP   $@
       cmd_mkcapflags = $(CONFIG_SHELL) $(srctree)/$(src)/mkcapflags.sh $< $@
 
-cpufeature = $(src)/../../include/asm/cpufeature.h
+cpufeature = $(src)/../../include/asm/cpufeatures.h
 
 targets += capflags.c
 $(obj)/capflags.c: $(cpufeature) $(src)/mkcapflags.sh FORCE

index ae20be6e483c..6608c03c2126 100644
@@ -1,7 +1,7 @@
 #include <linux/bitops.h>
 #include <linux/kernel.h>
 
-#include <asm/processor.h>
+#include <asm/cpufeature.h>
 #include <asm/e820.h>
 #include <asm/mtrr.h>
 #include <asm/msr.h>

index aaf152e79637..15e47c1cd412 100644
@@ -8,6 +8,7 @@
 #include <linux/timer.h>
 #include <asm/pci-direct.h>
 #include <asm/tsc.h>
+#include <asm/cpufeature.h>
 
 #include "cpu.h"
 

index 565648bc1a0a..9299e3bdfad6 100644
@@ -8,7 +8,7 @@
 #include <linux/module.h>
 #include <linux/uaccess.h>
 
-#include <asm/processor.h>
+#include <asm/cpufeature.h>
 #include <asm/pgtable.h>
 #include <asm/msr.h>
 #include <asm/bugs.h>

index 0b6c52388cf4..341449c49f34 100644
@@ -14,7 +14,7 @@
 #include <linux/sysfs.h>
 #include <linux/pci.h>
 
-#include <asm/processor.h>
+#include <asm/cpufeature.h>
 #include <asm/amd_nb.h>
 #include <asm/smp.h>
 

index afa9f0d487ea..fbb5e90557a5 100644
@@ -1,5 +1,5 @@
 #include <asm/cpu_device_id.h>
-#include <asm/processor.h>
+#include <asm/cpufeature.h>
 #include <linux/cpu.h>
 #include <linux/module.h>
 #include <linux/slab.h>

index 3f20710a5b23..6988c74409a8 100644
@@ -1,6 +1,6 @@
 #!/bin/sh
 #
-# Generate the x86_cap/bug_flags[] arrays from include/asm/cpufeature.h
+# Generate the x86_cap/bug_flags[] arrays from include/asm/cpufeatures.h
 #
 
 IN=$1

@@ -49,8 +49,8 @@ dump_array()
 trap 'rm "$OUT"' EXIT
 
 (
-	echo "#ifndef _ASM_X86_CPUFEATURE_H"
-	echo "#include <asm/cpufeature.h>"
+	echo "#ifndef _ASM_X86_CPUFEATURES_H"
+	echo "#include <asm/cpufeatures.h>"
 	echo "#endif"
 	echo ""
 

index 5c3d149ee91c..74f1d90f9c29 100644
@@ -47,7 +47,7 @@
 #include <linux/smp.h>
 #include <linux/syscore_ops.h>
 
-#include <asm/processor.h>
+#include <asm/cpufeature.h>
 #include <asm/e820.h>
 #include <asm/mtrr.h>
 #include <asm/msr.h>

index 252da7aceca6..a19a663282b5 100644
@@ -1,6 +1,6 @@
 #include <linux/kernel.h>
 #include <linux/mm.h>
-#include <asm/processor.h>
+#include <asm/cpufeature.h>
 #include <asm/msr.h>
 #include "cpu.h"
 

index 569c1e4f96fe..b3c2a697820a 100644
@@ -24,6 +24,7 @@
 #include <asm/e820.h>
 #include <asm/proto.h>
 #include <asm/setup.h>
+#include <asm/cpufeature.h>
 
 /*
  * The e820 map is the map that gets modified e.g. with command line parameters

index 6bc9ae24b6d2..af1112980dd4 100644
@@ -19,7 +19,7 @@
 #include <asm/setup.h>
 #include <asm/processor-flags.h>
 #include <asm/msr-index.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/percpu.h>
 #include <asm/nops.h>
 #include <asm/bootparam.h>

index b8e6ff5cd5d0..be0ebbb6d1d1 100644
@@ -12,6 +12,7 @@
 #include <linux/pm.h>
 #include <linux/io.h>
 
+#include <asm/cpufeature.h>
 #include <asm/irqdomain.h>
 #include <asm/fixmap.h>
 #include <asm/hpet.h>

index 64f9616f93f1..7f3550acde1b 100644
@@ -40,7 +40,7 @@
 #include <linux/uaccess.h>
 #include <linux/gfp.h>
 
-#include <asm/processor.h>
+#include <asm/cpufeature.h>
 #include <asm/msr.h>
 
 static struct class *msr_class;

index 07efb35ee4bc..014ea59aa153 100644
@@ -30,7 +30,7 @@
  * 	appropriately. Either display a message or halt.
  */
 
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/msr-index.h>
 
 verify_cpu:

index a2fe51b00cce..65be7cfaf947 100644
@@ -1,5 +1,5 @@
 #include <linux/linkage.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/alternative-asm.h>
 
 /*

index 009f98216b7e..24ef1c2104d4 100644
@@ -1,7 +1,7 @@
 /* Written 2003 by Andi Kleen, based on a kernel by Evandro Menezes */
 
 #include <linux/linkage.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/alternative-asm.h>
 
 /*

index 982ce34f4a9b..fba343062055 100644
@@ -10,7 +10,7 @@
 #include <asm/current.h>
 #include <asm/asm-offsets.h>
 #include <asm/thread_info.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/alternative-asm.h>
 #include <asm/asm.h>
 #include <asm/smap.h>

index 16698bba87de..a0de849435ad 100644
@@ -1,7 +1,7 @@
 /* Copyright 2002 Andi Kleen */
 
 #include <linux/linkage.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/alternative-asm.h>
 
 /*

index ca2afdd6d98e..90ce01bee00c 100644
@@ -6,7 +6,7 @@
  *	- Copyright 2011 Fenghua Yu <[email protected]>
  */
 #include <linux/linkage.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/alternative-asm.h>
 
 #undef memmove

index 2661fad05827..c9c81227ea37 100644
@@ -1,7 +1,7 @@
 /* Copyright 2002 Andi Kleen, SuSE Labs */
 
 #include <linux/linkage.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/alternative-asm.h>
 
 .weak memset

index 92e2eacb3321..f65a33f505b6 100644
@@ -4,6 +4,7 @@
 
 #include <asm/pgtable.h>
 #include <asm/proto.h>
+#include <asm/cpufeature.h>
 
 static int disable_nx;
 

index 50d86c0e9ba4..660a83c8287b 100644
@@ -24,7 +24,6 @@
 #include <asm/nmi.h>
 #include <asm/apic.h>
 #include <asm/processor.h>
-#include <asm/cpufeature.h>
 
 #include "op_x86_model.h"
 #include "op_counter.h"

index 755481f14d90..764ac2fc53fe 100644
@@ -3,7 +3,7 @@
 
 #include <asm/asm.h>
 #include <asm/segment.h>
-#include <asm/cpufeature.h>
+#include <asm/cpufeatures.h>
 #include <asm/cmpxchg.h>
 #include <asm/nops.h>
 

index d62de8bf022d..123481814320 100644
@@ -17,7 +17,7 @@
 #include <linux/atomic.h>
 
 #ifdef CONFIG_X86
-#include <asm/processor.h>	/* for boot_cpu_has below */
+#include <asm/cpufeature.h>	/* for boot_cpu_has below */
 #endif
 
 #define TEST(bit, op, c_op, val)				\

2.3.5

From: Borislav Petkov <[email protected]>

So the old one didn't work properly before alternatives had run. And it was supposed to provide an optimized JMP because the assumption was that the offset it is jumping to is within a signed byte and thus a two-byte JMP.

So I did an x86_64 allyesconfig build and dumped all possible sites where static_cpu_has() was used. The optimization amounted to all in all 12(!) places where static_cpu_has() had generated a 2-byte JMP. Which has saved us a whopping 36 bytes!

This clearly is not worth the trouble so we can remove it. The only place where the optimization might count - in __switch_to() - we will handle differently. But that's not subject of this patch.

---
 arch/x86/Kconfig.debug               |  10 ----
 arch/x86/include/asm/cpufeature.h    | 100 +++--------------------------------
 arch/x86/include/asm/fpu/internal.h  |  14 ++---
 arch/x86/kernel/apic/apic_numachip.c |   4 +-
 arch/x86/kernel/cpu/common.c         |  12 +----
 arch/x86/kernel/vm86_32.c            |   2 +-
 drivers/cpufreq/intel_pstate.c       |   2 +-
 fs/btrfs/disk-io.c                   |   2 +-
 8 files changed, 21 insertions(+), 125 deletions(-)

index 9b18ed97a8a2..68a2d1f0a683 100644
@@ -350,16 +350,6 @@ config DEBUG_IMR_SELFTEST
 
 	  If unsure say N here.
 
-config X86_DEBUG_STATIC_CPU_HAS
-	bool "Debug alternatives"
-	depends on DEBUG_KERNEL
-	---help---
-	  This option causes additional code to be generated which
-	  fails if static_cpu_has() is used before alternatives have
-	  run.
-
-	  If unsure, say N.
-
 config X86_DEBUG_FPU
 	bool "Debug the x86 FPU code"
 	depends on DEBUG_KERNEL

index d01199def781..c2e46eb96b6d 100644
@@ -59,22 +59,22 @@ extern u64 fpu__get_supported_xfeatures_mask(void);
  */
 static __always_inline __pure bool use_eager_fpu(void)
 {
-	return static_cpu_has_safe(X86_FEATURE_EAGER_FPU);
+	return static_cpu_has(X86_FEATURE_EAGER_FPU);
 }
 
 static __always_inline __pure bool use_xsaveopt(void)
 {
-	return static_cpu_has_safe(X86_FEATURE_XSAVEOPT);
+	return static_cpu_has(X86_FEATURE_XSAVEOPT);
 }
 
 static __always_inline __pure bool use_xsave(void)
 {
-	return static_cpu_has_safe(X86_FEATURE_XSAVE);
+	return static_cpu_has(X86_FEATURE_XSAVE);
 }
 
 static __always_inline __pure bool use_fxsr(void)
 {
-	return static_cpu_has_safe(X86_FEATURE_FXSR);
+	return static_cpu_has(X86_FEATURE_FXSR);
 }
 
 /*

@@ -301,7 +301,7 @@ static inline void copy_xregs_to_kernel_booting(struct xregs_state *xstate)
 
 	WARN_ON(system_state != SYSTEM_BOOTING);
 
-	if (static_cpu_has_safe(X86_FEATURE_XSAVES))
+	if (static_cpu_has(X86_FEATURE_XSAVES))
 		XSTATE_OP(XSAVES, xstate, lmask, hmask, err);
 	else
 		XSTATE_OP(XSAVE, xstate, lmask, hmask, err);

@@ -323,7 +323,7 @@ static inline void copy_kernel_to_xregs_booting(struct xregs_state *xstate)
 
 	WARN_ON(system_state != SYSTEM_BOOTING);
 
-	if (static_cpu_has_safe(X86_FEATURE_XSAVES))
+	if (static_cpu_has(X86_FEATURE_XSAVES))
 		XSTATE_OP(XRSTORS, xstate, lmask, hmask, err);
 	else
 		XSTATE_OP(XRSTOR, xstate, lmask, hmask, err);

@@ -461,7 +461,7 @@ static inline void copy_kernel_to_fpregs(union fpregs_state *fpstate)
 	 * pending. Clear the x87 state here by setting it to fixed values.
 	 * "m" is a random variable that should be in L1.
 	 */
-	if (unlikely(static_cpu_has_bug_safe(X86_BUG_FXSAVE_LEAK))) {
+	if (unlikely(static_cpu_has_bug(X86_BUG_FXSAVE_LEAK))) {
 		asm volatile(
 			"fnclex\n\t"
 			"emms\n\t"

index c80c02c6ec49..ab5c2c685a3c 100644
@@ -30,7 +30,7 @@ static unsigned int numachip1_get_apic_id(unsigned long x)
 	unsigned long value;
 	unsigned int id = (x >> 24) & 0xff;
 
-	if (static_cpu_has_safe(X86_FEATURE_NODEID_MSR)) {
+	if (static_cpu_has(X86_FEATURE_NODEID_MSR)) {
 		rdmsrl(MSR_FAM10H_NODE_ID, value);
 		id |= (value << 2) & 0xff00;
 	}

@@ -178,7 +178,7 @@ static void fixup_cpu_id(struct cpuinfo_x86 *c, int node)
 	this_cpu_write(cpu_llc_id, node);
 
 	/* Account for nodes per socket in multi-core-module processors */
-	if (static_cpu_has_safe(X86_FEATURE_NODEID_MSR)) {
+	if (static_cpu_has(X86_FEATURE_NODEID_MSR)) {
 		rdmsrl(MSR_FAM10H_NODE_ID, val);
 		nodes = ((val >> 3) & 7) + 1;
 	}

index 37830de8f60a..ee499817f3f5 100644
@@ -1475,19 +1475,11 @@ void cpu_init(void)
 }
 #endif
 
-#ifdef CONFIG_X86_DEBUG_STATIC_CPU_HAS
-void warn_pre_alternatives(void)
-{
-	WARN(1, "You're using static_cpu_has before alternatives have run!\n");
-}
-EXPORT_SYMBOL_GPL(warn_pre_alternatives);
-#endif
-
-inline bool __static_cpu_has_safe(u16 bit)
+inline bool __static_cpu_has(u16 bit)
 {
 	return boot_cpu_has(bit);
 }
-EXPORT_SYMBOL_GPL(__static_cpu_has_safe);
+EXPORT_SYMBOL_GPL(__static_cpu_has);
 
 static void bsp_resume(void)
 {

index 483231ebbb0b..e6a1edc2f958 100644
@@ -358,7 +358,7 @@ static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus)
 	/* make room for real-mode segments */
 	tsk->thread.sp0 += 16;
 
-	if (static_cpu_has_safe(X86_FEATURE_SEP))
+	if (static_cpu_has(X86_FEATURE_SEP))
 		tsk->thread.sysenter_cs = 0;
 
 	load_sp0(tss, &tsk->thread);

index cd83d477e32d..3a4b39afc0ab 100644
@@ -1431,7 +1431,7 @@ static int __init intel_pstate_init(void)
 	if (!all_cpu_data)
 		return -ENOMEM;
 
-	if (static_cpu_has_safe(X86_FEATURE_HWP) && !no_hwp) {
+	if (static_cpu_has(X86_FEATURE_HWP) && !no_hwp) {
 		pr_info("intel_pstate: HWP enabled\n");
 		hwp_active++;
 	}

index 42a378a4eefb..6baedf3d806f 100644
@@ -923,7 +923,7 @@ static int check_async_write(struct inode *inode, unsigned long bio_flags)
 	if (bio_flags & EXTENT_BIO_TREE_LOG)
 		return 0;
 #ifdef CONFIG_X86
-	if (static_cpu_has_safe(X86_FEATURE_XMM4_2))
+	if (static_cpu_has(X86_FEATURE_XMM4_2))
 		return 0;
 #endif
 	return 1;

2.3.5

From: Borislav Petkov <[email protected]>

I can simply quote hpa from the mail:

"Get rid of the non-asm goto variant and just fall back to dynamic if asm goto is unavailable. It doesn't make any sense, really, if it is supposed to be safe, and by now the asm goto-capable gcc is in more wide use. (Originally the gcc 3.x fallback to pure dynamic didn't exist, either.)"

Booy, am I lazy.

Cleanup the whole CC_HAVE_ASM_GOTO ifdeffery too, while at it.

---
 arch/x86/include/asm/cpufeature.h | 49 ++++-----------------------------------
 1 file changed, 5 insertions(+), 44 deletions(-)

index a261cf2e7907..d48bf024f335 100644
@@ -129,17 +129,16 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
  * fast paths and boot_cpu_has() otherwise!
  */
 
-#if __GNUC__ >= 4 && defined(CONFIG_X86_FAST_FEATURE_TESTS)
+#if CC_HAVE_ASM_GOTO && defined(CONFIG_X86_FAST_FEATURE_TESTS)
 extern bool __static_cpu_has(u16 bit);
 
 /*
  * Static testing of CPU features.  Used the same as boot_cpu_has().
- * These are only valid after alternatives have run, but will statically
- * patch the target code for additional performance.
+ * These will statically patch the target code for additional
+ * performance.
  */
 static __always_inline __pure bool _static_cpu_has(u16 bit)
 {
-#ifdef CC_HAVE_ASM_GOTO
 		asm_volatile_goto("1: jmp %l[t_dynamic]\n"
 			 "2:\n"
 			 ".skip -(((5f-4f) - (2b-1b)) > 0) * "

@@ -172,45 +171,6 @@ static __always_inline __pure bool _static_cpu_has(u16 bit)
 		return false;
 	t_dynamic:
 		return __static_cpu_has(bit);
-#else
-		u8 flag;
-		/* Open-coded due to __stringify() in ALTERNATIVE() */
-		asm volatile("1: movb $2,%0\n"
-			     "2:\n"
-			     ".section .altinstructions,\"a\"\n"
-			     " .long 1b - .\n"		/* src offset */
-			     " .long 3f - .\n"		/* repl offset */
-			     " .word %P2\n"		/* always replace */
-			     " .byte 2b - 1b\n"		/* source len */
-			     " .byte 4f - 3f\n"		/* replacement len */
-			     " .byte 0\n"		/* pad len */
-			     ".previous\n"
-			     ".section .discard,\"aw\",@progbits\n"
-			     " .byte 0xff + (4f-3f) - (2b-1b)\n" /* size check */
-			     ".previous\n"
-			     ".section .altinstr_replacement,\"ax\"\n"
-			     "3: movb $0,%0\n"
-			     "4:\n"
-			     ".previous\n"
-			     ".section .altinstructions,\"a\"\n"
-			     " .long 1b - .\n"		/* src offset */
-			     " .long 5f - .\n"		/* repl offset */
-			     " .word %P1\n"		/* feature bit */
-			     " .byte 4b - 3b\n"		/* src len */
-			     " .byte 6f - 5f\n"		/* repl len */
-			     " .byte 0\n"		/* pad len */
-			     ".previous\n"
-			     ".section .discard,\"aw\",@progbits\n"
-			     " .byte 0xff + (6f-5f) - (4b-3b)\n" /* size check */
-			     ".previous\n"
-			     ".section .altinstr_replacement,\"ax\"\n"
-			     "5: movb $1,%0\n"
-			     "6:\n"
-			     ".previous\n"
-			     : "=qm" (flag)
-			     : "i" (bit), "i" (X86_FEATURE_ALWAYS));
-		return (flag == 2 ? __static_cpu_has(bit) : flag);
-#endif /* CC_HAVE_ASM_GOTO */
 }
 
 #define static_cpu_has(bit)					\

@@ -221,7 +181,8 @@ static __always_inline __pure bool _static_cpu_has(u16 bit)
 )
 #else
 /*
- * gcc 3.x is too stupid to do the static test; fall back to dynamic.
+ * 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

2.3.5

Good catch, thanks!

I thought about it while typing and then gcc didn't complain so I forgot again. :-\

Ingo, I'm sending the patches which changed as a reply to this message.

Thanks.

Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

From: Borislav Petkov <[email protected]>
Date: Wed, 20 Jan 2016 19:14:03 +0100
Subject: [PATCH 1/4] x86/cpufeature: Get rid of the non-asm goto variant