* [PATCH 1/7] asm-generic/io.h: add io{read,write}64 accessors
@ 2016-05-04 17:16 Horia Geantă
2016-05-04 17:16 ` Horia Geantă
2016-05-04 22:35 ` Arnd Bergmann
0 siblings, 2 replies; 9+ messages in thread
From: Horia Geantă @ 2016-05-04 17:16 UTC (permalink / raw)
To: Herbert Xu, Arnd Bergmann
Cc: linux-crypto, linux-arch, linux-kernel, David S. Miller,
Scott Wood, Alexandru Porosanu, Tudor Ambarus, Cristian Stoica
This will allow device drivers to consistently use io{read,write}XX
also for 64-bit accesses.
Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
---
include/asm-generic/io.h | 63 +++++++++++++++++++++++++++++++++++++++++++++
include/asm-generic/iomap.h | 8 ++++++
2 files changed, 71 insertions(+)
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index eed3bbe88c8a..0e2c73bb6d56 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -585,6 +585,16 @@ static inline u32 ioread32(const volatile void __iomem *addr)
}
#endif
+#ifdef CONFIG_64BIT
+#ifndef ioread64
+#define ioread64 ioread64
+static inline u64 ioread64(const volatile void __iomem *addr)
+{
+ return readq(addr);
+}
+#endif
+#endif /* CONFIG_64BIT */
+
#ifndef iowrite8
#define iowrite8 iowrite8
static inline void iowrite8(u8 value, volatile void __iomem *addr)
@@ -609,6 +619,16 @@ static inline void iowrite32(u32 value, volatile void __iomem *addr)
}
#endif
+#ifdef CONFIG_64BIT
+#ifndef iowrite64
+#define iowrite64 iowrite64
+static inline void iowrite64(u64 value, volatile void __iomem *addr)
+{
+ writeq(value, addr);
+}
+#endif
+#endif /* CONFIG_64BIT */
+
#ifndef ioread16be
#define ioread16be ioread16be
static inline u16 ioread16be(const volatile void __iomem *addr)
@@ -625,6 +645,16 @@ static inline u32 ioread32be(const volatile void __iomem *addr)
}
#endif
+#ifdef CONFIG_64BIT
+#ifndef ioread64be
+#define ioread64be ioread64be
+static inline u64 ioread64be(const volatile void __iomem *addr)
+{
+ return __be64_to_cpu(__raw_readq(addr));
+}
+#endif
+#endif /* CONFIG_64BIT */
+
#ifndef iowrite16be
#define iowrite16be iowrite16be
static inline void iowrite16be(u16 value, void volatile __iomem *addr)
@@ -641,6 +671,16 @@ static inline void iowrite32be(u32 value, volatile void __iomem *addr)
}
#endif
+#ifdef CONFIG_64BIT
+#ifndef iowrite64be
+#define iowrite64be iowrite64be
+static inline void iowrite64be(u64 value, volatile void __iomem *addr)
+{
+ __raw_writeq(__cpu_to_be64(value), addr);
+}
+#endif
+#endif /* CONFIG_64BIT */
+
#ifndef ioread8_rep
#define ioread8_rep ioread8_rep
static inline void ioread8_rep(const volatile void __iomem *addr, void *buffer,
@@ -668,6 +708,17 @@ static inline void ioread32_rep(const volatile void __iomem *addr,
}
#endif
+#ifdef CONFIG_64BIT
+#ifndef ioread64_rep
+#define ioread64_rep ioread64_rep
+static inline void ioread64_rep(const volatile void __iomem *addr,
+ void *buffer, unsigned int count)
+{
+ readsq(addr, buffer, count);
+}
+#endif
+#endif /* CONFIG_64BIT */
+
#ifndef iowrite8_rep
#define iowrite8_rep iowrite8_rep
static inline void iowrite8_rep(volatile void __iomem *addr,
@@ -697,6 +748,18 @@ static inline void iowrite32_rep(volatile void __iomem *addr,
writesl(addr, buffer, count);
}
#endif
+
+#ifdef CONFIG_64BIT
+#ifndef iowrite64_rep
+#define iowrite64_rep iowrite64_rep
+static inline void iowrite64_rep(volatile void __iomem *addr,
+ const void *buffer,
+ unsigned int count)
+{
+ writesq(addr, buffer, count);
+}
+#endif
+#endif /* CONFIG_64BIT */
#endif /* CONFIG_GENERIC_IOMAP */
#ifdef __KERNEL__
diff --git a/include/asm-generic/iomap.h b/include/asm-generic/iomap.h
index d8f8622fa044..650fede33c25 100644
--- a/include/asm-generic/iomap.h
+++ b/include/asm-generic/iomap.h
@@ -30,12 +30,20 @@ extern unsigned int ioread16(void __iomem *);
extern unsigned int ioread16be(void __iomem *);
extern unsigned int ioread32(void __iomem *);
extern unsigned int ioread32be(void __iomem *);
+#ifdef CONFIG_64BIT
+extern u64 ioread64(void __iomem *);
+extern u64 ioread64be(void __iomem *);
+#endif
extern void iowrite8(u8, void __iomem *);
extern void iowrite16(u16, void __iomem *);
extern void iowrite16be(u16, void __iomem *);
extern void iowrite32(u32, void __iomem *);
extern void iowrite32be(u32, void __iomem *);
+#ifdef CONFIG_64BIT
+extern void iowrite64(u64, void __iomem *);
+extern void iowrite64be(u64, void __iomem *);
+#endif
/*
* "string" versions of the above. Note that they
--
2.4.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 1/7] asm-generic/io.h: add io{read,write}64 accessors
2016-05-04 17:16 [PATCH 1/7] asm-generic/io.h: add io{read,write}64 accessors Horia Geantă
@ 2016-05-04 17:16 ` Horia Geantă
2016-05-04 22:35 ` Arnd Bergmann
1 sibling, 0 replies; 9+ messages in thread
From: Horia Geantă @ 2016-05-04 17:16 UTC (permalink / raw)
To: Herbert Xu, Arnd Bergmann
Cc: linux-crypto, linux-arch, linux-kernel, David S. Miller,
Scott Wood, Alexandru Porosanu, Tudor Ambarus, Cristian Stoica
This will allow device drivers to consistently use io{read,write}XX
also for 64-bit accesses.
Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
---
include/asm-generic/io.h | 63 +++++++++++++++++++++++++++++++++++++++++++++
include/asm-generic/iomap.h | 8 ++++++
2 files changed, 71 insertions(+)
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index eed3bbe88c8a..0e2c73bb6d56 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -585,6 +585,16 @@ static inline u32 ioread32(const volatile void __iomem *addr)
}
#endif
+#ifdef CONFIG_64BIT
+#ifndef ioread64
+#define ioread64 ioread64
+static inline u64 ioread64(const volatile void __iomem *addr)
+{
+ return readq(addr);
+}
+#endif
+#endif /* CONFIG_64BIT */
+
#ifndef iowrite8
#define iowrite8 iowrite8
static inline void iowrite8(u8 value, volatile void __iomem *addr)
@@ -609,6 +619,16 @@ static inline void iowrite32(u32 value, volatile void __iomem *addr)
}
#endif
+#ifdef CONFIG_64BIT
+#ifndef iowrite64
+#define iowrite64 iowrite64
+static inline void iowrite64(u64 value, volatile void __iomem *addr)
+{
+ writeq(value, addr);
+}
+#endif
+#endif /* CONFIG_64BIT */
+
#ifndef ioread16be
#define ioread16be ioread16be
static inline u16 ioread16be(const volatile void __iomem *addr)
@@ -625,6 +645,16 @@ static inline u32 ioread32be(const volatile void __iomem *addr)
}
#endif
+#ifdef CONFIG_64BIT
+#ifndef ioread64be
+#define ioread64be ioread64be
+static inline u64 ioread64be(const volatile void __iomem *addr)
+{
+ return __be64_to_cpu(__raw_readq(addr));
+}
+#endif
+#endif /* CONFIG_64BIT */
+
#ifndef iowrite16be
#define iowrite16be iowrite16be
static inline void iowrite16be(u16 value, void volatile __iomem *addr)
@@ -641,6 +671,16 @@ static inline void iowrite32be(u32 value, volatile void __iomem *addr)
}
#endif
+#ifdef CONFIG_64BIT
+#ifndef iowrite64be
+#define iowrite64be iowrite64be
+static inline void iowrite64be(u64 value, volatile void __iomem *addr)
+{
+ __raw_writeq(__cpu_to_be64(value), addr);
+}
+#endif
+#endif /* CONFIG_64BIT */
+
#ifndef ioread8_rep
#define ioread8_rep ioread8_rep
static inline void ioread8_rep(const volatile void __iomem *addr, void *buffer,
@@ -668,6 +708,17 @@ static inline void ioread32_rep(const volatile void __iomem *addr,
}
#endif
+#ifdef CONFIG_64BIT
+#ifndef ioread64_rep
+#define ioread64_rep ioread64_rep
+static inline void ioread64_rep(const volatile void __iomem *addr,
+ void *buffer, unsigned int count)
+{
+ readsq(addr, buffer, count);
+}
+#endif
+#endif /* CONFIG_64BIT */
+
#ifndef iowrite8_rep
#define iowrite8_rep iowrite8_rep
static inline void iowrite8_rep(volatile void __iomem *addr,
@@ -697,6 +748,18 @@ static inline void iowrite32_rep(volatile void __iomem *addr,
writesl(addr, buffer, count);
}
#endif
+
+#ifdef CONFIG_64BIT
+#ifndef iowrite64_rep
+#define iowrite64_rep iowrite64_rep
+static inline void iowrite64_rep(volatile void __iomem *addr,
+ const void *buffer,
+ unsigned int count)
+{
+ writesq(addr, buffer, count);
+}
+#endif
+#endif /* CONFIG_64BIT */
#endif /* CONFIG_GENERIC_IOMAP */
#ifdef __KERNEL__
diff --git a/include/asm-generic/iomap.h b/include/asm-generic/iomap.h
index d8f8622fa044..650fede33c25 100644
--- a/include/asm-generic/iomap.h
+++ b/include/asm-generic/iomap.h
@@ -30,12 +30,20 @@ extern unsigned int ioread16(void __iomem *);
extern unsigned int ioread16be(void __iomem *);
extern unsigned int ioread32(void __iomem *);
extern unsigned int ioread32be(void __iomem *);
+#ifdef CONFIG_64BIT
+extern u64 ioread64(void __iomem *);
+extern u64 ioread64be(void __iomem *);
+#endif
extern void iowrite8(u8, void __iomem *);
extern void iowrite16(u16, void __iomem *);
extern void iowrite16be(u16, void __iomem *);
extern void iowrite32(u32, void __iomem *);
extern void iowrite32be(u32, void __iomem *);
+#ifdef CONFIG_64BIT
+extern void iowrite64(u64, void __iomem *);
+extern void iowrite64be(u64, void __iomem *);
+#endif
/*
* "string" versions of the above. Note that they
--
2.4.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/7] asm-generic/io.h: add io{read,write}64 accessors
2016-05-04 17:16 [PATCH 1/7] asm-generic/io.h: add io{read,write}64 accessors Horia Geantă
2016-05-04 17:16 ` Horia Geantă
@ 2016-05-04 22:35 ` Arnd Bergmann
2016-05-04 22:35 ` Arnd Bergmann
2016-05-05 8:16 ` Vineet Gupta
1 sibling, 2 replies; 9+ messages in thread
From: Arnd Bergmann @ 2016-05-04 22:35 UTC (permalink / raw)
To: Horia Geantă
Cc: Herbert Xu, linux-crypto, linux-arch, linux-kernel,
David S. Miller, Scott Wood, Alexandru Porosanu, Tudor Ambarus,
Cristian Stoica, Vineet Gupta
On Wednesday 04 May 2016 20:16:19 Horia Geantă wrote:
> @@ -625,6 +645,16 @@ static inline u32 ioread32be(const volatile void __iomem *addr)
> }
> #endif
>
> +#ifdef CONFIG_64BIT
> +#ifndef ioread64be
> +#define ioread64be ioread64be
> +static inline u64 ioread64be(const volatile void __iomem *addr)
> +{
> + return __be64_to_cpu(__raw_readq(addr));
> +}
> +#endif
> +#endif /* CONFIG_64BIT */
> +
> #ifndef iowrite16be
> #define iowrite16be iowrite16be
> static inline void iowrite16be(u16 value, void volatile __iomem *addr)
> @@ -641,6 +671,16 @@ static inline void iowrite32be(u32 value, volatile void __iomem *addr)
> }
> #endif
>
> +#ifdef CONFIG_64BIT
> +#ifndef iowrite64be
> +#define iowrite64be iowrite64be
> +static inline void iowrite64be(u64 value, volatile void __iomem *addr)
> +{
> + __raw_writeq(__cpu_to_be64(value), addr);
> +}
> +#endif
> +#endif /* CONFIG_64BIT */
> +
>
I just noticed that these two are both a bit wrong, but they copy the
mistake that already exists in the 16 and 32 bit versions: If an
architecture overrides readq/writeq to have barriers but does not override
ioread64be/iowrite64be, this will lack the barriers and behave differently
from the little-endian version. I think the only affected architecture
is ARC, since ARM and ARM64 both override the big-endian accessors to
have the correct barriers, and all others don't use barriers at all.
Maybe you can add a patch before this one to replace the 16/32-bit accessors
with ones that do a
static inline void iowrite32be(u32 value, volatile void __iomem *addr)
{
writel(swab32(value), addr);
}
This will lead to a double-swap on architectures that don't override it,
but it will work correctly on all architectures without them having
to override the big-endian accessors.
Aside from that, the patch looks fine.
Arnd
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/7] asm-generic/io.h: add io{read,write}64 accessors
2016-05-04 22:35 ` Arnd Bergmann
@ 2016-05-04 22:35 ` Arnd Bergmann
2016-05-05 8:16 ` Vineet Gupta
1 sibling, 0 replies; 9+ messages in thread
From: Arnd Bergmann @ 2016-05-04 22:35 UTC (permalink / raw)
To: Horia Geantă
Cc: Herbert Xu, linux-crypto, linux-arch, linux-kernel,
David S. Miller, Scott Wood, Alexandru Porosanu, Tudor Ambarus,
Cristian Stoica, Vineet Gupta
On Wednesday 04 May 2016 20:16:19 Horia Geantă wrote:
> @@ -625,6 +645,16 @@ static inline u32 ioread32be(const volatile void __iomem *addr)
> }
> #endif
>
> +#ifdef CONFIG_64BIT
> +#ifndef ioread64be
> +#define ioread64be ioread64be
> +static inline u64 ioread64be(const volatile void __iomem *addr)
> +{
> + return __be64_to_cpu(__raw_readq(addr));
> +}
> +#endif
> +#endif /* CONFIG_64BIT */
> +
> #ifndef iowrite16be
> #define iowrite16be iowrite16be
> static inline void iowrite16be(u16 value, void volatile __iomem *addr)
> @@ -641,6 +671,16 @@ static inline void iowrite32be(u32 value, volatile void __iomem *addr)
> }
> #endif
>
> +#ifdef CONFIG_64BIT
> +#ifndef iowrite64be
> +#define iowrite64be iowrite64be
> +static inline void iowrite64be(u64 value, volatile void __iomem *addr)
> +{
> + __raw_writeq(__cpu_to_be64(value), addr);
> +}
> +#endif
> +#endif /* CONFIG_64BIT */
> +
>
I just noticed that these two are both a bit wrong, but they copy the
mistake that already exists in the 16 and 32 bit versions: If an
architecture overrides readq/writeq to have barriers but does not override
ioread64be/iowrite64be, this will lack the barriers and behave differently
from the little-endian version. I think the only affected architecture
is ARC, since ARM and ARM64 both override the big-endian accessors to
have the correct barriers, and all others don't use barriers at all.
Maybe you can add a patch before this one to replace the 16/32-bit accessors
with ones that do a
static inline void iowrite32be(u32 value, volatile void __iomem *addr)
{
writel(swab32(value), addr);
}
This will lead to a double-swap on architectures that don't override it,
but it will work correctly on all architectures without them having
to override the big-endian accessors.
Aside from that, the patch looks fine.
Arnd
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/7] asm-generic/io.h: add io{read,write}64 accessors
2016-05-04 22:35 ` Arnd Bergmann
2016-05-04 22:35 ` Arnd Bergmann
@ 2016-05-05 8:16 ` Vineet Gupta
2016-05-05 8:16 ` Vineet Gupta
2016-05-05 10:56 ` Arnd Bergmann
1 sibling, 2 replies; 9+ messages in thread
From: Vineet Gupta @ 2016-05-05 8:16 UTC (permalink / raw)
To: Arnd Bergmann, Horia Geantă
Cc: Herbert Xu, linux-crypto@vger.kernel.org,
linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
David S. Miller, Scott Wood, Alexandru Porosanu, Tudor Ambarus,
Cristian Stoica, arcml, Noam Camus
On Thursday 05 May 2016 04:06 AM, Arnd Bergmann wrote:
> On Wednesday 04 May 2016 20:16:19 Horia Geantă wrote:
>> @@ -625,6 +645,16 @@ static inline u32 ioread32be(const volatile void __iomem *addr)
>> }
>> #endif
>>
>> +#ifdef CONFIG_64BIT
>> +#ifndef ioread64be
>> +#define ioread64be ioread64be
>> +static inline u64 ioread64be(const volatile void __iomem *addr)
>> +{
>> + return __be64_to_cpu(__raw_readq(addr));
>> +}
>> +#endif
>> +#endif /* CONFIG_64BIT */
>> +
>> #ifndef iowrite16be
>> #define iowrite16be iowrite16be
>> static inline void iowrite16be(u16 value, void volatile __iomem *addr)
>> @@ -641,6 +671,16 @@ static inline void iowrite32be(u32 value, volatile void __iomem *addr)
>> }
>> #endif
>>
>> +#ifdef CONFIG_64BIT
>> +#ifndef iowrite64be
>> +#define iowrite64be iowrite64be
>> +static inline void iowrite64be(u64 value, volatile void __iomem *addr)
>> +{
>> + __raw_writeq(__cpu_to_be64(value), addr);
>> +}
>> +#endif
>> +#endif /* CONFIG_64BIT */
>> +
>>
> I just noticed that these two are both a bit wrong, but they copy the
> mistake that already exists in the 16 and 32 bit versions: If an
> architecture overrides readq/writeq to have barriers but does not override
> ioread64be/iowrite64be, this will lack the barriers and behave differently
> from the little-endian version. I think the only affected architecture
> is ARC, since ARM and ARM64 both override the big-endian accessors to
> have the correct barriers, and all others don't use barriers at all.
>
> Maybe you can add a patch before this one to replace the 16/32-bit accessors
> with ones that do a
>
> static inline void iowrite32be(u32 value, volatile void __iomem *addr)
> {
> writel(swab32(value), addr);
> }
>
> This will lead to a double-swap on architectures that don't override it,
> but it will work correctly on all architectures without them having
> to override the big-endian accessors.
Thx for noticing this Arnd and the heads up. Does the patch below look ok to you ?
----------->
rom b7e719831c389ab4fa338b2e2e7c0d1ff90dabb0 Mon Sep 17 00:00:00 2001
From: Vineet Gupta <vgupta@synopsys.com>
Date: Thu, 5 May 2016 13:32:34 +0530
Subject: [PATCH] ARC: Add missing io barriers to io{read,write}{16,32}be()
While reviewing a different change to asm-generic/io.h Arnd spotted that
ARC ioread32 and ioread32be both of which come from asm-generic versions
are not symmetrical in terms of calling the io barriers.
generic ioread32 -> ARC readl() [ has barriers]
generic ioread32be -> __be32_to_cpu(__raw_readl()) [ lacks barriers]
While generic ioread32be is being remediated to call readl(), that involves
a swab32(), causing double swaps on ioread32be() on Big Endian systems.
So provide our versions of big endian IO accessors to ensure io barrier
calls while also keeping them optimal
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Cc: stable@vger.kernel.org [4.2+]
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
arch/arc/include/asm/io.h | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
index 17f85c9c73cf..c22b181e8206 100644
--- a/arch/arc/include/asm/io.h
+++ b/arch/arc/include/asm/io.h
@@ -13,6 +13,15 @@
#include <asm/byteorder.h>
#include <asm/page.h>
+#ifdef CONFIG_ISA_ARCV2
+#include <asm/barrier.h>
+#define __iormb() rmb()
+#define __iowmb() wmb()
+#else
+#define __iormb() do { } while (0)
+#define __iowmb() do { } while (0)
+#endif
+
extern void __iomem *ioremap(phys_addr_t paddr, unsigned long size);
extern void __iomem *ioremap_prot(phys_addr_t paddr, unsigned long size,
unsigned long flags);
@@ -31,6 +40,15 @@ extern void iounmap(const void __iomem *addr);
#define ioremap_wc(phy, sz) ioremap(phy, sz)
#define ioremap_wt(phy, sz) ioremap(phy, sz)
+/*
+ * io{read,write}{16,32}be() macros
+ */
+#define ioread16be(p) ({ u16 __v = be16_to_cpu((__force
__be16)__raw_readw(p)); __iormb(); __v; })
+#define ioread32be(p) ({ u32 __v = be32_to_cpu((__force
__be32)__raw_readl(p)); __iormb(); __v; })
+
+#define iowrite16be(v,p) ({ __iowmb(); __raw_writew((__force
u16)cpu_to_be16(v), p); })
+#define iowrite32be(v,p) ({ __iowmb(); __raw_writel((__force
u32)cpu_to_be32(v), p); })
+
/* Change struct page to physical address */
#define page_to_phys(page) (page_to_pfn(page) << PAGE_SHIFT)
@@ -108,15 +126,6 @@ static inline void __raw_writel(u32 w, volatile void __iomem
*addr)
}
-#ifdef CONFIG_ISA_ARCV2
-#include <asm/barrier.h>
-#define __iormb() rmb()
-#define __iowmb() wmb()
-#else
-#define __iormb() do { } while (0)
-#define __iowmb() do { } while (0)
-#endif
-
/*
* MMIO can also get buffered/optimized in micro-arch, so barriers needed
* Based on ARM model for the typical use case
--
2.5.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/7] asm-generic/io.h: add io{read,write}64 accessors
2016-05-05 8:16 ` Vineet Gupta
@ 2016-05-05 8:16 ` Vineet Gupta
2016-05-05 10:56 ` Arnd Bergmann
1 sibling, 0 replies; 9+ messages in thread
From: Vineet Gupta @ 2016-05-05 8:16 UTC (permalink / raw)
To: Arnd Bergmann, Horia Geantă
Cc: Herbert Xu, linux-crypto@vger.kernel.org,
linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
David S. Miller, Scott Wood, Alexandru Porosanu, Tudor Ambarus,
Cristian Stoica, arcml, Noam Camus
On Thursday 05 May 2016 04:06 AM, Arnd Bergmann wrote:
> On Wednesday 04 May 2016 20:16:19 Horia Geantă wrote:
>> @@ -625,6 +645,16 @@ static inline u32 ioread32be(const volatile void __iomem *addr)
>> }
>> #endif
>>
>> +#ifdef CONFIG_64BIT
>> +#ifndef ioread64be
>> +#define ioread64be ioread64be
>> +static inline u64 ioread64be(const volatile void __iomem *addr)
>> +{
>> + return __be64_to_cpu(__raw_readq(addr));
>> +}
>> +#endif
>> +#endif /* CONFIG_64BIT */
>> +
>> #ifndef iowrite16be
>> #define iowrite16be iowrite16be
>> static inline void iowrite16be(u16 value, void volatile __iomem *addr)
>> @@ -641,6 +671,16 @@ static inline void iowrite32be(u32 value, volatile void __iomem *addr)
>> }
>> #endif
>>
>> +#ifdef CONFIG_64BIT
>> +#ifndef iowrite64be
>> +#define iowrite64be iowrite64be
>> +static inline void iowrite64be(u64 value, volatile void __iomem *addr)
>> +{
>> + __raw_writeq(__cpu_to_be64(value), addr);
>> +}
>> +#endif
>> +#endif /* CONFIG_64BIT */
>> +
>>
> I just noticed that these two are both a bit wrong, but they copy the
> mistake that already exists in the 16 and 32 bit versions: If an
> architecture overrides readq/writeq to have barriers but does not override
> ioread64be/iowrite64be, this will lack the barriers and behave differently
> from the little-endian version. I think the only affected architecture
> is ARC, since ARM and ARM64 both override the big-endian accessors to
> have the correct barriers, and all others don't use barriers at all.
>
> Maybe you can add a patch before this one to replace the 16/32-bit accessors
> with ones that do a
>
> static inline void iowrite32be(u32 value, volatile void __iomem *addr)
> {
> writel(swab32(value), addr);
> }
>
> This will lead to a double-swap on architectures that don't override it,
> but it will work correctly on all architectures without them having
> to override the big-endian accessors.
Thx for noticing this Arnd and the heads up. Does the patch below look ok to you ?
----------->
rom b7e719831c389ab4fa338b2e2e7c0d1ff90dabb0 Mon Sep 17 00:00:00 2001
From: Vineet Gupta <vgupta@synopsys.com>
Date: Thu, 5 May 2016 13:32:34 +0530
Subject: [PATCH] ARC: Add missing io barriers to io{read,write}{16,32}be()
While reviewing a different change to asm-generic/io.h Arnd spotted that
ARC ioread32 and ioread32be both of which come from asm-generic versions
are not symmetrical in terms of calling the io barriers.
generic ioread32 -> ARC readl() [ has barriers]
generic ioread32be -> __be32_to_cpu(__raw_readl()) [ lacks barriers]
While generic ioread32be is being remediated to call readl(), that involves
a swab32(), causing double swaps on ioread32be() on Big Endian systems.
So provide our versions of big endian IO accessors to ensure io barrier
calls while also keeping them optimal
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Cc: stable@vger.kernel.org [4.2+]
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
arch/arc/include/asm/io.h | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
index 17f85c9c73cf..c22b181e8206 100644
--- a/arch/arc/include/asm/io.h
+++ b/arch/arc/include/asm/io.h
@@ -13,6 +13,15 @@
#include <asm/byteorder.h>
#include <asm/page.h>
+#ifdef CONFIG_ISA_ARCV2
+#include <asm/barrier.h>
+#define __iormb() rmb()
+#define __iowmb() wmb()
+#else
+#define __iormb() do { } while (0)
+#define __iowmb() do { } while (0)
+#endif
+
extern void __iomem *ioremap(phys_addr_t paddr, unsigned long size);
extern void __iomem *ioremap_prot(phys_addr_t paddr, unsigned long size,
unsigned long flags);
@@ -31,6 +40,15 @@ extern void iounmap(const void __iomem *addr);
#define ioremap_wc(phy, sz) ioremap(phy, sz)
#define ioremap_wt(phy, sz) ioremap(phy, sz)
+/*
+ * io{read,write}{16,32}be() macros
+ */
+#define ioread16be(p) ({ u16 __v = be16_to_cpu((__force
__be16)__raw_readw(p)); __iormb(); __v; })
+#define ioread32be(p) ({ u32 __v = be32_to_cpu((__force
__be32)__raw_readl(p)); __iormb(); __v; })
+
+#define iowrite16be(v,p) ({ __iowmb(); __raw_writew((__force
u16)cpu_to_be16(v), p); })
+#define iowrite32be(v,p) ({ __iowmb(); __raw_writel((__force
u32)cpu_to_be32(v), p); })
+
/* Change struct page to physical address */
#define page_to_phys(page) (page_to_pfn(page) << PAGE_SHIFT)
@@ -108,15 +126,6 @@ static inline void __raw_writel(u32 w, volatile void __iomem
*addr)
}
-#ifdef CONFIG_ISA_ARCV2
-#include <asm/barrier.h>
-#define __iormb() rmb()
-#define __iowmb() wmb()
-#else
-#define __iormb() do { } while (0)
-#define __iowmb() do { } while (0)
-#endif
-
/*
* MMIO can also get buffered/optimized in micro-arch, so barriers needed
* Based on ARM model for the typical use case
--
2.5.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/7] asm-generic/io.h: add io{read,write}64 accessors
2016-05-05 8:16 ` Vineet Gupta
2016-05-05 8:16 ` Vineet Gupta
@ 2016-05-05 10:56 ` Arnd Bergmann
2016-05-05 10:56 ` Arnd Bergmann
2016-05-05 11:05 ` Vineet Gupta
1 sibling, 2 replies; 9+ messages in thread
From: Arnd Bergmann @ 2016-05-05 10:56 UTC (permalink / raw)
To: Vineet Gupta
Cc: Horia Geantă, Herbert Xu, linux-crypto@vger.kernel.org,
linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
David S. Miller, Scott Wood, Alexandru Porosanu, Tudor Ambarus,
Cristian Stoica, arcml, Noam Camus
On Thursday 05 May 2016 08:16:47 Vineet Gupta wrote:
> Thx for noticing this Arnd and the heads up. Does the patch below look ok to you ?
>
> ----------->
> rom b7e719831c389ab4fa338b2e2e7c0d1ff90dabb0 Mon Sep 17 00:00:00 2001
> From: Vineet Gupta <vgupta@synopsys.com>
> Date: Thu, 5 May 2016 13:32:34 +0530
> Subject: [PATCH] ARC: Add missing io barriers to io{read,write}{16,32}be()
>
> While reviewing a different change to asm-generic/io.h Arnd spotted that
> ARC ioread32 and ioread32be both of which come from asm-generic versions
> are not symmetrical in terms of calling the io barriers.
>
> generic ioread32 -> ARC readl() [ has barriers]
> generic ioread32be -> __be32_to_cpu(__raw_readl()) [ lacks barriers]
>
> While generic ioread32be is being remediated to call readl(), that involves
> a swab32(), causing double swaps on ioread32be() on Big Endian systems.
>
> So provide our versions of big endian IO accessors to ensure io barrier
> calls while also keeping them optimal
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Cc: stable@vger.kernel.org [4.2+]
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
Yes, that looks correct. We probably want this regardless of the change
I proposed for the generic file, to avoid the double swap.
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/7] asm-generic/io.h: add io{read,write}64 accessors
2016-05-05 10:56 ` Arnd Bergmann
@ 2016-05-05 10:56 ` Arnd Bergmann
2016-05-05 11:05 ` Vineet Gupta
1 sibling, 0 replies; 9+ messages in thread
From: Arnd Bergmann @ 2016-05-05 10:56 UTC (permalink / raw)
To: Vineet Gupta
Cc: Horia Geantă, Herbert Xu, linux-crypto@vger.kernel.org,
linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
David S. Miller, Scott Wood, Alexandru Porosanu, Tudor Ambarus,
Cristian Stoica, arcml, Noam Camus
On Thursday 05 May 2016 08:16:47 Vineet Gupta wrote:
> Thx for noticing this Arnd and the heads up. Does the patch below look ok to you ?
>
> ----------->
> rom b7e719831c389ab4fa338b2e2e7c0d1ff90dabb0 Mon Sep 17 00:00:00 2001
> From: Vineet Gupta <vgupta@synopsys.com>
> Date: Thu, 5 May 2016 13:32:34 +0530
> Subject: [PATCH] ARC: Add missing io barriers to io{read,write}{16,32}be()
>
> While reviewing a different change to asm-generic/io.h Arnd spotted that
> ARC ioread32 and ioread32be both of which come from asm-generic versions
> are not symmetrical in terms of calling the io barriers.
>
> generic ioread32 -> ARC readl() [ has barriers]
> generic ioread32be -> __be32_to_cpu(__raw_readl()) [ lacks barriers]
>
> While generic ioread32be is being remediated to call readl(), that involves
> a swab32(), causing double swaps on ioread32be() on Big Endian systems.
>
> So provide our versions of big endian IO accessors to ensure io barrier
> calls while also keeping them optimal
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Cc: stable@vger.kernel.org [4.2+]
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
Yes, that looks correct. We probably want this regardless of the change
I proposed for the generic file, to avoid the double swap.
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/7] asm-generic/io.h: add io{read,write}64 accessors
2016-05-05 10:56 ` Arnd Bergmann
2016-05-05 10:56 ` Arnd Bergmann
@ 2016-05-05 11:05 ` Vineet Gupta
1 sibling, 0 replies; 9+ messages in thread
From: Vineet Gupta @ 2016-05-05 11:05 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Horia Geantă, Herbert Xu, linux-crypto@vger.kernel.org,
linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
David S. Miller, Scott Wood, Alexandru Porosanu, Tudor Ambarus,
Cristian Stoica, arcml, Noam Camus
On Thursday 05 May 2016 04:26 PM, Arnd Bergmann wrote:
> On Thursday 05 May 2016 08:16:47 Vineet Gupta wrote:
>> > Thx for noticing this Arnd and the heads up. Does the patch below look ok to you ?
>> >
>> > ----------->
>> > rom b7e719831c389ab4fa338b2e2e7c0d1ff90dabb0 Mon Sep 17 00:00:00 2001
>> > From: Vineet Gupta <vgupta@synopsys.com>
>> > Date: Thu, 5 May 2016 13:32:34 +0530
>> > Subject: [PATCH] ARC: Add missing io barriers to io{read,write}{16,32}be()
>> >
>> > While reviewing a different change to asm-generic/io.h Arnd spotted that
>> > ARC ioread32 and ioread32be both of which come from asm-generic versions
>> > are not symmetrical in terms of calling the io barriers.
>> >
>> > generic ioread32 -> ARC readl() [ has barriers]
>> > generic ioread32be -> __be32_to_cpu(__raw_readl()) [ lacks barriers]
>> >
>> > While generic ioread32be is being remediated to call readl(), that involves
>> > a swab32(), causing double swaps on ioread32be() on Big Endian systems.
>> >
>> > So provide our versions of big endian IO accessors to ensure io barrier
>> > calls while also keeping them optimal
>> >
>> > Suggested-by: Arnd Bergmann <arnd@arndb.de>
>> > Cc: stable@vger.kernel.org [4.2+]
>> > Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
>
> Yes, that looks correct. We probably want this regardless of the change
> I proposed for the generic file, to avoid the double swap.
Indeed so - I've queued this for 4.6 fixes !
> Acked-by: Arnd Bergmann <arnd@arndb.de>
Thx !
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-05-05 11:05 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-04 17:16 [PATCH 1/7] asm-generic/io.h: add io{read,write}64 accessors Horia Geantă
2016-05-04 17:16 ` Horia Geantă
2016-05-04 22:35 ` Arnd Bergmann
2016-05-04 22:35 ` Arnd Bergmann
2016-05-05 8:16 ` Vineet Gupta
2016-05-05 8:16 ` Vineet Gupta
2016-05-05 10:56 ` Arnd Bergmann
2016-05-05 10:56 ` Arnd Bergmann
2016-05-05 11:05 ` Vineet Gupta
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).