diff options
author | Jack Rosenthal <jrosenth@chromium.org> | 2021-01-22 16:58:36 -0700 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2021-01-26 23:21:35 +0000 |
commit | c10b22901e01e00a9c2c0a5e790a54487f391fe6 (patch) | |
tree | e2a6f406c1bd2223183964237288d671cf92c039 | |
parent | 061b3ee3a305833e061019744add3f96d5fd1529 (diff) | |
download | chrome-ec-c10b22901e01e00a9c2c0a5e790a54487f391fe6.tar.gz |
zephyr: Add support for panic output
Add basic panic implementation for Zephyr. Not using any fancy shared
or always-on memory for now ... need to resolve how that will be
handled later.
BUG=b:178011288
BRANCH=none
TEST=run various crash commands on volteer, observe output
Signed-off-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Jack Rosenthal <jrosenth@chromium.org>
Change-Id: Ia1ce386f738283a2a2b9b60ef7e0bf97f8317837
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2645687
-rw-r--r-- | common/panic_output.c | 33 | ||||
-rw-r--r-- | zephyr/CMakeLists.txt | 1 | ||||
-rw-r--r-- | zephyr/Kconfig | 52 | ||||
-rw-r--r-- | zephyr/shim/include/config_chip.h | 20 | ||||
-rw-r--r-- | zephyr/shim/src/CMakeLists.txt | 1 | ||||
-rw-r--r-- | zephyr/shim/src/panic.c | 80 | ||||
-rw-r--r-- | zephyr/shim/src/system.c | 7 |
7 files changed, 186 insertions, 8 deletions
diff --git a/common/panic_output.c b/common/panic_output.c index a3f34e6d47..66a39a60cc 100644 --- a/common/panic_output.c +++ b/common/panic_output.c @@ -19,6 +19,18 @@ #include "usb_console.h" #include "util.h" +/* + * TODO(b/178011288): use persistent storage for panic data in + * Zephyr OS + */ +#ifdef CONFIG_ZEPHYR +static struct panic_data zephyr_panic_data; +#undef PANIC_DATA_PTR +#undef CONFIG_PANIC_DATA_BASE +#define PANIC_DATA_PTR (&zephyr_panic_data) +#define CONFIG_PANIC_DATA_BASE (&zephyr_panic_data) +#endif + /* Panic data goes at the end of RAM. */ static struct panic_data * const pdata_ptr = PANIC_DATA_PTR; @@ -167,6 +179,9 @@ uintptr_t get_panic_data_start(void) if (pdata_ptr->magic != PANIC_DATA_MAGIC) return 0; + if (IS_ENABLED(CONFIG_ZEPHYR)) + return (uintptr_t)pdata_ptr; + return ((uintptr_t)CONFIG_PANIC_DATA_BASE + CONFIG_PANIC_DATA_SIZE - pdata_ptr->struct_size); @@ -185,7 +200,16 @@ static uint32_t get_panic_data_size(void) * Please note that this function can move jump data and jump tags. * It can also delete panic data from previous boot, so this function * should be used when we are sure that we don't need it. + * + * TODO(b/178011288): figure out an appropriate implementation for + * Zephyr. */ +#ifdef CONFIG_ZEPHYR +struct panic_data *get_panic_data_write(void) +{ + return pdata_ptr; +} +#else struct panic_data *get_panic_data_write(void) { /* @@ -268,6 +292,7 @@ struct panic_data *get_panic_data_write(void) return pdata_ptr; } +#endif /* CONFIG_ZEPHYR */ static void panic_init(void) { @@ -337,9 +362,13 @@ static int command_crash(int argc, char **argv) while (1) ; } else if (!strcasecmp(argv[1], "hang")) { - interrupt_disable(); + uint32_t lock_key = irq_lock(); + while (1) ; + + /* Unreachable, but included for consistency */ + irq_unlock(lock_key); } else { return EC_ERROR_PARAM1; } @@ -354,7 +383,7 @@ DECLARE_CONSOLE_COMMAND(crash, command_crash, #endif " | unaligned | watchdog | hang]", "Crash the system (for testing)"); -#endif +#endif /* CONFIG_CMD_CRASH */ static int command_panicinfo(int argc, char **argv) { diff --git a/zephyr/CMakeLists.txt b/zephyr/CMakeLists.txt index 3842220085..7a62e4d7e4 100644 --- a/zephyr/CMakeLists.txt +++ b/zephyr/CMakeLists.txt @@ -150,6 +150,7 @@ zephyr_sources_ifdef(CONFIG_PLATFORM_EC_POWERSEQ_INTEL "${PLATFORM_EC}/power/intel_x86.c") zephyr_sources_ifdef(CONFIG_PLATFORM_EC_POWERSEQ_HOST_SLEEP "${PLATFORM_EC}/power/host_sleep.c") +zephyr_sources_ifdef(CONFIG_PLATFORM_EC_PANIC "${PLATFORM_EC}/common/panic_output.c") zephyr_sources_ifdef(CONFIG_PLATFORM_EC_THROTTLE_AP "${PLATFORM_EC}/common/throttle_ap.c") zephyr_sources_ifdef(CONFIG_PLATFORM_EC_TABLET_MODE diff --git a/zephyr/Kconfig b/zephyr/Kconfig index 4be595f343..02570e3c2f 100644 --- a/zephyr/Kconfig +++ b/zephyr/Kconfig @@ -361,6 +361,58 @@ endchoice endif # PLATFORM_EC_MKBP_EVENT +config PLATFORM_EC_PANIC + bool "Panic output" + default y + help + Enable support for collecting and reporting panic information, + caused by exceptions in the EC. This can be the result of a watchdog + firing, a division by zero, unaligned access, stack overflow or + assertion failure. + + The panic information is made available to the AP via the + EC_CMD_GET_PANIC_INFO host command and a 'panicinfo' console command + +if PLATFORM_EC_PANIC + +config PLATFORM_EC_SOFTWARE_PANIC + bool "Software panic" + default y + help + Sometimes the EC has bugs that are provoked by unexpected events. + This enables storing a panic log and halt the system for + software-related reasons, such as stack overflow or assertion + failure. + +config PLATFORM_EC_CONSOLE_CMD_CRASH + bool "Console command: crash" + default y + help + For testing purposes it is useful to be able to generation exceptions + to check that the panic reporting is working correctly. The enables + the 'crash' command which supports generating various exceptions, + selected by its parameter: + + assert - assertion failure (ASSERT() macro) + divzero - division by zero + udivzero - division of unsigned value by zero + stack (if enabled) - stack overflow + unaligned - unaligned access (e.g. word load from 0x123) + watchdog - watchdog timer fired + hang - infinite loop in the EC code + +config PLATFORM_EC_STACKOVERFLOW + bool "Console command: crash stack" + depends on PLATFORM_EC_CONSOLE_CMD_CRASH + default y + help + This enables the 'stack' parameter for the 'crash' command. This + causes a stack overflow by recursing repeatedly while task switching. + This can be used to check that stack-overflow detection is working + as expected. + +endif # PLATFORM_EC_PANIC + config PLATFORM_EC_PORT80 bool "Port 80 support" default y if AP_X86 && PLATFORM_EC_POWERSEQ diff --git a/zephyr/shim/include/config_chip.h b/zephyr/shim/include/config_chip.h index 73a41547e2..a2e3b63d86 100644 --- a/zephyr/shim/include/config_chip.h +++ b/zephyr/shim/include/config_chip.h @@ -734,4 +734,24 @@ enum battery_type { #define CONFIG_POWER_BUTTON #endif +#undef CONFIG_COMMON_PANIC_OUTPUT +#ifdef CONFIG_PLATFORM_EC_PANIC +#define CONFIG_COMMON_PANIC_OUTPUT +#endif + +#undef CONFIG_SOFTWARE_PANIC +#ifdef CONFIG_PLATFORM_EC_SOFTWARE_PANIC +#define CONFIG_SOFTWARE_PANIC +#endif + +#undef CONFIG_CMD_CRASH +#ifdef CONFIG_PLATFORM_EC_CONSOLE_CMD_CRASH +#define CONFIG_CMD_CRASH +#endif + +#undef CONFIG_CMD_STACKOVERFLOW +#ifdef CONFIG_PLATFORM_EC_STACKOVERFLOW +#define CONFIG_CMD_STACKOVERFLOW +#endif + #endif /* __CROS_EC_CONFIG_CHIP_H */ diff --git a/zephyr/shim/src/CMakeLists.txt b/zephyr/shim/src/CMakeLists.txt index 03ff31eb8c..6512938e56 100644 --- a/zephyr/shim/src/CMakeLists.txt +++ b/zephyr/shim/src/CMakeLists.txt @@ -14,6 +14,7 @@ zephyr_sources_ifdef(CONFIG_PLATFORM_EC_FLASH flash.c) zephyr_sources_ifdef(CONFIG_PLATFORM_EC_HOOKS hooks.c) zephyr_sources_ifdef(CONFIG_PLATFORM_EC_HOSTCMD host_command.c) zephyr_sources_ifdef(CONFIG_PLATFORM_EC_MKBP_EVENT mkbp_event.c) +zephyr_sources_ifdef(CONFIG_PLATFORM_EC_PANIC panic.c) zephyr_sources_ifdef(CONFIG_PLATFORM_EC_TIMER hwtimer.c) zephyr_sources_ifdef(CONFIG_PLATFORM_EC_I2C i2c.c) zephyr_sources_ifdef(CONFIG_CROS_EC system.c) diff --git a/zephyr/shim/src/panic.c b/zephyr/shim/src/panic.c new file mode 100644 index 0000000000..e556fcbcca --- /dev/null +++ b/zephyr/shim/src/panic.c @@ -0,0 +1,80 @@ +/* Copyright 2021 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include <arch/cpu.h> +#include <fatal.h> +#include <logging/log.h> +#include <logging/log_ctrl.h> +#include <zephyr.h> + +#include "panic.h" + +/* + * Arch-specific configuration + * + * For each architecture, define: + * - PANIC_ARCH, which should be the corresponding arch field of the + * panic_data struct. + * - PANIC_REG_LIST, which is a macro that takes a parameter M, and + * applies M to 3-tuples of: + * - zephyr esf field name + * - panic_data struct field name + * - human readable name + */ + +#ifdef CONFIG_ARM +#define PANIC_ARCH PANIC_ARCH_CORTEX_M +#define PANIC_REG_LIST(M) \ + M(basic.r0, cm.frame[0], a1) \ + M(basic.r1, cm.frame[1], a2) \ + M(basic.r2, cm.frame[2], a3) \ + M(basic.r3, cm.frame[3], a4) \ + M(basic.r12, cm.frame[4], ip) \ + M(basic.lr, cm.frame[5], lr) \ + M(basic.pc, cm.frame[6], pc) \ + M(basic.xpsr, cm.frame[7], xpsr) +#else +/* Not implemented for this arch */ +#define PANIC_ARCH 0 +#define PANIC_REG_LIST(M) +#endif + +/* Macros to be applied to PANIC_REG_LIST as M */ +#define PANIC_COPY_REGS(esf_field, pdata_field, human_name) \ + pdata->pdata_field = esf->esf_field; +#define PANIC_PRINT_REGS(esf_field, pdata_field, human_name) \ + panic_printf(" %-8s = 0x%08X\n", #human_name, pdata->pdata_field); + +static void copy_esf_to_panic_data(const z_arch_esf_t *esf, + struct panic_data *pdata) +{ + pdata->arch = PANIC_ARCH; + pdata->struct_version = 2; + pdata->flags = 0; + pdata->reserved = 0; + pdata->struct_size = sizeof(*pdata); + pdata->magic = PANIC_DATA_MAGIC; + + PANIC_REG_LIST(PANIC_COPY_REGS); +} + +void panic_data_print(const struct panic_data *pdata) +{ + PANIC_REG_LIST(PANIC_PRINT_REGS); +} + +void k_sys_fatal_error_handler(unsigned int reason, const z_arch_esf_t *esf) +{ + panic_printf("Fatal error: %u\n", reason); + + if (PANIC_ARCH && esf) { + copy_esf_to_panic_data(esf, get_panic_data_write()); + panic_data_print(panic_get_data()); + } + + LOG_PANIC(); + k_fatal_halt(reason); + CODE_UNREACHABLE; +} diff --git a/zephyr/shim/src/system.c b/zephyr/shim/src/system.c index 7d62ac03fa..c407410931 100644 --- a/zephyr/shim/src/system.c +++ b/zephyr/shim/src/system.c @@ -10,6 +10,7 @@ #include "chipset.h" #include "config.h" #include "ec_commands.h" +#include "panic.h" #include "sysjump.h" #include "system.h" @@ -51,12 +52,6 @@ static bool jumped_to_image; /* static void jump_to_image */ -/* TODO(b/171407461) implement components/panic */ -static uintptr_t get_panic_data_start(void) -{ - return 0; -} - void system_common_pre_init(void) { /* TODO check for watchdog reset. */ |