From 4987a5ad904d7ab469853f28bc2345f9248bc872 Mon Sep 17 00:00:00 2001 From: Tian Hao Date: Mon, 28 Oct 2019 18:43:35 +0800 Subject: [PATCH] fix bug that semaphore may schedule out in Critical Section 1. Since BLE full-scan feature for BLE mesh change the controller code cause this problem, it cause coex semaphore take in "interrupt disable", then it may cause task schedule and cause crash in freertos 2. Fix newlib lock ISR context and critical section check 3. Fix bt controller ISR context and critical section check --- components/bt/controller/bt.c | 2 +- components/esp32/esp_adapter.c | 7 ++++++- .../freertos/include/freertos/portable.h | 20 +++++++++++++++++++ components/newlib/locks.c | 4 ++-- 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/components/bt/controller/bt.c b/components/bt/controller/bt.c index 81442d227..c0155dd41 100644 --- a/components/bt/controller/bt.c +++ b/components/bt/controller/bt.c @@ -743,7 +743,7 @@ static void task_delete_wrapper(void *task_handle) static bool IRAM_ATTR is_in_isr_wrapper(void) { - return (bool)xPortInIsrContext(); + return !xPortCanYield(); } static void IRAM_ATTR cause_sw_intr(void *arg) diff --git a/components/esp32/esp_adapter.c b/components/esp32/esp_adapter.c index 0e8ec3be7..90c623bcd 100644 --- a/components/esp32/esp_adapter.c +++ b/components/esp32/esp_adapter.c @@ -508,6 +508,11 @@ void IRAM_ATTR coex_bb_reset_unlock_wrapper(uint32_t restore) #endif } +int32_t IRAM_ATTR coex_is_in_isr_wrapper(void) +{ + return !xPortCanYield(); +} + wifi_osi_funcs_t g_wifi_osi_funcs = { ._version = ESP_WIFI_OS_ADAPTER_VERSION, ._set_isr = set_isr_wrapper, @@ -617,7 +622,7 @@ coex_adapter_funcs_t g_coex_adapter_funcs = { ._semphr_give_from_isr = semphr_give_from_isr_wrapper, ._semphr_take = semphr_take_wrapper, ._semphr_give = semphr_give_wrapper, - ._is_in_isr = xPortInIsrContext, + ._is_in_isr = coex_is_in_isr_wrapper, ._malloc_internal = malloc_internal_wrapper, ._free = free, ._timer_disarm = timer_disarm_wrapper, diff --git a/components/freertos/include/freertos/portable.h b/components/freertos/include/freertos/portable.h index 61e748f32..7fab98527 100644 --- a/components/freertos/include/freertos/portable.h +++ b/components/freertos/include/freertos/portable.h @@ -86,6 +86,8 @@ specific constants has been moved into the deprecated_definitions.h header file. */ #include "deprecated_definitions.h" +#include "soc/cpu.h" + /* If portENTER_CRITICAL is not defined then including deprecated_definitions.h did not result in a portmacro.h header file being included - and it should be included here. In this case the path to the correct portmacro.h header file @@ -215,6 +217,24 @@ static inline uint32_t IRAM_ATTR xPortGetCoreID(void) { /* Get tick rate per second */ uint32_t xPortGetTickRateHz(void); + +static inline bool IRAM_ATTR xPortCanYield(void) +{ + uint32_t ps_reg = 0; + + //Get the current value of PS (processor status) register + RSR(PS, ps_reg); + + /* + * intlevel = (ps_reg & 0xf); + * excm = (ps_reg >> 4) & 0x1; + * CINTLEVEL is max(excm * EXCMLEVEL, INTLEVEL), where EXCMLEVEL is 3. + * However, just return true, only intlevel is zero. + */ + + return ((ps_reg & PS_INTLEVEL_MASK) == 0); +} + #ifdef __cplusplus } #endif diff --git a/components/newlib/locks.c b/components/newlib/locks.c index 04754d814..0b7817efd 100644 --- a/components/newlib/locks.c +++ b/components/newlib/locks.c @@ -137,7 +137,7 @@ static int IRAM_ATTR lock_acquire_generic(_lock_t *lock, uint32_t delay, uint8_t } BaseType_t success; - if (xPortInIsrContext()) { + if (!xPortCanYield()) { /* In ISR Context */ if (mutex_type == queueQUEUE_TYPE_RECURSIVE_MUTEX) { abort(); /* recursive mutexes make no sense in ISR context */ @@ -191,7 +191,7 @@ static void IRAM_ATTR lock_release_generic(_lock_t *lock, uint8_t mutex_type) { return; } - if (xPortInIsrContext()) { + if (!xPortCanYield()) { if (mutex_type == queueQUEUE_TYPE_RECURSIVE_MUTEX) { abort(); /* indicates logic bug, it shouldn't be possible to lock recursively in ISR */ }