[PATCH 2/3] ARM: make disable_fiq macro optional

On Wed, Feb 08, 2012 at 01:58:37PM -0600, Rob Herring wrote:

On 02/08/2012 02:38 AM, Russell King - ARM Linux wrote:

quoted

On Tue, Feb 07, 2012 at 04:52:14PM -0600, Rob Herring wrote:

quoted

Wouldn't the fiq be masked then? rpc_init_irq masks out the interrupts
in the same register as disable_fiq macro:

iomd_writeb(0, IOMD_FIQMASK);

The point of the stuff in disable_fiq is to catch cases where the FIQMASK
register hasn't been disabled, and we receive a spurious FIQ.  What happens
in that case (without code in disable_fiq) is that we will endlessly spin
entering and re-entering the FIQ code.

No normal interrupts will be received, and no non-FIQ handler instructions
will ever be executed.

Without this, we're 100% reliant on the FIQMASK register being correctly
set.

I would suggest that other platforms which _can_ receive FIQs should
implement the disable_fiq macro for safety against these kinds of silent
lockups.  It shouldn't be needed in the same way that printascii()
shouldn't be needed.

Couldn't this be fixed generically by masking FIQ in the SPSR rather
than the source?

Yes, but that then makes it a lot harder to actually enable FIQs when
you want them re-enabled (eg, because you want FIQs to work.)

The problem is, the FIQ state will be cached in any local_irq_save()
call, and restored on any local_irq_restore() call.  That's rather a big
problem if your FIQ is being used as a software-driven DMA.

What we _could_ do is make the non-CONFIG_FIQ handler disable the FIQ
state - basically, if CONFIG_FIQ is off, we assume that no one will
_ever_ make use of FIQ.

Otherwise, if FIQ is enabled, then we require the platform to provide
a disable_fiq macro.

Nico suggested adding a default handler with set_fiq_handler which would
do this same write to IOMD_FIQMASK. Or I can leave the ifdef for RPC
around disable_fiq. Guidance with what you would like to see here would
be helpful.

I don't think so - you either end up leaving a window where we disable
FIQs in the CPSR (which could get propagated to other threads) or you
leave a window where an errant FIQ could lock the system.