[PATCH 0/8] Cleanup unaligned access macros

Hi,
There are two versions of get/set_unaligned, get_unaligned_be64, put_unaligned_le64 etc in U-Boot causing confusion (and bugs).
In this patch-set, I'm trying to fix that with a single unified version of the access macros to be used across all archs. This work is inspired by similar changes in this Linux kernel by Arnd Bergman, https://lore.kernel.org/lkml/20210514100106.3404011-1-arnd@kernel.org/
Thanks, Jens
Jens Wiklander (8): arm: use asm-generic/unaligned.h sh: use asm-generic/unaligned.h mips: use asm-generic/unaligned.h m68k: use asm-generic/unaligned.h powerpc: use asm-generic/unaligned.h fs/btrfs: use asm/unaligned.h linux/unaligned: remove unused access_ok.h asm-generic: simplify unaligned.h
arch/arm/include/asm/unaligned.h | 21 +------ arch/m68k/include/asm/unaligned.h | 17 +----- arch/mips/include/asm/unaligned.h | 23 +------ arch/powerpc/include/asm/unaligned.h | 18 +----- arch/sh/include/asm/unaligned.h | 22 +------ fs/btrfs/crypto/hash.c | 2 +- include/asm-generic/unaligned.h | 89 +++++++++++++++++++++++----- include/linux/unaligned/access_ok.h | 66 --------------------- 8 files changed, 83 insertions(+), 175 deletions(-) delete mode 100644 include/linux/unaligned/access_ok.h

Arm duplicates the content of asm-generic/unaligned.h, so use that file directly instead.
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org --- arch/arm/include/asm/unaligned.h | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/arch/arm/include/asm/unaligned.h b/arch/arm/include/asm/unaligned.h index 0a228fb8eea8..7fb482abc383 100644 --- a/arch/arm/include/asm/unaligned.h +++ b/arch/arm/include/asm/unaligned.h @@ -1,19 +1,2 @@ -#ifndef _ASM_ARM_UNALIGNED_H -#define _ASM_ARM_UNALIGNED_H - -#include <linux/unaligned/le_byteshift.h> -#include <linux/unaligned/be_byteshift.h> -#include <linux/unaligned/generic.h> - -/* - * Select endianness - */ -#if __BYTE_ORDER == __LITTLE_ENDIAN -#define get_unaligned __get_unaligned_le -#define put_unaligned __put_unaligned_le -#else -#define get_unaligned __get_unaligned_be -#define put_unaligned __put_unaligned_be -#endif - -#endif /* _ASM_ARM_UNALIGNED_H */ +/* SPDX-License-Identifier: GPL-2.0 */ +#include <asm-generic/unaligned.h>

Sh essentially duplicates the content of asm-generic/unaligned.h, so use that file directly instead.
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org --- arch/sh/include/asm/unaligned.h | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/arch/sh/include/asm/unaligned.h b/arch/sh/include/asm/unaligned.h index 5acf0819620e..7fb482abc383 100644 --- a/arch/sh/include/asm/unaligned.h +++ b/arch/sh/include/asm/unaligned.h @@ -1,20 +1,2 @@ -#ifndef _ASM_SH_UNALIGNED_H -#define _ASM_SH_UNALIGNED_H - -/* Copy from linux-kernel. */ - -/* Other than SH4A, SH can't handle unaligned accesses. */ -#include <linux/compiler.h> -#if defined(__BIG_ENDIAN__) -#define get_unaligned __get_unaligned_be -#define put_unaligned __put_unaligned_be -#elif defined(__LITTLE_ENDIAN__) -#define get_unaligned __get_unaligned_le -#define put_unaligned __put_unaligned_le -#endif - -#include <linux/unaligned/le_byteshift.h> -#include <linux/unaligned/be_byteshift.h> -#include <linux/unaligned/generic.h> - -#endif /* _ASM_SH_UNALIGNED_H */ +/* SPDX-License-Identifier: GPL-2.0 */ +#include <asm-generic/unaligned.h>

Mips essentially duplicates the content of asm-generic/unaligned.h, so use that file directly instead.
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org --- arch/mips/include/asm/unaligned.h | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-)
diff --git a/arch/mips/include/asm/unaligned.h b/arch/mips/include/asm/unaligned.h index debb9cf7aba6..7fb482abc383 100644 --- a/arch/mips/include/asm/unaligned.h +++ b/arch/mips/include/asm/unaligned.h @@ -1,23 +1,2 @@ /* SPDX-License-Identifier: GPL-2.0 */ -/* - * Copyright (C) 2007 Ralf Baechle (ralf@linux-mips.org) - */ -#ifndef _ASM_MIPS_UNALIGNED_H -#define _ASM_MIPS_UNALIGNED_H - -#include <linux/compiler.h> -#if defined(__MIPSEB__) -#define get_unaligned __get_unaligned_be -#define put_unaligned __put_unaligned_be -#elif defined(__MIPSEL__) -#define get_unaligned __get_unaligned_le -#define put_unaligned __put_unaligned_le -#else -#error "MIPS, but neither __MIPSEB__, nor __MIPSEL__???" -#endif - -#include <linux/unaligned/le_byteshift.h> -#include <linux/unaligned/be_byteshift.h> -#include <linux/unaligned/generic.h> - -#endif /* _ASM_MIPS_UNALIGNED_H */ +#include <asm-generic/unaligned.h>

M68k essentially duplicates the content of asm-generic/unaligned.h, with an exception for non-Coldfire configurations. Coldfire configurations are apparently able to do unaligned accesses. But in an attempt to clean up and handle unaligned accesses in the same way we ignore that and use the common asm-generic/unaligned.h directly instead.
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org --- arch/m68k/include/asm/unaligned.h | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/arch/m68k/include/asm/unaligned.h b/arch/m68k/include/asm/unaligned.h index 328aa0c316c9..7fb482abc383 100644 --- a/arch/m68k/include/asm/unaligned.h +++ b/arch/m68k/include/asm/unaligned.h @@ -1,15 +1,2 @@ -#ifndef _ASM_M68K_UNALIGNED_H -#define _ASM_M68K_UNALIGNED_H - -#ifdef CONFIG_COLDFIRE -#include <linux/unaligned/be_byteshift.h> -#else -#include <linux/unaligned/access_ok.h> -#endif - -#include <linux/unaligned/generic.h> - -#define get_unaligned __get_unaligned_be -#define put_unaligned __put_unaligned_be - -#endif /* _ASM_M68K_UNALIGNED_H */ +/* SPDX-License-Identifier: GPL-2.0 */ +#include <asm-generic/unaligned.h>

Hi,
On 22/05/23 2:22 PM, Jens Wiklander wrote:
M68k essentially duplicates the content of asm-generic/unaligned.h, with an exception for non-Coldfire configurations. Coldfire configurations are apparently able to do unaligned accesses. But in an attempt to clean up and handle unaligned accesses in the same way we ignore that and use the common asm-generic/unaligned.h directly instead.
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org
arch/m68k/include/asm/unaligned.h | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/arch/m68k/include/asm/unaligned.h b/arch/m68k/include/asm/unaligned.h index 328aa0c316c9..7fb482abc383 100644 --- a/arch/m68k/include/asm/unaligned.h +++ b/arch/m68k/include/asm/unaligned.h @@ -1,15 +1,2 @@ -#ifndef _ASM_M68K_UNALIGNED_H -#define _ASM_M68K_UNALIGNED_H
-#ifdef CONFIG_COLDFIRE -#include <linux/unaligned/be_byteshift.h> -#else -#include <linux/unaligned/access_ok.h> -#endif
-#include <linux/unaligned/generic.h>
-#define get_unaligned __get_unaligned_be -#define put_unaligned __put_unaligned_be
-#endif /* _ASM_M68K_UNALIGNED_H */ +/* SPDX-License-Identifier: GPL-2.0 */ +#include <asm-generic/unaligned.h>
sorry if a bit late, i tested this on coldfire mcf54415, no issues, so ok for me.
Acked-by: Angelo Dureghello angelo@kernel-space.org
Regards, angelo

Powerpc configurations are apparently able to do unaligned accesses. But in an attempt to clean up and handle unaligned accesses in the same way we ignore that and use the common asm-generic/unaligned.h directly instead.
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org --- arch/powerpc/include/asm/unaligned.h | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/include/asm/unaligned.h b/arch/powerpc/include/asm/unaligned.h index 5f1b1e3c2137..7fb482abc383 100644 --- a/arch/powerpc/include/asm/unaligned.h +++ b/arch/powerpc/include/asm/unaligned.h @@ -1,16 +1,2 @@ -#ifndef _ASM_POWERPC_UNALIGNED_H -#define _ASM_POWERPC_UNALIGNED_H - -#ifdef __KERNEL__ - -/* - * The PowerPC can do unaligned accesses itself in big endian mode. - */ -#include <linux/unaligned/access_ok.h> -#include <linux/unaligned/generic.h> - -#define get_unaligned __get_unaligned_be -#define put_unaligned __put_unaligned_be - -#endif /* __KERNEL__ */ -#endif /* _ASM_POWERPC_UNALIGNED_H */ +/* SPDX-License-Identifier: GPL-2.0 */ +#include <asm-generic/unaligned.h>

Use asm/unaligned.h instead of linux/unaligned/access_ok.h for unaligned access. This is needed on architectures that doesn't handle unaligned accesses directly.
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org --- fs/btrfs/crypto/hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/crypto/hash.c b/fs/btrfs/crypto/hash.c index 891a2974be05..0a0b35fe9b96 100644 --- a/fs/btrfs/crypto/hash.c +++ b/fs/btrfs/crypto/hash.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0+
+#include <asm/unaligned.h> #include <linux/xxhash.h> -#include <linux/unaligned/access_ok.h> #include <linux/types.h> #include <u-boot/sha256.h> #include <u-boot/blake2.h>

linux/unaligned/access_ok.h is unused, so remove it.
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org --- include/linux/unaligned/access_ok.h | 66 ----------------------------- 1 file changed, 66 deletions(-) delete mode 100644 include/linux/unaligned/access_ok.h
diff --git a/include/linux/unaligned/access_ok.h b/include/linux/unaligned/access_ok.h deleted file mode 100644 index 5f46eee23c38..000000000000 --- a/include/linux/unaligned/access_ok.h +++ /dev/null @@ -1,66 +0,0 @@ -#ifndef _LINUX_UNALIGNED_ACCESS_OK_H -#define _LINUX_UNALIGNED_ACCESS_OK_H - -#include <asm/byteorder.h> - -static inline u16 get_unaligned_le16(const void *p) -{ - return le16_to_cpup((__le16 *)p); -} - -static inline u32 get_unaligned_le32(const void *p) -{ - return le32_to_cpup((__le32 *)p); -} - -static inline u64 get_unaligned_le64(const void *p) -{ - return le64_to_cpup((__le64 *)p); -} - -static inline u16 get_unaligned_be16(const void *p) -{ - return be16_to_cpup((__be16 *)p); -} - -static inline u32 get_unaligned_be32(const void *p) -{ - return be32_to_cpup((__be32 *)p); -} - -static inline u64 get_unaligned_be64(const void *p) -{ - return be64_to_cpup((__be64 *)p); -} - -static inline void put_unaligned_le16(u16 val, void *p) -{ - *((__le16 *)p) = cpu_to_le16(val); -} - -static inline void put_unaligned_le32(u32 val, void *p) -{ - *((__le32 *)p) = cpu_to_le32(val); -} - -static inline void put_unaligned_le64(u64 val, void *p) -{ - *((__le64 *)p) = cpu_to_le64(val); -} - -static inline void put_unaligned_be16(u16 val, void *p) -{ - *((__be16 *)p) = cpu_to_be16(val); -} - -static inline void put_unaligned_be32(u32 val, void *p) -{ - *((__be32 *)p) = cpu_to_be32(val); -} - -static inline void put_unaligned_be64(u64 val, void *p) -{ - *((__be64 *)p) = cpu_to_be64(val); -} - -#endif /* _LINUX_UNALIGNED_ACCESS_OK_H */

The get_unaligned()/put_unaligned() implementations are more complex than necessary.
Move everything into one file and use a more compact implementation based on packed struct access and byte swapping macros.
This patch is based on the Linux kernel commit 803f4e1eab7a ("asm-generic: simplify asm/unaligned.h") by Arnd Bergmann.
Signed-off-by: Jens Wiklander jens.wiklander@linaro.org --- include/asm-generic/unaligned.h | 89 +++++++++++++++++++++++++++------ 1 file changed, 73 insertions(+), 16 deletions(-)
diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h index 3d33a5a063e8..9e5d93ec3041 100644 --- a/include/asm-generic/unaligned.h +++ b/include/asm-generic/unaligned.h @@ -1,24 +1,81 @@ +/* SPDX-License-Identifier: GPL-2.0 */ #ifndef _GENERIC_UNALIGNED_H #define _GENERIC_UNALIGNED_H
#include <asm/byteorder.h>
-#include <linux/unaligned/le_byteshift.h> -#include <linux/unaligned/be_byteshift.h> -#include <linux/unaligned/generic.h> - -/* - * Select endianness - */ -#if defined(__LITTLE_ENDIAN) -#define get_unaligned __get_unaligned_le -#define put_unaligned __put_unaligned_le -#elif defined(__BIG_ENDIAN) -#define get_unaligned __get_unaligned_be -#define put_unaligned __put_unaligned_be -#else -#error invalid endian -#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); +}
/* Allow unaligned memory access */ void allow_unaligned(void);

Hi Jens
On Mon, May 22, 2023 at 02:22:30PM +0200, Jens Wiklander wrote:
Hi,
There are two versions of get/set_unaligned, get_unaligned_be64, put_unaligned_le64 etc in U-Boot causing confusion (and bugs).
In this patch-set, I'm trying to fix that with a single unified version of the access macros to be used across all archs. This work is inspired by similar changes in this Linux kernel by Arnd Bergman, https://lore.kernel.org/lkml/20210514100106.3404011-1-arnd@kernel.org/
Thanks, Jens
Thanks for the cleanup.
For the series Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Although I'd like to hear from arch maintainers as well.
Tom, This did pass all the CI successfully, but regardless I think it should be pulled into -next. If you want me to pick it up via the TPM tree please let me know.
Thanks /Ilias
Jens Wiklander (8): arm: use asm-generic/unaligned.h sh: use asm-generic/unaligned.h mips: use asm-generic/unaligned.h m68k: use asm-generic/unaligned.h powerpc: use asm-generic/unaligned.h fs/btrfs: use asm/unaligned.h linux/unaligned: remove unused access_ok.h asm-generic: simplify unaligned.h
arch/arm/include/asm/unaligned.h | 21 +------ arch/m68k/include/asm/unaligned.h | 17 +----- arch/mips/include/asm/unaligned.h | 23 +------ arch/powerpc/include/asm/unaligned.h | 18 +----- arch/sh/include/asm/unaligned.h | 22 +------ fs/btrfs/crypto/hash.c | 2 +- include/asm-generic/unaligned.h | 89 +++++++++++++++++++++++----- include/linux/unaligned/access_ok.h | 66 --------------------- 8 files changed, 83 insertions(+), 175 deletions(-) delete mode 100644 include/linux/unaligned/access_ok.h
-- 2.34.1

On Mon, May 22, 2023 at 11:34:36PM +0300, Ilias Apalodimas wrote:
Hi Jens
On Mon, May 22, 2023 at 02:22:30PM +0200, Jens Wiklander wrote:
Hi,
There are two versions of get/set_unaligned, get_unaligned_be64, put_unaligned_le64 etc in U-Boot causing confusion (and bugs).
In this patch-set, I'm trying to fix that with a single unified version of the access macros to be used across all archs. This work is inspired by similar changes in this Linux kernel by Arnd Bergman, https://lore.kernel.org/lkml/20210514100106.3404011-1-arnd@kernel.org/
Thanks, Jens
Thanks for the cleanup.
For the series Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Although I'd like to hear from arch maintainers as well.
Tom, This did pass all the CI successfully, but regardless I think it should be pulled into -next. If you want me to pick it up via the TPM tree please let me know.
I'll pick this up for -next after it's been around a little bit longer, to let people test / ack it, thanks.

On Mon, 22 May 2023 14:22:30 +0200, Jens Wiklander wrote:
There are two versions of get/set_unaligned, get_unaligned_be64, put_unaligned_le64 etc in U-Boot causing confusion (and bugs).
In this patch-set, I'm trying to fix that with a single unified version of the access macros to be used across all archs. This work is inspired by similar changes in this Linux kernel by Arnd Bergman, https://lore.kernel.org/lkml/20210514100106.3404011-1-arnd@kernel.org/
[...]
Applied to u-boot/next, thanks!
participants (4)
-
Angelo Dureghello
-
Ilias Apalodimas
-
Jens Wiklander
-
Tom Rini