| Message ID | 20231216134257.1743345-19-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | powerpc: updates, P10, PNV support | expand |
On 16/12/2023 14.42, Nicholas Piggin wrote: > The backtrace handler terminates when it sees a NULL caller address, > but the powerpc stack setup does not keep such a NULL caller frame > at the start of the stack. > > This happens to work on pseries because the memory at 0 is mapped and > it contains 0 at the location of the return address pointer if it > were a stack frame. But this is fragile, and does not work with powernv > where address 0 contains firmware instructions. > > Use the existing dummy frame on stack as the NULL caller, and create a > new frame on stack for the entry code. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > powerpc/cstart64.S | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S > index e18ae9a2..14ab0c6c 100644 > --- a/powerpc/cstart64.S > +++ b/powerpc/cstart64.S > @@ -46,8 +46,16 @@ start: > add r1, r1, r31 > add r2, r2, r31 > > + /* Zero backpointers in initial stack frame so backtrace() stops */ > + li r0,0 > + std r0,0(r1) > + std r0,16(r1) > + > + /* Create entry frame */ > + stdu r1,-INT_FRAME_SIZE(r1) Shouldn't that rather be STACK_FRAME_OVERHEAD instead of INT_FRAME_SIZE... > /* save DTB pointer */ > - std r3, 56(r1) > + SAVE_GPR(3,r1) ... since SAVE_GPR uses STACK_FRAME_OVERHEAD (via GPR0), too? Thomas
On Tue Dec 19, 2023 at 10:22 PM AEST, Thomas Huth wrote: > On 16/12/2023 14.42, Nicholas Piggin wrote: > > The backtrace handler terminates when it sees a NULL caller address, > > but the powerpc stack setup does not keep such a NULL caller frame > > at the start of the stack. > > > > This happens to work on pseries because the memory at 0 is mapped and > > it contains 0 at the location of the return address pointer if it > > were a stack frame. But this is fragile, and does not work with powernv > > where address 0 contains firmware instructions. > > > > Use the existing dummy frame on stack as the NULL caller, and create a > > new frame on stack for the entry code. > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > powerpc/cstart64.S | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S > > index e18ae9a2..14ab0c6c 100644 > > --- a/powerpc/cstart64.S > > +++ b/powerpc/cstart64.S > > @@ -46,8 +46,16 @@ start: > > add r1, r1, r31 > > add r2, r2, r31 > > > > + /* Zero backpointers in initial stack frame so backtrace() stops */ > > + li r0,0 > > + std r0,0(r1) > > + std r0,16(r1) > > + > > + /* Create entry frame */ > > + stdu r1,-INT_FRAME_SIZE(r1) > > Shouldn't that rather be STACK_FRAME_OVERHEAD instead of INT_FRAME_SIZE... > > > /* save DTB pointer */ > > - std r3, 56(r1) > > + SAVE_GPR(3,r1) > > ... since SAVE_GPR uses STACK_FRAME_OVERHEAD (via GPR0), too? No I think it's correct. INT_FRAME_SIZE has STACK_FRAME_OVERHEAD and struct pt_regs. The STACK_FRAME_OVERHEAD in GPR offsets is just to skip that and get to pt_regs.gpr[]. Thanks, Nick
diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S index e18ae9a2..14ab0c6c 100644 --- a/powerpc/cstart64.S +++ b/powerpc/cstart64.S @@ -46,8 +46,16 @@ start: add r1, r1, r31 add r2, r2, r31 + /* Zero backpointers in initial stack frame so backtrace() stops */ + li r0,0 + std r0,0(r1) + std r0,16(r1) + + /* Create entry frame */ + stdu r1,-INT_FRAME_SIZE(r1) + /* save DTB pointer */ - std r3, 56(r1) + SAVE_GPR(3,r1) /* * Call relocate. relocate is C code, but careful to not use @@ -101,7 +109,7 @@ start: stw r4, 0(r3) /* complete setup */ -1: ld r3, 56(r1) +1: REST_GPR(3, r1) bl setup /* run the test */
The backtrace handler terminates when it sees a NULL caller address, but the powerpc stack setup does not keep such a NULL caller frame at the start of the stack. This happens to work on pseries because the memory at 0 is mapped and it contains 0 at the location of the return address pointer if it were a stack frame. But this is fragile, and does not work with powernv where address 0 contains firmware instructions. Use the existing dummy frame on stack as the NULL caller, and create a new frame on stack for the entry code. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- powerpc/cstart64.S | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)