Date: Wed, 23 May 2012 11:59:12 -0700 From: Adrian Chadd <adrian@freebsd.org> To: Nathan Whitehorn <nwhitehorn@freebsd.org> Cc: freebsd-wireless@freebsd.org Subject: Re: svn commit: r234579 - in head/sys/powerpc: include mpc85xx powerpc Message-ID: <CAJ-VmomZCJO_J9OTRRc-gNCvNwhMgXz_dCEyAdCbOY4_OVKN9w@mail.gmail.com> In-Reply-To: <201204221854.q3MIsqPg052441@svn.freebsd.org> References: <201204221854.q3MIsqPg052441@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Nathan, Would you please nag me to make sure I add explicit write barrier handling to ath / ath_hal? It's needed for USB support for some chips that use the HAL (performance reasons) and I can basically re-use exactly the same thing. We could then "extend" the bus API to allow a driver to say they implement correct bus coherence hooks, so don't just call sync on every write. Thanks, Adrian On 22 April 2012 11:54, Nathan Whitehorn <nwhitehorn@freebsd.org> wrote: > Author: nwhitehorn > Date: Sun Apr 22 18:54:51 2012 > New Revision: 234579 > URL: http://svn.freebsd.org/changeset/base/234579 > > Log: > Replace eieio; sync for creating bus-space memory barriers with sync. > sync performs a strict superset of the functions of eieio, so using both > is redundant. While here, expand bus barriers to all bus_space operations, > since many drivers do not correctly use bus_space_barrier(). > > In principle, we can also replace sync just with eieio, for a significant > performance increase, but it remains to be seen whether any poorly-written > drivers currently depend on the side effects of sync to properly function. > > MFC after: 1 week > > Modified: > head/sys/powerpc/include/pio.h > head/sys/powerpc/mpc85xx/mpc85xx.c > head/sys/powerpc/powerpc/bus_machdep.c > > Modified: head/sys/powerpc/include/pio.h > ============================================================================== > --- head/sys/powerpc/include/pio.h Sun Apr 22 18:51:38 2012 (r234578) > +++ head/sys/powerpc/include/pio.h Sun Apr 22 18:54:51 2012 (r234579) > @@ -39,46 +39,52 @@ > * I/O macros. > */ > > +/* > + * Note: this should be eieio, but many drivers expect ordering with > + * main storage too. > + */ > +#define powerpc_iomb() __asm __volatile("sync" : : : "memory") > + > static __inline void > __outb(volatile u_int8_t *a, u_int8_t v) > { > *a = v; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > __outw(volatile u_int16_t *a, u_int16_t v) > { > *a = v; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > __outl(volatile u_int32_t *a, u_int32_t v) > { > *a = v; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > __outll(volatile u_int64_t *a, u_int64_t v) > { > *a = v; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > __outwrb(volatile u_int16_t *a, u_int16_t v) > { > __asm__ volatile("sthbrx %0, 0, %1" :: "r"(v), "r"(a)); > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > __outlrb(volatile u_int32_t *a, u_int32_t v) > { > __asm__ volatile("stwbrx %0, 0, %1" :: "r"(v), "r"(a)); > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline u_int8_t > @@ -87,7 +93,7 @@ __inb(volatile u_int8_t *a) > u_int8_t _v_; > > _v_ = *a; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > return _v_; > } > > @@ -97,7 +103,7 @@ __inw(volatile u_int16_t *a) > u_int16_t _v_; > > _v_ = *a; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > return _v_; > } > > @@ -107,7 +113,7 @@ __inl(volatile u_int32_t *a) > u_int32_t _v_; > > _v_ = *a; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > return _v_; > } > > @@ -117,7 +123,7 @@ __inll(volatile u_int64_t *a) > u_int64_t _v_; > > _v_ = *a; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > return _v_; > } > > @@ -127,7 +133,7 @@ __inwrb(volatile u_int16_t *a) > u_int16_t _v_; > > __asm__ volatile("lhbrx %0, 0, %1" : "=r"(_v_) : "r"(a)); > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > return _v_; > } > > @@ -137,7 +143,7 @@ __inlrb(volatile u_int32_t *a) > u_int32_t _v_; > > __asm__ volatile("lwbrx %0, 0, %1" : "=r"(_v_) : "r"(a)); > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > return _v_; > } > > @@ -175,7 +181,7 @@ __outsb(volatile u_int8_t *a, const u_in > { > while (c--) > *a = *s++; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > @@ -183,7 +189,7 @@ __outsw(volatile u_int16_t *a, const u_i > { > while (c--) > *a = *s++; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > @@ -191,7 +197,7 @@ __outsl(volatile u_int32_t *a, const u_i > { > while (c--) > *a = *s++; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > @@ -199,7 +205,7 @@ __outsll(volatile u_int64_t *a, const u_ > { > while (c--) > *a = *s++; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > @@ -207,7 +213,7 @@ __outswrb(volatile u_int16_t *a, const u > { > while (c--) > __asm__ volatile("sthbrx %0, 0, %1" :: "r"(*s++), "r"(a)); > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > @@ -215,7 +221,7 @@ __outslrb(volatile u_int32_t *a, const u > { > while (c--) > __asm__ volatile("stwbrx %0, 0, %1" :: "r"(*s++), "r"(a)); > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > @@ -223,7 +229,7 @@ __insb(volatile u_int8_t *a, u_int8_t *d > { > while (c--) > *d++ = *a; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > @@ -231,7 +237,7 @@ __insw(volatile u_int16_t *a, u_int16_t > { > while (c--) > *d++ = *a; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > @@ -239,7 +245,7 @@ __insl(volatile u_int32_t *a, u_int32_t > { > while (c--) > *d++ = *a; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > @@ -247,7 +253,7 @@ __insll(volatile u_int64_t *a, u_int64_t > { > while (c--) > *d++ = *a; > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > @@ -255,7 +261,7 @@ __inswrb(volatile u_int16_t *a, u_int16_ > { > while (c--) > __asm__ volatile("lhbrx %0, 0, %1" : "=r"(*d++) : "r"(a)); > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > static __inline void > @@ -263,7 +269,7 @@ __inslrb(volatile u_int32_t *a, u_int32_ > { > while (c--) > __asm__ volatile("lwbrx %0, 0, %1" : "=r"(*d++) : "r"(a)); > - __asm__ volatile("eieio; sync"); > + powerpc_iomb(); > } > > #define outsb(a,s,c) (__outsb((volatile u_int8_t *)(a), s, c)) > > Modified: head/sys/powerpc/mpc85xx/mpc85xx.c > ============================================================================== > --- head/sys/powerpc/mpc85xx/mpc85xx.c Sun Apr 22 18:51:38 2012 (r234578) > +++ head/sys/powerpc/mpc85xx/mpc85xx.c Sun Apr 22 18:54:51 2012 (r234579) > @@ -60,7 +60,7 @@ ccsr_write4(uintptr_t addr, uint32_t val > volatile uint32_t *ptr = (void *)addr; > > *ptr = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > int > > Modified: head/sys/powerpc/powerpc/bus_machdep.c > ============================================================================== > --- head/sys/powerpc/powerpc/bus_machdep.c Sun Apr 22 18:51:38 2012 (r234578) > +++ head/sys/powerpc/powerpc/bus_machdep.c Sun Apr 22 18:54:51 2012 (r234579) > @@ -169,7 +169,8 @@ static void > bs_gen_barrier(bus_space_handle_t bsh __unused, bus_size_t ofs __unused, > bus_size_t size __unused, int flags __unused) > { > - __asm __volatile("eieio; sync" : : : "memory"); > + > + powerpc_iomb(); > } > > /* > @@ -183,6 +184,7 @@ bs_be_rs_1(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > res = *addr; > + powerpc_iomb(); > CTR4(KTR_BE_IO, "%s(bsh=%#x, ofs=%#x) = %#x", __func__, bsh, ofs, res); > return (res); > } > @@ -195,6 +197,7 @@ bs_be_rs_2(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > res = *addr; > + powerpc_iomb(); > CTR4(KTR_BE_IO, "%s(bsh=%#x, ofs=%#x) = %#x", __func__, bsh, ofs, res); > return (res); > } > @@ -207,6 +210,7 @@ bs_be_rs_4(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > res = *addr; > + powerpc_iomb(); > CTR4(KTR_BE_IO, "%s(bsh=%#x, ofs=%#x) = %#x", __func__, bsh, ofs, res); > return (res); > } > @@ -219,6 +223,7 @@ bs_be_rs_8(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > res = *addr; > + powerpc_iomb(); > return (res); > } > > @@ -253,7 +258,7 @@ bs_be_rr_1(bus_space_handle_t bsh, bus_s > > while (cnt--) > *addr++ = *s++; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -263,7 +268,7 @@ bs_be_rr_2(bus_space_handle_t bsh, bus_s > > while (cnt--) > *addr++ = *s++; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -273,7 +278,7 @@ bs_be_rr_4(bus_space_handle_t bsh, bus_s > > while (cnt--) > *addr++ = *s++; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -283,7 +288,7 @@ bs_be_rr_8(bus_space_handle_t bsh, bus_s > > while (cnt--) > *addr++ = *s++; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -293,7 +298,7 @@ bs_be_ws_1(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > *addr = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > CTR4(KTR_BE_IO, "%s(bsh=%#x, ofs=%#x, val=%#x)", __func__, bsh, ofs, val); > } > > @@ -304,7 +309,7 @@ bs_be_ws_2(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > *addr = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > CTR4(KTR_BE_IO, "%s(bsh=%#x, ofs=%#x, val=%#x)", __func__, bsh, ofs, val); > } > > @@ -315,7 +320,7 @@ bs_be_ws_4(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > *addr = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > CTR4(KTR_BE_IO, "%s(bsh=%#x, ofs=%#x, val=%#x)", __func__, bsh, ofs, val); > } > > @@ -326,7 +331,7 @@ bs_be_ws_8(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > *addr = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -365,7 +370,7 @@ bs_be_wr_1(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d++ = *addr++; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -376,7 +381,7 @@ bs_be_wr_2(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d++ = *addr++; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -387,7 +392,7 @@ bs_be_wr_4(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d++ = *addr++; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -398,7 +403,7 @@ bs_be_wr_8(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d++ = *addr++; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -408,7 +413,7 @@ bs_be_sm_1(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -418,7 +423,7 @@ bs_be_sm_2(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -428,7 +433,7 @@ bs_be_sm_4(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -438,7 +443,7 @@ bs_be_sm_8(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -448,7 +453,7 @@ bs_be_sr_1(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d++ = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -458,7 +463,7 @@ bs_be_sr_2(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d++ = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -468,7 +473,7 @@ bs_be_sr_4(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d++ = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -478,7 +483,7 @@ bs_be_sr_8(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d++ = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > /* > @@ -492,7 +497,7 @@ bs_le_rs_1(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > res = *addr; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > CTR4(KTR_LE_IO, "%s(bsh=%#x, ofs=%#x) = %#x", __func__, bsh, ofs, res); > return (res); > } > @@ -505,7 +510,7 @@ bs_le_rs_2(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > __asm __volatile("lhbrx %0, 0, %1" : "=r"(res) : "r"(addr)); > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > CTR4(KTR_LE_IO, "%s(bsh=%#x, ofs=%#x) = %#x", __func__, bsh, ofs, res); > return (res); > } > @@ -518,7 +523,7 @@ bs_le_rs_4(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > __asm __volatile("lwbrx %0, 0, %1" : "=r"(res) : "r"(addr)); > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > CTR4(KTR_LE_IO, "%s(bsh=%#x, ofs=%#x) = %#x", __func__, bsh, ofs, res); > return (res); > } > @@ -560,7 +565,7 @@ bs_le_rr_1(bus_space_handle_t bsh, bus_s > > while (cnt--) > *addr++ = *s++; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -570,7 +575,7 @@ bs_le_rr_2(bus_space_handle_t bsh, bus_s > > while (cnt--) > *addr++ = in16rb(s++); > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -580,7 +585,7 @@ bs_le_rr_4(bus_space_handle_t bsh, bus_s > > while (cnt--) > *addr++ = in32rb(s++); > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -596,6 +601,7 @@ bs_le_ws_1(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > *addr = val; > + powerpc_iomb(); > CTR4(KTR_LE_IO, "%s(bsh=%#x, ofs=%#x, val=%#x)", __func__, bsh, ofs, val); > } > > @@ -606,6 +612,7 @@ bs_le_ws_2(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > __asm __volatile("sthbrx %0, 0, %1" :: "r"(val), "r"(addr)); > + powerpc_iomb(); > CTR4(KTR_LE_IO, "%s(bsh=%#x, ofs=%#x, val=%#x)", __func__, bsh, ofs, val); > } > > @@ -616,6 +623,7 @@ bs_le_ws_4(bus_space_handle_t bsh, bus_s > > addr = __ppc_ba(bsh, ofs); > __asm __volatile("stwbrx %0, 0, %1" :: "r"(val), "r"(addr)); > + powerpc_iomb(); > CTR4(KTR_LE_IO, "%s(bsh=%#x, ofs=%#x, val=%#x)", __func__, bsh, ofs, val); > } > > @@ -661,7 +669,7 @@ bs_le_wr_1(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d++ = *addr++; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -672,7 +680,7 @@ bs_le_wr_2(bus_space_handle_t bsh, bus_s > > while (cnt--) > out16rb(d++, *addr++); > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -683,7 +691,7 @@ bs_le_wr_4(bus_space_handle_t bsh, bus_s > > while (cnt--) > out32rb(d++, *addr++); > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -700,7 +708,7 @@ bs_le_sm_1(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -710,7 +718,7 @@ bs_le_sm_2(bus_space_handle_t bsh, bus_s > > while (cnt--) > out16rb(d, val); > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -720,7 +728,7 @@ bs_le_sm_4(bus_space_handle_t bsh, bus_s > > while (cnt--) > out32rb(d, val); > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -736,7 +744,7 @@ bs_le_sr_1(bus_space_handle_t bsh, bus_s > > while (cnt--) > *d++ = val; > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -746,7 +754,7 @@ bs_le_sr_2(bus_space_handle_t bsh, bus_s > > while (cnt--) > out16rb(d++, val); > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void > @@ -756,7 +764,7 @@ bs_le_sr_4(bus_space_handle_t bsh, bus_s > > while (cnt--) > out32rb(d++, val); > - __asm __volatile("eieio; sync"); > + powerpc_iomb(); > } > > static void
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-VmomZCJO_J9OTRRc-gNCvNwhMgXz_dCEyAdCbOY4_OVKN9w>