| Message ID | alpine.LRH.2.02.2005071145250.15191@file01.intranet.prod.int.rdu2.redhat.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v4] alpha: add a barrier after outb, outw and outl | expand |
On Thu, May 7, 2020 at 5:46 PM Mikulas Patocka <mpatocka@redhat.com> wrote: > > The patch 92d7223a74235054f2aa7227d207d9c57f84dca0 ("alpha: io: reorder > barriers to guarantee writeX() and iowriteX() ordering #2") broke boot on > the Alpha Avanti platform. > > The patch changes timing between accesses to the ISA bus, in particular, > it reduces the time between "write" access and a subsequent "read" access. > > This causes lock-up when accessing the real time clock and serial ports. > > This patch fixes the bug by adding a memory barrier after the functions > that access the ISA ports - outb, outw, outl. The barrier causes that > there is some delay between the write to an IO port and a subsequent read. Based on your earlier explanations, I would mention here that the barrier avoids the back-to-back I/O instructions on the bus that seem to be causing the problem. As I understand it (having very little alpha specific knowledge), they should prevent them by design. However if you are sure it's just the added delay rather than any actual barrier effect, that would also be worth pointing out. > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > Fixes: 92d7223a7423 ("alpha: io: reorder barriers to guarantee writeX() and iowriteX() ordering #2") > Cc: stable@vger.kernel.org # v4.17+ With or without any further clarification Acked-by: Arnd Bergmann <arnd@arndb.de>
On Thu, 7 May 2020, Arnd Bergmann wrote: > > The patch 92d7223a74235054f2aa7227d207d9c57f84dca0 ("alpha: io: reorder > > barriers to guarantee writeX() and iowriteX() ordering #2") broke boot on > > the Alpha Avanti platform. > > > > The patch changes timing between accesses to the ISA bus, in particular, > > it reduces the time between "write" access and a subsequent "read" access. > > > > This causes lock-up when accessing the real time clock and serial ports. > > > > This patch fixes the bug by adding a memory barrier after the functions > > that access the ISA ports - outb, outw, outl. The barrier causes that > > there is some delay between the write to an IO port and a subsequent read. There is no delay guarantee, just an in-order completion guarantee: #define mb() __asm__ __volatile__("mb": : :"memory") MB is a hardware memory barrier instruction. > Based on your earlier explanations, I would mention here that the barrier > avoids the back-to-back I/O instructions on the bus that seem to be causing > the problem. As I understand it (having very little alpha specific knowledge), > they should prevent them by design. However if you are sure it's just the > added delay rather than any actual barrier effect, that would also be worth > pointing out. Alpha is weakly ordered, also WRT MMIO. Writing a simple test program to poke directly (e.g. using `ioremap' and then inline asm on the location obtained) at RTC and UART registers would be a good way to determine what is really going on here. Maciej
Index: linux-stable/arch/alpha/include/asm/io.h =================================================================== --- linux-stable.orig/arch/alpha/include/asm/io.h 2020-05-07 17:36:58.000000000 +0200 +++ linux-stable/arch/alpha/include/asm/io.h 2020-05-07 17:36:58.000000000 +0200 @@ -347,11 +347,13 @@ extern inline u16 inw(unsigned long port extern inline void outb(u8 b, unsigned long port) { iowrite8(b, ioport_map(port, 1)); + mb(); } extern inline void outw(u16 b, unsigned long port) { iowrite16(b, ioport_map(port, 2)); + mb(); } #endif @@ -377,6 +379,7 @@ extern inline u32 inl(unsigned long port extern inline void outl(u32 b, unsigned long port) { iowrite32(b, ioport_map(port, 4)); + mb(); } #endif Index: linux-stable/arch/alpha/kernel/io.c =================================================================== --- linux-stable.orig/arch/alpha/kernel/io.c 2020-05-07 17:36:58.000000000 +0200 +++ linux-stable/arch/alpha/kernel/io.c 2020-05-07 17:36:58.000000000 +0200 @@ -78,16 +78,19 @@ u32 inl(unsigned long port) void outb(u8 b, unsigned long port) { iowrite8(b, ioport_map(port, 1)); + mb(); } void outw(u16 b, unsigned long port) { iowrite16(b, ioport_map(port, 2)); + mb(); } void outl(u32 b, unsigned long port) { iowrite32(b, ioport_map(port, 4)); + mb(); } EXPORT_SYMBOL(inb);
The patch 92d7223a74235054f2aa7227d207d9c57f84dca0 ("alpha: io: reorder barriers to guarantee writeX() and iowriteX() ordering #2") broke boot on the Alpha Avanti platform. The patch changes timing between accesses to the ISA bus, in particular, it reduces the time between "write" access and a subsequent "read" access. This causes lock-up when accessing the real time clock and serial ports. This patch fixes the bug by adding a memory barrier after the functions that access the ISA ports - outb, outw, outl. The barrier causes that there is some delay between the write to an IO port and a subsequent read. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Fixes: 92d7223a7423 ("alpha: io: reorder barriers to guarantee writeX() and iowriteX() ordering #2") Cc: stable@vger.kernel.org # v4.17+ --- arch/alpha/include/asm/io.h | 3 +++ arch/alpha/kernel/io.c | 3 +++ 2 files changed, 6 insertions(+)