Re: Fixes for nforce2 hard lockup, apic, io-apic, udma133 covered

Re: Fixes for nforce2 hard lockup, apic, io-apic, udma133 covered

[Len Brown]

Maciej -- Sorry for the delayed response.  December was pretty busy and
there was  a window where I didn't catch up on e-mail unless [PATCH] was
in the subject.

On Thu, 2003-12-11 at 10:15, Maciej W. Rozycki wrote:
> On Thu, 11 Dec 2003, Ross Dickson wrote:
> 
> > ACPI: IOAPIC (id[0x02] address[0xfec00000] global_irq_base[0x0])
> > IOAPIC[0]: Assigned apic_id 2
> > IOAPIC[0]: apic_id 2, version 17, address 0xfec00000, IRQ 0-23
> > Bus #0 is ISA
> > Int: type 3, pol 0, trig 0, bus 0, irq 0, 2-0
> 
>  I've browsed the relevant part of the ACPI spec and the above entry
> is 
> incorrect.  It looks like INTIN0 is now the preferred line for the
> 8254 
> timer; at least it is the default one when using ACPI tables.  This is
> a 
> bug in Linux.

Int: type 3, pol 0, trig 0, bus 0, irq 0, 2-0
should be:
Int: type 0, pol 0, trig 0, bus 0, irq 0, 2-2

type should be mp_INT, not mp_ExtINT, and dstirq should be 2, not 0;
yes?

> > ACPI: INT_SRC_OVR (bus[0] irq[0x0] global_irq[0x2] polarity[0x0]
> trigger[0x0])
> > Int: type 0, pol 0, trig 0, bus 0, irq 0, 2-2
> 
>  Now this is an explicit entry stating the 8254 timer is connected to
> INTIN2.  If this is not the case, the BIOS is buggy and the solution
> is to
> fix it.  I don't consider it possible to be worked around in Linux
> except
> maybe with a command line option added manually.

Agreed.

> > ACPI: INT_SRC_OVR (bus[0] irq[0x9] global_irq[0x9] polarity[0x1]
> trigger[0x3])
> > Int: type 0, pol 1, trig 3, bus 0, irq 9, 2-9
> 
>  And yet another explicit entry which has an effect on configuration
> as
> reported below.

This is the ACPI SCI.  Mapping IRQ9 ot INTIN9 is a no-op.  The effect of
this entry is to explicitly setting the polarity/triger to high/level. 
If this were not present it would still be in IRQ9, but would be set to
the default, which is low/level.

> > IRQ to pin mappings:
> > IRQ0 -> 0:2-> 0:0
> [...]
> > IRQ9 -> 0:9-> 0:9
> 
>  These two entries are wrong -- the interrupts are set up as if they
> were
> connected to multiple I/O APIC inputs.  The first entry is a result of
> your hack, but the second one suggests a bug somewhere.

Right, this means that there are multiple irq_2_pin entries, which isn't
what we want.  Indeed, I've not seen a system were we _do_ want them --
do system actually exist where the same interrupt wires simultaneously
go to multiple interrupt input pins?

> patch-mips-2.6.0-test11-20031209-acpi-irq0-1
> diff -up --recursive --new-file
> linux-mips-2.6.0-test11-20031209.macro/arch/i386/kernel/mpparse.c
> linux-mips-2.6.0-test11-20031209/arch/i386/kernel/mpparse.c
> 
> ---
> linux-mips-2.6.0-test11-20031209.macro/arch/i386/kernel/mpparse.c  
> 2003-11-25 04:57:01.000000000 +0000
> +++ linux-mips-2.6.0-test11-20031209/arch/i386/kernel/mpparse.c
> 2003-12-11 09:43:26.000000000 +0000
> @@ -940,7 +940,7 @@ void __init mp_override_legacy_irq (
>          *      erroneously sets the trigger to level, resulting in a
> HUGE 
>          *      increase of timer interrupts!
>          */
> -       if ((bus_irq == 0) && (global_irq == 2) && (trigger == 3))
> +       if ((bus_irq == 0) && (trigger == 3))
>                 trigger = 1;
>  

This is a check for a bad BIOS that sets the timer to level triggered. 
I don't see this as the case for the nforce2 system on hand.  Indeed,
I've _never_ see this case, and I wonder if this clause can be deleted
altogether...  Did I miss something?

>         intsrc.mpc_type = MP_INTSRC;
> @@ -961,7 +961,7 @@ void __init mp_override_legacy_irq (
>          * Otherwise create a new entry (e.g. global_irq == 2).
>          */
>         for (i = 0; i < mp_irq_entries; i++) {
> -               if ((mp_irqs[i].mpc_dstapic == intsrc.mpc_dstapic) 
> +               if ((mp_irqs[i].mpc_srcbus == intsrc.mpc_srcbus) 
>                         && (mp_irqs[i].mpc_srcbusirq ==
> intsrc.mpc_srcbusirq)) {
>                         mp_irqs[i] = intsrc;
>                         found = 1;

This makes sense.  If we're over-riding the destination, we don't care
what the destination was before, ya?

this used to check dstapic and dstirq.
it was partially fixed last summer when checking srcbus replaced
checking dstirq.  Unclear why dstapic wasn't changed to srcbus at the
same time.

i think with this change the comment above the code is no longer
correct, yes?


> @@ -1008,9 +1008,10 @@ void __init mp_config_acpi_legacy_irqs (
>          */
>         for (i = 0; i < 16; i++) {
>  
> -               if (i == 2) continue;                   /* Don't
> connect IRQ2 */
> +               if (i == 2)
> +                       continue;                       /* Don't
> connect IRQ2 */

white-space -- ok.

>  
> -               intsrc.mpc_irqtype = i ? mp_INT : mp_ExtINT;   /*
> 8259A to #0 */
> +               intsrc.mpc_irqtype = mp_INT;

I believe this change is correct.  When this code runs, the system is in
APIC mode and there is no concept of vectored external PIC interrupts.

>                 intsrc.mpc_srcbusirq = i;                  /* Identity
> mapped */
>                 intsrc.mpc_dstirq = i;
>  

thanks,
-Len

Re: Fixes for nforce2 hard lockup, apic, io-apic, udma133 covered

[Maciej W. Rozycki]

On Sat, 7 Feb 2004, Len Brown wrote:

> Maciej -- Sorry for the delayed response.  December was pretty busy and
> there was  a window where I didn't catch up on e-mail unless [PATCH] was
> in the subject.

 Sorry about that -- I could have added it to the subject, indeed.

> > > ACPI: IOAPIC (id[0x02] address[0xfec00000] global_irq_base[0x0])
> > > IOAPIC[0]: Assigned apic_id 2
> > > IOAPIC[0]: apic_id 2, version 17, address 0xfec00000, IRQ 0-23
> > > Bus #0 is ISA
> > > Int: type 3, pol 0, trig 0, bus 0, irq 0, 2-0
> > 
> >  I've browsed the relevant part of the ACPI spec and the above entry
> > is 
> > incorrect.  It looks like INTIN0 is now the preferred line for the
> > 8254 
> > timer; at least it is the default one when using ACPI tables.  This is
> > a 
> > bug in Linux.
> 
> Int: type 3, pol 0, trig 0, bus 0, irq 0, 2-0
> should be:
> Int: type 0, pol 0, trig 0, bus 0, irq 0, 2-2
> 
> type should be mp_INT, not mp_ExtINT, and dstirq should be 2, not 0;
> yes?

 Nope -- it should be:

Int: type 0, pol 0, trig 0, bus 0, irq 0, 2-0

as the ACPI spec explicitly defines a 1:1 map -- see section 5.2.10.7 (I
have rev.2.0c): "This means that I/O APIC interrupt inputs 0-15 must be
mapped to global system interrupts 0-15 and have identical sources as the
8259 IRQs 0-15 unless overrides are used."

> > > ACPI: INT_SRC_OVR (bus[0] irq[0x9] global_irq[0x9] polarity[0x1]
> > trigger[0x3])
> > > Int: type 0, pol 1, trig 3, bus 0, irq 9, 2-9
> > 
> >  And yet another explicit entry which has an effect on configuration
> > as
> > reported below.
> 
> This is the ACPI SCI.  Mapping IRQ9 ot INTIN9 is a no-op.  The effect of
> this entry is to explicitly setting the polarity/triger to high/level. 
> If this were not present it would still be in IRQ9, but would be set to
> the default, which is low/level.

 Yep, just explaining the bits to Ross...

> > > IRQ to pin mappings:
> > > IRQ0 -> 0:2-> 0:0
> > [...]
> > > IRQ9 -> 0:9-> 0:9
> > 
> >  These two entries are wrong -- the interrupts are set up as if they
> > were
> > connected to multiple I/O APIC inputs.  The first entry is a result of
> > your hack, but the second one suggests a bug somewhere.
> 
> Right, this means that there are multiple irq_2_pin entries, which isn't
> what we want.  Indeed, I've not seen a system were we _do_ want them --
> do system actually exist where the same interrupt wires simultaneously
> go to multiple interrupt input pins?

 I'm told such systems exist(ed), at least in the MP-table era so code
has been added to handle them.  Technically, there's nothing wrong with
that, except that the interrupt has to be level-triggered at the APIC
level, of course.

> > @@ -940,7 +940,7 @@ void __init mp_override_legacy_irq (
> >          *      erroneously sets the trigger to level, resulting in a
> > HUGE 
> >          *      increase of timer interrupts!
> >          */
> > -       if ((bus_irq == 0) && (global_irq == 2) && (trigger == 3))
> > +       if ((bus_irq == 0) && (trigger == 3))
> >                 trigger = 1;
> 
> This is a check for a bad BIOS that sets the timer to level triggered. 
> I don't see this as the case for the nforce2 system on hand.  Indeed,
> I've _never_ see this case, and I wonder if this clause can be deleted
> altogether...  Did I miss something?

 Note that in principle the timer can be set up for a level-triggered 
operation -- this used to be the case for MCA systems that had additional 
circuitry to ack the interrupt at the source.  I'm not sure if ACPI 
supports such a configuration -- the MPS certainly did.

 Anyway, this workaround, if left in place, should be triggered for the
timer source (bus_irq == 0) regardless of the I/O APIC pin it's routed to
(global_irq).

> >         intsrc.mpc_type = MP_INTSRC;
> > @@ -961,7 +961,7 @@ void __init mp_override_legacy_irq (
> >          * Otherwise create a new entry (e.g. global_irq == 2).
> >          */
> >         for (i = 0; i < mp_irq_entries; i++) {
> > -               if ((mp_irqs[i].mpc_dstapic == intsrc.mpc_dstapic) 
> > +               if ((mp_irqs[i].mpc_srcbus == intsrc.mpc_srcbus) 
> >                         && (mp_irqs[i].mpc_srcbusirq ==
> > intsrc.mpc_srcbusirq)) {
> >                         mp_irqs[i] = intsrc;
> >                         found = 1;
> 
> This makes sense.  If we're over-riding the destination, we don't care
> what the destination was before, ya?

 Worse even -- we may replace a wrong entry.

> this used to check dstapic and dstirq.
> it was partially fixed last summer when checking srcbus replaced
> checking dstirq.  Unclear why dstapic wasn't changed to srcbus at the
> same time.
> 
> i think with this change the comment above the code is no longer
> correct, yes?

 It's still correct -- we are still replacing an [IOAPIC.PIN -> IRQ] entry
(perhaps the arrow should be reversed), except we disregard old IOAPIC.PIN
information without examining it.

> > -               intsrc.mpc_irqtype = i ? mp_INT : mp_ExtINT;   /*
> > 8259A to #0 */
> > +               intsrc.mpc_irqtype = mp_INT;
> 
> I believe this change is correct.  When this code runs, the system is in
> APIC mode and there is no concept of vectored external PIC interrupts.

 While being able to track an ExtINT source could be useful for the NMI
watchdog under certain circumstances, the ACPI spec does not recognize
this kind of interrupt and does not provide a means of expressing it in 
the tables.  So we cannot set any entry to ExtINT and the timer IRQ is an 
ordinary interrupt.  We assume LINT0 interrupts are ExtINT ones 
implicitly.

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: Fixes for nforce2 hard lockup, apic, io-apic, udma133 covered

[Len Brown]

On Sat, 2004-02-07 at 07:41, Maciej W. Rozycki wrote:
> On Sat, 7 Feb 2004, Len Brown wrote:
> 
> > Maciej -- Sorry for the delayed response.  December was pretty busy and
> > there was  a window where I didn't catch up on e-mail unless [PATCH] was
> > in the subject.
> 
>  Sorry about that -- I could have added it to the subject, indeed.
> 
> > > > ACPI: IOAPIC (id[0x02] address[0xfec00000] global_irq_base[0x0])
> > > > IOAPIC[0]: Assigned apic_id 2
> > > > IOAPIC[0]: apic_id 2, version 17, address 0xfec00000, IRQ 0-23
> > > > Bus #0 is ISA
> > > > Int: type 3, pol 0, trig 0, bus 0, irq 0, 2-0
> > > 
> > >  I've browsed the relevant part of the ACPI spec and the above entry
> > > is 
> > > incorrect.  It looks like INTIN0 is now the preferred line for the
> > > 8254 
> > > timer; at least it is the default one when using ACPI tables.  This is
> > > a 
> > > bug in Linux.
> > 
> > Int: type 3, pol 0, trig 0, bus 0, irq 0, 2-0
> > should be:
> > Int: type 0, pol 0, trig 0, bus 0, irq 0, 2-2
> > 
> > type should be mp_INT, not mp_ExtINT, and dstirq should be 2, not 0;
> > yes?
> 
>  Nope -- it should be:
> 
> Int: type 0, pol 0, trig 0, bus 0, irq 0, 2-0
> 
> as the ACPI spec explicitly defines a 1:1 map -- see section 5.2.10.7 (I
> have rev.2.0c): "This means that I/O APIC interrupt inputs 0-15 must be
> mapped to global system interrupts 0-15 and have identical sources as the
> 8259 IRQs 0-15 unless overrides are used."

ah, here is the one i was looking for -- the one from the over-ride, and
it looks correct.

> > ACPI: INT_SRC_OVR (bus[0] irq[0x0] global_irq[0x2] polarity[0x0]
> trigger[0x0])
> > Int: type 0, pol 0, trig 0, bus 0, irq 0, 2-2

I'm not sure what is going on with pin0 above -- (and below), i think
there is a workaround in this system that isn't in the base kernel? 
Indeed, I've lost track of the original failure on this thread, was it
simply that the timer came out as XT-PIC mode instead of IOAPIC-edge?


> > > > IRQ to pin mappings:
> > > > IRQ0 -> 0:2-> 0:0
> > > [...]
> > > > IRQ9 -> 0:9-> 0:9
> > > 
> > >  These two entries are wrong -- the interrupts are set up as if they
> > > were
> > > connected to multiple I/O APIC inputs.  The first entry is a result of
> > > your hack, but the second one suggests a bug somewhere.
> > 
> > Right, this means that there are multiple irq_2_pin entries, which isn't
> > what we want.  Indeed, I've not seen a system were we _do_ want them --
> > do system actually exist where the same interrupt wires simultaneously
> > go to multiple interrupt input pins?
> 
>  I'm told such systems exist(ed), at least in the MP-table era so code
> has been added to handle them.  Technically, there's nothing wrong with
> that, except that the interrupt has to be level-triggered at the APIC
> level, of course.
> 
Hmm, i wonder if any bad things go wrong for IRQ9 -> 0:9-> 0:9 -- I
guess we do all the APIC tickling twice because we thing there are two
pins associated with the IRQ.

I suspect that we have some failing systems due to this bug for when the
interrupt is _not_ identity mapped like this example -- b/c we'd ack the
wrong pin as a side-effect of an interrupt...

BTW. I've always disliked the IRQ -> pin mapping, it should really be
IRQ <- pin mapping -- or maybe more accurately the vector should be in
there...

> > > @@ -940,7 +940,7 @@ void __init mp_override_legacy_irq (
> > >          *      erroneously sets the trigger to level, resulting in a
> > > HUGE 
> > >          *      increase of timer interrupts!
> > >          */
> > > -       if ((bus_irq == 0) && (global_irq == 2) && (trigger == 3))
> > > +       if ((bus_irq == 0) && (trigger == 3))
> > >                 trigger = 1;
> > 
> > This is a check for a bad BIOS that sets the timer to level triggered. 
> > I don't see this as the case for the nforce2 system on hand.  Indeed,
> > I've _never_ see this case, and I wonder if this clause can be deleted
> > altogether...  Did I miss something?
> 
>  Note that in principle the timer can be set up for a level-triggered 
> operation -- this used to be the case for MCA systems that had additional 
> circuitry to ack the interrupt at the source.  I'm not sure if ACPI 
> supports such a configuration -- the MPS certainly did.

>  Anyway, this workaround, if left in place, should be triggered for the
> timer source (bus_irq == 0) regardless of the I/O APIC pin it's routed to
> (global_irq).

this is an ACPI specific routine, (and IMHO it is thus mis-named --
along with all the acpi-specific stuff in mpparse.c;-)  I'd be inclined
to squak about a bogus entry rather than using a silent workaround for a
system that may not actually exist.

> > >         intsrc.mpc_type = MP_INTSRC;
> > > @@ -961,7 +961,7 @@ void __init mp_override_legacy_irq (
> > >          * Otherwise create a new entry (e.g. global_irq == 2).
> > >          */
> > >         for (i = 0; i < mp_irq_entries; i++) {
> > > -               if ((mp_irqs[i].mpc_dstapic == intsrc.mpc_dstapic) 
> > > +               if ((mp_irqs[i].mpc_srcbus == intsrc.mpc_srcbus) 
> > >                         && (mp_irqs[i].mpc_srcbusirq ==
> > > intsrc.mpc_srcbusirq)) {
> > >                         mp_irqs[i] = intsrc;
> > >                         found = 1;
> > 
> > This makes sense.  If we're over-riding the destination, we don't care
> > what the destination was before, ya?
> 
>  Worse even -- we may replace a wrong entry.

oy!

> > this used to check dstapic and dstirq.
> > it was partially fixed last summer when checking srcbus replaced
> > checking dstirq.  Unclear why dstapic wasn't changed to srcbus at the
> > same time.
> > 
> > i think with this change the comment above the code is no longer
> > correct, yes?
> 
>  It's still correct -- we are still replacing an [IOAPIC.PIN -> IRQ] entry
> (perhaps the arrow should be reversed), except we disregard old IOAPIC.PIN
> information without examining it.

yes, not looking at the IOAPIC.PIN -- that is what i meant by obsolete
comment. (and the fact that the arrow points from pin to IRQ is the only
part of the comment i like;-)

> > > -               intsrc.mpc_irqtype = i ? mp_INT : mp_ExtINT;   /*
> > > 8259A to #0 */
> > > +               intsrc.mpc_irqtype = mp_INT;
> > 
> > I believe this change is correct.  When this code runs, the system is in
> > APIC mode and there is no concept of vectored external PIC interrupts.
> 
>  While being able to track an ExtINT source could be useful for the NMI
> watchdog under certain circumstances, the ACPI spec does not recognize
> this kind of interrupt and does not provide a means of expressing it in 
> the tables.  So we cannot set any entry to ExtINT and the timer IRQ is an 
> ordinary interrupt.  We assume LINT0 interrupts are ExtINT ones 
> implicitly.

ACPI has an additional MADT entry type for specifying the NMI.

thanks,
-Len

Re: Fixes for nforce2 hard lockup, apic, io-apic, udma133 covered

[Maciej W. Rozycki]

On Sat, 7 Feb 2004, Len Brown wrote:

> > > Int: type 3, pol 0, trig 0, bus 0, irq 0, 2-0
> > > should be:
> > > Int: type 0, pol 0, trig 0, bus 0, irq 0, 2-2
> > > 
> > > type should be mp_INT, not mp_ExtINT, and dstirq should be 2, not 0;
> > > yes?
> > 
> >  Nope -- it should be:
> > 
> > Int: type 0, pol 0, trig 0, bus 0, irq 0, 2-0
> > 
> > as the ACPI spec explicitly defines a 1:1 map -- see section 5.2.10.7 (I
> > have rev.2.0c): "This means that I/O APIC interrupt inputs 0-15 must be
> > mapped to global system interrupts 0-15 and have identical sources as the
> > 8259 IRQs 0-15 unless overrides are used."
> 
> ah, here is the one i was looking for -- the one from the over-ride, and
> it looks correct.
> 
> > > ACPI: INT_SRC_OVR (bus[0] irq[0x0] global_irq[0x2] polarity[0x0]
> > trigger[0x0])
> > > Int: type 0, pol 0, trig 0, bus 0, irq 0, 2-2
> 
> I'm not sure what is going on with pin0 above -- (and below), i think
> there is a workaround in this system that isn't in the base kernel? 

 The system reports the timer IRQ to be routed to pin 2, even though it is
connected to pin 0.  There's no workaround in the kernel as working it
around cannot be handled in a generic way.

> Indeed, I've lost track of the original failure on this thread, was it
> simply that the timer came out as XT-PIC mode instead of IOAPIC-edge?

 The ACPI table had no entry for the timer IRQ, so Linux reverted to 
the ExtINT (XT-PIC) mode for the IRQ as a last resort.  It's only thanks 
to ancient EISA systems the fallback is there at all and ACPI-based 
systems happened to work.

> > > > > IRQ to pin mappings:
> > > > > IRQ0 -> 0:2-> 0:0
> > > > [...]
> > > > > IRQ9 -> 0:9-> 0:9
> > > > 
> > > >  These two entries are wrong -- the interrupts are set up as if they
> > > > were
> > > > connected to multiple I/O APIC inputs.  The first entry is a result of
> > > > your hack, but the second one suggests a bug somewhere.
> > > 
> > > Right, this means that there are multiple irq_2_pin entries, which isn't
> > > what we want.  Indeed, I've not seen a system were we _do_ want them --
> > > do system actually exist where the same interrupt wires simultaneously
> > > go to multiple interrupt input pins?
> > 
> >  I'm told such systems exist(ed), at least in the MP-table era so code
> > has been added to handle them.  Technically, there's nothing wrong with
> > that, except that the interrupt has to be level-triggered at the APIC
> > level, of course.
> > 
> Hmm, i wonder if any bad things go wrong for IRQ9 -> 0:9-> 0:9 -- I
> guess we do all the APIC tickling twice because we thing there are two
> pins associated with the IRQ.

 That entry appears due to conditions making the following entry being 
reported in the log:

IOAPIC[0]: Set PCI routing entry (2-9 -> 0x71 -> IRQ 9 Mode:1 Active:0)

I don't know why the SCI interrupt is set up the second time here.  That's 
the reason the interrupt is routed twice.

 As a side note, I wonder why entries for PCI interrupts are processed so
late...

> I suspect that we have some failing systems due to this bug for when the
> interrupt is _not_ identity mapped like this example -- b/c we'd ack the
> wrong pin as a side-effect of an interrupt...

 As you can see there's only one pin recorded twice.

> BTW. I've always disliked the IRQ -> pin mapping, it should really be
> IRQ <- pin mapping -- or maybe more accurately the vector should be in
> there...

 Well, using vectors wouldn't be that bad -- I've already discovered this
once. :-)  It would require a clean-up of ISA drivers, but actually that
is desireable regardless, as there are non-i386 systems supporting the ISA
bus out there that want to use numbers outside the 0-15 range for ISA
interrupts.

> > > This is a check for a bad BIOS that sets the timer to level triggered. 
> > > I don't see this as the case for the nforce2 system on hand.  Indeed,
> > > I've _never_ see this case, and I wonder if this clause can be deleted
> > > altogether...  Did I miss something?
> > 
> >  Note that in principle the timer can be set up for a level-triggered 
> > operation -- this used to be the case for MCA systems that had additional 
> > circuitry to ack the interrupt at the source.  I'm not sure if ACPI 
> > supports such a configuration -- the MPS certainly did.
> 
> >  Anyway, this workaround, if left in place, should be triggered for the
> > timer source (bus_irq == 0) regardless of the I/O APIC pin it's routed to
> > (global_irq).
> 
> this is an ACPI specific routine, (and IMHO it is thus mis-named --
> along with all the acpi-specific stuff in mpparse.c;-)  I'd be inclined
> to squak about a bogus entry rather than using a silent workaround for a
> system that may not actually exist.

 Someone has introduced it for a reason, I suppose -- can't the originator 
be tracked down?

> >  It's still correct -- we are still replacing an [IOAPIC.PIN -> IRQ] entry
> > (perhaps the arrow should be reversed), except we disregard old IOAPIC.PIN
> > information without examining it.
> 
> yes, not looking at the IOAPIC.PIN -- that is what i meant by obsolete
> comment. (and the fact that the arrow points from pin to IRQ is the only
> part of the comment i like;-)

 If interpreted as a vector as opposed to a physical IRQ line, then I 
agree -- I'd leave that as is, then.

> >  While being able to track an ExtINT source could be useful for the NMI
> > watchdog under certain circumstances, the ACPI spec does not recognize
> > this kind of interrupt and does not provide a means of expressing it in 
> > the tables.  So we cannot set any entry to ExtINT and the timer IRQ is an 
> > ordinary interrupt.  We assume LINT0 interrupts are ExtINT ones 
> > implicitly.
> 
> ACPI has an additional MADT entry type for specifying the NMI.

 That's for LINT1 that's (almost?) universally configured for NMI
interrupts, indeed.  OTOH, LINT0 of the BSP is typically configured as
ExtINT for the virtual wire mode, although I've met a system using a
through-I/O-APIC variation of the mode as well as one using the PIC
compatibility mode (we used to support both variations; I hope it hasn't
got broken) -- both of them had LINT0 inputs simply masked out.  The
MP-table provides a way to report any sensible interrupt configurations
for LINT inputs (including ExtINT, Fixed, SMI, etc.), not only the NMI.

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

ACPI SCI IOAPIC bug (Re: Fixes for nforce2 hard lockup, apic, io-apic, udma133 covered)

[Len Brown]

On Thu, 2003-12-11 at 10:15, Maciej W. Rozycki wrote:
> On Thu, 11 Dec 2003, Ross Dickson wrote:

> > ACPI: INT_SRC_OVR (bus[0] irq[0x9] global_irq[0x9] polarity[0x1]
> trigger[0x3])
> > Int: type 0, pol 1, trig 3, bus 0, irq 9, 2-9
> 
>  ...
> 
> > IRQ to pin mappings:
...
> > IRQ9 -> 0:9-> 0:9
> 
>  ... wrong -- the interrupts are set up as if they were
> connected to multiple I/O APIC inputs.

Maciej,
You're right.  This bug is in mp_config_ioapic_for_sci(), which calls
io_apic_set_pci_routing(), which uncondnitionally calls
add_pin_to_irq().  Problem is that this IRQ has already been initialized
back in setup_IO_APIC_irqs().

Clearly in this case we shouldn't be calling io_apic_set_pci_routing()
at all.  But I've got to look more closely at the case where the SCI is
not identity mapped before simply ripping it out.

thanks,
-Len

 

Re: ACPI SCI IOAPIC bug (Re: Fixes for nforce2 hard lockup, apic, io-apic, udma133 covered)

[Maciej W. Rozycki]

On Sat, 13 Feb 2004, Len Brown wrote:

> > > ACPI: INT_SRC_OVR (bus[0] irq[0x9] global_irq[0x9] polarity[0x1]
> > trigger[0x3])
> > > Int: type 0, pol 1, trig 3, bus 0, irq 9, 2-9
> > 
> >  ...
> > 
> > > IRQ to pin mappings:
> ...
> > > IRQ9 -> 0:9-> 0:9
> > 
> >  ... wrong -- the interrupts are set up as if they were
> > connected to multiple I/O APIC inputs.
> 
> Maciej,
> You're right.  This bug is in mp_config_ioapic_for_sci(), which calls
> io_apic_set_pci_routing(), which uncondnitionally calls
> add_pin_to_irq().  Problem is that this IRQ has already been initialized
> back in setup_IO_APIC_irqs().

 Note that if changing an I/O APIC input was indeed needed, the
replace_pin_at_irq() function could be used.

> Clearly in this case we shouldn't be calling io_apic_set_pci_routing()
> at all.  But I've got to look more closely at the case where the SCI is
> not identity mapped before simply ripping it out.

 I still wonder why these arrangements are made so late in a boot -- after 
all, ACPI IRQ configuration is table-driven and does not require any 
specific hardware initialization to work.  So it could be done at the 
stage MP-table parsing happens, couldn't it?

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: ACPI SCI IOAPIC bug (Re: Fixes for nforce2 hard lockup, apic, io-apic, udma133 covered)

[Len Brown]

On Wed, 2004-02-18 at 12:43, Maciej W. Rozycki wrote:

>  Note that if changing an I/O APIC input was indeed needed, the
> replace_pin_at_irq() function could be used.

Why is it that all IRQs get their name from the
IOAPIC pin number, but the timer connected to
pin 2 is called IRQ0 instead of IRQ2?

Are there other exceptions to this rule,
or is all the code for re-naming IRQs & pins
effectively just for the timer?

I wonder if we should't be moving to at least a build option which
deletes support for multiple pins at an IRQ, and deletes
suport for non-identity pin->IRQ mapping.

>  I still wonder why these arrangements are made so late in a boot --
> after 
> all, ACPI IRQ configuration is table-driven and does not require any 
> specific hardware initialization to work.  So it could be done at the 
> stage MP-table parsing happens, couldn't it?

While the ACPI table parsing is very early, the _PRT parsing
can happen only after the ACPI interpreter is up, because
the _PRT's are encoded in AML.

thanks,
-Len

Re: ACPI SCI IOAPIC bug (Re: Fixes for nforce2 hard lockup, apic, io-apic, udma133 covered)

[Maciej W. Rozycki]

On Thu, 31 Mar 2004, Len Brown wrote:

> Why is it that all IRQs get their name from the
> IOAPIC pin number, but the timer connected to
> pin 2 is called IRQ0 instead of IRQ2?

 ISA interrupts get their numbers from 8259A IRQ numbers they would be
routed to in the PIC mode.  This lets the code operate in the mixed mode
sanely (which happens for certain hardware), although it's not necessarily
the reason for the numbering used.  Note that the MP Specification does
not mandate an identity map of ISA interrupts, although it seems to be
what vendors do.

 Also note that depending on the configuration, the timer can effectively
be routed to INTIN2 or INTIN0 by Linux -- one of the alternatives being a
direct connection and the other one being done through the 8259A
configured to be transparent for its IRQ 0.

> I wonder if we should't be moving to at least a build option which
> deletes support for multiple pins at an IRQ, and deletes
> suport for non-identity pin->IRQ mapping.

 This can't be done for the timer due to configuration variations.

> While the ACPI table parsing is very early, the _PRT parsing
> can happen only after the ACPI interpreter is up, because
> the _PRT's are encoded in AML.

 Hmm, that's unfortunate.  Couldn't the interpreter be started earlier?

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +