Date: Sat, 17 Nov 2018 14:55:05 -0800 From: Matthew Macy <mat.macy@gmail.com> To: Tijl Coosemans <tijl@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r339618 - head/sys/compat/linuxkpi/common/include/linux Message-ID: <CAPrugNq3HXvPccFzRxFK79o2fK3KzCzpn7BzEim9MyXxGnJ%2Bpw@mail.gmail.com> In-Reply-To: <CAPrugNoMVDyg-CnVh5NUgrxdaHcbo3CibC3RsPQ7FVtdJ=FJdQ@mail.gmail.com> References: <201810222055.w9MKtZPt013627@repo.freebsd.org> <CAPrugNoMVDyg-CnVh5NUgrxdaHcbo3CibC3RsPQ7FVtdJ=FJdQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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.macy@gmail.com> wrote: > > You should probably revert this. The implied understanding of the _relaxe= d version is incorrect. compiler_membar is there to prevent instruction reo= rdering which is possible on FreeBSD because the accesses are done in C. Th= e relaxed variants still do not permit instruction reordering. On Linux __c= ompiler_member (referred to as barrier there) isn=E2=80=99t necessary in th= e mmio accessors because they always use volatile asm which can=E2=80=99t b= e reordered. The distinction between the relaxed and non relaxed variants i= s that the relaxed variant lacks memory barriers (sync / lwsync / eieio on = ppc, membar on sparc, etc). Most of the time we don=E2=80=99t run in to pro= blems on x86 because with TSO the only reordering possible is writes that h= appen before reads can become visible in memory after they occur in the ins= truction stream. > > Thanks. > -M > > On Mon, Oct 22, 2018 at 13:55 Tijl Coosemans <tijl@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-kmo= d. >> 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" clobbe= r. >> 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 al= so >> 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 the= y 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 >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D >> --- 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 =3D 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 =3D 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 =3D b; >> + *(volatile uint32_t *)addr =3D 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 =3D b; >> + *(volatile uint64_t *)addr =3D 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 =3D *(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 =3D b; >> + __compiler_membar(); >> + *(volatile uint8_t *)addr =3D 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 =3D *(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 =3D b; >> + __compiler_membar(); >> + *(volatile uint16_t *)addr =3D 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 =3D *(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 =3D 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 =3D *(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 =3D b; >> + __compiler_membar(); >> + *(volatile uint64_t *)addr =3D v; >> + __compiler_membar(); >> } >> +#define writeq(v, addr) writeq(v, addr) >> +#endif >> >> -#undef writeb >> +/* Access little-endian MMIO registers atomically without memory barrie= rs. */ >> + >> +#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 =3D b; >> + *(volatile uint8_t *)addr =3D 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 =3D b; >> + *(volatile uint16_t *)addr =3D 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 =3D 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 =3D 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 =3D 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 =3D v; >> + writew(v, addr); >> } >> +#define iowrite16 iowrite16 >> >> #undef iowrite32 >> static inline void >> iowrite32(uint32_t v, volatile void *addr) >> { >> - *(volatile uint32_t *)addr =3D 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 =3D 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 >>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPrugNq3HXvPccFzRxFK79o2fK3KzCzpn7BzEim9MyXxGnJ%2Bpw>