On Wed, 20 Feb 2019, Maciej W. Rozycki wrote: > On Wed, 20 Feb 2019, Mikulas Patocka wrote: > > > > Well, actually `iowriteX' is generic and for MMIO you have `writeX'. > > > See lib/iomap.c, the comment at the top and e.g. `iowrite8' there for an > > > actual proof. Alpha may have an oddball implementation, but there you go. > > > Drivers will assume they can do `iowriteX' to any kind of I/O resource, > > > and ordering must be respected as per Documentation/memory-barriers.txt. > > > > So, do you think that the barrier whould be added to iowriteX and slow > > down every MMIO access? > > We need it either for `outX' and `iowriteX' calls operating on port I/O > resources, or for neither of them, both at a time, to ensure the required > consistency between the two interfaces. If that badly affects MMIO (and > is not required there; please remind me what the exact justification to > use `mb' here is, as it's not entirely clear to me from the commit > message; `mb' is a barrier and not means for a delay), then we need to > find a away for `iowriteX' to tell port I/O and MMIO accesses apart and > only apply the barrier for the former kind. > > Maciej I did some more testing and it turns out that mb() is not entirely sufficient to prevent the boot hang. mb()'s latecy varies, sometimes it is sufficient, sometimes not. So, I submit this patch that adds 1us delay between any I/O accesses directed at the ISA bus. This patch makes my machine boot. 1us seems to be minimal acceptable value, with 800ns I still get hangs. Mikulas From: Mikulas Patocka <mpatocka@xxxxxxxxxx> Subject: [PATCH] alpha: add udelay to io port paths The patches cd0e00c106722eca40b38ebf11cf134c01901086 and 92d7223a74235054f2aa7227d207d9c57f84dca0 fix a theoretical issue where the code didn't follow the specification. Unfortunatelly, they also reduce timing between IO port write and IO port read. This causes problem on Alpha Avanti. Old chipsets based in the ISA bus have problems with back-to-back I/O accesses and require delays. The "mb()" call used to serve as a delay, hiding the bug. However, using mb() for delay is unreliable, so we'd better add explicit delays. This patch adds the required delays between accesses to the ISA bus. Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx> Fixes: cd0e00c10672 ("alpha: io: reorder barriers to guarantee writeX() and iowriteX() ordering") Cc: stable@xxxxxxxxxxxxxxx # v4.17+ --- arch/alpha/kernel/io.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) Index: linux-stable/arch/alpha/kernel/io.c =================================================================== --- linux-stable.orig/arch/alpha/kernel/io.c 2019-04-03 15:26:53.000000000 +0200 +++ linux-stable/arch/alpha/kernel/io.c 2019-04-03 15:28:57.000000000 +0200 @@ -8,6 +8,7 @@ #include <linux/string.h> #include <linux/module.h> #include <asm/io.h> +#include <asm/delay.h> /* Out-of-line versions of the i/o routines that redirect into the platform-specific version. Note that "platform-specific" may mean @@ -60,34 +61,49 @@ EXPORT_SYMBOL(iowrite8); EXPORT_SYMBOL(iowrite16); EXPORT_SYMBOL(iowrite32); +static void udelay_if_isa(unsigned long port) +{ + if (port < 0x400) + udelay(1); +} + u8 inb(unsigned long port) { - return ioread8(ioport_map(port, 1)); + u8 r = ioread8(ioport_map(port, 1)); + udelay_if_isa(port); + return r; } u16 inw(unsigned long port) { - return ioread16(ioport_map(port, 2)); + u16 r = ioread16(ioport_map(port, 2)); + udelay_if_isa(port); + return r; } u32 inl(unsigned long port) { - return ioread32(ioport_map(port, 4)); + u32 r = ioread32(ioport_map(port, 4)); + udelay_if_isa(port); + return r; } void outb(u8 b, unsigned long port) { iowrite8(b, ioport_map(port, 1)); + udelay_if_isa(port); } void outw(u16 b, unsigned long port) { iowrite16(b, ioport_map(port, 2)); + udelay_if_isa(port); } void outl(u32 b, unsigned long port) { iowrite32(b, ioport_map(port, 4)); + udelay_if_isa(port); } EXPORT_SYMBOL(inb); @@ -242,6 +258,7 @@ void ioread8_rep(void __iomem *port, voi void insb(unsigned long port, void *dst, unsigned long count) { ioread8_rep(ioport_map(port, 1), dst, count); + udelay_if_isa(port); } EXPORT_SYMBOL(ioread8_rep); @@ -282,6 +299,7 @@ void ioread16_rep(void __iomem *port, vo void insw(unsigned long port, void *dst, unsigned long count) { ioread16_rep(ioport_map(port, 2), dst, count); + udelay_if_isa(port); } EXPORT_SYMBOL(ioread16_rep); @@ -314,6 +332,7 @@ void ioread32_rep(void __iomem *port, vo void insl(unsigned long port, void *dst, unsigned long count) { ioread32_rep(ioport_map(port, 4), dst, count); + udelay_if_isa(port); } EXPORT_SYMBOL(ioread32_rep); @@ -336,6 +355,7 @@ void iowrite8_rep(void __iomem *port, co void outsb(unsigned long port, const void *src, unsigned long count) { iowrite8_rep(ioport_map(port, 1), src, count); + udelay_if_isa(port); } EXPORT_SYMBOL(iowrite8_rep); @@ -376,6 +396,7 @@ void iowrite16_rep(void __iomem *port, c void outsw(unsigned long port, const void *src, unsigned long count) { iowrite16_rep(ioport_map(port, 2), src, count); + udelay_if_isa(port); } EXPORT_SYMBOL(iowrite16_rep); @@ -408,6 +429,7 @@ void iowrite32_rep(void __iomem *port, c void outsl(unsigned long port, const void *src, unsigned long count) { iowrite32_rep(ioport_map(port, 4), src, count); + udelay_if_isa(port); } EXPORT_SYMBOL(iowrite32_rep);