From 0652035a57945e14e611dafae2ec5b46a05bc1d1 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Sat, 8 May 2021 00:07:51 +0200 Subject: asm-generic: unaligned: remove byteshift helpers In theory, compilers should be able to work this out themselves so we can use a simpler version based on the swab() helpers. I have verified that this works on all supported compiler versions (gcc-4.9 and up, clang-10 and up). Looking at the object code produced by gcc-11, I found that the impact is mostly a change in inlining decisions that lead to slightly larger code. In other cases, this version produces explicit byte swaps in place of separate byte access, or comparing against pre-swapped constants. While the source code is clearly simpler, I have not seen an indication of the new version actually producing better code on Arm, so maybe we want to skip this after all. From what I can tell, gcc recognizes the byteswap pattern in the byteshift.h header and can turn it into explicit instructions, but it does not turn a __builtin_bswap32() back into individual bytes when that would result in better output, e.g. when storing a byte-reversed constant. Suggested-by: Linus Torvalds Signed-off-by: Arnd Bergmann --- include/asm-generic/unaligned.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'include/asm-generic') diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h index 374c940e9be1..d79df721ae60 100644 --- a/include/asm-generic/unaligned.h +++ b/include/asm-generic/unaligned.h @@ -16,7 +16,6 @@ #if defined(__LITTLE_ENDIAN) # ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS # include -# include # endif # include # define get_unaligned __get_unaligned_le @@ -24,7 +23,6 @@ #elif defined(__BIG_ENDIAN) # ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS # include -# include # endif # include # define get_unaligned __get_unaligned_be -- cgit v1.2.3 From 778aaefb8e864fc61f850539ea479554dd4caea1 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Sat, 8 May 2021 00:07:52 +0200 Subject: asm-generic: unaligned always use struct helpers As found by Vineet Gupta and Linus Torvalds, gcc has somewhat unexpected behavior when faced with overlapping unaligned pointers. The kernel's unaligned/access-ok.h header technically invokes undefined behavior that happens to usually work on the architectures using it, but if the compiler optimizes code based on the assumption that undefined behavior doesn't happen, it can create output that actually causes data corruption. A related problem was previously found on 32-bit ARMv7, where most instructions can be used on unaligned data, but 64-bit ldrd/strd causes an exception. The workaround was to always use the unaligned/le_struct.h helper instead of unaligned/access-ok.h, in commit 1cce91dfc8f7 ("ARM: 8715/1: add a private asm/unaligned.h"). The same solution should work on all other architectures as well, so remove the access-ok.h variant and use the other one unconditionally on all architectures, picking either the big-endian or little-endian version. With this, the arm specific header can be removed as well, and the only file including linux/unaligned/access_ok.h gets moved to including the normal file. Fortunately, this made almost no difference to the object code produced by gcc-11. On x86, s390, powerpc, and arc, the resulting binary appears to be identical to the previous version, while on arm64 and m68k there are minimal differences that looks like an optimization pass went into a different direction, usually using fewer stack spills on the new version. Signed-off-by: Arnd Bergmann Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100363 --- arch/arm/include/asm/unaligned.h | 25 -------------- arch/mips/crypto/crc32-mips.c | 2 +- include/asm-generic/unaligned.h | 13 ++----- include/linux/unaligned/access_ok.h | 68 ------------------------------------- 4 files changed, 3 insertions(+), 105 deletions(-) delete mode 100644 arch/arm/include/asm/unaligned.h delete mode 100644 include/linux/unaligned/access_ok.h (limited to 'include/asm-generic') diff --git a/arch/arm/include/asm/unaligned.h b/arch/arm/include/asm/unaligned.h deleted file mode 100644 index 3c5248fb4cdc..000000000000 --- a/arch/arm/include/asm/unaligned.h +++ /dev/null @@ -1,25 +0,0 @@ -#ifndef __ASM_ARM_UNALIGNED_H -#define __ASM_ARM_UNALIGNED_H - -/* - * We generally want to set CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS on ARMv6+, - * but we don't want to use linux/unaligned/access_ok.h since that can lead - * to traps on unaligned stm/ldm or strd/ldrd. - */ -#include - -#if defined(__LITTLE_ENDIAN) -# include -# include -# define get_unaligned __get_unaligned_le -# define put_unaligned __put_unaligned_le -#elif defined(__BIG_ENDIAN) -# include -# include -# define get_unaligned __get_unaligned_be -# define put_unaligned __put_unaligned_be -#else -# error need to define endianess -#endif - -#endif /* __ASM_ARM_UNALIGNED_H */ diff --git a/arch/mips/crypto/crc32-mips.c b/arch/mips/crypto/crc32-mips.c index faa88a6a74c0..0a03529cf317 100644 --- a/arch/mips/crypto/crc32-mips.c +++ b/arch/mips/crypto/crc32-mips.c @@ -8,13 +8,13 @@ * Copyright (C) 2018 MIPS Tech, LLC */ -#include #include #include #include #include #include #include +#include #include diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h index d79df721ae60..36bf03aaa674 100644 --- a/include/asm-generic/unaligned.h +++ b/include/asm-generic/unaligned.h @@ -8,22 +8,13 @@ */ #include -/* Set by the arch if it can handle unaligned accesses in hardware. */ -#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS -# include -#endif - #if defined(__LITTLE_ENDIAN) -# ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS -# include -# endif +# include # include # define get_unaligned __get_unaligned_le # define put_unaligned __put_unaligned_le #elif defined(__BIG_ENDIAN) -# ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS -# include -# endif +# include # include # define get_unaligned __get_unaligned_be # define put_unaligned __put_unaligned_be diff --git a/include/linux/unaligned/access_ok.h b/include/linux/unaligned/access_ok.h deleted file mode 100644 index 167aa849c0ce..000000000000 --- a/include/linux/unaligned/access_ok.h +++ /dev/null @@ -1,68 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -#ifndef _LINUX_UNALIGNED_ACCESS_OK_H -#define _LINUX_UNALIGNED_ACCESS_OK_H - -#include -#include - -static __always_inline u16 get_unaligned_le16(const void *p) -{ - return le16_to_cpup((__le16 *)p); -} - -static __always_inline u32 get_unaligned_le32(const void *p) -{ - return le32_to_cpup((__le32 *)p); -} - -static __always_inline u64 get_unaligned_le64(const void *p) -{ - return le64_to_cpup((__le64 *)p); -} - -static __always_inline u16 get_unaligned_be16(const void *p) -{ - return be16_to_cpup((__be16 *)p); -} - -static __always_inline u32 get_unaligned_be32(const void *p) -{ - return be32_to_cpup((__be32 *)p); -} - -static __always_inline u64 get_unaligned_be64(const void *p) -{ - return be64_to_cpup((__be64 *)p); -} - -static __always_inline void put_unaligned_le16(u16 val, void *p) -{ - *((__le16 *)p) = cpu_to_le16(val); -} - -static __always_inline void put_unaligned_le32(u32 val, void *p) -{ - *((__le32 *)p) = cpu_to_le32(val); -} - -static __always_inline void put_unaligned_le64(u64 val, void *p) -{ - *((__le64 *)p) = cpu_to_le64(val); -} - -static __always_inline void put_unaligned_be16(u16 val, void *p) -{ - *((__be16 *)p) = cpu_to_be16(val); -} - -static __always_inline void put_unaligned_be32(u32 val, void *p) -{ - *((__be32 *)p) = cpu_to_be32(val); -} - -static __always_inline void put_unaligned_be64(u64 val, void *p) -{ - *((__be64 *)p) = cpu_to_be64(val); -} - -#endif /* _LINUX_UNALIGNED_ACCESS_OK_H */ -- cgit v1.2.3 From d40d8179482c330df5b9049797fe94c2e8eb4f6e Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Sat, 8 May 2021 14:58:46 +0200 Subject: asm-generic: uaccess: 1-byte access is always aligned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the cleaned up version of asm-generic/unaligned.h, there is a warning about the get_user/put_user helpers using unaligned access for single-byte variables: include/asm-generic/uaccess.h: In function ‘__get_user_fn’: include/asm-generic/unaligned.h:13:15: warning: ‘packed’ attribute ignored for field of type ‘u8’ {aka ‘unsigned char’} [-Wattributes] const struct { type x __packed; } *__pptr = (typeof(__pptr))(ptr); \ Change these to use a direct pointer dereference to avoid the warnings. Signed-off-by: Arnd Bergmann --- include/asm-generic/uaccess.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'include/asm-generic') diff --git a/include/asm-generic/uaccess.h b/include/asm-generic/uaccess.h index 4973328f3c6e..a0b2f270dddc 100644 --- a/include/asm-generic/uaccess.h +++ b/include/asm-generic/uaccess.h @@ -19,7 +19,7 @@ __get_user_fn(size_t size, const void __user *from, void *to) switch (size) { case 1: - *(u8 *)to = get_unaligned((u8 __force *)from); + *(u8 *)to = *((u8 __force *)from); return 0; case 2: *(u16 *)to = get_unaligned((u16 __force *)from); @@ -45,7 +45,7 @@ __put_user_fn(size_t size, void __user *to, void *from) switch (size) { case 1: - put_unaligned(*(u8 *)from, (u8 __force *)to); + *(u8 __force *)to = *(u8 *)from; return 0; case 2: put_unaligned(*(u16 *)from, (u16 __force *)to); -- cgit v1.2.3 From 803f4e1eab7a8938ba3a3c30dd4eb5e9eeef5e63 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Sat, 8 May 2021 00:07:57 +0200 Subject: asm-generic: simplify asm/unaligned.h The get_unaligned()/put_unaligned() implementations are much more complex than necessary, now that all architectures use the same code. Move everything into one file and use a much more compact way to express the same logic. I've compared the binary output using gcc-11 across defconfig builds for all architectures and found this patch to make no difference, except for a single function on powerpc that needs two additional register moves because of random differences in register allocation. There are a handful of callers of the low-level __get_unaligned_cpu32, so leave that in place for the time being even though the common code no longer uses it. This adds a warning for any caller of get_unaligned()/put_unaligned() that passes in a single-byte pointer, but I've sent patches for all instances that show up in x86 and randconfig builds. It would be nice to change the arguments of the endian-specific accessors to take the matching __be16/__be32/__be64/__le16/__le32/__le64 arguments instead of a void pointer, but that requires more changes to the rest of the kernel. This new version does allow aggregate types into get_unaligned(), which was not the original goal but might come in handy. Signed-off-by: Arnd Bergmann --- include/asm-generic/unaligned.h | 130 ++++++++++++++++++++++++++++++++---- include/linux/unaligned/be_struct.h | 67 ------------------- include/linux/unaligned/generic.h | 115 ------------------------------- include/linux/unaligned/le_struct.h | 67 ------------------- 4 files changed, 117 insertions(+), 262 deletions(-) delete mode 100644 include/linux/unaligned/be_struct.h delete mode 100644 include/linux/unaligned/generic.h delete mode 100644 include/linux/unaligned/le_struct.h (limited to 'include/asm-generic') diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h index 36bf03aaa674..1c4242416c9f 100644 --- a/include/asm-generic/unaligned.h +++ b/include/asm-generic/unaligned.h @@ -6,20 +6,124 @@ * This is the most generic implementation of unaligned accesses * and should work almost anywhere. */ +#include #include -#if defined(__LITTLE_ENDIAN) -# include -# include -# define get_unaligned __get_unaligned_le -# define put_unaligned __put_unaligned_le -#elif defined(__BIG_ENDIAN) -# include -# include -# define get_unaligned __get_unaligned_be -# define put_unaligned __put_unaligned_be -#else -# error need to define endianess -#endif +#define __get_unaligned_t(type, ptr) ({ \ + const struct { type x; } __packed *__pptr = (typeof(__pptr))(ptr); \ + __pptr->x; \ +}) + +#define __put_unaligned_t(type, val, ptr) do { \ + struct { type x; } __packed *__pptr = (typeof(__pptr))(ptr); \ + __pptr->x = (val); \ +} while (0) + +#define get_unaligned(ptr) __get_unaligned_t(typeof(*(ptr)), (ptr)) +#define put_unaligned(val, ptr) __put_unaligned_t(typeof(*(ptr)), (val), (ptr)) + +static inline u16 get_unaligned_le16(const void *p) +{ + return le16_to_cpu(__get_unaligned_t(__le16, p)); +} + +static inline u32 get_unaligned_le32(const void *p) +{ + return le32_to_cpu(__get_unaligned_t(__le32, p)); +} + +static inline u64 get_unaligned_le64(const void *p) +{ + return le64_to_cpu(__get_unaligned_t(__le64, p)); +} + +static inline void put_unaligned_le16(u16 val, void *p) +{ + __put_unaligned_t(__le16, cpu_to_le16(val), p); +} + +static inline void put_unaligned_le32(u32 val, void *p) +{ + __put_unaligned_t(__le32, cpu_to_le32(val), p); +} + +static inline void put_unaligned_le64(u64 val, void *p) +{ + __put_unaligned_t(__le64, cpu_to_le64(val), p); +} + +static inline u16 get_unaligned_be16(const void *p) +{ + return be16_to_cpu(__get_unaligned_t(__be16, p)); +} + +static inline u32 get_unaligned_be32(const void *p) +{ + return be32_to_cpu(__get_unaligned_t(__be32, p)); +} + +static inline u64 get_unaligned_be64(const void *p) +{ + return be64_to_cpu(__get_unaligned_t(__be64, p)); +} + +static inline void put_unaligned_be16(u16 val, void *p) +{ + __put_unaligned_t(__be16, cpu_to_be16(val), p); +} + +static inline void put_unaligned_be32(u32 val, void *p) +{ + __put_unaligned_t(__be32, cpu_to_be32(val), p); +} + +static inline void put_unaligned_be64(u64 val, void *p) +{ + __put_unaligned_t(__be64, cpu_to_be64(val), p); +} + +static inline u32 __get_unaligned_be24(const u8 *p) +{ + return p[0] << 16 | p[1] << 8 | p[2]; +} + +static inline u32 get_unaligned_be24(const void *p) +{ + return __get_unaligned_be24(p); +} + +static inline u32 __get_unaligned_le24(const u8 *p) +{ + return p[0] | p[1] << 8 | p[2] << 16; +} + +static inline u32 get_unaligned_le24(const void *p) +{ + return __get_unaligned_le24(p); +} + +static inline void __put_unaligned_be24(const u32 val, u8 *p) +{ + *p++ = val >> 16; + *p++ = val >> 8; + *p++ = val; +} + +static inline void put_unaligned_be24(const u32 val, void *p) +{ + __put_unaligned_be24(val, p); +} + +static inline void __put_unaligned_le24(const u32 val, u8 *p) +{ + *p++ = val; + *p++ = val >> 8; + *p++ = val >> 16; +} + +static inline void put_unaligned_le24(const u32 val, void *p) +{ + __put_unaligned_le24(val, p); +} #endif /* __ASM_GENERIC_UNALIGNED_H */ diff --git a/include/linux/unaligned/be_struct.h b/include/linux/unaligned/be_struct.h deleted file mode 100644 index 76d9fe297c33..000000000000 --- a/include/linux/unaligned/be_struct.h +++ /dev/null @@ -1,67 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -#ifndef _LINUX_UNALIGNED_BE_STRUCT_H -#define _LINUX_UNALIGNED_BE_STRUCT_H - -#include - -static inline u16 get_unaligned_be16(const void *p) -{ - return __get_unaligned_cpu16((const u8 *)p); -} - -static inline u32 get_unaligned_be32(const void *p) -{ - return __get_unaligned_cpu32((const u8 *)p); -} - -static inline u64 get_unaligned_be64(const void *p) -{ - return __get_unaligned_cpu64((const u8 *)p); -} - -static inline void put_unaligned_be16(u16 val, void *p) -{ - __put_unaligned_cpu16(val, p); -} - -static inline void put_unaligned_be32(u32 val, void *p) -{ - __put_unaligned_cpu32(val, p); -} - -static inline void put_unaligned_be64(u64 val, void *p) -{ - __put_unaligned_cpu64(val, p); -} - -static inline u16 get_unaligned_le16(const void *p) -{ - return swab16(__get_unaligned_cpu16((const u8 *)p)); -} - -static inline u32 get_unaligned_le32(const void *p) -{ - return swab32(__get_unaligned_cpu32((const u8 *)p)); -} - -static inline u64 get_unaligned_le64(const void *p) -{ - return swab64(__get_unaligned_cpu64((const u8 *)p)); -} - -static inline void put_unaligned_le16(u16 val, void *p) -{ - __put_unaligned_cpu16(swab16(val), p); -} - -static inline void put_unaligned_le32(u32 val, void *p) -{ - __put_unaligned_cpu32(swab32(val), p); -} - -static inline void put_unaligned_le64(u64 val, void *p) -{ - __put_unaligned_cpu64(swab64(val), p); -} - -#endif /* _LINUX_UNALIGNED_BE_STRUCT_H */ diff --git a/include/linux/unaligned/generic.h b/include/linux/unaligned/generic.h deleted file mode 100644 index 303289492859..000000000000 --- a/include/linux/unaligned/generic.h +++ /dev/null @@ -1,115 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -#ifndef _LINUX_UNALIGNED_GENERIC_H -#define _LINUX_UNALIGNED_GENERIC_H - -#include - -/* - * Cause a link-time error if we try an unaligned access other than - * 1,2,4 or 8 bytes long - */ -extern void __bad_unaligned_access_size(void); - -#define __get_unaligned_le(ptr) ((__force typeof(*(ptr)))({ \ - __builtin_choose_expr(sizeof(*(ptr)) == 1, *(ptr), \ - __builtin_choose_expr(sizeof(*(ptr)) == 2, get_unaligned_le16((ptr)), \ - __builtin_choose_expr(sizeof(*(ptr)) == 4, get_unaligned_le32((ptr)), \ - __builtin_choose_expr(sizeof(*(ptr)) == 8, get_unaligned_le64((ptr)), \ - __bad_unaligned_access_size())))); \ - })) - -#define __get_unaligned_be(ptr) ((__force typeof(*(ptr)))({ \ - __builtin_choose_expr(sizeof(*(ptr)) == 1, *(ptr), \ - __builtin_choose_expr(sizeof(*(ptr)) == 2, get_unaligned_be16((ptr)), \ - __builtin_choose_expr(sizeof(*(ptr)) == 4, get_unaligned_be32((ptr)), \ - __builtin_choose_expr(sizeof(*(ptr)) == 8, get_unaligned_be64((ptr)), \ - __bad_unaligned_access_size())))); \ - })) - -#define __put_unaligned_le(val, ptr) ({ \ - void *__gu_p = (ptr); \ - switch (sizeof(*(ptr))) { \ - case 1: \ - *(u8 *)__gu_p = (__force u8)(val); \ - break; \ - case 2: \ - put_unaligned_le16((__force u16)(val), __gu_p); \ - break; \ - case 4: \ - put_unaligned_le32((__force u32)(val), __gu_p); \ - break; \ - case 8: \ - put_unaligned_le64((__force u64)(val), __gu_p); \ - break; \ - default: \ - __bad_unaligned_access_size(); \ - break; \ - } \ - (void)0; }) - -#define __put_unaligned_be(val, ptr) ({ \ - void *__gu_p = (ptr); \ - switch (sizeof(*(ptr))) { \ - case 1: \ - *(u8 *)__gu_p = (__force u8)(val); \ - break; \ - case 2: \ - put_unaligned_be16((__force u16)(val), __gu_p); \ - break; \ - case 4: \ - put_unaligned_be32((__force u32)(val), __gu_p); \ - break; \ - case 8: \ - put_unaligned_be64((__force u64)(val), __gu_p); \ - break; \ - default: \ - __bad_unaligned_access_size(); \ - break; \ - } \ - (void)0; }) - -static inline u32 __get_unaligned_be24(const u8 *p) -{ - return p[0] << 16 | p[1] << 8 | p[2]; -} - -static inline u32 get_unaligned_be24(const void *p) -{ - return __get_unaligned_be24(p); -} - -static inline u32 __get_unaligned_le24(const u8 *p) -{ - return p[0] | p[1] << 8 | p[2] << 16; -} - -static inline u32 get_unaligned_le24(const void *p) -{ - return __get_unaligned_le24(p); -} - -static inline void __put_unaligned_be24(const u32 val, u8 *p) -{ - *p++ = val >> 16; - *p++ = val >> 8; - *p++ = val; -} - -static inline void put_unaligned_be24(const u32 val, void *p) -{ - __put_unaligned_be24(val, p); -} - -static inline void __put_unaligned_le24(const u32 val, u8 *p) -{ - *p++ = val; - *p++ = val >> 8; - *p++ = val >> 16; -} - -static inline void put_unaligned_le24(const u32 val, void *p) -{ - __put_unaligned_le24(val, p); -} - -#endif /* _LINUX_UNALIGNED_GENERIC_H */ diff --git a/include/linux/unaligned/le_struct.h b/include/linux/unaligned/le_struct.h deleted file mode 100644 index 22f90a4afaa5..000000000000 --- a/include/linux/unaligned/le_struct.h +++ /dev/null @@ -1,67 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -#ifndef _LINUX_UNALIGNED_LE_STRUCT_H -#define _LINUX_UNALIGNED_LE_STRUCT_H - -#include - -static inline u16 get_unaligned_le16(const void *p) -{ - return __get_unaligned_cpu16((const u8 *)p); -} - -static inline u32 get_unaligned_le32(const void *p) -{ - return __get_unaligned_cpu32((const u8 *)p); -} - -static inline u64 get_unaligned_le64(const void *p) -{ - return __get_unaligned_cpu64((const u8 *)p); -} - -static inline void put_unaligned_le16(u16 val, void *p) -{ - __put_unaligned_cpu16(val, p); -} - -static inline void put_unaligned_le32(u32 val, void *p) -{ - __put_unaligned_cpu32(val, p); -} - -static inline void put_unaligned_le64(u64 val, void *p) -{ - __put_unaligned_cpu64(val, p); -} - -static inline u16 get_unaligned_be16(const void *p) -{ - return swab16(__get_unaligned_cpu16((const u8 *)p)); -} - -static inline u32 get_unaligned_be32(const void *p) -{ - return swab32(__get_unaligned_cpu32((const u8 *)p)); -} - -static inline u64 get_unaligned_be64(const void *p) -{ - return swab64(__get_unaligned_cpu64((const u8 *)p)); -} - -static inline void put_unaligned_be16(u16 val, void *p) -{ - __put_unaligned_cpu16(swab16(val), p); -} - -static inline void put_unaligned_be32(u32 val, void *p) -{ - __put_unaligned_cpu32(swab32(val), p); -} - -static inline void put_unaligned_be64(u64 val, void *p) -{ - __put_unaligned_cpu64(swab64(val), p); -} - -#endif /* _LINUX_UNALIGNED_LE_STRUCT_H */ -- cgit v1.2.3