diff options
author | Keith Short <keithshort@chromium.org> | 2021-05-07 11:28:03 -0600 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2021-05-10 17:11:51 +0000 |
commit | 0eb3a9b7b4df6bc4ac2ccdcc6ce979d225bc064a (patch) | |
tree | e4fdcbdf7d4c3dbbdc9e22d59e545fb694fe0856 | |
parent | e7e768c08a09d09507a47d8e4127e7222d2241c8 (diff) | |
download | chrome-ec-0eb3a9b7b4df6bc4ac2ccdcc6ce979d225bc064a.tar.gz |
zephyr: Set system workqueue priority to preemptive
The deferred tasks are now run as part of the workqueue (see
CL:2854850). By default, Zephyr runs the workqueue as the lowest
priority cooperative thread, which prevents preemption by any other
thread. This blocked the PD tasks from servicing new PD requests,
causing spec violations.
Change the system workqueue priority to be the lowest preemptive thread
priority. This is done at runtime because the Kconfig system doesn't
support setting integer values using other configuration options.
BUG=b:187173381
BRANCH=none
TEST=Volteer connect charger. Verify SrcCap response takes 23 ms.
Signed-off-by: Keith Short <keithshort@chromium.org>
Change-Id: I95d024a510a743e967fcc9bd10e3acb6c36e8b77
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2880716
Reviewed-by: Simon Glass <sjg@chromium.org>
-rw-r--r-- | zephyr/app/ec/ec_app_main.c | 9 | ||||
-rw-r--r-- | zephyr/shim/include/ec_tasks.h | 12 | ||||
-rw-r--r-- | zephyr/shim/src/hooks.c | 16 |
3 files changed, 36 insertions, 1 deletions
diff --git a/zephyr/app/ec/ec_app_main.c b/zephyr/app/ec/ec_app_main.c index 3853dc58fc..c792a2892f 100644 --- a/zephyr/app/ec/ec_app_main.c +++ b/zephyr/app/ec/ec_app_main.c @@ -3,6 +3,7 @@ * found in the LICENSE file. */ +#include <kernel.h> #include <sys/printk.h> #include <zephyr.h> @@ -75,6 +76,14 @@ void ec_app_main(void) vboot_main(); } + /* + * Hooks run from the system workqueue and must be the lowest priority + * thread. By default, the system workqueue is run at the lowest + * cooperative thread priority, blocking all preemptive threads until + * the deferred work is completed. + */ + k_thread_priority_set(&k_sys_work_q.thread, LOWEST_THREAD_PRIORITY); + /* Call init hooks before main tasks start */ if (IS_ENABLED(CONFIG_PLATFORM_EC_HOOKS)) { hook_notify(HOOK_INIT); diff --git a/zephyr/shim/include/ec_tasks.h b/zephyr/shim/include/ec_tasks.h index 0380920492..1f050302a0 100644 --- a/zephyr/shim/include/ec_tasks.h +++ b/zephyr/shim/include/ec_tasks.h @@ -6,7 +6,17 @@ #ifndef __CROS_EC_EC_TASKS_H #define __CROS_EC_EC_TASKS_H +/* + * The lowest preemptive thread priority is (CONFIG_NUM_PREEMT_PRIORITIES-1) + * while the lowest cooperative thread priority is -1. + * + * https://docs.zephyrproject.org/latest/reference/kernel/threads/index.html#thread-priorities + */ +#define LOWEST_THREAD_PRIORITY \ + COND_CODE_1(CONFIG_PREEMPT_ENABLED, \ + (CONFIG_NUM_PREEMPT_PRIORITIES - 1), (-1)) + /** Starts all of the shimmed EC tasks. Requires CONFIG_SHIMMED_TASKS=y. */ void start_ec_tasks(void); -#endif /* __CROS_EC_EC_TASKS_H */ +#endif /* __CROS_EC_EC_TASKS_H */ diff --git a/zephyr/shim/src/hooks.c b/zephyr/shim/src/hooks.c index 6630e970fa..20f3f84704 100644 --- a/zephyr/shim/src/hooks.c +++ b/zephyr/shim/src/hooks.c @@ -8,6 +8,7 @@ #include "common.h" #include "console.h" +#include "ec_tasks.h" #include "hooks.h" #include "task.h" #include "timer.h" @@ -61,12 +62,27 @@ void hook_notify(enum hook_type type) p->routine(); } +static void check_hook_task_priority(k_tid_t thread) +{ + if (k_thread_priority_get(thread) != LOWEST_THREAD_PRIORITY) + cprintf(CC_HOOK, + "ERROR: %s has priority %d but must be priority %d\n", + k_thread_name_get(thread), + k_thread_priority_get(thread), LOWEST_THREAD_PRIORITY); +} + void hook_task(void *u) { /* Periodic hooks will be called first time through the loop */ static uint64_t last_second = -SECOND; static uint64_t last_tick = -HOOK_TICK_INTERVAL; + /* + * Verify deferred routines are run at the lowest priority. + */ + check_hook_task_priority(&k_sys_work_q.thread); + check_hook_task_priority(k_current_get()); + while (1) { uint64_t t = get_time().val; int next = 0; |