Subject: Re: [kvm-unit-tests PATCH v5 18/29] powerpc: Fix stack backtrace termination

From: "Nicholas Piggin" <[email-masked]>
Date: Fri, 22 Dec 2023 19:55:53 +1000

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 <[email-masked]>
> > ---
> >   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