Message ID | 20040603101313.GB6578@gondor.apana.org.au |
---|---|
State | New, archived |
Headers | show |
Series |
|
Related | show |
Herbert Xu <herbert@gondor.apana.org.au> wrote: > > I've received two reports at http://bugs.debian.org/251207 where > ioapic caused machines to lock up during booting due to the > change apic_id panic in arch/i386/kernel/io_apic.c. > > Since it appears that we can avoid panicking at all, I think we > should replace the panic calls with the following patch which > attempts to continue after the failure. > > I've also done the same thing to the other panic() call in the > same function. Well. Question is, why are we getting insame APIC ID's in there in the first place? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Herbert Xu <herbert@gondor.apana.org.au> wrote: >> I've received two reports at http://bugs.debian.org/251207 where >> ioapic caused machines to lock up during booting due to the >> change apic_id panic in arch/i386/kernel/io_apic.c. >> Since it appears that we can avoid panicking at all, I think we >> should replace the panic calls with the following patch which >> attempts to continue after the failure. >> I've also done the same thing to the other panic() call in the >> same function. On Thu, Jun 03, 2004 at 04:20:45PM -0700, Andrew Morton wrote: > Well. Question is, why are we getting insame APIC ID's in there in the > first place? They're usually not insane. xAPIC's have 8-bit physical ID's, not 4-bit, but no one's bothered tracking whether the APIC's found were serial APIC or xAPIC, and then dispatching on that in the IO-APIC physid check to avoid unnecessarily renumbering the things or panicking. This is my longstanding complaint regarding APIC_BROADCAST_ID being wrong. -- wli - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
William Lee Irwin III <wli@holomorphy.com> wrote: > > Herbert Xu <herbert@gondor.apana.org.au> wrote: > >> I've received two reports at http://bugs.debian.org/251207 where > >> ioapic caused machines to lock up during booting due to the > >> change apic_id panic in arch/i386/kernel/io_apic.c. > >> Since it appears that we can avoid panicking at all, I think we > >> should replace the panic calls with the following patch which > >> attempts to continue after the failure. > >> I've also done the same thing to the other panic() call in the > >> same function. > > On Thu, Jun 03, 2004 at 04:20:45PM -0700, Andrew Morton wrote: > > Well. Question is, why are we getting insame APIC ID's in there in the > > first place? > > They're usually not insane. xAPIC's have 8-bit physical ID's, not 4-bit, > but no one's bothered tracking whether the APIC's found were serial APIC > or xAPIC, and then dispatching on that in the IO-APIC physid check to > avoid unnecessarily renumbering the things or panicking. So... what should we do? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
William Lee Irwin III <wli@holomorphy.com> wrote: >> They're usually not insane. xAPIC's have 8-bit physical ID's, not 4-bit, >> but no one's bothered tracking whether the APIC's found were serial APIC >> or xAPIC, and then dispatching on that in the IO-APIC physid check to >> avoid unnecessarily renumbering the things or panicking. On Thu, Jun 03, 2004 at 04:44:15PM -0700, Andrew Morton wrote: > So... what should we do? In all likelihood, make the thing a variable or return a value based on the result of GET_APIC_VERSION(), particularly since guessing wrong either way takes out machines before console_init(). Returning values based on the result of GET_APIC_VERSION() has a predecent, get_maxlvt(). -- wli - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff -Nru a/arch/i386/kernel/io_apic.c b/arch/i386/kernel/io_apic.c --- a/arch/i386/kernel/io_apic.c 2004-06-03 20:02:42 +10:00 +++ b/arch/i386/kernel/io_apic.c 2004-06-03 20:02:42 +10:00 @@ -2365,8 +2365,10 @@ break; } - if (i == IO_APIC_MAX_ID) - panic("Max apic_id exceeded!\n"); + if (i == IO_APIC_MAX_ID) { + printk(KERN_ERR "Max apic_id exceeded!\n"); + return -1; + } printk(KERN_WARNING "IOAPIC[%d]: apic_id %d already used, " "trying %d\n", ioapic, apic_id, i); @@ -2374,9 +2376,6 @@ apic_id = i; } - tmp = apicid_to_cpu_present(apic_id); - physids_or(apic_id_map, apic_id_map, tmp); - if (reg_00.bits.ID != apic_id) { reg_00.bits.ID = apic_id; @@ -2386,9 +2385,15 @@ spin_unlock_irqrestore(&ioapic_lock, flags); /* Sanity check */ - if (reg_00.bits.ID != apic_id) - panic("IOAPIC[%d]: Unable change apic_id!\n", ioapic); + if (reg_00.bits.ID != apic_id) { + printk(KERN_ERR "IOAPIC[%d]: Unable change apic_id!\n", + ioapic); + return -1; + } } + + tmp = apicid_to_cpu_present(apic_id); + physids_or(apic_id_map, apic_id_map, tmp); printk(KERN_INFO "IOAPIC[%d]: Assigned apic_id %d\n", ioapic, apic_id); diff -Nru a/arch/i386/kernel/mpparse.c b/arch/i386/kernel/mpparse.c --- a/arch/i386/kernel/mpparse.c 2004-06-03 20:02:41 +10:00 +++ b/arch/i386/kernel/mpparse.c 2004-06-03 20:02:41 +10:00 @@ -881,6 +881,7 @@ u32 gsi_base) { int idx = 0; + int apicid; if (nr_ioapics >= MAX_IO_APICS) { printk(KERN_ERR "ERROR: Max # of I/O APICs (%d) exceeded " @@ -893,14 +894,19 @@ return; } - idx = nr_ioapics++; + idx = nr_ioapics; mp_ioapics[idx].mpc_type = MP_IOAPIC; mp_ioapics[idx].mpc_flags = MPC_APIC_USABLE; mp_ioapics[idx].mpc_apicaddr = address; set_fixmap_nocache(FIX_IO_APIC_BASE_0 + idx, address); - mp_ioapics[idx].mpc_apicid = io_apic_get_unique_id(idx, id); + apicid = io_apic_get_unique_id(idx, id); + if (apicid < 0) { + clear_fixmap(FIX_IO_APIC_BASE_0 + idx); + return; + } + mp_ioapics[idx].mpc_apicid = apicid; mp_ioapics[idx].mpc_apicver = io_apic_get_version(idx); /* @@ -912,6 +918,7 @@ mp_ioapic_routing[idx].gsi_end = gsi_base + io_apic_get_redir_entries(idx); + nr_ioapics++; printk("IOAPIC[%d]: apic_id %d, version %d, address 0x%lx, " "GSI %d-%d\n", idx, mp_ioapics[idx].mpc_apicid, mp_ioapics[idx].mpc_apicver, mp_ioapics[idx].mpc_apicaddr,