Interrupt Enable/Disable in Microblaze Port

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

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)); ~~~

Interrupt Enable/Disable in Microblaze Port

Task1: Read IER <-- 0x80 (initial value in ier from interrupt controller) task1: or bit 2 local copy --> 0x84
Is 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 <– 0x88
Is 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 lost
If 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.

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.