From caa71d4462de86452fddc75bd3416b740af9353c Mon Sep 17 00:00:00 2001 From: Daisuke Nojiri Date: Mon, 11 Oct 2021 12:50:25 -0700 Subject: Panic: Save LR and code location in HFSR Since Nami's PMIC reset clears panic data, we currently report only a few fault registers (HFSR, MMSR, etc.), IPSR, r4, and r5. These are included in a feedback report but not very useful. To debug a crash happening remotely, this patch makes the assert handler, the watchdog handler, and the panic handler store PC, LR, and some additional info. | regs[1] | regs[3] | regs[4] | HFSR | (IPSR) | (r4) | (r5) | [29:2] -------+---------+----------+----------+-------------------- Assert | n/a | __func__ | __FILE__ | [29:26] Flags (=3) | | | | [25:10] Line# | | | | [ 9: 2] Task# -------+---------+----------+----------+-------------------- WDT | LR | Reason | PC | [29:26] Flags (=4) | | | | [ 9: 2] Task# -------+---------+----------+----------+-------------------- Panic | IPSR | LR | PC | n/a -------+---------+----------+----------+-------------------- Note that this patch is created under several constraints imposed by the released RO. Some of the constraints are as follows: - It's the RO not RW who saves the panic data into BBRAM. This is for avoiding executing extra code in a restricted environment (= panic). - If RO sees a watchdog reset without reason == PANIC_SW_WATCHDOG, it stores PANIC_SW_WATCHDOG in r4 and clears r5 and IPSR. - BBRAM is already used up to the max (64 bytes). Obviously, a patch for ToT would be very different (and much cleaner) since it's free from these constraints. Most importantly, there is no PMIC reset which erases panic data. > crash assert ASSERTION FAILURE '0' in command_crash() at common/panic_output.c:165 Rebooting... --- UART initialized after reboot --- [Reset cause: soft] [Image: RO, nami_v1.1.8956+f067821734 2021-11-09 12:31:41 some@host] [0.003853 init buttons] [0.004072 Inits done] Restarting system with PMIC. > panicinfo ASSERTION FAILURE in command_crash() at common/panic_output.c:165 r0 : r1 : r2 : r3 : r4 :100a9f82 r5 :100a89e0 r6 :00000000 r7 :00000000 r8 :00000000 r9 :00000000 r10:00000000 r11:00000000 r12: sp :00000000 lr : pc : mmfs = 0, shcsr = 0, hfsr = c029428, dfsr = 0, ipsr = 100995ad > crash divzero === PROCESS EXCEPTION: 06 ====== xPSR: 41000000 === r0 :00000000 r1 :100a8f09 r2 :400c4000 r3 :00000000 r4 :1009956f r5 :1009956e r6 :200c07c4 r7 :100a4fe3 r8 :100a4fde r9 :100a3a54 r10:00000000 r11:200c07d2 r12:00000000 sp :200c4508 lr :1009956f pc :1009956e Undefined instructions mmfs = 10000, shcsr = 70008, hfsr = 0, dfsr = 0, ipsr = 00000006 =========== Process Stack Contents =========== 200c4528: 00000002 200c4548 200c07c4 200c0400 200c4538: 00000002 1008d2c7 00000000 200c07c4 200c4548: 200c07c4 200c07ca 00000000 00000000 200c4558: 00000000 00000000 00000000 00000000 Rebooting... > crashinfo === PROCESS EXCEPTION: 06 ====== xPSR: ffffffff === r0 : r1 : r2 : r3 : r4 :1009956f r5 :1009956e r6 :00000000 r7 :00000000 r8 :00000000 r9 :00000000 r10:00000000 r11:00000000 r12: sp :00000000 lr : pc : Undefined instructions mmfs = 10000, shcsr = 0, hfsr = 0, dfsr = 0, ipsr = 00000006 > crash watchdog >### WATCHDOG PC=100995ae / LR=100995ad / pSP=200c4508 (task 10) ### Time: 0x0000000009dd5f1d us, 165.502749 s Deadline: 0x0000000009de2ab3 -> 0.052118 s from now Active timers: Tsk 14 0x0000000009de2ab3 -> 0.052118 Task Ready Name Events Time (s) StkUsed 0 R << idle >> 00000001 134.736448 80/672 1 R HOOKS 80000000 0.935280 648/800 2 USB_CHG_P0 00000000 0.001806 312/672 3 USB_CHG_P1 00000000 0.004606 320/672 4 R CHARGER 80000000 1.657817 424/800 5 R MOTIONSENSE 80000004 1.789985 560/928 6 CHIPSET 00000000 0.000396 296/800 7 KEYPROTO 00000000 0.002983 312/672 8 PDCMD 00000000 0.000065 112/672 9 R HOSTCMD 00000001 0.517519 568/800 10 R CONSOLE 00000000 2.074095 332/800 ... > panicinfo > ### WATCHDOG PC=100995ae / LR=100995ad / task=10 r0 : r1 : r2 : r3 : r4 :dead6664 r5 :100995ae r6 :00000000 r7 :00000000 r8 :00000000 r9 :00000000 r10:00000000 r11:00000000 r12: sp :00000000 lr : pc : mmfs = 0, shcsr = 0, hfsr = 10000028, dfsr = 0, ipsr = 100995ad BUG=b:200593658 BRANCH=Nami TEST=Sona. Run crash assert. Verify function name, file name, line number, task id are stored in r4, r5, ipsr by panicinfo after PMIC reset. Change-Id: I55b142ac4de14c05a8184a66ed127c9cbcd29745 Signed-off-by: Daisuke Nojiri Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3271351 Reviewed-by: caveh jalali Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3457946 Reviewed-by: Aseda Aboagye Commit-Queue: Ricardo Quesada Tested-by: Ricardo Quesada Auto-Submit: Ricardo Quesada --- common/panic_output.c | 2 +- core/cortex-m/panic.c | 54 ++++++++++++++++++++++++++++++++++++++++++++---- core/cortex-m/watchdog.c | 13 ++++++++---- include/panic.h | 13 ++++++++++++ 4 files changed, 73 insertions(+), 9 deletions(-) diff --git a/common/panic_output.c b/common/panic_output.c index e6b48a375d..fd1b1999a0 100644 --- a/common/panic_output.c +++ b/common/panic_output.c @@ -98,7 +98,7 @@ void panic_assert_fail(const char *msg, const char *func, const char *fname, panic_printf("\nASSERTION FAILURE '%s' in %s() at %s:%d\n", msg, func, fname, linenum); #ifdef CONFIG_SOFTWARE_PANIC - software_panic(PANIC_SW_ASSERT, linenum); + panic_assert(func, fname, (uint16_t)linenum); #else panic_reboot(); #endif diff --git a/core/cortex-m/panic.c b/core/cortex-m/panic.c index de8ca07b3e..c4954ec000 100644 --- a/core/cortex-m/panic.c +++ b/core/cortex-m/panic.c @@ -227,7 +227,8 @@ static void panic_show_extra(const struct panic_data *pdata) panic_printf("\ncfsr = %x, ", pdata->cm.cfsr); panic_printf("shcsr = %x, ", pdata->cm.shcsr); panic_printf("hfsr = %x, ", pdata->cm.hfsr); - panic_printf("dfsr = %x\n", pdata->cm.dfsr); + panic_printf("dfsr = %x, ", pdata->cm.dfsr); + panic_printf("ipsr = %x\n", pdata->cm.regs[1]); } /* @@ -268,9 +269,18 @@ void panic_data_print(const struct panic_data *pdata) if (pdata->flags & PANIC_DATA_FLAG_FRAME_VALID) sregs = pdata->cm.frame; - panic_printf("\n=== %s EXCEPTION: %02x ====== xPSR: %08x ===\n", - in_handler ? "HANDLER" : "PROCESS", - lregs[1] & 0xff, sregs ? sregs[7] : -1); + if (((pdata->cm.hfsr >> 26) & 0xf) == HFSR_FLAG_WATCHDOG) { + panic_printf("\n### WATCHDOG PC=%08x / LR=%08x / task=%d\n", + lregs[4], lregs[1], (pdata->cm.hfsr >> 2) & 0xff); + } else if (((pdata->cm.hfsr >> 26) & 0xf) == HFSR_FLAG_ASSERT) { + panic_printf("\nASSERTION FAILURE in %s() at %s:%d\n", + lregs[3], lregs[4], + (pdata->cm.hfsr >> 10) & 0xffff); + } else { + panic_printf("\n=== %s EXCEPTION: %02x ====== xPSR: %08x ===\n", + in_handler ? "HANDLER" : "PROCESS", + lregs[1] & 0xff, sregs ? sregs[7] : -1); + } for (i = 0; i < 4; i++) print_reg(i, sregs, i); for (i = 4; i < 10; i++) @@ -332,6 +342,12 @@ void __keep report_panic(void) pdata->cm.hfsr = CPU_NVIC_HFSR; pdata->cm.dfsr = CPU_NVIC_DFSR; + /* Store LR & PC in cm.regs to make them survive a PMIC reset. */ + if (pdata->flags & PANIC_DATA_FLAG_FRAME_VALID) { + pdata->cm.regs[3] = pdata->cm.frame[5]; /* LR */ + pdata->cm.regs[4] = pdata->cm.frame[6]; /* PC */ + } + #ifdef CONFIG_UART_PAD_SWITCH uart_reset_default_pad_panic(); #endif @@ -440,6 +456,36 @@ void panic_get_reason(uint32_t *reason, uint32_t *info, uint8_t *exception) } #endif +void panic_assert(const char *func, const char *file, uint16_t line) +{ + struct panic_data *pdata = pdata_ptr; + + pdata->magic = PANIC_DATA_MAGIC; + pdata->struct_size = sizeof(*pdata); + pdata->struct_version = 2; + pdata->arch = PANIC_ARCH_CORTEX_M; + pdata->flags = 0; + pdata->reserved = 0; + + /* + * Panic data is cleared by PMIC reset on Nami. HFSR is used because + * it's saved to BBRAM by the released RO (before PMIC reset). Bit + * assignments are as follows: + * + * ([31:30] Used for DEBUGEVT and FORCED) + * [29:26] Flags + * [25:10] Line # + * [9:2] Task # + * ([1:0] Used for VECTTBLE and reserved) + */ + pdata->cm.hfsr = HFSR_FLAG_ASSERT << 26 | line << 10 + | (task_get_current() & 0xff) << 2; + pdata->cm.regs[3] = (uint32_t)func; + pdata->cm.regs[4] = (uint32_t)file; + + panic_reboot(); +} + void bus_fault_handler(void) { if (!bus_fault_ignored) diff --git a/core/cortex-m/watchdog.c b/core/cortex-m/watchdog.c index 4da32d26d0..f9283bbbd7 100644 --- a/core/cortex-m/watchdog.c +++ b/core/cortex-m/watchdog.c @@ -32,12 +32,17 @@ void __keep watchdog_trace(uint32_t excep_lr, uint32_t excep_sp) panic_set_reason(PANIC_SW_WATCHDOG, stack[6], (excep_lr & 0xf) == 1 ? 0xff : task_get_current()); + /* Copy task# to cm.hfsr[9:2]. */ + pdata_ptr->cm.hfsr = HFSR_FLAG_WATCHDOG << 26 + | (pdata_ptr->cm.regs[1] & 0xff) << 2; + /* - * Store LR to cm.hfsr. It is for HardFault status register but it is - * probably the least informative register used by - * chip_panic_data_backup of the existing RO. + * Store LR in cm.regs[1] (ipsr). IPSR holds exception# but we don't + * need it because the reason (=PANIC_SW_WATCHDOG) is already stored + * in cm.regs[3] (r4). Note this overwrites task# in cm.regs[1] stored + * by panic_set_reason. */ - pdata_ptr->cm.hfsr = stack[5]; + pdata_ptr->cm.regs[1] = stack[5]; panic_printf("### WATCHDOG PC=%08x / LR=%08x / pSP=%08x ", stack[6], stack[5], psp); diff --git a/include/panic.h b/include/panic.h index 47c749d027..d1099d6cce 100644 --- a/include/panic.h +++ b/include/panic.h @@ -188,6 +188,19 @@ void panic_set_reason(uint32_t reason, uint32_t info, uint8_t exception); void panic_get_reason(uint32_t *reason, uint32_t *info, uint8_t *exception); #endif +#define HFSR_FLAG_ASSERT 3 +#define HFSR_FLAG_WATCHDOG 4 + +/** + * Save code location in struct panic_data then reboot. + * + * @param func Function name (i.e. __func__). Stored in regs[3]. + * @param file File path (i.e. __FILE__). Stored in regs[4]. + * @param line Line number (i.e. __LINE__). Stored in HFSR[25:10]. + */ +void panic_assert(const char *func, const char *file, uint16_t line) + __attribute__((noreturn)); + /** * Enable/disable bus fault handler * -- cgit v1.2.1