Hi all,
32-bit kernels currently crash/reboot in early microcode loading when
non-dynamic function tracing is enabled. Several functions in that path
get instrumented with mcount, but its non-dynamic implementation does
not work before paging is enabled (it accesses global variables at wrong
addresses).
Below some hunks to get it working again - at least in the absence of
any microcode in the initrd. Marking all involved functions as __init is
another option (as __init implies notrace). But I bet there is more
hidden. I see e.g. a pr_warn() in find_cpio_init that should trigger the
issue as well if we hit the error it reports (btw. printing at this
point of the boot should not work anyway, should it?).
Better ideas?
Jan
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 88d99ea..c093a9c 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -14,6 +14,9 @@ CFLAGS_REMOVE_pvclock.o = -pg
CFLAGS_REMOVE_kvmclock.o = -pg
CFLAGS_REMOVE_ftrace.o = -pg
CFLAGS_REMOVE_early_printk.o = -pg
+CFLAGS_REMOVE_microcode_core_early.o = -pg
+CFLAGS_REMOVE_microcode_intel_early.o = -pg
+CFLAGS_REMOVE_microcode_amd_early.o = -pg
endif
CFLAGS_irq.o := -I$(src)/../include/asm/trace
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index 96b2c66..0917142 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -2,6 +2,10 @@
# Makefile for x86 specific library files.
#
+ifdef CONFIG_FUNCTION_TRACER
+CFLAGS_REMOVE_string_32.o = -pg
+endif
+
inat_tables_script = $(srctree)/arch/x86/tools/gen-insn-attr-x86.awk
inat_tables_maps = $(srctree)/arch/x86/lib/x86-opcode-map.txt
quiet_cmd_inat_tables = GEN $@
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
On Fri, Aug 23, 2013 at 06:40:46PM +0200, Jan Kiszka wrote:
> Below some hunks to get it working again - at least in the absence of
> any microcode in the initrd. Marking all involved functions as __init
> is another option (as __init implies notrace). But I bet there is more
> hidden. I see e.g. a pr_warn() in find_cpio_init that should trigger
> the issue as well if we hit the error it reports (btw. printing at
> this point of the boot should not work anyway, should it?).
I guess we can do early_printk there instead as x86_64_start_kernel()
does it but I'm not sure for the 32-bit case where we call
load_ucode_bsp/ap before we've even enabled paging.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
On 2013-08-23 22:17, Borislav Petkov wrote:
> On Fri, Aug 23, 2013 at 06:40:46PM +0200, Jan Kiszka wrote:
>> Below some hunks to get it working again - at least in the absence of
>> any microcode in the initrd. Marking all involved functions as __init
>> is another option (as __init implies notrace). But I bet there is more
>> hidden. I see e.g. a pr_warn() in find_cpio_init that should trigger
>> the issue as well if we hit the error it reports (btw. printing at
>> this point of the boot should not work anyway, should it?).
>
> I guess we can do early_printk there instead as x86_64_start_kernel()
> does it but I'm not sure for the 32-bit case where we call
> load_ucode_bsp/ap before we've even enabled paging.
early_printk doesn't seem to work as well on 32-bit until paging was
enabled.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
On 08/23/2013 09:40 AM, Jan Kiszka wrote:
> Hi all,
>
> 32-bit kernels currently crash/reboot in early microcode loading when
> non-dynamic function tracing is enabled. Several functions in that path
> get instrumented with mcount, but its non-dynamic implementation does
> not work before paging is enabled (it accesses global variables at wrong
> addresses).
>
> Below some hunks to get it working again - at least in the absence of
> any microcode in the initrd. Marking all involved functions as __init is
> another option (as __init implies notrace). But I bet there is more
> hidden. I see e.g. a pr_warn() in find_cpio_init that should trigger the
> issue as well if we hit the error it reports (btw. printing at this
> point of the boot should not work anyway, should it?).
>
> Better ideas?
>
> Jan
>
A better idea would be for the mcount/__fentry__ function to simply
return until the function tracing stuff is actually ready.
-hpa
On Fri, 30 Aug 2013 08:36:58 -0700
"H. Peter Anvin" <[email protected]> wrote:
> On 08/23/2013 09:40 AM, Jan Kiszka wrote:
> > Hi all,
> >
> > 32-bit kernels currently crash/reboot in early microcode loading when
> > non-dynamic function tracing is enabled. Several functions in that path
> > get instrumented with mcount, but its non-dynamic implementation does
> > not work before paging is enabled (it accesses global variables at wrong
> > addresses).
> >
> > Below some hunks to get it working again - at least in the absence of
> > any microcode in the initrd. Marking all involved functions as __init is
> > another option (as __init implies notrace). But I bet there is more
> > hidden. I see e.g. a pr_warn() in find_cpio_init that should trigger the
> > issue as well if we hit the error it reports (btw. printing at this
> > point of the boot should not work anyway, should it?).
> >
> > Better ideas?
> >
> > Jan
> >
>
> A better idea would be for the mcount/__fentry__ function to simply
> return until the function tracing stuff is actually ready.
>
It does when dynamic tracing is enabled. But this issue is with static
tracing, there's no code modification, thus all functions call
mcount/__fentry__ and we need to look at a variable to determine if we
should trace or not.
-- Steve
On 08/30/2013 08:51 AM, Steven Rostedt wrote:
>
> It does when dynamic tracing is enabled. But this issue is with static
> tracing, there's no code modification, thus all functions call
> mcount/__fentry__ and we need to look at a variable to determine if we
> should trace or not.
>
And of course global variables are a no-no without special handling in
pre-paging mode. Sigh.
You *could* bail out of the calling address is < PAGE_OFFSET.
-hpa
On Fri, 30 Aug 2013 09:35:18 -0700
"H. Peter Anvin" <[email protected]> wrote:
> On 08/30/2013 08:51 AM, Steven Rostedt wrote:
> >
> > It does when dynamic tracing is enabled. But this issue is with static
> > tracing, there's no code modification, thus all functions call
> > mcount/__fentry__ and we need to look at a variable to determine if we
> > should trace or not.
> >
>
> And of course global variables are a no-no without special handling in
> pre-paging mode. Sigh.
>
> You *could* bail out of the calling address is < PAGE_OFFSET.
>
We could do that too, as static tracing already gives us high overhead,
I'm not sure how much more overhead another check like this even
matters.
Who uses static tracing? I only use it to test that it still works ;-)
-- Steve
On 08/30/2013 10:03 AM, Steven Rostedt wrote:
> On Fri, 30 Aug 2013 09:35:18 -0700
> "H. Peter Anvin" <[email protected]> wrote:
>
>> On 08/30/2013 08:51 AM, Steven Rostedt wrote:
>>>
>>> It does when dynamic tracing is enabled. But this issue is with static
>>> tracing, there's no code modification, thus all functions call
>>> mcount/__fentry__ and we need to look at a variable to determine if we
>>> should trace or not.
>>>
>>
>> And of course global variables are a no-no without special handling in
>> pre-paging mode. Sigh.
>>
>> You *could* bail out of the calling address is < PAGE_OFFSET.
>>
>
> We could do that too, as static tracing already gives us high overhead,
> I'm not sure how much more overhead another check like this even
> matters.
It's not too expensive: a compare and a jump.
> Who uses static tracing? I only use it to test that it still works ;-)
Can we get rid of it?
On Fri, 30 Aug 2013 10:16:00 -0700
"H. Peter Anvin" <[email protected]> wrote:
> > Who uses static tracing? I only use it to test that it still works ;-)
>
> Can we get rid of it?
>
I've though about it. But as some archs only have static tracing
available (I need to revisit to make sure that's still the case), I
kept it because I only test static tracing on x86.
I'm not sure if people still use it for x86 or not.
Jan, you use static tracing on x86?
-- Steve
On 2013-08-30 19:25, Steven Rostedt wrote:
> On Fri, 30 Aug 2013 10:16:00 -0700
> "H. Peter Anvin" <[email protected]> wrote:
>
>>> Who uses static tracing? I only use it to test that it still works ;-)
>>
>> Can we get rid of it?
>>
>
> I've though about it. But as some archs only have static tracing
> available (I need to revisit to make sure that's still the case), I
> kept it because I only test static tracing on x86.
>
> I'm not sure if people still use it for x86 or not.
>
> Jan, you use static tracing on x86?
Yes, due to out-of-tree code that is not compatible with dynamic
tracing. It might be feasible to change this, I didn't try again
recently. If static tracing goes away upstream, I'll have to sooner or
later.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux
On 08/30/2013 10:03 AM, Steven Rostedt wrote:
> On Fri, 30 Aug 2013 09:35:18 -0700
> "H. Peter Anvin" <[email protected]> wrote:
>
>> On 08/30/2013 08:51 AM, Steven Rostedt wrote:
>>>
>>> It does when dynamic tracing is enabled. But this issue is with static
>>> tracing, there's no code modification, thus all functions call
>>> mcount/__fentry__ and we need to look at a variable to determine if we
>>> should trace or not.
>>>
>>
>> And of course global variables are a no-no without special handling in
>> pre-paging mode. Sigh.
>>
>> You *could* bail out of the calling address is < PAGE_OFFSET.
>>
>
> We could do that too, as static tracing already gives us high overhead,
> I'm not sure how much more overhead another check like this even
> matters.
>
> Who uses static tracing? I only use it to test that it still works ;-)
>
Anyway... can we add this to the static tracing code? It is two
instructions and is only needed on x86-32. For performance, use dynamic.
-hpa
On Fri, 30 Aug 2013 13:48:55 -0700
"H. Peter Anvin" <[email protected]> wrote:
> >> You *could* bail out of the calling address is < PAGE_OFFSET.
>
> Anyway... can we add this to the static tracing code? It is two
> instructions and is only needed on x86-32. For performance, use dynamic.
Sure, do you want to add it ?
-- Steve
On Fri, Aug 30, 2013 at 08:17:41PM -0400, Steven Rostedt wrote:
> On Fri, 30 Aug 2013 13:48:55 -0700
> "H. Peter Anvin" <[email protected]> wrote:
>
> > >> You *could* bail out of the calling address is < PAGE_OFFSET.
> >
> > Anyway... can we add this to the static tracing code? It is two
> > instructions and is only needed on x86-32. For performance, use dynamic.
>
> Sure, do you want to add it ?
Another, simpler option could be to make static tracing depend on
!MICROCODE until the first has been removed.
--
Regards/Gruss,
Boris.
On Sat, 31 Aug 2013 07:07:37 +0200
Borislav Petkov <[email protected]> wrote:
> On Fri, Aug 30, 2013 at 08:17:41PM -0400, Steven Rostedt wrote:
> > On Fri, 30 Aug 2013 13:48:55 -0700
> > "H. Peter Anvin" <[email protected]> wrote:
> >
> > > >> You *could* bail out of the calling address is < PAGE_OFFSET.
> > >
> > > Anyway... can we add this to the static tracing code? It is two
> > > instructions and is only needed on x86-32. For performance, use dynamic.
> >
> > Sure, do you want to add it ?
>
> Another, simpler option could be to make static tracing depend on
> !MICROCODE until the first has been removed.
>
Might be trickier than you think. Static tracing is the default.
Dynamic tracing is the extension. By making static tracing depend
on !MICROCODE, you basically made function tracing depend on !MICROCODE.
I guess you could make MICROCODE select DYNAMIC_FTRACE, but that could
get messy.
-- Steve