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

From: Borislav Petkov
Date: Wed Jan 20 2016 - 11:05:26 EST

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)
--