diff options
author | Rob Barnes <robbarnes@google.com> | 2023-02-01 08:45:25 -0700 |
---|---|---|
committer | Chromeos LUCI <chromeos-scoped@luci-project-accounts.iam.gserviceaccount.com> | 2023-02-16 01:39:28 +0000 |
commit | 612895a4520f4199e3ef1634394a06218e24f933 (patch) | |
tree | 78b86ef5f38f67c40585b1c783c6ab9700833fcb | |
parent | 654c7f10f0447d04cdafee9ea19f887f964d8940 (diff) | |
download | chrome-ec-612895a4520f4199e3ef1634394a06218e24f933.tar.gz |
system_safe_mode: Use common method for identifying critical tasks
Legacy EC uses tasks, Zephyr uses threads, so separate methods were used
to identify critical tasks/threads.
This change switches Zephyr EC to use task ids to identify critical
tasks. This allows some duplicate code to be removed and avoids the
CONFIG_THREAD_MONITOR dependency. This relies on the tasks shim for
mapping between task ids and thread ids.
BUG=b:266696987
BRANCH=None
TEST=Observe threads being disabled when safe mode runs
Change-Id: I87bcb7de87b0575cfc5d90bd12b21314ddeea093
Signed-off-by: Rob Barnes <robbarnes@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4214562
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
-rw-r--r-- | common/system_safe_mode.c | 29 | ||||
-rw-r--r-- | include/system_safe_mode.h | 8 | ||||
-rw-r--r-- | zephyr/Kconfig.system | 2 | ||||
-rw-r--r-- | zephyr/shim/src/system_safe_mode.c | 54 | ||||
-rw-r--r-- | zephyr/test/system_safe_mode/prj.conf | 1 |
5 files changed, 42 insertions, 52 deletions
diff --git a/common/system_safe_mode.c b/common/system_safe_mode.c index e86aa5216f..bced1bd24e 100644 --- a/common/system_safe_mode.c +++ b/common/system_safe_mode.c @@ -26,17 +26,24 @@ static const int safe_mode_allowed_hostcmds[] = { EC_CMD_GET_UPTIME_INFO }; -#ifndef CONFIG_ZEPHYR - -/* TODO: This function can be generalized for zephyr and legacy EC by - * improving ec_tasks support in zephyr. - */ -static bool task_is_safe_mode_critical(task_id_t task_id) +bool is_task_safe_mode_critical(task_id_t task_id) { const task_id_t safe_mode_critical_tasks[] = { +#ifdef HAS_TASK_HOOK TASK_ID_HOOKS, +#endif +#ifdef HAS_TASK_IDLE TASK_ID_IDLE, +#endif +#ifdef HAS_TASK_HOSTCMD TASK_ID_HOSTCMD, +#endif +#ifdef HAS_TASK_MAIN + TASK_ID_MAIN, +#endif +#ifdef HAS_TASK_SYSWORKQ + TASK_ID_SYSWORKQ, +#endif }; for (int i = 0; i < ARRAY_SIZE(safe_mode_critical_tasks); i++) if (safe_mode_critical_tasks[i] == task_id) @@ -44,15 +51,17 @@ static bool task_is_safe_mode_critical(task_id_t task_id) return false; } -bool current_task_is_safe_mode_critical(void) +bool is_current_task_safe_mode_critical(void) { - return task_is_safe_mode_critical(task_get_current()); + return is_task_safe_mode_critical(task_get_current()); } +#ifndef CONFIG_ZEPHYR + int disable_non_safe_mode_critical_tasks(void) { for (task_id_t task_id = 0; task_id < TASK_ID_COUNT; task_id++) { - if (!task_is_safe_mode_critical(task_id)) { + if (!is_task_safe_mode_critical(task_id)) { task_disable_task(task_id); } } @@ -108,7 +117,7 @@ int start_system_safe_mode(void) return EC_ERROR_INVAL; } - if (current_task_is_safe_mode_critical()) { + if (is_current_task_safe_mode_critical()) { /* TODO: Restart critical tasks */ panic_printf( "Fault in critical task, cannot enter system safe mode\n"); diff --git a/include/system_safe_mode.h b/include/system_safe_mode.h index a8bf23d5b0..6c8909bf43 100644 --- a/include/system_safe_mode.h +++ b/include/system_safe_mode.h @@ -6,6 +6,8 @@ #ifndef __CROS_EC_SYSTEM_SAFE_MODE_H #define __CROS_EC_SYSTEM_SAFE_MODE_H +#include "task.h" + /** * Checks if running in system safe mode * @@ -21,11 +23,11 @@ bool system_is_in_safe_mode(void); bool command_is_allowed_in_safe_mode(int command); /** - * Checks if current task critical for system safe mode + * Checks if a task is critical for system safe mode * - * @return True if current task is safe mode critical + * @return True if task is safe mode critical */ -bool current_task_is_safe_mode_critical(void); +bool is_task_safe_mode_critical(task_id_t task_id); /** * Disables tasks that are not critical for safe mode diff --git a/zephyr/Kconfig.system b/zephyr/Kconfig.system index 226832cb63..1db45ef2a8 100644 --- a/zephyr/Kconfig.system +++ b/zephyr/Kconfig.system @@ -85,7 +85,7 @@ config PLATFORM_EC_FW_RESET_VECTOR config PLATFORM_EC_SYSTEM_SAFE_MODE bool "Enable System Safe Mode" default n - depends on (CPU_CORTEX_M || ARCH_POSIX) && !TASK_HOSTCMD_THREAD_MAIN + depends on (CPU_CORTEX_M || ARCH_POSIX) help This enables system safe mode for retrieving system state after a panic occurs. System safe mode only runs briefly after a panic. diff --git a/zephyr/shim/src/system_safe_mode.c b/zephyr/shim/src/system_safe_mode.c index ec52f210df..0acb5b270f 100644 --- a/zephyr/shim/src/system_safe_mode.c +++ b/zephyr/shim/src/system_safe_mode.c @@ -13,46 +13,24 @@ #include <zephyr/kernel.h> #include <zephyr/kernel/thread.h> -static bool thread_is_safe_mode_critical(const struct k_thread *thread) -{ - /* List of safe mode critical tasks */ - static const char *safe_mode_critical_threads[] = { - "main", - "sysworkq", - "idle", - "HOSTCMD", - }; - for (int i = 0; i < ARRAY_SIZE(safe_mode_critical_threads); i++) - if (strcmp(thread->name, safe_mode_critical_threads[i]) == 0) - return true; - return false; -} - -__override bool current_task_is_safe_mode_critical(void) -{ - return thread_is_safe_mode_critical(k_current_get()); -} - -static void abort_non_critical_threads_cb(const struct k_thread *thread, - void *user_data) -{ - ARG_UNUSED(user_data); - - /* - * Don't abort if thread is critical or current thread. - * Current thread will be canceled automatically after returning from - * exception handler. - */ - if (thread_is_safe_mode_critical(thread) || k_current_get() == thread) - return; - - printk("Aborting thread %s\n", thread->name); - k_thread_abort((struct k_thread *)thread); -} - __override int disable_non_safe_mode_critical_tasks(void) { - k_thread_foreach(abort_non_critical_threads_cb, NULL); + for (task_id_t task_id = 0; task_id < TASK_ID_COUNT + EXTRA_TASK_COUNT; + task_id++) { + k_tid_t thread_id = task_id_to_thread_id(task_id); + + /* Don't abort the thread when: + * 1) The task to thread lookup failed + * 2) It's the current thread since it will be aborted + * automatically by the Zephyr kernel. + * 3) The thread is safe mode critical + */ + if (thread_id == NULL || thread_id == k_current_get() || + is_task_safe_mode_critical(task_id)) { + continue; + } + k_thread_abort(thread_id); + } return EC_SUCCESS; } diff --git a/zephyr/test/system_safe_mode/prj.conf b/zephyr/test/system_safe_mode/prj.conf index f7aeb4b776..6ecdbe1b16 100644 --- a/zephyr/test/system_safe_mode/prj.conf +++ b/zephyr/test/system_safe_mode/prj.conf @@ -12,6 +12,7 @@ CONFIG_PLATFORM_EC_SYSTEM_SAFE_MODE=y CONFIG_TASK_HOSTCMD_THREAD_DEDICATED=y CONFIG_PLATFORM_EC_HOSTCMD=y CONFIG_ASSERT_TEST=y +CONFIG_SHIMMED_TASKS=y # Disable because not needed CONFIG_PLATFORM_EC_BACKLIGHT_LID=n |