From 2a856039445818b2bb3b89eff2e32376508db8b1 Mon Sep 17 00:00:00 2001 From: Daisuke Nojiri Date: Mon, 11 Oct 2021 12:50:25 -0700 Subject: npcx/timer: Unroll udelay This patch flattens udelay by unrolling __hw_clock_source_read. This increases the chance that we record LR of the instruction near which an infinite loop happened. > battery 1 1000000 ... WATCHDOG PC=1008a76c / LR=1008c81f / pSP=200c44f8 (task 10) ### Time: 0x0000000000f22630 us, 15.869488 s Deadline: 0x0000000000f43946 -> 0.135958 s from now Active timers: Tsk 14 0x0000000000f43946 -> 0.135958 Task Ready Name Events Time (s) StkUsed 0 R << idle >> 00000001 4.840735 80/672 1 R HOOKS 80000000 0.788073 648/800 2 USB_CHG_P0 00000000 0.001591 312/672 3 USB_CHG_P1 00000000 0.004209 320/672 4 R CHARGER 40000000 0.103775 400/800 5 R MOTIONSENSE 80000004 0.098946 560/928 6 CHIPSET 00000000 0.000588 296/800 7 KEYPROTO 00000000 0.000019 128/672 8 PDCMD 00000000 0.001017 328/672 9 HOSTCMD 00000000 0.043613 336/800 10 R CONSOLE 00000000 2.342869 384/800 --- UART initialized after reboot --- [Reset cause: watchdog] [Image: RO, nami_v1.1.8956-b85666cd37 2021-11-10 15:07:11 some@host] [0.003867 init buttons] [0.004086 Inits done] Restarting system with PMIC. ... WATCHDOG PC=1008a76c / LR=1008c81f / task=10 r0 : r1 : r2 : r3 : r4 :dead6664 r5 :1008a76c 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 = 1008c81f BUG=b:200593658, b:205841546 BRANCH= TEST=Run hacked battery command to trigger WDT on Sona. Verify the LR points to command_battery instead of udelay. See above. Change-Id: Ibd6cbcf18ab6d58c06ddfd19021058268289bf00 Signed-off-by: Daisuke Nojiri Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3235653 Reviewed-by: caveh jalali --- board/cr50/board.h | 3 --- chip/g/hwtimer.c | 4 +--- chip/npcx/hwtimer.c | 22 ++++++++++++++++++++++ common/timer.c | 4 +--- include/config.h | 3 --- include/timer.h | 2 +- 6 files changed, 25 insertions(+), 13 deletions(-) diff --git a/board/cr50/board.h b/board/cr50/board.h index 9a2f79717b..e2640dfd29 100644 --- a/board/cr50/board.h +++ b/board/cr50/board.h @@ -139,9 +139,6 @@ #define CONFIG_DCRYPTO #define CONFIG_UPTO_SHA512 -/* Implement custom udelay, due to usec hwtimer imprecision. */ -#define CONFIG_HW_SPECIFIC_UDELAY - #define CONFIG_TPM_LOGGING #ifndef __ASSEMBLER__ diff --git a/chip/g/hwtimer.c b/chip/g/hwtimer.c index 91dba78c40..267cf395cf 100644 --- a/chip/g/hwtimer.c +++ b/chip/g/hwtimer.c @@ -178,14 +178,13 @@ int __hw_clock_source_init(uint32_t start_t) return GC_IRQNUM_TIMELS0_TIMINT1; } -#ifdef CONFIG_HW_SPECIFIC_UDELAY /* * Custom chip/g udelay(), guaranteed to delay for at least us microseconds. * * Lost time during timer wrap is not taken into account since interrupt latency * and __hw_clock_source_irq() execution time likely exceeds the lost 3us. */ -void udelay(unsigned us) +__override void udelay(unsigned us) { unsigned t0 = __hw_clock_source_read(); @@ -210,4 +209,3 @@ void udelay(unsigned us) while (__hw_clock_source_read() - t0 <= us) ; } -#endif /* CONFIG_HW_SPECIFIC_UDELAY */ diff --git a/chip/npcx/hwtimer.c b/chip/npcx/hwtimer.c index e46a56fb1e..d9a5728942 100644 --- a/chip/npcx/hwtimer.c +++ b/chip/npcx/hwtimer.c @@ -16,6 +16,7 @@ #include "console.h" #include "task.h" #include "timer.h" +#include "registers.h" #include "util.h" /* Depth of event timer */ @@ -332,3 +333,24 @@ int __hw_clock_source_init(uint32_t start_t) return NPCX_IRQ_ITIM32; } + +/* + * Unrolled udelay. It preserves LR, which points to the root cause of WD crash. + */ +__override void udelay(unsigned us) +{ + uint32_t cnt, cnt2; + unsigned t0; + + cnt = NPCX_ITCNT32; + while ((cnt2 = NPCX_ITCNT32) != cnt) + cnt = cnt2; + + t0 = TICK_ITIM32_MAX_CNT - cnt; + + do { + cnt = NPCX_ITCNT32; + while ((cnt2 = NPCX_ITCNT32) != cnt) + cnt = cnt2; + } while (TICK_ITIM32_MAX_CNT - cnt - t0 <= us); +} diff --git a/common/timer.c b/common/timer.c index 117cea4b71..6f31d93b09 100644 --- a/common/timer.c +++ b/common/timer.c @@ -94,8 +94,7 @@ void process_timers(int overflow) } while (next.val <= get_time().val); } -#ifndef CONFIG_HW_SPECIFIC_UDELAY -void udelay(unsigned us) +__overridable void udelay(unsigned us) { unsigned t0 = __hw_clock_source_read(); @@ -112,7 +111,6 @@ void udelay(unsigned us) while (__hw_clock_source_read() - t0 <= us) ; } -#endif int timer_arm(timestamp_t tstamp, task_id_t tskid) { diff --git a/include/config.h b/include/config.h index 443f93d738..86c1707512 100644 --- a/include/config.h +++ b/include/config.h @@ -1850,9 +1850,6 @@ */ #undef CONFIG_HIBERNATE_PSL -/* Use a hardware specific udelay(). */ -#undef CONFIG_HW_SPECIFIC_UDELAY - /*****************************************************************************/ /* I2C configuration */ diff --git a/include/timer.h b/include/timer.h index fb7800ce2e..4a008f6d06 100644 --- a/include/timer.h +++ b/include/timer.h @@ -71,7 +71,7 @@ int timestamp_expired(timestamp_t deadline, const timestamp_t *now); * * @param us Number of microseconds to delay. */ -void udelay(unsigned us); +__override_proto void udelay(unsigned us); /** * Sleep. -- cgit v1.2.1