-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Description
Describe the bug
On target ARM Cortex-R5, I was browsing the ISR path, shown below:
FreeRTOS-Kernel/portable/GCC/ARM_CR5/portASM.S
Lines 171 to 249 in 03db672
| .align 4 | |
| .type FreeRTOS_IRQ_Handler, %function | |
| FreeRTOS_IRQ_Handler: | |
| /* Return to the interrupted instruction. */ | |
| SUB lr, lr, #4 | |
| /* Push the return address and SPSR. */ | |
| PUSH {lr} | |
| MRS lr, SPSR | |
| PUSH {lr} | |
| /* Change to supervisor mode to allow reentry. */ | |
| CPS #SVC_MODE | |
| /* Push used registers. */ | |
| PUSH {r0-r4, r12} | |
| /* Increment nesting count. r3 holds the address of ulPortInterruptNesting | |
| for future use. r1 holds the original ulPortInterruptNesting value for | |
| future use. */ | |
| LDR r3, ulPortInterruptNestingConst | |
| LDR r1, [r3] | |
| ADD r4, r1, #1 | |
| STR r4, [r3] | |
| /* Read value from the interrupt acknowledge register, which is stored in r0 | |
| for future parameter and interrupt clearing use. */ | |
| LDR r2, ulICCIARConst | |
| LDR r2, [r2] | |
| LDR r0, [r2] | |
| /* Ensure bit 2 of the stack pointer is clear. r2 holds the bit 2 value for | |
| future use. _RB_ Is this ever needed provided the start of the stack is | |
| alligned on an 8-byte boundary? */ | |
| MOV r2, sp | |
| AND r2, r2, #4 | |
| SUB sp, sp, r2 | |
| /* Call the interrupt handler. */ | |
| PUSH {r0-r4, lr} | |
| LDR r1, vApplicationIRQHandlerConst | |
| BLX r1 | |
| POP {r0-r4, lr} | |
| ADD sp, sp, r2 | |
| CPSID i | |
| DSB | |
| ISB | |
| /* Write the value read from ICCIAR to ICCEOIR. */ | |
| LDR r4, ulICCEOIRConst | |
| LDR r4, [r4] | |
| STR r0, [r4] | |
| /* Restore the old nesting count. */ | |
| STR r1, [r3] | |
| /* A context switch is never performed if the nesting count is not 0. */ | |
| CMP r1, #0 | |
| BNE exit_without_switch | |
| /* Did the interrupt request a context switch? r1 holds the address of | |
| ulPortYieldRequired and r0 the value of ulPortYieldRequired for future | |
| use. */ | |
| LDR r1, =ulPortYieldRequired | |
| LDR r0, [r1] | |
| CMP r0, #0 | |
| BNE switch_before_exit | |
| exit_without_switch: | |
| /* No context switch. Restore used registers, LR_irq and SPSR before | |
| returning. */ | |
| POP {r0-r4, r12} | |
| CPS #IRQ_MODE | |
| POP {LR} | |
| MSR SPSR_cxsf, LR | |
| POP {LR} | |
| MOVS PC, LR |
Upon entering the ISR, the LR and SPSR are pushed into the IRQ_sp respectively. Afterwards, the processor enters the SVC mode to handle the IRQ accordingly, and right before returning to the interrupted instruction, which typically lays in the System mode, the stack popping operations seems incorrectly performed:
exit_without_switch:
/* No context switch. Restore used registers, LR_irq and SPSR before
returning. */
POP {r0-r4, r12}
CPS #IRQ_MODE
POP {LR}
MSR SPSR_cxsf, LR
POP {LR}
MOVS PC, LRThe first POP {LR} is correct as the processor still operates in the IRQ mode. However, the second one uses the stack that is mostly not (say it is not nested IRQ) the IRQ_SP (stack pointer of IRQ mode), this can result in the processor jumping to unexpected instruction address due to the unexpected processor mode during the LR popping operation (right after MSR SPSR_cxsf, LR).
Target
- Instruction Set Architecture: ARMv7-R
Expected behavior
I was expecting that, before switching the processor mode back to the interrupted instruction (MSR SPSR_cxsf, LR), the return address previously stored in the IRQ_SP should be popped into the preserved registers such as r5, and use r5 to restore PC after switching back to the interrupted instruction.
Additional context
I'm using my knowledge to understand this code snippet, please feel free to criticize me.
Thanks!