From 9f7f9643637fbfff1891874b08c56fe6702436b9 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Wed, 12 Oct 2022 22:18:15 +0800 Subject: [PATCH 1/2] freertos: Fix FPU ISR core pinning bug This commit fixes a bug where if an unpinned task is interrupted by a level 1 ISR that users the FPU, the FPU usage will cause the interrupted task to become pinned to the current core. Note: This bug was already fixed in SMP FreeRTOS in commit d69361779ecb24e1d5cb8c33b8d77d87f182ca34. This commit simply backports the fix to IDF FreeRTOS. --- .../portable/xtensa/xtensa_vectors.S | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/components/freertos/FreeRTOS-Kernel/portable/xtensa/xtensa_vectors.S b/components/freertos/FreeRTOS-Kernel/portable/xtensa/xtensa_vectors.S index 35862e1fef..776bdf0ec3 100644 --- a/components/freertos/FreeRTOS-Kernel/portable/xtensa/xtensa_vectors.S +++ b/components/freertos/FreeRTOS-Kernel/portable/xtensa/xtensa_vectors.S @@ -908,35 +908,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 @@ -946,7 +943,11 @@ Keep loading _xt_coproc_owner_sa[n] atomic (=load once, then use that value 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] */ From 423fb361e793c8ce187358e953eb2ff87d6cd3a9 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Wed, 12 Oct 2022 22:14:27 +0800 Subject: [PATCH 2/2] freertos: Enable FPU ISR core pinning test for IDF FreeRTOS --- components/freertos/test/port/test_fpu_in_isr.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/components/freertos/test/port/test_fpu_in_isr.c b/components/freertos/test/port/test_fpu_in_isr.c index cc5adc571e..2917f3c0b7 100644 --- a/components/freertos/test/port/test_fpu_in_isr.c +++ b/components/freertos/test/port/test_fpu_in_isr.c @@ -106,9 +106,6 @@ Expected: - The ISR using the FPU will not affect the unpinned task's affinity */ -// Known issue in IDF FreeRTOS (IDF-6068), already fixed in SMP FreeRTOS -#if CONFIG_FREERTOS_SMP - static void unpinned_task(void *arg) { // Disable scheduling/preemption to make sure the current task doesn't switch cores @@ -159,6 +156,4 @@ TEST_CASE("FPU: Level 1 ISR does not affect unpinned task", "[freertos]") vTaskDelay(10); // Short delay to allow task memory to be freed } -#endif // CONFIG_FREERTOS_SMP - #endif // SOC_CPU_HAS_FPU && CONFIG_FREERTOS_FPU_IN_ISR