When looking at powerpc io.h raw and relaxed are not aliases, but it appears that on x86, they are: https://github.com/torvalds/linux/blob/master/arch/x86/include/asm/io.h
Sorry for the noise. But let's starting moving the x86 specific atomic.h and io.h under asm/x86. Thanks. On Sat, Nov 17, 2018 at 2:01 PM Matthew Macy <mat.m...@gmail.com> wrote: > > 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-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"