oops in ioapic_write_entry
in [Kernel]
|
From: Dave Airlie on 3 Aug 2010 19:20 On Wed, Aug 4, 2010 at 7:38 AM, Eric W. Biederman <ebiederm(a)xmission.com> wrote: > 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. Oh wow, I totally missed I had ACPI off on this build, I turned it off a few kernels back to test the build didn't break, and forgot to turn it back on. So clearly my mptable is just busted and we can probably move along, I'll rebuild/boot with ACPI now, and I think you can assume it works fine unless I get back to you. Thanks, and sorry for wasting time. Dave. -- 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 20:40 On 08/03/2010 02:38 PM, Eric W. Biederman wrote: > 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? >> >> [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. That is real problem in pin_2_irq() Nvidia chipset system with legacy bios will have problem when user try to boot with acpi is disabled. LinuxBIOS should be ok, already have external devices to use pin above 15. Also your patch does cause kernel crash when acpi is disabled in virtualbox even there could be BIOS problem there. kernel before your patch does work in that conf. Yinghai -- 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 21:30 Yinghai Lu <yinghai(a)kernel.org> writes: > On 08/03/2010 02:38 PM, Eric W. Biederman wrote: >> 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? >>> >>> [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. > > That is real problem in pin_2_irq() > > Nvidia chipset system with legacy bios will have problem when user try to boot with acpi is disabled. > LinuxBIOS should be ok, already have external devices to use pin above 15. The Nvidia chipset will have what problem? > Also your patch does cause kernel crash when acpi is disabled in virtualbox even there > could be BIOS problem there. > kernel before your patch does work in that conf. What is the kernel crash? I believe you have told me earlier that the problems you see with 2.6.35 are roughly, while those setups work with 2.3.34? > cause kernel with acpi crash in virtual box. > > [ 5.536000] querying PCI -> IRQ mapping bus:0, slot:11, pin:0. > [ 5.540000] ehci_hcd 0000:00:0b.0: can't find IRQ for PCI INT A; probably buggy MP table > [ > > and on kvm it got: > [ 4.352280] e1000: Intel(R) PRO/1000 Network Driver - version 7.3.21-k6-NAPI > [ 4.356012] e1000: Copyright (c) 1999-2006 Intel Corporation. > [ 4.360120] querying PCI -> IRQ mapping bus:0, slot:3, pin:0. > [ 4.364006] PCI BIOS passed nonexistent PCI bus 0! > [ 4.368007] e1000 0000:00:03.0: can't find IRQ for PCI INT A; probably buggy MP table > [ 4.372049] e1000 0000:00:03.0: setting latency timer to 64 Can you send out boot log with "debug apic=debug pci=routeirq" for the working 2.6.34, the failing 2.6.35, and your .config? With a little luck that will be enough to allow us to pinpoint the problem case. 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: Ingo Molnar on 4 Aug 2010 03:40 * Eric W. Biederman <ebiederm(a)xmission.com> wrote: > 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. The kernel should not crash obviously, so that's a bug that has to be addressed - or failing any timely resolution the original patch which introduced the regression [if it's a regression] needs to be reverted. Thanks, Ingo -- 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 4 Aug 2010 05:10
On 08/03/2010 02:38 PM, Eric W. Biederman wrote: > > 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. ok, how about this one? it will try to add entries to mp_irqs[] with some checking. Yinghai --- arch/x86/kernel/mpparse.c | 44 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 7 deletions(-) Index: linux-2.6/arch/x86/kernel/mpparse.c =================================================================== --- linux-2.6.orig/arch/x86/kernel/mpparse.c +++ linux-2.6/arch/x86/kernel/mpparse.c @@ -173,17 +173,17 @@ static int __init mp_irq_mpc_intsrc_cmp( { if (mp_irq->dstapic != m->dstapic) return 1; - if (mp_irq->type != m->type) + if (mp_irq->dstirq != m->dstirq) return 2; - if (mp_irq->irqtype != m->irqtype) + if (mp_irq->srcbus != m->srcbus) return 3; - if (mp_irq->irqflag != m->irqflag) + if (mp_irq->type != m->type) return 4; - if (mp_irq->srcbus != m->srcbus) + if (mp_irq->irqtype != m->irqtype) return 5; - if (mp_irq->srcbusirq != m->srcbusirq) + if (mp_irq->irqflag != m->irqflag) return 6; - if (mp_irq->dstirq != m->dstirq) + if (mp_irq->srcbusirq != m->srcbusirq) return 7; return 0; @@ -195,9 +195,39 @@ static void __init MP_intsrc_info(struct print_MP_intsrc_info(m); + /* + * Assume BUS, and IOAPIC entries come first all before + * INTSRC entries + */ + + /* check if dstapic is right */ + for (i = 0; i < nr_ioapics; i++) { + if (mp_ioapics[idx].apicid == m->dstapic) + break; + } + if (i == nr_ioapics) + return; + for (i = 0; i < mp_irq_entries; i++) { - if (!mp_irq_mpc_intsrc_cmp(&mp_irqs[i], m)) + int ret = mp_irq_mpc_intsrc_cmp(&mp_irqs[i], m); + + /* duplicated entries ? */ + if (!ret) return; + + /* same apic/pin, but different bus */ + if (ret == 3) { + /* overwrite wrong legacy one */ + if (test_bit(mp_irqs[i].srcbus, mp_bus_not_pci) && + !test_bit(m->srcbus, mp_bus_not_pci)) { + assign_to_mp_irq(m, &mp_irqs[mp_irq_entries]); + return; + } + /* dump this legacy one */ + if (!test_bit(mp_irqs[i].srcbus, mp_bus_not_pci) && + test_bit(m->srcbus, mp_bus_not_pci)) + return; + } } assign_to_mp_irq(m, &mp_irqs[mp_irq_entries]); -- 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/ |