[PATCH][PPC64] strengthen I/O and memory barriers

From: Paul Mackerras
Date: Thu May 13 2004 - 06:51:47 EST


After I sent the recent patch to include/asm-ppc64/io.h which put
stronger barriers in the I/O accessor macros, Paul McKenney pointed
out to me that a writex/outx could still slide out from inside a
spinlocked region. This patch makes the barriers a bit stronger so
that this can't happen. It means that we need to use a sync
instruction for wmb (a full "heavyweight" sync), since drivers rely on
wmb for ordering between writes to system memory and writes to a
device.

I have left smb_wmb() as a lighter-weight barrier that orders stores,
and doesn't impose an ordering between cacheable and non-cacheable
accesses (the amusingly-named eieio instruction). I am assuming here
that smp_wmb is only used for ordering stores to system memory so that
another cpu will see them in order. It can't be used for enforcing
any ordering that a device will see, because it is just a gcc barrier
on UP.

This also changes the spinlock/rwlock unlock code to use lwsync
("light-weight sync") rather than eieio, since eieio doesn't order
loads, and we need to ensure that loads stay inside the spinlocked
region.

Please apply.

Thanks,
Paul.

diff -urN linux-2.5/arch/ppc64/kernel/misc.S g5-ppc64/arch/ppc64/kernel/misc.S
--- linux-2.5/arch/ppc64/kernel/misc.S 2004-05-11 07:53:04.000000000 +1000
+++ g5-ppc64/arch/ppc64/kernel/misc.S 2004-05-13 21:17:44.750913072 +1000
@@ -316,6 +316,8 @@
eieio
stbu r5,1(r4)
bdnz 00b
+ twi 0,r5,0
+ isync
blr

_GLOBAL(_outsb)
@@ -325,8 +327,8 @@
blelr-
00: lbzu r5,1(r4)
stb r5,0(r3)
- eieio
bdnz 00b
+ sync
blr

_GLOBAL(_insw)
@@ -338,6 +340,8 @@
eieio
sthu r5,2(r4)
bdnz 00b
+ twi 0,r5,0
+ isync
blr

_GLOBAL(_outsw)
@@ -346,9 +350,9 @@
subi r4,r4,2
blelr-
00: lhzu r5,2(r4)
- eieio
sthbrx r5,0,r3
bdnz 00b
+ sync
blr

_GLOBAL(_insl)
@@ -360,6 +364,8 @@
eieio
stwu r5,4(r4)
bdnz 00b
+ twi 0,r5,0
+ isync
blr

_GLOBAL(_outsl)
@@ -369,8 +375,8 @@
blelr-
00: lwzu r5,4(r4)
stwbrx r5,0,r3
- eieio
bdnz 00b
+ sync
blr

/* _GLOBAL(ide_insw) now in drivers/ide/ide-iops.c */
@@ -383,6 +389,8 @@
eieio
sthu r5,2(r4)
bdnz 00b
+ twi 0,r5,0
+ isync
blr

/* _GLOBAL(ide_outsw) now in drivers/ide/ide-iops.c */
@@ -393,8 +401,8 @@
blelr-
00: lhzu r5,2(r4)
sth r5,0(r3)
- eieio
bdnz 00b
+ sync
blr

_GLOBAL(_insl_ns)
@@ -406,6 +414,8 @@
eieio
stwu r5,4(r4)
bdnz 00b
+ twi 0,r5,0
+ isync
blr

_GLOBAL(_outsl_ns)
@@ -415,8 +425,8 @@
blelr-
00: lwzu r5,4(r4)
stw r5,0(r3)
- eieio
bdnz 00b
+ sync
blr

_GLOBAL(abs)
diff -urN linux-2.5/include/asm-ppc64/io.h g5-ppc64/include/asm-ppc64/io.h
--- linux-2.5/include/asm-ppc64/io.h 2004-05-11 13:19:51.000000000 +1000
+++ g5-ppc64/include/asm-ppc64/io.h 2004-05-11 16:24:47.000000000 +1000
@@ -240,14 +240,14 @@
{
int ret;

- __asm__ __volatile__("eieio; lbz%U1%X1 %0,%1; twi 0,%0,0; isync"
+ __asm__ __volatile__("lbz%U1%X1 %0,%1; twi 0,%0,0; isync"
: "=r" (ret) : "m" (*addr));
return ret;
}

static inline void out_8(volatile unsigned char *addr, int val)
{
- __asm__ __volatile__("sync; stb%U0%X0 %1,%0"
+ __asm__ __volatile__("stb%U0%X0 %1,%0; sync"
: "=m" (*addr) : "r" (val));
}

@@ -255,7 +255,7 @@
{
int ret;

- __asm__ __volatile__("eieio; lhbrx %0,0,%1; twi 0,%0,0; isync"
+ __asm__ __volatile__("lhbrx %0,0,%1; twi 0,%0,0; isync"
: "=r" (ret) : "r" (addr), "m" (*addr));
return ret;
}
@@ -264,20 +264,20 @@
{
int ret;

- __asm__ __volatile__("eieio; lhz%U1%X1 %0,%1; twi 0,%0,0; isync"
+ __asm__ __volatile__("lhz%U1%X1 %0,%1; twi 0,%0,0; isync"
: "=r" (ret) : "m" (*addr));
return ret;
}

static inline void out_le16(volatile unsigned short *addr, int val)
{
- __asm__ __volatile__("sync; sthbrx %1,0,%2"
+ __asm__ __volatile__("sthbrx %1,0,%2; sync"
: "=m" (*addr) : "r" (val), "r" (addr));
}

static inline void out_be16(volatile unsigned short *addr, int val)
{
- __asm__ __volatile__("sync; sth%U0%X0 %1,%0"
+ __asm__ __volatile__("sth%U0%X0 %1,%0; sync"
: "=m" (*addr) : "r" (val));
}

@@ -285,7 +285,7 @@
{
unsigned ret;

- __asm__ __volatile__("eieio; lwbrx %0,0,%1; twi 0,%0,0; isync"
+ __asm__ __volatile__("lwbrx %0,0,%1; twi 0,%0,0; isync"
: "=r" (ret) : "r" (addr), "m" (*addr));
return ret;
}
@@ -294,20 +294,20 @@
{
unsigned ret;

- __asm__ __volatile__("eieio; lwz%U1%X1 %0,%1; twi 0,%0,0; isync"
+ __asm__ __volatile__("lwz%U1%X1 %0,%1; twi 0,%0,0; isync"
: "=r" (ret) : "m" (*addr));
return ret;
}

static inline void out_le32(volatile unsigned *addr, int val)
{
- __asm__ __volatile__("sync; stwbrx %1,0,%2" : "=m" (*addr)
+ __asm__ __volatile__("stwbrx %1,0,%2; sync" : "=m" (*addr)
: "r" (val), "r" (addr));
}

static inline void out_be32(volatile unsigned *addr, int val)
{
- __asm__ __volatile__("sync; stw%U0%X0 %1,%0; eieio"
+ __asm__ __volatile__("stw%U0%X0 %1,%0; eieio"
: "=m" (*addr) : "r" (val));
}

@@ -316,7 +316,7 @@
unsigned long tmp, ret;

__asm__ __volatile__(
- "eieio; ld %1,0(%2)\n"
+ "ld %1,0(%2)\n"
"twi 0,%1,0\n"
"isync\n"
"rldimi %0,%1,5*8,1*8\n"
@@ -334,7 +334,7 @@
{
unsigned long ret;

- __asm__ __volatile__("eieio; ld %0,0(%1); twi 0,%0,0; isync"
+ __asm__ __volatile__("ld %0,0(%1); twi 0,%0,0; isync"
: "=r" (ret) : "m" (*addr));
return ret;
}
@@ -351,13 +351,14 @@
"rldicl %1,%1,32,0\n"
"rlwimi %0,%1,8,8,31\n"
"rlwimi %0,%1,24,16,23\n"
- "sync; std %0,0(%2)\n"
+ "std %0,0(%2)\n"
+ "sync"
: "=r" (tmp) : "r" (val), "b" (addr) , "m" (*addr));
}

static inline void out_be64(volatile unsigned long *addr, int val)
{
- __asm__ __volatile__("sync; std %1,0(%0)" : "=m" (*addr) : "r" (val));
+ __asm__ __volatile__("std %1,0(%0); sync" : "=m" (*addr) : "r" (val));
}

#ifndef CONFIG_PPC_ISERIES
diff -urN linux-2.5/include/asm-ppc64/spinlock.h g5-ppc64/include/asm-ppc64/spinlock.h
--- linux-2.5/include/asm-ppc64/spinlock.h 2004-05-11 07:53:05.000000000 +1000
+++ g5-ppc64/include/asm-ppc64/spinlock.h 2004-05-11 16:10:12.000000000 +1000
@@ -28,7 +28,7 @@

static __inline__ void _raw_spin_unlock(spinlock_t *lock)
{
- __asm__ __volatile__("eieio # spin_unlock": : :"memory");
+ __asm__ __volatile__("lwsync # spin_unlock": : :"memory");
lock->lock = 0;
}

@@ -159,7 +159,7 @@

static __inline__ void _raw_write_unlock(rwlock_t *rw)
{
- __asm__ __volatile__("eieio # write_unlock": : :"memory");
+ __asm__ __volatile__("lwsync # write_unlock": : :"memory");
rw->lock = 0;
}

@@ -223,7 +223,7 @@
unsigned int tmp;

__asm__ __volatile__(
- "eieio # read_unlock\n\
+ "lwsync # read_unlock\n\
1: lwarx %0,0,%1\n\
addic %0,%0,-1\n\
stwcx. %0,0,%1\n\
diff -urN linux-2.5/include/asm-ppc64/system.h g5-ppc64/include/asm-ppc64/system.h
--- linux-2.5/include/asm-ppc64/system.h 2004-04-13 09:25:10.000000000 +1000
+++ g5-ppc64/include/asm-ppc64/system.h 2004-05-11 16:09:43.000000000 +1000
@@ -29,22 +29,26 @@
* read_barrier_depends() prevents data-dependent loads being reordered
* across this point (nop on PPC).
*
- * We can use the eieio instruction for wmb, but since it doesn't
- * give any ordering guarantees about loads, we have to use the
- * stronger but slower sync instruction for mb and rmb.
+ * We have to use the sync instructions for mb(), since lwsync doesn't
+ * order loads with respect to previous stores. Lwsync is fine for
+ * rmb(), though.
+ * For wmb(), we use sync since wmb is used in drivers to order
+ * stores to system memory with respect to writes to the device.
+ * However, smp_wmb() can be a lighter-weight eieio barrier on
+ * SMP since it is only used to order updates to system memory.
*/
#define mb() __asm__ __volatile__ ("sync" : : : "memory")
#define rmb() __asm__ __volatile__ ("lwsync" : : : "memory")
-#define wmb() __asm__ __volatile__ ("eieio" : : : "memory")
+#define wmb() __asm__ __volatile__ ("sync" : : : "memory")
#define read_barrier_depends() do { } while(0)

-#define set_mb(var, value) do { var = value; mb(); } while (0)
-#define set_wmb(var, value) do { var = value; wmb(); } while (0)
+#define set_mb(var, value) do { var = value; smp_mb(); } while (0)
+#define set_wmb(var, value) do { var = value; smp_wmb(); } while (0)

#ifdef CONFIG_SMP
#define smp_mb() mb()
#define smp_rmb() rmb()
-#define smp_wmb() wmb()
+#define smp_wmb() __asm__ __volatile__ ("eieio" : : : "memory")
#define smp_read_barrier_depends() read_barrier_depends()
#else
#define smp_mb() __asm__ __volatile__("": : :"memory")
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/