On Fri Mar 1, 2024 at 7:53 PM AEST, Thomas Huth wrote:

On 26/02/2024 11.11, Nicholas Piggin wrote:

quoted

Add support for backtracing across interrupt stacks, and
add interrupt frame backtrace for unhandled interrupts.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
  lib/powerpc/processor.c |  4 ++-
  lib/ppc64/asm/stack.h   |  3 +++
  lib/ppc64/stack.c       | 55 +++++++++++++++++++++++++++++++++++++++++
  powerpc/Makefile.ppc64  |  1 +
  powerpc/cstart64.S      |  7 ++++--
  5 files changed, 67 insertions(+), 3 deletions(-)
  create mode 100644 lib/ppc64/stack.c

diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c
index ad0d95666..114584024 100644
--- a/lib/powerpc/processor.c
+++ b/lib/powerpc/processor.c

@@ -51,7 +51,9 @@ void do_handle_exception(struct pt_regs *regs)
  		return;
  	}
  
-	printf("unhandled cpu exception %#lx at NIA:0x%016lx MSR:0x%016lx\n", regs->trap, regs->nip, regs->msr);
+	printf("Unhandled cpu exception %#lx at NIA:0x%016lx MSR:0x%016lx\n",
+			regs->trap, regs->nip, regs->msr);
+	dump_frame_stack((void *)regs->nip, (void *)regs->gpr[1]);
  	abort();
  }
  

diff --git a/lib/ppc64/asm/stack.h b/lib/ppc64/asm/stack.h
index 9734bbb8f..94fd1021c 100644
--- a/lib/ppc64/asm/stack.h
+++ b/lib/ppc64/asm/stack.h

@@ -5,4 +5,7 @@
  #error Do not directly include <asm/stack.h>. Just use <stack.h>.
  #endif
  
+#define HAVE_ARCH_BACKTRACE
+#define HAVE_ARCH_BACKTRACE_FRAME
+
  #endif

diff --git a/lib/ppc64/stack.c b/lib/ppc64/stack.c
new file mode 100644
index 000000000..fcb7fa860
--- /dev/null
+++ b/lib/ppc64/stack.c

@@ -0,0 +1,55 @@
+#include <libcflat.h>
+#include <asm/ptrace.h>
+#include <stack.h>
+
+extern char exception_stack_marker[];
+
+int backtrace_frame(const void *frame, const void **return_addrs, int max_depth)
+{
+	static int walking;
+	int depth = 0;
+	const unsigned long *bp = (unsigned long *)frame;
+	void *return_addr;
+
+	asm volatile("" ::: "lr"); /* Force it to save LR */
+
+	if (walking) {
+		printf("RECURSIVE STACK WALK!!!\n");
+		return 0;
+	}
+	walking = 1;
+
+	bp = (unsigned long *)bp[0];
+	return_addr = (void *)bp[2];
+
+	for (depth = 0; bp && depth < max_depth; depth++) {
+		return_addrs[depth] = return_addr;
+		if (return_addrs[depth] == 0)
+			break;
+		if (return_addrs[depth] == exception_stack_marker) {
+			struct pt_regs *regs;
+
+			regs = (void *)bp + STACK_FRAME_OVERHEAD;
+			bp = (unsigned long *)bp[0];
+			/* Represent interrupt frame with vector number */
+			return_addr = (void *)regs->trap;
+			if (depth + 1 < max_depth) {
+				depth++;
+				return_addrs[depth] = return_addr;
+				return_addr = (void *)regs->nip;
+			}
+		} else {
+			bp = (unsigned long *)bp[0];
+			return_addr = (void *)bp[2];
+		}
+	}
+
+	walking = 0;
+	return depth;
+}
+
+int backtrace(const void **return_addrs, int max_depth)
+{
+	return backtrace_frame(__builtin_frame_address(0), return_addrs,
+			       max_depth);
+}

diff --git a/powerpc/Makefile.ppc64 b/powerpc/Makefile.ppc64
index b0ed2b104..eb682c226 100644
--- a/powerpc/Makefile.ppc64
+++ b/powerpc/Makefile.ppc64

@@ -17,6 +17,7 @@ cstart.o = $(TEST_DIR)/cstart64.o
  reloc.o  = $(TEST_DIR)/reloc64.o
  
  OBJDIRS += lib/ppc64
+cflatobjs += lib/ppc64/stack.o
  
  # ppc64 specific tests
  tests = $(TEST_DIR)/spapr_vpa.elf

diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index 14ab0c6c8..278af84a6 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S

@@ -188,6 +188,7 @@ call_handler:
  	.endr
  	mfsprg1	r0
  	std	r0,GPR1(r1)
+	std	r0,0(r1)
  
  	/* lr, xer, ccr */
  

@@ -206,12 +207,12 @@ call_handler:
  	subi	r31, r31, 0b - start_text
  	ld	r2, (p_toc_text - start_text)(r31)
  
-	/* FIXME: build stack frame */
-
  	/* call generic handler */
  
  	addi	r3,r1,STACK_FRAME_OVERHEAD
  	bl	do_handle_exception
+	.global exception_stack_marker
+exception_stack_marker:
  
  	/* restore context */
  

@@ -321,6 +322,7 @@ handler_trampoline:
  	/* nip and msr */
  	mfsrr0	r0
  	std	r0, _NIP(r1)
+	std	r0, INT_FRAME_SIZE+16(r1)

So if I got that right, INT_FRAME_SIZE+16 points to the stack frame of the 
function that was running before the interrupt handler? Is it such a good 
idea to change that here?

No, we switch to exception stack and don't support recursing interrupts
on stack at the moment, so this goes into the initial frame.

Please add a comment here explaining this line. 
Thanks!

Yes, good idea.

Thanks,
Nick

quoted

  	mfsrr1	r0
  	std	r0, _MSR(r1)

@@ -337,6 +339,7 @@ handler_htrampoline:
  	/* nip and msr */
  	mfspr	r0, SPR_HSRR0
  	std	r0, _NIP(r1)
+	std	r0, INT_FRAME_SIZE+16(r1)

dito.

quoted

  	mfspr	r0, SPR_HSRR1
  	std	r0, _MSR(r1)

  Thomas