[pxe] Avoid potential interrupt storms when using shared interrupts master
authorMichael Brown <mcb30@etherboot.org>
Tue, 23 Mar 2010 00:58:52 +0000 (00:58 +0000)
committerMichael Brown <mcb30@etherboot.org>
Tue, 23 Mar 2010 01:39:52 +0000 (01:39 +0000)
Current gPXE code always returns "OURS" in response to
PXENV_UNDI_ISR:START.  This is harmless for non-shared interrupt
lines, and avoids the complexity of trying to determine whether or not
we really did cause the interrupt.  (This is a non-trivial
determination; some drivers don't have interrupt support and hook the
system timer interrupt instead, for example.)

A problem occurs when we have a shared interrupt line, the other
device asserts an interrupt, and the controlling ISR does not chain to
the other device's ISR when we return "OURS".  Under these
circumstances, the other device's ISR never executes, and so the
interrupt remains asserted, causing an interrupt storm.

Work around this by returning "OURS" if and only if our net device's
interrupt is currently recorded as being enabled.  Since we always
disable interrupts as a result of a call to PXENV_UNDI_ISR:START, this
guarantees that we will eventually (on the second call) return "NOT
OURS", allowing the other ISR to be called.  Under normal operation,
including a non-shared interrupt situation, this change will make no
difference since PXENV_UNDI_ISR:START would be called only when
interrupts were enabled anyway.

Signed-off-by: Michael Brown <mcb30@etherboot.org>
src/arch/i386/interface/pxe/pxe_undi.c

index 2f66c77..a2b0b77 100644 (file)
@@ -674,12 +674,29 @@ PXENV_EXIT_t pxenv_undi_isr ( struct s_PXENV_UNDI_ISR *undi_isr ) {
                 */
                netdev_poll ( pxe_netdev );
 
-               /* Disable interrupts to avoid interrupt storm */
+               /* A 100% accurate determination of "OURS" vs "NOT
+                * OURS" is difficult to achieve without invasive and
+                * unpleasant changes to the driver model.  We settle
+                * for always returning "OURS" if interrupts are
+                * currently enabled.
+                *
+                * Returning "NOT OURS" when interrupts are disabled
+                * allows us to avoid a potential interrupt storm when
+                * we are on a shared interrupt line; if we were to
+                * always return "OURS" then the other device's ISR
+                * may never be called.
+                */
+               if ( netdev_irq_enabled ( pxe_netdev ) ) {
+                       DBGC2 ( &pxenv_undi_isr, " OURS" );
+                       undi_isr->FuncFlag = PXENV_UNDI_ISR_OUT_OURS;
+               } else {
+                       DBGC2 ( &pxenv_undi_isr, " NOT OURS" );
+                       undi_isr->FuncFlag = PXENV_UNDI_ISR_OUT_NOT_OURS;
+               }
+
+               /* Disable interrupts */
                netdev_irq ( pxe_netdev, 0 );
 
-               /* Always say it was ours for the sake of simplicity */
-               DBGC2 ( &pxenv_undi_isr, " OURS" );
-               undi_isr->FuncFlag = PXENV_UNDI_ISR_OUT_OURS;
                break;
        case PXENV_UNDI_ISR_IN_PROCESS :
        case PXENV_UNDI_ISR_IN_GET_NEXT :