You should probably revert this. The implied understanding of the _relaxed version is incorrect. compiler_membar is there to prevent instruction reordering which is possible on FreeBSD because the accesses are done in C. The relaxed variants still do not permit instruction reordering. On Linux __compiler_member (referred to as barrier there) isn’t necessary in the mmio accessors because they always use volatile asm which can’t be reordered. The distinction between the relaxed and non relaxed variants is that the relaxed variant lacks memory barriers (sync / lwsync / eieio on ppc, membar on sparc, etc). Most of the time we don’t run in to problems on x86 because with TSO the only reordering possible is writes that happen before reads can become visible in memory after they occur in the instruction stream.
Thanks. -M On Mon, Oct 22, 2018 at 13:55 Tijl Coosemans <t...@freebsd.org> wrote: > Author: tijl > Date: Mon Oct 22 20:55:35 2018 > New Revision: 339618 > URL: https://svnweb.freebsd.org/changeset/base/339618 > > Log: > Define linuxkpi readq for 64-bit architectures. It is used by drm-kmod. > Currently the compiler picks up the definition in machine/cpufunc.h. > > Add compiler memory barriers to read* and write*. The Linux x86 > implementation of these functions uses inline asm with "memory" clobber. > The Linux x86 implementation of read_relaxed* and write_relaxed* uses the > same inline asm without "memory" clobber. > > Implement ioread* and iowrite* in terms of read* and write* so they also > have memory barriers. > > Qualify the addr parameter in write* as volatile. > > Like Linux, define macros with the same name as the inline functions. > > Only define 64-bit versions on 64-bit architectures because generally > 32-bit architectures can't do atomic 64-bit loads and stores. > > Regroup the functions a bit and add brief comments explaining what they > do: > - __raw_read*, __raw_write*: atomic, no barriers, no byte swapping > - read_relaxed*, write_relaxed*: atomic, no barriers, little-endian > - read*, write*: atomic, with barriers, little-endian > > Add a comment that says our implementation of ioread* and iowrite* > only handles MMIO and does not support port IO. > > Reviewed by: hselasky > MFC after: 3 days > > Modified: > head/sys/compat/linuxkpi/common/include/linux/io.h > > Modified: head/sys/compat/linuxkpi/common/include/linux/io.h > > ============================================================================== > --- head/sys/compat/linuxkpi/common/include/linux/io.h Mon Oct 22 > 20:22:33 2018 (r339617) > +++ head/sys/compat/linuxkpi/common/include/linux/io.h Mon Oct 22 > 20:55:35 2018 (r339618) > @@ -38,153 +38,309 @@ > #include <linux/compiler.h> > #include <linux/types.h> > > +/* > + * XXX This is all x86 specific. It should be bus space access. > + */ > + > +/* Access MMIO registers atomically without barriers and byte swapping. */ > + > +static inline uint8_t > +__raw_readb(const volatile void *addr) > +{ > + return (*(const volatile uint8_t *)addr); > +} > +#define __raw_readb(addr) __raw_readb(addr) > + > +static inline void > +__raw_writeb(uint8_t v, volatile void *addr) > +{ > + *(volatile uint8_t *)addr = v; > +} > +#define __raw_writeb(v, addr) __raw_writeb(v, addr) > + > +static inline uint16_t > +__raw_readw(const volatile void *addr) > +{ > + return (*(const volatile uint16_t *)addr); > +} > +#define __raw_readw(addr) __raw_readw(addr) > + > +static inline void > +__raw_writew(uint16_t v, volatile void *addr) > +{ > + *(volatile uint16_t *)addr = v; > +} > +#define __raw_writew(v, addr) __raw_writew(v, addr) > + > static inline uint32_t > __raw_readl(const volatile void *addr) > { > - return *(const volatile uint32_t *)addr; > + return (*(const volatile uint32_t *)addr); > } > +#define __raw_readl(addr) __raw_readl(addr) > > static inline void > -__raw_writel(uint32_t b, volatile void *addr) > +__raw_writel(uint32_t v, volatile void *addr) > { > - *(volatile uint32_t *)addr = b; > + *(volatile uint32_t *)addr = v; > } > +#define __raw_writel(v, addr) __raw_writel(v, addr) > > +#ifdef __LP64__ > static inline uint64_t > __raw_readq(const volatile void *addr) > { > - return *(const volatile uint64_t *)addr; > + return (*(const volatile uint64_t *)addr); > } > +#define __raw_readq(addr) __raw_readq(addr) > > static inline void > -__raw_writeq(uint64_t b, volatile void *addr) > +__raw_writeq(uint64_t v, volatile void *addr) > { > - *(volatile uint64_t *)addr = b; > + *(volatile uint64_t *)addr = v; > } > +#define __raw_writeq(v, addr) __raw_writeq(v, addr) > +#endif > > -/* > - * XXX This is all x86 specific. It should be bus space access. > - */ > #define mmiowb() barrier() > > -#undef writel > +/* Access little-endian MMIO registers atomically with memory barriers. */ > + > +#undef readb > +static inline uint8_t > +readb(const volatile void *addr) > +{ > + uint8_t v; > + > + __compiler_membar(); > + v = *(const volatile uint8_t *)addr; > + __compiler_membar(); > + return (v); > +} > +#define readb(addr) readb(addr) > + > +#undef writeb > static inline void > -writel(uint32_t b, void *addr) > +writeb(uint8_t v, volatile void *addr) > { > - *(volatile uint32_t *)addr = b; > + __compiler_membar(); > + *(volatile uint8_t *)addr = v; > + __compiler_membar(); > } > +#define writeb(v, addr) writeb(v, addr) > > -#undef writel_relaxed > +#undef readw > +static inline uint16_t > +readw(const volatile void *addr) > +{ > + uint16_t v; > + > + __compiler_membar(); > + v = *(const volatile uint16_t *)addr; > + __compiler_membar(); > + return (v); > +} > +#define readw(addr) readw(addr) > + > +#undef writew > static inline void > -writel_relaxed(uint32_t b, void *addr) > +writew(uint16_t v, volatile void *addr) > { > - *(volatile uint32_t *)addr = b; > + __compiler_membar(); > + *(volatile uint16_t *)addr = v; > + __compiler_membar(); > } > +#define writew(v, addr) writew(v, addr) > > +#undef readl > +static inline uint32_t > +readl(const volatile void *addr) > +{ > + uint32_t v; > + > + __compiler_membar(); > + v = *(const volatile uint32_t *)addr; > + __compiler_membar(); > + return (v); > +} > +#define readl(addr) readl(addr) > + > +#undef writel > +static inline void > +writel(uint32_t v, volatile void *addr) > +{ > + __compiler_membar(); > + *(volatile uint32_t *)addr = v; > + __compiler_membar(); > +} > +#define writel(v, addr) writel(v, addr) > + > +#undef readq > #undef writeq > +#ifdef __LP64__ > +static inline uint64_t > +readq(const volatile void *addr) > +{ > + uint64_t v; > + > + __compiler_membar(); > + v = *(const volatile uint64_t *)addr; > + __compiler_membar(); > + return (v); > +} > +#define readq(addr) readq(addr) > + > static inline void > -writeq(uint64_t b, void *addr) > +writeq(uint64_t v, volatile void *addr) > { > - *(volatile uint64_t *)addr = b; > + __compiler_membar(); > + *(volatile uint64_t *)addr = v; > + __compiler_membar(); > } > +#define writeq(v, addr) writeq(v, addr) > +#endif > > -#undef writeb > +/* Access little-endian MMIO registers atomically without memory > barriers. */ > + > +#undef readb_relaxed > +static inline uint8_t > +readb_relaxed(const volatile void *addr) > +{ > + return (*(const volatile uint8_t *)addr); > +} > +#define readb_relaxed(addr) readb_relaxed(addr) > + > +#undef writeb_relaxed > static inline void > -writeb(uint8_t b, void *addr) > +writeb_relaxed(uint8_t v, volatile void *addr) > { > - *(volatile uint8_t *)addr = b; > + *(volatile uint8_t *)addr = v; > } > +#define writeb_relaxed(v, addr) writeb_relaxed(v, addr) > > -#undef writew > +#undef readw_relaxed > +static inline uint16_t > +readw_relaxed(const volatile void *addr) > +{ > + return (*(const volatile uint16_t *)addr); > +} > +#define readw_relaxed(addr) readw_relaxed(addr) > + > +#undef writew_relaxed > static inline void > -writew(uint16_t b, void *addr) > +writew_relaxed(uint16_t v, volatile void *addr) > { > - *(volatile uint16_t *)addr = b; > + *(volatile uint16_t *)addr = v; > } > +#define writew_relaxed(v, addr) writew_relaxed(v, addr) > > +#undef readl_relaxed > +static inline uint32_t > +readl_relaxed(const volatile void *addr) > +{ > + return (*(const volatile uint32_t *)addr); > +} > +#define readl_relaxed(addr) readl_relaxed(addr) > + > +#undef writel_relaxed > +static inline void > +writel_relaxed(uint32_t v, volatile void *addr) > +{ > + *(volatile uint32_t *)addr = v; > +} > +#define writel_relaxed(v, addr) writel_relaxed(v, addr) > + > +#undef readq_relaxed > +#undef writeq_relaxed > +#ifdef __LP64__ > +static inline uint64_t > +readq_relaxed(const volatile void *addr) > +{ > + return (*(const volatile uint64_t *)addr); > +} > +#define readq_relaxed(addr) readq_relaxed(addr) > + > +static inline void > +writeq_relaxed(uint64_t v, volatile void *addr) > +{ > + *(volatile uint64_t *)addr = v; > +} > +#define writeq_relaxed(v, addr) writeq_relaxed(v, addr) > +#endif > + > +/* XXX On Linux ioread and iowrite handle both MMIO and port IO. */ > + > #undef ioread8 > static inline uint8_t > ioread8(const volatile void *addr) > { > - return *(const volatile uint8_t *)addr; > + return (readb(addr)); > } > +#define ioread8(addr) ioread8(addr) > > #undef ioread16 > static inline uint16_t > ioread16(const volatile void *addr) > { > - return *(const volatile uint16_t *)addr; > + return (readw(addr)); > } > +#define ioread16(addr) ioread16(addr) > > #undef ioread16be > static inline uint16_t > ioread16be(const volatile void *addr) > { > - return be16toh(*(const volatile uint16_t *)addr); > + return (bswap16(readw(addr))); > } > +#define ioread16be(addr) ioread16be(addr) > > #undef ioread32 > static inline uint32_t > ioread32(const volatile void *addr) > { > - return *(const volatile uint32_t *)addr; > + return (readl(addr)); > } > +#define ioread32(addr) ioread32(addr) > > #undef ioread32be > static inline uint32_t > ioread32be(const volatile void *addr) > { > - return be32toh(*(const volatile uint32_t *)addr); > + return (bswap32(readl(addr))); > } > +#define ioread32be(addr) ioread32be(addr) > > #undef iowrite8 > static inline void > iowrite8(uint8_t v, volatile void *addr) > { > - *(volatile uint8_t *)addr = v; > + writeb(v, addr); > } > +#define iowrite8(v, addr) iowrite8(v, addr) > > #undef iowrite16 > static inline void > iowrite16(uint16_t v, volatile void *addr) > { > - *(volatile uint16_t *)addr = v; > + writew(v, addr); > } > +#define iowrite16 iowrite16 > > #undef iowrite32 > static inline void > iowrite32(uint32_t v, volatile void *addr) > { > - *(volatile uint32_t *)addr = v; > + writel(v, addr); > } > +#define iowrite32(v, addr) iowrite32(v, addr) > > #undef iowrite32be > static inline void > iowrite32be(uint32_t v, volatile void *addr) > { > - *(volatile uint32_t *)addr = htobe32(v); > + writel(bswap32(v), addr); > } > - > -#undef readb > -static inline uint8_t > -readb(const volatile void *addr) > -{ > - return *(const volatile uint8_t *)addr; > -} > - > -#undef readw > -static inline uint16_t > -readw(const volatile void *addr) > -{ > - return *(const volatile uint16_t *)addr; > -} > - > -#undef readl > -static inline uint32_t > -readl(const volatile void *addr) > -{ > - return *(const volatile uint32_t *)addr; > -} > +#define iowrite32be(v, addr) iowrite32be(v, addr) > > #if defined(__i386__) || defined(__amd64__) > static inline void > > _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
