All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/6]  Implement byteswap and update references
@ 2022-05-23  9:52 Lin Liu
  2022-05-23  9:52 ` [PATCH v4 1/6] xen: implement byteswap Lin Liu
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Lin Liu @ 2022-05-23  9:52 UTC (permalink / raw)
  To: xen-devel
  Cc: Lin Liu, Andrew Cooper, Daniel De Graaf, Daniel P. Smith,
	George Dunlap, Ian Jackson, Jan Beulich, Julien Grall,
	Bertrand Marquis, Konrad Rzeszutek Wilk, Roger Pau Monné,
	Ross Lagerwall, Stefano Stabellini, Volodymyr Babchuk, Wei Liu


Lin Liu (6):
  xen: implement byteswap
  crypto/vmac: Simplify code with byteswap
  arm64/find_next_bit: Remove ext2_swab()
  xen: Switch to byteswap
  tools: Use new byteswap helper
  byteorder: Remove byteorder

 .../libs/guest/xg_dom_decompress_unsafe_xz.c  |   5 +
 .../guest/xg_dom_decompress_unsafe_zstd.c     |   3 +-
 xen/arch/arm/arm64/lib/find_next_bit.c        |  36 +---
 xen/arch/arm/include/asm/byteorder.h          |   6 +-
 xen/arch/x86/include/asm/byteorder.h          |  34 +---
 xen/common/device_tree.c                      |  44 ++---
 xen/common/libelf/libelf-private.h            |   6 +-
 xen/common/xz/private.h                       |   2 +-
 xen/crypto/vmac.c                             |  76 +-------
 xen/include/xen/byteorder.h                   |  56 ++++++
 xen/include/xen/byteorder/big_endian.h        | 102 ----------
 xen/include/xen/byteorder/generic.h           |  68 -------
 xen/include/xen/byteorder/little_endian.h     | 102 ----------
 xen/include/xen/byteorder/swab.h              | 183 ------------------
 xen/include/xen/byteswap.h                    |  44 +++++
 xen/include/xen/compiler.h                    |  24 +++
 xen/include/xen/lib.h                         |   4 -
 xen/include/xen/unaligned.h                   |  12 +-
 18 files changed, 180 insertions(+), 627 deletions(-)
 create mode 100644 xen/include/xen/byteorder.h
 delete mode 100644 xen/include/xen/byteorder/big_endian.h
 delete mode 100644 xen/include/xen/byteorder/generic.h
 delete mode 100644 xen/include/xen/byteorder/little_endian.h
 delete mode 100644 xen/include/xen/byteorder/swab.h
 create mode 100644 xen/include/xen/byteswap.h

-- 
2.27.0



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 1/6] xen: implement byteswap
  2022-05-23  9:52 [PATCH v4 0/6] Implement byteswap and update references Lin Liu
@ 2022-05-23  9:52 ` Lin Liu
  2022-05-23 10:07   ` Roger Pau Monné
  2022-05-23  9:52 ` [PATCH v4 2/6] crypto/vmac: Simplify code with byteswap Lin Liu
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Lin Liu @ 2022-05-23  9:52 UTC (permalink / raw)
  To: xen-devel
  Cc: Lin Liu, Andrew Cooper, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, George Dunlap, Jan Beulich,
	Wei Liu, Roger Pau Monné

swab() is massively over complicated and can be simplified
by re-implementing using compiler builtins.
The compilers provide builtin function to swap bytes.
* gcc:   https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
* clang: https://clang.llvm.org/docs/LanguageExtensions.html
This patch introduces a new byteswapping infrastructure in
terms of compiler builtins and bswapXX(), so the swab() infrastructure
can be retired.

Signed-off-by: Lin Liu <lin.liu@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien@xen.org>
Cc: Bertrand Marquis <bertrand.marquis@arm.com>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Wei Liu <wl@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Changes in v4:
- Move `PASTE` definition to xen/compiler.h to support tools
- Revert emacs magics

Changes in v3:
- Check __has_builtin instead of GNUC version 

Changes in v2:
- Add fallback for compilers without __builtin_bswap
- Implement with plain C instead of macros
---
 xen/arch/arm/include/asm/byteorder.h |  6 ++-
 xen/arch/x86/include/asm/byteorder.h | 34 ++---------------
 xen/include/xen/byteorder.h          | 56 ++++++++++++++++++++++++++++
 xen/include/xen/byteswap.h           | 44 ++++++++++++++++++++++
 xen/include/xen/compiler.h           | 24 ++++++++++++
 xen/include/xen/lib.h                |  4 --
 6 files changed, 132 insertions(+), 36 deletions(-)
 create mode 100644 xen/include/xen/byteorder.h
 create mode 100644 xen/include/xen/byteswap.h

diff --git a/xen/arch/arm/include/asm/byteorder.h b/xen/arch/arm/include/asm/byteorder.h
index 9c712c4788..b6a33b23c0 100644
--- a/xen/arch/arm/include/asm/byteorder.h
+++ b/xen/arch/arm/include/asm/byteorder.h
@@ -1,9 +1,11 @@
 #ifndef __ASM_ARM_BYTEORDER_H__
 #define __ASM_ARM_BYTEORDER_H__
 
-#define __BYTEORDER_HAS_U64__
+#ifndef __BYTE_ORDER__
+   #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
+#endif
 
-#include <xen/byteorder/little_endian.h>
+#include <xen/byteorder.h>
 
 #endif /* __ASM_ARM_BYTEORDER_H__ */
 /*
diff --git a/xen/arch/x86/include/asm/byteorder.h b/xen/arch/x86/include/asm/byteorder.h
index 1f77e502a5..82aadee7bd 100644
--- a/xen/arch/x86/include/asm/byteorder.h
+++ b/xen/arch/x86/include/asm/byteorder.h
@@ -1,36 +1,10 @@
 #ifndef __ASM_X86_BYTEORDER_H__
 #define __ASM_X86_BYTEORDER_H__
 
-#include <asm/types.h>
-#include <xen/compiler.h>
+#ifndef __BYTE_ORDER__
+   #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
+#endif
 
-static inline __attribute_const__ __u32 ___arch__swab32(__u32 x)
-{
-    asm("bswap %0" : "=r" (x) : "0" (x));
-    return x;
-}
-
-static inline __attribute_const__ __u64 ___arch__swab64(__u64 val)
-{ 
-    union { 
-        struct { __u32 a,b; } s;
-        __u64 u;
-    } v;
-    v.u = val;
-    asm("bswapl %0 ; bswapl %1 ; xchgl %0,%1" 
-        : "=r" (v.s.a), "=r" (v.s.b) 
-        : "0" (v.s.a), "1" (v.s.b)); 
-    return v.u;
-} 
-
-/* Do not define swab16.  Gcc is smart enough to recognize "C" version and
-   convert it into rotation or exhange.  */
-
-#define __arch__swab64(x) ___arch__swab64(x)
-#define __arch__swab32(x) ___arch__swab32(x)
-
-#define __BYTEORDER_HAS_U64__
-
-#include <xen/byteorder/little_endian.h>
+#include <xen/byteorder.h>
 
 #endif /* __ASM_X86_BYTEORDER_H__ */
diff --git a/xen/include/xen/byteorder.h b/xen/include/xen/byteorder.h
new file mode 100644
index 0000000000..2ec434e6a6
--- /dev/null
+++ b/xen/include/xen/byteorder.h
@@ -0,0 +1,56 @@
+#ifndef __XEN_BYTEORDER_H__
+#define __XEN_BYTEORDER_H__
+
+#include <xen/byteswap.h>
+
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+
+# ifndef __LITTLE_ENDIAN
+#  define __LITTLE_ENDIAN 1234
+# endif
+
+# ifndef __LITTLE_ENDIAN_BITFIELD
+#  define __LITTLE_ENDIAN_BITFIELD
+# endif
+
+# define cpu_to_le64(x) (x)
+# define le64_to_cpu(x) (x)
+# define cpu_to_le32(x) (x)
+# define le32_to_cpu(x) (x)
+# define cpu_to_le16(x) (x)
+# define le16_to_cpu(x) (x)
+# define cpu_to_be64(x) bswap64(x)
+# define be64_to_cpu(x) bswap64(x)
+# define cpu_to_be32(x) bswap32(x)
+# define be32_to_cpu(x) bswap32(x)
+# define cpu_to_be16(x) bswap16(x)
+# define be16_to_cpu(x) bswap16(x)
+
+#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+
+# ifndef __BIG_ENDIAN
+#  define __BIG_ENDIAN 4321
+# endif
+
+# ifndef __BIG_ENDIAN_BITFIELD
+#  define __BIG_ENDIAN_BITFIELD
+# endif
+
+# define cpu_to_le64(x) bswap64(x)
+# define le64_to_cpu(x) bswap64(x)
+# define cpu_to_le32(x) bswap32(x)
+# define le32_to_cpu(x) bswap32(x)
+# define cpu_to_le16(x) bswap16(x)
+# define le16_to_cpu(x) bswap16(x)
+# define cpu_to_be64(x) (x)
+# define be64_to_cpu(x) (x)
+# define cpu_to_be32(x) (x)
+# define be32_to_cpu(x) (x)
+# define cpu_to_be16(x) (x)
+# define be16_to_cpu(x) (x)
+
+#else
+# error "Unknown Endianness"
+#endif /* __BYTE_ORDER__ */
+
+#endif /* __XEN_BYTEORDER_H__ */
diff --git a/xen/include/xen/byteswap.h b/xen/include/xen/byteswap.h
new file mode 100644
index 0000000000..d2e371fbe7
--- /dev/null
+++ b/xen/include/xen/byteswap.h
@@ -0,0 +1,44 @@
+#ifndef __XEN_BYTESWAP_H__
+#define __XEN_BYTESWAP_H__
+
+#include <xen/types.h>
+#include <xen/compiler.h>
+
+#if !__has_builtin(__builtin_bswap16)
+static always_inline uint16_t __builtin_bswap16(uint16_t val)
+{
+    return ((val & 0x00FF) << 8) | ((val & 0xFF00) >> 8);
+}
+#endif
+
+#if !__has_builtin(__builtin_bswap32)
+static always_inline uint32_t __builtin_bswap32(uint32_t val)
+{
+    return ((val & 0x000000FF) << 24) |
+           ((val & 0x0000FF00) <<  8) |
+           ((val & 0x00FF0000) >>  8) |
+           ((val & 0xFF000000) >> 24);
+}
+#endif
+
+#if !__has_builtin(__builtin_bswap64)
+static always_inline uint64_t __builtin_bswap64(uint64_t val)
+{
+    return ((val & 0x00000000000000FF) << 56) |
+           ((val & 0x000000000000FF00) << 40) |
+           ((val & 0x0000000000FF0000) << 24) |
+           ((val & 0x00000000FF000000) <<  8) |
+           ((val & 0x000000FF00000000) >>  8) |
+           ((val & 0x0000FF0000000000) >> 24) |
+           ((val & 0x00FF000000000000) >> 40) |
+           ((val & 0xFF00000000000000) >> 56);
+}
+#endif
+
+#define bswap16(x) __builtin_bswap16(x)
+#define bswap32(x) __builtin_bswap32(x)
+#define bswap64(x) __builtin_bswap64(x)
+
+#define bswap_ul(x) PASTE(bswap,BITS_PER_LONG)(x)
+
+#endif /* __XEN_BYTESWAP_H__ */
diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
index 933aec09a9..ae029afa14 100644
--- a/xen/include/xen/compiler.h
+++ b/xen/include/xen/compiler.h
@@ -185,4 +185,28 @@
 # define CLANG_DISABLE_WARN_GCC_COMPAT_END
 #endif
 
+#ifndef __has_builtin
+/*
+ * Backwards compatibility for GCC < 10.
+ * All supported versions of Clang support __has_builtin
+ * */
+#define __has_builtin(x) GCC_has ## x
+
+#define GCC_has__builtin_bswap16 (CONFIG_GCC_VERSION >= 40800)
+#define GCC_has__builtin_bswap32 (CONFIG_GCC_VERSION >= 40400)
+#define GCC_has__builtin_bswap64 (CONFIG_GCC_VERSION >= 40400)
+#endif
+
+#ifndef __ORDER_LITTLE_ENDIAN__
+# define __ORDER_LITTLE_ENDIAN__ 1234
+#endif
+
+#ifndef __ORDER_BIG_ENDIAN__
+# define __ORDER_BIG_ENDIAN__ 4321
+#endif
+
+/* Indirect macros required for expanded argument pasting. */
+#define PASTE_(a, b) a ## b
+#define PASTE(a, b) PASTE_(a, b)
+
 #endif /* __LINUX_COMPILER_H */
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index aab1fc7c4a..ebf2193569 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -15,10 +15,6 @@
 #define count_args(args...) \
     count_args_(., ## args, 8, 7, 6, 5, 4, 3, 2, 1, 0)
 
-/* Indirect macros required for expanded argument pasting. */
-#define PASTE_(a, b) a ## b
-#define PASTE(a, b) PASTE_(a, b)
-
 #define __STR(...) #__VA_ARGS__
 #define STR(...) __STR(__VA_ARGS__)
 
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 2/6] crypto/vmac: Simplify code with byteswap
  2022-05-23  9:52 [PATCH v4 0/6] Implement byteswap and update references Lin Liu
  2022-05-23  9:52 ` [PATCH v4 1/6] xen: implement byteswap Lin Liu
@ 2022-05-23  9:52 ` Lin Liu
  2022-05-23  9:52 ` [PATCH v4 3/6] arm64/find_next_bit: Remove ext2_swab() Lin Liu
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Lin Liu @ 2022-05-23  9:52 UTC (permalink / raw)
  To: xen-devel
  Cc: Lin Liu, Jan Beulich, Andrew Cooper, George Dunlap, Julien Grall,
	Bertrand Marquis, Stefano Stabellini, Wei Liu

This file has its own implementation of swap bytes. Clean up
the code with xen/byteswap.h.

No functional change.

Signed-off-by: Lin Liu <lin.liu@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>
Cc: Bertrand Marquis <bertrand.marquis@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Wei Liu <wl@xen.org>
----
 xen/crypto/vmac.c | 76 ++---------------------------------------------
 1 file changed, 3 insertions(+), 73 deletions(-)

diff --git a/xen/crypto/vmac.c b/xen/crypto/vmac.c
index 294dd16a52..acb4e015f5 100644
--- a/xen/crypto/vmac.c
+++ b/xen/crypto/vmac.c
@@ -8,6 +8,7 @@
 
 /* start for Xen */
 #include <xen/init.h>
+#include <xen/byteswap.h>
 #include <xen/types.h>
 #include <xen/lib.h>
 #include <crypto/vmac.h>
@@ -50,7 +51,6 @@ const uint64_t mpoly = UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
  * MUL64: 64x64->128-bit multiplication
  * PMUL64: assumes top bits cleared on inputs
  * ADD128: 128x128->128-bit addition
- * GET_REVERSED_64: load and byte-reverse 64-bit word  
  * ----------------------------------------------------------------------- */
 
 /* ----------------------------------------------------------------------- */
@@ -68,22 +68,6 @@ const uint64_t mpoly = UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
 
 #define PMUL64 MUL64
 
-#define GET_REVERSED_64(p)                                                \
-    ({uint64_t x;                                                         \
-     asm ("bswapq %0" : "=r" (x) : "0"(*(uint64_t *)(p))); x;})
-
-/* ----------------------------------------------------------------------- */
-#elif (__GNUC__ && __i386__)
-/* ----------------------------------------------------------------------- */
-
-#define GET_REVERSED_64(p)                                                \
-    ({ uint64_t x;                                                        \
-    uint32_t *tp = (uint32_t *)(p);                                       \
-    asm  ("bswap %%edx\n\t"                                               \
-          "bswap %%eax"                                                   \
-    : "=A"(x)                                                             \
-    : "a"(tp[1]), "d"(tp[0]));                                            \
-    x; })
 
 /* ----------------------------------------------------------------------- */
 #elif (__GNUC__ && __ppc64__)
@@ -103,37 +87,6 @@ const uint64_t mpoly = UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
 
 #define PMUL64 MUL64
 
-#define GET_REVERSED_64(p)                                                \
-    ({ uint32_t hi, lo, *_p = (uint32_t *)(p);                            \
-       asm volatile ("lwbrx %0, %1, %2" : "=r"(lo) : "b%"(0), "r"(_p) );  \
-       asm volatile ("lwbrx %0, %1, %2" : "=r"(hi) : "b%"(4), "r"(_p) );  \
-       ((uint64_t)hi << 32) | (uint64_t)lo; } )
-
-/* ----------------------------------------------------------------------- */
-#elif (__GNUC__ && (__ppc__ || __PPC__))
-/* ----------------------------------------------------------------------- */
-
-#define GET_REVERSED_64(p)                                                \
-    ({ uint32_t hi, lo, *_p = (uint32_t *)(p);                            \
-       asm volatile ("lwbrx %0, %1, %2" : "=r"(lo) : "b%"(0), "r"(_p) );  \
-       asm volatile ("lwbrx %0, %1, %2" : "=r"(hi) : "b%"(4), "r"(_p) );  \
-       ((uint64_t)hi << 32) | (uint64_t)lo; } )
-
-/* ----------------------------------------------------------------------- */
-#elif (__GNUC__ && (__ARMEL__ || __ARM__))
-/* ----------------------------------------------------------------------- */
-
-#define bswap32(v)                                                        \
-({ uint32_t tmp,out;                                                      \
-    asm volatile(                                                         \
-        "eor    %1, %2, %2, ror #16\n"                                    \
-        "bic    %1, %1, #0x00ff0000\n"                                    \
-        "mov    %0, %2, ror #8\n"                                         \
-        "eor    %0, %0, %1, lsr #8"                                       \
-    : "=r" (out), "=&r" (tmp)                                             \
-    : "r" (v));                                                           \
-    out;})
-
 /* ----------------------------------------------------------------------- */
 #elif _MSC_VER
 /* ----------------------------------------------------------------------- */
@@ -154,11 +107,6 @@ const uint64_t mpoly = UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
         (rh) += (ih) + ((rl) < (_il));                               \
     }
 
-#if _MSC_VER >= 1300
-#define GET_REVERSED_64(p) _byteswap_uint64(*(uint64_t *)(p))
-#pragma intrinsic(_byteswap_uint64)
-#endif
-
 #if _MSC_VER >= 1400 && \
     (!defined(__INTEL_COMPILER) || __INTEL_COMPILER >= 1000)
 #define MUL32(i1,i2)    (__emulu((uint32_t)(i1),(uint32_t)(i2)))
@@ -219,24 +167,6 @@ const uint64_t mpoly = UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
     }
 #endif
 
-#ifndef GET_REVERSED_64
-#ifndef bswap64
-#ifndef bswap32
-#define bswap32(x)                                                        \
-  ({ uint32_t bsx = (x);                                                  \
-      ((((bsx) & 0xff000000u) >> 24) | (((bsx) & 0x00ff0000u) >>  8) |    \
-       (((bsx) & 0x0000ff00u) <<  8) | (((bsx) & 0x000000ffu) << 24)); })
-#endif
-#define bswap64(x)                                                        \
-     ({ union { uint64_t ll; uint32_t l[2]; } w, r;                       \
-         w.ll = (x);                                                      \
-         r.l[0] = bswap32 (w.l[1]);                                       \
-         r.l[1] = bswap32 (w.l[0]);                                       \
-         r.ll; })
-#endif
-#define GET_REVERSED_64(p) bswap64(*(uint64_t *)(p)) 
-#endif
-
 /* ----------------------------------------------------------------------- */
 
 #if (VMAC_PREFER_BIG_ENDIAN)
@@ -247,9 +177,9 @@ const uint64_t mpoly = UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
 
 #if (VMAC_ARCH_BIG_ENDIAN)
 #  define get64BE(ptr) (*(uint64_t *)(ptr))
-#  define get64LE(ptr) GET_REVERSED_64(ptr)
+#  define get64LE(ptr) bswap64(*(uint64_t *)(ptr))
 #else /* assume little-endian */
-#  define get64BE(ptr) GET_REVERSED_64(ptr)
+#  define get64BE(ptr) bswap64(*(uint64_t *)(ptr))
 #  define get64LE(ptr) (*(uint64_t *)(ptr))
 #endif
 
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 3/6] arm64/find_next_bit: Remove ext2_swab()
  2022-05-23  9:52 [PATCH v4 0/6] Implement byteswap and update references Lin Liu
  2022-05-23  9:52 ` [PATCH v4 1/6] xen: implement byteswap Lin Liu
  2022-05-23  9:52 ` [PATCH v4 2/6] crypto/vmac: Simplify code with byteswap Lin Liu
@ 2022-05-23  9:52 ` Lin Liu
  2022-05-23 10:10   ` Julien Grall
  2022-05-23  9:52 ` [PATCH v4 4/6] xen: Switch to byteswap Lin Liu
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Lin Liu @ 2022-05-23  9:52 UTC (permalink / raw)
  To: xen-devel
  Cc: Lin Liu, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk

ext2 has nothing to do with this logic.  Clean up the code with
xen/byteswap.h which now has an unsigned long helper.

No functional change.

Signed-off-by: Lin Liu <lin.liu@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien@xen.org>
Cc: Bertrand Marquis <bertrand.marquis@arm.com>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
---
 xen/arch/arm/arm64/lib/find_next_bit.c | 36 +++++---------------------
 1 file changed, 6 insertions(+), 30 deletions(-)

diff --git a/xen/arch/arm/arm64/lib/find_next_bit.c b/xen/arch/arm/arm64/lib/find_next_bit.c
index 8ebf8bfe97..e3b3720ff4 100644
--- a/xen/arch/arm/arm64/lib/find_next_bit.c
+++ b/xen/arch/arm/arm64/lib/find_next_bit.c
@@ -161,30 +161,6 @@ EXPORT_SYMBOL(find_first_zero_bit);
 
 #ifdef __BIG_ENDIAN
 
-/* include/linux/byteorder does not support "unsigned long" type */
-static inline unsigned long ext2_swabp(const unsigned long * x)
-{
-#if BITS_PER_LONG == 64
-	return (unsigned long) __swab64p((u64 *) x);
-#elif BITS_PER_LONG == 32
-	return (unsigned long) __swab32p((u32 *) x);
-#else
-#error BITS_PER_LONG not defined
-#endif
-}
-
-/* include/linux/byteorder doesn't support "unsigned long" type */
-static inline unsigned long ext2_swab(const unsigned long y)
-{
-#if BITS_PER_LONG == 64
-	return (unsigned long) __swab64((u64) y);
-#elif BITS_PER_LONG == 32
-	return (unsigned long) __swab32((u32) y);
-#else
-#error BITS_PER_LONG not defined
-#endif
-}
-
 #ifndef find_next_zero_bit_le
 unsigned long find_next_zero_bit_le(const void *addr, unsigned
 		long size, unsigned long offset)
@@ -199,7 +175,7 @@ unsigned long find_next_zero_bit_le(const void *addr, unsigned
 	size -= result;
 	offset &= (BITS_PER_LONG - 1UL);
 	if (offset) {
-		tmp = ext2_swabp(p++);
+		tmp = bswap_ul(*p++);
 		tmp |= (~0UL >> (BITS_PER_LONG - offset));
 		if (size < BITS_PER_LONG)
 			goto found_first;
@@ -217,7 +193,7 @@ unsigned long find_next_zero_bit_le(const void *addr, unsigned
 	}
 	if (!size)
 		return result;
-	tmp = ext2_swabp(p);
+	tmp = bswap_ul(*p);
 found_first:
 	tmp |= ~0UL << size;
 	if (tmp == ~0UL)	/* Are any bits zero? */
@@ -226,7 +202,7 @@ found_middle:
 	return result + ffz(tmp);
 
 found_middle_swap:
-	return result + ffz(ext2_swab(tmp));
+	return result + ffz(bswap_ul(tmp));
 }
 EXPORT_SYMBOL(find_next_zero_bit_le);
 #endif
@@ -245,7 +221,7 @@ unsigned long find_next_bit_le(const void *addr, unsigned
 	size -= result;
 	offset &= (BITS_PER_LONG - 1UL);
 	if (offset) {
-		tmp = ext2_swabp(p++);
+		tmp = bswap_ul(*p++);
 		tmp &= (~0UL << offset);
 		if (size < BITS_PER_LONG)
 			goto found_first;
@@ -264,7 +240,7 @@ unsigned long find_next_bit_le(const void *addr, unsigned
 	}
 	if (!size)
 		return result;
-	tmp = ext2_swabp(p);
+	tmp = bswap_ul(*p);
 found_first:
 	tmp &= (~0UL >> (BITS_PER_LONG - size));
 	if (tmp == 0UL)		/* Are any bits set? */
@@ -273,7 +249,7 @@ found_middle:
 	return result + __ffs(tmp);
 
 found_middle_swap:
-	return result + __ffs(ext2_swab(tmp));
+	return result + __ffs(bswap_ul(tmp));
 }
 EXPORT_SYMBOL(find_next_bit_le);
 #endif
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 4/6] xen: Switch to byteswap
  2022-05-23  9:52 [PATCH v4 0/6] Implement byteswap and update references Lin Liu
                   ` (2 preceding siblings ...)
  2022-05-23  9:52 ` [PATCH v4 3/6] arm64/find_next_bit: Remove ext2_swab() Lin Liu
@ 2022-05-23  9:52 ` Lin Liu
  2022-05-23 10:12   ` Julien Grall
  2022-05-23  9:52 ` [PATCH v4 5/6] tools: Use new byteswap helper Lin Liu
  2022-05-23  9:52 ` [PATCH v4 6/6] byteorder: Remove byteorder Lin Liu
  5 siblings, 1 reply; 20+ messages in thread
From: Lin Liu @ 2022-05-23  9:52 UTC (permalink / raw)
  To: xen-devel
  Cc: Lin Liu, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Andrew Cooper, George Dunlap, Jan Beulich, Wei Liu

Update to use byteswap to swap bytes.

No functional change.

Signed-off-by: Lin Liu <lin.liu@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien@xen.org>
Cc: Bertrand Marquis <bertrand.marquis@arm.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Wei Liu <wl@xen.org>
Changes in v4:
- Revert the __force in type casting

Changes in v3:
- Update xen/common/device_tree.c to use be32_to_cpu
- Keep const in type cast in unaligned.h

---
 xen/common/device_tree.c           | 44 +++++++++++++++---------------
 xen/common/libelf/libelf-private.h |  6 ++--
 xen/common/xz/private.h            |  2 +-
 xen/include/xen/unaligned.h        | 12 ++++----
 4 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 4aae281e89..70d3be3be6 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -171,7 +171,7 @@ bool_t dt_property_read_u32(const struct dt_device_node *np,
     if ( !val || len < sizeof(*out_value) )
         return 0;
 
-    *out_value = be32_to_cpup(val);
+    *out_value = be32_to_cpu(*val);
 
     return 1;
 }
@@ -264,7 +264,7 @@ int dt_property_read_variable_u32_array(const struct dt_device_node *np,
 
     count = sz;
     while ( count-- )
-        *out_values++ = be32_to_cpup(val++);
+        *out_values++ = be32_to_cpu(*val++);
 
     return sz;
 }
@@ -490,7 +490,7 @@ static int __dt_n_addr_cells(const struct dt_device_node *np, bool_t parent)
 
         ip = dt_get_property(np, "#address-cells", NULL);
         if ( ip )
-            return be32_to_cpup(ip);
+            return be32_to_cpu(*ip);
     } while ( np->parent );
     /* No #address-cells property for the root node */
     return DT_ROOT_NODE_ADDR_CELLS_DEFAULT;
@@ -507,7 +507,7 @@ int __dt_n_size_cells(const struct dt_device_node *np, bool_t parent)
 
         ip = dt_get_property(np, "#size-cells", NULL);
         if ( ip )
-            return be32_to_cpup(ip);
+            return be32_to_cpu(*ip);
     } while ( np->parent );
     /* No #address-cells property for the root node */
     return DT_ROOT_NODE_SIZE_CELLS_DEFAULT;
@@ -660,7 +660,7 @@ static void dt_bus_pci_count_cells(const struct dt_device_node *np,
 static unsigned int dt_bus_pci_get_flags(const __be32 *addr)
 {
     unsigned int flags = 0;
-    u32 w = be32_to_cpup(addr);
+    u32 w = be32_to_cpu(*addr);
 
     switch((w >> 24) & 0x03) {
     case 0x01:
@@ -1077,7 +1077,7 @@ dt_irq_find_parent(const struct dt_device_node *child)
         if ( parp == NULL )
             p = dt_get_parent(child);
         else
-            p = dt_find_node_by_phandle(be32_to_cpup(parp));
+            p = dt_find_node_by_phandle(be32_to_cpu(*parp));
         child = p;
     } while ( p && dt_get_property(p, "#interrupt-cells", NULL) == NULL );
 
@@ -1110,7 +1110,7 @@ unsigned int dt_number_of_irq(const struct dt_device_node *device)
     intlen /= sizeof(*intspec);
 
     dt_dprintk(" using 'interrupts' property\n");
-    dt_dprintk(" intspec=%d intlen=%d\n", be32_to_cpup(intspec), intlen);
+    dt_dprintk(" intspec=%d intlen=%d\n", be32_to_cpu(*intspec), intlen);
 
     /* Look for the interrupt parent. */
     p = dt_irq_find_parent(device);
@@ -1241,7 +1241,7 @@ int dt_for_each_irq_map(const struct dt_device_node *dev,
         imaplen -= addrsize + intsize;
 
         /* Get the interrupt parent */
-        ipar = dt_find_node_by_phandle(be32_to_cpup(imap));
+        ipar = dt_find_node_by_phandle(be32_to_cpu(*imap));
         imap++;
         --imaplen;
 
@@ -1358,8 +1358,8 @@ static int dt_irq_map_raw(const struct dt_device_node *parent,
     int match, i;
 
     dt_dprintk("dt_irq_map_raw: par=%s,intspec=[0x%08x 0x%08x...],ointsize=%d\n",
-               parent->full_name, be32_to_cpup(intspec),
-               be32_to_cpup(intspec + 1), ointsize);
+               parent->full_name, be32_to_cpu(*intspec),
+               be32_to_cpu(*(intspec+1)), ointsize);
 
     ipar = parent;
 
@@ -1471,7 +1471,7 @@ static int dt_irq_map_raw(const struct dt_device_node *parent,
             dt_dprintk(" -> match=%d (imaplen=%d)\n", match, imaplen);
 
             /* Get the interrupt parent */
-            newpar = dt_find_node_by_phandle(be32_to_cpup(imap));
+            newpar = dt_find_node_by_phandle(be32_to_cpu(*imap));
             imap++;
             --imaplen;
 
@@ -1565,7 +1565,7 @@ int dt_device_get_raw_irq(const struct dt_device_node *device,
     intlen /= sizeof(*intspec);
 
     dt_dprintk(" using 'interrupts' property\n");
-    dt_dprintk(" intspec=%d intlen=%d\n", be32_to_cpup(intspec), intlen);
+    dt_dprintk(" intspec=%d intlen=%d\n", be32_to_cpu(*intspec), intlen);
 
     /* Look for the interrupt parent. */
     p = dt_irq_find_parent(device);
@@ -1676,7 +1676,7 @@ static int __dt_parse_phandle_with_args(const struct dt_device_node *np,
          * If phandle is 0, then it is an empty entry with no
          * arguments.  Skip forward to the next entry.
          * */
-        phandle = be32_to_cpup(list++);
+        phandle = be32_to_cpu(*list++);
         if ( phandle )
         {
             /*
@@ -1745,7 +1745,7 @@ static int __dt_parse_phandle_with_args(const struct dt_device_node *np,
                 out_args->np = node;
                 out_args->args_count = count;
                 for ( i = 0; i < count; i++ )
-                    out_args->args[i] = be32_to_cpup(list++);
+                    out_args->args[i] = be32_to_cpu(*list++);
             }
 
             /* Found it! return success */
@@ -1826,7 +1826,7 @@ static unsigned long __init unflatten_dt_node(const void *fdt,
     int has_name = 0;
     int new_format = 0;
 
-    tag = be32_to_cpup((__be32 *)(*p));
+    tag = be32_to_cpu(*(__be32 *)(*p));
     if ( tag != FDT_BEGIN_NODE )
     {
         printk(XENLOG_WARNING "Weird tag at start of node: %x\n", tag);
@@ -1919,7 +1919,7 @@ static unsigned long __init unflatten_dt_node(const void *fdt,
         u32 sz, noff;
         const char *pname;
 
-        tag = be32_to_cpup((__be32 *)(*p));
+        tag = be32_to_cpu(*(__be32 *)(*p));
         if ( tag == FDT_NOP )
         {
             *p += 4;
@@ -1928,8 +1928,8 @@ static unsigned long __init unflatten_dt_node(const void *fdt,
         if ( tag != FDT_PROP )
             break;
         *p += 4;
-        sz = be32_to_cpup((__be32 *)(*p));
-        noff = be32_to_cpup((__be32 *)((*p) + 4));
+        sz = be32_to_cpu(*(__be32 *)(*p));
+        noff = be32_to_cpu(*(__be32 *)((*p) + 4));
         *p += 8;
         if ( fdt_version(fdt) < 0x10 )
             *p = ROUNDUP(*p, sz >= 8 ? 8 : 4);
@@ -1956,13 +1956,13 @@ static unsigned long __init unflatten_dt_node(const void *fdt,
                  (strcmp(pname, "linux,phandle") == 0) )
             {
                 if ( np->phandle == 0 )
-                    np->phandle = be32_to_cpup((__be32*)*p);
+                    np->phandle = be32_to_cpu(*(__be32*)*p);
             }
             /* And we process the "ibm,phandle" property
              * used in pSeries dynamic device tree
              * stuff */
             if ( strcmp(pname, "ibm,phandle") == 0 )
-                np->phandle = be32_to_cpup((__be32 *)*p);
+                np->phandle = be32_to_cpu(*(__be32 *)*p);
             pp->name = pname;
             pp->length = sz;
             pp->value = (void *)*p;
@@ -2034,7 +2034,7 @@ static unsigned long __init unflatten_dt_node(const void *fdt,
             *p += 4;
         else
             mem = unflatten_dt_node(fdt, mem, p, np, allnextpp, fpsize);
-        tag = be32_to_cpup((__be32 *)(*p));
+        tag = be32_to_cpu(*(__be32 *)(*p));
     }
     if ( tag != FDT_END_NODE )
     {
@@ -2086,7 +2086,7 @@ static void __init __unflatten_device_tree(const void *fdt,
     /* Second pass, do actual unflattening */
     start = ((unsigned long)fdt) + fdt_off_dt_struct(fdt);
     unflatten_dt_node(fdt, mem, &start, NULL, &allnextp, 0);
-    if ( be32_to_cpup((__be32 *)start) != FDT_END )
+    if ( be32_to_cpu(*(__be32 *)start) != FDT_END )
         printk(XENLOG_WARNING "Weird tag at end of tree: %08x\n",
                   *((u32 *)start));
     if ( be32_to_cpu(((__be32 *)mem)[size / 4]) != 0xdeadbeef )
diff --git a/xen/common/libelf/libelf-private.h b/xen/common/libelf/libelf-private.h
index 47db679966..6062598fb8 100644
--- a/xen/common/libelf/libelf-private.h
+++ b/xen/common/libelf/libelf-private.h
@@ -31,9 +31,9 @@
    printk(fmt, ## args )
 
 #define strtoull(str, end, base) simple_strtoull(str, end, base)
-#define bswap_16(x) swab16(x)
-#define bswap_32(x) swab32(x)
-#define bswap_64(x) swab64(x)
+#define bswap_16(x) bswap16(x)
+#define bswap_32(x) bswap32(x)
+#define bswap_64(x) bswap64(x)
 
 #else /* !__XEN__ */
 
diff --git a/xen/common/xz/private.h b/xen/common/xz/private.h
index 511343fcc2..97131fa714 100644
--- a/xen/common/xz/private.h
+++ b/xen/common/xz/private.h
@@ -28,7 +28,7 @@ static inline void put_unaligned_le32(u32 val, void *p)
 
 #endif
 
-#define get_le32(p) le32_to_cpup((const uint32_t *)(p))
+#define get_le32(p) le32_to_cpu(*(const uint32_t *)(p))
 
 #define false 0
 #define true 1
diff --git a/xen/include/xen/unaligned.h b/xen/include/xen/unaligned.h
index 0a2b16d05d..56807bd157 100644
--- a/xen/include/xen/unaligned.h
+++ b/xen/include/xen/unaligned.h
@@ -20,7 +20,7 @@
 
 static inline uint16_t get_unaligned_be16(const void *p)
 {
-	return be16_to_cpup(p);
+	return be16_to_cpu(*(const uint16_t *)p);
 }
 
 static inline void put_unaligned_be16(uint16_t val, void *p)
@@ -30,7 +30,7 @@ static inline void put_unaligned_be16(uint16_t val, void *p)
 
 static inline uint32_t get_unaligned_be32(const void *p)
 {
-	return be32_to_cpup(p);
+	return be32_to_cpu(*(const uint32_t *)p);
 }
 
 static inline void put_unaligned_be32(uint32_t val, void *p)
@@ -40,7 +40,7 @@ static inline void put_unaligned_be32(uint32_t val, void *p)
 
 static inline uint64_t get_unaligned_be64(const void *p)
 {
-	return be64_to_cpup(p);
+	return be64_to_cpu(*(const uint64_t *)p);
 }
 
 static inline void put_unaligned_be64(uint64_t val, void *p)
@@ -50,7 +50,7 @@ static inline void put_unaligned_be64(uint64_t val, void *p)
 
 static inline uint16_t get_unaligned_le16(const void *p)
 {
-	return le16_to_cpup(p);
+	return le16_to_cpu(*(const uint16_t *)p);
 }
 
 static inline void put_unaligned_le16(uint16_t val, void *p)
@@ -60,7 +60,7 @@ static inline void put_unaligned_le16(uint16_t val, void *p)
 
 static inline uint32_t get_unaligned_le32(const void *p)
 {
-	return le32_to_cpup(p);
+	return le32_to_cpu(*(const uint32_t *)p);
 }
 
 static inline void put_unaligned_le32(uint32_t val, void *p)
@@ -70,7 +70,7 @@ static inline void put_unaligned_le32(uint32_t val, void *p)
 
 static inline uint64_t get_unaligned_le64(const void *p)
 {
-	return le64_to_cpup(p);
+	return le64_to_cpu(*(const uint64_t *)p);
 }
 
 static inline void put_unaligned_le64(uint64_t val, void *p)
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 5/6] tools: Use new byteswap helper
  2022-05-23  9:52 [PATCH v4 0/6] Implement byteswap and update references Lin Liu
                   ` (3 preceding siblings ...)
  2022-05-23  9:52 ` [PATCH v4 4/6] xen: Switch to byteswap Lin Liu
@ 2022-05-23  9:52 ` Lin Liu
  2022-05-23 11:10   ` Jan Beulich
  2022-05-23  9:52 ` [PATCH v4 6/6] byteorder: Remove byteorder Lin Liu
  5 siblings, 1 reply; 20+ messages in thread
From: Lin Liu @ 2022-05-23  9:52 UTC (permalink / raw)
  To: xen-devel; +Cc: Lin Liu, Wei Liu, Anthony PERARD, Juergen Gross

Include new header to use new byteswap helper

No functional change.

Signed-off-by: Lin Liu <lin.liu@citrix.com>
---
Cc: Wei Liu <wl@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Juergen Gross <jgross@suse.com>
---
 tools/libs/guest/xg_dom_decompress_unsafe_xz.c   | 5 +++++
 tools/libs/guest/xg_dom_decompress_unsafe_zstd.c | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/libs/guest/xg_dom_decompress_unsafe_xz.c b/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
index fc48198741..493427d517 100644
--- a/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
+++ b/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
@@ -34,6 +34,11 @@ static inline u32 le32_to_cpup(const u32 *p)
 	return cpu_to_le32(*p);
 }
 
+static inline u32 le32_to_cpu(u32 val)
+{
+   return le32_to_cpup((const u32 *)&val);
+}
+
 #define __force
 #define always_inline
 
diff --git a/tools/libs/guest/xg_dom_decompress_unsafe_zstd.c b/tools/libs/guest/xg_dom_decompress_unsafe_zstd.c
index 01eafaaaa6..b06f2e767f 100644
--- a/tools/libs/guest/xg_dom_decompress_unsafe_zstd.c
+++ b/tools/libs/guest/xg_dom_decompress_unsafe_zstd.c
@@ -31,7 +31,8 @@ typedef uint64_t __be64;
 
 #define __BYTEORDER_HAS_U64__
 #define __TYPES_H__ /* xen/types.h guard */
-#include "../../xen/include/xen/byteorder/little_endian.h"
+#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
+#include "../../xen/include/xen/byteorder.h"
 #define __ASM_UNALIGNED_H__ /* asm/unaligned.h guard */
 #include "../../xen/include/xen/unaligned.h"
 #include "../../xen/include/xen/xxhash.h"
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v4 6/6] byteorder: Remove byteorder
  2022-05-23  9:52 [PATCH v4 0/6] Implement byteswap and update references Lin Liu
                   ` (4 preceding siblings ...)
  2022-05-23  9:52 ` [PATCH v4 5/6] tools: Use new byteswap helper Lin Liu
@ 2022-05-23  9:52 ` Lin Liu
  5 siblings, 0 replies; 20+ messages in thread
From: Lin Liu @ 2022-05-23  9:52 UTC (permalink / raw)
  To: xen-devel
  Cc: Lin Liu, Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall,
	Bertrand Marquis, Stefano Stabellini, Wei Liu

include/xen/byteswap.h has simplify the interface, just clean
the old interface

No functional change

Signed-off-by: Lin Liu <lin.liu@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>
Cc: Bertrand Marquis <bertrand.marquis@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Wei Liu <wl@xen.org>
---
 xen/include/xen/byteorder/big_endian.h    | 102 ------------
 xen/include/xen/byteorder/generic.h       |  68 --------
 xen/include/xen/byteorder/little_endian.h | 102 ------------
 xen/include/xen/byteorder/swab.h          | 183 ----------------------
 4 files changed, 455 deletions(-)
 delete mode 100644 xen/include/xen/byteorder/big_endian.h
 delete mode 100644 xen/include/xen/byteorder/generic.h
 delete mode 100644 xen/include/xen/byteorder/little_endian.h
 delete mode 100644 xen/include/xen/byteorder/swab.h

diff --git a/xen/include/xen/byteorder/big_endian.h b/xen/include/xen/byteorder/big_endian.h
deleted file mode 100644
index 40eb80a390..0000000000
--- a/xen/include/xen/byteorder/big_endian.h
+++ /dev/null
@@ -1,102 +0,0 @@
-#ifndef __XEN_BYTEORDER_BIG_ENDIAN_H__
-#define __XEN_BYTEORDER_BIG_ENDIAN_H__
-
-#ifndef __BIG_ENDIAN
-#define __BIG_ENDIAN 4321
-#endif
-#ifndef __BIG_ENDIAN_BITFIELD
-#define __BIG_ENDIAN_BITFIELD
-#endif
-
-#include <xen/types.h>
-#include <xen/byteorder/swab.h>
-
-#define __constant_cpu_to_le64(x) ((__force __le64)___constant_swab64((x)))
-#define __constant_le64_to_cpu(x) ___constant_swab64((__force __u64)(__le64)(x))
-#define __constant_cpu_to_le32(x) ((__force __le32)___constant_swab32((x)))
-#define __constant_le32_to_cpu(x) ___constant_swab32((__force __u32)(__le32)(x))
-#define __constant_cpu_to_le16(x) ((__force __le16)___constant_swab16((x)))
-#define __constant_le16_to_cpu(x) ___constant_swab16((__force __u16)(__le16)(x))
-#define __constant_cpu_to_be64(x) ((__force __be64)(__u64)(x))
-#define __constant_be64_to_cpu(x) ((__force __u64)(__be64)(x))
-#define __constant_cpu_to_be32(x) ((__force __be32)(__u32)(x))
-#define __constant_be32_to_cpu(x) ((__force __u32)(__be32)(x))
-#define __constant_cpu_to_be16(x) ((__force __be16)(__u16)(x))
-#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))
-#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)))
-#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
-#define __cpu_to_be64(x) ((__force __be64)(__u64)(x))
-#define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
-#define __cpu_to_be32(x) ((__force __be32)(__u32)(x))
-#define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
-#define __cpu_to_be16(x) ((__force __be16)(__u16)(x))
-#define __be16_to_cpu(x) ((__force __u16)(__be16)(x))
-
-static inline __le64 __cpu_to_le64p(const __u64 *p)
-{
-    return (__force __le64)__swab64p(p);
-}
-static inline __u64 __le64_to_cpup(const __le64 *p)
-{
-    return __swab64p((__u64 *)p);
-}
-static inline __le32 __cpu_to_le32p(const __u32 *p)
-{
-    return (__force __le32)__swab32p(p);
-}
-static inline __u32 __le32_to_cpup(const __le32 *p)
-{
-    return __swab32p((__u32 *)p);
-}
-static inline __le16 __cpu_to_le16p(const __u16 *p)
-{
-    return (__force __le16)__swab16p(p);
-}
-static inline __u16 __le16_to_cpup(const __le16 *p)
-{
-    return __swab16p((__u16 *)p);
-}
-static inline __be64 __cpu_to_be64p(const __u64 *p)
-{
-    return (__force __be64)*p;
-}
-static inline __u64 __be64_to_cpup(const __be64 *p)
-{
-    return (__force __u64)*p;
-}
-static inline __be32 __cpu_to_be32p(const __u32 *p)
-{
-    return (__force __be32)*p;
-}
-static inline __u32 __be32_to_cpup(const __be32 *p)
-{
-    return (__force __u32)*p;
-}
-static inline __be16 __cpu_to_be16p(const __u16 *p)
-{
-    return (__force __be16)*p;
-}
-static inline __u16 __be16_to_cpup(const __be16 *p)
-{
-    return (__force __u16)*p;
-}
-#define __cpu_to_le64s(x) __swab64s((x))
-#define __le64_to_cpus(x) __swab64s((x))
-#define __cpu_to_le32s(x) __swab32s((x))
-#define __le32_to_cpus(x) __swab32s((x))
-#define __cpu_to_le16s(x) __swab16s((x))
-#define __le16_to_cpus(x) __swab16s((x))
-#define __cpu_to_be64s(x) do {} while (0)
-#define __be64_to_cpus(x) do {} while (0)
-#define __cpu_to_be32s(x) do {} while (0)
-#define __be32_to_cpus(x) do {} while (0)
-#define __cpu_to_be16s(x) do {} while (0)
-#define __be16_to_cpus(x) do {} while (0)
-
-#include <xen/byteorder/generic.h>
-
-#endif /* __XEN_BYTEORDER_BIG_ENDIAN_H__ */
diff --git a/xen/include/xen/byteorder/generic.h b/xen/include/xen/byteorder/generic.h
deleted file mode 100644
index 8a0006b755..0000000000
--- a/xen/include/xen/byteorder/generic.h
+++ /dev/null
@@ -1,68 +0,0 @@
-#ifndef __XEN_BYTEORDER_GENERIC_H__
-#define __XEN_BYTEORDER_GENERIC_H__
-
-/*
- * Generic Byte-reordering support
- *
- * The "... p" macros, like le64_to_cpup, can be used with pointers
- * to unaligned data, but there will be a performance penalty on 
- * some architectures.  Use get_unaligned for unaligned data.
- *
- * The following macros are to be defined by <asm/byteorder.h>:
- *
- * Conversion of XX-bit integers (16- 32- or 64-)
- * between native CPU format and little/big endian format
- * 64-bit stuff only defined for proper architectures
- *     cpu_to_[bl]eXX(__uXX x)
- *     [bl]eXX_to_cpu(__uXX x)
- *
- * The same, but takes a pointer to the value to convert
- *     cpu_to_[bl]eXXp(__uXX x)
- *     [bl]eXX_to_cpup(__uXX x)
- *
- * The same, but change in situ
- *     cpu_to_[bl]eXXs(__uXX x)
- *     [bl]eXX_to_cpus(__uXX x)
- *
- * See asm-foo/byteorder.h for examples of how to provide
- * architecture-optimized versions
- */
-
-#define cpu_to_le64 __cpu_to_le64
-#define le64_to_cpu __le64_to_cpu
-#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
-#define cpu_to_be32 __cpu_to_be32
-#define be32_to_cpu __be32_to_cpu
-#define cpu_to_be16 __cpu_to_be16
-#define be16_to_cpu __be16_to_cpu
-#define cpu_to_le64p __cpu_to_le64p
-#define le64_to_cpup __le64_to_cpup
-#define cpu_to_le32p __cpu_to_le32p
-#define le32_to_cpup __le32_to_cpup
-#define cpu_to_le16p __cpu_to_le16p
-#define le16_to_cpup __le16_to_cpup
-#define cpu_to_be64p __cpu_to_be64p
-#define be64_to_cpup __be64_to_cpup
-#define cpu_to_be32p __cpu_to_be32p
-#define be32_to_cpup __be32_to_cpup
-#define cpu_to_be16p __cpu_to_be16p
-#define be16_to_cpup __be16_to_cpup
-#define cpu_to_le64s __cpu_to_le64s
-#define le64_to_cpus __le64_to_cpus
-#define cpu_to_le32s __cpu_to_le32s
-#define le32_to_cpus __le32_to_cpus
-#define cpu_to_le16s __cpu_to_le16s
-#define le16_to_cpus __le16_to_cpus
-#define cpu_to_be64s __cpu_to_be64s
-#define be64_to_cpus __be64_to_cpus
-#define cpu_to_be32s __cpu_to_be32s
-#define be32_to_cpus __be32_to_cpus
-#define cpu_to_be16s __cpu_to_be16s
-#define be16_to_cpus __be16_to_cpus
-
-#endif /* __XEN_BYTEORDER_GENERIC_H__ */
diff --git a/xen/include/xen/byteorder/little_endian.h b/xen/include/xen/byteorder/little_endian.h
deleted file mode 100644
index 4955632793..0000000000
--- a/xen/include/xen/byteorder/little_endian.h
+++ /dev/null
@@ -1,102 +0,0 @@
-#ifndef __XEN_BYTEORDER_LITTLE_ENDIAN_H__
-#define __XEN_BYTEORDER_LITTLE_ENDIAN_H__
-
-#ifndef __LITTLE_ENDIAN
-#define __LITTLE_ENDIAN 1234
-#endif
-#ifndef __LITTLE_ENDIAN_BITFIELD
-#define __LITTLE_ENDIAN_BITFIELD
-#endif
-
-#include <xen/types.h>
-#include <xen/byteorder/swab.h>
-
-#define __constant_cpu_to_le64(x) ((__force __le64)(__u64)(x))
-#define __constant_le64_to_cpu(x) ((__force __u64)(__le64)(x))
-#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))
-#define __constant_le16_to_cpu(x) ((__force __u16)(__le16)(x))
-#define __constant_cpu_to_be64(x) ((__force __be64)___constant_swab64((x)))
-#define __constant_be64_to_cpu(x) ___constant_swab64((__force __u64)(__be64)(x))
-#define __constant_cpu_to_be32(x) ((__force __be32)___constant_swab32((x)))
-#define __constant_be32_to_cpu(x) ___constant_swab32((__force __u32)(__be32)(x))
-#define __constant_cpu_to_be16(x) ((__force __be16)___constant_swab16((x)))
-#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))
-#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))
-#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
-#define __cpu_to_be64(x) ((__force __be64)__swab64((x)))
-#define __be64_to_cpu(x) __swab64((__force __u64)(__be64)(x))
-#define __cpu_to_be32(x) ((__force __be32)__swab32((x)))
-#define __be32_to_cpu(x) __swab32((__force __u32)(__be32)(x))
-#define __cpu_to_be16(x) ((__force __be16)__swab16((x)))
-#define __be16_to_cpu(x) __swab16((__force __u16)(__be16)(x))
-
-static inline __le64 __cpu_to_le64p(const __u64 *p)
-{
-    return (__force __le64)*p;
-}
-static inline __u64 __le64_to_cpup(const __le64 *p)
-{
-    return (__force __u64)*p;
-}
-static inline __le32 __cpu_to_le32p(const __u32 *p)
-{
-    return (__force __le32)*p;
-}
-static inline __u32 __le32_to_cpup(const __le32 *p)
-{
-    return (__force __u32)*p;
-}
-static inline __le16 __cpu_to_le16p(const __u16 *p)
-{
-    return (__force __le16)*p;
-}
-static inline __u16 __le16_to_cpup(const __le16 *p)
-{
-    return (__force __u16)*p;
-}
-static inline __be64 __cpu_to_be64p(const __u64 *p)
-{
-    return (__force __be64)__swab64p(p);
-}
-static inline __u64 __be64_to_cpup(const __be64 *p)
-{
-    return __swab64p((__u64 *)p);
-}
-static inline __be32 __cpu_to_be32p(const __u32 *p)
-{
-    return (__force __be32)__swab32p(p);
-}
-static inline __u32 __be32_to_cpup(const __be32 *p)
-{
-    return __swab32p((__u32 *)p);
-}
-static inline __be16 __cpu_to_be16p(const __u16 *p)
-{
-    return (__force __be16)__swab16p(p);
-}
-static inline __u16 __be16_to_cpup(const __be16 *p)
-{
-    return __swab16p((__u16 *)p);
-}
-#define __cpu_to_le64s(x) do {} while (0)
-#define __le64_to_cpus(x) do {} while (0)
-#define __cpu_to_le32s(x) do {} while (0)
-#define __le32_to_cpus(x) do {} while (0)
-#define __cpu_to_le16s(x) do {} while (0)
-#define __le16_to_cpus(x) do {} while (0)
-#define __cpu_to_be64s(x) __swab64s((x))
-#define __be64_to_cpus(x) __swab64s((x))
-#define __cpu_to_be32s(x) __swab32s((x))
-#define __be32_to_cpus(x) __swab32s((x))
-#define __cpu_to_be16s(x) __swab16s((x))
-#define __be16_to_cpus(x) __swab16s((x))
-
-#include <xen/byteorder/generic.h>
-
-#endif /* __XEN_BYTEORDER_LITTLE_ENDIAN_H__ */
diff --git a/xen/include/xen/byteorder/swab.h b/xen/include/xen/byteorder/swab.h
deleted file mode 100644
index b7e30f0503..0000000000
--- a/xen/include/xen/byteorder/swab.h
+++ /dev/null
@@ -1,183 +0,0 @@
-#ifndef __XEN_BYTEORDER_SWAB_H__
-#define __XEN_BYTEORDER_SWAB_H__
-
-/*
- * Byte-swapping, independently from CPU endianness
- *     swabXX[ps]?(foo)
- *
- * Francois-Rene Rideau <fare@tunes.org> 19971205
- *    separated swab functions from cpu_to_XX,
- *    to clean up support for bizarre-endian architectures.
- */
-
-/* casts are necessary for constants, because we never know how for sure
- * how U/UL/ULL map to __u16, __u32, __u64. At least not in a portable way.
- */
-#define ___swab16(x)                                    \
-({                                                      \
-    __u16 __x = (x);                                    \
-    ((__u16)(                                           \
-        (((__u16)(__x) & (__u16)0x00ffU) << 8) |        \
-        (((__u16)(__x) & (__u16)0xff00U) >> 8) ));      \
-})
-
-#define ___swab32(x)                                            \
-({                                                              \
-    __u32 __x = (x);                                            \
-    ((__u32)(                                                   \
-        (((__u32)(__x) & (__u32)0x000000ffUL) << 24) |          \
-        (((__u32)(__x) & (__u32)0x0000ff00UL) <<  8) |          \
-        (((__u32)(__x) & (__u32)0x00ff0000UL) >>  8) |          \
-        (((__u32)(__x) & (__u32)0xff000000UL) >> 24) ));        \
-})
-
-#define ___swab64(x)                                                       \
-({                                                                         \
-    __u64 __x = (x);                                                       \
-    ((__u64)(                                                              \
-        (__u64)(((__u64)(__x) & (__u64)0x00000000000000ffULL) << 56) |     \
-        (__u64)(((__u64)(__x) & (__u64)0x000000000000ff00ULL) << 40) |     \
-        (__u64)(((__u64)(__x) & (__u64)0x0000000000ff0000ULL) << 24) |     \
-        (__u64)(((__u64)(__x) & (__u64)0x00000000ff000000ULL) <<  8) |     \
-            (__u64)(((__u64)(__x) & (__u64)0x000000ff00000000ULL) >>  8) | \
-        (__u64)(((__u64)(__x) & (__u64)0x0000ff0000000000ULL) >> 24) |     \
-        (__u64)(((__u64)(__x) & (__u64)0x00ff000000000000ULL) >> 40) |     \
-        (__u64)(((__u64)(__x) & (__u64)0xff00000000000000ULL) >> 56) ));   \
-})
-
-#define ___constant_swab16(x)                   \
-    ((__u16)(                                   \
-        (((__u16)(x) & (__u16)0x00ffU) << 8) |  \
-        (((__u16)(x) & (__u16)0xff00U) >> 8) ))
-#define ___constant_swab32(x)                           \
-    ((__u32)(                                           \
-        (((__u32)(x) & (__u32)0x000000ffUL) << 24) |    \
-        (((__u32)(x) & (__u32)0x0000ff00UL) <<  8) |    \
-        (((__u32)(x) & (__u32)0x00ff0000UL) >>  8) |    \
-        (((__u32)(x) & (__u32)0xff000000UL) >> 24) ))
-#define ___constant_swab64(x)                                            \
-    ((__u64)(                                                            \
-        (__u64)(((__u64)(x) & (__u64)0x00000000000000ffULL) << 56) |     \
-        (__u64)(((__u64)(x) & (__u64)0x000000000000ff00ULL) << 40) |     \
-        (__u64)(((__u64)(x) & (__u64)0x0000000000ff0000ULL) << 24) |     \
-        (__u64)(((__u64)(x) & (__u64)0x00000000ff000000ULL) <<  8) |     \
-            (__u64)(((__u64)(x) & (__u64)0x000000ff00000000ULL) >>  8) | \
-        (__u64)(((__u64)(x) & (__u64)0x0000ff0000000000ULL) >> 24) |     \
-        (__u64)(((__u64)(x) & (__u64)0x00ff000000000000ULL) >> 40) |     \
-        (__u64)(((__u64)(x) & (__u64)0xff00000000000000ULL) >> 56) ))
-
-/*
- * provide defaults when no architecture-specific optimization is detected
- */
-#ifndef __arch__swab16
-#  define __arch__swab16(x) ({ __u16 __tmp = (x) ; ___swab16(__tmp); })
-#endif
-#ifndef __arch__swab32
-#  define __arch__swab32(x) ({ __u32 __tmp = (x) ; ___swab32(__tmp); })
-#endif
-#ifndef __arch__swab64
-#  define __arch__swab64(x) ({ __u64 __tmp = (x) ; ___swab64(__tmp); })
-#endif
-
-#ifndef __arch__swab16p
-#  define __arch__swab16p(x) __arch__swab16(*(x))
-#endif
-#ifndef __arch__swab32p
-#  define __arch__swab32p(x) __arch__swab32(*(x))
-#endif
-#ifndef __arch__swab64p
-#  define __arch__swab64p(x) __arch__swab64(*(x))
-#endif
-
-#ifndef __arch__swab16s
-#  define __arch__swab16s(x) do { *(x) = __arch__swab16p((x)); } while (0)
-#endif
-#ifndef __arch__swab32s
-#  define __arch__swab32s(x) do { *(x) = __arch__swab32p((x)); } while (0)
-#endif
-#ifndef __arch__swab64s
-#  define __arch__swab64s(x) do { *(x) = __arch__swab64p((x)); } while (0)
-#endif
-
-
-/*
- * Allow constant folding
- */
-#if defined(__GNUC__) && defined(__OPTIMIZE__)
-#  define __swab16(x) \
-(__builtin_constant_p((__u16)(x)) ? \
- ___swab16((x)) : \
- __fswab16((x)))
-#  define __swab32(x) \
-(__builtin_constant_p((__u32)(x)) ? \
- ___swab32((x)) : \
- __fswab32((x)))
-#  define __swab64(x) \
-(__builtin_constant_p((__u64)(x)) ? \
- ___swab64((x)) : \
- __fswab64((x)))
-#else
-#  define __swab16(x) __fswab16(x)
-#  define __swab32(x) __fswab32(x)
-#  define __swab64(x) __fswab64(x)
-#endif /* OPTIMIZE */
-
-
-static inline __attribute_const__ __u16 __fswab16(__u16 x)
-{
-    return __arch__swab16(x);
-}
-static inline __u16 __swab16p(const __u16 *x)
-{
-    return __arch__swab16p(x);
-}
-static inline void __swab16s(__u16 *addr)
-{
-    __arch__swab16s(addr);
-}
-
-static inline __attribute_const__ __u32 __fswab32(__u32 x)
-{
-    return __arch__swab32(x);
-}
-static inline __u32 __swab32p(const __u32 *x)
-{
-    return __arch__swab32p(x);
-}
-static inline void __swab32s(__u32 *addr)
-{
-    __arch__swab32s(addr);
-}
-
-#ifdef __BYTEORDER_HAS_U64__
-static inline __attribute_const__ __u64 __fswab64(__u64 x)
-{
-#  ifdef __SWAB_64_THRU_32__
-    __u32 h = x >> 32;
-        __u32 l = x & ((1ULL<<32)-1);
-        return (((__u64)__swab32(l)) << 32) | ((__u64)(__swab32(h)));
-#  else
-    return __arch__swab64(x);
-#  endif
-}
-static inline __u64 __swab64p(const __u64 *x)
-{
-    return __arch__swab64p(x);
-}
-static inline void __swab64s(__u64 *addr)
-{
-    __arch__swab64s(addr);
-}
-#endif /* __BYTEORDER_HAS_U64__ */
-
-#define swab16 __swab16
-#define swab32 __swab32
-#define swab64 __swab64
-#define swab16p __swab16p
-#define swab32p __swab32p
-#define swab64p __swab64p
-#define swab16s __swab16s
-#define swab32s __swab32s
-#define swab64s __swab64s
-
-#endif /* __XEN_BYTEORDER_SWAB_H__ */
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 1/6] xen: implement byteswap
  2022-05-23  9:52 ` [PATCH v4 1/6] xen: implement byteswap Lin Liu
@ 2022-05-23 10:07   ` Roger Pau Monné
  2022-05-23 11:00     ` Jan Beulich
  0 siblings, 1 reply; 20+ messages in thread
From: Roger Pau Monné @ 2022-05-23 10:07 UTC (permalink / raw)
  To: Lin Liu
  Cc: xen-devel, Andrew Cooper, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, George Dunlap, Jan Beulich,
	Wei Liu

On Mon, May 23, 2022 at 05:52:17AM -0400, Lin Liu wrote:
> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
> index 933aec09a9..ae029afa14 100644
> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -185,4 +185,28 @@
>  # define CLANG_DISABLE_WARN_GCC_COMPAT_END
>  #endif
>  
> +#ifndef __has_builtin
> +/*
> + * Backwards compatibility for GCC < 10.
> + * All supported versions of Clang support __has_builtin
> + * */
> +#define __has_builtin(x) GCC_has ## x
> +
> +#define GCC_has__builtin_bswap16 (CONFIG_GCC_VERSION >= 40800)
> +#define GCC_has__builtin_bswap32 (CONFIG_GCC_VERSION >= 40400)
> +#define GCC_has__builtin_bswap64 (CONFIG_GCC_VERSION >= 40400)
> +#endif
> +
> +#ifndef __ORDER_LITTLE_ENDIAN__
> +# define __ORDER_LITTLE_ENDIAN__ 1234
> +#endif
> +
> +#ifndef __ORDER_BIG_ENDIAN__
> +# define __ORDER_BIG_ENDIAN__ 4321
> +#endif
> +
> +/* Indirect macros required for expanded argument pasting. */
> +#define PASTE_(a, b) a ## b
> +#define PASTE(a, b) PASTE_(a, b)

I think it would be better if byteswap.h included lib.h, rather than
moving the PASTE define into compiler.h.

Likewise the __ORDER_{BIG,LITTLE}_ENDIAN__ defines would be better
placed in byteswap.h itself if possible IMO, since it's not strictly
related to the compiler.

Thanks, Roger.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 3/6] arm64/find_next_bit: Remove ext2_swab()
  2022-05-23  9:52 ` [PATCH v4 3/6] arm64/find_next_bit: Remove ext2_swab() Lin Liu
@ 2022-05-23 10:10   ` Julien Grall
  0 siblings, 0 replies; 20+ messages in thread
From: Julien Grall @ 2022-05-23 10:10 UTC (permalink / raw)
  To: Lin Liu, xen-devel
  Cc: Stefano Stabellini, Bertrand Marquis, Volodymyr Babchuk

Hi,

On 23/05/2022 10:52, Lin Liu wrote:
> ext2 has nothing to do with this logic.  Clean up the code with
> xen/byteswap.h which now has an unsigned long helper.

It looks like my comment in v3 wasn't addressed. This could possibly be 
done on commit if there are no other version.

> 
> No functional change.
> 
> Signed-off-by: Lin Liu <lin.liu@citrix.com>

You forgot to carry the tags from Andrew and I.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 4/6] xen: Switch to byteswap
  2022-05-23  9:52 ` [PATCH v4 4/6] xen: Switch to byteswap Lin Liu
@ 2022-05-23 10:12   ` Julien Grall
  2022-05-23 11:09     ` Jan Beulich
  0 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2022-05-23 10:12 UTC (permalink / raw)
  To: Lin Liu, xen-devel
  Cc: Stefano Stabellini, Bertrand Marquis, Andrew Cooper,
	George Dunlap, Jan Beulich, Wei Liu

Hi,

On 23/05/2022 10:52, Lin Liu wrote:
> Update to use byteswap to swap bytes.

I am still objecting on switching from be*_to_cpup() to be*_to_cpu().

I will not Nack, however the strict minimum is to explain why you want 
to replace the helpers as I think the reason that was currently provided 
is incorrect.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 1/6] xen: implement byteswap
  2022-05-23 10:07   ` Roger Pau Monné
@ 2022-05-23 11:00     ` Jan Beulich
  2022-05-23 11:05       ` Jan Beulich
  0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2022-05-23 11:00 UTC (permalink / raw)
  To: Roger Pau Monné, Lin Liu
  Cc: xen-devel, Andrew Cooper, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, George Dunlap, Wei Liu

On 23.05.2022 12:07, Roger Pau Monné wrote:
> On Mon, May 23, 2022 at 05:52:17AM -0400, Lin Liu wrote:
>> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
>> index 933aec09a9..ae029afa14 100644
>> --- a/xen/include/xen/compiler.h
>> +++ b/xen/include/xen/compiler.h
>> @@ -185,4 +185,28 @@
>>  # define CLANG_DISABLE_WARN_GCC_COMPAT_END
>>  #endif
>>  
>> +#ifndef __has_builtin
>> +/*
>> + * Backwards compatibility for GCC < 10.
>> + * All supported versions of Clang support __has_builtin
>> + * */
>> +#define __has_builtin(x) GCC_has ## x
>> +
>> +#define GCC_has__builtin_bswap16 (CONFIG_GCC_VERSION >= 40800)
>> +#define GCC_has__builtin_bswap32 (CONFIG_GCC_VERSION >= 40400)
>> +#define GCC_has__builtin_bswap64 (CONFIG_GCC_VERSION >= 40400)
>> +#endif
>> +
>> +#ifndef __ORDER_LITTLE_ENDIAN__
>> +# define __ORDER_LITTLE_ENDIAN__ 1234
>> +#endif
>> +
>> +#ifndef __ORDER_BIG_ENDIAN__
>> +# define __ORDER_BIG_ENDIAN__ 4321
>> +#endif
>> +
>> +/* Indirect macros required for expanded argument pasting. */
>> +#define PASTE_(a, b) a ## b
>> +#define PASTE(a, b) PASTE_(a, b)
> 
> I think it would be better if byteswap.h included lib.h, rather than
> moving the PASTE define into compiler.h.

+1

> Likewise the __ORDER_{BIG,LITTLE}_ENDIAN__ defines would be better
> placed in byteswap.h itself if possible IMO, since it's not strictly
> related to the compiler.

These need to live in per-arch headers, i.e. asm/byteorder.h.

Jan



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 1/6] xen: implement byteswap
  2022-05-23 11:00     ` Jan Beulich
@ 2022-05-23 11:05       ` Jan Beulich
  0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2022-05-23 11:05 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: xen-devel, Andrew Cooper, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, George Dunlap, Wei Liu,
	Lin Liu

On 23.05.2022 13:00, Jan Beulich wrote:
> On 23.05.2022 12:07, Roger Pau Monné wrote:
>> Likewise the __ORDER_{BIG,LITTLE}_ENDIAN__ defines would be better
>> placed in byteswap.h itself if possible IMO, since it's not strictly
>> related to the compiler.
> 
> These need to live in per-arch headers, i.e. asm/byteorder.h.

Or rather not. __BYTE_ORDER__ looks to come from the compiler, so
#define-ing respective constants in compiler.h would seem quite
reasonable to me.

Jan



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 4/6] xen: Switch to byteswap
  2022-05-23 10:12   ` Julien Grall
@ 2022-05-23 11:09     ` Jan Beulich
  0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2022-05-23 11:09 UTC (permalink / raw)
  To: Julien Grall, Lin Liu
  Cc: Stefano Stabellini, Bertrand Marquis, Andrew Cooper,
	George Dunlap, Wei Liu, xen-devel

On 23.05.2022 12:12, Julien Grall wrote:
> On 23/05/2022 10:52, Lin Liu wrote:
>> Update to use byteswap to swap bytes.
> 
> I am still objecting on switching from be*_to_cpup() to be*_to_cpu().

And I agree. Especially the cast to explicitly aligned types in
get_unaligned_*() is rather unhelpful (and kind of a lie) imo.

Jan

> I will not Nack, however the strict minimum is to explain why you want 
> to replace the helpers as I think the reason that was currently provided 
> is incorrect.
> 
> Cheers,
> 



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 5/6] tools: Use new byteswap helper
  2022-05-23  9:52 ` [PATCH v4 5/6] tools: Use new byteswap helper Lin Liu
@ 2022-05-23 11:10   ` Jan Beulich
  2022-05-24  2:13     ` Lin Liu (刘林)
  0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2022-05-23 11:10 UTC (permalink / raw)
  To: Lin Liu; +Cc: Wei Liu, Anthony PERARD, Juergen Gross, xen-devel

On 23.05.2022 11:52, Lin Liu wrote:
> --- a/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
> +++ b/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
> @@ -34,6 +34,11 @@ static inline u32 le32_to_cpup(const u32 *p)
>  	return cpu_to_le32(*p);
>  }
>  
> +static inline u32 le32_to_cpu(u32 val)
> +{
> +   return le32_to_cpup((const u32 *)&val);
> +}

Why the cast? And why not uint32_t?

Jan



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 5/6] tools: Use new byteswap helper
  2022-05-23 11:10   ` Jan Beulich
@ 2022-05-24  2:13     ` Lin Liu (刘林)
  2022-05-24  6:10       ` Jan Beulich
  0 siblings, 1 reply; 20+ messages in thread
From: Lin Liu (刘林) @ 2022-05-24  2:13 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Wei Liu, Anthony Perard, Juergen Gross, xen-devel

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

On 23.05.2022 11:52, Lin Liu wrote:
>> --- a/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
>> +++ b/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
>> @@ -34,6 +34,11 @@ static inline u32 le32_to_cpup(const u32 *p)
>>        return cpu_to_le32(*p);
>>  }
>>
>> +static inline u32 le32_to_cpu(u32 val)
>> +{
>> +   return le32_to_cpup((const u32 *)&val);
>> +}
>
>Why the cast? And why not uint32_t?
>
>Jan

le32_to_cpup has following prototye and definition

static inline u32 le32_to_cpup(const u32 *p)
{
        return cpu_to_le32(*p);
}

xg_dom_decompress_unsafe_xz.c redefine and use u32, use u32 to keep consistent
typedef uint32_t u32;

[-- Attachment #2: Type: text/html, Size: 4543 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 5/6] tools: Use new byteswap helper
  2022-05-24  2:13     ` Lin Liu (刘林)
@ 2022-05-24  6:10       ` Jan Beulich
  2022-05-24  6:52         ` Lin Liu (刘林)
  0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2022-05-24  6:10 UTC (permalink / raw)
  To: Lin Liu (刘林)
  Cc: Wei Liu, Anthony Perard, Juergen Gross, xen-devel

On 24.05.2022 04:13, Lin Liu (刘林) wrote:
> On 23.05.2022 11:52, Lin Liu wrote:
>>> --- a/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
>>> +++ b/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
>>> @@ -34,6 +34,11 @@ static inline u32 le32_to_cpup(const u32 *p)
>>>        return cpu_to_le32(*p);
>>>  }
>>>
>>> +static inline u32 le32_to_cpu(u32 val)
>>> +{
>>> +   return le32_to_cpup((const u32 *)&val);
>>> +}
>>
>> Why the cast? And why not uint32_t?
>>
>> Jan
> 
> le32_to_cpup has following prototye and definition
> 
> static inline u32 le32_to_cpup(const u32 *p)
> {
>         return cpu_to_le32(*p);
> }
> 
> xg_dom_decompress_unsafe_xz.c redefine and use u32, use u32 to keep consistent
> typedef uint32_t u32;

This answers neither part of my question. For u32 vs uint32_t, please
also see ./CODING_STYLE.

Jan



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 5/6] tools: Use new byteswap helper
  2022-05-24  6:10       ` Jan Beulich
@ 2022-05-24  6:52         ` Lin Liu (刘林)
  2022-05-24  6:58           ` Jan Beulich
  0 siblings, 1 reply; 20+ messages in thread
From: Lin Liu (刘林) @ 2022-05-24  6:52 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Wei Liu, Anthony Perard, Juergen Gross, xen-devel

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

>> On 23.05.2022 11:52, Lin Liu wrote:
>>>> --- a/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
>>>> +++ b/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
>>>> @@ -34,6 +34,11 @@ static inline u32 le32_to_cpup(const u32 *p)
>>>>        return cpu_to_le32(*p);
>>>>  }
>>>>
>>>> +static inline u32 le32_to_cpu(u32 val)
>>>> +{
>>>> +   return le32_to_cpup((const u32 *)&val);
>>>> +}
>>>
>>> Why the cast? And why not uint32_t?
>>>
>>> Jan
>>
>> le32_to_cpup has following prototye and definition
>>
>> static inline u32 le32_to_cpup(const u32 *p)
>> {
>>         return cpu_to_le32(*p);
>> }
>>
>> xg_dom_decompress_unsafe_xz.c redefine and use u32, use u32 to keep consistent
>> typedef uint32_t u32;
>
>This answers neither part of my question. For u32 vs uint32_t, please
>also see ./CODING_STYLE.

Type cast is unnecessary, will be removed in next version of patch
CODING_STYLE encourage uint32_t instead of u32,
However, Current xg_dom_decompress_unsafe_xz.c already use u32 instead of unit32_t, so I
use u32 to keep censistent, otherwise, the code look strange

[-- Attachment #2: Type: text/html, Size: 5307 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 5/6] tools: Use new byteswap helper
  2022-05-24  6:52         ` Lin Liu (刘林)
@ 2022-05-24  6:58           ` Jan Beulich
  2022-05-24  7:17             ` Lin Liu (刘林)
  0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2022-05-24  6:58 UTC (permalink / raw)
  To: Lin Liu (刘林)
  Cc: Wei Liu, Anthony Perard, Juergen Gross, xen-devel

On 24.05.2022 08:52, Lin Liu (刘林) wrote:
>>> On 23.05.2022 11:52, Lin Liu wrote:
>>>>> --- a/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
>>>>> +++ b/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
>>>>> @@ -34,6 +34,11 @@ static inline u32 le32_to_cpup(const u32 *p)
>>>>>        return cpu_to_le32(*p);
>>>>>  }
>>>>>
>>>>> +static inline u32 le32_to_cpu(u32 val)
>>>>> +{
>>>>> +   return le32_to_cpup((const u32 *)&val);
>>>>> +}
>>>>
>>>> Why the cast? And why not uint32_t?
>>>>
>>>> Jan
>>>
>>> le32_to_cpup has following prototye and definition
>>>
>>> static inline u32 le32_to_cpup(const u32 *p)
>>> {
>>>         return cpu_to_le32(*p);
>>> }
>>>
>>> xg_dom_decompress_unsafe_xz.c redefine and use u32, use u32 to keep consistent
>>> typedef uint32_t u32;
>>
>> This answers neither part of my question. For u32 vs uint32_t, please
>> also see ./CODING_STYLE.
> 
> Type cast is unnecessary, will be removed in next version of patch
> CODING_STYLE encourage uint32_t instead of u32,
> However, Current xg_dom_decompress_unsafe_xz.c already use u32 instead of unit32_t, so I
> use u32 to keep censistent, otherwise, the code look strange

Strange or not, that's the only way to phase out certain things without
using gigantic patches / series touching the entire tree at one time.
New code should not use these deprecated (for our purposes) types
anymore. Note how the file you adjust here already has to introduce
these type aliases for things to build. These typedefs really want to
go away, and any new use of those types is another hindrance in doing
so.

Jan



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 5/6] tools: Use new byteswap helper
  2022-05-24  6:58           ` Jan Beulich
@ 2022-05-24  7:17             ` Lin Liu (刘林)
  2022-05-24  8:52               ` Jan Beulich
  0 siblings, 1 reply; 20+ messages in thread
From: Lin Liu (刘林) @ 2022-05-24  7:17 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Wei Liu, Anthony Perard, Juergen Gross, xen-devel

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

>>>> On 23.05.2022 11:52, Lin Liu wrote:
>>>>>> --- a/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
>>>>>> +++ b/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
>>>>>> @@ -34,6 +34,11 @@ static inline u32 le32_to_cpup(const u32 *p)
>>>>>>        return cpu_to_le32(*p);
>>>>>>  }
>>>>>>
>>>>>> +static inline u32 le32_to_cpu(u32 val)
>>>>>> +{
>>>>>> +   return le32_to_cpup((const u32 *)&val);
>>>>>> +}
>>>>>
>>>>> Why the cast? And why not uint32_t?
>>>>>
>>>>> Jan
>>>>
>>>> le32_to_cpup has following prototye and definition
>>>>
>>>> static inline u32 le32_to_cpup(const u32 *p)
>>>> {
>>>>         return cpu_to_le32(*p);
>>>> }
>>>>
>>>> xg_dom_decompress_unsafe_xz.c redefine and use u32, use u32 to keep consistent
>>>> typedef uint32_t u32;
>>>
>>> This answers neither part of my question. For u32 vs uint32_t, please
>>> also see ./CODING_STYLE.
>>
>> Type cast is unnecessary, will be removed in next version of patch
>> CODING_STYLE encourage uint32_t instead of u32,
>> However, Current xg_dom_decompress_unsafe_xz.c already use u32 instead of unit32_t, so I
>> use u32 to keep censistent, otherwise, the code look strange
>
>Strange or not, that's the only way to phase out certain things without
>using gigantic patches / series touching the entire tree at one time.
>New code should not use these deprecated (for our purposes) types
>anymore. Note how the file you adjust here already has to introduce
>these type aliases for things to build. These typedefs really want to
>go away, and any new use of those types is another hindrance in doing

well, you convinced me to use uint32_t instead of u32.
However, This patch will not update other u32(s) to get focus.
I can raise another patch to update parts if necessary.

Cheers,
Lin

[-- Attachment #2: Type: text/html, Size: 7146 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 5/6] tools: Use new byteswap helper
  2022-05-24  7:17             ` Lin Liu (刘林)
@ 2022-05-24  8:52               ` Jan Beulich
  0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2022-05-24  8:52 UTC (permalink / raw)
  To: Lin Liu (刘林)
  Cc: Wei Liu, Anthony Perard, Juergen Gross, xen-devel

On 24.05.2022 09:17, Lin Liu (刘林) wrote:
>>>>> On 23.05.2022 11:52, Lin Liu wrote:
>>>>>>> --- a/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
>>>>>>> +++ b/tools/libs/guest/xg_dom_decompress_unsafe_xz.c
>>>>>>> @@ -34,6 +34,11 @@ static inline u32 le32_to_cpup(const u32 *p)
>>>>>>>        return cpu_to_le32(*p);
>>>>>>>  }
>>>>>>>
>>>>>>> +static inline u32 le32_to_cpu(u32 val)
>>>>>>> +{
>>>>>>> +   return le32_to_cpup((const u32 *)&val);
>>>>>>> +}
>>>>>>
>>>>>> Why the cast? And why not uint32_t?
>>>>>>
>>>>>> Jan
>>>>>
>>>>> le32_to_cpup has following prototye and definition
>>>>>
>>>>> static inline u32 le32_to_cpup(const u32 *p)
>>>>> {
>>>>>         return cpu_to_le32(*p);
>>>>> }
>>>>>
>>>>> xg_dom_decompress_unsafe_xz.c redefine and use u32, use u32 to keep consistent
>>>>> typedef uint32_t u32;
>>>>
>>>> This answers neither part of my question. For u32 vs uint32_t, please
>>>> also see ./CODING_STYLE.
>>>
>>> Type cast is unnecessary, will be removed in next version of patch
>>> CODING_STYLE encourage uint32_t instead of u32,
>>> However, Current xg_dom_decompress_unsafe_xz.c already use u32 instead of unit32_t, so I
>>> use u32 to keep censistent, otherwise, the code look strange
>>
>> Strange or not, that's the only way to phase out certain things without
>> using gigantic patches / series touching the entire tree at one time.
>> New code should not use these deprecated (for our purposes) types
>> anymore. Note how the file you adjust here already has to introduce
>> these type aliases for things to build. These typedefs really want to
>> go away, and any new use of those types is another hindrance in doing
> 
> well, you convinced me to use uint32_t instead of u32.
> However, This patch will not update other u32(s) to get focus.

Of course, that's fine.

> I can raise another patch to update parts if necessary.

FTAOD: This would certainly be appreciated, but is by no means a
requirement here.

Jan



^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2022-05-24  8:53 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-23  9:52 [PATCH v4 0/6] Implement byteswap and update references Lin Liu
2022-05-23  9:52 ` [PATCH v4 1/6] xen: implement byteswap Lin Liu
2022-05-23 10:07   ` Roger Pau Monné
2022-05-23 11:00     ` Jan Beulich
2022-05-23 11:05       ` Jan Beulich
2022-05-23  9:52 ` [PATCH v4 2/6] crypto/vmac: Simplify code with byteswap Lin Liu
2022-05-23  9:52 ` [PATCH v4 3/6] arm64/find_next_bit: Remove ext2_swab() Lin Liu
2022-05-23 10:10   ` Julien Grall
2022-05-23  9:52 ` [PATCH v4 4/6] xen: Switch to byteswap Lin Liu
2022-05-23 10:12   ` Julien Grall
2022-05-23 11:09     ` Jan Beulich
2022-05-23  9:52 ` [PATCH v4 5/6] tools: Use new byteswap helper Lin Liu
2022-05-23 11:10   ` Jan Beulich
2022-05-24  2:13     ` Lin Liu (刘林)
2022-05-24  6:10       ` Jan Beulich
2022-05-24  6:52         ` Lin Liu (刘林)
2022-05-24  6:58           ` Jan Beulich
2022-05-24  7:17             ` Lin Liu (刘林)
2022-05-24  8:52               ` Jan Beulich
2022-05-23  9:52 ` [PATCH v4 6/6] byteorder: Remove byteorder Lin Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.