Re: RFC: outb 0x80 in inb_p, outb_p harmful on some modern AMD64 with MCP51 laptops

Ingo Molnar
2007-12-14T13:34:28Z

* Andi Kleen <andi@firstfloor.org> wrote:

> > Why is TSC significant? udelay() based on bogomips seems to be good
> > enough...?
>
> Maybe I'm not sure how accurate it really is on non TSC system. On the
> other hand it is unclear that the port 80 IO is always the same time
> so it's probably ok to vary a bit. So most likely going to udelay()
> unconditionally is fine.

yep, agreed, and have queued up the patch below. I've killed the
misc_*.c outb_p() uses because they happen before there's an udelay()
available - but that should be perfectly fine anyway: i dont remember
any video hardware that needed pauses for cursor updates, i think those
_p()'s just came in accidentally. (there's hardware that needed _p() for
other aspects of video such as mode switching - but cursor updates ...)

Ingo

------------------>
Subject: x86: fix in/out_p delays
From: Ingo Molnar <mingo@elte.hu>

Debugged by David P. Reed <dpreed@reed.com>.

Do not use port 0x80, it can cause crashes, see:

http://bugzilla.kernel.org/show_bug.cgi?id=6307
http://bugzilla.kernel.org/show_bug.cgi?id=9511

instead of just removing _p postfixes en masse, lets just first
remove the 0x80 port usage, then remove any unnecessary _p io ops
gradually. It's more debuggable this way.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
arch/x86/boot/compressed/misc_32.c | 8 ++++----
arch/x86/boot/compressed/misc_64.c | 8 ++++----
arch/x86/kernel/quirks.c | 10 ++++++++++
include/asm-x86/io_32.h | 5 +----
include/asm-x86/io_64.h | 5 +----
5 files changed, 20 insertions(+), 16 deletions(-)

Index: linux-x86.q/arch/x86/boot/compressed/misc_32.c
===================================================================
--- linux-x86.q.orig/arch/x86/boot/compressed/misc_32.c
+++ linux-x86.q/arch/x86/boot/compressed/misc_32.c
@@ -276,10 +276,10 @@ static void putstr(const char *s)
RM_SCREEN_INFO.orig_y = y;

pos = (x + cols * y) * 2; /* Update cursor position */
- outb_p(14, vidport);
- outb_p(0xff & (pos >> 9), vidport+1);
- outb_p(15, vidport);
- outb_p(0xff & (pos >> 1), vidport+1);
+ outb(14, vidport);
+ outb(0xff & (pos >> 9), vidport+1);
+ outb(15, vidport);
+ outb(0xff & (pos >> 1), vidport+1);
}

static void* memset(void* s, int c, unsigned n)
Index: linux-x86.q/arch/x86/boot/compressed/misc_64.c
===================================================================
--- linux-x86.q.orig/arch/x86/boot/compressed/misc_64.c
+++ linux-x86.q/arch/x86/boot/compressed/misc_64.c
@@ -275,10 +275,10 @@ static void putstr(const char *s)
RM_SCREEN_INFO.orig_y = y;

pos = (x + cols * y) * 2; /* Update cursor position */
- outb_p(14, vidport);
- outb_p(0xff & (pos >> 9), vidport+1);
- outb_p(15, vidport);
- outb_p(0xff & (pos >> 1), vidport+1);
+ outb(14, vidport);
+ outb(0xff & (pos >> 9), vidport+1);
+ outb(15, vidport);
+ outb(0xff & (pos >> 1), vidport+1);
}

static void* memset(void* s, int c, unsigned n)
Index: linux-x86.q/arch/x86/kernel/quirks.c
===================================================================
--- linux-x86.q.orig/arch/x86/kernel/quirks.c
+++ linux-x86.q/arch/x86/kernel/quirks.c
@@ -3,9 +3,19 @@
*/
#include <linux/pci.h>
#include <linux/irq.h>
+#include <linux/delay.h>

#include <asm/hpet.h>

+/*
+ * Some legacy devices need delays for IN/OUT sequences. Most are
+ * probably not needed but it's the safest to just do this short delay:
+ */
+void native_io_delay(void)
+{
+ udelay(1);
+}
+
#if defined(CONFIG_X86_IO_APIC) && defined(CONFIG_SMP) && defined(CONFIG_PCI)

static void __devinit quirk_intel_irqbalance(struct pci_dev *dev)
Index: linux-x86.q/include/asm-x86/io_32.h
===================================================================
--- linux-x86.q.orig/include/asm-x86/io_32.h
+++ linux-x86.q/include/asm-x86/io_32.h
@@ -250,10 +250,7 @@ static inline void flush_write_buffers(v

#endif /* __KERNEL__ */

-static inline void native_io_delay(void)
-{
- asm volatile("outb %%al,$0x80" : : : "memory");
-}
+extern void native_io_delay(void);

#if defined(CONFIG_PARAVIRT)
#include <asm/paravirt.h>
Index: linux-x86.q/include/asm-x86/io_64.h
===================================================================
--- linux-x86.q.orig/include/asm-x86/io_64.h
+++ linux-x86.q/include/asm-x86/io_64.h
@@ -35,10 +35,7 @@
* - Arnaldo Carvalho de Melo <acme@conectiva.com.br>
*/

-static inline void native_io_delay(void)
-{
- asm volatile("outb %%al,$0x80" : : : "memory");
-}
+extern void native_io_delay(void);

#if defined(CONFIG_PARAVIRT)
#include <asm/paravirt.h>