From f6ee7f289245c8ffd83e4e80b5aea67eda3bdd8f Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Tue, 5 May 2026 20:13:26 -0600 Subject: [PATCH 1/5] feat: k_timer ISR dispatch; align callback context with upstream Zephyr --- components/zkernel/Kconfig | 13 +++ .../zkernel/include/boreas/zephyr/kernel.h | 44 +++++---- components/zkernel/src/k_thread.c | 34 ++++++- components/zkernel/src/k_timer.c | 19 ++-- components/zkernel/src/k_work.c | 5 ++ components/zsys/src/watchdog.c | 18 +++- examples/work_queue_demo/main/main.c | 15 +++- test/main/test_k_timer.c | 89 ++++++++++++++++++- test/sdkconfig.defaults | 5 +- 9 files changed, 205 insertions(+), 37 deletions(-) diff --git a/components/zkernel/Kconfig b/components/zkernel/Kconfig index d3eef97..154fd10 100644 --- a/components/zkernel/Kconfig +++ b/components/zkernel/Kconfig @@ -63,6 +63,19 @@ menu "Boreas Kernel (zkernel)" responsiveness for equal-or-lower-priority work. Mirrors upstream Zephyr's CONFIG_SYSTEM_WORKQUEUE_NO_YIELD. + config K_TIMER_DISPATCH_ISR + bool "Dispatch k_timer callbacks in ISR context" + default y + select ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD + help + When enabled, k_timer expiry callbacks run in true ISR context + (ESP_TIMER_ISR dispatch), matching upstream Zephyr's contract. + Callbacks must be IRAM_ATTR and must not call blocking APIs. + + When disabled, callbacks run on the ESP_TIMER_TASK (a normal + FreeRTOS task). Blocking calls are permitted but the behavior + diverges from upstream Zephyr. + config ZKERNEL_FATAL_CAPTURE bool "Capture fatal error context to NVS" default n diff --git a/components/zkernel/include/boreas/zephyr/kernel.h b/components/zkernel/include/boreas/zephyr/kernel.h index b66f6d6..c9ac0b4 100644 --- a/components/zkernel/include/boreas/zephyr/kernel.h +++ b/components/zkernel/include/boreas/zephyr/kernel.h @@ -302,21 +302,24 @@ struct k_timer; /** * Timer expiry function type. * - * @warning Callback context divergence from upstream Zephyr. - * Upstream Zephyr documents this callback as running in - * "system clock interrupt handler" context. Boreas dispatches - * via esp_timer with ESP_TIMER_TASK, which is a normal - * FreeRTOS task -- NOT an ISR. Practical consequences: - * - Blocking calls (k_sem_take, k_mutex_lock, k_msleep) are - * permitted from this callback. Upstream forbids them. - * - There is no K_ISR_LOCK-equivalent guarantee. The - * callback IS preemptible by higher-priority tasks. - * - Code ported from upstream that assumes ISR semantics - * (e.g. relying on irq_lock for atomicity vs. user code) - * must be reviewed. - * - * Code that needs both behaviors should not assume either -- - * use k_sem / k_mutex for synchronization regardless. + * @warning Callback context — matches upstream Zephyr when + * CONFIG_K_TIMER_DISPATCH_ISR=y (the default). + * + * **Default (ISR dispatch):** Callback runs in true ISR context + * via ESP_TIMER_ISR. The function must be declared IRAM_ATTR + * and must be ISR-safe: + * - Must NOT call blocking APIs (k_sem_take with timeout, + * k_mutex_lock, k_msleep, k_thread_join). + * - The following are ISR-safe and may be called: + * k_sem_give, k_work_submit, k_event_set/post/clear, + * k_msgq_put (K_NO_WAIT), LOG_* (with + * CONFIG_ZSYS_LOG_MODE_DEFERRED=y only). + * - Must not call malloc, printf, or any flash-resident + * function (IRAM_ATTR is required for cache survival). + * + * **Fallback (CONFIG_K_TIMER_DISPATCH_ISR=n):** Callback runs + * on the ESP_TIMER_TASK, a normal FreeRTOS task. Blocking calls + * are permitted. This diverges from upstream Zephyr. */ typedef void (*k_timer_expiry_t)(struct k_timer *timer); @@ -400,8 +403,15 @@ k_ticks_t k_timer_remaining_ticks(const struct k_timer *timer); */ k_ticks_t k_timer_expires_ticks(const struct k_timer *timer); -void k_timer_user_data_set(struct k_timer *timer, void *user_data); -void *k_timer_user_data_get(struct k_timer *timer); +static inline void k_timer_user_data_set(struct k_timer *timer, void *user_data) +{ + timer->user_data = user_data; +} + +static inline void *k_timer_user_data_get(struct k_timer *timer) +{ + return timer->user_data; +} /* ---------------------------------------------------------------- * Work Queue diff --git a/components/zkernel/src/k_thread.c b/components/zkernel/src/k_thread.c index 310e7b3..23b26db 100644 --- a/components/zkernel/src/k_thread.c +++ b/components/zkernel/src/k_thread.c @@ -6,6 +6,9 @@ #include "zephyr/kernel.h" #include +#include "esp_attr.h" +#include "freertos/timers.h" +#include "sdkconfig.h" /* Trampoline: adapts Zephyr's 3-arg entry to FreeRTOS's 1-arg entry. * If _start_suspended is set, suspends self before calling entry @@ -24,6 +27,27 @@ static void k_thread_entry_wrapper(void *arg) vTaskSuspend(NULL); } +#ifdef CONFIG_K_TIMER_DISPATCH_ISR +static void k_thread_delay_resume_pended(void *param, uint32_t unused) +{ + (void)unused; + struct k_thread *thread = (struct k_thread *)param; + if (thread && thread->handle) { + vTaskResume(thread->handle); + } +} + +static void IRAM_ATTR k_thread_delay_expiry(struct k_timer *timer) +{ + struct k_thread *thread = (struct k_thread *)k_timer_user_data_get(timer); + if (thread) { + BaseType_t woken = pdFALSE; + xTimerPendFunctionCallFromISR(k_thread_delay_resume_pended, + thread, 0, &woken); + portYIELD_FROM_ISR(woken); + } +} +#else static void k_thread_delay_expiry(struct k_timer *timer) { struct k_thread *thread = (struct k_thread *)k_timer_user_data_get(timer); @@ -31,6 +55,7 @@ static void k_thread_delay_expiry(struct k_timer *timer) vTaskResume(thread->handle); } } +#endif k_tid_t k_thread_create(struct k_thread *thread, StackType_t *stack, size_t stack_size, k_thread_entry_t entry, void *p1, void *p2, void *p3, int prio, @@ -49,9 +74,13 @@ k_tid_t k_thread_create(struct k_thread *thread, StackType_t *stack, size_t stac /* Set flag BEFORE creating task so the wrapper sees it immediately */ thread->_start_suspended = !k_timeout_is_no_wait(delay); - thread->handle = xTaskCreateStatic( + /* Pin to core 0. ESP-IDF's vPortCleanUpCoprocArea passes + * tskNO_AFFINITY (0x7FFFFFFF) to _xt_coproc_release, which + * overflows the owner-table index and silently skips the real + * entries — leaving stale FPU ownership that crashes later. */ + thread->handle = xTaskCreateStaticPinnedToCore( k_thread_entry_wrapper, thread->name ? thread->name : "k_thread", - stack_size / sizeof(StackType_t), thread, prio, stack, &thread->tcb); + stack_size / sizeof(StackType_t), thread, prio, stack, &thread->tcb, 0); /* xTaskCreateStatic only fails on programmer error (misaligned * stack, NULL stack pointer, etc.). Match upstream Zephyr's @@ -60,7 +89,6 @@ k_tid_t k_thread_create(struct k_thread *thread, StackType_t *stack, size_t stac __ASSERT(thread->handle != NULL, "k_thread_create: xTaskCreateStatic failed"); if (!k_timeout_is_forever(delay) && !k_timeout_is_no_wait(delay)) { - /* Finite delay: thread self-suspended, set up timer to resume */ k_timer_init(&thread->_delay_timer, k_thread_delay_expiry, NULL); k_timer_user_data_set(&thread->_delay_timer, thread); k_timer_start(&thread->_delay_timer, delay, K_NO_WAIT); diff --git a/components/zkernel/src/k_timer.c b/components/zkernel/src/k_timer.c index 2350953..f8c36c6 100644 --- a/components/zkernel/src/k_timer.c +++ b/components/zkernel/src/k_timer.c @@ -5,11 +5,17 @@ #include "zephyr/kernel.h" +#include "esp_attr.h" #include "esp_log.h" +#include "sdkconfig.h" static const char *TAG = "k_timer"; +#ifdef CONFIG_K_TIMER_DISPATCH_ISR +static void IRAM_ATTR k_timer_esp_callback(void *arg) +#else static void k_timer_esp_callback(void *arg) +#endif { struct k_timer *timer = (struct k_timer *)arg; @@ -50,7 +56,11 @@ void k_timer_init(struct k_timer *timer, k_timer_expiry_t expiry_fn, k_timer_sto const esp_timer_create_args_t args = { .callback = k_timer_esp_callback, .arg = timer, +#ifdef CONFIG_K_TIMER_DISPATCH_ISR + .dispatch_method = ESP_TIMER_ISR, +#else .dispatch_method = ESP_TIMER_TASK, +#endif .name = "k_timer", }; esp_err_t err = esp_timer_create(&args, &timer->handle); @@ -205,12 +215,3 @@ k_ticks_t k_timer_expires_ticks(const struct k_timer *timer) return (k_ticks_t)(expiry / (uint64_t)tick_us); } -void k_timer_user_data_set(struct k_timer *timer, void *user_data) -{ - timer->user_data = user_data; -} - -void *k_timer_user_data_get(struct k_timer *timer) -{ - return timer->user_data; -} diff --git a/components/zkernel/src/k_work.c b/components/zkernel/src/k_work.c index 3471a34..c93448e 100644 --- a/components/zkernel/src/k_work.c +++ b/components/zkernel/src/k_work.c @@ -10,6 +10,7 @@ #include "sdkconfig.h" #include +#include "esp_attr.h" /* Internal queue flag (k_work_q.flags). Not exposed in the public * header because it's an implementation detail. */ @@ -326,7 +327,11 @@ static void __attribute__((constructor)) sys_work_q_auto_init(void) * Delayable Work * ---------------------------------------------------------------- */ +#ifdef CONFIG_K_TIMER_DISPATCH_ISR +static void IRAM_ATTR k_work_delayable_timer_expiry(struct k_timer *timer) +#else static void k_work_delayable_timer_expiry(struct k_timer *timer) +#endif { struct k_work_delayable *dwork = CONTAINER_OF(timer, struct k_work_delayable, timer); if (dwork->queue) { diff --git a/components/zsys/src/watchdog.c b/components/zsys/src/watchdog.c index 8cbddad..bf513a9 100644 --- a/components/zsys/src/watchdog.c +++ b/components/zsys/src/watchdog.c @@ -5,6 +5,8 @@ #include "zsys/watchdog.h" +#include "esp_attr.h" +#include "sdkconfig.h" #include "zsys/log.h" #include "zephyr/kernel.h" @@ -18,10 +20,11 @@ static zsys_watchdog_timeout_cb_t timeout_callback = NULL; static portMUX_TYPE wdt_spinlock = portMUX_INITIALIZER_UNLOCKED; static struct k_timer supervisor_timer; +static struct k_work supervisor_work; -static void supervisor_check(struct k_timer *timer) +static void supervisor_work_handler(struct k_work *work) { - (void)timer; + (void)work; int64_t now = k_uptime_get(); for (int i = 0; i < entry_count; i++) { @@ -43,9 +46,20 @@ static void supervisor_check(struct k_timer *timer) } } +#ifdef CONFIG_K_TIMER_DISPATCH_ISR +static void IRAM_ATTR supervisor_check(struct k_timer *timer) +#else +static void supervisor_check(struct k_timer *timer) +#endif +{ + (void)timer; + k_work_submit(&supervisor_work); +} + void zsys_watchdog_init(k_timeout_t check_interval, zsys_watchdog_timeout_cb_t timeout_cb) { timeout_callback = timeout_cb; + k_work_init(&supervisor_work, supervisor_work_handler); k_timer_init(&supervisor_timer, supervisor_check, NULL); k_timer_start(&supervisor_timer, check_interval, check_interval); LOG_INF("Watchdog supervisor started"); diff --git a/examples/work_queue_demo/main/main.c b/examples/work_queue_demo/main/main.c index 8782421..b2348bd 100644 --- a/examples/work_queue_demo/main/main.c +++ b/examples/work_queue_demo/main/main.c @@ -17,21 +17,31 @@ #include #include +#include LOG_MODULE_REGISTER(demo, LOG_LEVEL_INF); /* ------------------------------------------------------------------- * Example 1: Periodic timer with expiry callback + * + * With CONFIG_K_TIMER_DISPATCH_ISR=y (default), the expiry callback + * runs in ISR context. Use k_work_submit to defer logging to a task. * ---------------------------------------------------------------- */ static int heartbeat_count = 0; +static struct k_work heartbeat_log_work; -static void heartbeat_expiry(struct k_timer *timer) +static void heartbeat_log_handler(struct k_work *work) { - heartbeat_count++; LOG_INF("[Timer] Heartbeat #%d", heartbeat_count); } +static void IRAM_ATTR heartbeat_expiry(struct k_timer *timer) +{ + heartbeat_count++; + k_work_submit(&heartbeat_log_work); +} + static void heartbeat_stop(struct k_timer *timer) { LOG_INF("[Timer] Heartbeat stopped after %d beats", heartbeat_count); @@ -95,6 +105,7 @@ void app_main(void) /* --- Example 1: Periodic timer --- */ LOG_INF("Starting heartbeat timer (every 1s, runs for 5s)..."); + k_work_init(&heartbeat_log_work, heartbeat_log_handler); k_timer_init(&heartbeat_timer, heartbeat_expiry, heartbeat_stop); k_timer_start(&heartbeat_timer, K_SECONDS(1), K_SECONDS(1)); diff --git a/test/main/test_k_timer.c b/test/main/test_k_timer.c index 240c502..4aa535d 100644 --- a/test/main/test_k_timer.c +++ b/test/main/test_k_timer.c @@ -5,11 +5,16 @@ #include "unity.h" #include "zephyr/kernel.h" +#include "sdkconfig.h" + +#include "esp_attr.h" +#include "freertos/FreeRTOS.h" +#include "freertos/portmacro.h" static volatile int timer_count; static volatile int stop_called; -static void test_timer_cb(struct k_timer *timer) +static void IRAM_ATTR test_timer_cb(struct k_timer *timer) { timer_count++; } @@ -296,6 +301,83 @@ static void test_timer_first_interval(void) k_timer_stop(&timer); } +/* ---------------------------------------------------------------- + * ISR dispatch tests (CONFIG_K_TIMER_DISPATCH_ISR) + * ---------------------------------------------------------------- */ + +#ifdef CONFIG_K_TIMER_DISPATCH_ISR + +static volatile bool isr_context_result; + +static void IRAM_ATTR isr_context_check_cb(struct k_timer *timer) +{ + isr_context_result = xPortInIsrContext(); +} + +static void test_timer_callback_in_isr_context(void) +{ + struct k_timer timer; + k_timer_init(&timer, isr_context_check_cb, NULL); + isr_context_result = false; + + k_timer_start(&timer, K_MSEC(10), K_NO_WAIT); + k_msleep(100); + k_timer_stop(&timer); + + TEST_ASSERT_TRUE_MESSAGE(isr_context_result, + "Expiry callback did not run in ISR context"); +} + +extern int _iram_text_start; +extern int _iram_text_end; + +static void test_timer_callback_iram_attr(void) +{ + void *fn = (void *)isr_context_check_cb; + TEST_ASSERT_TRUE_MESSAGE( + (fn >= (void *)&_iram_text_start && fn < (void *)&_iram_text_end), + "IRAM_ATTR callback is not in IRAM address range"); +} + +static struct k_sem isr_test_sem; +static struct k_work isr_test_work; +static volatile bool isr_work_ran; + +static void IRAM_ATTR isr_safe_apis_cb(struct k_timer *timer) +{ + k_sem_give(&isr_test_sem); + k_work_submit(&isr_test_work); +} + +static void isr_test_work_handler(struct k_work *work) +{ + isr_work_ran = true; +} + +static void test_timer_isr_safe_apis(void) +{ + struct k_timer timer; + + k_sem_init(&isr_test_sem, 0, 1); + k_work_init(&isr_test_work, isr_test_work_handler); + isr_work_ran = false; + + k_timer_init(&timer, isr_safe_apis_cb, NULL); + k_timer_start(&timer, K_MSEC(10), K_NO_WAIT); + + /* k_sem_take blocks until the ISR callback gives the sem. */ + int ret = k_sem_take(&isr_test_sem, K_MSEC(500)); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, "k_sem_give from ISR did not wake waiter"); + + /* Give the work queue a moment to drain. */ + k_msleep(50); + TEST_ASSERT_TRUE_MESSAGE(isr_work_ran, "k_work_submit from ISR did not run"); + + k_timer_stop(&timer); +} + +#endif /* CONFIG_K_TIMER_DISPATCH_ISR */ + void test_k_timer_group(void) { RUN_TEST(test_timer_one_shot); @@ -311,4 +393,9 @@ void test_k_timer_group(void) RUN_TEST(test_timer_remaining_ticks); RUN_TEST(test_timer_expires_ticks); RUN_TEST(test_timer_one_shot_clears_running); +#ifdef CONFIG_K_TIMER_DISPATCH_ISR + RUN_TEST(test_timer_callback_in_isr_context); + RUN_TEST(test_timer_callback_iram_attr); + RUN_TEST(test_timer_isr_safe_apis); +#endif } diff --git a/test/sdkconfig.defaults b/test/sdkconfig.defaults index ab7fcfd..2887631 100644 --- a/test/sdkconfig.defaults +++ b/test/sdkconfig.defaults @@ -1,6 +1,7 @@ # Boreas test defaults CONFIG_FREERTOS_HZ=1000 CONFIG_ESP_MAIN_TASK_STACK_SIZE=8192 +CONFIG_FREERTOS_IDLE_TASK_STACKSIZE=2048 # Enable all Boreas features for testing CONFIG_ZKERNEL_SYS_INIT=y @@ -11,6 +12,7 @@ CONFIG_ZKERNEL_MUTEX_HOLD_WARNING_MS=5000 CONFIG_ZSYS_LOG_MODULE=y CONFIG_ZSYS_LOG_MAX_LEVEL=4 CONFIG_ZSYS_RETRY=y +CONFIG_K_TIMER_DISPATCH_ISR=y # Shell (for unit testing parsing/history/completion) CONFIG_ZSHELL=y @@ -18,6 +20,3 @@ CONFIG_ZSHELL_CMD_KERNEL=n CONFIG_ZSHELL_CMD_LOG=n CONFIG_ZSHELL_CMD_THREAD=n CONFIG_ZSHELL_CMD_DEVICE=n - -# Unity test framework -CONFIG_UNITY_ENABLE_FLOAT=y From cb116fef40faea8a6e1a2610cf428c154296833c Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Thu, 7 May 2026 11:21:02 -0600 Subject: [PATCH 2/5] =?UTF-8?q?feat:=20k=5Ftimer=20ISR=20dispatch=20?= =?UTF-8?q?=E2=80=94=20align=20callback=20context=20with=20upstream=20Zeph?= =?UTF-8?q?yr?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- components/zkernel/include/boreas/zephyr/kernel.h | 2 +- components/zkernel/src/k_thread.c | 5 ++--- components/zkernel/src/k_timer.c | 1 - examples/work_queue_demo/main/main.c | 2 +- test/main/test_k_timer.c | 8 +++----- test/sdkconfig.defaults | 4 +++- 6 files changed, 10 insertions(+), 12 deletions(-) diff --git a/components/zkernel/include/boreas/zephyr/kernel.h b/components/zkernel/include/boreas/zephyr/kernel.h index c9ac0b4..6dce66c 100644 --- a/components/zkernel/include/boreas/zephyr/kernel.h +++ b/components/zkernel/include/boreas/zephyr/kernel.h @@ -408,7 +408,7 @@ static inline void k_timer_user_data_set(struct k_timer *timer, void *user_data) timer->user_data = user_data; } -static inline void *k_timer_user_data_get(struct k_timer *timer) +static inline void *k_timer_user_data_get(const struct k_timer *timer) { return timer->user_data; } diff --git a/components/zkernel/src/k_thread.c b/components/zkernel/src/k_thread.c index 23b26db..37a9aca 100644 --- a/components/zkernel/src/k_thread.c +++ b/components/zkernel/src/k_thread.c @@ -42,8 +42,7 @@ static void IRAM_ATTR k_thread_delay_expiry(struct k_timer *timer) struct k_thread *thread = (struct k_thread *)k_timer_user_data_get(timer); if (thread) { BaseType_t woken = pdFALSE; - xTimerPendFunctionCallFromISR(k_thread_delay_resume_pended, - thread, 0, &woken); + xTimerPendFunctionCallFromISR(k_thread_delay_resume_pended, thread, 0, &woken); portYIELD_FROM_ISR(woken); } } @@ -77,7 +76,7 @@ k_tid_t k_thread_create(struct k_thread *thread, StackType_t *stack, size_t stac /* Pin to core 0. ESP-IDF's vPortCleanUpCoprocArea passes * tskNO_AFFINITY (0x7FFFFFFF) to _xt_coproc_release, which * overflows the owner-table index and silently skips the real - * entries — leaving stale FPU ownership that crashes later. */ + * entries — leaking stale FPU ownership. */ thread->handle = xTaskCreateStaticPinnedToCore( k_thread_entry_wrapper, thread->name ? thread->name : "k_thread", stack_size / sizeof(StackType_t), thread, prio, stack, &thread->tcb, 0); diff --git a/components/zkernel/src/k_timer.c b/components/zkernel/src/k_timer.c index f8c36c6..c98a12a 100644 --- a/components/zkernel/src/k_timer.c +++ b/components/zkernel/src/k_timer.c @@ -214,4 +214,3 @@ k_ticks_t k_timer_expires_ticks(const struct k_timer *timer) } return (k_ticks_t)(expiry / (uint64_t)tick_us); } - diff --git a/examples/work_queue_demo/main/main.c b/examples/work_queue_demo/main/main.c index b2348bd..65f383d 100644 --- a/examples/work_queue_demo/main/main.c +++ b/examples/work_queue_demo/main/main.c @@ -28,7 +28,7 @@ LOG_MODULE_REGISTER(demo, LOG_LEVEL_INF); * runs in ISR context. Use k_work_submit to defer logging to a task. * ---------------------------------------------------------------- */ -static int heartbeat_count = 0; +static volatile int heartbeat_count = 0; static struct k_work heartbeat_log_work; static void heartbeat_log_handler(struct k_work *work) diff --git a/test/main/test_k_timer.c b/test/main/test_k_timer.c index 4aa535d..b582a77 100644 --- a/test/main/test_k_timer.c +++ b/test/main/test_k_timer.c @@ -324,8 +324,7 @@ static void test_timer_callback_in_isr_context(void) k_msleep(100); k_timer_stop(&timer); - TEST_ASSERT_TRUE_MESSAGE(isr_context_result, - "Expiry callback did not run in ISR context"); + TEST_ASSERT_TRUE_MESSAGE(isr_context_result, "Expiry callback did not run in ISR context"); } extern int _iram_text_start; @@ -334,9 +333,8 @@ extern int _iram_text_end; static void test_timer_callback_iram_attr(void) { void *fn = (void *)isr_context_check_cb; - TEST_ASSERT_TRUE_MESSAGE( - (fn >= (void *)&_iram_text_start && fn < (void *)&_iram_text_end), - "IRAM_ATTR callback is not in IRAM address range"); + TEST_ASSERT_TRUE_MESSAGE((fn >= (void *)&_iram_text_start && fn < (void *)&_iram_text_end), + "IRAM_ATTR callback is not in IRAM address range"); } static struct k_sem isr_test_sem; diff --git a/test/sdkconfig.defaults b/test/sdkconfig.defaults index 2887631..714e348 100644 --- a/test/sdkconfig.defaults +++ b/test/sdkconfig.defaults @@ -1,7 +1,6 @@ # Boreas test defaults CONFIG_FREERTOS_HZ=1000 CONFIG_ESP_MAIN_TASK_STACK_SIZE=8192 -CONFIG_FREERTOS_IDLE_TASK_STACKSIZE=2048 # Enable all Boreas features for testing CONFIG_ZKERNEL_SYS_INIT=y @@ -20,3 +19,6 @@ CONFIG_ZSHELL_CMD_KERNEL=n CONFIG_ZSHELL_CMD_LOG=n CONFIG_ZSHELL_CMD_THREAD=n CONFIG_ZSHELL_CMD_DEVICE=n + +# Unity test framework +CONFIG_UNITY_ENABLE_FLOAT=y From b237f5f831ce69c0c816bbacf5fe1263581af881 Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Thu, 7 May 2026 13:27:24 -0600 Subject: [PATCH 3/5] fix: copilot suggestions --- components/zkernel/src/k_sem.c | 6 ++++++ components/zkernel/src/k_thread.c | 4 +++- components/zkernel/src/k_timer.c | 13 +++++++++---- components/zkernel/src/k_work.c | 12 ++++++++++++ examples/work_queue_demo/main/main.c | 1 + test/main/test_k_timer.c | 27 ++++++++++++++++++++++++--- 6 files changed, 55 insertions(+), 8 deletions(-) diff --git a/components/zkernel/src/k_sem.c b/components/zkernel/src/k_sem.c index 42a6ec4..cde1cfa 100644 --- a/components/zkernel/src/k_sem.c +++ b/components/zkernel/src/k_sem.c @@ -7,7 +7,9 @@ #include +#include "esp_attr.h" #include "esp_log.h" +#include "sdkconfig.h" static const char *TAG = "k_sem"; @@ -30,7 +32,11 @@ int k_sem_take(struct k_sem *sem, k_timeout_t timeout) return k_timeout_is_no_wait(timeout) ? -EBUSY : -EAGAIN; } +#ifdef CONFIG_K_TIMER_DISPATCH_ISR +void IRAM_ATTR k_sem_give(struct k_sem *sem) +#else void k_sem_give(struct k_sem *sem) +#endif { if (xPortInIsrContext()) { BaseType_t wake = pdFALSE; diff --git a/components/zkernel/src/k_thread.c b/components/zkernel/src/k_thread.c index 37a9aca..90553e0 100644 --- a/components/zkernel/src/k_thread.c +++ b/components/zkernel/src/k_thread.c @@ -42,7 +42,9 @@ static void IRAM_ATTR k_thread_delay_expiry(struct k_timer *timer) struct k_thread *thread = (struct k_thread *)k_timer_user_data_get(timer); if (thread) { BaseType_t woken = pdFALSE; - xTimerPendFunctionCallFromISR(k_thread_delay_resume_pended, thread, 0, &woken); + BaseType_t ret = xTimerPendFunctionCallFromISR(k_thread_delay_resume_pended, thread, + 0, &woken); + __ASSERT(ret == pdPASS, "xTimerPendFunctionCallFromISR failed (queue full)"); portYIELD_FROM_ISR(woken); } } diff --git a/components/zkernel/src/k_timer.c b/components/zkernel/src/k_timer.c index c98a12a..6d5420e 100644 --- a/components/zkernel/src/k_timer.c +++ b/components/zkernel/src/k_timer.c @@ -22,7 +22,9 @@ static void k_timer_esp_callback(void *arg) /* First-interval transition: first expiry was start_once, now switch to periodic */ if (__atomic_load_n(&timer->first_interval_pending, __ATOMIC_ACQUIRE)) { __atomic_store_n(&timer->first_interval_pending, false, __ATOMIC_RELAXED); - esp_timer_start_periodic(timer->handle, timer->period_us); + esp_err_t err = esp_timer_start_periodic(timer->handle, timer->period_us); + __ASSERT(err == ESP_OK, "esp_timer_start_periodic failed in callback"); + (void)err; } __atomic_fetch_add(&timer->status, 1, __ATOMIC_RELEASE); @@ -71,9 +73,12 @@ void k_timer_init(struct k_timer *timer, k_timer_expiry_t expiry_fn, k_timer_sto void k_timer_start(struct k_timer *timer, k_timeout_t duration, k_timeout_t period) { - /* Stop any in-flight callback synchronously; esp_timer_stop blocks - * until the dispatch task is idle, so subsequent stores are - * race-free with respect to the callback. */ + /* Stop any in-flight timer. With ESP_TIMER_TASK dispatch, + * esp_timer_stop blocks until the callback finishes; with + * ESP_TIMER_ISR dispatch it only removes the timer from the + * armed list (the ISR callback may still be executing on + * another core). Callers must not restart a timer whose + * callback is still running — same constraint as upstream Zephyr. */ if (__atomic_load_n(&timer->running, __ATOMIC_ACQUIRE)) { esp_timer_stop(timer->handle); } diff --git a/components/zkernel/src/k_work.c b/components/zkernel/src/k_work.c index c93448e..65ee148 100644 --- a/components/zkernel/src/k_work.c +++ b/components/zkernel/src/k_work.c @@ -113,7 +113,11 @@ void k_work_init(struct k_work *work, k_work_handler_t handler) work->node.prev = NULL; } +#ifdef CONFIG_K_TIMER_DISPATCH_ISR +static int IRAM_ATTR k_work_submit_internal(struct k_work_q *queue, struct k_work *work) +#else static int k_work_submit_internal(struct k_work_q *queue, struct k_work *work) +#endif { z_work_lock(queue); @@ -134,7 +138,11 @@ static int k_work_submit_internal(struct k_work_q *queue, struct k_work *work) return 0; } +#ifdef CONFIG_K_TIMER_DISPATCH_ISR +int IRAM_ATTR k_work_submit(struct k_work *work) +#else int k_work_submit(struct k_work *work) +#endif { if (!(__atomic_load_n(&k_sys_work_q.flags, __ATOMIC_RELAXED) & Z_WORK_QUEUE_STARTED)) { return -EINVAL; @@ -142,7 +150,11 @@ int k_work_submit(struct k_work *work) return k_work_submit_internal(&k_sys_work_q, work); } +#ifdef CONFIG_K_TIMER_DISPATCH_ISR +int IRAM_ATTR k_work_submit_to_queue(struct k_work_q *queue, struct k_work *work) +#else int k_work_submit_to_queue(struct k_work_q *queue, struct k_work *work) +#endif { if (!(__atomic_load_n(&queue->flags, __ATOMIC_RELAXED) & Z_WORK_QUEUE_STARTED)) { return -EINVAL; diff --git a/examples/work_queue_demo/main/main.c b/examples/work_queue_demo/main/main.c index 65f383d..86cb709 100644 --- a/examples/work_queue_demo/main/main.c +++ b/examples/work_queue_demo/main/main.c @@ -33,6 +33,7 @@ static struct k_work heartbeat_log_work; static void heartbeat_log_handler(struct k_work *work) { + ARG_UNUSED(work); LOG_INF("[Timer] Heartbeat #%d", heartbeat_count); } diff --git a/test/main/test_k_timer.c b/test/main/test_k_timer.c index b582a77..d992f84 100644 --- a/test/main/test_k_timer.c +++ b/test/main/test_k_timer.c @@ -3,6 +3,8 @@ * Copyright 2026 Intercreate */ +#include + #include "unity.h" #include "zephyr/kernel.h" #include "sdkconfig.h" @@ -332,9 +334,10 @@ extern int _iram_text_end; static void test_timer_callback_iram_attr(void) { - void *fn = (void *)isr_context_check_cb; - TEST_ASSERT_TRUE_MESSAGE((fn >= (void *)&_iram_text_start && fn < (void *)&_iram_text_end), - "IRAM_ATTR callback is not in IRAM address range"); + uintptr_t fn = (uintptr_t)isr_context_check_cb; + TEST_ASSERT_TRUE_MESSAGE( + (fn >= (uintptr_t)&_iram_text_start && fn < (uintptr_t)&_iram_text_end), + "IRAM_ATTR callback is not in IRAM address range"); } static struct k_sem isr_test_sem; @@ -374,6 +377,22 @@ static void test_timer_isr_safe_apis(void) k_timer_stop(&timer); } +static void test_k_work_submit_iram_attr(void) +{ + uintptr_t fn = (uintptr_t)k_work_submit; + TEST_ASSERT_TRUE_MESSAGE( + (fn >= (uintptr_t)&_iram_text_start && fn < (uintptr_t)&_iram_text_end), + "k_work_submit is not in IRAM address range"); +} + +static void test_k_sem_give_iram_attr(void) +{ + uintptr_t fn = (uintptr_t)k_sem_give; + TEST_ASSERT_TRUE_MESSAGE( + (fn >= (uintptr_t)&_iram_text_start && fn < (uintptr_t)&_iram_text_end), + "k_sem_give is not in IRAM address range"); +} + #endif /* CONFIG_K_TIMER_DISPATCH_ISR */ void test_k_timer_group(void) @@ -395,5 +414,7 @@ void test_k_timer_group(void) RUN_TEST(test_timer_callback_in_isr_context); RUN_TEST(test_timer_callback_iram_attr); RUN_TEST(test_timer_isr_safe_apis); + RUN_TEST(test_k_work_submit_iram_attr); + RUN_TEST(test_k_sem_give_iram_attr); #endif } From 3921d242e0cb1aa8e59d0c1f95ae10df71798777 Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Sun, 10 May 2026 08:14:24 -0600 Subject: [PATCH 4/5] fix: IRAM-harden all ISR-callable kernel APIs --- .../zkernel/include/boreas/zephyr/kernel.h | 17 ++++++- .../zkernel/include/boreas/zephyr/sys/util.h | 19 ++++++- components/zkernel/src/k_event.c | 8 +-- components/zkernel/src/k_msgq.c | 4 +- components/zkernel/src/k_sem.c | 6 +-- components/zkernel/src/k_thread.c | 18 ++++--- components/zkernel/src/k_timer.c | 13 +++-- components/zkernel/src/k_work.c | 24 ++------- components/zsys/src/watchdog.c | 6 +-- examples/work_queue_demo/main/main.c | 10 ++-- test/main/test_k_timer.c | 49 +++++++++++++++++++ 11 files changed, 119 insertions(+), 55 deletions(-) diff --git a/components/zkernel/include/boreas/zephyr/kernel.h b/components/zkernel/include/boreas/zephyr/kernel.h index 6dce66c..ce172a5 100644 --- a/components/zkernel/include/boreas/zephyr/kernel.h +++ b/components/zkernel/include/boreas/zephyr/kernel.h @@ -312,8 +312,9 @@ struct k_timer; * k_mutex_lock, k_msleep, k_thread_join). * - The following are ISR-safe and may be called: * k_sem_give, k_work_submit, k_event_set/post/clear, - * k_msgq_put (K_NO_WAIT), LOG_* (with - * CONFIG_ZSYS_LOG_MODE_DEFERRED=y only). + * k_msgq_put (K_NO_WAIT). + * - LOG_* is NOT ISR-safe (vsnprintf in the emit path is + * flash-resident). Defer logging via k_work_submit. * - Must not call malloc, printf, or any flash-resident * function (IRAM_ATTR is required for cache survival). * @@ -348,6 +349,18 @@ struct k_timer { } void k_timer_init(struct k_timer *timer, k_timer_expiry_t expiry_fn, k_timer_stop_t stop_fn); +/** + * @brief Start or restart a timer. + * + * @param timer Timer to start. + * @param duration Time until the first expiry. + * @param period Repeat interval after the first expiry (K_NO_WAIT for one-shot). + * + * @warning Must not be called while the timer's expiry callback is + * executing (same constraint as upstream Zephyr). With + * CONFIG_K_TIMER_DISPATCH_ISR=y, esp_timer_stop does not + * synchronize with an in-flight ISR callback on SMP targets. + */ void k_timer_start(struct k_timer *timer, k_timeout_t duration, k_timeout_t period); void k_timer_stop(struct k_timer *timer); diff --git a/components/zkernel/include/boreas/zephyr/sys/util.h b/components/zkernel/include/boreas/zephyr/sys/util.h index 0553540..dde0c4e 100644 --- a/components/zkernel/include/boreas/zephyr/sys/util.h +++ b/components/zkernel/include/boreas/zephyr/sys/util.h @@ -7,6 +7,9 @@ #pragma once +#include "esp_attr.h" +#include "sdkconfig.h" + #ifdef __cplusplus extern "C" { #endif @@ -101,7 +104,9 @@ extern "C" { #define ALWAYS_INLINE __attribute__((always_inline)) inline #endif -/* Runtime assertion -- logs and aborts */ +/* Runtime assertion -- logs and aborts. + * NOT safe from IRAM ISR context (ESP_LOGE + abort are flash-resident). + * Use k_panic() for unrecoverable errors in ISR/IRAM context. */ #ifndef __ASSERT #include "esp_log.h" #define __ASSERT(cond, msg) \ @@ -113,6 +118,18 @@ extern "C" { } while (0) #endif +/* IRAM-safe panic -- triggers an illegal-instruction exception caught by + * the ESP-IDF panic handler (which is IRAM-resident). Produces a full + * backtrace on UART and reboots. Safe to call from IRAM_ATTR ISR context. + * Mirrors upstream Zephyr's k_panic() contract. */ +#define k_panic() __builtin_trap() + +/* Attribute for kernel functions callable from ISR context. + * Always IRAM-resident: upstream Zephyr marks these isr-ok + * unconditionally, and ESP-IDF ISRs registered with + * ESP_INTR_FLAG_IRAM fire during cache-disabled windows. */ +#define K_ISR_SAFE IRAM_ATTR + #ifdef __cplusplus } #endif diff --git a/components/zkernel/src/k_event.c b/components/zkernel/src/k_event.c index 1be1c50..e4b69fb 100644 --- a/components/zkernel/src/k_event.c +++ b/components/zkernel/src/k_event.c @@ -7,7 +7,9 @@ #include +#include "esp_attr.h" #include "esp_log.h" +#include "sdkconfig.h" static const char *TAG = "k_event"; @@ -21,12 +23,12 @@ int k_event_init(struct k_event *event) return 0; } -uint32_t k_event_post(struct k_event *event, uint32_t events) +uint32_t K_ISR_SAFE k_event_post(struct k_event *event, uint32_t events) { return k_event_set(event, events); } -uint32_t k_event_set(struct k_event *event, uint32_t events) +uint32_t K_ISR_SAFE k_event_set(struct k_event *event, uint32_t events) { if (xPortInIsrContext()) { BaseType_t wake = pdFALSE; @@ -40,7 +42,7 @@ uint32_t k_event_set(struct k_event *event, uint32_t events) return (uint32_t)xEventGroupSetBits(event->handle, (EventBits_t)events); } -uint32_t k_event_clear(struct k_event *event, uint32_t events) +uint32_t K_ISR_SAFE k_event_clear(struct k_event *event, uint32_t events) { if (xPortInIsrContext()) { return (uint32_t)xEventGroupClearBitsFromISR(event->handle, (EventBits_t)events); diff --git a/components/zkernel/src/k_msgq.c b/components/zkernel/src/k_msgq.c index 15693a2..55ed712 100644 --- a/components/zkernel/src/k_msgq.c +++ b/components/zkernel/src/k_msgq.c @@ -7,7 +7,9 @@ #include +#include "esp_attr.h" #include "esp_log.h" +#include "sdkconfig.h" static const char *TAG = "k_msgq"; @@ -24,7 +26,7 @@ int k_msgq_init(struct k_msgq *msgq, char *buffer, size_t msg_size, uint32_t max return 0; } -int k_msgq_put(struct k_msgq *msgq, const void *data, k_timeout_t timeout) +int K_ISR_SAFE k_msgq_put(struct k_msgq *msgq, const void *data, k_timeout_t timeout) { BaseType_t ret; diff --git a/components/zkernel/src/k_sem.c b/components/zkernel/src/k_sem.c index cde1cfa..0a90904 100644 --- a/components/zkernel/src/k_sem.c +++ b/components/zkernel/src/k_sem.c @@ -32,11 +32,7 @@ int k_sem_take(struct k_sem *sem, k_timeout_t timeout) return k_timeout_is_no_wait(timeout) ? -EBUSY : -EAGAIN; } -#ifdef CONFIG_K_TIMER_DISPATCH_ISR -void IRAM_ATTR k_sem_give(struct k_sem *sem) -#else -void k_sem_give(struct k_sem *sem) -#endif +void K_ISR_SAFE k_sem_give(struct k_sem *sem) { if (xPortInIsrContext()) { BaseType_t wake = pdFALSE; diff --git a/components/zkernel/src/k_thread.c b/components/zkernel/src/k_thread.c index 90553e0..8b0f4c0 100644 --- a/components/zkernel/src/k_thread.c +++ b/components/zkernel/src/k_thread.c @@ -37,14 +37,16 @@ static void k_thread_delay_resume_pended(void *param, uint32_t unused) } } -static void IRAM_ATTR k_thread_delay_expiry(struct k_timer *timer) +static void K_ISR_SAFE k_thread_delay_expiry(struct k_timer *timer) { struct k_thread *thread = (struct k_thread *)k_timer_user_data_get(timer); if (thread) { BaseType_t woken = pdFALSE; BaseType_t ret = xTimerPendFunctionCallFromISR(k_thread_delay_resume_pended, thread, 0, &woken); - __ASSERT(ret == pdPASS, "xTimerPendFunctionCallFromISR failed (queue full)"); + if (ret != pdPASS) { + k_panic(); + } portYIELD_FROM_ISR(woken); } } @@ -83,12 +85,14 @@ k_tid_t k_thread_create(struct k_thread *thread, StackType_t *stack, size_t stac k_thread_entry_wrapper, thread->name ? thread->name : "k_thread", stack_size / sizeof(StackType_t), thread, prio, stack, &thread->tcb, 0); - /* xTaskCreateStatic only fails on programmer error (misaligned - * stack, NULL stack pointer, etc.). Match upstream Zephyr's - * "always returns a valid tid" contract by asserting here rather - * than returning NULL -- callers ported from Zephyr won't check. */ - __ASSERT(thread->handle != NULL, "k_thread_create: xTaskCreateStatic failed"); + /* xTaskCreateStaticPinnedToCore only fails on programmer error + * (misaligned stack, NULL stack pointer, etc.). Match upstream + * Zephyr's "always returns a valid tid" contract by asserting + * here rather than returning NULL -- callers ported from Zephyr + * won't check. */ + __ASSERT(thread->handle != NULL, "k_thread_create: xTaskCreateStaticPinnedToCore failed"); + /* Finite delay: thread self-suspended, set up timer to resume */ if (!k_timeout_is_forever(delay) && !k_timeout_is_no_wait(delay)) { k_timer_init(&thread->_delay_timer, k_thread_delay_expiry, NULL); k_timer_user_data_set(&thread->_delay_timer, thread); diff --git a/components/zkernel/src/k_timer.c b/components/zkernel/src/k_timer.c index 6d5420e..ab4d0dc 100644 --- a/components/zkernel/src/k_timer.c +++ b/components/zkernel/src/k_timer.c @@ -11,20 +11,19 @@ static const char *TAG = "k_timer"; -#ifdef CONFIG_K_TIMER_DISPATCH_ISR -static void IRAM_ATTR k_timer_esp_callback(void *arg) -#else -static void k_timer_esp_callback(void *arg) -#endif +static void K_ISR_SAFE k_timer_esp_callback(void *arg) { struct k_timer *timer = (struct k_timer *)arg; /* First-interval transition: first expiry was start_once, now switch to periodic */ if (__atomic_load_n(&timer->first_interval_pending, __ATOMIC_ACQUIRE)) { __atomic_store_n(&timer->first_interval_pending, false, __ATOMIC_RELAXED); + /* Cannot fail: timer_process_alarm zeroes alarm before callback, + * so timer_armed() is false and start_periodic succeeds. */ esp_err_t err = esp_timer_start_periodic(timer->handle, timer->period_us); - __ASSERT(err == ESP_OK, "esp_timer_start_periodic failed in callback"); - (void)err; + if (err != ESP_OK) { + k_panic(); + } } __atomic_fetch_add(&timer->status, 1, __ATOMIC_RELEASE); diff --git a/components/zkernel/src/k_work.c b/components/zkernel/src/k_work.c index 65ee148..1622599 100644 --- a/components/zkernel/src/k_work.c +++ b/components/zkernel/src/k_work.c @@ -113,11 +113,7 @@ void k_work_init(struct k_work *work, k_work_handler_t handler) work->node.prev = NULL; } -#ifdef CONFIG_K_TIMER_DISPATCH_ISR -static int IRAM_ATTR k_work_submit_internal(struct k_work_q *queue, struct k_work *work) -#else -static int k_work_submit_internal(struct k_work_q *queue, struct k_work *work) -#endif +static int K_ISR_SAFE k_work_submit_internal(struct k_work_q *queue, struct k_work *work) { z_work_lock(queue); @@ -138,11 +134,7 @@ static int k_work_submit_internal(struct k_work_q *queue, struct k_work *work) return 0; } -#ifdef CONFIG_K_TIMER_DISPATCH_ISR -int IRAM_ATTR k_work_submit(struct k_work *work) -#else -int k_work_submit(struct k_work *work) -#endif +int K_ISR_SAFE k_work_submit(struct k_work *work) { if (!(__atomic_load_n(&k_sys_work_q.flags, __ATOMIC_RELAXED) & Z_WORK_QUEUE_STARTED)) { return -EINVAL; @@ -150,11 +142,7 @@ int k_work_submit(struct k_work *work) return k_work_submit_internal(&k_sys_work_q, work); } -#ifdef CONFIG_K_TIMER_DISPATCH_ISR -int IRAM_ATTR k_work_submit_to_queue(struct k_work_q *queue, struct k_work *work) -#else -int k_work_submit_to_queue(struct k_work_q *queue, struct k_work *work) -#endif +int K_ISR_SAFE k_work_submit_to_queue(struct k_work_q *queue, struct k_work *work) { if (!(__atomic_load_n(&queue->flags, __ATOMIC_RELAXED) & Z_WORK_QUEUE_STARTED)) { return -EINVAL; @@ -339,11 +327,7 @@ static void __attribute__((constructor)) sys_work_q_auto_init(void) * Delayable Work * ---------------------------------------------------------------- */ -#ifdef CONFIG_K_TIMER_DISPATCH_ISR -static void IRAM_ATTR k_work_delayable_timer_expiry(struct k_timer *timer) -#else -static void k_work_delayable_timer_expiry(struct k_timer *timer) -#endif +static void K_ISR_SAFE k_work_delayable_timer_expiry(struct k_timer *timer) { struct k_work_delayable *dwork = CONTAINER_OF(timer, struct k_work_delayable, timer); if (dwork->queue) { diff --git a/components/zsys/src/watchdog.c b/components/zsys/src/watchdog.c index bf513a9..57f0365 100644 --- a/components/zsys/src/watchdog.c +++ b/components/zsys/src/watchdog.c @@ -46,11 +46,7 @@ static void supervisor_work_handler(struct k_work *work) } } -#ifdef CONFIG_K_TIMER_DISPATCH_ISR -static void IRAM_ATTR supervisor_check(struct k_timer *timer) -#else -static void supervisor_check(struct k_timer *timer) -#endif +static void K_ISR_SAFE supervisor_check(struct k_timer *timer) { (void)timer; k_work_submit(&supervisor_work); diff --git a/examples/work_queue_demo/main/main.c b/examples/work_queue_demo/main/main.c index 86cb709..e8967e6 100644 --- a/examples/work_queue_demo/main/main.c +++ b/examples/work_queue_demo/main/main.c @@ -28,24 +28,26 @@ LOG_MODULE_REGISTER(demo, LOG_LEVEL_INF); * runs in ISR context. Use k_work_submit to defer logging to a task. * ---------------------------------------------------------------- */ -static volatile int heartbeat_count = 0; +static atomic_t heartbeat_count = ATOMIC_INIT(0); static struct k_work heartbeat_log_work; static void heartbeat_log_handler(struct k_work *work) { ARG_UNUSED(work); - LOG_INF("[Timer] Heartbeat #%d", heartbeat_count); + LOG_INF("[Timer] Heartbeat #%ld", atomic_get(&heartbeat_count)); } static void IRAM_ATTR heartbeat_expiry(struct k_timer *timer) { - heartbeat_count++; + ARG_UNUSED(timer); + atomic_add(&heartbeat_count, 1); k_work_submit(&heartbeat_log_work); } static void heartbeat_stop(struct k_timer *timer) { - LOG_INF("[Timer] Heartbeat stopped after %d beats", heartbeat_count); + ARG_UNUSED(timer); + LOG_INF("[Timer] Heartbeat stopped after %ld beats", atomic_get(&heartbeat_count)); } static struct k_timer heartbeat_timer; diff --git a/test/main/test_k_timer.c b/test/main/test_k_timer.c index d992f84..d29cb1e 100644 --- a/test/main/test_k_timer.c +++ b/test/main/test_k_timer.c @@ -18,11 +18,13 @@ static volatile int stop_called; static void IRAM_ATTR test_timer_cb(struct k_timer *timer) { + ARG_UNUSED(timer); timer_count++; } static void test_stop_cb(struct k_timer *timer) { + ARG_UNUSED(timer); stop_called++; } @@ -313,6 +315,7 @@ static volatile bool isr_context_result; static void IRAM_ATTR isr_context_check_cb(struct k_timer *timer) { + ARG_UNUSED(timer); isr_context_result = xPortInIsrContext(); } @@ -346,6 +349,7 @@ static volatile bool isr_work_ran; static void IRAM_ATTR isr_safe_apis_cb(struct k_timer *timer) { + ARG_UNUSED(timer); k_sem_give(&isr_test_sem); k_work_submit(&isr_test_work); } @@ -393,6 +397,46 @@ static void test_k_sem_give_iram_attr(void) "k_sem_give is not in IRAM address range"); } +static void test_k_event_set_iram_attr(void) +{ + uintptr_t fn = (uintptr_t)k_event_set; + TEST_ASSERT_TRUE_MESSAGE( + (fn >= (uintptr_t)&_iram_text_start && fn < (uintptr_t)&_iram_text_end), + "k_event_set is not in IRAM address range"); +} + +static void test_k_event_post_iram_attr(void) +{ + uintptr_t fn = (uintptr_t)k_event_post; + TEST_ASSERT_TRUE_MESSAGE( + (fn >= (uintptr_t)&_iram_text_start && fn < (uintptr_t)&_iram_text_end), + "k_event_post is not in IRAM address range"); +} + +static void test_k_event_clear_iram_attr(void) +{ + uintptr_t fn = (uintptr_t)k_event_clear; + TEST_ASSERT_TRUE_MESSAGE( + (fn >= (uintptr_t)&_iram_text_start && fn < (uintptr_t)&_iram_text_end), + "k_event_clear is not in IRAM address range"); +} + +static void test_k_msgq_put_iram_attr(void) +{ + uintptr_t fn = (uintptr_t)k_msgq_put; + TEST_ASSERT_TRUE_MESSAGE( + (fn >= (uintptr_t)&_iram_text_start && fn < (uintptr_t)&_iram_text_end), + "k_msgq_put is not in IRAM address range"); +} + +static void test_k_work_submit_to_queue_iram_attr(void) +{ + uintptr_t fn = (uintptr_t)k_work_submit_to_queue; + TEST_ASSERT_TRUE_MESSAGE( + (fn >= (uintptr_t)&_iram_text_start && fn < (uintptr_t)&_iram_text_end), + "k_work_submit_to_queue is not in IRAM address range"); +} + #endif /* CONFIG_K_TIMER_DISPATCH_ISR */ void test_k_timer_group(void) @@ -416,5 +460,10 @@ void test_k_timer_group(void) RUN_TEST(test_timer_isr_safe_apis); RUN_TEST(test_k_work_submit_iram_attr); RUN_TEST(test_k_sem_give_iram_attr); + RUN_TEST(test_k_event_set_iram_attr); + RUN_TEST(test_k_event_post_iram_attr); + RUN_TEST(test_k_event_clear_iram_attr); + RUN_TEST(test_k_msgq_put_iram_attr); + RUN_TEST(test_k_work_submit_to_queue_iram_attr); #endif } From d6714479b0129765f419564f1eb979b8e95c1787 Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Sun, 10 May 2026 20:29:33 -0600 Subject: [PATCH 5/5] fix: review hardening; ALWAYS_INLINE, watchdog panic, assert on init --- components/zkernel/include/boreas/zephyr/kernel.h | 8 ++++++-- components/zkernel/include/boreas/zephyr/sys/util.h | 1 - components/zkernel/src/k_timer.c | 4 +--- components/zkernel/src/k_work.c | 4 ++-- components/zsys/src/watchdog.c | 4 +++- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/components/zkernel/include/boreas/zephyr/kernel.h b/components/zkernel/include/boreas/zephyr/kernel.h index ce172a5..479494e 100644 --- a/components/zkernel/include/boreas/zephyr/kernel.h +++ b/components/zkernel/include/boreas/zephyr/kernel.h @@ -288,6 +288,10 @@ struct k_event { int k_event_init(struct k_event *event); uint32_t k_event_post(struct k_event *event, uint32_t events); +/** @note In ISR context, returns 0 on failure (timer command queue full) + * rather than the previous event state. FreeRTOS's + * xEventGroupSetBitsFromISR defers to the timer daemon and cannot + * report the prior bits synchronously. */ uint32_t k_event_set(struct k_event *event, uint32_t events); uint32_t k_event_clear(struct k_event *event, uint32_t events); uint32_t k_event_wait(struct k_event *event, uint32_t events, bool reset, k_timeout_t timeout); @@ -416,12 +420,12 @@ k_ticks_t k_timer_remaining_ticks(const struct k_timer *timer); */ k_ticks_t k_timer_expires_ticks(const struct k_timer *timer); -static inline void k_timer_user_data_set(struct k_timer *timer, void *user_data) +static ALWAYS_INLINE void k_timer_user_data_set(struct k_timer *timer, void *user_data) { timer->user_data = user_data; } -static inline void *k_timer_user_data_get(const struct k_timer *timer) +static ALWAYS_INLINE void *k_timer_user_data_get(const struct k_timer *timer) { return timer->user_data; } diff --git a/components/zkernel/include/boreas/zephyr/sys/util.h b/components/zkernel/include/boreas/zephyr/sys/util.h index dde0c4e..b401aaa 100644 --- a/components/zkernel/include/boreas/zephyr/sys/util.h +++ b/components/zkernel/include/boreas/zephyr/sys/util.h @@ -8,7 +8,6 @@ #pragma once #include "esp_attr.h" -#include "sdkconfig.h" #ifdef __cplusplus extern "C" { diff --git a/components/zkernel/src/k_timer.c b/components/zkernel/src/k_timer.c index ab4d0dc..ba85dc3 100644 --- a/components/zkernel/src/k_timer.c +++ b/components/zkernel/src/k_timer.c @@ -65,9 +65,7 @@ void k_timer_init(struct k_timer *timer, k_timer_expiry_t expiry_fn, k_timer_sto .name = "k_timer", }; esp_err_t err = esp_timer_create(&args, &timer->handle); - if (err != ESP_OK) { - ESP_LOGE(TAG, "esp_timer_create failed: %s", esp_err_to_name(err)); - } + __ASSERT(err == ESP_OK, "k_timer_init: esp_timer_create failed"); } void k_timer_start(struct k_timer *timer, k_timeout_t duration, k_timeout_t period) diff --git a/components/zkernel/src/k_work.c b/components/zkernel/src/k_work.c index 1622599..41bd661 100644 --- a/components/zkernel/src/k_work.c +++ b/components/zkernel/src/k_work.c @@ -36,7 +36,7 @@ struct k_work_q k_sys_work_q; /* Branch-once helpers around portENTER/EXIT_CRITICAL: ESP-IDF needs * different macros for ISR vs task context. Inlined to avoid runtime * overhead in the common (task) case. */ -static inline void z_work_lock(struct k_work_q *queue) +static ALWAYS_INLINE void z_work_lock(struct k_work_q *queue) { if (xPortInIsrContext()) { portENTER_CRITICAL_ISR(&queue->lock); @@ -45,7 +45,7 @@ static inline void z_work_lock(struct k_work_q *queue) } } -static inline void z_work_unlock(struct k_work_q *queue) +static ALWAYS_INLINE void z_work_unlock(struct k_work_q *queue) { if (xPortInIsrContext()) { portEXIT_CRITICAL_ISR(&queue->lock); diff --git a/components/zsys/src/watchdog.c b/components/zsys/src/watchdog.c index 57f0365..3182bc9 100644 --- a/components/zsys/src/watchdog.c +++ b/components/zsys/src/watchdog.c @@ -49,7 +49,9 @@ static void supervisor_work_handler(struct k_work *work) static void K_ISR_SAFE supervisor_check(struct k_timer *timer) { (void)timer; - k_work_submit(&supervisor_work); + if (k_work_submit(&supervisor_work) < 0) { + k_panic(); + } } void zsys_watchdog_init(k_timeout_t check_interval, zsys_watchdog_timeout_cb_t timeout_cb)