lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 20 Mar 2012 05:40:48 -0400 From: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com> To: Josh Boyer <jwboyer@...hat.com> Cc: Suresh Siddha <suresh.b.siddha@...el.com>, Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>, yinghai@...nel.org, linux-kernel@...r.kernel.org, kernel-team@...oraproject.org, midgoon@...il.com Subject: Re: 3.2.1 Unable to reset IRR messages on boot > > > > > Yes, it was helpful. Something like the appended patch should ignore the > > > > > bogus io-apic entry all together. As I can't test this, can you or the > > > > > reporter give the appended patch a try and ack please? > > > > > > > > Hi Suresh, > > > > > > > > Apologies for the delay. The original reporter had to return the > > > > machine he was using. We've since had another report where this > > > > happened and your patch below does indeed fix the issue. > > > > > > > > I'd suggest pushing this soon. > > > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=801501 > > > > > > > > > > Thanks Josh. Peter/Ingo, please queue the appended patch for -tip. > > > > Hi Suresh, > > > > Seems this patch and Xen don't get along very well. See the bug link > > below. I've CC'd Konrad and hopefully he'll have some insight as to why > > that might be. > > Quick glance at the code tells me that the 'mp_register_ioapic' with the > patch won't increment the gsi_top. That value (gsi_top) is used in .. snip.. > So the IO_APIC is all 0xfff.. My "quick glance" was wrong. The reason we are dying is b/c the call acpi_get_override_irq() is used, which returns the polarity and trigger for the IRQs. But that function calls mp_find_ioapics to get the 'struct ioapic' structure - which along with the mp_irq[x] is used to figure out the default values and the polarity/trigger overrides. Since the mp_find_ioapics now returns -1 [b/c the IOAPIC is filled with 0xffffffff], the acpi_get_override_irq() stops trying to lookup in the mp_irq[x] the proper INT_SRV_OVR and we can't install the SCI interrupt. Furthermore, we end up using that function in a loop to setup the sixteen legacy interrupts: /* Pre-allocate legacy irqs */ 480 for (irq = 0; irq < NR_IRQS_LEGACY; irq++) { 481 int trigger, polarity; 482 483 if (acpi_get_override_irq(irq, &trigger, &polarity) == -1) 484 continue; 485 486 xen_register_pirq(irq, -1 /* no GSI override */, 487 trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE, 488 true /* Map GSI to PIRQ */); and since we get -1, we never end up setting any interrupts. Also the nr_ioapics ends up being zero, so we think we are running in legacy mode with XT-PIC interrupts (ugh). By the time ACPI kicks in and wants to install the SCI interrupt we blow up since we cannot get the proper irq_desc. The interesting thing is that the same issue could be reproduced on baremetal if the first IOAPIC had 0xfff all over it - and the call to acpi_get_override_irq() done by the ACPI layer to setup the SCI would have triggered a similar failure. But the baremetal failing case has the first IOAPIC with the INT_SRV_OVR with valid values - it is just the second IOAPIC is busted. Or if the second IOAPIC was busted and there was an INT_SRV_OVR for the second APIC to handle the SCI. I think there are three ways of fixing this: 1). Revert Suresh's patch and look at just removing the "Unable to reset IRR" warning (perhaps by being conditional on running in kexec-env?). 2). Make the Xen layer fake out an IOAPIC - so instead of 0xffffff, make sure to clear the three bits that Suresh' patch is testing for (Ewwwww, I don't actually like that - that stinks of a hack). 3). Rework Suresh's patch - to only remove the IOAPIC entry if there is no INT_SRV_OVR that depend on it. I made a stab at it and here is draft patch, that looks to work on my boxes that have more than one IOAPIC and are booting under Xen: But I am not 100% confident about it so would appreciate somebody looking at it. diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h index 690d1cc..d92be91 100644 --- a/arch/x86/include/asm/io_apic.h +++ b/arch/x86/include/asm/io_apic.h @@ -170,6 +170,7 @@ extern u32 gsi_top; int mp_find_ioapic(u32 gsi); int mp_find_ioapic_pin(int ioapic, u32 gsi); void __init mp_register_ioapic(int id, u32 address, u32 gsi_base); +void __init mp_erase_defective_ioapics(void); extern void __init pre_init_apic_IRQ0(void); extern void mp_save_irq(struct mpc_intsrc *m); diff --git a/arch/x86/include/asm/mpspec.h b/arch/x86/include/asm/mpspec.h index 9c7d95f..e886853 100644 --- a/arch/x86/include/asm/mpspec.h +++ b/arch/x86/include/asm/mpspec.h @@ -103,6 +103,7 @@ extern void mp_config_acpi_legacy_irqs(void); struct device; extern int mp_register_gsi(struct device *dev, u32 gsi, int edge_level, int active_high_low); +extern void mp_erase_defective_ioapics(void); #endif /* CONFIG_ACPI */ #define PHYSID_ARRAY_SIZE BITS_TO_LONGS(MAX_LOCAL_APIC) diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index c3a5b95..b5115e7 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -1193,6 +1193,10 @@ static int __init acpi_parse_madt_ioapic_entries(void) } /* + * Cleanup up invalid IOAPICs. + */ + mp_erase_defective_ioapics(); + /* * If BIOS did not supply an INT_SRC_OVR for the SCI * pretend we got one so we can set the SCI flags. */ diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index 6d10a66..77f1e84 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -3989,14 +3989,43 @@ static __init int bad_ioapic_register(int idx) reg_02.raw = io_apic_read(idx, 2); if (reg_00.raw == -1 && reg_01.raw == -1 && reg_02.raw == -1) { - pr_warn("I/O APIC 0x%x registers return all ones, skipping!\n", - mpc_ioapic_addr(idx)); return 1; } return 0; } +bool __init no_gsi_override_for_ioapic(int idx) +{ + unsigned int gsi, i; + struct mp_ioapic_gsi *gsi_cfg; + + gsi_cfg = mp_ioapic_gsi_routing(idx); + for (gsi = gsi_cfg->gsi_base; gsi < gsi_cfg->gsi_end; gsi++) { + unsigned int pin = mp_find_ioapic_pin(idx, gsi); + for (i = 0; i < mp_irq_entries; i++) { + if (mp_irqs[i].dstirq == pin && + mp_irqs[i].dstapic == mpc_ioapic_id(idx)) + return false; + } + } + return true; +} +void __init mp_erase_defective_ioapics(void) +{ + unsigned int idx = 0; + while (idx < nr_ioapics) { + if (bad_ioapic_register(idx) && no_gsi_override_for_ioapic(idx)) { + pr_warn("I/O APIC 0x%x registers return all ones, skipping!\n", + mpc_ioapic_addr(idx)); + clear_fixmap(FIX_IO_APIC_BASE_0 + idx); + memmove(&ioapics[idx], &(ioapics[idx+1]), + sizeof(struct ioapic) * (nr_ioapics - 1 - idx)); + --nr_ioapics; + } else + idx++; + } +} void __init mp_register_ioapic(int id, u32 address, u32 gsi_base) { int idx = 0; @@ -4014,11 +4043,6 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base) set_fixmap_nocache(FIX_IO_APIC_BASE_0 + idx, address); - if (bad_ioapic_register(idx)) { - clear_fixmap(FIX_IO_APIC_BASE_0 + idx); - return; - } - ioapics[idx].mp_config.apicid = io_apic_unique_id(id); ioapics[idx].mp_config.apicver = io_apic_get_version(idx); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists