Thread (71 messages) 71 messages, 4 authors, 2024-03-05

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

From: Thomas Huth <hidden>
Date: 2024-02-27 08:51:14
Also in: kvm

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...
quoted hunk ↗ jump to hunk
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...)

Anyway, a comment in the source would be helpful here.
+
+	/* Create entry frame */
+	stdu	r1,-INT_FRAME_SIZE(r1)
Since we already create an initial frame via stackptr from powerpc/flat.lds, 
do we really need to create this additional one here? Or does the one from 
flat.lds have to be completely empty, i.e. also no DTB pointer in it?
  	/* save DTB pointer */
-	std	r3, 56(r1)
+	SAVE_GPR(3,r1)
Isn't SAVE_GPR rather meant for the interrupt frame, not for the normal C 
calling convention frames?

Sorry for asking dumb questions ... I still have a hard time understanding 
the changes here... :-/
quoted hunk ↗ jump to hunk
  	/*
  	 * 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 */
  Thomas
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help