diff mbox

io: prevent compiler reordering on the default writeX() implementation

Message ID 1522420199-23548-1-git-send-email-okaya@codeaurora.org (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Sinan Kaya March 30, 2018, 2:29 p.m. UTC
The default implementation of mapping writeX() to __raw_writeX() is wrong.
writeX() has stronger ordering semantics. Compiler is allowed to reorder
__raw_writeX().

In the abscence of a write barrier or when using a strongly ordered
architecture, writeX() should at least have a compiler barrier in
it to prevent commpiler from clobbering the execution order.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 include/asm-generic/io.h | 3 +++
 1 file changed, 3 insertions(+)

Comments

Sinan Kaya March 30, 2018, 2:46 p.m. UTC | #1
On 3/30/2018 10:29 AM, Sinan Kaya wrote:
> In the abscence of a write barrier or when using a strongly ordered
> architecture, writeX() should at least have a compiler barrier in
> it to prevent commpiler from clobbering the execution order.

Same is true for readX(). I'll wait for review feedback on this before
posting another change for readX().
Russell King (Oracle) March 30, 2018, 3:16 p.m. UTC | #2
On Fri, Mar 30, 2018 at 10:29:58AM -0400, Sinan Kaya wrote:
> The default implementation of mapping writeX() to __raw_writeX() is wrong.
> writeX() has stronger ordering semantics. Compiler is allowed to reorder
> __raw_writeX().
> 
> In the abscence of a write barrier or when using a strongly ordered
> architecture, writeX() should at least have a compiler barrier in
> it to prevent commpiler from clobbering the execution order.

You want the barrier _before_ the call to __raw_writel() - you need to
ensure that writes to memory are emitted by the compiler _before_ the
write to the hardware - the write to the hardware may start DMA, and it
may be reading data that the program thinks it previously wrote.

Similarly, for readl(), you need the barrier after __raw_readl() to
ensure that other reads in the program aren't scheduled before a
potential DMA status register read.
diff mbox

Patch

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index b4531e3..fbbf2bb 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -153,6 +153,7 @@  static inline void writeb(u8 value, volatile void __iomem *addr)
 static inline void writew(u16 value, volatile void __iomem *addr)
 {
 	__raw_writew(cpu_to_le16(value), addr);
+	barrier();
 }
 #endif
 
@@ -161,6 +162,7 @@  static inline void writew(u16 value, volatile void __iomem *addr)
 static inline void writel(u32 value, volatile void __iomem *addr)
 {
 	__raw_writel(__cpu_to_le32(value), addr);
+	barrier();
 }
 #endif
 
@@ -170,6 +172,7 @@  static inline void writel(u32 value, volatile void __iomem *addr)
 static inline void writeq(u64 value, volatile void __iomem *addr)
 {
 	__raw_writeq(__cpu_to_le64(value), addr);
+	barrier();
 }
 #endif
 #endif /* CONFIG_64BIT */