Re: Serial related oops

["Michael K. Edwards" <medwards.linux@gmail.com>]

On 2/19/07, Russell King <rmk+lkml@arm.linux.org.uk> wrote:
> On Mon, Feb 19, 2007 at 12:37:00PM -0800, Michael K. Edwards wrote:
> > What we've seen on our embedded ARM is that enabling an interrupt that
> > is shared between multiple UARTs, at a stage when you have not set up
> > all the data structures touched by the ISR and softirq, can have
> > horrible consequences, including soft lockups and fandangos on core.
>
> Incorrect.  We have:
>
> 1. registered an interrupt handler at this point.
> 2. disabled interrupts (we're under the spin lock)

setup_irq() is where things go wrong, at least for us, at least on
2.6.16.x.  Interrupts are not disabled at the point in request_irq()
when the interrupt controller is poked to enable the IRQ source.  If
you're lucky, and you're on an architecture where the UART interrupt
is properly level-triggered, and the worst thing that happens when you
attempt to service an interrupt that isn't yours is that it stays on,
then you get a soft lockup with two or three recursive __irq_svc hits
in the backtrace.  If you're not lucky you do a fandango on core.

> So, no interrupt will be seen by the CPU since the interrupt is masked.

The interrupt would need to be masked for the entire duration of the
outer loop that calls serial8250_init() or the equivalent for all
platform devices that share the IRQ.

> The test is intentionally designed to be safe from the interrupt
> generation point of view.

But its context is not.  Shared IRQ lines are a _problem_.  You cannot
safely enable an IRQ until all devices that share it have had their
ISRs installed, unless you can absolutely guarantee at a hardware
level that the unitialized ones cannot assert the IRQ line.  That does
not apply to any device that might have been touched by the bootloader
or the early init code, especially a UART.

Cheers,
- Michael