alpha: io: reorder barriers to guarantee writeX() and iowriteX() ordering #2

PATCH In review Sinan Kaya 2 participants 5 messages Apr 16, 2018 – May 9, 2018

Subsystems (2)

Arm Qualcomm mailing list Alpha port

lore

Share & tools

lore

Sinan Kaya

author

2018-04-16 22:16 UTC #0 lore raw

PATCH 1/1 alpha: io: reorder barriers to guarantee writeX() and iowriteX() ordering #2

memory-barriers.txt has been updated with the following requirement.

"When using writel(), a prior wmb() is not needed to guarantee that the cache coherent memory writes have completed before writing to the MMIO region."

Current writeX() and iowriteX() implementations on alpha are not satisfying this requirement as the barrier is after the register write.

Move mb() in writeX() and iowriteX() functions to guarantee that HW observes memory changes before performing register operations.

Signed-off-by Sinan Kaya <[email protected]>

Reported-by Arnd Bergmann <[email protected]>

---
 arch/alpha/kernel/io.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

arch/alpha/kernel/io.c +7 -7

index 3e3d49c..c025a3e 100644
@@ -37,20 +37,20 @@ unsigned int ioread32(void __iomem *addr)
 
 void iowrite8(u8 b, void __iomem *addr)
 {
-	IO_CONCAT(__IO_PREFIX,iowrite8)(b, addr);
 	mb();
+	IO_CONCAT(__IO_PREFIX,iowrite8)(b, addr);
 }
 
 void iowrite16(u16 b, void __iomem *addr)
 {
-	IO_CONCAT(__IO_PREFIX,iowrite16)(b, addr);
 	mb();
+	IO_CONCAT(__IO_PREFIX,iowrite16)(b, addr);
 }
 
 void iowrite32(u32 b, void __iomem *addr)
 {
-	IO_CONCAT(__IO_PREFIX,iowrite32)(b, addr);
 	mb();
+	IO_CONCAT(__IO_PREFIX,iowrite32)(b, addr);
 }
 
 EXPORT_SYMBOL(ioread8);

@@ -176,26 +176,26 @@ u64 readq(const volatile void __iomem *addr)
 
 void writeb(u8 b, volatile void __iomem *addr)
 {
-	__raw_writeb(b, addr);
 	mb();
+	__raw_writeb(b, addr);
 }
 
 void writew(u16 b, volatile void __iomem *addr)
 {
-	__raw_writew(b, addr);
 	mb();
+	__raw_writew(b, addr);
 }
 
 void writel(u32 b, volatile void __iomem *addr)
 {
-	__raw_writel(b, addr);
 	mb();
+	__raw_writel(b, addr);
 }
 
 void writeq(u64 b, volatile void __iomem *addr)
 {
-	__raw_writeq(b, addr);
 	mb();
+	__raw_writeq(b, addr);
 }
 
 EXPORT_SYMBOL(readb);

Signature · 1 line

2.7.4

Sinan Kaya

author

2018-04-17 18:43 UTC #1 lore raw

Quoted 9 lines · Sinan Kaya

memory-barriers.txt has been updated with the following requirement.
"When using writel(), a prior wmb() is not needed to guarantee that the cache coherent memory writes have completed before writing to the MMIO region."
Current writeX() and iowriteX() implementations on alpha are not satisfying this requirement as the barrier is after the register write.

Quoted 8 lines

Move mb() in writeX() and iowriteX() functions to guarantee that HW observes memory changes before performing register operations.
Signed-off-by Sinan Kaya <[email protected]>
Reported-by Arnd Bergmann <[email protected]>
---
 arch/alpha/kernel/io.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Sorry for catching this late but this also needs to go to 4.17 after review.

I missed the writel() implementation on arch/alpha/kernel/io.c file on my first patch.

Signature · 3 lines

Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Sinan Kaya

author

2018-04-20 16:20 UTC #2 lore raw

Hi Matt,

Quoted 19 lines · Sinan Kaya

Quoted 9 lines · Sinan Kaya
memory-barriers.txt has been updated with the following requirement.
"When using writel(), a prior wmb() is not needed to guarantee that the cache coherent memory writes have completed before writing to the MMIO region."
Current writeX() and iowriteX() implementations on alpha are not satisfying this requirement as the barrier is after the register write.
Quoted 8 lines
Move mb() in writeX() and iowriteX() functions to guarantee that HW observes memory changes before performing register operations.
Signed-off-by Sinan Kaya <[email protected]>
Reported-by Arnd Bergmann <[email protected]>
---
 arch/alpha/kernel/io.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Quoted 6 lines

Sorry for catching this late but this also needs to go to 4.17 after review.
I missed the writel() implementation on arch/alpha/kernel/io.c file on my first patch.

Can you also queue this for 4.17?

There are already drivers checked into 4.17 that dropped the unnecessary barriers.

I really hate to see Alpha broken because of this.

Sinan

Signature · 3 lines

Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Sinan Kaya

author

2018-05-01 22:48 UTC #3 lore raw

Quoted 29 lines · Sinan Kaya

Hi Matt,
Quoted 19 lines · Sinan Kaya
Quoted 9 lines · Sinan Kaya
memory-barriers.txt has been updated with the following requirement.
"When using writel(), a prior wmb() is not needed to guarantee that the cache coherent memory writes have completed before writing to the MMIO region."
Current writeX() and iowriteX() implementations on alpha are not satisfying this requirement as the barrier is after the register write.
Quoted 8 lines
Move mb() in writeX() and iowriteX() functions to guarantee that HW observes memory changes before performing register operations.
Signed-off-by Sinan Kaya <[email protected]>
Reported-by Arnd Bergmann <[email protected]>
---
 arch/alpha/kernel/io.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
Quoted 6 lines
Sorry for catching this late but this also needs to go to 4.17 after review.
I missed the writel() implementation on arch/alpha/kernel/io.c file on my first patch.

Quoted 6 lines

Can you also queue this for 4.17?
There are already drivers checked into 4.17 that dropped the unnecessary barriers.
I really hate to see Alpha broken because of this.

ping.

Quoted 3 lines

Sinan

Signature · 3 lines

Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Matt Turner

maintainer

2018-05-09 02:57 UTC #4 lore raw

In reply to Sinan Kaya

Quoted 29 lines · Sinan Kaya

Hi Matt,
Quoted 19 lines · Sinan Kaya
Quoted 9 lines · Sinan Kaya
memory-barriers.txt has been updated with the following requirement.
"When using writel(), a prior wmb() is not needed to guarantee that the cache coherent memory writes have completed before writing to the MMIO region."
Current writeX() and iowriteX() implementations on alpha are not satisfying this requirement as the barrier is after the register write.
Quoted 8 lines
Move mb() in writeX() and iowriteX() functions to guarantee that HW observes memory changes before performing register operations.
Signed-off-by Sinan Kaya <[email protected]>
Reported-by Arnd Bergmann <[email protected]>
---
 arch/alpha/kernel/io.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
Quoted 6 lines
Sorry for catching this late but this also needs to go to 4.17 after review.
I missed the writel() implementation on arch/alpha/kernel/io.c file on my first patch.

Quoted 6 lines

Can you also queue this for 4.17?
There are already drivers checked into 4.17 that dropped the unnecessary barriers.
I really hate to see Alpha broken because of this.

Yes, I will pick it up for 4.17.

Thanks for the patch.