[PATCH] i386 io_apic.c: Memorize at bootup where the i8259 is connected

[PATCH] i386 io_apic.c: Memorize at bootup where the i8259 is connected

[Eric W. Biederman]

Currently we attempt to restore virtual wire mode on reboot, which
only works if we can figure out where the i8259 is connected.  This
is very useful when we kexec another kernel and likely helpful
when dealing with a BIOS that make assumptions about how the system is setup.

Since the acpi MADT table does not provide the location where the i8259
is connected we have to look at the hardware to figure it out.

Most systems have the i8259 connected the local apic of the cpu so
won't be affected but people running Opteron and some serverworks chipsets
should be able to use kexec now.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---

 arch/i386/kernel/io_apic.c |   52 ++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 47 insertions(+), 5 deletions(-)

17388b65d11d4e9db0a1f716895f15a5fa0ec2b0
diff --git a/arch/i386/kernel/io_apic.c b/arch/i386/kernel/io_apic.c
--- a/arch/i386/kernel/io_apic.c
+++ b/arch/i386/kernel/io_apic.c
@@ -46,6 +46,9 @@
 int (*ioapic_renumber_irq)(int ioapic, int irq);
 atomic_t irq_mis_count;
 
+/* Where if anywhere is the i8259 connect in external int mode */
+static int ioapic_i8259_pin = -1;
+
 static DEFINE_SPINLOCK(ioapic_lock);
 
 /*
@@ -748,7 +751,7 @@ static int find_irq_entry(int apic, int 
 /*
  * Find the pin to which IRQ[irq] (ISA) is connected
  */
-static int find_isa_irq_pin(int irq, int type)
+static int __init find_isa_irq_pin(int irq, int type)
 {
 	int i;
 
@@ -1599,6 +1602,44 @@ void /*__init*/ print_PIC(void)
 
 #endif  /*  0  */
 
+static void __init find_i8259_pin(void)
+{
+	struct IO_APIC_route_entry entry;
+	unsigned long flags;
+	int pin, pins;
+
+	ioapic_i8259_pin = -1;
+
+	/* Find the number of pins on the primary ioapic */
+	spin_lock_irqsave(&ioapic_lock, flags);
+	pins = ((io_apic_read(0, 0x01) >> 16) & 0xff) + 1;
+	spin_unlock_irqrestore(&ioapic_lock, flags);
+
+	/* See if any of the pins is in ExtINT mode */
+	for(pin = 0; pin < pins; pin++) {
+		spin_lock_irqsave(&ioapic_lock, flags);
+		*(((int *)&entry) + 0) = io_apic_read(0, 0x10 + 2 * pin);
+		*(((int *)&entry) + 1) = io_apic_read(0, 0x11 + 2 * pin);
+		spin_unlock_irqrestore(&ioapic_lock, flags);
+
+		/* If the interrupt line is enabled and in ExtInt mode 
+		 * I have found the pin where the i8259 is connected.
+		 */
+		if ((entry.mask == 0) && (entry.delivery_mode == dest_ExtINT)) {
+			ioapic_i8259_pin = pin;
+			break;
+		}
+	}
+
+	/* If we could not find an appropriate pin by looking at the ioapic
+	 * the i8259 probably isn't connected to the ioapic but give
+	 * the mptable a chance anyway.
+	 */
+	if (ioapic_i8259_pin == -1) {
+		ioapic_i8259_pin = find_isa_irq_pin(0, mp_ExtINT);
+	}
+}
+
 static void __init enable_IO_APIC(void)
 {
 	union IO_APIC_reg_01 reg_01;
@@ -1641,11 +1682,11 @@ void disable_IO_APIC(void)
 	clear_IO_APIC();
 
 	/*
-	 * If the i82559 is routed through an IOAPIC
+	 * If the i8259 is routed through an IOAPIC
 	 * Put that IOAPIC in virtual wire mode
 	 * so legacy interrups can be delivered.
 	 */
-	pin = find_isa_irq_pin(0, mp_ExtINT);
+	pin = ioapic_i8259_pin;
 	if (pin != -1) {
 		struct IO_APIC_route_entry entry;
 		unsigned long flags;
@@ -1657,7 +1698,7 @@ void disable_IO_APIC(void)
 		entry.polarity        = 0; /* High */
 		entry.delivery_status = 0;
 		entry.dest_mode       = 0; /* Physical */
-		entry.delivery_mode   = 7; /* ExtInt */
+		entry.delivery_mode   = dest_ExtINT; /* ExtInt */
 		entry.vector          = 0;
 		entry.dest.physical.physical_dest = 0;
 
@@ -2195,7 +2236,7 @@ static inline void check_timer(void)
 	enable_8259A_irq(0);
 
 	pin1 = find_isa_irq_pin(0, mp_INT);
-	pin2 = find_isa_irq_pin(0, mp_ExtINT);
+	pin2 = ioapic_i8259_pin;
 
 	printk(KERN_INFO "..TIMER: vector=0x%02X pin1=%d pin2=%d\n", vector, pin1, pin2);
 
@@ -2289,6 +2330,7 @@ static inline void check_timer(void)
 
 void __init setup_IO_APIC(void)
 {
+	find_i8259_pin();
 	enable_IO_APIC();
 
 	if (acpi_ioapic)

Re: [PATCH] i386 io_apic.c: Memorize at bootup where the i8259 is connected

[Linus Torvalds]

On Fri, 29 Jul 2005, Eric W. Biederman wrote:
> 
> Since the acpi MADT table does not provide the location where the i8259
> is connected we have to look at the hardware to figure it out.

I'm not really happy with this.

First off, it kind of assumes that extINT is always the 8259. Maybe that's
true, maybe it's not. Maybe there is hardware out there that has a
specialty interrupt controller that also uses extInt? Secondly, why always
just on IO-APIC 0? This would make a lot more sense to do inside the
loop-over-apics in enable_IO_APIC, no?

Especially since that one already calculates the number of entries, and
does it a lot more nicely than you do.. (ie no shifting and masking with
magic constants).

Finally, the third issue I have is that _if_ the MP table is correct,
we'll never know. Wouldn't it be better to query the MP table regardless,
and see if it agrees with what we found, and if it doesn't, at least print
a message so that it is easier to debug things if sh*t happens?

		Linus

Re: [PATCH] i386 io_apic.c: Memorize at bootup where the i8259 is connected

[Eric W. Biederman]

Linus Torvalds <torvalds@osdl.org> writes:

> On Fri, 29 Jul 2005, Eric W. Biederman wrote:
>> 
>> Since the acpi MADT table does not provide the location where the i8259
>> is connected we have to look at the hardware to figure it out.
>
> I'm not really happy with this.
>
> First off, it kind of assumes that extINT is always the 8259. Maybe that's
> true, maybe it's not. Maybe there is hardware out there that has a
> specialty interrupt controller that also uses extInt? 

I believe the definition of extInt is that it is an external
interrupt controller that sends interrupts like an 8259.

So it might be possible but it would be an extreme hardware.
And it would be an old hardware configuration because acpi
doesn't even allow you to setup that kind of thing.

> Secondly, why always just on IO-APIC 0? 

Good question the assumption was already in the code, but
it isn't hard to lift.

> This would make a lot more sense to do inside the
> loop-over-apics in enable_IO_APIC, no?

Probably.  It has to come before the call to clear_IO_APIC().
I was just be extra careful about that.

> Especially since that one already calculates the number of entries, and
> does it a lot more nicely than you do.. (ie no shifting and masking with
> magic constants).

:)

> Finally, the third issue I have is that _if_ the MP table is correct,
> we'll never know. Wouldn't it be better to query the MP table regardless,
> and see if it agrees with what we found, and if it doesn't, at least print
> a message so that it is easier to debug things if sh*t happens?

The reason I generated the patch is because reading the acpi
tables is the default no one is even using the MP table anymore.
The acpi MADT table can't represent the notion of an a pin
in ExtInt mode.  Even in the MP table has the information is pretty
much advisory as the OS doesn't use it except on very old systems.

The practical question is which is the better route.  Save
off all of the entries in the apic and ioapic and restore
them on reboot, or simply save off which pin the i8259 is
talking through and restore one pin in ExtInt mode.  I like
the latter because we have enough information that we can
and if there is a weird system we can specify it with a command
line parameter.  But the save/restore approach may be more general,
and less prone to coder error.

Eric

RE: [PATCH] i386 io_apic.c: Memorize at bootup where the i8259 is connected

[Andy Currid]

 

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org 
> [mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of 
> Linus Torvalds
> Sent: Friday, July 29, 2005 13:09
> To: Eric W. Biederman
> Cc: Andrew Morton; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] i386 io_apic.c: Memorize at bootup where 
> the i8259 is connected
> 
> 
> 
> On Fri, 29 Jul 2005, Eric W. Biederman wrote:
> > 
> > Since the acpi MADT table does not provide the location 
> where the i8259
> > is connected we have to look at the hardware to figure it out.
> 
> I'm not really happy with this.
> 
> First off, it kind of assumes that extINT is always the 8259. 
> Maybe that's true, maybe it's not.

Since this code is the i386 architecture branch, I think it's safe to
assume that ExtINT always emanates from something that is 8259A
compatible. The Intel APIC / IOAPIC and MP specifications which govern
this architecture are quite specific on this point. The same is true for
x86_64.

> Maybe there is hardware out there that has a
> specialty interrupt controller that also uses extInt? 
> Secondly, why always
> just on IO-APIC 0? This would make a lot more sense to do inside the
> loop-over-apics in enable_IO_APIC, no?

Agreed. Any IO-APIC is fair game for virtual wire routing.
 
> Especially since that one already calculates the number of 
> entries, and
> does it a lot more nicely than you do.. (ie no shifting and 
> masking with
> magic constants).
> 
> Finally, the third issue I have is that _if_ the MP table is correct,
> we'll never know. Wouldn't it be better to query the MP table 
> regardless,
> and see if it agrees with what we found, and if it doesn't, 
> at least print
> a message so that it is easier to debug things if sh*t happens?

MP tables on IA32 / AMD64 systems are frequently wrong when it comes to
interrupt mappings. That's an indirect consequence of Microsoft
mandating ACPI  since 2000: system vendors tend to test the hell out of
that configuration and nothing else.

But I think it's a good idea to cross check as Linus suggests, and
notify the user of any discrepancy. After all, the only *guaranteed* way
to get back into virtual wire mode on a platform is to do a reset; the
original MP spec didn't envisage supporting this mode of operation.

Andy
--