[PATCH 0/8] Cleanup unaligned access macros

[PATCH 0/8] Cleanup unaligned access macros

[Jens Wiklander]

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

-- 
2.34.1

[PATCH 1/8] arm: use asm-generic/unaligned.h

[Jens Wiklander]

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>
-- 
2.34.1

[PATCH 2/8] sh: use asm-generic/unaligned.h

[Jens Wiklander]

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>
-- 
2.34.1

[PATCH 3/8] mips: use asm-generic/unaligned.h

[Jens Wiklander]

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>
-- 
2.34.1

[PATCH 4/8] m68k: use asm-generic/unaligned.h

[Jens Wiklander]

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>
-- 
2.34.1

Re: [PATCH 4/8] m68k: use asm-generic/unaligned.h

[Angelo Dureghello]

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

[PATCH 5/8] powerpc: use asm-generic/unaligned.h

[Jens Wiklander]

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>
-- 
2.34.1

[PATCH 6/8] fs/btrfs: use asm/unaligned.h

[Jens Wiklander]

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>
-- 
2.34.1

[PATCH 7/8] linux/unaligned: remove unused access_ok.h

[Jens Wiklander]

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 */
-- 
2.34.1

[PATCH 8/8] asm-generic: simplify unaligned.h

[Jens Wiklander]

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);
-- 
2.34.1

Re: [PATCH 0/8] Cleanup unaligned access macros

[Ilias Apalodimas]

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
>

Re: [PATCH 0/8] Cleanup unaligned access macros

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]