Hi, I’m using the FreeRTOS 8.2.3 port for the microblaze.
I’m concerned that a context switch at the wrong time could cause an interrupt enable or disable to be lost.
vPortEnableInterrupt calls XIntcEnable without calling portENTERCRITICAL.
Within XIntc_Enable, there is a read-modify-write of the IER interrupt enable register.
Should this be a critical section? If not, can you explain?
Cheers,
Neil
Interrupt Enable/Disable in Microblaze Port
Interrupt Enable/Disable in Microblaze Port
Just from your explanation, without looking at the code, I don’t think a
critical section is not required because the task takes a copy of the
IER register in the read part of the read modify write. If it were to
then be switched out before the write part then it could only run again
if the interrupt enable/state was the same as when it read the register.
Eg. If interrupts were enabled when it read the register, then it was
swapped out and the next task to run disabled interrupts, then a context
switch back to the original task could not take place until interrupts
were enabled again, at which time the IER register would be back to the
value the original task has already read (at least the interrupt enable
bit).
Some ports take a copy of the interrupt enable state during a context
switch anyway, and restore it before the task runs again – depends on
how the port is implemented.
Interrupt Enable/Disable in Microblaze Port
I’m not sure I explained it well, here’s a more specific example:
Two tasks are both attempting to enable a different interrupt in the same interrupt controller.
Task1 is preempted in the middle of its call to XIntcEnable (from intcv3_4) Task1: Read IER <-- 0x80 (initial value in ier from interrupt controller) task1: or bit 2 local copy --> 0x84 …. context switch due to, say a timer event Task2: Read IER <-- 0x80 (initial value in ier) task2: or bit 3 local copy --> 0x88 // task enables IRQ3 Task2: Write IER <– 0x88 …. context switch back to Task1 Task1: Write IER <– 0x84 * problem, Task2’s enable got lost From XIntcEnable, the context switch would have to occur between these two lines: ~~~ CurrentIER = XIntcIn32(InstancePtr->BaseAddress + XINIEROFFSET); XIntcOut32(InstancePtr->BaseAddress + XINIER_OFFSET, (CurrentIER | Mask)); ~~~
Task1 is preempted in the middle of its call to XIntcEnable (from intcv3_4) Task1: Read IER <-- 0x80 (initial value in ier from interrupt controller) task1: or bit 2 local copy --> 0x84 …. context switch due to, say a timer event Task2: Read IER <-- 0x80 (initial value in ier) task2: or bit 3 local copy --> 0x88 // task enables IRQ3 Task2: Write IER <– 0x88 …. context switch back to Task1 Task1: Write IER <– 0x84 * problem, Task2’s enable got lost From XIntcEnable, the context switch would have to occur between these two lines: ~~~ CurrentIER = XIntcIn32(InstancePtr->BaseAddress + XINIEROFFSET); XIntcOut32(InstancePtr->BaseAddress + XINIER_OFFSET, (CurrentIER | Mask)); ~~~
Interrupt Enable/Disable in Microblaze Port
Task1: Read IER <-- 0x80 (initial value in ier from interrupt controller) task1: or bit 2 local copy --> 0x84Is this bit in the RTOS code? Probably in the portDISABLE_INTERRUPTS macro.
…. context switch due to, say a timer event Task2: Read IER <-- 0x80 (initial value in ier) task2: or bit 3 local copy --> 0x88 // task enables IRQ3 Task2: Write IER <– 0x88Is this bit in user code (that is, outside of the RTOS code)?
…. context switch back to Task1 Task1: Write IER <– 0x84 * problem, Task2’s enable got lostIf the answer to both these questions is ‘yes’ then the user can wrap their access to the IER register in a critical section. If both the accesses are within the RTOS code then (again from your description rather than by looking at the code) I would agree the enabling of the IRQ should be in a critical section.
Interrupt Enable/Disable in Microblaze Port
Yes, those bits are in the Xilinx interrupt controller driver, which is called from the RTOS code.
How should I go about reporting this possible bug to FreeRTOS?
Interrupt Enable/Disable in Microblaze Port
How should I go about reporting this possible bug to FreeRTOS?You already have ;o)
Interrupt Enable/Disable in Microblaze Port
Thanks!
Would you please look at the code and confirm that there really is a problem that needs to be fixed?
I proposed a code change in port.c to fix this, but I’m getting some pushback from my colleagues who find it hard to believe the Microblaze FreeRTOS port would have this problem.
I proposed a code change in port.c to fix this, but I’m getting some pushback from my colleagues who find it hard to believe the Microblaze FreeRTOS port would have this problem.
Interrupt Enable/Disable in Microblaze Port
Will do, but won’t be able to do so immediately. I’ll report back here.
Interrupt Enable/Disable in Microblaze Port
A fix that just wraps the single call to XIntc_Enable() inside a critical section was pushed to the public SVN repository today.