Merge branch 'bugfix/freertos_fpu_isr_pins_task_v4.4' into 'release/v4.4'

FreeRTOS: Fix bug where FPU usage in ISR pins the interrupted task (v4.4)

See merge request espressif/esp-idf!20627
This commit is contained in:
Zim Kalinowski 2022-11-09 18:39:28 +08:00
commit 2b0de33816
4 changed files with 328 additions and 85 deletions

View File

@ -903,34 +903,32 @@ _xt_coproc_exc:
/* Get co-processor state save area of new owner thread. */
call0 XT_RTOS_CP_STATE /* a15 = new owner's save area */
#ifndef CONFIG_FREERTOS_FPU_IN_ISR
beqz a15, .L_goto_invalid
#if CONFIG_FREERTOS_FPU_IN_ISR
beqz a15, .L_skip_core_pin /* CP used in ISR, skip task pinning */
#else
beqz a15, .L_goto_invalid /* not in a thread (invalid) */
#endif
/*When FPU in ISR is enabled we could deal with zeroed a15 */
#if configNUM_CORES > 1
/* CP operations are incompatible with unpinned tasks. Thus we pin the task
to the current running core. */
movi a2, pxCurrentTCB
getcoreid a3 /* a3 = current core ID */
addx4 a2, a3, a2
l32i a2, a2, 0 /* a2 = start of pxCurrentTCB[cpuid] */
addi a2, a2, TASKTCB_XCOREID_OFFSET /* a2 = &TCB.xCoreID */
s32i a3, a2, 0 /* TCB.xCoreID = current core ID */
#endif // configNUM_CORES > 1
#if CONFIG_FREERTOS_FPU_IN_ISR
.L_skip_core_pin:
#endif
/* Enable the co-processor's bit in CPENABLE. */
movi a0, _xt_coproc_mask
rsr a4, CPENABLE /* a4 = CPENABLE */
addx4 a0, a5, a0 /* a0 = &_xt_coproc_mask[n] */
l32i a0, a0, 0 /* a0 = (n << 16) | (1 << n) */
/* FPU operations are incompatible with non-pinned tasks. If we have a FPU operation
here, to keep the entire thing from crashing, it's better to pin the task to whatever
core we're running on now. */
movi a2, pxCurrentTCB
getcoreid a3
addx4 a2, a3, a2
l32i a2, a2, 0 /* a2 = start of pxCurrentTCB[cpuid] */
addi a2, a2, TASKTCB_XCOREID_OFFSET /* offset to xCoreID in tcb struct */
s32i a3, a2, 0 /* store current cpuid */
/* Grab correct xt_coproc_owner_sa for this core */
movi a2, XCHAL_CP_MAX << 2
mull a2, a2, a3 /* multiply by current processor id */
movi a3, _xt_coproc_owner_sa /* a3 = base of owner array */
add a3, a3, a2 /* a3 = owner area needed for this processor */
extui a2, a0, 0, 16 /* coprocessor bitmask portion */
or a4, a4, a2 /* a4 = CPENABLE | (1 << n) */
wsr a4, CPENABLE
@ -941,6 +939,11 @@ everywhere): _xt_coproc_release assumes it works like this in order not to need
locking.
*/
/* Grab correct xt_coproc_owner_sa for this core */
movi a2, XCHAL_CP_MAX << 2
mull a2, a2, a3 /* multiply by current processor id */
movi a3, _xt_coproc_owner_sa /* a3 = base of owner array */
add a3, a3, a2 /* a3 = owner area needed for this processor */
/* Get old coprocessor owner thread (save area ptr) and assign new one. */
addx4 a3, a5, a3 /* a3 = &_xt_coproc_owner_sa[n] */

View File

@ -1,65 +0,0 @@
#include <esp_types.h>
#include <stdio.h>
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "freertos/semphr.h"
#include "freertos/queue.h"
#include "esp_intr_alloc.h"
#include "unity.h"
#include "soc/cpu.h"
#include "test_utils.h"
#include "math.h"
#define SW_ISR_LEVEL_1 7
#ifdef CONFIG_FREERTOS_FPU_IN_ISR
struct fp_test_context {
SemaphoreHandle_t sync;
float expected;
};
static void software_isr(void *arg) {
(void)arg;
BaseType_t yield;
xt_set_intclear(1 << SW_ISR_LEVEL_1);
struct fp_test_context *ctx = (struct fp_test_context *)arg;
for(int i = 0; i < 16; i++) {
ctx->expected = ctx->expected * 2.0f * cosf(0.0f);
}
xSemaphoreGiveFromISR(ctx->sync, &yield);
if(yield) {
portYIELD_FROM_ISR();
}
}
TEST_CASE("Floating point usage in ISR test", "[freertos]" "[fp]")
{
struct fp_test_context ctx;
float fp_math_operation_result = 0.0f;
intr_handle_t handle;
esp_err_t err = esp_intr_alloc(ETS_INTERNAL_SW0_INTR_SOURCE, ESP_INTR_FLAG_LEVEL1, &software_isr, &ctx, &handle);
TEST_ASSERT_EQUAL_HEX32(ESP_OK, err);
ctx.sync = xSemaphoreCreateBinary();
TEST_ASSERT(ctx.sync != NULL);
ctx.expected = 1.0f;
fp_math_operation_result = cosf(0.0f);
xt_set_intset(1 << SW_ISR_LEVEL_1);
xSemaphoreTake(ctx.sync, portMAX_DELAY);
esp_intr_free(handle);
vSemaphoreDelete(ctx.sync);
printf("FP math isr result: %f \n", ctx.expected);
TEST_ASSERT_FLOAT_WITHIN(0.1f, ctx.expected, fp_math_operation_result * 65536.0f);
}
#endif

View File

@ -0,0 +1,146 @@
/*
* SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#include "sdkconfig.h"
#include <math.h>
#include "soc/soc_caps.h"
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "freertos/semphr.h"
#include "unity.h"
#include "test_utils.h"
#if SOC_CPU_HAS_FPU && CONFIG_FREERTOS_FPU_IN_ISR
// We can use xtensa API here as currently, non of the RISC-V targets have an FPU
#include "xtensa/xtensa_api.h"
#include "esp_intr_alloc.h"
#define SW_ISR_LEVEL_1 7
static void fpu_isr(void *arg)
{
// Clear the interrupt
xt_set_intclear(1 << SW_ISR_LEVEL_1);
/*
Use the FPU
- We test using a calculation that will cause a change in mantissa and exponent for extra thoroughness
- cosf(0.0f) should return 1.0f, thus we are simply doubling test_float every iteration.
- Therefore, we should end up with (0.01) * (2^8) = 2.56 at the end of the loop
*/
volatile float test_float = 0.01f;
for (int i = 0; i < 8; i++) {
test_float = test_float * 2.0f * cosf(0.0f);
}
// We allow a 0.1% delta on the final result in case of any loss of precision from floating point calculations
TEST_ASSERT_FLOAT_WITHIN(0.00256f, 2.56f, test_float);
}
/* ------------------------------------------------------------------------------------------------------------------ */
/*
Test FPU usage from a level 1 ISR
Purpose:
- Test that the FPU can be used from a level 1 ISR
- Test that the ISR using the FPU does not corrupt the interrupted task's FPU context
Procedure:
- Allocate a level 1 ISR
- Task uses the FPU then triggers the ISR
- ISR uses the FPU as well (forcing the task's FPU context to be saved)
- Task continues using the FPU (forcing its FPU context to be restored)
Expected:
- ISR should use the FPU without issue
- The interrupted task can continue using the FPU without issue
*/
TEST_CASE("FPU: Usage in level 1 ISR", "[freertos]")
{
intr_handle_t isr_handle;
TEST_ASSERT_EQUAL(ESP_OK, esp_intr_alloc(ETS_INTERNAL_SW0_INTR_SOURCE, ESP_INTR_FLAG_LEVEL1, &fpu_isr, NULL, &isr_handle));
/*
Use the FPU (calculate a different value than in the ISR)
- We test using a calculation that will cause a change in mantissa and exponent for extra thoroughness
- cosf(0.0f) should return 1.0f, thus we are simply dividing test_float every iteration.
*/
// We should end up with (2.56) / (2^4) = 0.16 at the end of the first loop
volatile float test_float = 2.56f;
for (int i = 0; i < 4; i++) {
test_float = test_float / (2.0f * cosf(0.0f));
}
// We allow a 0.1% delta on the final result in case of any loss of precision from floating point calculations
TEST_ASSERT_FLOAT_WITHIN(0.00016f, 0.16f, test_float);
// Trigger the ISR
xt_set_intset(1 << SW_ISR_LEVEL_1);
// Continue using the FPU from a task context after the interrupt returns
// We should end up with (0.16) / (2^4) = 0.01 at the end of the first loop
for (int i = 0; i < 4; i++) {
test_float = test_float / (2.0f * cosf(0.0f));
}
// We allow a 0.1% delta on the final result in case of any loss of precision from floating point calculations
TEST_ASSERT_FLOAT_WITHIN(0.00001f, 0.01f, test_float);
// Free the ISR
esp_intr_free(isr_handle);
}
/* ------------------------------------------------------------------------------------------------------------------ */
/*
Test FPU usage in ISR does not affect an unpinned tasks
Purpose:
- Test that the ISR using the FPU will not affect the interrupted task's affinity
Procedure:
- Create an unpinned task
- Unpinned task disables scheduling/preemption to ensure that it does not switch cores
- Unpinned task allocates an ISR then triggers the ISR
- The ISR interrupts the unpinned task then uses the FPU
- Task reenables scheduling/preemption and cleans up
Expected:
- The ISR using the FPU will not affect the unpinned task's affinity
*/
static void unpinned_task(void *arg)
{
// Disable scheduling to make sure the current task doesn't switch cores
vTaskSuspendAll();
// Check that the task is unpinned
TEST_ASSERT_EQUAL(tskNO_AFFINITY, xTaskGetAffinity(NULL));
// Allocate an ISR to use the FPU
intr_handle_t isr_handle;
TEST_ASSERT_EQUAL(ESP_OK, esp_intr_alloc(ETS_INTERNAL_SW0_INTR_SOURCE, ESP_INTR_FLAG_LEVEL1, &fpu_isr, NULL, &isr_handle));
// Trigger the ISR
xt_set_intset(1 << SW_ISR_LEVEL_1);
// Free the ISR
esp_intr_free(isr_handle);
// Task should remain unpinned after the ISR uses the FPU
TEST_ASSERT_EQUAL(tskNO_AFFINITY, xTaskGetAffinity(NULL));
// Reenable scheduling
xTaskResumeAll();
// Indicate done and self delete
xTaskNotifyGive((TaskHandle_t)arg);
vTaskDelete(NULL);
}
TEST_CASE("FPU: Level 1 ISR does not affect unpinned task", "[freertos]")
{
TaskHandle_t unity_task_handle = xTaskGetCurrentTaskHandle();
xTaskCreate(unpinned_task, "unpin", 2048, (void *)unity_task_handle, UNITY_FREERTOS_PRIORITY + 1, NULL);
// Wait for task to complete
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
vTaskDelay(10); // Short delay to allow task memory to be freed
}
#endif // SOC_CPU_HAS_FPU && CONFIG_FREERTOS_FPU_IN_ISR

View File

@ -0,0 +1,159 @@
/*
* SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#include "sdkconfig.h"
#include <math.h>
#include "soc/soc_caps.h"
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "freertos/semphr.h"
#include "unity.h"
#include "test_utils.h"
#if SOC_CPU_HAS_FPU
/* ------------------------------------------------------------------------------------------------------------------ */
/*
Test FPU usage from a task context
Purpose:
- Test that the FPU can be used from a task context
- Test that FPU context is properly saved and restored
Procedure:
- Create TEST_PINNED_NUM_TASKS tasks pinned to each core
- Start each task
- Each task updates a float variable and then blocks (to allow other tasks to run thus forcing the an FPU context
save and restore).
Expected:
- Correct float value calculated by each task
*/
#define TEST_PINNED_NUM_TASKS 3
static void pinned_task(void *arg)
{
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
/*
Use the FPU
- We test using a calculation that will cause a change in mantissa and exponent for extra thoroughness
- cosf(0.0f) should return 1.0f, thus we are simply doubling test_float every iteration.
- Therefore, we should end up with (0.01) * (2^8) = 2.56 at the end of the loop
*/
volatile float test_float = 0.01f;
for (int i = 0; i < 8; i++) {
test_float = test_float * 2.0f * cosf(0.0f);
vTaskDelay(1); // Block to cause a context switch, forcing the FPU context to be saved
}
// We allow a 0.1% delta on the final result in case of any loss of precision from floating point calculations
TEST_ASSERT_FLOAT_WITHIN(0.00256f, 2.56f, test_float);
// Indicate done wand wait to be deleted
xSemaphoreGive((SemaphoreHandle_t)arg);
vTaskSuspend(NULL);
}
TEST_CASE("FPU: Usage in task", "[freertos]")
{
SemaphoreHandle_t done_sem = xSemaphoreCreateCounting(configNUM_CORES * TEST_PINNED_NUM_TASKS, 0);
TEST_ASSERT_NOT_EQUAL(NULL, done_sem);
TaskHandle_t task_handles[configNUM_CORES][TEST_PINNED_NUM_TASKS];
// Create test tasks for each core
for (int i = 0; i < configNUM_CORES; i++) {
for (int j = 0; j < TEST_PINNED_NUM_TASKS; j++) {
TEST_ASSERT_EQUAL(pdTRUE, xTaskCreatePinnedToCore(pinned_task, "task", 4096, (void *)done_sem, UNITY_FREERTOS_PRIORITY + 1, &task_handles[i][j], i));
}
}
// Start the created tasks simultaneously
for (int i = 0; i < configNUM_CORES; i++) {
for (int j = 0; j < TEST_PINNED_NUM_TASKS; j++) {
xTaskNotifyGive(task_handles[i][j]);
}
}
// Wait for the tasks to complete
for (int i = 0; i < configNUM_CORES * TEST_PINNED_NUM_TASKS; i++) {
xSemaphoreTake(done_sem, portMAX_DELAY);
}
// Delete the tasks
for (int i = 0; i < configNUM_CORES; i++) {
for (int j = 0; j < TEST_PINNED_NUM_TASKS; j++) {
vTaskDelete(task_handles[i][j]);
}
}
vTaskDelay(10); // Short delay to allow idle task to be free task memory and FPU contexts
vSemaphoreDelete(done_sem);
}
/* ------------------------------------------------------------------------------------------------------------------ */
/*
Test FPU usage will pin an unpinned task
Purpose:
- Test that unpinned tasks are automatically pinned to the current core on the task's first use of the FPU
Procedure:
- Create an unpinned task
- Task disables scheduling/preemption to ensure that it does not switch cores
- Task uses the FPU
- Task checks its core affinity after FPU usage
Expected:
- Task remains unpinned until its first usage of the FPU
- The task becomes pinned to the current core after first use of the FPU
*/
#if configNUM_CORES > 1
static void unpinned_task(void *arg)
{
// Disable scheduling to make sure current core ID doesn't change
vTaskSuspendAll();
BaseType_t cur_core_num = xPortGetCoreID();
// Check that the task is unpinned
TEST_ASSERT_EQUAL(tskNO_AFFINITY, xTaskGetAffinity(NULL));
/*
Use the FPU
- We test using a calculation that will cause a change in mantissa and exponent for extra thoroughness
- cosf(0.0f) should return 1.0f, thus we are simply doubling test_float every iteration.
- Therefore, we should end up with (0.01) * (2^8) = 2.56 at the end of the loop
*/
volatile float test_float = 0.01f;
for (int i = 0; i < 8; i++) {
test_float = test_float * 2.0f * cosf(0.0f);
}
// We allow a 0.1% delta on the final result in case of any loss of precision from floating point calculations
TEST_ASSERT_FLOAT_WITHIN(0.00256f, 2.56f, test_float);
TEST_ASSERT_EQUAL(cur_core_num, xTaskGetAffinity(NULL));
// Reenable scheduling
xTaskResumeAll();
// Indicate done and self delete
xTaskNotifyGive((TaskHandle_t)arg);
vTaskDelete(NULL);
}
TEST_CASE("FPU: Usage in unpinned task", "[freertos]")
{
TaskHandle_t unity_task_handle = xTaskGetCurrentTaskHandle();
// Create unpinned task
xTaskCreate(unpinned_task, "unpin", 4096, (void *)unity_task_handle, UNITY_FREERTOS_PRIORITY + 1, NULL);
// Wait for task to complete
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
vTaskDelay(10); // Short delay to allow task memory to be freed
}
#endif // configNUM_CORES > 1
#endif // SOC_CPU_HAS_FPU