From 5c6b9adf11154266ac4a7d29bf89efe6d8b6c0f0 Mon Sep 17 00:00:00 2001 From: Sachin Parekh Date: Mon, 25 Mar 2019 15:55:57 +0530 Subject: [PATCH 01/10] freertos: port*_CRITICAL_SAFE API added port*_CRITICAL_SAFE API calls port*_CRITICAL or port*_CRITICAL_ISR depending on the context (Non-ISR or ISR respectively). FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE Kconfig option added Signed-off-by: Sachin Parekh --- components/freertos/Kconfig | 7 +++ .../freertos/include/freertos/portmacro.h | 46 +++++++++++++++++++ docs/en/api-guides/freertos-smp.rst | 5 ++ 3 files changed, 58 insertions(+) diff --git a/components/freertos/Kconfig b/components/freertos/Kconfig index 8063c266bc..7eeec02b93 100644 --- a/components/freertos/Kconfig +++ b/components/freertos/Kconfig @@ -426,4 +426,11 @@ menu "FreeRTOS" If enabled, assert that when a mutex semaphore is given, the task giving the semaphore is the task which is currently holding the mutex. + config FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE + bool "Tests compliance with Vanilla FreeRTOS port*_CRITICAL calls" + default n + help + If enabled, context of port*_CRITICAL calls (ISR or Non-ISR) would be checked to be in compliance with Vanilla FreeRTOS. + e.g Calling port*_CRITICAL from ISR context would cause assert failure + endmenu diff --git a/components/freertos/include/freertos/portmacro.h b/components/freertos/include/freertos/portmacro.h index d0023fee95..88da1c0c0e 100644 --- a/components/freertos/include/freertos/portmacro.h +++ b/components/freertos/include/freertos/portmacro.h @@ -216,8 +216,23 @@ void vPortCPUReleaseMutex(portMUX_TYPE *mux, const char *function, int line); void vTaskEnterCritical( portMUX_TYPE *mux, const char *function, int line ); void vTaskExitCritical( portMUX_TYPE *mux, const char *function, int line ); + +#ifdef CONFIG_FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE +/* Calling port*_CRITICAL from ISR context would cause an assert failure. + * If the parent function is called from both ISR and Non-ISR context then call port*_CRITICAL_SAFE + **/ +#define portENTER_CRITICAL(mux) do { \ + configASSERT(!xPortInIsrContext()); \ + vTaskEnterCritical(mux, __FUNCTION__, __LINE__); \ + } while(0) +#define portEXIT_CRITICAL(mux) do { \ + configASSERT(!xPortInIsrContext()); \ + vTaskExitCritical(mux, __FUNCTION__, __LINE__); \ + } while(0) +#else #define portENTER_CRITICAL(mux) vTaskEnterCritical(mux, __FUNCTION__, __LINE__) #define portEXIT_CRITICAL(mux) vTaskExitCritical(mux, __FUNCTION__, __LINE__) +#endif #define portENTER_CRITICAL_ISR(mux) vTaskEnterCritical(mux, __FUNCTION__, __LINE__) #define portEXIT_CRITICAL_ISR(mux) vTaskExitCritical(mux, __FUNCTION__, __LINE__) #else @@ -236,12 +251,43 @@ void vPortCPUAcquireMutex(portMUX_TYPE *mux); bool vPortCPUAcquireMutexTimeout(portMUX_TYPE *mux, int timeout_cycles); void vPortCPUReleaseMutex(portMUX_TYPE *mux); +#ifdef CONFIG_FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE +/* Calling port*_CRITICAL from ISR context would cause an assert failure. + * If the parent function is called from both ISR and Non-ISR context then call port*_CRITICAL_SAFE + **/ +#define portENTER_CRITICAL(mux) do { \ + configASSERT(!xPortInIsrContext()); \ + vTaskEnterCritical(mux); \ + } while(0) +#define portEXIT_CRITICAL(mux) do { \ + configASSERT(!xPortInIsrContext()); \ + vTaskExitCritical(mux); \ + } while(0) +#else #define portENTER_CRITICAL(mux) vTaskEnterCritical(mux) #define portEXIT_CRITICAL(mux) vTaskExitCritical(mux) +#endif #define portENTER_CRITICAL_ISR(mux) vTaskEnterCritical(mux) #define portEXIT_CRITICAL_ISR(mux) vTaskExitCritical(mux) #endif +#define portENTER_CRITICAL_SAFE(mux) do { \ + if (xPortInIsrContext()) { \ + portENTER_CRITICAL_ISR(mux); \ + } else { \ + portENTER_CRITICAL(mux); \ + } \ + } while(0) + +#define portEXIT_CRITICAL_SAFE(mux) do { \ + if (xPortInIsrContext()) { \ + portEXIT_CRITICAL_ISR(mux); \ + } else { \ + portEXIT_CRITICAL(mux); \ + } \ + } while(0) + + // Critical section management. NW-TODO: replace XTOS_SET_INTLEVEL with more efficient version, if any? // These cannot be nested. They should be used with a lot of care and cannot be called from interrupt level. // diff --git a/docs/en/api-guides/freertos-smp.rst b/docs/en/api-guides/freertos-smp.rst index 96738c45af..5e765a00a2 100644 --- a/docs/en/api-guides/freertos-smp.rst +++ b/docs/en/api-guides/freertos-smp.rst @@ -377,6 +377,11 @@ The ESP-IDF FreeRTOS critical section functions have been modified as follows… ``portEXIT_CRITICAL(mux)``, ``portEXIT_CRITICAL_ISR(mux)`` are all macro defined to call :cpp:func:`vTaskExitCritical` + - ``portENTER_CRITICAL_SAFE(mux)``, ``portEXIT_CRITICAL_SAFE(mux)`` macro identifies + the context of execution, i.e ISR or Non-ISR, and calls appropriate critical + section functions (``port*_CRITICAL`` in Non-ISR and ``port*_CRITICAL_ISR`` in ISR) + in order to be in compliance with Vanilla FreeRTOS. + For more details see :component_file:`freertos/include/freertos/portmacro.h` and :component_file:`freertos/task.c` From e9898b1280da3fe914b09259fa549cf5329778f3 Mon Sep 17 00:00:00 2001 From: Sachin Parekh Date: Mon, 25 Mar 2019 16:02:15 +0530 Subject: [PATCH 02/10] periph_ctrl: port*_CRITICAL vanilla FreeRTOS compliance Signed-off-by: Sachin Parekh --- components/driver/periph_ctrl.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/components/driver/periph_ctrl.c b/components/driver/periph_ctrl.c index f0a01a61ac..522c8d587f 100644 --- a/components/driver/periph_ctrl.c +++ b/components/driver/periph_ctrl.c @@ -29,26 +29,26 @@ static uint32_t get_rst_en_reg(periph_module_t periph); void periph_module_enable(periph_module_t periph) { - portENTER_CRITICAL(&periph_spinlock); + portENTER_CRITICAL_SAFE(&periph_spinlock); DPORT_SET_PERI_REG_MASK(get_clk_en_reg(periph), get_clk_en_mask(periph)); DPORT_CLEAR_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph, true)); - portEXIT_CRITICAL(&periph_spinlock); + portEXIT_CRITICAL_SAFE(&periph_spinlock); } void periph_module_disable(periph_module_t periph) { - portENTER_CRITICAL(&periph_spinlock); + portENTER_CRITICAL_SAFE(&periph_spinlock); DPORT_CLEAR_PERI_REG_MASK(get_clk_en_reg(periph), get_clk_en_mask(periph)); DPORT_SET_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph, false)); - portEXIT_CRITICAL(&periph_spinlock); + portEXIT_CRITICAL_SAFE(&periph_spinlock); } void periph_module_reset(periph_module_t periph) { - portENTER_CRITICAL(&periph_spinlock); + portENTER_CRITICAL_SAFE(&periph_spinlock); DPORT_SET_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph, false)); DPORT_CLEAR_PERI_REG_MASK(get_rst_en_reg(periph), get_rst_en_mask(periph, false)); - portEXIT_CRITICAL(&periph_spinlock); + portEXIT_CRITICAL_SAFE(&periph_spinlock); } static uint32_t get_clk_en_mask(periph_module_t periph) From da41885e85313c2ca722f930966a39e81507dc0a Mon Sep 17 00:00:00 2001 From: Sachin Parekh Date: Mon, 25 Mar 2019 16:06:22 +0530 Subject: [PATCH 03/10] rmt: port*_CRITICAL vanilla FreeRTOS compliance Signed-off-by: Sachin Parekh --- components/driver/rmt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/driver/rmt.c b/components/driver/rmt.c index 5bf01bbd2d..af345a4942 100644 --- a/components/driver/rmt.c +++ b/components/driver/rmt.c @@ -502,9 +502,9 @@ esp_err_t rmt_config(const rmt_config_t* rmt_param) static void IRAM_ATTR rmt_fill_memory(rmt_channel_t channel, const rmt_item32_t* item, uint16_t item_num, uint16_t mem_offset) { - portENTER_CRITICAL(&rmt_spinlock); + portENTER_CRITICAL_SAFE(&rmt_spinlock); RMT.apb_conf.fifo_mask = RMT_DATA_MODE_MEM; - portEXIT_CRITICAL(&rmt_spinlock); + portEXIT_CRITICAL_SAFE(&rmt_spinlock); int i; for(i = 0; i < item_num; i++) { RMTMEM.chan[channel].data32[i + mem_offset].val = item[i].val; From 7f37824eb740ff7dd9c541bc5864cdeda3ae083f Mon Sep 17 00:00:00 2001 From: Sachin Parekh Date: Mon, 25 Mar 2019 16:07:04 +0530 Subject: [PATCH 04/10] rtc_module: port*_CRITICAL vanilla FreeRTOS compliance Signed-off-by: Sachin Parekh --- components/driver/rtc_module.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/driver/rtc_module.c b/components/driver/rtc_module.c index 9c35d040a4..12afe08ae6 100644 --- a/components/driver/rtc_module.c +++ b/components/driver/rtc_module.c @@ -1962,15 +1962,15 @@ static void rtc_isr(void* arg) { uint32_t status = REG_READ(RTC_CNTL_INT_ST_REG); rtc_isr_handler_t* it; - portENTER_CRITICAL(&s_rtc_isr_handler_list_lock); + portENTER_CRITICAL_ISR(&s_rtc_isr_handler_list_lock); SLIST_FOREACH(it, &s_rtc_isr_handler_list, next) { if (it->mask & status) { - portEXIT_CRITICAL(&s_rtc_isr_handler_list_lock); + portEXIT_CRITICAL_ISR(&s_rtc_isr_handler_list_lock); (*it->handler)(it->handler_arg); - portENTER_CRITICAL(&s_rtc_isr_handler_list_lock); + portENTER_CRITICAL_ISR(&s_rtc_isr_handler_list_lock); } } - portEXIT_CRITICAL(&s_rtc_isr_handler_list_lock); + portEXIT_CRITICAL_ISR(&s_rtc_isr_handler_list_lock); REG_WRITE(RTC_CNTL_INT_CLR_REG, status); } From 59790863796d00cc1c3864cf978b30185ba55f11 Mon Sep 17 00:00:00 2001 From: Sachin Parekh Date: Mon, 25 Mar 2019 16:08:28 +0530 Subject: [PATCH 05/10] timer: port*_CRITICAL vanilla FreeRTOS compliance Signed-off-by: Sachin Parekh --- components/driver/timer.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/components/driver/timer.c b/components/driver/timer.c index 9fa3d5516f..8b1532fbd0 100644 --- a/components/driver/timer.c +++ b/components/driver/timer.c @@ -39,19 +39,19 @@ static const char* TIMER_TAG = "timer_group"; static timg_dev_t *TG[2] = {&TIMERG0, &TIMERG1}; static portMUX_TYPE timer_spinlock[TIMER_GROUP_MAX] = {portMUX_INITIALIZER_UNLOCKED, portMUX_INITIALIZER_UNLOCKED}; -#define TIMER_ENTER_CRITICAL(mux) portENTER_CRITICAL(mux); -#define TIMER_EXIT_CRITICAL(mux) portEXIT_CRITICAL(mux); +#define TIMER_ENTER_CRITICAL(mux) portENTER_CRITICAL_SAFE(mux); +#define TIMER_EXIT_CRITICAL(mux) portEXIT_CRITICAL_SAFE(mux); esp_err_t timer_get_counter_value(timer_group_t group_num, timer_idx_t timer_num, uint64_t* timer_val) { TIMER_CHECK(group_num < TIMER_GROUP_MAX, TIMER_GROUP_NUM_ERROR, ESP_ERR_INVALID_ARG); TIMER_CHECK(timer_num < TIMER_MAX, TIMER_NUM_ERROR, ESP_ERR_INVALID_ARG); TIMER_CHECK(timer_val != NULL, TIMER_PARAM_ADDR_ERROR, ESP_ERR_INVALID_ARG); - portENTER_CRITICAL(&timer_spinlock[group_num]); + portENTER_CRITICAL_SAFE(&timer_spinlock[group_num]); TG[group_num]->hw_timer[timer_num].update = 1; *timer_val = ((uint64_t) TG[group_num]->hw_timer[timer_num].cnt_high << 32) | (TG[group_num]->hw_timer[timer_num].cnt_low); - portEXIT_CRITICAL(&timer_spinlock[group_num]); + portEXIT_CRITICAL_SAFE(&timer_spinlock[group_num]); return ESP_OK; } @@ -154,10 +154,10 @@ esp_err_t timer_get_alarm_value(timer_group_t group_num, timer_idx_t timer_num, TIMER_CHECK(group_num < TIMER_GROUP_MAX, TIMER_GROUP_NUM_ERROR, ESP_ERR_INVALID_ARG); TIMER_CHECK(timer_num < TIMER_MAX, TIMER_NUM_ERROR, ESP_ERR_INVALID_ARG); TIMER_CHECK(alarm_value != NULL, TIMER_PARAM_ADDR_ERROR, ESP_ERR_INVALID_ARG); - portENTER_CRITICAL(&timer_spinlock[group_num]); + portENTER_CRITICAL_SAFE(&timer_spinlock[group_num]); *alarm_value = ((uint64_t) TG[group_num]->hw_timer[timer_num].alarm_high << 32) | (TG[group_num]->hw_timer[timer_num].alarm_low); - portEXIT_CRITICAL(&timer_spinlock[group_num]); + portEXIT_CRITICAL_SAFE(&timer_spinlock[group_num]); return ESP_OK; } From f3db0b5a4a9b3e76845d28af9d90cb809dd14cae Mon Sep 17 00:00:00 2001 From: Sachin Parekh Date: Mon, 25 Mar 2019 16:09:21 +0530 Subject: [PATCH 06/10] crosscore_init: port*_CRITICAL vanilla FreeRTOS compliance Signed-off-by: Sachin Parekh --- components/esp32/crosscore_int.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/esp32/crosscore_int.c b/components/esp32/crosscore_int.c index 3802a5600e..986deb2845 100644 --- a/components/esp32/crosscore_int.c +++ b/components/esp32/crosscore_int.c @@ -96,9 +96,9 @@ void esp_crosscore_int_init() { static void IRAM_ATTR esp_crosscore_int_send(int core_id, uint32_t reason_mask) { assert(core_id Date: Mon, 25 Mar 2019 16:09:55 +0530 Subject: [PATCH 07/10] intr_alloc: port*_CRITICAL vanilla FreeRTOS compliance Signed-off-by: Sachin Parekh --- components/esp32/intr_alloc.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/components/esp32/intr_alloc.c b/components/esp32/intr_alloc.c index 35e6023976..856cebb55f 100644 --- a/components/esp32/intr_alloc.c +++ b/components/esp32/intr_alloc.c @@ -494,7 +494,7 @@ static void IRAM_ATTR shared_intr_isr(void *arg) { vector_desc_t *vd=(vector_desc_t*)arg; shared_vector_desc_t *sh_vec=vd->shared_vec_info; - portENTER_CRITICAL(&spinlock); + portENTER_CRITICAL_ISR(&spinlock); while(sh_vec) { if (!sh_vec->disabled) { if ((sh_vec->statusreg == NULL) || (*sh_vec->statusreg & sh_vec->statusmask)) { @@ -512,7 +512,7 @@ static void IRAM_ATTR shared_intr_isr(void *arg) } sh_vec=sh_vec->next; } - portEXIT_CRITICAL(&spinlock); + portEXIT_CRITICAL_ISR(&spinlock); } #if CONFIG_SYSVIEW_ENABLE @@ -520,7 +520,7 @@ static void IRAM_ATTR shared_intr_isr(void *arg) static void IRAM_ATTR non_shared_intr_isr(void *arg) { non_shared_isr_arg_t *ns_isr_arg=(non_shared_isr_arg_t*)arg; - portENTER_CRITICAL(&spinlock); + portENTER_CRITICAL_ISR(&spinlock); traceISR_ENTER(ns_isr_arg->source+ETS_INTERNAL_INTR_SOURCE_OFF); // FIXME: can we call ISR and check port_switch_flag after releasing spinlock? // when CONFIG_SYSVIEW_ENABLE = 0 ISRs for non-shared IRQs are called without spinlock @@ -529,7 +529,7 @@ static void IRAM_ATTR non_shared_intr_isr(void *arg) if (!port_switch_flag[xPortGetCoreID()]) { traceISR_EXIT(); } - portEXIT_CRITICAL(&spinlock); + portEXIT_CRITICAL_ISR(&spinlock); } #endif @@ -794,7 +794,7 @@ int esp_intr_get_cpu(intr_handle_t handle) esp_err_t IRAM_ATTR esp_intr_enable(intr_handle_t handle) { if (!handle) return ESP_ERR_INVALID_ARG; - portENTER_CRITICAL(&spinlock); + portENTER_CRITICAL_SAFE(&spinlock); int source; if (handle->shared_vector_desc) { handle->shared_vector_desc->disabled=0; @@ -810,14 +810,14 @@ esp_err_t IRAM_ATTR esp_intr_enable(intr_handle_t handle) if (handle->vector_desc->cpu!=xPortGetCoreID()) return ESP_ERR_INVALID_ARG; //Can only enable these ints on this cpu ESP_INTR_ENABLE(handle->vector_desc->intno); } - portEXIT_CRITICAL(&spinlock); + portEXIT_CRITICAL_SAFE(&spinlock); return ESP_OK; } esp_err_t IRAM_ATTR esp_intr_disable(intr_handle_t handle) { if (!handle) return ESP_ERR_INVALID_ARG; - portENTER_CRITICAL(&spinlock); + portENTER_CRITICAL_SAFE(&spinlock); int source; bool disabled = 1; if (handle->shared_vector_desc) { @@ -850,7 +850,7 @@ esp_err_t IRAM_ATTR esp_intr_disable(intr_handle_t handle) } ESP_INTR_DISABLE(handle->vector_desc->intno); } - portEXIT_CRITICAL(&spinlock); + portEXIT_CRITICAL_SAFE(&spinlock); return ESP_OK; } From a190b527ac3642ea54464e16303752f49665adb9 Mon Sep 17 00:00:00 2001 From: Sachin Parekh Date: Mon, 25 Mar 2019 16:11:56 +0530 Subject: [PATCH 08/10] power_management: port*_CRITICAL vanilla FreeRTOS compliance Signed-off-by: Sachin Parekh --- components/esp32/pm_esp32.c | 4 ++-- components/esp_common/src/pm_locks.c | 11 ++++------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/components/esp32/pm_esp32.c b/components/esp32/pm_esp32.c index 9442b6619a..746a1ca400 100644 --- a/components/esp32/pm_esp32.c +++ b/components/esp32/pm_esp32.c @@ -265,7 +265,7 @@ void IRAM_ATTR esp_pm_impl_switch_mode(pm_mode_t mode, { bool need_switch = false; uint32_t mode_mask = BIT(mode); - portENTER_CRITICAL(&s_switch_lock); + portENTER_CRITICAL_SAFE(&s_switch_lock); uint32_t count; if (lock_or_unlock == MODE_LOCK) { count = ++s_mode_lock_counts[mode]; @@ -292,7 +292,7 @@ void IRAM_ATTR esp_pm_impl_switch_mode(pm_mode_t mode, s_last_mode_change_time = now; #endif // WITH_PROFILING } - portEXIT_CRITICAL(&s_switch_lock); + portEXIT_CRITICAL_SAFE(&s_switch_lock); if (need_switch && new_mode != s_mode) { do_switch(new_mode); } diff --git a/components/esp_common/src/pm_locks.c b/components/esp_common/src/pm_locks.c index 79d8fbefee..0784676da1 100644 --- a/components/esp_common/src/pm_locks.c +++ b/components/esp_common/src/pm_locks.c @@ -111,7 +111,7 @@ esp_err_t IRAM_ATTR esp_pm_lock_acquire(esp_pm_lock_handle_t handle) return ESP_ERR_INVALID_ARG; } - portENTER_CRITICAL(&handle->spinlock); + portENTER_CRITICAL_SAFE(&handle->spinlock); if (handle->count++ == 0) { pm_time_t now = 0; #ifdef WITH_PROFILING @@ -123,7 +123,7 @@ esp_err_t IRAM_ATTR esp_pm_lock_acquire(esp_pm_lock_handle_t handle) handle->times_taken++; #endif } - portEXIT_CRITICAL(&handle->spinlock); + portEXIT_CRITICAL_SAFE(&handle->spinlock); return ESP_OK; } @@ -137,7 +137,7 @@ esp_err_t IRAM_ATTR esp_pm_lock_release(esp_pm_lock_handle_t handle) return ESP_ERR_INVALID_ARG; } esp_err_t ret = ESP_OK; - portENTER_CRITICAL(&handle->spinlock); + portENTER_CRITICAL_SAFE(&handle->spinlock); if (handle->count == 0) { ret = ESP_ERR_INVALID_STATE; goto out; @@ -151,11 +151,10 @@ esp_err_t IRAM_ATTR esp_pm_lock_release(esp_pm_lock_handle_t handle) esp_pm_impl_switch_mode(handle->mode, MODE_UNLOCK, now); } out: - portEXIT_CRITICAL(&handle->spinlock); + portEXIT_CRITICAL_SAFE(&handle->spinlock); return ret; } - esp_err_t esp_pm_dump_locks(FILE* stream) { #ifndef CONFIG_PM_ENABLE @@ -201,5 +200,3 @@ esp_err_t esp_pm_dump_locks(FILE* stream) #endif return ESP_OK; } - - From d803465ec6a9a709658a96d5edcf7733eda299aa Mon Sep 17 00:00:00 2001 From: Sachin Parekh Date: Mon, 25 Mar 2019 16:14:09 +0530 Subject: [PATCH 09/10] ref_clock: port*_CRITICAL vanilla FreeRTOS compliance Signed-off-by: Sachin Parekh --- tools/unit-test-app/components/test_utils/ref_clock.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/unit-test-app/components/test_utils/ref_clock.c b/tools/unit-test-app/components/test_utils/ref_clock.c index 668a79e3da..3a25a6227c 100644 --- a/tools/unit-test-app/components/test_utils/ref_clock.c +++ b/tools/unit-test-app/components/test_utils/ref_clock.c @@ -130,10 +130,10 @@ void ref_clock_init() static void IRAM_ATTR pcnt_isr(void* arg) { - portENTER_CRITICAL(&s_lock); + portENTER_CRITICAL_ISR(&s_lock); PCNT.int_clr.val = BIT(REF_CLOCK_PCNT_UNIT); s_milliseconds += REF_CLOCK_PRESCALER_MS; - portEXIT_CRITICAL(&s_lock); + portEXIT_CRITICAL_ISR(&s_lock); } void ref_clock_deinit() From e6a714480dd357384b5ed073b6f8cb5f3d008001 Mon Sep 17 00:00:00 2001 From: Sachin Parekh Date: Mon, 25 Mar 2019 16:15:02 +0530 Subject: [PATCH 10/10] unit-test-app: freertos_compliance config added Signed-off-by: Sachin Parekh --- .gitlab-ci.yml | 98 ++++++++++++++++++- components/driver/test/test_spi_master.c | 3 + components/freertos/Kconfig | 3 +- .../freertos/include/freertos/portmacro.h | 50 +++++++--- components/freertos/test/test_spinlocks.c | 4 +- .../unit-test-app/configs/freertos_compliance | 2 + 6 files changed, 142 insertions(+), 18 deletions(-) create mode 100644 tools/unit-test-app/configs/freertos_compliance diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b8b6421146..48a9624ef3 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1668,6 +1668,12 @@ UT_006_03: - UT_T1_GPIO UT_006_04: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_GPIO + +UT_006_05: <<: *unit_test_template tags: - ESP32_IDF @@ -1693,6 +1699,12 @@ UT_007_03: - UT_T1_PCNT UT_007_04: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_PCNT + +UT_007_05: <<: *unit_test_template tags: - ESP32_IDF @@ -1718,6 +1730,12 @@ UT_008_03: - UT_T1_LEDC UT_008_04: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_LEDC + +UT_008_05: <<: *unit_test_template tags: - ESP32_IDF @@ -1743,6 +1761,12 @@ UT_009_03: - UT_T2_RS485 UT_009_04: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T2_RS485 + +UT_009_05: <<: *unit_test_template tags: - ESP32_IDF @@ -1768,6 +1792,12 @@ UT_010_03: - UT_T1_RMT UT_010_04: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_RMT + +UT_010_05: <<: *unit_test_template tags: - ESP32_IDF @@ -1846,6 +1876,12 @@ UT_013_03: - Example_SPI_Multi_device UT_013_04: + <<: *unit_test_template + tags: + - ESP32_IDF + - Example_SPI_Multi_device + +UT_013_05: <<: *unit_test_template tags: - ESP32_IDF @@ -1871,6 +1907,12 @@ UT_014_03: - UT_T2_I2C UT_014_04: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T2_I2C + +UT_014_05: <<: *unit_test_template tags: - ESP32_IDF @@ -1896,6 +1938,12 @@ UT_015_03: - UT_T1_MCPWM UT_015_04: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_MCPWM + +UT_015_05: <<: *unit_test_template tags: - ESP32_IDF @@ -1921,6 +1969,12 @@ UT_016_03: - UT_T1_I2S UT_016_04: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_I2S + +UT_016_05: <<: *unit_test_template tags: - ESP32_IDF @@ -1950,9 +2004,15 @@ UT_017_04: tags: - ESP32_IDF - UT_T2_1 - - psram UT_017_05: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T2_1 + - psram + +UT_017_06: <<: *unit_test_template tags: - ESP32_IDF @@ -1977,6 +2037,42 @@ UT_601_01: - ESP32_IDF - UT_T1_1 +UT_601_02: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_1 + +UT_601_03: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_1 + +UT_601_04: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_1 + +UT_601_05: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_1 + +UT_601_06: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_1 + +UT_601_07: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_1 + IT_001_01: <<: *test_template tags: diff --git a/components/driver/test/test_spi_master.c b/components/driver/test/test_spi_master.c index 5b69689107..ef34f978f8 100644 --- a/components/driver/test/test_spi_master.c +++ b/components/driver/test/test_spi_master.c @@ -992,6 +992,9 @@ static IRAM_ATTR void spi_transmit_polling_measure(spi_device_handle_t spi, spi_ TEST_CASE("spi_speed","[spi]") { +#ifdef CONFIG_FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE + return; +#endif uint32_t t_flight; //to get rid of the influence of randomly interrupts, we measured the performance by median value uint32_t t_flight_sorted[TEST_TIMES]; diff --git a/components/freertos/Kconfig b/components/freertos/Kconfig index 7eeec02b93..777ca7b604 100644 --- a/components/freertos/Kconfig +++ b/components/freertos/Kconfig @@ -430,7 +430,8 @@ menu "FreeRTOS" bool "Tests compliance with Vanilla FreeRTOS port*_CRITICAL calls" default n help - If enabled, context of port*_CRITICAL calls (ISR or Non-ISR) would be checked to be in compliance with Vanilla FreeRTOS. + If enabled, context of port*_CRITICAL calls (ISR or Non-ISR) + would be checked to be in compliance with Vanilla FreeRTOS. e.g Calling port*_CRITICAL from ISR context would cause assert failure endmenu diff --git a/components/freertos/include/freertos/portmacro.h b/components/freertos/include/freertos/portmacro.h index 88da1c0c0e..183d38a4dc 100644 --- a/components/freertos/include/freertos/portmacro.h +++ b/components/freertos/include/freertos/portmacro.h @@ -220,14 +220,25 @@ void vTaskExitCritical( portMUX_TYPE *mux, const char *function, int line ); #ifdef CONFIG_FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE /* Calling port*_CRITICAL from ISR context would cause an assert failure. * If the parent function is called from both ISR and Non-ISR context then call port*_CRITICAL_SAFE - **/ -#define portENTER_CRITICAL(mux) do { \ - configASSERT(!xPortInIsrContext()); \ - vTaskEnterCritical(mux, __FUNCTION__, __LINE__); \ + */ +#define portENTER_CRITICAL(mux) do { \ + if(!xPortInIsrContext()) { \ + vTaskEnterCritical(mux, __FUNCTION__, __LINE__); \ + } else { \ + ets_printf("%s:%d (%s)- port*_CRITICAL called from ISR context!\n", __FILE__, __LINE__, \ + __FUNCTION__); \ + abort(); \ + } \ } while(0) -#define portEXIT_CRITICAL(mux) do { \ - configASSERT(!xPortInIsrContext()); \ - vTaskExitCritical(mux, __FUNCTION__, __LINE__); \ + +#define portEXIT_CRITICAL(mux) do { \ + if(!xPortInIsrContext()) { \ + vTaskExitCritical(mux, __FUNCTION__, __LINE__); \ + } else { \ + ets_printf("%s:%d (%s)- port*_CRITICAL called from ISR context!\n", __FILE__, __LINE__, \ + __FUNCTION__); \ + abort(); \ + } \ } while(0) #else #define portENTER_CRITICAL(mux) vTaskEnterCritical(mux, __FUNCTION__, __LINE__) @@ -254,14 +265,25 @@ void vPortCPUReleaseMutex(portMUX_TYPE *mux); #ifdef CONFIG_FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE /* Calling port*_CRITICAL from ISR context would cause an assert failure. * If the parent function is called from both ISR and Non-ISR context then call port*_CRITICAL_SAFE - **/ -#define portENTER_CRITICAL(mux) do { \ - configASSERT(!xPortInIsrContext()); \ - vTaskEnterCritical(mux); \ + */ +#define portENTER_CRITICAL(mux) do { \ + if(!xPortInIsrContext()) { \ + vTaskEnterCritical(mux); \ + } else { \ + ets_printf("%s:%d (%s)- port*_CRITICAL called from ISR context!\n", __FILE__, __LINE__, \ + __FUNCTION__); \ + abort(); \ + } \ } while(0) -#define portEXIT_CRITICAL(mux) do { \ - configASSERT(!xPortInIsrContext()); \ - vTaskExitCritical(mux); \ + +#define portEXIT_CRITICAL(mux) do { \ + if(!xPortInIsrContext()) { \ + vTaskExitCritical(mux); \ + } else { \ + ets_printf("%s:%d (%s)- port*_CRITICAL called from ISR context!\n", __FILE__, __LINE__, \ + __FUNCTION__); \ + abort(); \ + } \ } while(0) #else #define portENTER_CRITICAL(mux) vTaskEnterCritical(mux) diff --git a/components/freertos/test/test_spinlocks.c b/components/freertos/test/test_spinlocks.c index f64ca26047..c6fb6bcf08 100644 --- a/components/freertos/test/test_spinlocks.c +++ b/components/freertos/test/test_spinlocks.c @@ -40,8 +40,8 @@ TEST_CASE("portMUX spinlocks (no contention)", "[freertos]") BENCHMARK_START(); for (int i = 0; i < REPEAT_OPS; i++) { - portENTER_CRITICAL(&mux); - portEXIT_CRITICAL(&mux); + portENTER_CRITICAL_ISR(&mux); + portEXIT_CRITICAL_ISR(&mux); } BENCHMARK_END("no contention lock"); diff --git a/tools/unit-test-app/configs/freertos_compliance b/tools/unit-test-app/configs/freertos_compliance new file mode 100644 index 0000000000..2d7a6202c2 --- /dev/null +++ b/tools/unit-test-app/configs/freertos_compliance @@ -0,0 +1,2 @@ +TEST_COMPONENTS=driver esp32 spi_flash +CONFIG_FREERTOS_CHECK_PORT_CRITICAL_COMPLIANCE=y