Re: [PATCH] Fix race in the ARM IRQ handler.

[Sergei Organov <osv at javad dot com>]

Andrew Lunn <andrew@lunn.ch> writes:
> On Mon, Feb 06, 2006 at 09:30:27PM +0300, Sergei Organov wrote:

[...]
>> + // We must disable FIQ here (IRQ is already disabled) as switching 
>> + // from IRQ to SVC mode will cheat FIQ handler and it will pass through 
>> + // the control back to us (should FIQ happen after the statement below) 
>> + // resulting in recursion that we aren't ready to handle, e.g., what 
>> + // we have saved on the exception stack will be clobbered.
>
> I don't realy understand the comment. 
>
> I guess it is about getting a FIQ while handling an IRQ.
> What do you mean about "cheat FIQ handler"?

I meant that FIQ handler checks for current mode to be IRQ to avoid
calling IRQ handler recursively, and changing mode to SVC in IRQ handler
will in effect "cheat" FIQ handler forcing it to believe we are not in
IRQ handling code anymore. Thus we need to disable FIQ while switching
from IRQ to any other mode (in fact not necessarily to SVC).

Though I agree my comment is not very clear.

>
> How about the following comment
>
> // When handling an IRQ we must disable FIQ.
> // If we where to get a FIQ while in supervisor mode, the FIQ handling
            ^ typo? did you mean "were"?
> // code would transforms the FIQ into an IRQ and call this section of
                         ^ typo? did you mean "transform"?
> // code again. The stack pointer would be set to the beginning of the
> // exception_stack, so clobbering the registers we have just saved.

Ah, you did understand the comment indeed ;)

Then how about your version slightly modified. The intent of
modification is emphasizing that clobbering of the registers saved on
exception stack is only an example of what makes IRQ handler
non-reentrant as I'm almost sure there are other issues.

Also I tried to avoid mentioning SVC mode as any other mode (but IRQ)
will in fact have the same effect:

// When handling an IRQ we must disable FIQ unless current mode in CPSR
// is IRQ. If we were to get a FIQ while in another mode, the FIQ
// handling code would transform the FIQ into an IRQ and call the
// (non-reentrant) IRQ handler again. As a result, for example, the
// stack pointer would be set to the beginning of the exception_stack
// clobbering the registers we have just saved.

-- Sergei.