Hi, i have discovered a bug in portRESTORE_CONTEXT() in the AVR32 port of FreeRTOS. The issue relates to both the official port and the one in 'atmel software framework'.

Problem: The portRESTORE_CONTEXT() call does not restore the status register and PC in one atomic operation. It first restores SR, then the PC. When restoring the SR, the interrupts will be enabled before the correct program counter has been set. This will allow any pending interrupt to be processed, and when the interrupt routine finishes the stack have been altered, which eventually will cause the call to set the program counter to crash the CPU.

Solution: By using the 'rets' instruction, the processor will pop both the SR and PC off the stack in one operation. And the pending interrupt will first be allowed to run after the PC have been set.

Here's a patch:

--- a/thirdparty/freertos/freertos-8.0.0/Source/portable/GCC/AVR32_UC3/portmacro.h
+++ b/thirdparty/freertos/freertos-8.0.0/Source/portable/GCC/AVR32_UC3/portmacro.h
@@ -215,18 +215,7 @@ extern void *pvPortRealloc( void *pv, size_t xSize );
     /* Restore R0..R7 */
     "ldm     sp++, r0-r7
-    /* R0-R7 should not be used below this line */
-    /* Skip PC and SR (will do it at the end) */
-    "sub     sp, -2*4
-    /* Restore R8..R12 and LR */
-    "ldm     sp++, r8-r12, lr
-    /* Restore SR */
-    "ld.w    r0, sp[-8*4]\n\t" /* R0 is modified, is restored later. */
-    "mtsr    %[SR], r0
-    /* Restore r0 */
-    "ld.w    r0, sp[-9*4]
-    /* Restore PC */
-    "ld.w    pc, sp[-7*4]" /* Get PC from stack - PC is the 7th register saved */
+    "rets"
     :
     : [ulCriticalNesting] "i" (&ulCriticalNesting),
       [pxCurrentTCB] "i" (&pxCurrentTCB),

Note: I know that this will also not set the R8...R12 and LR, but doing so would require a change of the initialStackLayout to set SR and PC at the top, not in the middle. But it proves the issue.

Steps to reproduce: 1) Setup freertos on avr32 target 2) Setup a perihperal interrupt in main.c. Do not enable the global interrupt. 3) Trigger the interrupt source, thus causing an interrupt to be pending. 4) call vTaskStartScheduler()

Problematic behaviour: 5) The system executes the ISR, then hangs.

Expected behaviour: 5) The system executes the ISR, then continues to the highest priority task.

Regards, Johan Christiansen

Hi Johan,

Thanks for this patch!

I must admit that I have never set up any interrupt before starting the scheduler. My main() would just configure the clocks and memory and start the first task.

If you still want to load the complete initial stack (including R8...R12 and LR), and also use the rets instruction, this patch could be used:

--- a/portmacro.h +++ b/portmacro.h @@ -219,14 +219,9 @@ extern void pvPortRealloc( void *pv, size_t xSize ); / Skip PC and SR (will do it at the end) / "sub sp, -24" /* Restore R8..R12 and LR / - "ldm sp++, r8-r12, lr" - / Restore SR / - "ld.w r0, sp[-84]nt" /* R0 is modified, is restored later. / - "mtsr %[SR], r0" - / Restore r0 / - "ld.w r0, sp[-94]" - /* Restore PC / - "ld.w pc, sp[-74]" /* Get PC from stack - PC is the 7th register saved */ + "ldm sp, r8-r12, lr" + "sub sp, 8" + "rets : : [ulCriticalNesting] "i" (&ulCriticalNesting),

1.9.1

Regards, Hein

Thank you for your addition to my patch, together that will actually make a complete patch that we might get pushed upstream.

Just a single comment, shouldn't it be:

ldm sp++, r8-r12, lr
sub sp, 8
rets

By omitting the sp++, we might also just

ldm sp, r8-r12, lr
rets

Or am i wrong? I'm not currently at work and able to test it.

Hi Johan,

Maybe my patch was a little confusing. I didn't know yet how to insert literal text in a post (between two lines containing 6 tildes).

I'll put it as a patch on top of your patch:

~~~~~~ --- a/portmacro.h +++ b/portmacro.h /* Restore R0..R7 / ldm sp++, r0-r7 + sub sp, -8 + / Restore R8..R12 and LR / + ldm sp, r8-r12, lr + sub sp, 8 / Pop SR and PC */

~~~~~~

Writing "ldm sp++" with 6 registers would increase the stack pointer too much: 6x4 bytes in stead of 8

Regards, Hein