feedc0de/esp-idf
1
0
Fork 0
forked from espressif/esp-idf
Code Pull Requests Activity

Merge branch 'bugfix/fpu_registers_overwritten' into 'master'

Browse Source 
Xtensa: fix a bug that altered CPU registers in FPU exception handlers

Closes IDFGH-10439

See merge request espressif/esp-idf!24518
This commit is contained in:
Omar Chebib
2023-07-05 17:28:53 +08:00
parent b0b722da33 2aee63a773
commit d2471b11e7
2 changed files with 77 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

65
components/freertos/test_apps/freertos/port/test_fpu_in_task.c
View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -185,5 +185,68 @@ TEST_CASE("FPU: Usage in unpinned task", "[freertos]")
}
}
typedef struct {
bool negative;
TaskHandle_t main;
} ParamsFPU;
/**
* @brief Function performing some simple calculation using several FPU registers.
* The goal is to be preempted by a task that also uses the FPU on the same core.
*/
void fpu_calculation(void* arg)
{
ParamsFPU* p = (ParamsFPU*) arg;
const bool negative = p->negative;
const float init = negative ? -1.f : 1.f;
float f = init;
for(int i = 0; i < 10; i++)
{
/* The following calculation doesn't really have any meaning, we try to use several FPU registers and operations */
float delta = negative ? -1.1f : 1.1f;
for (int i = 0; i < 1000; i++) {
f += delta;
delta += negative ? -0.1f : 0.1f;
}
/* Give some time to the other to interrupt us before checking `f` value */
esp_rom_delay_us(1000);
/**
* If the FPU context was not saved properly by FreeRTOS, our value `f` will not have to correct sign!
* It'll have the sign of the other tasks' `f` value.
* Use assert to make sure the sign is correct. Using TEST_ASSERT_TRUE triggers a stack overflow.
*/
assert( (negative && f < 0.0f) || (!negative && f > 0.0f) );
f = init;
/* Give the hand back to FreeRTOS to avoid any watchdog */
vTaskDelay(2);
}
xTaskNotifyGive(p->main);
vTaskDelete(NULL);
}
TEST_CASE("FPU: Unsolicited context switch between tasks using FPU", "[freertos]")
{
/* Create two tasks that are on the same core and use the same FPU */
TaskHandle_t unity_task_handle = xTaskGetCurrentTaskHandle();
TaskHandle_t tasks[2];
ParamsFPU params[2] = {
{ .negative = false, .main = unity_task_handle },
{ .negative = true, .main = unity_task_handle },
};
xTaskCreatePinnedToCore(fpu_calculation, "Task1", 2048, params + 0, UNITY_FREERTOS_PRIORITY + 1, &tasks[0], 1);
xTaskCreatePinnedToCore(fpu_calculation, "Task2", 2048, params + 1, UNITY_FREERTOS_PRIORITY + 2, &tasks[2], 1);
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
}
#endif // configNUM_CORES > 1
#endif // SOC_CPU_HAS_FPU

16
components/xtensa/xtensa_vectors.S
View File

@ -3,7 +3,7 @@
*
* SPDX-License-Identifier: MIT
*
* SPDX-FileContributor: 2016-2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileContributor: 2016-2023 Espressif Systems (Shanghai) CO LTD
*/
/*
* Copyright (c) 2015-2019 Cadence Design Systems, Inc.
@ -1016,6 +1016,12 @@ _xt_coproc_exc:
s32i a2, sp, XT_STK_A2
s32i a3, sp, XT_STK_A3
s32i a4, sp, XT_STK_A4
#if portNUM_PROCESSORS > 1
/* If multicore, we must save two more interruptee's register to use as
* scratch when taking/releasing the _xt_coproc_owner_sa_lock spinlock. */
s32i a6, sp, XT_STK_A6
s32i a7, sp, XT_STK_A7
#endif /* portNUM_PROCESSORS > 1 */
s32i a15, sp, XT_STK_A15
/* Call the RTOS coprocessor exception hook */
@ -1046,7 +1052,7 @@ _xt_coproc_exc:
#if portNUM_PROCESSORS > 1
/* If multicore, we must also acquire the _xt_coproc_owner_sa_lock spinlock
* to ensure thread safe access of _xt_coproc_owner_sa between cores. */
spinlock_take a0 a2 _xt_coproc_owner_sa_lock
spinlock_take a6 a7 _xt_coproc_owner_sa_lock
#endif /* portNUM_PROCESSORS > 1 */
/* Get old coprocessor owner thread (save area ptr) and assign new one. */
@ -1057,7 +1063,7 @@ _xt_coproc_exc:
#if portNUM_PROCESSORS > 1
/* Release previously taken spinlock */
spinlock_release a0 a2 _xt_coproc_owner_sa_lock
spinlock_release a6 a7 _xt_coproc_owner_sa_lock
#endif /* portNUM_PROCESSORS > 1 */
/* Only need to context switch if new owner != old owner. */
@ -1140,6 +1146,10 @@ _xt_coproc_exc:
/* Can omit rsync for wsr.CPENABLE here because _xt_user_exit does it. */
.L_xt_coproc_done:
l32i a15, sp, XT_STK_A15
#if portNUM_PROCESSORS > 1
l32i a6, sp, XT_STK_A6
l32i a7, sp, XT_STK_A7
#endif /* portNUM_PROCESSORS > 1 */
l32i a5, sp, XT_STK_A5
l32i a4, sp, XT_STK_A4
l32i a3, sp, XT_STK_A3