2001-02-02 12:24:12

by Maciej W. Rozycki

[permalink] [raw]
Subject: Re: [patch] 2.4.0, 2.4.0-ac12: APIC lock-ups

On Thu, 1 Feb 2001, Andrew Morton wrote:

> Your latest patch passes all my testing.
>
> 2.4.1+irq-whacker+netperf: APIC dies instantly
> 2.4.1+irq-whacker+netperf+patch: 8 million interrupts, then I got bored.

Linus, would you please apply the following patch for 2.4.2? The idea of
operation is described in the comment below.

Maciej

--
+ Maciej W. Rozycki, Technical University of Gdansk, Poland +
+--------------------------------------------------------------+
+ e-mail: [email protected], PGP key available +

patch-2.4.0-io_apic-4
diff -up --recursive --new-file linux-2.4.0.macro/arch/i386/kernel/apic.c linux-2.4.0/arch/i386/kernel/apic.c
--- linux-2.4.0.macro/arch/i386/kernel/apic.c Wed Dec 13 23:54:27 2000
+++ linux-2.4.0/arch/i386/kernel/apic.c Sun Jan 28 08:58:02 2001
@@ -270,7 +270,7 @@ void __init setup_local_APIC (void)
* PCI Ne2000 networking cards and PII/PIII processors, dual
* BX chipset. ]
*/
-#if 0
+#if 1
/* Enable focus processor (bit==0) */
value &= ~(1<<9);
#else
diff -up --recursive --new-file linux-2.4.0.macro/arch/i386/kernel/io_apic.c linux-2.4.0/arch/i386/kernel/io_apic.c
--- linux-2.4.0.macro/arch/i386/kernel/io_apic.c Thu Oct 5 21:08:17 2000
+++ linux-2.4.0/arch/i386/kernel/io_apic.c Tue Jan 30 07:49:01 2001
@@ -122,8 +122,27 @@ static void add_pin_to_irq(unsigned int
static void name##_IO_APIC_irq (unsigned int irq) \
__DO_ACTION(R, ACTION, FINAL)

-DO_ACTION( __mask, 0, |= 0x00010000, io_apic_sync(entry->apic))/* mask = 1 */
-DO_ACTION( __unmask, 0, &= 0xfffeffff, ) /* mask = 0 */
+/*
+ * It appears there is an erratum which affects at least the 82093AA
+ * I/O APIC. If a level-triggered interrupt input is being masked in
+ * the redirection entry while the interrupt is send pending (its
+ * delivery status bit is set), the interrupt is erroneously
+ * delivered as edge-triggered but the IRR bit gets set nevertheless.
+ * As a result the I/O unit expects an EOI message but it will never
+ * arrive and further interrupts are blocked for the source.
+ *
+ * A workaround is to set the trigger mode to edge when masking
+ * a level-triggered interrupt and to revert the mode when unmasking.
+ * The idea is from Manfred Spraul. --macro
+ */
+DO_ACTION( __mask, 0, |= 0x00010000,
+ ) /* mask = 1 */
+DO_ACTION( __unmask, 0, &= 0xfffeffff,
+ io_apic_sync(entry->apic)) /* mask = 0 */
+DO_ACTION( __mask_level, 0, = (reg & 0xffff7fff) | 0x00010000,
+ io_apic_sync(entry->apic)) /* mask = 1, trigger = edge */
+DO_ACTION( __unmask_level, 0, = (reg & 0xfffeffff) | 0x00008000,
+ ) /* mask = 0, trigger = level */

static void mask_IO_APIC_irq (unsigned int irq)
{
@@ -143,6 +162,24 @@ static void unmask_IO_APIC_irq (unsigned
spin_unlock_irqrestore(&ioapic_lock, flags);
}

+static void mask_level_IO_APIC_irq (unsigned int irq)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&ioapic_lock, flags);
+ __mask_level_IO_APIC_irq(irq);
+ spin_unlock_irqrestore(&ioapic_lock, flags);
+}
+
+static void unmask_level_IO_APIC_irq (unsigned int irq)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&ioapic_lock, flags);
+ __unmask_level_IO_APIC_irq(irq);
+ spin_unlock_irqrestore(&ioapic_lock, flags);
+}
+
void clear_IO_APIC_pin(unsigned int apic, unsigned int pin)
{
struct IO_APIC_route_entry entry;
@@ -1181,14 +1218,18 @@ static void end_edge_ioapic_irq (unsigne
*/
static unsigned int startup_level_ioapic_irq (unsigned int irq)
{
- unmask_IO_APIC_irq(irq);
+ unmask_level_IO_APIC_irq(irq);

return 0; /* don't check for pending */
}

-#define shutdown_level_ioapic_irq mask_IO_APIC_irq
-#define enable_level_ioapic_irq unmask_IO_APIC_irq
-#define disable_level_ioapic_irq mask_IO_APIC_irq
+#define shutdown_level_ioapic_irq mask_level_IO_APIC_irq
+#define enable_level_ioapic_irq unmask_level_IO_APIC_irq
+#define disable_level_ioapic_irq mask_level_IO_APIC_irq
+
+#define shutdown_level_82489dx_irq mask_IO_APIC_irq
+#define enable_level_82489dx_irq unmask_IO_APIC_irq
+#define disable_level_82489dx_irq mask_IO_APIC_irq

static void end_level_ioapic_irq (unsigned int i)
{
@@ -1503,6 +1544,27 @@ static inline void check_timer(void)
}

/*
+ * We can't set the trigger mode to edge when masking a
+ * level-triggered interrupt in the 82489DX I/O APIC as
+ * no deassert message will be sent in this case and a
+ * local APIC may keep delivering the interrupt to a CPU.
+ * Hence we substitute generic versions for affected
+ * handlers.
+ */
+
+static inline void setup_IO_APIC_irq_handlers(void)
+{
+ struct IO_APIC_reg_01 reg_01;
+
+ *(int *)&reg_01 = io_apic_read(0, 1);
+ if (reg_01.version < 0x10) {
+ ioapic_level_irq_type.shutdown = shutdown_level_82489dx_irq;
+ ioapic_level_irq_type.enable = enable_level_82489dx_irq;
+ ioapic_level_irq_type.disable = disable_level_82489dx_irq;
+ }
+}
+
+/*
*
* IRQ's that are handled by the old PIC in all cases:
* - IRQ2 is the cascade IRQ, and cannot be a io-apic IRQ.
@@ -1520,6 +1582,8 @@ static inline void check_timer(void)

void __init setup_IO_APIC(void)
{
+ setup_IO_APIC_irq_handlers();
+
enable_IO_APIC();

io_apic_irqs = ~PIC_IRQS;


2001-02-02 19:15:08

by Gérard Roudier

[permalink] [raw]
Subject: Re: [patch] 2.4.0, 2.4.0-ac12: APIC lock-ups



On Fri, 2 Feb 2001, Maciej W. Rozycki wrote:

> On Thu, 1 Feb 2001, Andrew Morton wrote:

> +/*
> + * It appears there is an erratum which affects at least the 82093AA
> + * I/O APIC. If a level-triggered interrupt input is being masked in
> + * the redirection entry while the interrupt is send pending (its
> + * delivery status bit is set), the interrupt is erroneously
> + * delivered as edge-triggered but the IRR bit gets set nevertheless.
> + * As a result the I/O unit expects an EOI message but it will never
> + * arrive and further interrupts are blocked for the source.
> + *
> + * A workaround is to set the trigger mode to edge when masking
> + * a level-triggered interrupt and to revert the mode when unmasking.
> + * The idea is from Manfred Spraul. --macro
> + */

Is the below idea feasible or just stupid:

Once a level-sensitive interrupt has been accepted by a local APIC, the IO
APIC will wait for the EIO message prior to delivering again this
interrupt. Therefore masking a level-sensitive interrupt once it has been
delivered and prior to EIOing it should not race with the APIC hardware.

So, why not using a pure software flag in memory and only tampering the
things if the offending interrupt is actually delivered ? If the given
interrupt is delivered and the software mask is set we could simply do:

- MASK the given interrupt
- EOI it.
- return

G?rard.

2001-02-02 22:21:40

by Manfred Spraul

[permalink] [raw]
Subject: Re: [patch] 2.4.0, 2.4.0-ac12: APIC lock-ups

quick enable_level_ioapic_irq for 16.
irq 0x16 seen on cpu 1.
quick enable_level_ioapic_irq for 16.
irq 0x16 seen on cpu 0.
irq 0x16 seen on cpu 1.
quick enable_level_ioapic_irq for 16.
irq 0x16 seen on cpu 0.
irq 0x16 seen on cpu 1.
irq 0x16 seen on cpu 0.
quick enable_level_ioapic_irq for 16.
irq 0x16 seen on cpu 1.
physically disabling int 16.
physically reenabling int 16, writing a9a1h.
overwriting 129a1h.
new val f9a1h.
irq 0x16 seen on cpu 1.
quick enable_level_ioapic_irq for 16.
NETDEV WATCHDOG: eth0: transmit timed out
eth0: Tx timed out, lost interrupt? TSR=0x3, ISR=0x3, t=37.
quick enable_level_ioapic_irq for 16.
quick enable_level_ioapic_irq for 16.
quick enable_level_ioapic_irq for 16.
NETDEV WATCHDOG: eth0: transmit timed out
eth0: Tx timed out, lost interrupt? TSR=0x3, ISR=0x3, t=40.
quick enable_level_ioapic_irq for 16.


Attachments:
patch-ger2 (4.76 kB)
dmesg-ger2 (847.00 B)
Download all attachments

2001-02-03 11:30:14

by Gérard Roudier

[permalink] [raw]
Subject: Re: [patch] 2.4.0, 2.4.0-ac12: APIC lock-ups



On Fri, 2 Feb 2001, Manfred Spraul wrote:

> G?rard Roudier wrote:
> >
> > So, why not using a pure software flag in memory and only tampering the
> > things if the offending interrupt is actually delivered ? If the given
> > interrupt is delivered and the software mask is set we could simply do:
> >
> > - MASK the given interrupt
> > - EOI it.
> > - return
> >
> Good idea.
> I implemented it, and it was a full success: not it always locks up :-(
>
> If I apply the attached patch, then I get a lockup after ~ 100 packets
> flood ping.
> I've also attached the dmesg print.
> I know that startup is currently wrong (must set trigger to level), but
> that doesn't matter since I only ifup once.
>
> But I think we can change the bug description:
>
> If an io apic io redirection entry is unmasked while the irq pin is
> active, then the io apic sends out the interrupt as edge triggered, but
> nevertheless sets the IRR bit.

Interesting.

My little finger tells me that O/Ses that thread interrupts might well
want to rely on MASK + EOI in order to quiesce incoming level-sensitive
interrupts.

Note that tampering the IO/APIC after initializations looks extremally
ugly to me. In my opinion, only the local APIC was intended by Intel
designers to be accessed by CPU after initialization (I may be wrong
here).

> In a second test run I checked the TMR bit in the local apics: the bit
> on the cpu that received the last interrupt is really 0.
>
> I'll not try a 2 step enable:
> * unmask.
> * io_apic_sync()
> * set trigger mode to level.

Thanks for having tried my suggestion.

G?rard.

2001-02-03 12:15:09

by Manfred Spraul

[permalink] [raw]
Subject: Re: [patch] 2.4.0, 2.4.0-ac12: APIC lock-ups

Manfred Spraul wrote:
>
> But I think we can change the bug description:
>
> If an io apic io redirection entry is unmasked while the irq pin is
> active, then the io apic sends out the interrupt as edge triggered, but
> nevertheless sets the IRR bit.
>

I found another workaround:
8390.c currently calls

outb_p(ENISR_ALL, e8390_base + EN0_IMR);
enable_irq(dev->irq);

and locks up after ~ 100 packets flood ping.

If I reorder these calls to

enable_irq(dev->irq);
outb_p(ENISR_ALL, e8390_base + EN0_IMR);
(and the correct spin_lock()'s)

the lockup disappears.

But I have no idea how io_apic.c could prevent lockups.

Playing with the trigger mode is not 100% reliable - I assume it kicks
the io apic only after several changes of the trigger mode bit. Maciej's
patch switches that bit twice during every start_tx operation and thus
doesn't lock up, my patch touches the redirection entry exactly once and
reliably locks up - even if I change trigger mode, polarity, delivery
mode and vector during enable_level_irq().

Any ideas?

--
Manfred

2001-02-03 14:46:03

by Manfred Spraul

[permalink] [raw]
Subject: [test patch] reliable apic lockup with one enable/disable_irq()

// $Header$
// Kernel Version:
// VERSION = 2
// PATCHLEVEL = 4
// SUBLEVEL = 1
// EXTRAVERSION =
--- 2.4/drivers/net/tulip/tulip_core.c Sat Feb 3 14:02:54 2001
+++ build-2.4/drivers/net/tulip/tulip_core.c Sat Feb 3 14:04:58 2001
@@ -421,6 +421,24 @@

tulip_up (dev);

+ disable_irq(dev->irq);
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ schedule_timeout(10);
+
+{
+ long ioaddr = dev->base_addr;
+ long csr7 = inl(ioaddr + CSR7);
+ outl(NormalIntr|AbnormalIntr|TPLnkPass|TPLnkFail, ioaddr + CSR7);
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ schedule_timeout(10);
+ enable_irq(dev->irq);
+
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ schedule_timeout(10);
+ outl(csr7, ioaddr + CSR7);
+
+}
+
netif_start_queue (dev);

return 0;
--- 2.4/arch/i386/kernel/io_apic.c Sat Feb 3 14:02:24 2001
+++ build-2.4/arch/i386/kernel/io_apic.c Sat Feb 3 14:54:14 2001
@@ -693,7 +693,7 @@
printk(KERN_WARNING " to [email protected]\n");
}

-void __init print_IO_APIC(void)
+void print_IO_APIC(void)
{
int apic, i;
struct IO_APIC_reg_00 reg_00;
@@ -1189,14 +1189,22 @@

#define shutdown_level_ioapic_irq mask_IO_APIC_irq
#define enable_level_ioapic_irq unmask_IO_APIC_irq
-#define disable_level_ioapic_irq mask_IO_APIC_irq
+static void disable_level_ioapic_irq(unsigned int i)
+{
+ /* delayed. */
+}

static void end_level_ioapic_irq (unsigned int i)
{
ack_APIC_irq();
}

-static void mask_and_ack_level_ioapic_irq (unsigned int i) { /* nothing */ }
+static void mask_and_ack_level_ioapic_irq (unsigned int i)
+{
+ if (irq_desc[i].status & IRQ_DISABLED) {
+ mask_IO_APIC_irq(i);
+ }
+}

static void set_ioapic_affinity (unsigned int irq, unsigned long mask)
{
--- 2.4/arch/i386/kernel/apic.c Sat Feb 3 14:02:24 2001
+++ build-2.4/arch/i386/kernel/apic.c Sat Feb 3 14:42:47 2001
@@ -270,7 +270,7 @@
* PCI Ne2000 networking cards and PII/PIII processors, dual
* BX chipset. ]
*/
-#if 0
+#if 1
/* Enable focus processor (bit==0) */
value &= ~(1<<9);
#else
--- 2.4/drivers/char/sysrq.c Mon Dec 4 02:48:19 2000
+++ build-2.4/drivers/char/sysrq.c Sat Feb 3 14:33:51 2001
@@ -137,6 +137,10 @@
send_sig_all(SIGKILL, 1);
orig_log_level = 8;
break;
+ case 'q':
+ print_all_local_APICs();
+ print_IO_APIC();
+ break;
default: /* Unknown: help */
if (kbd)
printk("unRaw ");








Attachments:
patch-hang (2.31 kB)

2001-02-05 10:44:15

by Maciej W. Rozycki

[permalink] [raw]
Subject: Re: [patch] 2.4.0, 2.4.0-ac12: APIC lock-ups

On Sat, 3 Feb 2001, [ISO-8859-1] G?rard Roudier wrote:

> Note that tampering the IO/APIC after initializations looks extremally
> ugly to me. In my opinion, only the local APIC was intended by Intel
> designers to be accessed by CPU after initialization (I may be wrong
> here).

In "82489DX Datasheet" Intel explicitly points to masking and unmasking
an interrupt pin in an I/O APIC as one of three ways of controlling
incoming interrupts (other two being the Task Priority Register in a local
APIC and the IF flag in a CPU) at run time. So far this is about the only
exhaustive APIC architecture description (a few further hints are also
present in "AP-388 82489DX User's Manual" but the datasheet is mostly a
superset). I haven't seen any other APIC architecture description -- all
others are mostly register programming guidelines only.

Neither of these documents are available online, AFAIK. Last year I
asked Intel if providing electronic copies is possible, but they replied
it's not.

--
+ Maciej W. Rozycki, Technical University of Gdansk, Poland +
+--------------------------------------------------------------+
+ e-mail: [email protected], PGP key available +

2001-02-05 11:51:02

by Maciej W. Rozycki

[permalink] [raw]
Subject: Re: [patch] 2.4.0, 2.4.0-ac12: APIC lock-ups

On Sat, 3 Feb 2001, Manfred Spraul wrote:

> I found another workaround:
> 8390.c currently calls
>
> outb_p(ENISR_ALL, e8390_base + EN0_IMR);
> enable_irq(dev->irq);
>
> and locks up after ~ 100 packets flood ping.
>
> If I reorder these calls to
>
> enable_irq(dev->irq);
> outb_p(ENISR_ALL, e8390_base + EN0_IMR);
> (and the correct spin_lock()'s)
>
> the lockup disappears.

Is it possible that asserting the IRQ when the mask is active makes it be
mishandled?

> Playing with the trigger mode is not 100% reliable - I assume it kicks
> the io apic only after several changes of the trigger mode bit. Maciej's
> patch switches that bit twice during every start_tx operation and thus
> doesn't lock up, my patch touches the redirection entry exactly once and
> reliably locks up - even if I change trigger mode, polarity, delivery
> mode and vector during enable_level_irq().

I believe I recover from the lockup -- it's the mask function that
recovers. But another one happens at the unmask time, I suppose.

> Any ideas?

I'll implement the IRQ unlocker I was thinking first. The idea is not to
try to prevent the lockup from happening as it might even be impossible
but to make the unlocker trigger after a lockup happens instead. I should
have an implementation ready soon.

--
+ Maciej W. Rozycki, Technical University of Gdansk, Poland +
+--------------------------------------------------------------+
+ e-mail: [email protected], PGP key available +