system_safe_mode: Print process stack dump

Add the CONFIG_SYSTEM_SAFE_MODE_PRINT_STACK option for
printing the faulting process stack to the console buffer.
This must be done in safe mode because the console buffer
ignores log messages while in an ISR.

This option is on by default when system safe mode is enabled.

get_panic_stack_pointer is a new overidable function that each
architecture needs to implement to support this feature. The default
implementation returns 0, which will result in no stack being printed.

This is a workaround until coredumps are fully supported.

BUG=b:266084064
BRANCH=None
TEST=System safe mode zephyr test passes
Stack print observed on boten, guybrush, and skyrim boards.
LOW_COVERAGE_REASON=Cortex-M and NDS32 specific code cannot be tested

Orig-Change-Id: Ied78ab7e6edca9cfa97c50323d94e39a3fca0eef
Orig-Signed-off-by: Rob Barnes <robbarnes@google.com>
Orig-Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4301695
Orig-Reviewed-by: Boris Mittelberg <bmbm@google.com>
Change-Id: Ic517bef23ebf6c18f9a0a5f4e3ce144236e3019f
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4455295
Reviewed-by: Boris Mittelberg <bmbm@google.com>
Commit-Queue: Rob Barnes <robbarnes@google.com>
Tested-by: Rob Barnes <robbarnes@google.com>

6 files changed, 118 insertions, 48 deletions

diff --git a/common/panic_output.c b/common/panic_output.c
index 85927380ac..b168cd57bc 100644
--- a/common/panic_output.c
+++ b/common/panic_output.c
@@ -148,6 +148,12 @@ static uint32_t get_panic_data_size(void)
 	return pdata_ptr->struct_size;
 }
 
+__overridable uint32_t get_panic_stack_pointer(const struct panic_data *pdata)
+{
+	/* Not Implemented */
+	return 0;
+}
+
 /*
  * Returns pointer to panic_data structure that can be safely written.
  * Please note that this function can move jump data and jump tags.
diff --git a/common/system_safe_mode.c b/common/system_safe_mode.c
index 22d6e4f0e7..04a24c3ba6 100644
--- a/common/system_safe_mode.c
+++ b/common/system_safe_mode.c
@@ -8,6 +8,7 @@
 #include "cpu.h"
 #include "ec_commands.h"
 #include "hooks.h"
+#include "host_command.h"
 #include "panic.h"
 #include "stdbool.h"
 #include "stddef.h"
@@ -17,6 +18,8 @@
 #include "timer.h"
 #include "watchdog.h"
 
+#define STACK_PRINT_SIZE_WORDS 32
+
 static bool in_safe_mode;
 
 static const int safe_mode_allowed_hostcmds[] = {
@@ -88,6 +91,32 @@ bool system_is_in_safe_mode(void)
 	return !!in_safe_mode;
 }
 
+/*
+ * Print contents of stack to console buffer.
+ */
+static void print_panic_stack(void)
+{
+	uint32_t sp;
+	const struct panic_data *pdata = panic_get_data();
+
+	ccprintf("\n========== Stack Contents ===========");
+	sp = get_panic_stack_pointer(pdata);
+	for (int i = 0; i < STACK_PRINT_SIZE_WORDS; i++) {
+		if (sp == 0 ||
+		    sp + sizeof(uint32_t) > CONFIG_RAM_BASE + CONFIG_RAM_SIZE) {
+			ccprintf("\nSP(%x) out of range", sp);
+			break;
+		}
+		if (i % 4 == 0)
+			ccprintf("\n%08x:", sp);
+		ccprintf(" %08x", *(uint32_t *)sp);
+		sp += sizeof(uint32_t);
+	}
+	ccprintf("\n");
+	/* Flush so dump isn't mixed with other output */
+	cflush();
+}
+
 bool command_is_allowed_in_safe_mode(int command)
 {
 	for (int i = 0; i < ARRAY_SIZE(safe_mode_allowed_hostcmds); i++)
@@ -96,6 +125,16 @@ bool command_is_allowed_in_safe_mode(int command)
 	return false;
 }
 
+static void system_safe_mode_start(void)
+{
+	ccprintf("*** Post Panic System Safe Mode ***\n");
+	if (IS_ENABLED(CONFIG_SYSTEM_SAFE_MODE_PRINT_STACK))
+		print_panic_stack();
+	if (IS_ENABLED(CONFIG_HOSTCMD_EVENTS))
+		host_set_single_event(EC_HOST_EVENT_PANIC);
+}
+DECLARE_DEFERRED(system_safe_mode_start);
+
 int start_system_safe_mode(void)
 {
 	if (!system_is_in_rw()) {
diff --git a/core/cortex-m/panic.c b/core/cortex-m/panic.c
index 5866a14ca6..5ecad1535b 100644
--- a/core/cortex-m/panic.c
+++ b/core/cortex-m/panic.c
@@ -18,6 +18,9 @@
 #include "util.h"
 #include "watchdog.h"
 
+#define BASE_EXCEPTION_FRAME_SIZE_BYTES (8 * sizeof(uint32_t))
+#define FPU_EXCEPTION_FRAME_SIZE_BYTES (18 * sizeof(uint32_t))
+
 /* Whether bus fault is ignored */
 static int bus_fault_ignored;
 
@@ -77,6 +80,53 @@ static int32_t is_frame_in_handler_stack(const uint32_t exc_return)
 	return (exc_return & 0xf) == 1 || (exc_return & 0xf) == 9;
 }
 
+/*
+ * Returns the size of the exception frame.
+ *
+ * See B1.5.7 "Stack alignment on exception entry" of ARM DDI 0403D for details.
+ * In short, the exception frame size can be either 0x20, 0x24, 0x68, or 0x6c
+ * depending on FPU context and padding for 8-byte alignment.
+ */
+static uint32_t get_exception_frame_size(const struct panic_data *pdata)
+{
+	uint32_t frame_size = 0;
+
+	/* base exception frame */
+	frame_size += BASE_EXCEPTION_FRAME_SIZE_BYTES;
+
+	/* CPU uses xPSR[9] to indicate whether it padded the stack for
+	 * alignment or not.
+	 */
+	if (pdata->cm.frame[CORTEX_PANIC_FRAME_REGISTER_PSR] & BIT(9))
+		frame_size += sizeof(uint32_t);
+
+#ifdef CONFIG_FPU
+	/* CPU uses EXC_RETURN[4] to indicate whether it stored extended
+	 * frame for FPU or not.
+	 */
+	if (!(pdata->cm.regs[CORTEX_PANIC_REGISTER_LR] & BIT(4)))
+		frame_size += FPU_EXCEPTION_FRAME_SIZE_BYTES;
+#endif
+
+	return frame_size;
+}
+
+/*
+ * Returns the position of the process stack before the exception frame.
+ * It computes the size of the exception frame and adds it to psp.
+ * If the exception happened in the exception context, it returns psp as is.
+ */
+uint32_t get_panic_stack_pointer(const struct panic_data *pdata)
+{
+	uint32_t psp = pdata->cm.regs[CORTEX_PANIC_REGISTER_PSP];
+
+	if (!is_frame_in_handler_stack(
+		    pdata->cm.regs[CORTEX_PANIC_REGISTER_LR]))
+		psp += get_exception_frame_size(pdata);
+
+	return psp;
+}
+
 #ifdef CONFIG_DEBUG_EXCEPTIONS
 /* Names for each of the bits in the mmfs register, starting at bit 0 */
 static const char * const mmfs_name[32] = {
@@ -185,50 +235,6 @@ static void show_fault(uint32_t mmfs, uint32_t hfsr, uint32_t dfsr)
 }
 
 /*
- * Returns the size of the exception frame.
- *
- * See B1.5.7 "Stack alignment on exception entry" of ARM DDI 0403D for details.
- * In short, the exception frame size can be either 0x20, 0x24, 0x68, or 0x6c
- * depending on FPU context and padding for 8-byte alignment.
- */
-static uint32_t get_exception_frame_size(const struct panic_data *pdata)
-{
-	uint32_t frame_size = 0;
-
-	/* base exception frame */
-	frame_size += 8 * sizeof(uint32_t);
-
-	/* CPU uses xPSR[9] to indicate whether it padded the stack for
-	 * alignment or not. */
-	if (pdata->cm.frame[7] & (1 << 9))
-		frame_size += sizeof(uint32_t);
-
-#ifdef CONFIG_FPU
-	/* CPU uses EXC_RETURN[4] to indicate whether it stored extended
-	 * frame for FPU or not. */
-	if (!(pdata->cm.regs[11] & (1 << 4)))
-		frame_size += 18 * sizeof(uint32_t);
-#endif
-
-	return frame_size;
-}
-
-/*
- * Returns the position of the process stack before the exception frame.
- * It computes the size of the exception frame and adds it to psp.
- * If the exception happened in the exception context, it returns psp as is.
- */
-static uint32_t get_process_stack_position(const struct panic_data *pdata)
-{
-	uint32_t psp = pdata->cm.regs[0];
-
-	if (!is_frame_in_handler_stack(pdata->cm.regs[11]))
-		psp += get_exception_frame_size(pdata);
-
-	return psp;
-}
-
-/*
  * Show extra information that might be useful to understand a panic()
  *
  * We show fault register information, including the fault address registers
@@ -254,7 +260,7 @@ static void panic_show_process_stack(const struct panic_data *pdata)
 {
 	panic_printf("\n=========== Process Stack Contents ===========");
 	if (pdata->flags & PANIC_DATA_FLAG_FRAME_VALID) {
-		uint32_t psp = get_process_stack_position(pdata);
+		uint32_t psp = get_panic_stack_pointer(pdata);
 		int i;
 		for (i = 0; i < 16; i++) {
 			if (psp + sizeof(uint32_t) >
@@ -334,9 +340,9 @@ void __keep report_panic(void)
 	sp = is_frame_in_handler_stack(pdata->cm.regs[11])
 		? pdata->cm.regs[2] : pdata->cm.regs[0];
 	/* If stack is valid, copy exception frame to pdata */
-	if ((sp & 3) == 0 &&
-	    sp >= CONFIG_RAM_BASE &&
-	    sp <= CONFIG_RAM_BASE + CONFIG_RAM_SIZE - 8 * sizeof(uint32_t)) {
+	if ((sp & 3) == 0 && sp >= CONFIG_RAM_BASE &&
+	    sp <= CONFIG_RAM_BASE + CONFIG_RAM_SIZE -
+			    BASE_EXCEPTION_FRAME_SIZE_BYTES) {
 		const uint32_t *sregs = (const uint32_t *)sp;
 		int i;
 
diff --git a/core/nds32/panic.c b/core/nds32/panic.c
index 8b87cc0ef8..4945bae53e 100644
--- a/core/nds32/panic.c
+++ b/core/nds32/panic.c
@@ -141,6 +141,14 @@ void panic_get_reason(uint32_t *reason, uint32_t *info, uint8_t *exception)
 }
 #endif /* CONFIG_SOFTWARE_PANIC */
 
+/**
+ * Returns the SP register
+ */
+uint32_t get_panic_stack_pointer(const struct panic_data *pdata)
+{
+	return pdata->nds_n8.regs[15];
+}
+
 static void print_panic_information(uint32_t *regs, uint32_t itype,
 					uint32_t ipc, uint32_t ipsw)
 {
diff --git a/include/config.h b/include/config.h
index 80abb601c5..40d5f58fdd 100644
--- a/include/config.h
+++ b/include/config.h
@@ -1167,6 +1167,11 @@
  */
 #undef CONFIG_SYSTEM_SAFE_MODE
 #define CONFIG_SYSTEM_SAFE_MODE_TIMEOUT_MSEC 2000
+/*
+ * Prints the stack of the faulting task to the console buffer in system safe
+ * mode.
+ */
+#define CONFIG_SYSTEM_SAFE_MODE_PRINT_STACK
 
 /*
  * Provide the default GPIO abstraction layer.
diff --git a/include/panic.h b/include/panic.h
index b366567468..336c9e27cd 100644
--- a/include/panic.h
+++ b/include/panic.h
@@ -227,6 +227,12 @@ uintptr_t get_panic_data_start(void);
 struct panic_data *get_panic_data_write(void);
 
 /**
+ * Return a pointer to the stack of the process that caused the panic.
+ * The implementation of this function will depend on the architecture.
+ */
+uint32_t get_panic_stack_pointer(const struct panic_data *pdata);
+
+/**
  * Chip-specific implementation for backing up panic data to persistent
  * storage. This function is used to ensure that the panic data can survive loss
  * of VCC power rail.