2018-01-24 09:05:16

by Yury Norov

[permalink] [raw]
Subject: [PATCH RFC 0/3] API for 128-bit IO access

Hi all,

This series adds API for 128-bit memory IO access and enables it for ARM64.
The original motivation for 128-bit API came from new Cavium network device
driver. The hardware requires 128-bit access to make things work. See
description in patch 3 for details.

Also, starting from ARMv8.4, stp and ldp instructions become atomic, and
API for 128-bit access would be helpful in core arm64 code.

This series is RFC. I'd like to collect opinions on idea and implementation
details.
* I didn't implement all 128-bit operations existing for 64-bit variables
and other types (__swab128p etc). Do we need them all right now, or we
can add them when actually needed?
* u128 name is already used in crypto code. So here I use __uint128_t that
comes from GCC for 128-bit types. Should I rename existing type in crypto
and make core code for 128-bit variables consistent with u64, u32 etc? (I
think yes, but would like to ask crypto people for it.)
* Some compilers don't support __uint128_t, so I protected all generic code
with config option HAVE_128BIT_ACCESS. I think it's OK, but...
* For 128-bit read/write functions I take suffix 'o', which means read/write
the octet of bytes. Is this name OK?
* my mips-linux-gnu-gcc v6.3.0 doesn't support __uint128_t, and I
don't have other BE setup on hand, so BE case is formally not tested.
BE code for arm64 is looking well though.

With all that, this example code:

static int __init 128bit_test(void)
{
__uint128_t v;
__uint128_t addr;
__uint128_t val = (__uint128_t) 0x1234567890abc;

val |= ((__uint128_t) 0xdeadbeaf) << 64;

writeo(val, &addr);
v = reado(&addr);

pr_err("%llx%llx\n", (u64) (val >> 64), (u64) val);
pr_err("%llx%llx\n", (u64) (v >> 64), (u64) v);
return v != val;
}

Generates this listing for arm64-le:

0000000000000000 <128bit_test>:
0: a9bb7bfd stp x29, x30, [sp, #-80]!
4: 910003fd mov x29, sp
8: a90153f3 stp x19, x20, [sp, #16]
c: a9025bf5 stp x21, x22, [sp, #32]
10: f9001bf7 str x23, [sp, #48]
14: d5033e9f dsb st
18: d2815797 mov x23, #0xabc // #2748
1c: d297d5f6 mov x22, #0xbeaf // #48815
20: f2acf137 movk x23, #0x6789, lsl #16
24: f2bbd5b6 movk x22, #0xdead, lsl #16
28: f2c468b7 movk x23, #0x2345, lsl #32
2c: f2e00037 movk x23, #0x1, lsl #48
30: a9045bb7 stp x23, x22, [x29, #64]
34: a94453b3 ldp x19, x20, [x29, #64]
38: d5033d9f dsb ld
3c: 90000015 adrp x21, 0 <128bit_test>
40: 910002b5 add x21, x21, #0x0
44: aa1703e2 mov x2, x23
48: aa1603e1 mov x1, x22
4c: aa1503e0 mov x0, x21
50: 94000000 bl 0 <printk>
54: aa1303e2 mov x2, x19
58: aa1403e1 mov x1, x20
5c: ca170273 eor x19, x19, x23
60: ca160294 eor x20, x20, x22
64: aa1503e0 mov x0, x21
68: aa140273 orr x19, x19, x20
6c: 94000000 bl 0 <printk>
70: f9401bf7 ldr x23, [sp, #48]
74: f100027f cmp x19, #0x0
78: a94153f3 ldp x19, x20, [sp, #16]
7c: 1a9f07e0 cset w0, ne // ne = any
80: a9425bf5 ldp x21, x22, [sp, #32]
84: a8c57bfd ldp x29, x30, [sp], #80
88: d65f03c0 ret

And for arm64-be:

0000000000000000 <128bit_test>:
0: a9bb7bfd stp x29, x30, [sp, #-80]!
4: 910003fd mov x29, sp
8: a90153f3 stp x19, x20, [sp, #16]
c: a9025bf5 stp x21, x22, [sp, #32]
10: f9001bf7 str x23, [sp, #48]
14: d5033e9f dsb st
18: d2802001 mov x1, #0x100 // #256
1c: d2d5bbc0 mov x0, #0xadde00000000 // #191168994344960
20: f2a8a461 movk x1, #0x4523, lsl #16
24: f2f5f7c0 movk x0, #0xafbe, lsl #48
28: f2d12ce1 movk x1, #0x8967, lsl #32
2c: f2f78141 movk x1, #0xbc0a, lsl #48
30: a90407a0 stp x0, x1, [x29, #64]
34: a94453b3 ldp x19, x20, [x29, #64]
38: dac00e73 rev x19, x19
3c: dac00e94 rev x20, x20
40: d5033d9f dsb ld
44: d2815796 mov x22, #0xabc // #2748
48: 90000015 adrp x21, 0 <128bit_test>
4c: f2acf136 movk x22, #0x6789, lsl #16
50: 910002b5 add x21, x21, #0x0
54: f2c468b6 movk x22, #0x2345, lsl #32
58: d297d5f7 mov x23, #0xbeaf // #48815
5c: f2e00036 movk x22, #0x1, lsl #48
60: f2bbd5b7 movk x23, #0xdead, lsl #16
64: aa1603e2 mov x2, x22
68: aa1703e1 mov x1, x23
6c: aa1503e0 mov x0, x21
70: 94000000 bl 0 <printk>
74: aa1403e2 mov x2, x20
78: aa1303e1 mov x1, x19
7c: ca160294 eor x20, x20, x22
80: ca170273 eor x19, x19, x23
84: aa1503e0 mov x0, x21
88: aa140273 orr x19, x19, x20
8c: 94000000 bl 0 <printk>
90: f9401bf7 ldr x23, [sp, #48]
94: f100027f cmp x19, #0x0
98: a94153f3 ldp x19, x20, [sp, #16]
9c: 1a9f07e0 cset w0, ne // ne = any
a0: a9425bf5 ldp x21, x22, [sp, #32]
a4: a8c57bfd ldp x29, x30, [sp], #80
a8: d65f03c0 ret

I tested LE kernel with this, and it works OK for me. BE version adds
few extra instructions to swap bytes, but generated code looks reasonable.
We can avoid byteswapping, if not needed, by using __raw_reado() and
__raw_writeo().

Yury Norov (3):
UAPI: Introduce 128-bit types and byteswap operations
asm-generic/io.h: API for 128-bit I/O accessors
arm64: enable 128-bit memory read/write support

arch/Kconfig | 7 ++
arch/arm64/include/asm/io.h | 31 ++++++
include/asm-generic/io.h | 147 +++++++++++++++++++++++++++
include/linux/byteorder/generic.h | 4 +
include/uapi/asm-generic/int-ll64.h | 8 ++
include/uapi/linux/byteorder/big_endian.h | 2 +
include/uapi/linux/byteorder/little_endian.h | 4 +
include/uapi/linux/swab.h | 22 ++++
include/uapi/linux/types.h | 4 +
9 files changed, 229 insertions(+)

--
2.11.0


2018-01-24 09:05:52

by Yury Norov

[permalink] [raw]
Subject: [PATCH 1/3] UAPI: Introduce 128-bit types and byteswap operations

Architectures like arm64 support 128-bit integer types and
operations. This patch introduces corresponding types and
__swab128() operation for be/le conversions.

They are required to implement 128-bit access to the memory,
in following patches.

Signed-off-by: Yury Norov <[email protected]>
---
include/linux/byteorder/generic.h | 8 ++++++++
include/uapi/asm-generic/int-ll64.h | 8 ++++++++
include/uapi/linux/byteorder/big_endian.h | 4 ++++
include/uapi/linux/byteorder/little_endian.h | 8 ++++++++
include/uapi/linux/swab.h | 22 ++++++++++++++++++++++
include/uapi/linux/types.h | 4 ++++
6 files changed, 54 insertions(+)

diff --git a/include/linux/byteorder/generic.h b/include/linux/byteorder/generic.h
index 451aaa0786ae..aa61662ee3dc 100644
--- a/include/linux/byteorder/generic.h
+++ b/include/linux/byteorder/generic.h
@@ -85,12 +85,20 @@

#define cpu_to_le64 __cpu_to_le64
#define le64_to_cpu __le64_to_cpu
+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#define cpu_to_le128 __cpu_to_le128
+#define le128_to_cpu __le128_to_cpu
+#endif
#define cpu_to_le32 __cpu_to_le32
#define le32_to_cpu __le32_to_cpu
#define cpu_to_le16 __cpu_to_le16
#define le16_to_cpu __le16_to_cpu
#define cpu_to_be64 __cpu_to_be64
#define be64_to_cpu __be64_to_cpu
+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#define cpu_to_be128 __cpu_to_be128
+#define be128_to_cpu __be128_to_cpu
+#endif
#define cpu_to_be32 __cpu_to_be32
#define be32_to_cpu __be32_to_cpu
#define cpu_to_be16 __cpu_to_be16
diff --git a/include/uapi/asm-generic/int-ll64.h b/include/uapi/asm-generic/int-ll64.h
index 1ed06964257c..4bc2241988a9 100644
--- a/include/uapi/asm-generic/int-ll64.h
+++ b/include/uapi/asm-generic/int-ll64.h
@@ -29,9 +29,17 @@ typedef unsigned int __u32;
#ifdef __GNUC__
__extension__ typedef __signed__ long long __s64;
__extension__ typedef unsigned long long __u64;
+#ifdef CONFIG_HAVE_128BIT_ACCESS
+__extension__ typedef __int128_t __s128;
+__extension__ typedef __uint128_t __u128;
+#endif
#else
typedef __signed__ long long __s64;
typedef unsigned long long __u64;
+#ifdef CONFIG_HAVE_128BIT_ACCESS
+typedef __int128_t __s128;
+typedef __uint128_t __u128;
+#endif
#endif

#endif /* __ASSEMBLY__ */
diff --git a/include/uapi/linux/byteorder/big_endian.h b/include/uapi/linux/byteorder/big_endian.h
index 2199adc6a6c2..28a69ec10dd2 100644
--- a/include/uapi/linux/byteorder/big_endian.h
+++ b/include/uapi/linux/byteorder/big_endian.h
@@ -30,6 +30,10 @@
#define __constant_be16_to_cpu(x) ((__force __u16)(__be16)(x))
#define __cpu_to_le64(x) ((__force __le64)__swab64((x)))
#define __le64_to_cpu(x) __swab64((__force __u64)(__le64)(x))
+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#define __cpu_to_le128(x) ((__force __le128)__swab128((x)))
+#define __le128_to_cpu(x) __swab128((__force __u128)(__le128)(x))
+#endif
#define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
#define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
diff --git a/include/uapi/linux/byteorder/little_endian.h b/include/uapi/linux/byteorder/little_endian.h
index 601c904fd5cd..15365bd0fe29 100644
--- a/include/uapi/linux/byteorder/little_endian.h
+++ b/include/uapi/linux/byteorder/little_endian.h
@@ -18,6 +18,10 @@
#define __constant_ntohs(x) ___constant_swab16((__force __be16)(x))
#define __constant_cpu_to_le64(x) ((__force __le64)(__u64)(x))
#define __constant_le64_to_cpu(x) ((__force __u64)(__le64)(x))
+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#define __constant_cpu_to_le128(x) ((__force __le128)(__u128)(x))
+#define __constant_le128_to_cpu(x) ((__force __u128)(__le128)(x))
+#endif
#define __constant_cpu_to_le32(x) ((__force __le32)(__u32)(x))
#define __constant_le32_to_cpu(x) ((__force __u32)(__le32)(x))
#define __constant_cpu_to_le16(x) ((__force __le16)(__u16)(x))
@@ -30,6 +34,10 @@
#define __constant_be16_to_cpu(x) ___constant_swab16((__force __u16)(__be16)(x))
#define __cpu_to_le64(x) ((__force __le64)(__u64)(x))
#define __le64_to_cpu(x) ((__force __u64)(__le64)(x))
+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#define __cpu_to_le128(x) ((__force __le128)(__u128)(x))
+#define __le128_to_cpu(x) ((__force __u128)(__le128)(x))
+#endif
#define __cpu_to_le32(x) ((__force __le32)(__u32)(x))
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
#define __cpu_to_le16(x) ((__force __le16)(__u16)(x))
diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h
index 23cd84868cc3..a7e97eb06a3e 100644
--- a/include/uapi/linux/swab.h
+++ b/include/uapi/linux/swab.h
@@ -75,6 +75,20 @@ static inline __attribute_const__ __u64 __fswab64(__u64 val)
#endif
}

+#ifdef CONFIG_HAVE_128BIT_ACCESS
+static inline __attribute_const__ __u128 __fswab128(__u128 val)
+{
+#if defined(__arch_swab128)
+ return __arch_swab128(val);
+#else
+ __u64 h = (__u64) (val >> 64);
+ __u64 l = (__u64) val;
+
+ return (((__u128)__fswab64(l)) << 64) | (__u128)(__fswab64(h));
+#endif
+}
+#endif
+
static inline __attribute_const__ __u32 __fswahw32(__u32 val)
{
#ifdef __arch_swahw32
@@ -132,6 +146,14 @@ static inline __attribute_const__ __u32 __fswahb32(__u32 val)
__fswab64(x))
#endif

+#ifdef CONFIG_HAVE_128BIT_ACCESS
+/**
+ * __swab128 - return a byteswapped 128-bit value
+ * @x: value to byteswap
+ */
+#define __swab128(x) __fswab128(x)
+#endif
+
/**
* __swahw32 - return a word-swapped 32-bit value
* @x: value to wordswap
diff --git a/include/uapi/linux/types.h b/include/uapi/linux/types.h
index cd4f0b897a48..a4500baaccd6 100644
--- a/include/uapi/linux/types.h
+++ b/include/uapi/linux/types.h
@@ -32,6 +32,10 @@ typedef __u32 __bitwise __le32;
typedef __u32 __bitwise __be32;
typedef __u64 __bitwise __le64;
typedef __u64 __bitwise __be64;
+#ifdef CONFIG_HAVE_128BIT_ACCESS
+typedef __u128 __bitwise __le128;
+typedef __u128 __bitwise __be128;
+#endif

typedef __u16 __bitwise __sum16;
typedef __u32 __bitwise __wsum;
--
2.11.0

2018-01-24 09:05:18

by Yury Norov

[permalink] [raw]
Subject: [PATCH 2/3] asm-generic/io.h: API for 128-bit memory accessors

Some architectures, like arm64, support 128-bit memory access. For
ARM64 - using load/store pair instructions. This patch introduces
reado() and writeo() functions family, where suffix 'o' stands for
reading and writing the octet of bytes at once.

Signed-off-by: Yury Norov <[email protected]>
---
include/asm-generic/io.h | 147 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 147 insertions(+)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index b4531e3b2120..ac4a4de69efc 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -67,6 +67,16 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
#endif
#endif /* CONFIG_64BIT */

+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#ifndef __raw_reado
+#define __raw_reado __raw_reado
+static inline __uint128_t __raw_reado(const volatile void __iomem *addr)
+{
+ return *(const volatile __uint128_t __force *)addr;
+}
+#endif
+#endif /* CONFIG_HAVE_128BIT_ACCESS */
+
#ifndef __raw_writeb
#define __raw_writeb __raw_writeb
static inline void __raw_writeb(u8 value, volatile void __iomem *addr)
@@ -101,6 +111,16 @@ static inline void __raw_writeq(u64 value, volatile void __iomem *addr)
#endif
#endif /* CONFIG_64BIT */

+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#ifndef __raw_writeo
+#define __raw_writeo __raw_writeo
+static inline void __raw_writeo(__uint128_t value, volatile void __iomem *addr)
+{
+ *(volatile __uint128_t __force *)addr = value;
+}
+#endif
+#endif /* CONFIG_HAVE_128BIT_ACCESS */
+
/*
* {read,write}{b,w,l,q}() access little endian memory and return result in
* native endianness.
@@ -140,6 +160,16 @@ static inline u64 readq(const volatile void __iomem *addr)
#endif
#endif /* CONFIG_64BIT */

+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#ifndef reado
+#define reado reado
+static inline __uint128_t reado(const volatile void __iomem *addr)
+{
+ return __le128_to_cpu(__raw_reado(addr));
+}
+#endif
+#endif /* CONFIG_HAVE_128BIT_ACCESS */
+
#ifndef writeb
#define writeb writeb
static inline void writeb(u8 value, volatile void __iomem *addr)
@@ -174,6 +204,16 @@ static inline void writeq(u64 value, volatile void __iomem *addr)
#endif
#endif /* CONFIG_64BIT */

+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#ifndef writeo
+#define writeo writeo
+static inline void writeo(__uint128_t value, volatile void __iomem *addr)
+{
+ __raw_writeo(__cpu_to_le128(value), addr);
+}
+#endif
+#endif /* CONFIG_HAVE_128BIT_ACCESS */
+
/*
* {read,write}{b,w,l,q}_relaxed() are like the regular version, but
* are not guaranteed to provide ordering against spinlocks or memory
@@ -195,6 +235,10 @@ static inline void writeq(u64 value, volatile void __iomem *addr)
#define readq_relaxed readq
#endif

+#if defined(reado) && !defined(reado_relaxed)
+#define reado_relaxed reado
+#endif
+
#ifndef writeb_relaxed
#define writeb_relaxed writeb
#endif
@@ -211,6 +255,10 @@ static inline void writeq(u64 value, volatile void __iomem *addr)
#define writeq_relaxed writeq
#endif

+#if defined(writeo) && !defined(writeo_relaxed)
+#define writeo_relaxed writeo
+#endif
+
/*
* {read,write}s{b,w,l,q}() repeatedly access the same memory address in
* native endianness in 8-, 16-, 32- or 64-bit chunks (@count times).
@@ -281,6 +329,24 @@ static inline void readsq(const volatile void __iomem *addr, void *buffer,
#endif
#endif /* CONFIG_64BIT */

+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#ifndef readso
+#define readso readso
+static inline void readso(const volatile void __iomem *addr, void *buffer,
+ unsigned int count)
+{
+ if (count) {
+ __uint128_t *buf = buffer;
+
+ do {
+ __uint128_t x = __raw_reado(addr);
+ *buf++ = x;
+ } while (--count);
+ }
+}
+#endif
+#endif /* CONFIG_HAVE_128BIT_ACCESS */
+
#ifndef writesb
#define writesb writesb
static inline void writesb(volatile void __iomem *addr, const void *buffer,
@@ -343,6 +409,23 @@ static inline void writesq(volatile void __iomem *addr, const void *buffer,
#endif
#endif /* CONFIG_64BIT */

+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#ifndef writeso
+#define writeso writeso
+static inline void writeso(volatile void __iomem *addr, const void *buffer,
+ unsigned int count)
+{
+ if (count) {
+ const __uint128_t *buf = buffer;
+
+ do {
+ __raw_writeo(*buf++, addr);
+ } while (--count);
+ }
+}
+#endif
+#endif /* CONFIG_HAVE_128BIT_ACCESS */
+
#ifndef PCI_IOBASE
#define PCI_IOBASE ((void __iomem *)0)
#endif
@@ -595,6 +678,16 @@ static inline u64 ioread64(const volatile void __iomem *addr)
#endif
#endif /* CONFIG_64BIT */

+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#ifndef ioread128
+#define ioread128 ioread128
+static inline __uint128_t ioread128(const volatile void __iomem *addr)
+{
+ return reado(addr);
+}
+#endif
+#endif /* CONFIG_HAVE_128BIT_ACCESS */
+
#ifndef iowrite8
#define iowrite8 iowrite8
static inline void iowrite8(u8 value, volatile void __iomem *addr)
@@ -629,6 +722,16 @@ static inline void iowrite64(u64 value, volatile void __iomem *addr)
#endif
#endif /* CONFIG_64BIT */

+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#ifndef iowrite128
+#define iowrite128 iowrite128
+static inline void iowrite128(__uint128_t value, volatile void __iomem *addr)
+{
+ writeo(value, addr);
+}
+#endif
+#endif /* CONFIG_HAVE_128BIT_ACCESS */
+
#ifndef ioread16be
#define ioread16be ioread16be
static inline u16 ioread16be(const volatile void __iomem *addr)
@@ -655,6 +758,16 @@ static inline u64 ioread64be(const volatile void __iomem *addr)
#endif
#endif /* CONFIG_64BIT */

+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#ifndef ioread128be
+#define ioread128be ioread128be
+static inline __uint128_t ioread128be(const volatile void __iomem *addr)
+{
+ return swab128(reado(addr));
+}
+#endif
+#endif /* CONFIG_HAVE_128BIT_ACCESS */
+
#ifndef iowrite16be
#define iowrite16be iowrite16be
static inline void iowrite16be(u16 value, void volatile __iomem *addr)
@@ -681,6 +794,16 @@ static inline void iowrite64be(u64 value, volatile void __iomem *addr)
#endif
#endif /* CONFIG_64BIT */

+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#ifndef iowrite128be
+#define iowrite128be iowrite128be
+static inline void iowrite128be(__uint128_t value, volatile void __iomem *addr)
+{
+ writeo(swab128(value), addr);
+}
+#endif
+#endif /* CONFIG_HAVE_128BIT_ACCESS */
+
#ifndef ioread8_rep
#define ioread8_rep ioread8_rep
static inline void ioread8_rep(const volatile void __iomem *addr, void *buffer,
@@ -719,6 +842,17 @@ static inline void ioread64_rep(const volatile void __iomem *addr,
#endif
#endif /* CONFIG_64BIT */

+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#ifndef ioread128_rep
+#define ioread128_rep ioread128_rep
+static inline void ioread128_rep(const volatile void __iomem *addr,
+ void *buffer, unsigned int count)
+{
+ readso(addr, buffer, count);
+}
+#endif
+#endif /* CONFIG_HAVE_128BIT_ACCESS */
+
#ifndef iowrite8_rep
#define iowrite8_rep iowrite8_rep
static inline void iowrite8_rep(volatile void __iomem *addr,
@@ -760,6 +894,19 @@ static inline void iowrite64_rep(volatile void __iomem *addr,
}
#endif
#endif /* CONFIG_64BIT */
+
+#ifdef CONFIG_HAVE_128BIT_ACCESS
+#ifndef iowrite128_rep
+#define iowrite128_rep iowrite128_rep
+static inline void iowrite128_rep(volatile void __iomem *addr,
+ const void *buffer,
+ unsigned int count)
+{
+ writeso(addr, buffer, count);
+}
+#endif
+#endif /* CONFIG_HAVE_128BIT_ACCESS */
+
#endif /* CONFIG_GENERIC_IOMAP */

#ifdef __KERNEL__
--
2.11.0

2018-01-24 09:05:19

by Yury Norov

[permalink] [raw]
Subject: [PATCH 3/3] arm64: enable 128-bit memory read/write support

Introduce __raw_writeo(), __raw_reado() and other arch-specific
RW functions for 128-bit memory access, and enable it for arm64.

128-bit I/O is required for example by Octeon TX2 device to access
some registers. According to Hardware Reference Manual:

A 128-bit write to the OP_FREE0/1 registers frees a pointer into a
given [...] pool. All other accesses to these registers (e.g. reads
and 64-bit writes) are RAZ/WI.

Starting from ARMv8.4, stp and ldp instructions become atomic, and
API for 128-bit access would be helpful for core code.

Signed-off-by: Yury Norov <[email protected]>
---
arch/Kconfig | 7 +++++++
arch/arm64/include/asm/io.h | 31 +++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index 76c0b54443b1..2baff7de405d 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -116,6 +116,13 @@ config UPROBES
managed by the kernel and kept transparent to the probed
application. )

+config HAVE_128BIT_ACCESS
+ def_bool ARM64
+ help
+ Architectures having 128-bit access require corresponding APIs,
+ like reado() and writeo(), which stands for reading and writing
+ the octet of bytes at once.
+
config HAVE_64BIT_ALIGNED_ACCESS
def_bool 64BIT && !HAVE_EFFICIENT_UNALIGNED_ACCESS
help
diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index 35b2e50f17fb..7c5d834abfd8 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -60,6 +60,18 @@ static inline void __raw_writeq(u64 val, volatile void __iomem *addr)
asm volatile("str %x0, [%1]" : : "rZ" (val), "r" (addr));
}

+#define __raw_writeo __raw_writeo
+static inline void __raw_writeo(__uint128_t val, volatile void __iomem *addr)
+{
+ u64 l = (u64) val;
+ u64 h = (u64) (val >> 64);
+ __uint128_t *__addr = (__uint128_t *) addr;
+
+ asm volatile("stp %x[x0], %x[x1], %x[p1]"
+ : [p1]"=Ump"(*__addr)
+ : [x0]"r"(l), [x1]"r"(h));
+}
+
#define __raw_readb __raw_readb
static inline u8 __raw_readb(const volatile void __iomem *addr)
{
@@ -105,6 +117,19 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
return val;
}

+#define __raw_reado __raw_reado
+static inline __uint128_t __raw_reado(const volatile void __iomem *addr)
+{
+ u64 l, h;
+ __uint128_t *__addr = (__uint128_t *) addr;
+
+ asm volatile("ldp %x[x0], %x[x1], %x[p1]"
+ : [x0]"=r"(l), [x1]"=r"(h)
+ : [p1]"Ump"(*__addr));
+
+ return (__uint128_t) l | ((__uint128_t) h) << 64;
+}
+
/* IO barriers */
#define __iormb() rmb()
#define __iowmb() wmb()
@@ -120,11 +145,13 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
#define readw_relaxed(c) ({ u16 __r = le16_to_cpu((__force __le16)__raw_readw(c)); __r; })
#define readl_relaxed(c) ({ u32 __r = le32_to_cpu((__force __le32)__raw_readl(c)); __r; })
#define readq_relaxed(c) ({ u64 __r = le64_to_cpu((__force __le64)__raw_readq(c)); __r; })
+#define reado_relaxed(c) ({ __uint128_t __r = le128_to_cpu((__force __le128)__raw_reado(c)); __r; })

#define writeb_relaxed(v,c) ((void)__raw_writeb((v),(c)))
#define writew_relaxed(v,c) ((void)__raw_writew((__force u16)cpu_to_le16(v),(c)))
#define writel_relaxed(v,c) ((void)__raw_writel((__force u32)cpu_to_le32(v),(c)))
#define writeq_relaxed(v,c) ((void)__raw_writeq((__force u64)cpu_to_le64(v),(c)))
+#define writeo_relaxed(v,c) ((void)__raw_writeo((__force __uint128_t)cpu_to_le128(v),(c)))

/*
* I/O memory access primitives. Reads are ordered relative to any
@@ -135,11 +162,13 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
#define readw(c) ({ u16 __v = readw_relaxed(c); __iormb(); __v; })
#define readl(c) ({ u32 __v = readl_relaxed(c); __iormb(); __v; })
#define readq(c) ({ u64 __v = readq_relaxed(c); __iormb(); __v; })
+#define reado(c) ({ __uint128_t __v = reado_relaxed(c); __iormb(); __v; })

#define writeb(v,c) ({ __iowmb(); writeb_relaxed((v),(c)); })
#define writew(v,c) ({ __iowmb(); writew_relaxed((v),(c)); })
#define writel(v,c) ({ __iowmb(); writel_relaxed((v),(c)); })
#define writeq(v,c) ({ __iowmb(); writeq_relaxed((v),(c)); })
+#define writeo(v,c) ({ __iowmb(); writeo_relaxed((v),(c)); })

/*
* I/O port access primitives.
@@ -188,10 +217,12 @@ extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
#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 ioread64be(p) ({ __u64 __v = be64_to_cpu((__force __be64)__raw_readq(p)); __iormb(); __v; })
+#define ioread128be(p) ({ __uint128_t __v = be128_to_cpu((__force __be128)__raw_readq(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); })
#define iowrite64be(v,p) ({ __iowmb(); __raw_writeq((__force __u64)cpu_to_be64(v), p); })
+#define iowrite128be(v,p) ({ __iowmb(); __raw_writeo((__force __u128)cpu_to_be128(v), p); })

#include <asm-generic/io.h>

--
2.11.0

2018-01-24 10:22:13

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] API for 128-bit IO access

On Wed, Jan 24, 2018 at 12:05:16PM +0300, Yury Norov wrote:
> This series adds API for 128-bit memory IO access and enables it for ARM64.
> The original motivation for 128-bit API came from new Cavium network device
> driver. The hardware requires 128-bit access to make things work. See
> description in patch 3 for details.
>
> Also, starting from ARMv8.4, stp and ldp instructions become atomic, and
> API for 128-bit access would be helpful in core arm64 code.

Only for normal, cacheable memory, so they're not suitable for IO accesses
as you're proposing here.

Will

2018-01-24 10:28:55

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] API for 128-bit IO access

On Wed, Jan 24, 2018 at 10:05 AM, Yury Norov <[email protected]> wrote:
> This series adds API for 128-bit memory IO access and enables it for ARM64.
> The original motivation for 128-bit API came from new Cavium network device
> driver. The hardware requires 128-bit access to make things work. See
> description in patch 3 for details.

We might also want to do something similar to the
include/linux/io-64-nonatomic-lo-hi.h
and hi-lo.h files, to simulate 128-bit access using pairs of 64-bit access on
other targets. It's apparently driver specific which half you need to do first
to make it work, so we need both.

> Also, starting from ARMv8.4, stp and ldp instructions become atomic, and
> API for 128-bit access would be helpful in core arm64 code.
>
> This series is RFC. I'd like to collect opinions on idea and implementation
> details.
> * I didn't implement all 128-bit operations existing for 64-bit variables
> and other types (__swab128p etc). Do we need them all right now, or we
> can add them when actually needed?

I think in this case it's better to do them all at once.

> * u128 name is already used in crypto code. So here I use __uint128_t that
> comes from GCC for 128-bit types. Should I rename existing type in crypto
> and make core code for 128-bit variables consistent with u64, u32 etc? (I
> think yes, but would like to ask crypto people for it.)

Hmm, that file probably predates the __uint128_t support. My guess would
be that the crypto code using it can actually benefit from the new types as
well, so maybe move the existing file to include/linux/int128.h and add
an #if/#else logic to it so we use 'typedef __uint128_t __u128' if that
is available.

> * Some compilers don't support __uint128_t, so I protected all generic code
> with config option HAVE_128BIT_ACCESS. I think it's OK, but...

That would be nicely solved by using the #if/#else definition above.

> * For 128-bit read/write functions I take suffix 'o', which means read/write
> the octet of bytes. Is this name OK?

Can't think of anything better. It's not an octet though, but 16 bytes
('q' is for quadword, meaning four 16-bit words in Intel terminology).

> * my mips-linux-gnu-gcc v6.3.0 doesn't support __uint128_t, and I
> don't have other BE setup on hand, so BE case is formally not tested.
> BE code for arm64 is looking well though.

I've run it through my collection of compilers, it seems that most but not
all 64-bit targets support it (exceptions appear to be older versions of
gcc for s390x and parisc), and none of the 32-bit targets do:

$ for i in /home/arnd/cross-gcc/bin/*gcc-[3-8]* ; do echo -n $i" " ;
echo '__uint128_t v;' | $i -xc -S - -o /dev/null && echo ok ; done
/home/arnd/cross-gcc/bin/aarch64-linux-gcc-4.8.5 ok
/home/arnd/cross-gcc/bin/aarch64-linux-gcc-4.9.3 ok
/home/arnd/cross-gcc/bin/aarch64-linux-gcc-4.9.4 ok
/home/arnd/cross-gcc/bin/aarch64-linux-gcc-5.2.1 ok
/home/arnd/cross-gcc/bin/aarch64-linux-gcc-5.4.1 ok
/home/arnd/cross-gcc/bin/aarch64-linux-gcc-5.5.0 ok
/home/arnd/cross-gcc/bin/aarch64-linux-gcc-6.3.1 ok
/home/arnd/cross-gcc/bin/aarch64-linux-gcc-7.0.0 ok
/home/arnd/cross-gcc/bin/aarch64-linux-gcc-7.0.1 ok
/home/arnd/cross-gcc/bin/aarch64-linux-gcc-7.1.1 ok
/home/arnd/cross-gcc/bin/aarch64-linux-gcc-7.2.1 ok
/home/arnd/cross-gcc/bin/aarch64-linux-gcc-8.0.0 ok
/home/arnd/cross-gcc/bin/alpha-linux-gcc-4.1.3 ok
/home/arnd/cross-gcc/bin/alpha-linux-gcc-4.3.6 ok
/home/arnd/cross-gcc/bin/alpha-linux-gcc-4.9.3 ok
/home/arnd/cross-gcc/bin/alpha-linux-gcc-7.2.1 ok
/home/arnd/cross-gcc/bin/am33_2.0-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/am33_2.0-linux-gcc-5.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/am33_2.0-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/arc-elf-gcc-7.2.1 <stdin>:1:1: error: unknown
type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-3.4.6 ok
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.3.6 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.4.7 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.5.4 <stdin>:1:13:
error: expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.6.4 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.7.4 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.8.5 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.0 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.1 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.2 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.3 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.4 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.0.0 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.1.1 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.2.1 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.3.1 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.4.1 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.5.0 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-6.0.0 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-6.1.1 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-6.3.1 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-7.0.0 <stdin>:1:1:
error: unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-7.0.1 <stdin>:1:1:
error: unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-7.1.1 <stdin>:1:1:
error: unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-7.2.1 <stdin>:1:1:
error: unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-8.0.0 <stdin>:1:1:
error: unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/bfin-uclinux-gcc-7.0.0
bfin-uclinux-gcc-7.0.0: error trying to exec 'cc1': execvp: No such
file or directory
/home/arnd/cross-gcc/bin/bfin-uclinux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/c6x-elf-gcc-7.2.1 <stdin>:1:1: error: unknown
type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/cris-linux-gcc-4.1.3 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/cris-linux-gcc-4.3.6 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/cris-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/cris-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/frv-linux-gcc-4.1.3 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/frv-linux-gcc-4.3.6 <stdin>:1: internal
compiler error: in default_secondary_reload, at targhooks.c:618
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
/home/arnd/cross-gcc/bin/frv-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/frv-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/h8300-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/hppa64-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/hppa64-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/hppa-linux-gcc-4.1.3 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/hppa-linux-gcc-4.3.6 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/hppa-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/hppa-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/i386-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/i386-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/ia64-linux-gcc-7.2.1 ok
/home/arnd/cross-gcc/bin/m32r-linux-gcc-4.1.3 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/m32r-linux-gcc-4.3.6 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/m32r-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/m32r-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/m68k-linux-gcc-4.1.3 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/m68k-linux-gcc-4.3.6 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/m68k-linux-gcc-6.0.0 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/m68k-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/microblaze-linux-gcc-4.9.3 <stdin>:1:1:
error: unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/microblaze-linux-gcc-7.2.1 <stdin>:1:1:
error: unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/mips64-linux-gcc-4.9.3 ok
/home/arnd/cross-gcc/bin/mips64-linux-gcc-7.2.1 ok
/home/arnd/cross-gcc/bin/mips-linux-gcc-4.0.4 <stdin>:1: error: syntax
error before 'v'
<stdin>:1: warning: data definition has no type or storage class
/home/arnd/cross-gcc/bin/mips-linux-gcc-4.1.3 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/mips-linux-gcc-4.3.6 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/mips-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/mips-linux-gcc-7.0.0 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/mips-linux-gcc-7.0.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/mips-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/nios2-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/parisc-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/powerpc64-linux-gcc-4.1.3 ok
/home/arnd/cross-gcc/bin/powerpc64-linux-gcc-4.3.6 ok
/home/arnd/cross-gcc/bin/powerpc64-linux-gcc-4.9.3 ok
/home/arnd/cross-gcc/bin/powerpc64-linux-gcc-7.2.1 ok
/home/arnd/cross-gcc/bin/powerpc-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/powerpc-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/riscv32-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/riscv64-linux-gcc-7.2.1 ok
/home/arnd/cross-gcc/bin/s390-linux-gcc-4.1.3 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/s390-linux-gcc-4.3.6 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/s390-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/s390-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/sh2-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/sh3-linux-gcc-4.1.3 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/sh3-linux-gcc-4.3.6 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/sh3-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/sh4-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/sh-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/sparc64-linux-gcc-4.9.3 ok
/home/arnd/cross-gcc/bin/sparc64-linux-gcc-7.2.1 ok
/home/arnd/cross-gcc/bin/sparc-linux-gcc-4.1.3 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/sparc-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/sparc-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/tilegx-linux-gcc-7.2.1 ok
/home/arnd/cross-gcc/bin/tilepro-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-3.4.6 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.0.4 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.1.3 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.2.5 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.3.6 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.4.7 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.5.4 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.6.4 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.7.4 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.8.5 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.9.3 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.9.4 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-5.4.1 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-5.5.0 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-6.1.1 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-6.3.1 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-7.0.0 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-7.0.1 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-7.1.1 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-7.2.1 ok
/home/arnd/cross-gcc/bin/x86_64-linux-gcc-8.0.0 ok
/home/arnd/cross-gcc/bin/xtensa-linux-gcc-4.1.3 <stdin>:1: error:
expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
/home/arnd/cross-gcc/bin/xtensa-linux-gcc-4.9.3 <stdin>:1:1: error:
unknown type name '__uint128_t'
/home/arnd/cross-gcc/bin/xtensa-linux-gcc-7.2.1 <stdin>:1:1: error:
unknown type name '__uint128_t'; did you mean '__int128'?

2018-01-24 13:00:42

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 3/3] arm64: enable 128-bit memory read/write support

Hi Yury,

On Wed, Jan 24, 2018 at 10:05 AM, Yury Norov <[email protected]> wrote:
> Introduce __raw_writeo(), __raw_reado() and other arch-specific
> RW functions for 128-bit memory access, and enable it for arm64.
>
> 128-bit I/O is required for example by Octeon TX2 device to access
> some registers. According to Hardware Reference Manual:
>
> A 128-bit write to the OP_FREE0/1 registers frees a pointer into a
> given [...] pool. All other accesses to these registers (e.g. reads
> and 64-bit writes) are RAZ/WI.
>
> Starting from ARMv8.4, stp and ldp instructions become atomic, and
> API for 128-bit access would be helpful for core code.
>
> Signed-off-by: Yury Norov <[email protected]>

Thanks for your patch!

> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -116,6 +116,13 @@ config UPROBES
> managed by the kernel and kept transparent to the probed
> application. )
>
> +config HAVE_128BIT_ACCESS
> + def_bool ARM64

I think it's better to select this symbol from arch/arm64/Kconfig instead.
Else this file has to be modified each and every time an architecture
adds support for 128-bit, causing conflicts.

> + help
> + Architectures having 128-bit access require corresponding APIs,
> + like reado() and writeo(), which stands for reading and writing
> + the octet of bytes at once.
> +
> config HAVE_64BIT_ALIGNED_ACCESS
> def_bool 64BIT && !HAVE_EFFICIENT_UNALIGNED_ACCESS
> help

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2018-01-24 15:48:03

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] API for 128-bit IO access

On Wed, Jan 24, 2018 at 12:28 PM, Arnd Bergmann <[email protected]> wrote:
> On Wed, Jan 24, 2018 at 10:05 AM, Yury Norov <yn[email protected]> wrote:

>> * For 128-bit read/write functions I take suffix 'o', which means read/write
>> the octet of bytes. Is this name OK?
>
> Can't think of anything better. It's not an octet though, but 16 bytes
> ('q' is for quadword, meaning four 16-bit words in Intel terminology).

It's apparently follows Intel's terminology by implying "word", so, "octetword".

--
With Best Regards,
Andy Shevchenko

2018-01-24 16:38:42

by Jeffrey Walton

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] API for 128-bit IO access

On Wed, Jan 24, 2018 at 4:05 AM, Yury Norov <[email protected]> wrote:
>
> ...
> With all that, this example code:
>
> static int __init 128bit_test(void)
> {
> __uint128_t v;
> __uint128_t addr;
> __uint128_t val = (__uint128_t) 0x1234567890abc;
> ...

In case it matters, you can check for GCC support of the 128-bit types
in userland with:

#if (__SIZEOF_INT128__ >= 16)
...
#endif

Also see https://gcc.gnu.org/ml/gcc-help/2015-08/msg00185.html .

Jeff

2018-01-24 18:20:10

by Yury Norov

[permalink] [raw]
Subject: Re: [PATCH 3/3] arm64: enable 128-bit memory read/write support

On Wed, Jan 24, 2018 at 02:00:42PM +0100, Geert Uytterhoeven wrote:
> Hi Yury,
>
> On Wed, Jan 24, 2018 at 10:05 AM, Yury Norov <[email protected]> wrote:
> > Introduce __raw_writeo(), __raw_reado() and other arch-specific
> > RW functions for 128-bit memory access, and enable it for arm64.
> >
> > 128-bit I/O is required for example by Octeon TX2 device to access
> > some registers. According to Hardware Reference Manual:
> >
> > A 128-bit write to the OP_FREE0/1 registers frees a pointer into a
> > given [...] pool. All other accesses to these registers (e.g. reads
> > and 64-bit writes) are RAZ/WI.
> >
> > Starting from ARMv8.4, stp and ldp instructions become atomic, and
> > API for 128-bit access would be helpful for core code.
> >
> > Signed-off-by: Yury Norov <[email protected]>
>
> Thanks for your patch!
>
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -116,6 +116,13 @@ config UPROBES
> > managed by the kernel and kept transparent to the probed
> > application. )
> >
> > +config HAVE_128BIT_ACCESS
> > + def_bool ARM64
>
> I think it's better to select this symbol from arch/arm64/Kconfig instead.
> Else this file has to be modified each and every time an architecture
> adds support for 128-bit, causing conflicts.

Shure, thanks.

Yury

> > + help
> > + Architectures having 128-bit access require corresponding APIs,
> > + like reado() and writeo(), which stands for reading and writing
> > + the octet of bytes at once.
> > +
> > config HAVE_64BIT_ALIGNED_ACCESS
> > def_bool 64BIT && !HAVE_EFFICIENT_UNALIGNED_ACCESS
> > help
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds

2018-01-25 11:38:43

by Yury Norov

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] API for 128-bit IO access

On Wed, Jan 24, 2018 at 11:28:55AM +0100, Arnd Bergmann wrote:
> On Wed, Jan 24, 2018 at 10:05 AM, Yury Norov <[email protected]> wrote:
> > This series adds API for 128-bit memory IO access and enables it for ARM64.
> > The original motivation for 128-bit API came from new Cavium network device
> > driver. The hardware requires 128-bit access to make things work. See
> > description in patch 3 for details.
>
> We might also want to do something similar to the
> include/linux/io-64-nonatomic-lo-hi.h
> and hi-lo.h files, to simulate 128-bit access using pairs of 64-bit access on
> other targets. It's apparently driver specific which half you need to do first
> to make it work, so we need both.

OK, will do.

> > Also, starting from ARMv8.4, stp and ldp instructions become atomic, and
> > API for 128-bit access would be helpful in core arm64 code.
> >
> > This series is RFC. I'd like to collect opinions on idea and implementation
> > details.
> > * I didn't implement all 128-bit operations existing for 64-bit variables
> > and other types (__swab128p etc). Do we need them all right now, or we
> > can add them when actually needed?
>
> I think in this case it's better to do them all at once.

Ack.

> > * u128 name is already used in crypto code. So here I use __uint128_t that
> > comes from GCC for 128-bit types. Should I rename existing type in crypto
> > and make core code for 128-bit variables consistent with u64, u32 etc? (I
> > think yes, but would like to ask crypto people for it.)
>
> Hmm, that file probably predates the __uint128_t support. My guess would
> be that the crypto code using it can actually benefit from the new types as
> well, so maybe move the existing file to include/linux/int128.h and add
> an #if/#else logic to it so we use 'typedef __uint128_t __u128' if that
> is available.

It sounds reasonable. But I worry about testing that changes. Hope,
crypto community will help with it.

> > * Some compilers don't support __uint128_t, so I protected all generic code
> > with config option HAVE_128BIT_ACCESS. I think it's OK, but...
>
> That would be nicely solved by using the #if/#else definition above.
>
> > * For 128-bit read/write functions I take suffix 'o', which means read/write
> > the octet of bytes. Is this name OK?
>
> Can't think of anything better. It's not an octet though, but 16 bytes
> ('q' is for quadword, meaning four 16-bit words in Intel terminology).

Ah, sure. Octet of words. Will change it.

> > * my mips-linux-gnu-gcc v6.3.0 doesn't support __uint128_t, and I
> > don't have other BE setup on hand, so BE case is formally not tested.
> > BE code for arm64 is looking well though.
>
> I've run it through my collection of compilers, it seems that most but not
> all 64-bit targets support it (exceptions appear to be older versions of
> gcc for s390x and parisc), and none of the 32-bit targets do:

Thanks for doing this test. Looking at this I realize that this is
not the architecture feature but compiler feature. So if we add
128-bit interface, it would be reasonable to add it for all targets
that compiled with toolchain supporting 128-bit accesses.

There's already the option ARCH_SUPPORTS_INT128 that is enabled for
x86_64 in arch/x86/Kconfig and conditionally enabled for arm64 in
arch/arm64/Makefile:
KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0500, -DCONFIG_ARCH_SUPPORTS_INT128)

It is used in include/linux/math64.h and in lib/ubsan.c, not so wide.

So I find things little messed. Crypto code ignores compilers' ability
to operate with 128-bit numbers. Ubsan and math64 relies on compiler
version (at least for arm64, and I doubt it would work correctly with clang).
And now I introduce HAVE_128BIT_ACCESS with the same meaning for memory
access.

I think it's time to unify 128-bit code:
- enable CONFIG_ARCH_SUPPORTS_INT128 if compiler supports it, ie check
it like you do below;
- declare u128 as structure or typedef depending on ARCH_SUPPORTS_INT128
in generic include/linux/int128.h, as you suggest here;
- switch this series to ARCH_SUPPORTS_INT128.

Does it sound reasonable?

Yury

> $ for i in /home/arnd/cross-gcc/bin/*gcc-[3-8]* ; do echo -n $i" " ;
> echo '__uint128_t v;' | $i -xc -S - -o /dev/null && echo ok ; done
> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-4.8.5 ok
> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-4.9.3 ok
> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-4.9.4 ok
> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-5.2.1 ok
> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-5.4.1 ok
> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-5.5.0 ok
> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-6.3.1 ok
> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-7.0.0 ok
> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-7.0.1 ok
> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-7.1.1 ok
> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-7.2.1 ok
> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-8.0.0 ok
> /home/arnd/cross-gcc/bin/alpha-linux-gcc-4.1.3 ok
> /home/arnd/cross-gcc/bin/alpha-linux-gcc-4.3.6 ok
> /home/arnd/cross-gcc/bin/alpha-linux-gcc-4.9.3 ok
> /home/arnd/cross-gcc/bin/alpha-linux-gcc-7.2.1 ok
> /home/arnd/cross-gcc/bin/am33_2.0-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/am33_2.0-linux-gcc-5.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/am33_2.0-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/arc-elf-gcc-7.2.1 <stdin>:1:1: error: unknown
> type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-3.4.6 ok
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.3.6 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.4.7 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.5.4 <stdin>:1:13:
> error: expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.6.4 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.7.4 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.8.5 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.0 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.1 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.2 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.3 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.4 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.0.0 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.1.1 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.2.1 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.3.1 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.4.1 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.5.0 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-6.0.0 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-6.1.1 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-6.3.1 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-7.0.0 <stdin>:1:1:
> error: unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-7.0.1 <stdin>:1:1:
> error: unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-7.1.1 <stdin>:1:1:
> error: unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-7.2.1 <stdin>:1:1:
> error: unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-8.0.0 <stdin>:1:1:
> error: unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/bfin-uclinux-gcc-7.0.0
> bfin-uclinux-gcc-7.0.0: error trying to exec 'cc1': execvp: No such
> file or directory
> /home/arnd/cross-gcc/bin/bfin-uclinux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/c6x-elf-gcc-7.2.1 <stdin>:1:1: error: unknown
> type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/cris-linux-gcc-4.1.3 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/cris-linux-gcc-4.3.6 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/cris-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/cris-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/frv-linux-gcc-4.1.3 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/frv-linux-gcc-4.3.6 <stdin>:1: internal
> compiler error: in default_secondary_reload, at targhooks.c:618
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See <http://gcc.gnu.org/bugs.html> for instructions.
> /home/arnd/cross-gcc/bin/frv-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/frv-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/h8300-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/hppa64-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/hppa64-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/hppa-linux-gcc-4.1.3 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/hppa-linux-gcc-4.3.6 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/hppa-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/hppa-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/i386-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/i386-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/ia64-linux-gcc-7.2.1 ok
> /home/arnd/cross-gcc/bin/m32r-linux-gcc-4.1.3 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/m32r-linux-gcc-4.3.6 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/m32r-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/m32r-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/m68k-linux-gcc-4.1.3 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/m68k-linux-gcc-4.3.6 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/m68k-linux-gcc-6.0.0 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/m68k-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/microblaze-linux-gcc-4.9.3 <stdin>:1:1:
> error: unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/microblaze-linux-gcc-7.2.1 <stdin>:1:1:
> error: unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/mips64-linux-gcc-4.9.3 ok
> /home/arnd/cross-gcc/bin/mips64-linux-gcc-7.2.1 ok
> /home/arnd/cross-gcc/bin/mips-linux-gcc-4.0.4 <stdin>:1: error: syntax
> error before 'v'
> <stdin>:1: warning: data definition has no type or storage class
> /home/arnd/cross-gcc/bin/mips-linux-gcc-4.1.3 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/mips-linux-gcc-4.3.6 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/mips-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/mips-linux-gcc-7.0.0 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/mips-linux-gcc-7.0.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/mips-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/nios2-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/parisc-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/powerpc64-linux-gcc-4.1.3 ok
> /home/arnd/cross-gcc/bin/powerpc64-linux-gcc-4.3.6 ok
> /home/arnd/cross-gcc/bin/powerpc64-linux-gcc-4.9.3 ok
> /home/arnd/cross-gcc/bin/powerpc64-linux-gcc-7.2.1 ok
> /home/arnd/cross-gcc/bin/powerpc-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/powerpc-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/riscv32-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/riscv64-linux-gcc-7.2.1 ok
> /home/arnd/cross-gcc/bin/s390-linux-gcc-4.1.3 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/s390-linux-gcc-4.3.6 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/s390-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/s390-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/sh2-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/sh3-linux-gcc-4.1.3 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/sh3-linux-gcc-4.3.6 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/sh3-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/sh4-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/sh-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/sparc64-linux-gcc-4.9.3 ok
> /home/arnd/cross-gcc/bin/sparc64-linux-gcc-7.2.1 ok
> /home/arnd/cross-gcc/bin/sparc-linux-gcc-4.1.3 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/sparc-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/sparc-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/tilegx-linux-gcc-7.2.1 ok
> /home/arnd/cross-gcc/bin/tilepro-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-3.4.6 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.0.4 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.1.3 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.2.5 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.3.6 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.4.7 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.5.4 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.6.4 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.7.4 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.8.5 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.9.3 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.9.4 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-5.4.1 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-5.5.0 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-6.1.1 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-6.3.1 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-7.0.0 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-7.0.1 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-7.1.1 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-7.2.1 ok
> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-8.0.0 ok
> /home/arnd/cross-gcc/bin/xtensa-linux-gcc-4.1.3 <stdin>:1: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
> /home/arnd/cross-gcc/bin/xtensa-linux-gcc-4.9.3 <stdin>:1:1: error:
> unknown type name '__uint128_t'
> /home/arnd/cross-gcc/bin/xtensa-linux-gcc-7.2.1 <stdin>:1:1: error:
> unknown type name '__uint128_t'; did you mean '__int128'?

2018-01-25 12:11:19

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] API for 128-bit IO access

On 25/01/18 11:38, Yury Norov wrote:
> On Wed, Jan 24, 2018 at 11:28:55AM +0100, Arnd Bergmann wrote:
>> On Wed, Jan 24, 2018 at 10:05 AM, Yury Norov <[email protected]> wrote:
>>> This series adds API for 128-bit memory IO access and enables it for ARM64.
>>> The original motivation for 128-bit API came from new Cavium network device
>>> driver. The hardware requires 128-bit access to make things work. See
>>> description in patch 3 for details.
>>
>> We might also want to do something similar to the
>> include/linux/io-64-nonatomic-lo-hi.h
>> and hi-lo.h files, to simulate 128-bit access using pairs of 64-bit access on
>> other targets. It's apparently driver specific which half you need to do first
>> to make it work, so we need both.
>
> OK, will do.
>
>>> Also, starting from ARMv8.4, stp and ldp instructions become atomic, and
>>> API for 128-bit access would be helpful in core arm64 code.
>>>
>>> This series is RFC. I'd like to collect opinions on idea and implementation
>>> details.
>>> * I didn't implement all 128-bit operations existing for 64-bit variables
>>> and other types (__swab128p etc). Do we need them all right now, or we
>>> can add them when actually needed?
>>
>> I think in this case it's better to do them all at once.
>
> Ack.
>
>>> * u128 name is already used in crypto code. So here I use __uint128_t that
>>> comes from GCC for 128-bit types. Should I rename existing type in crypto
>>> and make core code for 128-bit variables consistent with u64, u32 etc? (I
>>> think yes, but would like to ask crypto people for it.)
>>
>> Hmm, that file probably predates the __uint128_t support. My guess would
>> be that the crypto code using it can actually benefit from the new types as
>> well, so maybe move the existing file to include/linux/int128.h and add
>> an #if/#else logic to it so we use 'typedef __uint128_t __u128' if that
>> is available.
>
> It sounds reasonable. But I worry about testing that changes. Hope,
> crypto community will help with it.
>
>>> * Some compilers don't support __uint128_t, so I protected all generic code
>>> with config option HAVE_128BIT_ACCESS. I think it's OK, but...
>>
>> That would be nicely solved by using the #if/#else definition above.
>>
>>> * For 128-bit read/write functions I take suffix 'o', which means read/write
>>> the octet of bytes. Is this name OK?
>>
>> Can't think of anything better. It's not an octet though, but 16 bytes
>> ('q' is for quadword, meaning four 16-bit words in Intel terminology).
>
> Ah, sure. Octet of words. Will change it.
>
>>> * my mips-linux-gnu-gcc v6.3.0 doesn't support __uint128_t, and I
>>> don't have other BE setup on hand, so BE case is formally not tested.
>>> BE code for arm64 is looking well though.
>>
>> I've run it through my collection of compilers, it seems that most but not
>> all 64-bit targets support it (exceptions appear to be older versions of
>> gcc for s390x and parisc), and none of the 32-bit targets do:
>
> Thanks for doing this test. Looking at this I realize that this is
> not the architecture feature but compiler feature. So if we add
> 128-bit interface, it would be reasonable to add it for all targets
> that compiled with toolchain supporting 128-bit accesses.
>
> There's already the option ARCH_SUPPORTS_INT128 that is enabled for
> x86_64 in arch/x86/Kconfig and conditionally enabled for arm64 in
> arch/arm64/Makefile:
> KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0500, -DCONFIG_ARCH_SUPPORTS_INT128)
>
> It is used in include/linux/math64.h and in lib/ubsan.c, not so wide.
>
> So I find things little messed. Crypto code ignores compilers' ability
> to operate with 128-bit numbers. Ubsan and math64 relies on compiler
> version (at least for arm64, and I doubt it would work correctly with clang).
> And now I introduce HAVE_128BIT_ACCESS with the same meaning for memory
> access.
>
> I think it's time to unify 128-bit code:
> - enable CONFIG_ARCH_SUPPORTS_INT128 if compiler supports it, ie check
> it like you do below;
> - declare u128 as structure or typedef depending on ARCH_SUPPORTS_INT128
> in generic include/linux/int128.h, as you suggest here;
> - switch this series to ARCH_SUPPORTS_INT128.
>
> Does it sound reasonable?

It probably is about time to formalise the current scattered fragments
of uint_128_t support, but to reiterate Will's comment it is almost
certainly not worth the effort to implement 'generic' I/O accessors
which only work under implementation-defined and undiscoverable hardware
conditions, and will be unusable on the overwhelming majority of
systems. Just open-code LDP/STP accessors in the one driver which needs
them and (by definition) only runs on SoCs where they *are* known to
work correctly.

Robin.

>
> Yury
>
>> $ for i in /home/arnd/cross-gcc/bin/*gcc-[3-8]* ; do echo -n $i" " ;
>> echo '__uint128_t v;' | $i -xc -S - -o /dev/null && echo ok ; done
>> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-4.8.5 ok
>> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-4.9.3 ok
>> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-4.9.4 ok
>> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-5.2.1 ok
>> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-5.4.1 ok
>> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-5.5.0 ok
>> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-6.3.1 ok
>> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-7.0.0 ok
>> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-7.0.1 ok
>> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-7.1.1 ok
>> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-7.2.1 ok
>> /home/arnd/cross-gcc/bin/aarch64-linux-gcc-8.0.0 ok
>> /home/arnd/cross-gcc/bin/alpha-linux-gcc-4.1.3 ok
>> /home/arnd/cross-gcc/bin/alpha-linux-gcc-4.3.6 ok
>> /home/arnd/cross-gcc/bin/alpha-linux-gcc-4.9.3 ok
>> /home/arnd/cross-gcc/bin/alpha-linux-gcc-7.2.1 ok
>> /home/arnd/cross-gcc/bin/am33_2.0-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/am33_2.0-linux-gcc-5.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/am33_2.0-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/arc-elf-gcc-7.2.1 <stdin>:1:1: error: unknown
>> type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-3.4.6 ok
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.3.6 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.4.7 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.5.4 <stdin>:1:13:
>> error: expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.6.4 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.7.4 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.8.5 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.0 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.1 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.2 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.3 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-4.9.4 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.0.0 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.1.1 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.2.1 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.3.1 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.4.1 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-5.5.0 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-6.0.0 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-6.1.1 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-6.3.1 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-7.0.0 <stdin>:1:1:
>> error: unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-7.0.1 <stdin>:1:1:
>> error: unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-7.1.1 <stdin>:1:1:
>> error: unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-7.2.1 <stdin>:1:1:
>> error: unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/arm-linux-gnueabi-gcc-8.0.0 <stdin>:1:1:
>> error: unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/bfin-uclinux-gcc-7.0.0
>> bfin-uclinux-gcc-7.0.0: error trying to exec 'cc1': execvp: No such
>> file or directory
>> /home/arnd/cross-gcc/bin/bfin-uclinux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/c6x-elf-gcc-7.2.1 <stdin>:1:1: error: unknown
>> type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/cris-linux-gcc-4.1.3 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/cris-linux-gcc-4.3.6 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/cris-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/cris-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/frv-linux-gcc-4.1.3 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/frv-linux-gcc-4.3.6 <stdin>:1: internal
>> compiler error: in default_secondary_reload, at targhooks.c:618
>> Please submit a full bug report,
>> with preprocessed source if appropriate.
>> See <http://gcc.gnu.org/bugs.html> for instructions.
>> /home/arnd/cross-gcc/bin/frv-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/frv-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/h8300-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/hppa64-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/hppa64-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/hppa-linux-gcc-4.1.3 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/hppa-linux-gcc-4.3.6 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/hppa-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/hppa-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/i386-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/i386-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/ia64-linux-gcc-7.2.1 ok
>> /home/arnd/cross-gcc/bin/m32r-linux-gcc-4.1.3 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/m32r-linux-gcc-4.3.6 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/m32r-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/m32r-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/m68k-linux-gcc-4.1.3 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/m68k-linux-gcc-4.3.6 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/m68k-linux-gcc-6.0.0 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/m68k-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/microblaze-linux-gcc-4.9.3 <stdin>:1:1:
>> error: unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/microblaze-linux-gcc-7.2.1 <stdin>:1:1:
>> error: unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/mips64-linux-gcc-4.9.3 ok
>> /home/arnd/cross-gcc/bin/mips64-linux-gcc-7.2.1 ok
>> /home/arnd/cross-gcc/bin/mips-linux-gcc-4.0.4 <stdin>:1: error: syntax
>> error before 'v'
>> <stdin>:1: warning: data definition has no type or storage class
>> /home/arnd/cross-gcc/bin/mips-linux-gcc-4.1.3 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/mips-linux-gcc-4.3.6 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/mips-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/mips-linux-gcc-7.0.0 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/mips-linux-gcc-7.0.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/mips-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/nios2-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/parisc-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/powerpc64-linux-gcc-4.1.3 ok
>> /home/arnd/cross-gcc/bin/powerpc64-linux-gcc-4.3.6 ok
>> /home/arnd/cross-gcc/bin/powerpc64-linux-gcc-4.9.3 ok
>> /home/arnd/cross-gcc/bin/powerpc64-linux-gcc-7.2.1 ok
>> /home/arnd/cross-gcc/bin/powerpc-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/powerpc-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/riscv32-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/riscv64-linux-gcc-7.2.1 ok
>> /home/arnd/cross-gcc/bin/s390-linux-gcc-4.1.3 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/s390-linux-gcc-4.3.6 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/s390-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/s390-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/sh2-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/sh3-linux-gcc-4.1.3 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/sh3-linux-gcc-4.3.6 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/sh3-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/sh4-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/sh-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/sparc64-linux-gcc-4.9.3 ok
>> /home/arnd/cross-gcc/bin/sparc64-linux-gcc-7.2.1 ok
>> /home/arnd/cross-gcc/bin/sparc-linux-gcc-4.1.3 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/sparc-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/sparc-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/tilegx-linux-gcc-7.2.1 ok
>> /home/arnd/cross-gcc/bin/tilepro-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-3.4.6 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.0.4 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.1.3 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.2.5 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.3.6 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.4.7 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.5.4 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.6.4 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.7.4 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.8.5 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.9.3 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-4.9.4 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-5.4.1 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-5.5.0 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-6.1.1 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-6.3.1 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-7.0.0 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-7.0.1 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-7.1.1 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-7.2.1 ok
>> /home/arnd/cross-gcc/bin/x86_64-linux-gcc-8.0.0 ok
>> /home/arnd/cross-gcc/bin/xtensa-linux-gcc-4.1.3 <stdin>:1: error:
>> expected '=', ',', ';', 'asm' or '__attribute__' before 'v'
>> /home/arnd/cross-gcc/bin/xtensa-linux-gcc-4.9.3 <stdin>:1:1: error:
>> unknown type name '__uint128_t'
>> /home/arnd/cross-gcc/bin/xtensa-linux-gcc-7.2.1 <stdin>:1:1: error:
>> unknown type name '__uint128_t'; did you mean '__int128'?
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

2018-01-25 13:59:34

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] API for 128-bit IO access

On Thu, Jan 25, 2018 at 12:38 PM, Yury Norov <[email protected]> wrote:
> On Wed, Jan 24, 2018 at 11:28:55AM +0100, Arnd Bergmann wrote:
>> On Wed, Jan 24, 2018 at 10:05 AM, Yury Norov <[email protected]> wrote:

> Thanks for doing this test. Looking at this I realize that this is
> not the architecture feature but compiler feature. So if we add
> 128-bit interface, it would be reasonable to add it for all targets
> that compiled with toolchain supporting 128-bit accesses.
>
> There's already the option ARCH_SUPPORTS_INT128 that is enabled for
> x86_64 in arch/x86/Kconfig and conditionally enabled for arm64 in
> arch/arm64/Makefile:
> KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0500, -DCONFIG_ARCH_SUPPORTS_INT128)
>
> It is used in include/linux/math64.h and in lib/ubsan.c, not so wide.
>
> So I find things little messed. Crypto code ignores compilers' ability
> to operate with 128-bit numbers. Ubsan and math64 relies on compiler
> version (at least for arm64, and I doubt it would work correctly with clang).
> And now I introduce HAVE_128BIT_ACCESS with the same meaning for memory
> access.
>
> I think it's time to unify 128-bit code:
> - enable CONFIG_ARCH_SUPPORTS_INT128 if compiler supports it, ie check
> it like you do below;
> - declare u128 as structure or typedef depending on ARCH_SUPPORTS_INT128
> in generic include/linux/int128.h, as you suggest here;
> - switch this series to ARCH_SUPPORTS_INT128.
>
> Does it sound reasonable?

The CONFIG_* symbol namespace should not be set dynamically. However, you
can define a symbol with another name, e.g. ARCH_SUPPORTS_INT128 (without
CONFIG_ prefix) in linux/compiler-gcc.h based on the version and BITS_PER_LONG.

Arnd

2018-01-26 09:05:42

by Yury Norov

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] API for 128-bit IO access

On Wed, Jan 24, 2018 at 10:22:13AM +0000, Will Deacon wrote:
> On Wed, Jan 24, 2018 at 12:05:16PM +0300, Yury Norov wrote:
> > This series adds API for 128-bit memory IO access and enables it for ARM64.
> > The original motivation for 128-bit API came from new Cavium network device
> > driver. The hardware requires 128-bit access to make things work. See
> > description in patch 3 for details.
> >
> > Also, starting from ARMv8.4, stp and ldp instructions become atomic, and
> > API for 128-bit access would be helpful in core arm64 code.
>
> Only for normal, cacheable memory, so they're not suitable for IO accesses
> as you're proposing here.

Hi Will,

Thanks for clarification.

Could you elaborate, do you find 128-bit read/write API useless, or
you just correct my comment?

I think that ordered uniform 128-bit access API would be helpful, even
if not atomic.

Yury.

2018-01-26 18:11:51

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] API for 128-bit IO access

On Fri, Jan 26, 2018 at 12:05:42PM +0300, Yury Norov wrote:
> On Wed, Jan 24, 2018 at 10:22:13AM +0000, Will Deacon wrote:
> > On Wed, Jan 24, 2018 at 12:05:16PM +0300, Yury Norov wrote:
> > > This series adds API for 128-bit memory IO access and enables it for ARM64.
> > > The original motivation for 128-bit API came from new Cavium network device
> > > driver. The hardware requires 128-bit access to make things work. See
> > > description in patch 3 for details.
> > >
> > > Also, starting from ARMv8.4, stp and ldp instructions become atomic, and
> > > API for 128-bit access would be helpful in core arm64 code.
> >
> > Only for normal, cacheable memory, so they're not suitable for IO accesses
> > as you're proposing here.
>
> Hi Will,
>
> Thanks for clarification.
>
> Could you elaborate, do you find 128-bit read/write API useless, or
> you just correct my comment?
>
> I think that ordered uniform 128-bit access API would be helpful, even
> if not atomic.

Sorry, but I strongly disagree here. Having an IO accessor that isn't
guaranteed to be atomic is a recipe for disaster if it's not called out
explicitly. You're much better off implementing something along the lines
of <linux/io-128-nonatomic-hi-lo.h> using 2x64-bit accessors like we already
have for the 2x32-bit case.

However, that doesn't solve your problem and is somewhat of a distraction.
I'd suggest that in your case, where you have a device that relies on
128-bit atomic access that is assumedly tightly integrated into your SoC,
then the driver just codes it's own local implementation of the accessor,
given that there isn't a way to guarantee the atomicity architecturally
(and even within your SoC it might not be atomic to all endpoints).

Will

2018-01-29 10:25:58

by Yury Norov

[permalink] [raw]
Subject: Re: [PATCH RFC 0/3] API for 128-bit IO access

On Fri, Jan 26, 2018 at 06:11:49PM +0000, Will Deacon wrote:
> On Fri, Jan 26, 2018 at 12:05:42PM +0300, Yury Norov wrote:
> > On Wed, Jan 24, 2018 at 10:22:13AM +0000, Will Deacon wrote:
> > > On Wed, Jan 24, 2018 at 12:05:16PM +0300, Yury Norov wrote:
> > > > This series adds API for 128-bit memory IO access and enables it for ARM64.
> > > > The original motivation for 128-bit API came from new Cavium network device
> > > > driver. The hardware requires 128-bit access to make things work. See
> > > > description in patch 3 for details.
> > > >
> > > > Also, starting from ARMv8.4, stp and ldp instructions become atomic, and
> > > > API for 128-bit access would be helpful in core arm64 code.
> > >
> > > Only for normal, cacheable memory, so they're not suitable for IO accesses
> > > as you're proposing here.
> >
> > Hi Will,
> >
> > Thanks for clarification.
> >
> > Could you elaborate, do you find 128-bit read/write API useless, or
> > you just correct my comment?
> >
> > I think that ordered uniform 128-bit access API would be helpful, even
> > if not atomic.
>
> Sorry, but I strongly disagree here. Having an IO accessor that isn't
> guaranteed to be atomic is a recipe for disaster if it's not called out
> explicitly. You're much better off implementing something along the lines
> of <linux/io-128-nonatomic-hi-lo.h> using 2x64-bit accessors like we already
> have for the 2x32-bit case.
>
> However, that doesn't solve your problem and is somewhat of a distraction.
> I'd suggest that in your case, where you have a device that relies on
> 128-bit atomic access that is assumedly tightly integrated into your SoC,
> then the driver just codes it's own local implementation of the accessor,
> given that there isn't a way to guarantee the atomicity architecturally
> (and even within your SoC it might not be atomic to all endpoints).

OK. Understand that. So we'll drop this RFC and implement those accessors
in driver.

Thank you and all for the review. Maybe later I'll submit 128-bit unification
patch that was discussed here.

Yury