* [PATCH 0/5] Add a safe static_cpu_has variant
@ 2013-06-09 10:07 Borislav Petkov
2013-06-09 10:07 ` [PATCH 1/5] x86, cpu: Add a synthetic cpu feature Borislav Petkov
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Borislav Petkov @ 2013-06-09 10:07 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: X86 ML, LKML, Borislav Petkov
From: Borislav Petkov <bp@suse.de>
As recently experienced, using static_cpu_has too early (before
alternatives have run) causes some obscure bugs and decyphering those
doesn't simply point to such premature usage.
Therefore, let's add a static_cpu_has_safe variant which always works
and can be used in early code.
Also, 2/5 adds a debugging option, the idea behind it being to enable
all sensible debugging code which we want to enable on x86 in order to
catch build and runtime issues. The idea is to save a bunch of time of
wading through "Kernel hacking" and staring at options. We'll see how
that actually pans out though.
Borislav Petkov (5):
x86, cpu: Add a synthetic cpu feature
x86, debug: Add a collect-all misc debug checks option
x86: Sanity-check static_cpu_has usage
x86: Add a static_cpu_has_safe variant
x86, FPU: Use static_cpu_has_safe before alternatives
arch/x86/Kconfig.debug | 12 ++++
arch/x86/include/asm/cpufeature.h | 118 ++++++++++++++++++++++++++++++++++--
arch/x86/include/asm/fpu-internal.h | 2 +-
arch/x86/kernel/cpu/common.c | 16 +++++
4 files changed, 143 insertions(+), 5 deletions(-)
--
1.8.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/5] x86, cpu: Add a synthetic cpu feature
2013-06-09 10:07 [PATCH 0/5] Add a safe static_cpu_has variant Borislav Petkov
@ 2013-06-09 10:07 ` Borislav Petkov
2013-06-21 0:51 ` [tip:x86/fpu] x86, cpu: Add a synthetic, always true, " tip-bot for Borislav Petkov
2013-06-09 10:07 ` [PATCH 2/5] x86, debug: Add a collect-all misc debug checks option Borislav Petkov
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2013-06-09 10:07 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: X86 ML, LKML, Borislav Petkov
From: Borislav Petkov <bp@suse.de>
This will be used in alternatives later as an always-replace flag.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/include/asm/cpufeature.h | 2 +-
arch/x86/kernel/cpu/common.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index e99ac27f95b2..043d61e66efc 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -92,7 +92,7 @@
#define X86_FEATURE_LFENCE_RDTSC (3*32+18) /* "" Lfence synchronizes RDTSC */
#define X86_FEATURE_11AP (3*32+19) /* "" Bad local APIC aka 11AP */
#define X86_FEATURE_NOPL (3*32+20) /* The NOPL (0F 1F) instructions */
- /* 21 available, was AMD_C1E */
+#define X86_FEATURE_ALWAYS (3*32+21) /* "" Always-present feature */
#define X86_FEATURE_XTOPOLOGY (3*32+22) /* cpu topology enum extensions */
#define X86_FEATURE_TSC_RELIABLE (3*32+23) /* TSC is known to be reliable */
#define X86_FEATURE_NONSTOP_TSC (3*32+24) /* TSC does not stop in C states */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 22018f70a671..6a0361d7132c 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -724,6 +724,8 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
if (this_cpu->c_bsp_init)
this_cpu->c_bsp_init(c);
+
+ setup_force_cpu_cap(X86_FEATURE_ALWAYS);
}
void __init early_cpu_init(void)
--
1.8.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] x86, debug: Add a collect-all misc debug checks option
2013-06-09 10:07 [PATCH 0/5] Add a safe static_cpu_has variant Borislav Petkov
2013-06-09 10:07 ` [PATCH 1/5] x86, cpu: Add a synthetic cpu feature Borislav Petkov
@ 2013-06-09 10:07 ` Borislav Petkov
2013-06-09 10:07 ` [PATCH 3/5] x86: Sanity-check static_cpu_has usage Borislav Petkov
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Borislav Petkov @ 2013-06-09 10:07 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: X86 ML, LKML, Borislav Petkov
From: Borislav Petkov <bp@suse.de>
Instead of adding a Kconfig debug option for every single aspect of x86
debugging code we have in the kernel, let's just simply add one which
people can enable and get all the debugging stuff switched on; boot and
test the resulting kernel and if they're satisfied, ship their patches.
This should save a lot of time of wading through "Kernel hacking" and
wondering what option to enable.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/Kconfig.debug | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index b6a770132b67..1dab525cd358 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -303,4 +303,16 @@ config DEBUG_NMI_SELFTEST
If unsure, say N.
+config X86_MISC_DEBUG_CHECKS
+ bool "Miscellaneous debugging checks"
+ depends on DEBUG_KERNEL
+ ---help---
+ Enable different build and runtime sanity checks. Those are
+ lumped together here so as not to search through Kconfig debug
+ options but have this one only enabled and get them all.
+
+ This option enables:
+ - static_cpu_has premature usage
+
+ If unsure, say N.
endmenu
--
1.8.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/5] x86: Sanity-check static_cpu_has usage
2013-06-09 10:07 [PATCH 0/5] Add a safe static_cpu_has variant Borislav Petkov
2013-06-09 10:07 ` [PATCH 1/5] x86, cpu: Add a synthetic cpu feature Borislav Petkov
2013-06-09 10:07 ` [PATCH 2/5] x86, debug: Add a collect-all misc debug checks option Borislav Petkov
@ 2013-06-09 10:07 ` Borislav Petkov
2013-06-21 0:51 ` [tip:x86/fpu] " tip-bot for Borislav Petkov
2013-06-09 10:07 ` [PATCH 4/5] x86: Add a static_cpu_has_safe variant Borislav Petkov
2013-06-09 10:07 ` [PATCH 5/5] x86, FPU: Use static_cpu_has_safe before alternatives Borislav Petkov
4 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2013-06-09 10:07 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: X86 ML, LKML, Borislav Petkov
From: Borislav Petkov <bp@suse.de>
static_cpu_has may be used only after alternatives have run. Before that
it always returns false if constant folding with __builtin_constant_p()
doesn't happen. And you don't want that.
This patch is the result of me debugging an issue where I overzealously
put static_cpu_has in code which executed before alternatives have run
and had to spend some time with scratching head and cursing at the
monitor.
So add a jump to a warning which screams loudly when we use this
function too early. The alternatives patch that check away in
conjunction with patching the rest of the kernel image.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/include/asm/cpufeature.h | 30 ++++++++++++++++++++++++++++--
arch/x86/kernel/cpu/common.c | 8 ++++++++
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 043d61e66efc..eba671c0e5b1 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -356,15 +356,35 @@ extern const char * const x86_power_flags[32];
#endif /* CONFIG_X86_64 */
#if __GNUC__ >= 4
+extern void warn_pre_alternatives(void);
+
/*
* 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.
- *
*/
static __always_inline __pure bool __static_cpu_has(u16 bit)
{
#if __GNUC__ > 4 || __GNUC_MINOR__ >= 5
+
+#ifdef CONFIG_X86_MISC_DEBUG_CHECKS
+ /*
+ * Catch too early usage of this before alternatives
+ * have run.
+ */
+ asm goto("1: jmp %l[t_warn]\n"
+ "2:\n"
+ ".section .altinstructions,\"a\"\n"
+ " .long 1b - .\n"
+ " .long 0\n" /* no replacement */
+ " .word %P0\n" /* 1: do replace */
+ " .byte 2b - 1b\n" /* source len */
+ " .byte 0\n" /* replacement len */
+ ".previous\n"
+ /* skipping size check since replacement size = 0 */
+ : : "i" (X86_FEATURE_ALWAYS) : : t_warn);
+#endif
+
asm goto("1: jmp %l[t_no]\n"
"2:\n"
".section .altinstructions,\"a\"\n"
@@ -379,7 +399,13 @@ static __always_inline __pure bool __static_cpu_has(u16 bit)
return true;
t_no:
return false;
-#else
+
+#ifdef CONFIG_X86_MISC_DEBUG_CHECKS
+ t_warn:
+ warn_pre_alternatives();
+ return false;
+#endif
+#else /* GCC_VERSION >= 40500 */
u8 flag;
/* Open-coded due to __stringify() in ALTERNATIVE() */
asm volatile("1: movb $0,%0\n"
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 6a0361d7132c..407510155f40 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1365,3 +1365,11 @@ void __cpuinit cpu_init(void)
fpu_init();
}
#endif
+
+#ifdef CONFIG_X86_MISC_DEBUG_CHECKS
+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
--
1.8.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] x86: Add a static_cpu_has_safe variant
2013-06-09 10:07 [PATCH 0/5] Add a safe static_cpu_has variant Borislav Petkov
` (2 preceding siblings ...)
2013-06-09 10:07 ` [PATCH 3/5] x86: Sanity-check static_cpu_has usage Borislav Petkov
@ 2013-06-09 10:07 ` Borislav Petkov
2013-06-21 0:51 ` [tip:x86/fpu] " tip-bot for Borislav Petkov
2013-06-09 10:07 ` [PATCH 5/5] x86, FPU: Use static_cpu_has_safe before alternatives Borislav Petkov
4 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2013-06-09 10:07 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: X86 ML, LKML, Borislav Petkov
From: Borislav Petkov <bp@suse.de>
We want to use this in early code where alternatives might not have run
yet and for that case we fall back to the dynamic boot_cpu_has.
For that, force a 5-byte jump since the compiler could be generating
differently sized jumps for each label.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/include/asm/cpufeature.h | 86 ++++++++++++++++++++++++++++++++++++++-
arch/x86/kernel/cpu/common.c | 6 +++
2 files changed, 91 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index eba671c0e5b1..0a803b74ac35 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -357,6 +357,7 @@ extern const char * const x86_power_flags[32];
#if __GNUC__ >= 4
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().
@@ -437,11 +438,94 @@ static __always_inline __pure bool __static_cpu_has(u16 bit)
__static_cpu_has(bit) : \
boot_cpu_has(bit) \
)
+
+static __always_inline __pure bool _static_cpu_has_safe(u16 bit)
+{
+#if __GNUC__ > 4 || __GNUC_MINOR__ >= 5
+/*
+ * We need to spell the jumps to the compiler because, depending on the offset,
+ * the replacement jump can be bigger than the original jump, and this we cannot
+ * have. Thus, we force the jump to the widest, 4-byte, signed relative
+ * offset even though the last would often fit in less bytes.
+ */
+ asm goto("1: .byte 0xe9\n .long %l[t_dynamic] - 2f\n"
+ "2:\n"
+ ".section .altinstructions,\"a\"\n"
+ " .long 1b - .\n" /* src offset */
+ " .long 3f - .\n" /* repl offset */
+ " .word %P1\n" /* always replace */
+ " .byte 2b - 1b\n" /* src len */
+ " .byte 4f - 3f\n" /* repl len */
+ ".previous\n"
+ ".section .altinstr_replacement,\"ax\"\n"
+ "3: .byte 0xe9\n .long %l[t_no] - 2b\n"
+ "4:\n"
+ ".previous\n"
+ ".section .altinstructions,\"a\"\n"
+ " .long 1b - .\n" /* src offset */
+ " .long 0\n" /* no replacement */
+ " .word %P0\n" /* feature bit */
+ " .byte 2b - 1b\n" /* src len */
+ " .byte 0\n" /* repl len */
+ ".previous\n"
+ : : "i" (bit), "i" (X86_FEATURE_ALWAYS)
+ : : t_dynamic, t_no);
+ return true;
+ t_no:
+ return false;
+ t_dynamic:
+ return __static_cpu_has_safe(bit);
+#else /* GCC_VERSION >= 40500 */
+ 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 */
+ ".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 */
+ ".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_safe(bit) : flag);
+#endif
+}
+
+#define static_cpu_has_safe(bit) \
+( \
+ __builtin_constant_p(boot_cpu_has(bit)) ? \
+ boot_cpu_has(bit) : \
+ _static_cpu_has_safe(bit) \
+)
#else
/*
* gcc 3.x is too stupid to do the static test; fall back to dynamic.
*/
-#define static_cpu_has(bit) boot_cpu_has(bit)
+#define static_cpu_has(bit) boot_cpu_has(bit)
+#define static_cpu_has_safe(bit) boot_cpu_has(bit)
#endif
#define cpu_has_bug(c, bit) cpu_has(c, (bit))
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 407510155f40..f30267ac9371 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1373,3 +1373,9 @@ 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);
--
1.8.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] x86, FPU: Use static_cpu_has_safe before alternatives
2013-06-09 10:07 [PATCH 0/5] Add a safe static_cpu_has variant Borislav Petkov
` (3 preceding siblings ...)
2013-06-09 10:07 ` [PATCH 4/5] x86: Add a static_cpu_has_safe variant Borislav Petkov
@ 2013-06-09 10:07 ` Borislav Petkov
2013-06-21 0:52 ` [tip:x86/fpu] x86, fpu: " tip-bot for Borislav Petkov
4 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2013-06-09 10:07 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: X86 ML, LKML, Borislav Petkov
From: Borislav Petkov <bp@suse.de>
The call stack below shows how this happens: basically eager_fpu_init()
calls __thread_fpu_begin(current) which then does if (!use_eager_fpu()),
which, in turn, uses static_cpu_has.
And we're executing before alternatives so static_cpu_has doesn't work
there yet.
Use the safe variant in this path which becomes optimal after
alternatives have run.
WARNING: at arch/x86/kernel/cpu/common.c:1368 warn_pre_alternatives+0x1e/0x20()
You're using static_cpu_has before alternatives have run!
Modules linked in:
Pid: 0, comm: swapper Not tainted 3.9.0-rc8+ #1
Call Trace:
warn_slowpath_common
warn_slowpath_fmt
? fpu_finit
warn_pre_alternatives
eager_fpu_init
fpu_init
cpu_init
trap_init
start_kernel
? repair_env_string
x86_64_start_reservations
x86_64_start_kernel
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/include/asm/fpu-internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index e25cc33ec54d..b36f31b18664 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -345,7 +345,7 @@ static inline void __thread_fpu_end(struct task_struct *tsk)
static inline void __thread_fpu_begin(struct task_struct *tsk)
{
- if (!use_eager_fpu())
+ if (!static_cpu_has_safe(X86_FEATURE_EAGER_FPU))
clts();
__thread_set_has_fpu(tsk);
}
--
1.8.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [tip:x86/fpu] x86, cpu: Add a synthetic, always true, cpu feature
2013-06-09 10:07 ` [PATCH 1/5] x86, cpu: Add a synthetic cpu feature Borislav Petkov
@ 2013-06-21 0:51 ` tip-bot for Borislav Petkov
0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Borislav Petkov @ 2013-06-21 0:51 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, tglx, hpa, bp
Commit-ID: c3b83598c1eeb1507603b461f5843ec2a49e3033
Gitweb: http://git.kernel.org/tip/c3b83598c1eeb1507603b461f5843ec2a49e3033
Author: Borislav Petkov <bp@suse.de>
AuthorDate: Sun, 9 Jun 2013 12:07:30 +0200
Committer: H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Thu, 20 Jun 2013 17:06:07 -0700
x86, cpu: Add a synthetic, always true, cpu feature
This will be used in alternatives later as an always-replace flag.
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: http://lkml.kernel.org/r/1370772454-6106-2-git-send-email-bp@alien8.de
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
arch/x86/include/asm/cpufeature.h | 2 +-
arch/x86/kernel/cpu/common.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index e99ac27..043d61e 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -92,7 +92,7 @@
#define X86_FEATURE_LFENCE_RDTSC (3*32+18) /* "" Lfence synchronizes RDTSC */
#define X86_FEATURE_11AP (3*32+19) /* "" Bad local APIC aka 11AP */
#define X86_FEATURE_NOPL (3*32+20) /* The NOPL (0F 1F) instructions */
- /* 21 available, was AMD_C1E */
+#define X86_FEATURE_ALWAYS (3*32+21) /* "" Always-present feature */
#define X86_FEATURE_XTOPOLOGY (3*32+22) /* cpu topology enum extensions */
#define X86_FEATURE_TSC_RELIABLE (3*32+23) /* TSC is known to be reliable */
#define X86_FEATURE_NONSTOP_TSC (3*32+24) /* TSC does not stop in C states */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index d4dd993..d388ce1 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -723,6 +723,8 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
if (this_cpu->c_bsp_init)
this_cpu->c_bsp_init(c);
+
+ setup_force_cpu_cap(X86_FEATURE_ALWAYS);
}
void __init early_cpu_init(void)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [tip:x86/fpu] x86: Sanity-check static_cpu_has usage
2013-06-09 10:07 ` [PATCH 3/5] x86: Sanity-check static_cpu_has usage Borislav Petkov
@ 2013-06-21 0:51 ` tip-bot for Borislav Petkov
0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Borislav Petkov @ 2013-06-21 0:51 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, tglx, hpa, bp
Commit-ID: 5700f743b597951743da9c7d891d3989aac0486e
Gitweb: http://git.kernel.org/tip/5700f743b597951743da9c7d891d3989aac0486e
Author: Borislav Petkov <bp@suse.de>
AuthorDate: Sun, 9 Jun 2013 12:07:32 +0200
Committer: H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Thu, 20 Jun 2013 17:37:19 -0700
x86: Sanity-check static_cpu_has usage
static_cpu_has may be used only after alternatives have run. Before that
it always returns false if constant folding with __builtin_constant_p()
doesn't happen. And you don't want that.
This patch is the result of me debugging an issue where I overzealously
put static_cpu_has in code which executed before alternatives have run
and had to spend some time with scratching head and cursing at the
monitor.
So add a jump to a warning which screams loudly when we use this
function too early. The alternatives patch that check away in
conjunction with patching the rest of the kernel image.
[ hpa: factored this into its own configuration option. If we want to
have an overarching option, it should be an option which selects
other options, not as a group option in the source code. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: http://lkml.kernel.org/r/1370772454-6106-4-git-send-email-bp@alien8.de
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
arch/x86/Kconfig.debug | 10 ++++++++++
arch/x86/include/asm/cpufeature.h | 30 ++++++++++++++++++++++++++++--
arch/x86/kernel/cpu/common.c | 8 ++++++++
3 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index c198b7e..c6acdf7 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -304,4 +304,14 @@ config DEBUG_NMI_SELFTEST
If unsure, say N.
+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.
+
endmenu
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 043d61e..252b28f 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -356,15 +356,35 @@ extern const char * const x86_power_flags[32];
#endif /* CONFIG_X86_64 */
#if __GNUC__ >= 4
+extern void warn_pre_alternatives(void);
+
/*
* 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.
- *
*/
static __always_inline __pure bool __static_cpu_has(u16 bit)
{
#if __GNUC__ > 4 || __GNUC_MINOR__ >= 5
+
+#ifdef CONFIG_X86_DEBUG_STATIC_CPU_HAS
+ /*
+ * Catch too early usage of this before alternatives
+ * have run.
+ */
+ asm goto("1: jmp %l[t_warn]\n"
+ "2:\n"
+ ".section .altinstructions,\"a\"\n"
+ " .long 1b - .\n"
+ " .long 0\n" /* no replacement */
+ " .word %P0\n" /* 1: do replace */
+ " .byte 2b - 1b\n" /* source len */
+ " .byte 0\n" /* replacement len */
+ ".previous\n"
+ /* skipping size check since replacement size = 0 */
+ : : "i" (X86_FEATURE_ALWAYS) : : t_warn);
+#endif
+
asm goto("1: jmp %l[t_no]\n"
"2:\n"
".section .altinstructions,\"a\"\n"
@@ -379,7 +399,13 @@ static __always_inline __pure bool __static_cpu_has(u16 bit)
return true;
t_no:
return false;
-#else
+
+#ifdef CONFIG_X86_DEBUG_STATIC_CPU_HAS
+ t_warn:
+ warn_pre_alternatives();
+ return false;
+#endif
+#else /* GCC_VERSION >= 40500 */
u8 flag;
/* Open-coded due to __stringify() in ALTERNATIVE() */
asm volatile("1: movb $0,%0\n"
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index d388ce1..59adaa1 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1364,3 +1364,11 @@ void __cpuinit cpu_init(void)
fpu_init();
}
#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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [tip:x86/fpu] x86: Add a static_cpu_has_safe variant
2013-06-09 10:07 ` [PATCH 4/5] x86: Add a static_cpu_has_safe variant Borislav Petkov
@ 2013-06-21 0:51 ` tip-bot for Borislav Petkov
0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Borislav Petkov @ 2013-06-21 0:51 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, tglx, hpa, bp
Commit-ID: 4a90a99c4f8002edaa6be11bd756872ebf3f3d97
Gitweb: http://git.kernel.org/tip/4a90a99c4f8002edaa6be11bd756872ebf3f3d97
Author: Borislav Petkov <bp@suse.de>
AuthorDate: Sun, 9 Jun 2013 12:07:33 +0200
Committer: H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Thu, 20 Jun 2013 17:38:14 -0700
x86: Add a static_cpu_has_safe variant
We want to use this in early code where alternatives might not have run
yet and for that case we fall back to the dynamic boot_cpu_has.
For that, force a 5-byte jump since the compiler could be generating
differently sized jumps for each label.
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: http://lkml.kernel.org/r/1370772454-6106-5-git-send-email-bp@alien8.de
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
arch/x86/include/asm/cpufeature.h | 86 ++++++++++++++++++++++++++++++++++++++-
arch/x86/kernel/cpu/common.c | 6 +++
2 files changed, 91 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 252b28f..47538a6 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -357,6 +357,7 @@ extern const char * const x86_power_flags[32];
#if __GNUC__ >= 4
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().
@@ -437,11 +438,94 @@ static __always_inline __pure bool __static_cpu_has(u16 bit)
__static_cpu_has(bit) : \
boot_cpu_has(bit) \
)
+
+static __always_inline __pure bool _static_cpu_has_safe(u16 bit)
+{
+#if __GNUC__ > 4 || __GNUC_MINOR__ >= 5
+/*
+ * We need to spell the jumps to the compiler because, depending on the offset,
+ * the replacement jump can be bigger than the original jump, and this we cannot
+ * have. Thus, we force the jump to the widest, 4-byte, signed relative
+ * offset even though the last would often fit in less bytes.
+ */
+ asm goto("1: .byte 0xe9\n .long %l[t_dynamic] - 2f\n"
+ "2:\n"
+ ".section .altinstructions,\"a\"\n"
+ " .long 1b - .\n" /* src offset */
+ " .long 3f - .\n" /* repl offset */
+ " .word %P1\n" /* always replace */
+ " .byte 2b - 1b\n" /* src len */
+ " .byte 4f - 3f\n" /* repl len */
+ ".previous\n"
+ ".section .altinstr_replacement,\"ax\"\n"
+ "3: .byte 0xe9\n .long %l[t_no] - 2b\n"
+ "4:\n"
+ ".previous\n"
+ ".section .altinstructions,\"a\"\n"
+ " .long 1b - .\n" /* src offset */
+ " .long 0\n" /* no replacement */
+ " .word %P0\n" /* feature bit */
+ " .byte 2b - 1b\n" /* src len */
+ " .byte 0\n" /* repl len */
+ ".previous\n"
+ : : "i" (bit), "i" (X86_FEATURE_ALWAYS)
+ : : t_dynamic, t_no);
+ return true;
+ t_no:
+ return false;
+ t_dynamic:
+ return __static_cpu_has_safe(bit);
+#else /* GCC_VERSION >= 40500 */
+ 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 */
+ ".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 */
+ ".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_safe(bit) : flag);
+#endif
+}
+
+#define static_cpu_has_safe(bit) \
+( \
+ __builtin_constant_p(boot_cpu_has(bit)) ? \
+ boot_cpu_has(bit) : \
+ _static_cpu_has_safe(bit) \
+)
#else
/*
* gcc 3.x is too stupid to do the static test; fall back to dynamic.
*/
-#define static_cpu_has(bit) boot_cpu_has(bit)
+#define static_cpu_has(bit) boot_cpu_has(bit)
+#define static_cpu_has_safe(bit) boot_cpu_has(bit)
#endif
#define cpu_has_bug(c, bit) cpu_has(c, (bit))
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 59adaa1..a4a07c0 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1372,3 +1372,9 @@ 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);
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [tip:x86/fpu] x86, fpu: Use static_cpu_has_safe before alternatives
2013-06-09 10:07 ` [PATCH 5/5] x86, FPU: Use static_cpu_has_safe before alternatives Borislav Petkov
@ 2013-06-21 0:52 ` tip-bot for Borislav Petkov
0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Borislav Petkov @ 2013-06-21 0:52 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, tglx, hpa, bp
Commit-ID: 5f8c4218148822fde6eebbeefc34bd0a6061e031
Gitweb: http://git.kernel.org/tip/5f8c4218148822fde6eebbeefc34bd0a6061e031
Author: Borislav Petkov <bp@suse.de>
AuthorDate: Sun, 9 Jun 2013 12:07:34 +0200
Committer: H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Thu, 20 Jun 2013 17:38:22 -0700
x86, fpu: Use static_cpu_has_safe before alternatives
The call stack below shows how this happens: basically eager_fpu_init()
calls __thread_fpu_begin(current) which then does if (!use_eager_fpu()),
which, in turn, uses static_cpu_has.
And we're executing before alternatives so static_cpu_has doesn't work
there yet.
Use the safe variant in this path which becomes optimal after
alternatives have run.
WARNING: at arch/x86/kernel/cpu/common.c:1368 warn_pre_alternatives+0x1e/0x20()
You're using static_cpu_has before alternatives have run!
Modules linked in:
Pid: 0, comm: swapper Not tainted 3.9.0-rc8+ #1
Call Trace:
warn_slowpath_common
warn_slowpath_fmt
? fpu_finit
warn_pre_alternatives
eager_fpu_init
fpu_init
cpu_init
trap_init
start_kernel
? repair_env_string
x86_64_start_reservations
x86_64_start_kernel
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: http://lkml.kernel.org/r/1370772454-6106-6-git-send-email-bp@alien8.de
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
arch/x86/include/asm/fpu-internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/fpu-internal.h b/arch/x86/include/asm/fpu-internal.h
index fb808d7..4d0bda7 100644
--- a/arch/x86/include/asm/fpu-internal.h
+++ b/arch/x86/include/asm/fpu-internal.h
@@ -343,7 +343,7 @@ static inline void __thread_fpu_end(struct task_struct *tsk)
static inline void __thread_fpu_begin(struct task_struct *tsk)
{
- if (!use_eager_fpu())
+ if (!static_cpu_has_safe(X86_FEATURE_EAGER_FPU))
clts();
__thread_set_has_fpu(tsk);
}
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-06-21 0:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-09 10:07 [PATCH 0/5] Add a safe static_cpu_has variant Borislav Petkov
2013-06-09 10:07 ` [PATCH 1/5] x86, cpu: Add a synthetic cpu feature Borislav Petkov
2013-06-21 0:51 ` [tip:x86/fpu] x86, cpu: Add a synthetic, always true, " tip-bot for Borislav Petkov
2013-06-09 10:07 ` [PATCH 2/5] x86, debug: Add a collect-all misc debug checks option Borislav Petkov
2013-06-09 10:07 ` [PATCH 3/5] x86: Sanity-check static_cpu_has usage Borislav Petkov
2013-06-21 0:51 ` [tip:x86/fpu] " tip-bot for Borislav Petkov
2013-06-09 10:07 ` [PATCH 4/5] x86: Add a static_cpu_has_safe variant Borislav Petkov
2013-06-21 0:51 ` [tip:x86/fpu] " tip-bot for Borislav Petkov
2013-06-09 10:07 ` [PATCH 5/5] x86, FPU: Use static_cpu_has_safe before alternatives Borislav Petkov
2013-06-21 0:52 ` [tip:x86/fpu] x86, fpu: " tip-bot for Borislav Petkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).