[PATCH 1/3] x86: fix init_8259A() to not use outb_pic

[PATCH 1/3] x86: fix init_8259A() to not use outb_pic

[David P. Reed]

[-- Attachment #1: fix-8259-iodelay.patch --]
[-- Type: text/plain, Size: 5261 bytes --]

fix init_8259A() which initializes the 8259 PIC to not use outb_pic,
which is a renamed version of outb_p, and delete outb_pic define.
There is already code in the .c files that does accesses to CMD & IMR registers
in successive outb() calls without _p.  Thus the outb_p is obviously not
needed, if it ever was.  Research into chipset documentation and old BIOS
listings shows that IODELAY was not used even in early machines.  Thus
the delay between i/o port writes was deleted for the 8259.

Again, the primary reason for fixing this is to use proper delay strategy,
and in particular to fix crashes that can result from using port 80 writes
on machines that have resources on port 80, such as the ENE chips used by Quanta
in latops it designs and sells to, e.g. HP.

Signed-off-by: David P. Reed <dpreed@reed.com>
Index: linux-2.6/arch/x86/kernel/i8259_32.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/i8259_32.c
+++ linux-2.6/arch/x86/kernel/i8259_32.c
@@ -285,24 +285,30 @@ void init_8259A(int auto_eoi)
 
 	spin_lock_irqsave(&i8259A_lock, flags);
 
-	outb(0xff, PIC_MASTER_IMR);	/* mask all of 8259A-1 */
-	outb(0xff, PIC_SLAVE_IMR);	/* mask all of 8259A-2 */
-
-	/*
-	 * outb_pic - this has to work on a wide range of PC hardware.
-	 */
-	outb_pic(0x11, PIC_MASTER_CMD);	/* ICW1: select 8259A-1 init */
-	outb_pic(0x20 + 0, PIC_MASTER_IMR);	/* ICW2: 8259A-1 IR0-7 mapped to 0x20-0x27 */
-	outb_pic(1U << PIC_CASCADE_IR, PIC_MASTER_IMR);	/* 8259A-1 (the master) has a slave on IR2 */
+	/* mask all of 8259A-1 */
+	outb(0xff, PIC_MASTER_IMR);
+	/* mask all of 8259A-2 */
+	outb(0xff, PIC_SLAVE_IMR);
+
+	/* ICW1: select 8259A-1 init */
+	outb(0x11, PIC_MASTER_CMD);
+	/* ICW2: 8259A-1 IR0-7 mapped to 0x20-0x27 */
+	outb(0x20 + 0, PIC_MASTER_IMR);
+	/* 8259A-1 (the master) has a slave on IR2 */
+	outb(1U << PIC_CASCADE_IR, PIC_MASTER_IMR);
 	if (auto_eoi)	/* master does Auto EOI */
-		outb_pic(MASTER_ICW4_DEFAULT | PIC_ICW4_AEOI, PIC_MASTER_IMR);
+		outb(MASTER_ICW4_DEFAULT | PIC_ICW4_AEOI, PIC_MASTER_IMR);
 	else		/* master expects normal EOI */
-		outb_pic(MASTER_ICW4_DEFAULT, PIC_MASTER_IMR);
+		outb(MASTER_ICW4_DEFAULT, PIC_MASTER_IMR);
 
-	outb_pic(0x11, PIC_SLAVE_CMD);	/* ICW1: select 8259A-2 init */
-	outb_pic(0x20 + 8, PIC_SLAVE_IMR);	/* ICW2: 8259A-2 IR0-7 mapped to 0x28-0x2f */
-	outb_pic(PIC_CASCADE_IR, PIC_SLAVE_IMR);	/* 8259A-2 is a slave on master's IR2 */
-	outb_pic(SLAVE_ICW4_DEFAULT, PIC_SLAVE_IMR); /* (slave's support for AEOI in flat mode is to be investigated) */
+	/* ICW1: select 8259A-2 init */
+	outb(0x11, PIC_SLAVE_CMD);
+	/* ICW2: 8259A-2 IR0-7 mapped to 0x28-0x2f */
+	outb(0x20 + 8, PIC_SLAVE_IMR);
+	/* 8259A-2 is a slave on master's IR2 */
+	outb(PIC_CASCADE_IR, PIC_SLAVE_IMR);
+	/* (slave's support for AEOI in flat mode is to be investigated) */
+	outb(SLAVE_ICW4_DEFAULT, PIC_SLAVE_IMR);
 	if (auto_eoi)
 		/*
 		 * In AEOI mode we just have to mask the interrupt
Index: linux-2.6/arch/x86/kernel/i8259_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/i8259_64.c
+++ linux-2.6/arch/x86/kernel/i8259_64.c
@@ -355,29 +355,30 @@ void init_8259A(int auto_eoi)
 
 	spin_lock_irqsave(&i8259A_lock, flags);
 
-	outb(0xff, PIC_MASTER_IMR);	/* mask all of 8259A-1 */
-	outb(0xff, PIC_SLAVE_IMR);	/* mask all of 8259A-2 */
+	/* mask all of 8259A-1 */
+	outb(0xff, PIC_MASTER_IMR);
+	/* mask all of 8259A-2 */
+	outb(0xff, PIC_SLAVE_IMR);
 
-	/*
-	 * outb_pic - this has to work on a wide range of PC hardware.
-	 */
-	outb_pic(0x11, PIC_MASTER_CMD);	/* ICW1: select 8259A-1 init */
+	/* ICW1: select 8259A-1 init */
+	outb(0x11, PIC_MASTER_CMD);
 	/* ICW2: 8259A-1 IR0-7 mapped to 0x30-0x37 */
-	outb_pic(IRQ0_VECTOR, PIC_MASTER_IMR);
+	outb(IRQ0_VECTOR, PIC_MASTER_IMR);
 	/* 8259A-1 (the master) has a slave on IR2 */
-	outb_pic(0x04, PIC_MASTER_IMR);
+	outb(0x04, PIC_MASTER_IMR);
 	if (auto_eoi)	/* master does Auto EOI */
-		outb_pic(MASTER_ICW4_DEFAULT | PIC_ICW4_AEOI, PIC_MASTER_IMR);
+		outb(MASTER_ICW4_DEFAULT | PIC_ICW4_AEOI, PIC_MASTER_IMR);
 	else		/* master expects normal EOI */
-		outb_pic(MASTER_ICW4_DEFAULT, PIC_MASTER_IMR);
+		outb(MASTER_ICW4_DEFAULT, PIC_MASTER_IMR);
 
-	outb_pic(0x11, PIC_SLAVE_CMD);	/* ICW1: select 8259A-2 init */
+	/* ICW1: select 8259A-2 init */
+	outb(0x11, PIC_SLAVE_CMD);
 	/* ICW2: 8259A-2 IR0-7 mapped to 0x38-0x3f */
-	outb_pic(IRQ8_VECTOR, PIC_SLAVE_IMR);
+	outb(IRQ8_VECTOR, PIC_SLAVE_IMR);
 	/* 8259A-2 is a slave on master's IR2 */
-	outb_pic(PIC_CASCADE_IR, PIC_SLAVE_IMR);
+	outb(PIC_CASCADE_IR, PIC_SLAVE_IMR);
 	/* (slave's support for AEOI in flat mode is to be investigated) */
-	outb_pic(SLAVE_ICW4_DEFAULT, PIC_SLAVE_IMR);
+	outb(SLAVE_ICW4_DEFAULT, PIC_SLAVE_IMR);
 
 	if (auto_eoi)
 		/*
Index: linux-2.6/include/asm-x86/i8259.h
===================================================================
--- linux-2.6.orig/include/asm-x86/i8259.h
+++ linux-2.6/include/asm-x86/i8259.h
@@ -29,7 +29,5 @@ extern void enable_8259A_irq(unsigned in
 extern void disable_8259A_irq(unsigned int irq);
 extern unsigned int startup_8259A_irq(unsigned int irq);
 
-#define inb_pic		inb_p
-#define outb_pic	outb_p
 
 #endif	/* __ASM_I8259_H__ */

-- 

Re: [PATCH 1/3] x86: fix init_8259A() to not use outb_pic

[Alan Cox]

On Sun, 17 Feb 2008 16:56:28 -0500 (EST)
"David P. Reed" <dpreed@reed.com> wrote:

> fix init_8259A() which initializes the 8259 PIC to not use outb_pic,
> which is a renamed version of outb_p, and delete outb_pic define.

NAK

The entire point of inb_pic/outb_pic is to isolate the various methods
and keep the logic for delays in one place. Undoing this just creates a
nasty mess.

Quite probably inb_pic/outb_pic will end up as static inlines that do inb
or outb with a udelay of 1 or 2 but that is where the knowledge belongs.

Re: [PATCH 1/3] x86: fix init_8259A() to not use outb_pic

[Rene Herman]

On 17-02-08 23:25, Alan Cox wrote:

> On Sun, 17 Feb 2008 16:56:28 -0500 (EST)
> "David P. Reed" <dpreed@reed.com> wrote:
> 
>> fix init_8259A() which initializes the 8259 PIC to not use outb_pic,
>> which is a renamed version of outb_p, and delete outb_pic define.
> 
> NAK
> 
> The entire point of inb_pic/outb_pic is to isolate the various methods
> and keep the logic for delays in one place. Undoing this just creates a
> nasty mess.
> 
> Quite probably inb_pic/outb_pic will end up as static inlines that do inb
> or outb with a udelay of 1 or 2 but that is where the knowledge belongs.

Additional NAK in sofar that the PIC delays were reported to be necesary 
with some VIA chipsets earlier in these threads.

Rene.

Re: [linux-kernel] Re: [PATCH 1/3] x86: fix init_8259A() to not use outb_pic

[David P. Reed]

Rene Herman wrote:
> On 17-02-08 23:25, Alan Cox wrote:
>
>> On Sun, 17 Feb 2008 16:56:28 -0500 (EST)
>> "David P. Reed" <dpreed@reed.com> wrote:
>>
>>> fix init_8259A() which initializes the 8259 PIC to not use outb_pic,
>>> which is a renamed version of outb_p, and delete outb_pic define.
>>
>> NAK
>>
>> The entire point of inb_pic/outb_pic is to isolate the various methods
>> and keep the logic for delays in one place. Undoing this just creates a
>> nasty mess.
>>
>> Quite probably inb_pic/outb_pic will end up as static inlines that do 
>> inb
>> or outb with a udelay of 1 or 2 but that is where the knowledge belongs.
>
> Additional NAK in sofar that the PIC delays were reported to be 
> necesary with some VIA chipsets earlier in these threads.
>
> Rene.
>
This not being a place where performance matters, I will submit a new 
patch that changes inb_pic and outb_pic to use udelay(2).  However, note 
that init_8259A does not use these consistently in its own accesses to 
the PIC registers.  Should I change it to use the _pic calls whereever 
it touches the PIC registers to be conservative?  Note that there is a 
udelay(100) after the registers are all setup, perhaps this is the real 
VIA requirement...

Re: [linux-kernel] Re: [PATCH 1/3] x86: fix init_8259A() to not use outb_pic

[Alan Cox]

I left those that were previously not _p alone. I suspect they should
proeprly be _pic however.