oops in ioapic_write_entry
in [Kernel]
|
From: Yinghai Lu on 3 Aug 2010 05:40 On 08/03/2010 02:15 AM, Eric W. Biederman wrote: > Yinghai Lu <yinghai(a)kernel.org> writes: > >> On 08/03/2010 01:56 AM, Eric W. Biederman wrote: >>> Yinghai Lu <yinghai(a)kernel.org> writes: >>> >>>> On 08/03/2010 01:00 AM, Eric W. Biederman wrote: >>>>> Yinghai Lu <yinghai(a)kernel.org> writes: >>>>> >>>>>>>> Index: linux-2.6/arch/x86/kernel/apic/io_apic.c >>>>>>>> =================================================================== >>>>>>>> --- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c >>>>>>>> +++ linux-2.6/arch/x86/kernel/apic/io_apic.c >>>>>>>> @@ -1029,10 +1029,7 @@ static int pin_2_irq(int idx, int apic, >>>>>>>> } else { >>>>>>>> u32 gsi = mp_gsi_routing[apic].gsi_base + pin; >>>>>>>> >>>>>>>> - if (gsi >= NR_IRQS_LEGACY) >>>>>>>> - irq = gsi; >>>>>>>> - else >>>>>>>> - irq = gsi_top + gsi; >>>>>>>> + irq = gsi_to_irq(gsi); >>>>>>>> } >>>>>>>> >>>>>>>> #ifdef CONFIG_X86_32 >>>>>> >>>>>> what is the point for making irq = gsi_top + gsi when mptable is used instead of acpi? >>>>> >>>>> Because it is only convention that when mptables are used that the >>>>> first apic pins 0-15 are the ISA irqs. This thread witnessed and a >>>>> pci irq that came in pin < 16 that was not an ISA irq. The truly rare >>>>> and exotic case would be for the ISA irqs to be outside the first 16 >>>>> ioapic pins but the es7000 did exactly that. >>>> >>>> nvidia chipset if acpi is enabled, external pci device will use ioapic from 16 to 23. >>>> >>>> if mptable is used, external pci device will not use pin from 16 to 23..., and lot of devices will share same pin. >>> >>> Exactly. Pins < 16 are not necessarily ISA irqs, and can be possibly >>> shared level triggered PCI irqs. Unfortunately there are strange >>> boards like the es7000 where pins > 16 are ISA irqs. >>> >>> The other thing that is gained by having pin_2_irq always remap pins < >>> 16 is we can get away with the numerous hard codes in the arch/x86 and elsewhere >>> that assume irq < 16 is an ISA irq. >> >> how about this one ? > > You can't share an edge triggered ISA irq, it isn't really physically > possible. So I don't see how this extra complexity will change anything. > Dave's system mptble: MPTABLE: OEM ID: DELL MPTABLE: Product ID: Dell XPS710 MPTABLE: APIC at: 0xFEE00000 Processor #0 (Bootup-CPU) Processor #1 Bus #0 is PCI Bus #1 is PCI Bus #2 is PCI Bus #3 is PCI Bus #4 is PCI Bus #5 is PCI Bus #6 is PCI Bus #7 is PCI Bus #8 is PCI Bus #9 is PCI Bus #10 is ISA I/O APIC #8 Version 17 at 0xFEC00000. IOAPIC[0]: apic_id 8, version 17, address 0xfec00000, GSI 0-23 Int: type 0, pol 0, trig 0, bus 0a, IRQ 00, APIC ID 8, APIC INT 00 Int: type 0, pol 0, trig 0, bus 0a, IRQ 01, APIC ID 8, APIC INT 01 Int: type 3, pol 1, trig 1, bus 0a, IRQ 00, APIC ID 8, APIC INT 02 Int: type 0, pol 0, trig 0, bus 0a, IRQ 03, APIC ID 8, APIC INT 03 Int: type 0, pol 0, trig 0, bus 0a, IRQ 04, APIC ID 8, APIC INT 04 Int: type 0, pol 0, trig 0, bus 0a, IRQ 05, APIC ID 8, APIC INT 05 Int: type 0, pol 0, trig 0, bus 0a, IRQ 06, APIC ID 8, APIC INT 06 Int: type 0, pol 0, trig 0, bus 0a, IRQ 07, APIC ID 8, APIC INT 07 Int: type 0, pol 0, trig 0, bus 0a, IRQ 08, APIC ID 8, APIC INT 08 Int: type 0, pol 0, trig 0, bus 0a, IRQ 09, APIC ID 8, APIC INT 09 Int: type 0, pol 0, trig 0, bus 0a, IRQ 0a, APIC ID 8, APIC INT 0a Int: type 0, pol 0, trig 0, bus 0a, IRQ 0b, APIC ID 8, APIC INT 0b Int: type 0, pol 0, trig 0, bus 0a, IRQ 0c, APIC ID 8, APIC INT 0c Int: type 0, pol 0, trig 0, bus 0a, IRQ 0e, APIC ID 8, APIC INT 0e Int: type 0, pol 0, trig 0, bus 0a, IRQ 0f, APIC ID 8, APIC INT 0f Int: type 0, pol 0, trig 0, bus 00, IRQ 28, APIC ID 8, APIC INT 09 Int: type 0, pol 0, trig 0, bus 00, IRQ 2c, APIC ID 8, APIC INT 0a Int: type 0, pol 0, trig 0, bus 00, IRQ 2d, APIC ID 8, APIC INT 0a Int: type 0, pol 0, trig 0, bus 00, IRQ 34, APIC ID 8, APIC INT 0e Int: type 0, pol 0, trig 0, bus 00, IRQ 38, APIC ID 8, APIC INT 0b Int: type 0, pol 0, trig 0, bus 00, IRQ 38, APIC ID 8, APIC INT 0b Int: type 0, pol 0, trig 0, bus 00, IRQ 38, APIC ID 8, APIC INT 0b Int: type 0, pol 0, trig 0, bus 00, IRQ 34, APIC ID 8, APIC INT 09 Int: type 0, pol 0, trig 0, bus 01, IRQ 00, APIC ID 8, APIC INT 0b Int: type 0, pol 0, trig 0, bus 01, IRQ 01, APIC ID 8, APIC INT 0a Int: type 0, pol 0, trig 0, bus 01, IRQ 02, APIC ID 8, APIC INT 09 Int: type 0, pol 0, trig 0, bus 03, IRQ 00, APIC ID 0, APIC INT 10 Int: type 0, pol 0, trig 0, bus 04, IRQ 00, APIC ID 0, APIC INT 10 Int: type 0, pol 0, trig 0, bus 04, IRQ 01, APIC ID 0, APIC INT 11 Int: type 0, pol 0, trig 0, bus 05, IRQ 00, APIC ID 8, APIC INT 0a Int: type 0, pol 0, trig 0, bus 05, IRQ 01, APIC ID 8, APIC INT 09 Int: type 0, pol 0, trig 0, bus 05, IRQ 03, APIC ID 8, APIC INT 0b Int: type 0, pol 0, trig 0, bus 06, IRQ 00, APIC ID 8, APIC INT 09 Int: type 0, pol 0, trig 0, bus 07, IRQ 0c, APIC ID 8, APIC INT 09 Int: type 0, pol 0, trig 0, bus 07, IRQ 0e, APIC ID 8, APIC INT 0b Int: type 0, pol 0, trig 0, bus 07, IRQ 0f, APIC ID 8, APIC INT 0a Int: type 0, pol 0, trig 0, bus 07, IRQ 10, APIC ID 8, APIC INT 0b Int: type 0, pol 0, trig 0, bus 07, IRQ 11, APIC ID 8, APIC INT 0a Int: type 0, pol 0, trig 0, bus 07, IRQ 12, APIC ID 8, APIC INT 09 Int: type 0, pol 0, trig 0, bus 07, IRQ 14, APIC ID 8, APIC INT 0a Int: type 0, pol 0, trig 0, bus 07, IRQ 15, APIC ID 8, APIC INT 09 Int: type 0, pol 0, trig 0, bus 07, IRQ 17, APIC ID 8, APIC INT 0b Int: type 0, pol 0, trig 0, bus 07, IRQ 28, APIC ID 8, APIC INT 09 Int: type 0, pol 0, trig 0, bus 08, IRQ 00, APIC ID 8, APIC INT 0a Int: type 0, pol 0, trig 0, bus 08, IRQ 01, APIC ID 8, APIC INT 09 Int: type 0, pol 0, trig 0, bus 08, IRQ 03, APIC ID 8, APIC INT 0b Int: type 0, pol 0, trig 0, bus 09, IRQ 00, APIC ID 8, APIC INT 0b Int: type 0, pol 0, trig 0, bus 09, IRQ 01, APIC ID 8, APIC INT 0a Int: type 0, pol 0, trig 0, bus 09, IRQ 02, APIC ID 8, APIC INT 09 Lint: type 3, pol 1, trig 1, bus 0a, IRQ 00, APIC ID ff, APIC LINT 00 Lint: type 1, pol 1, trig 1, bus 0a, IRQ 00, APIC ID ff, APIC LINT 01 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Eric W. Biederman on 3 Aug 2010 07:10 Yinghai Lu <yinghai(a)kernel.org> writes: > On 08/03/2010 02:15 AM, Eric W. Biederman wrote: >> Yinghai Lu <yinghai(a)kernel.org> writes: >> >> >> You can't share an edge triggered ISA irq, it isn't really physically >> possible. So I don't see how this extra complexity will change anything. >> > > Dave's system mptble: Interesting he has ISA irqs on bus #10 on the same apic id and pin as pci irqs. Blink. I had missed we had that print out. The immediate issue are these lines: > Int: type 0, pol 0, trig 0, bus 03, IRQ 00, APIC ID 0, APIC INT 10 > Int: type 0, pol 0, trig 0, bus 04, IRQ 00, APIC ID 0, APIC INT 10 > Int: type 0, pol 0, trig 0, bus 04, IRQ 01, APIC ID 0, APIC INT 11 Which get the apic id wrong, and thus cause us to mishandle them and get You are right Dave's mptable does the arguably broken: > Int: type 0, pol 0, trig 0, bus 00, IRQ 28, APIC ID 8, APIC INT 09 > Int: type 0, pol 0, trig 0, bus 00, IRQ 34, APIC ID 8, APIC INT 09 > Int: type 0, pol 0, trig 0, bus 01, IRQ 02, APIC ID 8, APIC INT 09 > Int: type 0, pol 0, trig 0, bus 05, IRQ 01, APIC ID 8, APIC INT 09 > Int: type 0, pol 0, trig 0, bus 06, IRQ 00, APIC ID 8, APIC INT 09 > Int: type 0, pol 0, trig 0, bus 07, IRQ 0c, APIC ID 8, APIC INT 09 > Int: type 0, pol 0, trig 0, bus 07, IRQ 12, APIC ID 8, APIC INT 09 > Int: type 0, pol 0, trig 0, bus 07, IRQ 15, APIC ID 8, APIC INT 09 > Int: type 0, pol 0, trig 0, bus 07, IRQ 28, APIC ID 8, APIC INT 09 > Int: type 0, pol 0, trig 0, bus 08, IRQ 01, APIC ID 8, APIC INT 09 > Int: type 0, pol 0, trig 0, bus 09, IRQ 02, APIC ID 8, APIC INT 09 > Int: type 0, pol 0, trig 0, bus 0a, IRQ 09, APIC ID 8, APIC INT 09 Where busses 0,1,5,6,7,8,9 result in level triggered interrupts and bus 0x0a results in an edge triggered interrupt. As I read the code. First we will do a generic isa setup, marking the interrupt ioapic table entry edge triggered. Then we will do a pci setup for any pin we use as pci, and then we will set pin_programmed stopping us from updating the pin any more, during setup. For the common case I think we still do the right thing, even now, for these broken bios tables. There is likely an uncommon case for which something like your shared_legacy_irq deserves to be used, especially at it preserves our well tested historical behavior. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on 3 Aug 2010 15:50 On 08/03/2010 04:08 AM, Eric W. Biederman wrote: > > For the common case I think we still do the right thing, even now, for > these broken bios tables. There is likely an uncommon case for which > something like your shared_legacy_irq deserves to be used, especially > at it preserves our well tested historical behavior. Dave, Irnna, Gary: can you check this patch on your systems? Thanks Yinghai [PATCH] x86: check if apic/pin is shared with legacy one fix system that external device that have io apic on apic0/pin(0-15) also for the io apic out of order system: <6>ACPI: IOAPIC (id[0x10] address[0xfecff000] gsi_base[0]) <6>IOAPIC[0]: apic_id 16, version 0, address 0xfecff000, GSI 0-2 <6>ACPI: IOAPIC (id[0x0f] address[0xfec00000] gsi_base[3]) <6>IOAPIC[1]: apic_id 15, version 0, address 0xfec00000, GSI 3-38 <6>ACPI: IOAPIC (id[0x0e] address[0xfec01000] gsi_base[39]) <6>IOAPIC[2]: apic_id 14, version 0, address 0xfec01000, GSI 39-74 <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 1 global_irq 4 dfl dfl) <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 5 dfl dfl) <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 3 global_irq 6 dfl dfl) <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 4 global_irq 7 dfl dfl) <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 6 global_irq 9 dfl dfl) <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 7 global_irq 10 dfl dfl) <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 8 global_irq 11 low edge) <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 12 dfl dfl) <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 12 global_irq 15 dfl dfl) <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 13 global_irq 16 dfl dfl) <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 14 global_irq 17 low edge) <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 15 global_irq 18 dfl dfl) after this patch will get apic0, pin0, GSI 0: irq 0+75 apic0, pin1, GSI 1: irq 1+75 apic0, pin2, GSI 2: irq 2 apic1, pin0, GSI 3: irq 3+75 apic1, pin5, GSI 8: irq 8+75 apic1, pin10,GSI 13: irq 13+75 apic1, pin11,GSI 14: irq 14+75 because mp_config_acpi_legacy_irqs will put apic0, pin2, irq2 in mp_irqs... so pin_2_irq_legacy will report 2. irq_to_gsi will still report 2. so it is right. gsi_to_irq will report 2. for GSI 0, 1, 3, 8, 13, 14: still right as before. Signed-off-by: Yinghai Lu <yinghai(a)kernel.org> --- arch/x86/kernel/apic/io_apic.c | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) Index: linux-2.6/arch/x86/kernel/apic/io_apic.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c +++ linux-2.6/arch/x86/kernel/apic/io_apic.c @@ -1013,6 +1013,28 @@ static inline int irq_trigger(int idx) return MPBIOS_trigger(idx); } +static int pin_2_irq_leagcy(int apic, int pin) +{ + int i; + + for (i = 0; i < mp_irq_entries; i++) { + int bus = mp_irqs[i].srcbus; + + if (!test_bit(bus, mp_bus_not_pci)) + continue; + + if (mp_ioapics[apic].apicid != mp_irqs[i].dstapic) + continue; + + if (mp_irqs[i].dstirq != pin) + continue; + + return mp_irqs[i].srcbusirq; + } + + return -1; +} + static int pin_2_irq(int idx, int apic, int pin) { int irq; @@ -1029,10 +1051,13 @@ static int pin_2_irq(int idx, int apic, } else { u32 gsi = mp_gsi_routing[apic].gsi_base + pin; - if (gsi >= NR_IRQS_LEGACY) + if (gsi >= NR_IRQS_LEGACY) { irq = gsi; - else - irq = gsi_top + gsi; + } else { + irq = pin_2_irq_legacy(apic, pin); + if (irq < 0) + irq = gsi_top + gsi; + } } #ifdef CONFIG_X86_32 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on 3 Aug 2010 16:10 On 08/03/2010 12:45 PM, Yinghai Lu wrote: > On 08/03/2010 04:08 AM, Eric W. Biederman wrote: >> >> For the common case I think we still do the right thing, even now, for >> these broken bios tables. There is likely an uncommon case for which >> something like your shared_legacy_irq deserves to be used, especially >> at it preserves our well tested historical behavior. > > Dave, Irnna, Gary: > > can you check this patch on your systems? > > Thanks > > Yinghai > > [PATCH] x86: check if apic/pin is shared with legacy one > > fix system that external device that have io apic on apic0/pin(0-15) > > also > for the io apic out of order system: > <6>ACPI: IOAPIC (id[0x10] address[0xfecff000] gsi_base[0]) > <6>IOAPIC[0]: apic_id 16, version 0, address 0xfecff000, GSI 0-2 > <6>ACPI: IOAPIC (id[0x0f] address[0xfec00000] gsi_base[3]) > <6>IOAPIC[1]: apic_id 15, version 0, address 0xfec00000, GSI 3-38 > <6>ACPI: IOAPIC (id[0x0e] address[0xfec01000] gsi_base[39]) > <6>IOAPIC[2]: apic_id 14, version 0, address 0xfec01000, GSI 39-74 > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 1 global_irq 4 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 5 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 3 global_irq 6 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 4 global_irq 7 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 6 global_irq 9 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 7 global_irq 10 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 8 global_irq 11 low edge) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 12 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 12 global_irq 15 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 13 global_irq 16 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 14 global_irq 17 low edge) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 15 global_irq 18 dfl dfl) > > after this patch will get > > apic0, pin0, GSI 0: irq 0+75 > apic0, pin1, GSI 1: irq 1+75 > apic0, pin2, GSI 2: irq 2 > apic1, pin0, GSI 3: irq 3+75 > apic1, pin5, GSI 8: irq 8+75 > apic1, pin10,GSI 13: irq 13+75 > apic1, pin11,GSI 14: irq 14+75 > > because mp_config_acpi_legacy_irqs will put apic0, pin2, irq2 in mp_irqs... > so pin_2_irq_legacy will report 2. > irq_to_gsi will still report 2. so it is right. > gsi_to_irq will report 2. > > for GSI 0, 1, 3, 8, 13, 14: still right as before. > > Signed-off-by: Yinghai Lu <yinghai(a)kernel.org> > > --- > arch/x86/kernel/apic/io_apic.c | 31 ++++++++++++++++++++++++++++--- > 1 file changed, 28 insertions(+), 3 deletions(-) > > Index: linux-2.6/arch/x86/kernel/apic/io_apic.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c > +++ linux-2.6/arch/x86/kernel/apic/io_apic.c > @@ -1013,6 +1013,28 @@ static inline int irq_trigger(int idx) > return MPBIOS_trigger(idx); > } > > +static int pin_2_irq_leagcy(int apic, int pin) should be pin_2_irq_legacy here..., change the function name without compiling... > +{ > + int i; > + > + for (i = 0; i < mp_irq_entries; i++) { > + int bus = mp_irqs[i].srcbus; > + > + if (!test_bit(bus, mp_bus_not_pci)) > + continue; > + > + if (mp_ioapics[apic].apicid != mp_irqs[i].dstapic) > + continue; > + > + if (mp_irqs[i].dstirq != pin) > + continue; > + > + return mp_irqs[i].srcbusirq; > + } > + > + return -1; > +} > + > static int pin_2_irq(int idx, int apic, int pin) > { > int irq; > @@ -1029,10 +1051,13 @@ static int pin_2_irq(int idx, int apic, > } else { > u32 gsi = mp_gsi_routing[apic].gsi_base + pin; > > - if (gsi >= NR_IRQS_LEGACY) > + if (gsi >= NR_IRQS_LEGACY) { > irq = gsi; > - else > - irq = gsi_top + gsi; > + } else { > + irq = pin_2_irq_legacy(apic, pin); > + if (irq < 0) > + irq = gsi_top + gsi; > + } > } > > #ifdef CONFIG_X86_32 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Eric W. Biederman on 3 Aug 2010 17:40
Yinghai Lu <yinghai(a)kernel.org> writes: > On 08/03/2010 04:08 AM, Eric W. Biederman wrote: >> >> For the common case I think we still do the right thing, even now, for >> these broken bios tables. There is likely an uncommon case for which >> something like your shared_legacy_irq deserves to be used, especially >> at it preserves our well tested historical behavior. > > Dave, Irnna, Gary: > > can you check this patch on your systems? > > Thanks > > Yinghai > > [PATCH] x86: check if apic/pin is shared with legacy one > > fix system that external device that have io apic on apic0/pin(0-15) Nacked-by: "Eric W. Biederman" <ebiederm(a)xmission.com> Your patch addresses what appears to be a theoretical issue, caused by a BIOS bug. So far you have not presented a credible scenario where this would affect anything in real life except the user visible irq number. Will you please stop, think, and describe what is going on clearly and how you expect this patch to affect anything, and please stop selling this patch as the solution to all of the world's ills. You are being sloppy and wasting everyone's time. This patch definitely does not solve Dave Airlie's problem. That was clearly caused by a (ACPI being disabled compared to the test case) and a bad BIOS provided MP table with an invalid apic_id, and our attempt to lookup the ioapic and failed. This patch doesn't have much of a chance of solving the no VGA output on boot problem. It comes much much to late affect anything there. > also > for the io apic out of order system: > <6>ACPI: IOAPIC (id[0x10] address[0xfecff000] gsi_base[0]) > <6>IOAPIC[0]: apic_id 16, version 0, address 0xfecff000, GSI 0-2 > <6>ACPI: IOAPIC (id[0x0f] address[0xfec00000] gsi_base[3]) > <6>IOAPIC[1]: apic_id 15, version 0, address 0xfec00000, GSI 3-38 > <6>ACPI: IOAPIC (id[0x0e] address[0xfec01000] gsi_base[39]) > <6>IOAPIC[2]: apic_id 14, version 0, address 0xfec01000, GSI 39-74 > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 1 global_irq 4 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 5 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 3 global_irq 6 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 4 global_irq 7 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 6 global_irq 9 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 7 global_irq 10 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 8 global_irq 11 low edge) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 12 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 12 global_irq 15 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 13 global_irq 16 dfl dfl) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 14 global_irq 17 low edge) > <6>ACPI: INT_SRC_OVR (bus 0 bus_irq 15 global_irq 18 dfl dfl) > > after this patch will get > > apic0, pin0, GSI 0: irq 0+75 > apic0, pin1, GSI 1: irq 1+75 > apic0, pin2, GSI 2: irq 2 > apic1, pin0, GSI 3: irq 3+75 > apic1, pin5, GSI 8: irq 8+75 > apic1, pin10,GSI 13: irq 13+75 > apic1, pin11,GSI 14: irq 14+75 If your description of what this patch will do is correct we very definitely do not want this patch. For the previously mentioned acpi tables we want GSI17 -> apic1, pin 14 -> irq 14. Not that your description is correct, but a buggy description is an equally valid reason to reject this incarnation of the patch. > because mp_config_acpi_legacy_irqs will put apic0, pin2, irq2 in mp_irqs... > so pin_2_irq_legacy will report 2. > irq_to_gsi will still report 2. so it is right. > gsi_to_irq will report 2. > > for GSI 0, 1, 3, 8, 13, 14: still right as before. YH you have a point that a certain class of buggy firmware exists, that reports some ioapic pins as both edge triggered ISA irqs and level triggered PCI irqs, and assigning different irqs to the same ioapic pin is confusing and suboptimal. In practice I don't see that behavior affecting how we program that pin, especially since I don't expect any bios that exports that setup to actually use the ISA irq. So we should just have an unused irq number. A clean solution would be to scrub the input from the MP table before we attempt to use of it, instead of scrubbing the data as we use in code paths like pin_2_irq. Just touching pin_2_irq is certainly an incomplete solution because you have not resolved if the pins should be edge or level triggered, and what polarity we should be sampling them at. Until I see a plausible scenario where not handling buggy MP tables exactly as we have done in the past I don't see hacks like you are proposing making much sense at all. Eric > Signed-off-by: Yinghai Lu <yinghai(a)kernel.org> > > --- > arch/x86/kernel/apic/io_apic.c | 31 ++++++++++++++++++++++++++++--- > 1 file changed, 28 insertions(+), 3 deletions(-) > > Index: linux-2.6/arch/x86/kernel/apic/io_apic.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c > +++ linux-2.6/arch/x86/kernel/apic/io_apic.c > @@ -1013,6 +1013,28 @@ static inline int irq_trigger(int idx) > return MPBIOS_trigger(idx); > } > > +static int pin_2_irq_leagcy(int apic, int pin) > +{ > + int i; > + > + for (i = 0; i < mp_irq_entries; i++) { > + int bus = mp_irqs[i].srcbus; > + > + if (!test_bit(bus, mp_bus_not_pci)) > + continue; > + > + if (mp_ioapics[apic].apicid != mp_irqs[i].dstapic) > + continue; > + > + if (mp_irqs[i].dstirq != pin) > + continue; > + > + return mp_irqs[i].srcbusirq; > + } > + > + return -1; > +} > + > static int pin_2_irq(int idx, int apic, int pin) > { > int irq; > @@ -1029,10 +1051,13 @@ static int pin_2_irq(int idx, int apic, > } else { > u32 gsi = mp_gsi_routing[apic].gsi_base + pin; > > - if (gsi >= NR_IRQS_LEGACY) > + if (gsi >= NR_IRQS_LEGACY) { > irq = gsi; > - else > - irq = gsi_top + gsi; > + } else { > + irq = pin_2_irq_legacy(apic, pin); > + if (irq < 0) > + irq = gsi_top + gsi; > + } > } > > #ifdef CONFIG_X86_32 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |