Hi,
We have seen the following kernel panic, happened during loading a kernel module:
[ 536.107430] Unable to handle kernel paging request for data at address 0xd76a907c
[ 536.114922] Faulting instruction address: 0xc0000770
[ 536.119891] Oops: Kernel access of bad area, sig: 11 [#1]
[ 536.125291] CCEP MPC8541E
[ 536.127908] Modules linked in: pppoe(+) nf_conntrack_ipv6 ...
[ 536.155705] NIP: c0000770 LR: c0000770 CTR: d76ab0d4
[ 536.160674] REGS: d76a8f24 TRAP: 0300 Not tainted (2.6.33-ccep)
[ 536.166857] MSR: 00021000 <ME,CE> CR: 24000482 XER: 20000000
[ 536.172718] DEAR: d76a907c, ESR: 00800000
[ 536.176728] TASK = cbd7f9f0[972] 'insmod' THREAD: cbeb2000
[ 536.182041] GPR00: 83cbfff8 d76a8fd4 cbd7f9f0 00000000 83cbfff8 00000000 cbeb3e1e 00000000
[ 536.190438] GPR08: 0000327b d76aa000 24000482 d76a8fd4 cbd7fc08 10019b04 100c2e5c 00000000
[ 536.198836] GPR16: 00000000 1009bafc 100c44a4 100c2ed4 100c0000 100d1e60 100d1ca8 100017ab
[ 536.207235] GPR24: 100017ae 10001936 c0343ae8 10012018 00000000 834bffe8 836bffec 838bfff0
[ 536.215819] NIP [c0000770] InstructionStorage+0xb0/0xc0
[ 536.221048] LR [c0000770] InstructionStorage+0xb0/0xc0
[ 536.226188] Call Trace:
[ 536.228630] Instruction dump:
[ 536.231600] 90eb002c 910b0030 7cbe0aa6 90ab00b8 7d846378 38a00000 39400401 914b00b0
[ 536.239386] 3d400002 614a1002 512a0420 4800d6f5 <c000e43c> c000e65c 60000000 60000000
[ 536.247348] Kernel panic - not syncing: Fatal exception in interrupt
[ 536.253704] Call Trace:
[ 536.256149] Rebooting in 10 seconds..
The system crashes inside the return of the init entry point of the kernel module.
I've found the following root cause:
(1) The system has a high number of NAT rules configured, which created a bigger vmalloc area.
I've checked this by looking at /proc/vmallocinfo.
(2) The kernel module ELF file contains the separate section .init.text for the init entry point,
which is marked with __init, as usual.
(3) The kernel module ELF file contains the function prologue and epilogue in the .text section.
(4) The epilogue is also called from the init entry point, in order to return to the caller.
It is intended to restore the non-volatile registers from the stack and to jump to the caller.
(5) Because of (1), it is not more possible to jump by a relative branch instruction. The distance is too big.
Instead, the trampoline method is applied, which allows longer jumps via register.
(please see see do_plt_call() in arch/powerpc/kernel/module_32.c)
(6) Unfortunately, the trampoline code (do_plt_call()) is using register r11 to setup the jump.
It looks like the prologue and epilogue are using also the register r11, in order to point to the previous stack frame.
This is a conflict !!! The trampoline code is damaging the content of r11.
According to the current EABI definitions, the register r11 has got a dedicated function (pointer to previous stack frame).
In the following, there are parts of the prologue/epilogue shown, which are generated by the compiler:
...
00000084 <_rest32gpr_28>:
84: 83 8b ff f0 lwz r28,-16(r11)
00000088 <_rest32gpr_29>:
88: 83 ab ff f4 lwz r29,-12(r11)
0000008c <_rest32gpr_30>:
8c: 83 cb ff f8 lwz r30,-8(r11)
00000090 <_rest32gpr_31>:
90: 83 eb ff fc lwz r31,-4(r11)
94: 4e 80 00 20 blr
00000098 <_rest32gpr_14_x>:
98: 81 cb ff b8 lwz r14,-72(r11)
...
I'd suggest to use register r12 instead of r11 in the trampoline generation code, in do_plt_call() (arch/powerpc/kernel/module_32.c).
I'm using kernel 2.6.33, but I think it is also relevant for the current kernel release.
Below, there is the complete debug sessions, showing more the details.
Thanks
Steffen Rumler
--
0xd54990c0: addi r11,r1,48
0xd54990c4: mr r3,r29
0xd54990c8: b 0xd5499100 <-- going to return from the init entry point
(gdb) bt
#0 0xd5499100 in ?? ()
#1 0xd54990ac in ?? ()
#2 0xc0001db0 in do_one_initcall (fn=0, wait=1131130) at init/main.c:719
#3 0xc0059e50 in sys_init_module (umod=<value optimized out>, ...
#4 0xc000e038 in syscall_dotrace_cont () at arch/powerpc/kernel/entry_32.S:331
Backtrace stopped: frame did not save the PC
(gdb) info reg r1
r1 0xcbbdbed0 3418210000
(gdb) x/2x 0xcbbdbed0
0xcbbdbed0: 0xcbbdbf00 0xd54990ac
(gdb) x/2x 0xcbbdbf00
0xcbbdbf00: 0xcbbdbf20 0xc0001db0
(gdb) info reg r11
r11 0xcbbdbf00 3418210048
--> the stack is OK here
--> r11 is OK, pointing to the previous stack frame
0xd5499100: lis r11,-10381 <-- this is the trampoline code using/changing r11 (do_plt_call()).
0xd5499104: addi r11,r11,-3888
0xd5499108: mtctr r11
(gdb) info reg r11
r11 0xd772f0d0 3614634192
<-- r11 is now damaged !!!
0xd549910c: bctr
0xd772f0d0: lwz r28,-16(r11) <-- the epilogue, using damaged r11 as stack pointer
0xd772f0d4: lwz r29,-12(r11)
0xd772f0d8: lwz r30,-8(r11)
0xd772f0dc: lwz r0,4(r11)
0xd772f0e0: lwz r31,-4(r11)
0xd772f0e4: mtlr r0
0xd772f0e8: mr r1,r11
(gdb) info reg lr
lr 0x83abfff4 0x83abfff4
0xd772f0ec: blr
(gdb) stepi
Program received signal SIGSTOP, Stopped (signal).
---> the kernel panic !!!
From: Michael Ellerman <hidden> Date: 2012-05-30 23:24:02
On Wed, 2012-05-30 at 16:33 +0200, Steffen Rumler wrote:
Hi,
The system crashes inside the return of the init entry point of the kernel module.
I've found the following root cause:
(6) Unfortunately, the trampoline code (do_plt_call()) is using register r11 to setup the jump.
It looks like the prologue and epilogue are using also the register r11, in order to point to the previous stack frame.
This is a conflict !!! The trampoline code is damaging the content of r11.
Hi Steffen,
Great bug report!
I can't quite work out what the standards say, the versions I'm looking
at are probably old anyway.
Have you tried the obvious fix?
cheers
On Wed, 2012-05-30 at 16:33 +0200, Steffen Rumler wrote:
quoted
I've found the following root cause:
(6) Unfortunately, the trampoline code (do_plt_call()) is using
register r11 to setup the jump.
quoted
It looks like the prologue and epilogue are using also the
register r11, in order to point to the previous stack frame.
quoted
This is a conflict !!! The trampoline code is damaging the
content of r11.
=20
Hi Steffen,
=20
Great bug report!
=20
I can't quite work out what the standards say, the versions I'm looking
at are probably old anyway.
The ABI supplement from https://www.power.org/resources/downloads/ is expli=
cit about r11 being a requirement for the statically lined save/restore fun=
ctions in section 3.3.4 on page 59.
This means that any trampoline code must not ever use r11 or it can't be us=
ed to get to such save/restore functions safely from far away.
Unfortunately the same doc and predecessors show r11 in all basic examples =
for PLT/trampoline code AFAICS, which is likely why all trampoline code use=
s r11 in any known case.
I would guess that it was never envisioned that compiler generated code wou=
ld be in a different section than save/restore functions, i.e., the Linux m=
odule "__init" assumptions for Power break the ABI. Or does the ABI break t=
he __init concept?!
Using r12 in the trampoline seems to be the obvious solution for module loa=
ding.
But what about other code loading done? If, e.g., a user runs any app from =
bash it gets loaded and relocated and trampolines might get set up somehow.
Wouldn't we have to find fix ANY trampoline code generator remotely related=
to a basic Power Architecture Linux?
Or is it a basic assumption for anything but modules that compiler generate=
d code may never ever be outside the .text section? I am not sure that woul=
d be a safe assumption.
Isn't this problem going beyond just module loading for Power Architecture =
Linux?
Regards,
Heinz
From: Gabriel Paubert <hidden> Date: 2012-05-31 11:05:00
On Thu, May 31, 2012 at 07:04:42AM +0000, Wrobel Heinz-R39252 wrote:
Michael,
quoted
On Wed, 2012-05-30 at 16:33 +0200, Steffen Rumler wrote:
quoted
I've found the following root cause:
(6) Unfortunately, the trampoline code (do_plt_call()) is using
register r11 to setup the jump.
quoted
It looks like the prologue and epilogue are using also the
register r11, in order to point to the previous stack frame.
quoted
This is a conflict !!! The trampoline code is damaging the
content of r11.
Hi Steffen,
Great bug report!
I can't quite work out what the standards say, the versions I'm looking
at are probably old anyway.
The ABI supplement from https://www.power.org/resources/downloads/ is explicit about r11 being a requirement for the statically lined save/restore functions in section 3.3.4 on page 59.
This means that any trampoline code must not ever use r11 or it can't be used to get to such save/restore functions safely from far away.
I believe that the basic premise is that you should provide a directly reachable copy
of the save/rstore functions, even if this means that you need several copies of the functions.
Unfortunately the same doc and predecessors show r11 in all basic examples for PLT/trampoline code AFAICS, which is likely why all trampoline code uses r11 in any known case.
I would guess that it was never envisioned that compiler generated code would be in a different section than save/restore functions, i.e., the Linux module "__init" assumptions for Power break the ABI. Or does the ABI break the __init concept?!
Using r12 in the trampoline seems to be the obvious solution for module loading.
But what about other code loading done? If, e.g., a user runs any app from bash it gets loaded and relocated and trampolines might get set up somehow.
I don't think so. The linker/whatever should generate a copy of the save/restore functions for every
executable code area (shared library), and probably more than one copy if the text becomes too large.
For 64 bit code, these functions are actually inserted by the linker.
[Ok, I just recompiled my 64 bit kernel with -Os and I see that vmlinux gets one copy
of the save/restore functions and every module also gets its copy.]
This makes sense, really these functions are there for a compromise between locality
and performance, there should be one per code section, otherwise the cache line
used by the trampoline negates a large part of their advantage.
Wouldn't we have to find fix ANY trampoline code generator remotely related to a basic Power Architecture Linux?
Or is it a basic assumption for anything but modules that compiler generated code may never ever be outside the .text section? I am not sure that would be a safe assumption.
Isn't this problem going beyond just module loading for Power Architecture Linux?
I don't think so. It really seems to be a 32 bit kernel problem.
Regards,
Gabriel
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2012-06-01 09:19:07
On Thu, 2012-05-31 at 13:04 +0200, Gabriel Paubert wrote:
I believe that the basic premise is that you should provide a directly reachable copy
of the save/rstore functions, even if this means that you need several copies of the functions.
I just fixed a very similar problem with grub2 in fact. It was using r0
and trashing the saved LR that way.
The real fix is indeed to statically link those gcc "helpers", we
shouldn't generate things like cross-module calls inside function
prologs and epilogues, when stackframes aren't even guaranteed to be
reliable.
However, in the grub2 case, it was easier to just use r12 :-)
Cheers,
Ben.
quoted
Unfortunately the same doc and predecessors show r11 in all basic examples for PLT/trampoline code AFAICS, which is likely why all trampoline code uses r11 in any known case.
I would guess that it was never envisioned that compiler generated code would be in a different section than save/restore functions, i.e., the Linux module "__init" assumptions for Power break the ABI. Or does the ABI break the __init concept?!
Using r12 in the trampoline seems to be the obvious solution for module loading.
But what about other code loading done? If, e.g., a user runs any app from bash it gets loaded and relocated and trampolines might get set up somehow.
I don't think so. The linker/whatever should generate a copy of the save/restore functions for every
executable code area (shared library), and probably more than one copy if the text becomes too large.
For 64 bit code, these functions are actually inserted by the linker.
[Ok, I just recompiled my 64 bit kernel with -Os and I see that vmlinux gets one copy
of the save/restore functions and every module also gets its copy.]
This makes sense, really these functions are there for a compromise between locality
and performance, there should be one per code section, otherwise the cache line
used by the trampoline negates a large part of their advantage.
quoted
Wouldn't we have to find fix ANY trampoline code generator remotely related to a basic Power Architecture Linux?
Or is it a basic assumption for anything but modules that compiler generated code may never ever be outside the .text section? I am not sure that would be a safe assumption.
Isn't this problem going beyond just module loading for Power Architecture Linux?
I don't think so. It really seems to be a 32 bit kernel problem.
Regards,
Gabriel
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
I believe that the basic premise is that you should provide a directly
reachable copy of the save/rstore functions, even if this means that
you need several copies of the functions.
I just fixed a very similar problem with grub2 in fact. It was using r0
and trashing the saved LR that way.
The real fix is indeed to statically link those gcc "helpers", we
shouldn't generate things like cross-module calls inside function prologs
and epilogues, when stackframes aren't even guaranteed to be reliable.
However, in the grub2 case, it was easier to just use r12 :-)
For not just the module loading case, I believe r12 is the only real solution now. I checked one debugger capable of doing ELF download. It also uses r12 for trampoline code. I am guessing for the reason that prompted this discussion.
Without r12 we'd have to change standard libraries to automagically link in gcc helpers for any conceivable non-.text section, which I am not sure is feasible. How would you write section independent helper functions which link to any section needing them?!
Asking users to create their own section specific copy of helper functions is definitely not portable if the module or other code is not architecture dependent.
It is a normal gcc feature that you can assign specific code to non-.text sections and it is not documented that it may crash depending on the OS arch the ELF is built for, so asking for a Power Architecture specific change on tool libs to make Power Architecture Linux happy seems a bit much to ask.
Using r12 in any Linux related trampoline code seems a reachable goal, and it would eliminate the conflict to the ABI.
Regards,
Heinz
Hi,
I've tried the fix using register r12 for the trampoline code, instead of register r11.
I'd to adapt entry_matches() too:
@@ -215,10 +215,9 @@entry++;}-/* Stolen from Paul Mackerras as well... */-entry->jump[0]=0x3d600000+((val+0x8000)>>16);/* lis r11,sym@ha */-entry->jump[1]=0x396b0000+(val&0xffff);/* addi r11,r11,sym@l*/-entry->jump[2]=0x7d6903a6;/* mtctr r11 */+entry->jump[0]=0x3d800000+((val+0x8000)>>16);/* lis r12,sym@ha */+entry->jump[1]=0x398c0000+(val&0xffff);/* addi r12,r12,sym@l*/+entry->jump[2]=0x7d8903a6;/* mtctr r12 */entry->jump[3]=0x4e800420;/* bctr */DEBUGP("Initialized plt for 0x%x at %p\n",val,entry);
It looks working, also in the case the trampoline code is used.
Please see also the debug session below.
Let me summarize the situation:
+ According to Freescale, EABI assigns a dedicated function to register r11.
+ The ELF sections .text and .init.text may trigger the usage of the trampoline code.
+ The trampoline code must not user register r11, too.
+ We must not depend on addresses returned by vmalloc during kernel module loading.
I think the bug must be fixed !!!
Regards
Steffen
--
(gdb) bt
#0 0xd54990f0 in ?? ()
#1 0xd5499088 in ?? ()
#2 0xc0001d9c in do_one_initcall (fn=0xd76e26ac) at init/main.c:716
#3 0xc0059630 in sys_init_module (umod=0x4802f008, len=<value optimized out>, uargs=0x10012008 "") at kernel/module.c:2470
#4 0xc000dff8 in syscall_dotrace_cont () at arch/powerpc/kernel/entry_32.S:331
Backtrace stopped: frame did not save the PC
<-- we are returning from the driver init entry point
(gdb) info reg r11
r11 0xcbb57f00 3417669376
(gdb) x/2x 0xcbb57f00
0xcbb57f00: 0xcbb57f20 0xc0001d9c
<-- register r11 is fine here
0xd54990f0: lis r12,-10386
0xd54990f4: addi r12,r12,-20268
0xd54990f8: mtctr r12
<-- this is the trampoline code using now r12, instead of r11
(gdb) info reg r12
r12 0xd76db0d4 3614290132
(gdb) info reg ctr
ctr 0xd76db0d4 3614290132
0xd54990fc: bctr
<-- we are jumping to the epilogue
0xd76db0d4: lwz r29,-12(r11)
0xd76db0d8: lwz r30,-8(r11)
0xd76db0dc: lwz r0,4(r11)
0xd76db0e0: lwz r31,-4(r11)
0xd76db0e4: mtlr r0
0xd76db0e8: mr r1,r11
<- the epilogue
(gdb) info reg lr
lr 0xc0001d9c 0xc0001d9c <do_one_initcall+100>
0xc0001d9c <do_one_initcall+100>: lis r9,-16330
0xc0001da0 <do_one_initcall+104>: lwz r0,12296(r9)
0xc0001da4 <do_one_initcall+108>: lis r9,-16330
0xd76db0ec: blr
<-- the jump back to do_one_initcall()
<-- no crash
From: Paul Mackerras <hidden> Date: 2012-06-04 10:53:55
On Mon, Jun 04, 2012 at 09:43:01AM +0200, Steffen Rumler wrote:
Let me summarize the situation:
+ According to Freescale, EABI assigns a dedicated function to register r11.
+ The ELF sections .text and .init.text may trigger the usage of the trampoline code.
+ The trampoline code must not user register r11, too.
+ We must not depend on addresses returned by vmalloc during kernel module loading.
I think the bug must be fixed !!!
I agree. If you submit your patch with a nice description of the
problem and how the patch fixes it, along with your Signed-off-by,
I'll send it upstream.
Paul.
From: Gabriel Paubert <hidden> Date: 2012-06-04 11:28:50
On Fri, Jun 01, 2012 at 11:33:37AM +0000, Wrobel Heinz-R39252 wrote:
quoted
quoted
I believe that the basic premise is that you should provide a directly
reachable copy of the save/rstore functions, even if this means that
you need several copies of the functions.
I just fixed a very similar problem with grub2 in fact. It was using r0
and trashing the saved LR that way.
The real fix is indeed to statically link those gcc "helpers", we
shouldn't generate things like cross-module calls inside function prologs
and epilogues, when stackframes aren't even guaranteed to be reliable.
However, in the grub2 case, it was easier to just use r12 :-)
For not just the module loading case, I believe r12 is the only real solution now. I checked one debugger capable of doing ELF download. It also uses r12 for trampoline code. I am guessing for the reason that prompted this discussion.
I disagree. Look carefully at Be's answer: cross-module calls
are intrinsically dangerous when stack frames are in a transient
state.
Without r12 we'd have to change standard libraries to automagically link in gcc helpers for any conceivable non-.text section, which I am not sure is feasible. How would you write section independent helper functions which link to any section needing them?!
I don't thnk that it is tha bad: the helpers should be linked to the default .text section
when needed, typically the init code and so on are mapped within the reach of that
section (otherwise you'll end up with the linker complaining that it finds overflowing
branch offsets between .text and .init.text).
Asking users to create their own section specific copy of helper functions is definitely not portable if the module or other code is not architecture dependent.
Well, it automagically works on 64 bit. There is is performed by magic built into the linker.
It is a normal gcc feature that you can assign specific code to non-.text sections and it is not documented that it may crash depending on the OS arch the ELF is built for, so asking for a Power Architecture specific change on tool libs to make Power Architecture Linux happy seems a bit much to ask.
Once again I disagree.
Using r12 in any Linux related trampoline code seems a reachable goal, and it would eliminate the conflict to the ABI.
There is no conflict to the ABI. These functions are supposed to be directly reachable from whatever code
section may need them.
Now I have a question: how did you get the need for this?
None of my kernels uses them:
- if I compile with -O2, the compiler simply expands epilogue and prologue to series of lwz and stw
- if I compile with -Os, the compiler generates lmw/stmw which give the smallest possible cache footprint
Neither did I find a single reference to these functions in several systems that I grepped for.
Regards,
Gabriel
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2012-06-04 22:01:23
On Mon, 2012-06-04 at 13:03 +0200, Gabriel Paubert wrote:
There is no conflict to the ABI. These functions are supposed to be
directly reachable from whatever code
section may need them.
Now I have a question: how did you get the need for this?
None of my kernels uses them:
- if I compile with -O2, the compiler simply expands epilogue and
prologue to series of lwz and stw
- if I compile with -Os, the compiler generates lmw/stmw which give
the smallest possible cache footprint
Neither did I find a single reference to these functions in several
systems that I grepped for.
Newer gcc's ... even worse, with -Os, it does it for a single register
spill afaik. At least that's how I hit it with grub2 using whatever gcc
is in fc17.
Cheers,
Ben.
From: Gabriel Paubert <hidden> Date: 2012-06-05 10:45:18
On Tue, Jun 05, 2012 at 08:00:42AM +1000, Benjamin Herrenschmidt wrote:
On Mon, 2012-06-04 at 13:03 +0200, Gabriel Paubert wrote:
quoted
There is no conflict to the ABI. These functions are supposed to be
directly reachable from whatever code
section may need them.
Now I have a question: how did you get the need for this?
None of my kernels uses them:
- if I compile with -O2, the compiler simply expands epilogue and
prologue to series of lwz and stw
- if I compile with -Os, the compiler generates lmw/stmw which give
the smallest possible cache footprint
Neither did I find a single reference to these functions in several
systems that I grepped for.
Newer gcc's ... even worse, with -Os, it does it for a single register
spill afaik. At least that's how I hit it with grub2 using whatever gcc
is in fc17.
Ok, it's beyond stupid, and at this point must be considered a gcc bug.
Gabriel
From: Gabriel Paubert <hidden> Date: 2012-06-05 11:32:39
On Tue, Jun 05, 2012 at 08:00:42AM +1000, Benjamin Herrenschmidt wrote:
On Mon, 2012-06-04 at 13:03 +0200, Gabriel Paubert wrote:
quoted
There is no conflict to the ABI. These functions are supposed to be
directly reachable from whatever code
section may need them.
Now I have a question: how did you get the need for this?
None of my kernels uses them:
- if I compile with -O2, the compiler simply expands epilogue and
prologue to series of lwz and stw
- if I compile with -Os, the compiler generates lmw/stmw which give
the smallest possible cache footprint
Neither did I find a single reference to these functions in several
systems that I grepped for.
Newer gcc's ... even worse, with -Os, it does it for a single register
spill afaik. At least that's how I hit it with grub2 using whatever gcc
is in fc17.
Well, I've not yet been able to make it call the save/restore routines,
but here on a machine with Debian testing and several gcc installed:
- gcc-4.4 never generates lmw/stmw, it always uses individual
lwz/stw whatever options are set (-Os -mmultiple).
- gcc-4.6 and gcc-4.7 behave identically, if -Os is set, they
generate by default lmw/stmw. But if I combine -Os with
-mno-multiple, they call the helper functions.
In other words, on this system, gcc-4.4 is broken but should not
cause any harm. gcc-4.6 and gcc-4.7 look correct, but are there
any processors on which -mno-multiple is worth setting?
Regards,
Gabriel
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2012-06-05 22:15:24
On Tue, 2012-06-05 at 13:32 +0200, Gabriel Paubert wrote:
- gcc-4.6 and gcc-4.7 behave identically, if -Os is set, they
generate by default lmw/stmw. But if I combine -Os with
-mno-multiple, they call the helper functions.
In other words, on this system, gcc-4.4 is broken but should not
cause any harm. gcc-4.6 and gcc-4.7 look correct, but are there
any processors on which -mno-multiple is worth setting?
It could be an artifact of the -mtune we use, dunno. And yes, I'm pretty
sure some embedded processors don't like multiple load/store.
Cheers,
Ben.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2012-06-05 22:47:20
On Tue, 2012-06-05 at 12:44 +0200, Gabriel Paubert wrote:
On Tue, Jun 05, 2012 at 08:00:42AM +1000, Benjamin Herrenschmidt wrote:
quoted
On Mon, 2012-06-04 at 13:03 +0200, Gabriel Paubert wrote:
quoted
There is no conflict to the ABI. These functions are supposed to be
directly reachable from whatever code
section may need them.
Now I have a question: how did you get the need for this?
None of my kernels uses them:
- if I compile with -O2, the compiler simply expands epilogue and
prologue to series of lwz and stw
- if I compile with -Os, the compiler generates lmw/stmw which give
the smallest possible cache footprint
Neither did I find a single reference to these functions in several
systems that I grepped for.
Newer gcc's ... even worse, with -Os, it does it for a single register
spill afaik. At least that's how I hit it with grub2 using whatever gcc
is in fc17.
Ok, it's beyond stupid, and at this point must be considered a gcc bug.
Probably, I haven't had a chance to report it...
It would make more sense if the out of line functions also handled
creating the stack frame & disposing of it (ie, maybe a tail call for
return) but they don't even do that so yes it's quite gross.
Cheers,
Ben.
On Fri, Jun 01, 2012 at 11:33:37AM +0000, Wrobel Heinz-R39252 wrote:
quoted
quoted
quoted
I believe that the basic premise is that you should provide a directly
reachable copy of the save/rstore functions, even if this means that
you need several copies of the functions.
I just fixed a very similar problem with grub2 in fact. It was using r0
and trashing the saved LR that way.
The real fix is indeed to statically link those gcc "helpers", we
shouldn't generate things like cross-module calls inside function prologs
and epilogues, when stackframes aren't even guaranteed to be reliable.
However, in the grub2 case, it was easier to just use r12 :-)
For not just the module loading case, I believe r12 is the only real solution now. I checked one debugger capable of doing ELF download. It also uses r12 for trampoline code. I am guessing for the reason that prompted this discussion.
I disagree. Look carefully at Be's answer: cross-module calls
are intrinsically dangerous when stack frames are in a transient
state.
quoted
Without r12 we'd have to change standard libraries to automagically link in gcc helpers for any conceivable non-.text section, which I am not sure is feasible. How would you write section independent helper functions which link to any section needing them?!
I don't thnk that it is tha bad: the helpers should be linked to the default .text section
when needed, typically the init code and so on are mapped within the reach of that
section (otherwise you'll end up with the linker complaining that it finds overflowing
branch offsets between .text and .init.text).
quoted
Asking users to create their own section specific copy of helper functions is definitely not portable if the module or other code is not architecture dependent.
Well, it automagically works on 64 bit. There is is performed by magic built into the linker.
quoted
It is a normal gcc feature that you can assign specific code to non-.text sections and it is not documented that it may crash depending on the OS arch the ELF is built for, so asking for a Power Architecture specific change on tool libs to make Power Architecture Linux happy seems a bit much to ask.
Once again I disagree.
quoted
Using r12 in any Linux related trampoline code seems a reachable goal, and it would eliminate the conflict to the ABI.
There is no conflict to the ABI. These functions are supposed to be directly reachable from whatever code
section may need them.
Now I have a question: how did you get the need for this?
None of my kernels uses them:
- if I compile with -O2, the compiler simply expands epilogue and prologue to series of lwz and stw
- if I compile with -Os, the compiler generates lmw/stmw which give the smallest possible cache footprint
Neither did I find a single reference to these functions in several systems that I grepped for.
Regards,
Gabriel
Hi,
how should we continue here ?
There is the kernel panic, I've described.
Technically, there is an conflict between the code generated by the compiler and the loader in module_32.c, at least by using -Os.
Because the prologue/epilogue is part of the .text and init_module() is part of .init.text (in the case __init is applied, as usual),
a directly reachable call is not always possible.
Thanks
Steffen
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2012-06-06 11:32:24
On Wed, 2012-06-06 at 09:36 +0200, Steffen Rumler wrote:
how should we continue here ?
There is the kernel panic, I've described.
Technically, there is an conflict between the code generated by the
compiler and the loader in module_32.c, at least by using -Os.
Because the prologue/epilogue is part of the .text and init_module()
is part of .init.text (in the case __init is applied, as usual),
a directly reachable call is not always possible.
As we discussed earlier, if you could submit a patch to use r12 instead,
we should merge that.
Cheers,
Ben.
Hi,
The patch below is intended to fix the following problem.
According to the PowerPC EABI specification, the GPR r11 is assigned
the dedicated function to point to the previous stack frame.
In the powerpc-specific kernel module loader, do_plt_call()
(in arch/powerpc/kernel/module_32.c), the GPR r11 is also used
to generate trampoline code.
This combination crashes the kernel, in the following case:
+ The compiler has been generated the prologue and epilogue,
which is part of the .text section.
+ The compiler has been generated the code for the module init entry point,
part of the .init.text section (in the case it is marked with __init).
+ By returning from the module init entry point, the epilogue is called by doing
a branch instruction.
+ If the epilogue is too far away, a relative branch instruction cannot be applied.
Instead trampoline code is generated in do_plt_call(), in order to jump via register.
Unfortunately the code generated by do_plt_call() destroys the content of GPR r11.
+ Because GPR r11 does not more keep the right stack frame pointer,
the kernel crashes right after the epilogue.
The fix just uses GPR r12 instead of GPR r11 for generating the trampoline code.
According to the statements from Freescale, this is also save from EABI perspective.
I've tested the fix for kernel 2.6.33 on MPC8541.
Signed-off-by: Steffen Rumler <redacted>
---
From: roger blofeld <hidden> Date: 2012-06-21 15:27:19
--- On Wed, 6/6/12, Steffen Rumler <steffen.rumler.ext@nsn.com> wrote:=0A=
=0A> From: Steffen Rumler [off-list ref]=0A> Subject: [PATCH]=
kernel panic during kernel module load (powerpc specific part)=0A> To: "ex=
t Benjamin Herrenschmidt" [off-list ref], paulus@samba.org=0A> =
Cc: "Wrobel Heinz-R39252" <redacted>, "Michael Ellerman" <micha=
el@ellerman.id.au>, "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozl=
abs.org>=0A> Date: Wednesday, June 6, 2012, 7:37 AM=0A> Hi,=0A> =0A> The pa=
tch below is intended to fix the following problem.=0A> =0A> According to t=
he PowerPC EABI specification, the GPR r11 is=0A> assigned=0A> the dedicate=
d function to point to the previous stack=0A> frame.=0A> In the powerpc-spe=
cific kernel module loader, do_plt_call()=0A> (in arch/powerpc/kernel/modul=
e_32.c), the GPR r11 is also=0A> used=0A> to generate trampoline code.=0A> =
=0A> This combination crashes the kernel, in the following case:=0A> =0A> =
=A0 + The compiler has been generated the prologue and=0A> epilogue,=0A> =
=A0 =A0 which is part of the .text section.=0A> =A0 + The compiler has been=
generated the code for the=0A> module init entry point,=0A> =A0 =A0 part o=
f the .init.text section (in the case it=0A> is marked with __init).=0A> =
=A0 + By returning from the module init entry point, the=0A> epilogue is ca=
lled by doing=0A> =A0 =A0 a branch instruction.=0A> =A0 + If the epilogue i=
s too far away, a relative branch=0A> instruction cannot be applied.=0A> =
=A0 =A0 Instead trampoline code is generated in=0A> do_plt_call(), in order=
to jump via register.=0A> =A0 =A0 Unfortunately the code generated by=0A> =
do_plt_call() destroys the content of GPR r11.=0A> =A0 + Because GPR r11 do=
es not more keep the right stack=0A> frame pointer,=0A> =A0 =A0 the kernel =
crashes right after the epilogue.=0A> =0A> The fix just uses GPR r12 instea=
d of GPR r11 for generating=0A> the trampoline code.=0A> According to the s=
tatements from Freescale, this is also=0A> save from EABI perspective.=0A> =
=0A> I've tested the fix for kernel 2.6.33 on MPC8541.=0A> =0A> Signed-off-=
by: Steffen Rumler [off-list ref]=0A> ---=0A> =0A> --- orig/a=
rch/powerpc/kernel/module_32.c=A0=A0=A0=0A> 2012-06-06 16:04:28.956446788 +=
0200=0A> +++ new/arch/powerpc/kernel/module_32.c=A0=A0=A0=0A> =A0=A0=A0 201=
2-06-06 16:04:17.746290683 +0200=0A> @@ -187,8 +187,8 @@=0A> =0A> static i=
nline int entry_matches(struct ppc_plt_entry=0A> *entry, Elf32_Addr val)=0A=
{=0A> -=A0=A0=A0 if (entry->jump[0] =3D=3D 0x3d600000 +=0A> ((val + 0x80=