From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934683AbcATQFX (ORCPT ); Wed, 20 Jan 2016 11:05:23 -0500 Received: from mx2.suse.de ([195.135.220.15]:35442 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933889AbcATQFS (ORCPT ); Wed, 20 Jan 2016 11:05:18 -0500 Date: Wed, 20 Jan 2016 17:04:51 +0100 From: Borislav Petkov To: "H. Peter Anvin" Cc: Brian Gerst , the arch/x86 maintainers , Linux Kernel Mailing List , Ingo Molnar , Denys Vlasenko , Andy Lutomirski , Linus Torvalds Subject: Re: [PATCH] x86: static_cpu_has_safe: discard dynamic check after init Message-ID: <20160120160451.GG23350@pd.tnic> References: <20160117103337.GC8549@pd.tnic> <20160118181457.GG12651@pd.tnic> <20160118185107.GI12651@pd.tnic> <20160119011026.GA12911@pd.tnic> <20160119092213.GA15071@pd.tnic> <569F06B9.4060903@zytor.com> <20160120150109.GF23350@pd.tnic> <569FA333.7020401@zytor.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <569FA333.7020401@zytor.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 +#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 +/* + * 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) --