From bc257547d8d819af64d6e90341900e23030151c0 Mon Sep 17 00:00:00 2001 From: Patryk Duda Date: Wed, 9 Jun 2021 15:35:29 +0200 Subject: cortex-m0/task: Check if interrupts are enabled before switching task Switching task with disabled interrupts leads to Hard Fault on Cortex-M0 because: - SVCall exception have configurable priority (full list can be found at 2.3.2 Exception types PM0215 p.23) - We are using 'cpsid i' to disable interrupts. This instruction sets PRIMASK bit (3.7.2 CPSID CPSIE PM0215 p.62) - When PRIMASK bit is set, all exceptions with configurable priority are disabled (PM0215 p.16), so SVCall is masked too If Hard Fault is inevitable, it will be a good idea to catch this earlier. It will save time spent debugging why Forced Hard Fault happens. In functions responsible for enabling, disabling or making task ready we postpone task switch when interrupts are disabled BUG=b:190597666 BRANCH=none TEST=Compile and flash EC on boards with Cortex-M0 and make sure that it works properly. Signed-off-by: Patryk Duda Change-Id: Id3be74e977ae5d5eed79aad78ee378fa413ed4ee Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2953229 Commit-Queue: Marcin Wojtas Tested-by: Patryk Duda Reviewed-by: Aseda Aboagye --- core/cortex-m0/task.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) (limited to 'core') diff --git a/core/cortex-m0/task.c b/core/cortex-m0/task.c index 5c9893ace8..ba40b667b6 100644 --- a/core/cortex-m0/task.c +++ b/core/cortex-m0/task.c @@ -336,6 +336,12 @@ static uint32_t __wait_evt(int timeout_us, task_id_t resched) uint32_t evt; int ret __attribute__((unused)); + /* + * Scheduling task when interrupts are disabled will result in Forced + * Hard Fault because disabling interrupt using 'cpsid i' also disables + * SVCall handler (because it has configurable priority) + */ + ASSERT(is_interrupt_enabled()); ASSERT(!in_interrupt_context()); if (timeout_us > 0) { @@ -373,7 +379,7 @@ uint32_t task_set_event(task_id_t tskid, uint32_t event) atomic_or(&receiver->events, event); /* Re-schedule if priorities have changed */ - if (in_interrupt_context()) { + if (in_interrupt_context() || !is_interrupt_enabled()) { /* The receiver might run again */ atomic_or(&tasks_ready, 1 << tskid); if (start_called) { @@ -436,7 +442,8 @@ void task_enable_all_tasks(void) /* Mark all tasks as ready and able to run. */ tasks_ready = tasks_enabled = BIT(TASK_ID_COUNT) - 1; /* Reschedule the highest priority task. */ - __schedule(0, 0); + if (is_interrupt_enabled()) + __schedule(0, 0); } void task_enable_task(task_id_t tskid) @@ -448,7 +455,8 @@ void task_disable_task(task_id_t tskid) { atomic_clear_bits(&tasks_enabled, BIT(tskid)); - if (!in_interrupt_context() && tskid == task_get_current()) + if (!in_interrupt_context() && is_interrupt_enabled() && + tskid == task_get_current()) __schedule(0, 0); } -- cgit v1.2.1