Re: [PATCH] x86: static_cpu_has_safe: discard dynamic check after init

[Borislav Petkov <bp@suse.de>]

On Wed, Jan 20, 2016 at 07:09:39AM -0800, H. Peter Anvin wrote:
> But then you're using testl and get long immediates.
> 
> (And the parentheses around boot_cpu_data->x86_capability are redundant.)

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

diff --git a/arch/x86/boot/mkcpustr.c b/arch/x86/boot/mkcpustr.c
index 637097e66a62..c32113a0e3d4 100644
--- a/arch/x86/boot/mkcpustr.c
+++ b/arch/x86/boot/mkcpustr.c
@@ -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"
 
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 7ad8c9464297..a16cee2376c4 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -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 */
 }
 
diff --git a/arch/x86/include/asm/cpuinfo.h b/arch/x86/include/asm/cpuinfo.h
new file mode 100644
index 000000000000..f906b538fdb4
--- /dev/null
+++ b/arch/x86/include/asm/cpuinfo.h
@@ -0,0 +1,73 @@
+#ifndef _ASM_X86_CPUINFO_H
+#define _ASM_X86_CPUINFO_H
+
+#ifndef __ASSEMBLY__
+
+#include <asm/types.h>
+/*
+ *  CPU type and hardware bug flags. Kept separately for each CPU.
+ *  Members of this structure are referenced in head.S, so think twice
+ *  before touching them. [mj]
+ */
+
+struct cpuinfo_x86 {
+	__u8			x86;		/* CPU family */
+	__u8			x86_vendor;	/* CPU vendor */
+	__u8			x86_model;
+	__u8			x86_mask;
+#ifdef CONFIG_X86_32
+	char			wp_works_ok;	/* It doesn't on 386's */
+
+	/* Problems on some 486Dx4's and old 386's: */
+	char			rfu;
+	char			pad0;
+	char			pad1;
+#else
+	/* Number of 4K pages in DTLB/ITLB combined(in pages): */
+	int			x86_tlbsize;
+#endif
+	__u8			x86_virt_bits;
+	__u8			x86_phys_bits;
+	/* CPUID returned core id bits: */
+	__u8			x86_coreid_bits;
+	/* Max extended CPUID function supported: */
+	__u32			extended_cpuid_level;
+	/* Maximum supported CPUID level, -1=no CPUID: */
+	int			cpuid_level;
+	__u32			x86_capability[NCAPINTS + NBUGINTS];
+	char			x86_vendor_id[16];
+	char			x86_model_id[64];
+	/* in KB - valid for CPUS which support this call: */
+	int			x86_cache_size;
+	int			x86_cache_alignment;	/* In bytes */
+	/* Cache QoS architectural values: */
+	int			x86_cache_max_rmid;	/* max index */
+	int			x86_cache_occ_scale;	/* scale to bytes */
+	int			x86_power;
+	unsigned long		loops_per_jiffy;
+	/* cpuid returned max cores value: */
+	u16			 x86_max_cores;
+	u16			apicid;
+	u16			initial_apicid;
+	u16			x86_clflush_size;
+	/* number of cores as seen by the OS: */
+	u16			booted_cores;
+	/* Physical processor id: */
+	u16			phys_proc_id;
+	/* Core id: */
+	u16			cpu_core_id;
+	/* Compute unit id */
+	u8			compute_unit_id;
+	/* Index into per_cpu list: */
+	u16			cpu_index;
+	u32			microcode;
+};
+
+/*
+ * capabilities of CPUs
+ */
+extern struct cpuinfo_x86	boot_cpu_data;
+extern struct cpuinfo_x86	new_cpu_data;
+#endif
+
+#endif /* _ASM_X86_CPUINFO_H */
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 2d5a50cb61a2..240d8f8d8c1b 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -79,65 +79,6 @@ extern u16 __read_mostly tlb_lld_2m[NR_INFO];
 extern u16 __read_mostly tlb_lld_4m[NR_INFO];
 extern u16 __read_mostly tlb_lld_1g[NR_INFO];
 
-/*
- *  CPU type and hardware bug flags. Kept separately for each CPU.
- *  Members of this structure are referenced in head.S, so think twice
- *  before touching them. [mj]
- */
-
-struct cpuinfo_x86 {
-	__u8			x86;		/* CPU family */
-	__u8			x86_vendor;	/* CPU vendor */
-	__u8			x86_model;
-	__u8			x86_mask;
-#ifdef CONFIG_X86_32
-	char			wp_works_ok;	/* It doesn't on 386's */
-
-	/* Problems on some 486Dx4's and old 386's: */
-	char			rfu;
-	char			pad0;
-	char			pad1;
-#else
-	/* Number of 4K pages in DTLB/ITLB combined(in pages): */
-	int			x86_tlbsize;
-#endif
-	__u8			x86_virt_bits;
-	__u8			x86_phys_bits;
-	/* CPUID returned core id bits: */
-	__u8			x86_coreid_bits;
-	/* Max extended CPUID function supported: */
-	__u32			extended_cpuid_level;
-	/* Maximum supported CPUID level, -1=no CPUID: */
-	int			cpuid_level;
-	__u32			x86_capability[NCAPINTS + NBUGINTS];
-	char			x86_vendor_id[16];
-	char			x86_model_id[64];
-	/* in KB - valid for CPUS which support this call: */
-	int			x86_cache_size;
-	int			x86_cache_alignment;	/* In bytes */
-	/* Cache QoS architectural values: */
-	int			x86_cache_max_rmid;	/* max index */
-	int			x86_cache_occ_scale;	/* scale to bytes */
-	int			x86_power;
-	unsigned long		loops_per_jiffy;
-	/* cpuid returned max cores value: */
-	u16			 x86_max_cores;
-	u16			apicid;
-	u16			initial_apicid;
-	u16			x86_clflush_size;
-	/* number of cores as seen by the OS: */
-	u16			booted_cores;
-	/* Physical processor id: */
-	u16			phys_proc_id;
-	/* Core id: */
-	u16			cpu_core_id;
-	/* Compute unit id */
-	u8			compute_unit_id;
-	/* Index into per_cpu list: */
-	u16			cpu_index;
-	u32			microcode;
-};
-
 #define X86_VENDOR_INTEL	0
 #define X86_VENDOR_CYRIX	1
 #define X86_VENDOR_AMD		2
@@ -149,12 +90,6 @@ struct cpuinfo_x86 {
 
 #define X86_VENDOR_UNKNOWN	0xff
 
-/*
- * capabilities of CPUs
- */
-extern struct cpuinfo_x86	boot_cpu_data;
-extern struct cpuinfo_x86	new_cpu_data;
-
 extern struct tss_struct	doublefault_tss;
 extern __u32			cpu_caps_cleared[NCAPINTS];
 extern __u32			cpu_caps_set[NCAPINTS];
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 37830de8f60a..897c65bd3faa 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -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)
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 35868bf529b9..486dc0e60599 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -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)
--