Re: [kvm-unit-tests PATCH 03/32] powerpc: Fix stack backtrace termination

["Nicholas Piggin" <npiggin@gmail.com>]

On Fri Mar 1, 2024 at 7:45 PM AEST, Thomas Huth wrote:
> On 27/02/2024 09.50, Thomas Huth wrote:
> > On 26/02/2024 11.11, 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(-)
> > 
> > Thanks for tackling this! ... however, not doing powerpc work since years 
> > anymore, I have some ignorant questions below...
> > 
> >> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> >> index e18ae9a22..14ab0c6c8 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)
> > 
> > 0(r1) is the back chain pointer ...
> > 
> >> +    std    r0,16(r1)
> > 
> > ... but what is 16(r1) ? I suppose that should be the "LR save word" ? But 
> > isn't that at 8(r1) instead?? (not sure whether I'm looking at the right ELF 
> > abi spec right now...)
>
> Ok, I was looking at the wrong ELF spec, indeed (it was an ancient 32-bit 
> spec, not the 64-bit ABI). Sorry for the confusion. Having a proper #define 
> or a comment for the 16 here would still be helpful, though.

Thanks for the deailed reviews as always. I've been a little busy with
QEMU so may not get another series out for a bit. I'll probably wait for
Andrew's stack backtrace changes to land too before resend.

But, yes a comment makes sense. I'll add.

Thanks,
Nick