From 91893ee143fed8e99da15f8b3b522e7750b5a1dd Mon Sep 17 00:00:00 2001 From: Tristan Honscheid Date: Mon, 31 Oct 2022 15:52:50 -0600 Subject: zephyr: test: Test `crash` console command Test various `crash` subcommands that cause different types of fatal errors. Not all were possible to test due to limitations with running these on a PC. Also had to make a tweak to the busy loops in the watchdog and hang subcommands so that they don't interfere with the simulated sys clock in native_posix. Add a public wrapper function for calling the crash command outside of the shell interface so we don't mess up its internal state when the command doesn't return. BUG=None BRANCH=None TEST=./twister Signed-off-by: Tristan Honscheid Change-Id: I393e26441d44c0a5edaf1359413efe6280703add Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4053360 Reviewed-by: Keith Short Code-Coverage: Zoss --- common/panic_output.c | 24 ++- include/panic.h | 16 ++ zephyr/test/drivers/CMakeLists.txt | 1 + zephyr/test/drivers/Kconfig | 3 + zephyr/test/drivers/panic_output/CMakeLists.txt | 8 + .../panic_output/src/panic_output_crash_cmd.c | 167 +++++++++++++++++++++ zephyr/test/drivers/testcase.yaml | 5 + 7 files changed, 220 insertions(+), 4 deletions(-) create mode 100644 zephyr/test/drivers/panic_output/CMakeLists.txt create mode 100644 zephyr/test/drivers/panic_output/src/panic_output_crash_cmd.c diff --git a/common/panic_output.c b/common/panic_output.c index fd163b21f5..066eb05d6f 100644 --- a/common/panic_output.c +++ b/common/panic_output.c @@ -373,13 +373,21 @@ static int command_crash(int argc, const char **argv) cflush(); ccprintf("%08x", *(volatile int *)unaligned_ptr); } else if (!strcasecmp(argv[1], "watchdog")) { - while (1) - ; + while (1) { +/* Yield on native posix to avoid locking up the simulated sys clock */ +#ifdef CONFIG_ARCH_POSIX + k_cpu_idle(); +#endif + } } else if (!strcasecmp(argv[1], "hang")) { uint32_t lock_key = irq_lock(); - while (1) - ; + while (1) { +/* Yield on native posix to avoid locking up the simulated sys clock */ +#ifdef CONFIG_ARCH_POSIX + k_cpu_idle(); +#endif + } /* Unreachable, but included for consistency */ irq_unlock(lock_key); @@ -390,6 +398,7 @@ static int command_crash(int argc, const char **argv) /* Everything crashes, so shouldn't get back here */ return EC_ERROR_UNKNOWN; } + DECLARE_CONSOLE_COMMAND(crash, command_crash, "[assert | divzero | udivzero" #ifdef CONFIG_CMD_STACKOVERFLOW @@ -397,6 +406,13 @@ DECLARE_CONSOLE_COMMAND(crash, command_crash, #endif " | unaligned | watchdog | hang]", "Crash the system (for testing)"); + +#ifdef TEST_BUILD +int test_command_crash(int argc, const char **argv) +{ + return command_crash(argc, argv); +} +#endif /* TEST_BUILD*/ #endif /* CONFIG_CMD_CRASH */ static int command_panicinfo(int argc, const char **argv) diff --git a/include/panic.h b/include/panic.h index efd3e4d86b..44f21c7ab5 100644 --- a/include/panic.h +++ b/include/panic.h @@ -325,4 +325,20 @@ void chip_panic_data_backup(void); } #endif +#ifdef TEST_BUILD +/** + * @brief Wrapper for accessing the command_crash() console command + * implementation directly in unit tests. It cannot be called normally through + * the shell interface because it upsets the shell's internal state when the + * command doesn't return after a crash. command_crash() cannot be marked + * test_export_static directly due to an implementation detail in + * DECLARE_CONSOLE_COMMAND(). + * + * @param argc Number of CLI args in `argv` + * @param argv CLI arguments + * @return int Return value + */ +int test_command_crash(int argc, const char **argv); +#endif /* TEST_BUILD*/ + #endif /* __CROS_EC_PANIC_H */ diff --git a/zephyr/test/drivers/CMakeLists.txt b/zephyr/test/drivers/CMakeLists.txt index 78c1274843..ffe3733762 100644 --- a/zephyr/test/drivers/CMakeLists.txt +++ b/zephyr/test/drivers/CMakeLists.txt @@ -25,6 +25,7 @@ add_subdirectory_ifdef(CONFIG_LINK_TEST_SUITE_I2C_CONTROLLER i2c_controller) add_subdirectory_ifdef(CONFIG_LINK_TEST_SUITE_KEYBOARD_SCAN keyboard_scan) add_subdirectory_ifdef(CONFIG_LINK_TEST_SUITE_LED_DRIVER led_driver) add_subdirectory_ifdef(CONFIG_LINK_TEST_SUITE_MKBP mkbp) +add_subdirectory_ifdef(CONFIG_LINK_TEST_SUITE_PANIC_OUTPUT panic_output) add_subdirectory_ifdef(CONFIG_LINK_TEST_SUITE_POWER_HOST_SLEEP power_host_sleep) add_subdirectory_ifdef(CONFIG_LINK_TEST_SUITE_RT9490 rt9490) add_subdirectory_ifdef(CONFIG_LINK_TEST_SUITE_SHIM_GPIO_ID shim_gpio_id) diff --git a/zephyr/test/drivers/Kconfig b/zephyr/test/drivers/Kconfig index b2a9648f21..e5c66946c0 100644 --- a/zephyr/test/drivers/Kconfig +++ b/zephyr/test/drivers/Kconfig @@ -62,6 +62,9 @@ config LINK_TEST_SUITE_LOCATE_CHIP_ALTS config LINK_TEST_SUITE_MKBP bool "Link and test the mkbp tests" +config LINK_TEST_SUITE_PANIC_OUTPUT + bool "Link and test the panic_output tests" + config LINK_TEST_SUITE_POWER_HOST_SLEEP bool "Link and run the power/host_sleep.c specific tests" diff --git a/zephyr/test/drivers/panic_output/CMakeLists.txt b/zephyr/test/drivers/panic_output/CMakeLists.txt new file mode 100644 index 0000000000..79cf21fc5f --- /dev/null +++ b/zephyr/test/drivers/panic_output/CMakeLists.txt @@ -0,0 +1,8 @@ +# Copyright 2022 The ChromiumOS Authors +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +# Add source files +target_sources(app PRIVATE + src/panic_output_crash_cmd.c +) diff --git a/zephyr/test/drivers/panic_output/src/panic_output_crash_cmd.c b/zephyr/test/drivers/panic_output/src/panic_output_crash_cmd.c new file mode 100644 index 0000000000..9608869609 --- /dev/null +++ b/zephyr/test/drivers/panic_output/src/panic_output_crash_cmd.c @@ -0,0 +1,167 @@ +/* Copyright 2022 The ChromiumOS Authors + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include +#include +#include +#include +#include +#include +#include + +#include "console.h" +#include "panic.h" +#include "test/drivers/test_state.h" + +/* Special return codes for run_crash_command() */ +#define RETURN_CODE_CRASHED (-1) +#define RETURN_CODE_TIMEOUT (-2) + +/** Create a new thread for running the `crash` console command. As its name + * suggests, this command causes a number of fatal errors (e.g. divide by zero). + * Run it in a separate thread so that we can observe these crashes without + * causing the ztest thread that runs the test functions to get aborted. + */ +static struct k_thread crash_thread; + +/* Create a stack for the crash thread */ +K_THREAD_STACK_DEFINE(crash_thread_stack, 1024); + +/** Captures the last signal number received by `handle_signal()`. */ +static int signal_received; + +/** + * @brief Handler for signals sent to the process. Log the signal number and + * abort the crash test. + * + * @param sig Signal number, from signal.h + */ +void handle_signal(int sig) +{ + signal_received = sig; + + k_thread_abort(&crash_thread); +} + +/** + * @brief Worker function for the crash thread that calls the `crash` console + * command. + * + * @param a The value of `argc` (forced as a pointer type). + * @param b `argv` pointer. + * @param c Pointer to location to write the return value. + */ +static void crash_thread_worker(void *a, void *b, void *c) +{ + int argc = (int)a; + const char **argv = (const char **)b; + int *return_val = c; + + /* If the return value remains RETURN_CODE_CRASHED, then the command + * did not return. + */ + *return_val = RETURN_CODE_CRASHED; + signal_received = 0; + + *return_val = test_command_crash(argc, argv); +} + +/** + * @brief Helper function to spawn a new thread that runs the `crash` console + * command. Waits for the command to exit/fail. + * + * @param argc Number of elements in `argv` + * @param argv CLI arguments to pass to command. Include the command name. + * @param timeout Time to wait for the thread to exit, in milliseconds. + * @return int Console command's return value, or RETURN_CODE_CRASHED if caused + * a fatal error, or RETURN_CODE_TIMEOUT if it hung. + */ +static int run_crash_command(int argc, const char **argv, k_timeout_t timeout) +{ + static int return_val; + + k_thread_create(&crash_thread, crash_thread_stack, + K_THREAD_STACK_SIZEOF(crash_thread_stack), + (k_thread_entry_t)crash_thread_worker, (void *)argc, + argv, &return_val, CONFIG_ZTEST_THREAD_PRIORITY + 1, + K_INHERIT_PERMS, K_NO_WAIT); + + if (k_thread_join(&crash_thread, timeout) == -EAGAIN) { + k_thread_abort(&crash_thread); + return RETURN_CODE_TIMEOUT; + } + + return return_val; +} + +ZTEST(panic_output, test_console_cmd__unaligned) +{ + int rv; + const char *cmd[] = { "crash", "unaligned" }; + + rv = run_crash_command(2, cmd, K_FOREVER); + + zassert_equal(RETURN_CODE_CRASHED, rv, + "Command returned %d but shouldn't have exited", rv); + zassert_equal(SIGSEGV, signal_received); +} + +ZTEST(panic_output, test_console_cmd__watchdog) +{ + /* Note: this does not verify that the watchdog fired, but that is + * covered in a different test suite. + */ + + int rv; + const char *cmd[] = { "crash", "watchdog" }; + + rv = run_crash_command(2, cmd, K_MSEC(100)); + + zassert_equal(RETURN_CODE_TIMEOUT, rv, + "Command returned %d but shouldn't have exited", rv); +} + +ZTEST(panic_output, test_console_cmd__hang) +{ + int rv; + const char *cmd[] = { "crash", "hang" }; + + rv = run_crash_command(2, cmd, K_MSEC(100)); + + zassert_equal(RETURN_CODE_TIMEOUT, rv, + "Command returned %d but shouldn't have exited", rv); +} + +ZTEST(panic_output, test_console_cmd__bad_param) +{ + int rv; + const char *cmd[] = { "crash", "xyz" }; + + rv = run_crash_command(2, cmd, K_FOREVER); + + zassert_equal(EC_ERROR_PARAM1, rv, "Command returned %d", rv); +} + +ZTEST(panic_output, test_console_cmd__no_param) +{ + int rv; + const char *cmd[] = { "crash" }; + + rv = run_crash_command(1, cmd, K_FOREVER); + + zassert_equal(EC_ERROR_PARAM1, rv, "Command returned %d", rv); +} + +static void reset(void *data) +{ + ARG_UNUSED(data); + + signal(SIGSEGV, handle_signal); + signal(SIGFPE, handle_signal); + signal_received = 0; +} + +ZTEST_SUITE(panic_output, drivers_predicate_post_main, NULL, reset, reset, + NULL); diff --git a/zephyr/test/drivers/testcase.yaml b/zephyr/test/drivers/testcase.yaml index 68e77fab94..39c2fe12f3 100644 --- a/zephyr/test/drivers/testcase.yaml +++ b/zephyr/test/drivers/testcase.yaml @@ -145,6 +145,11 @@ tests: tags: common mkbp + drivers.panic_output: + extra_configs: + - CONFIG_LINK_TEST_SUITE_PANIC_OUTPUT=y + - CONFIG_PLATFORM_EC_CONSOLE_CMD_CRASH=y + - CONFIG_ZTEST_THREAD_PRIORITY=1 drivers.power_host_sleep: extra_configs: - CONFIG_LINK_TEST_SUITE_POWER_HOST_SLEEP=y -- cgit v1.2.1