From e09a0255da7f10d288c71c51354368dfb355aa12 Mon Sep 17 00:00:00 2001
From: Andrew Boie <andrew.p.boie@intel.com>
Date: Wed, 6 Nov 2019 14:17:17 -0800
Subject: [PATCH] kernel: sychronize irq_offload() access

Entering irq_offload() on multiple CPUs can cause
difficult to debug/reproduce crashes. Demote irq_offload()
to non-inline (it never needed to be inline anyway) and
wrap the arch call in a semaphore.

Some tests which were unnecessarily killing threads
have been fixed; these threads exit by themselves anyway
and we won't leave the semaphore dangling.

The definition of z_arch_irq_offload() moved to
arch_interface.h as it only gets called by kernel C code.

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
---
 include/irq_offload.h                       |  9 +++------
 include/sys/arch_interface.h                |  1 +
 kernel/thread.c                             | 12 ++++++++++++
 tests/kernel/mem_protect/sys_sem/src/main.c |  6 ------
 tests/kernel/semaphore/semaphore/src/main.c |  2 --
 5 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/include/irq_offload.h b/include/irq_offload.h
index d308e3e44c0..29ce19b945e 100644
--- a/include/irq_offload.h
+++ b/include/irq_offload.h
@@ -15,9 +15,9 @@
 extern "C" {
 #endif
 
-#include <arch/cpu.h>
-
 #ifdef CONFIG_IRQ_OFFLOAD
+typedef void (*irq_offload_routine_t)(void *parameter);
+
 /**
  * @brief Run a function in interrupt context
  *
@@ -30,10 +30,7 @@ extern "C" {
  * @param parameter Argument to pass to the function when it is run as an
  * interrupt
  */
-static inline void irq_offload(irq_offload_routine_t routine, void *parameter)
-{
-	arch_irq_offload(routine, parameter);
-}
+void irq_offload(irq_offload_routine_t routine, void *parameter);
 #endif
 
 #ifdef __cplusplus
diff --git a/include/sys/arch_interface.h b/include/sys/arch_interface.h
index 6fe2c56bc91..4e82744ffda 100644
--- a/include/sys/arch_interface.h
+++ b/include/sys/arch_interface.h
@@ -31,6 +31,7 @@
 #include <stddef.h>
 #include <zephyr/types.h>
 #include <arch/cpu.h>
+#include <irq_offload.h>
 
 #ifdef __cplusplus
 extern "C" {
diff --git a/kernel/thread.c b/kernel/thread.c
index b163d489796..a45b30010df 100644
--- a/kernel/thread.c
+++ b/kernel/thread.c
@@ -26,6 +26,7 @@
 #include <debug/tracing.h>
 #include <string.h>
 #include <stdbool.h>
+#include <irq_offload.h>
 
 static struct k_spinlock lock;
 
@@ -903,3 +904,14 @@ static inline void z_vrfy_k_thread_abort(k_tid_t thread)
 #include <syscalls/k_thread_abort_mrsh.c>
 
 #endif /* CONFIG_USERSPACE */
+
+#ifdef CONFIG_IRQ_OFFLOAD
+static K_SEM_DEFINE(offload_sem, 1, 1);
+
+void irq_offload(irq_offload_routine_t routine, void *parameter)
+{
+	k_sem_take(&offload_sem, K_FOREVER);
+	arch_irq_offload(routine, parameter);
+	k_sem_give(&offload_sem);
+}
+#endif
diff --git a/tests/kernel/mem_protect/sys_sem/src/main.c b/tests/kernel/mem_protect/sys_sem/src/main.c
index 0b4b94bf472..085569423aa 100644
--- a/tests/kernel/mem_protect/sys_sem/src/main.c
+++ b/tests/kernel/mem_protect/sys_sem/src/main.c
@@ -283,8 +283,6 @@ void test_sem_take_timeout(void)
 	ret_value = sys_sem_take(&simple_sem, SEM_TIMEOUT);
 	zassert_true(ret_value == 0,
 		     "sys_sem_take failed when its shouldn't have");
-
-	k_thread_abort(&sem_tid);
 }
 
 /**
@@ -309,8 +307,6 @@ void test_sem_take_timeout_forever(void)
 	ret_value = sys_sem_take(&simple_sem, K_FOREVER);
 	zassert_true(ret_value == 0,
 		     "sys_sem_take failed when its shouldn't have");
-
-	k_thread_abort(&sem_tid);
 }
 
 /**
@@ -329,8 +325,6 @@ void test_sem_take_timeout_isr(void)
 	ret_value = sys_sem_take(&simple_sem, SEM_TIMEOUT);
 	zassert_true(ret_value == 0,
 		     "sys_sem_take failed when its shouldn't have");
-
-	k_thread_abort(&sem_tid);
 }
 
 /**
diff --git a/tests/kernel/semaphore/semaphore/src/main.c b/tests/kernel/semaphore/semaphore/src/main.c
index fadc60f8c48..fbc3c9751e7 100644
--- a/tests/kernel/semaphore/semaphore/src/main.c
+++ b/tests/kernel/semaphore/semaphore/src/main.c
@@ -326,8 +326,6 @@ void test_sem_take_timeout_isr(void)
 	ret_value = k_sem_take(&simple_sem, SEM_TIMEOUT);
 	zassert_true(ret_value == 0,
 		     "k_sem_take failed when its shouldn't have");
-	k_thread_abort(&sem_tid);
-
 }
 
 /**