xen/arch/arm/include/asm/bitops.h | 4 +--- xen/arch/arm/include/asm/config.h | 8 -------- xen/arch/ppc/include/asm/bitops.h | 4 +--- xen/arch/ppc/include/asm/config.h | 7 ------- xen/arch/riscv/include/asm/config.h | 13 ------------- xen/arch/x86/include/asm/config.h | 8 -------- xen/include/xen/config.h | 10 ++++++++++ 7 files changed, 12 insertions(+), 42 deletions(-)
BITS_PER_* values can be defined in a common way using compiler-provided macros.
Thus, these definitions are moved to xen/config.h to reduce duplication across
architectures.
Additionally, *_BYTEORDER macros are removed, as BITS_PER_* values now come
directly from the compiler environment.
The arch_fls() implementation for Arm and PPC is updated to use BITS_PER_INT
instead of a hardcoded value of 32.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
GitLab CI tests results:
https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/1738505446
As Anrew mentioned here https://lore.kernel.org/xen-devel/f294e2b1-db03-46b2-b46d-61e89b55cef3@suse.com/T/#ma4205392964ff913d9dfa8e044a4af59ed6aef88:
"This patch possibly wants to wait until I've fixed the compiler checks to update to the new baseline, ..."
The patch with compiler fixes is https://lore.kernel.org/xen-devel/f79117a2-1763-4458-862f-a5219b09989a@citrix.com/T/#mf79cf8c1f6f6a3661c49bac84c1c15bbaae7422d.
---
Changes in v2:
- Add the comment "It is assumed that sizeof(void *) == __alignof(void *)"
above POINTER_ALIGN definition.
- Replace "<< 3" with "BITS_PER_BYTE" in definitions of
BITS_PER_{INT,LONG,LONG_LONG}.
---
xen/arch/arm/include/asm/bitops.h | 4 +---
xen/arch/arm/include/asm/config.h | 8 --------
xen/arch/ppc/include/asm/bitops.h | 4 +---
xen/arch/ppc/include/asm/config.h | 7 -------
xen/arch/riscv/include/asm/config.h | 13 -------------
xen/arch/x86/include/asm/config.h | 8 --------
xen/include/xen/config.h | 10 ++++++++++
7 files changed, 12 insertions(+), 42 deletions(-)
diff --git a/xen/arch/arm/include/asm/bitops.h b/xen/arch/arm/include/asm/bitops.h
index f163d9bb45..60686a3a55 100644
--- a/xen/arch/arm/include/asm/bitops.h
+++ b/xen/arch/arm/include/asm/bitops.h
@@ -22,8 +22,6 @@
#define __set_bit(n,p) set_bit(n,p)
#define __clear_bit(n,p) clear_bit(n,p)
-#define BITS_PER_BYTE 8
-
#define ADDR (*(volatile int *) addr)
#define CONST_ADDR (*(const volatile int *) addr)
@@ -75,7 +73,7 @@ bool clear_mask16_timeout(uint16_t mask, volatile void *p,
#define arch_ffs(x) ((x) ? 1 + __builtin_ctz(x) : 0)
#define arch_ffsl(x) ((x) ? 1 + __builtin_ctzl(x) : 0)
-#define arch_fls(x) ((x) ? 32 - __builtin_clz(x) : 0)
+#define arch_fls(x) ((x) ? BITS_PER_INT - __builtin_clz(x) : 0)
#define arch_flsl(x) ((x) ? BITS_PER_LONG - __builtin_clzl(x) : 0)
#endif /* _ARM_BITOPS_H */
diff --git a/xen/arch/arm/include/asm/config.h b/xen/arch/arm/include/asm/config.h
index 0a51142efd..5a02db6937 100644
--- a/xen/arch/arm/include/asm/config.h
+++ b/xen/arch/arm/include/asm/config.h
@@ -8,19 +8,11 @@
#define __ARM_CONFIG_H__
#if defined(CONFIG_ARM_64)
-# define LONG_BYTEORDER 3
# define ELFSIZE 64
#else
-# define LONG_BYTEORDER 2
# define ELFSIZE 32
#endif
-#define BYTES_PER_LONG (1 << LONG_BYTEORDER)
-#define BITS_PER_LONG (BYTES_PER_LONG << 3)
-#define POINTER_ALIGN BYTES_PER_LONG
-
-#define BITS_PER_LLONG 64
-
/* xen_ulong_t is always 64 bits */
#define BITS_PER_XEN_ULONG 64
diff --git a/xen/arch/ppc/include/asm/bitops.h b/xen/arch/ppc/include/asm/bitops.h
index c942e9432e..e72942cca0 100644
--- a/xen/arch/ppc/include/asm/bitops.h
+++ b/xen/arch/ppc/include/asm/bitops.h
@@ -15,8 +15,6 @@
#define __set_bit(n, p) set_bit(n, p)
#define __clear_bit(n, p) clear_bit(n, p)
-#define BITS_PER_BYTE 8
-
/* PPC bit number conversion */
#define PPC_BITLSHIFT(be) (BITS_PER_LONG - 1 - (be))
#define PPC_BIT(bit) (1UL << PPC_BITLSHIFT(bit))
@@ -121,7 +119,7 @@ static inline int test_and_set_bit(unsigned int nr, volatile void *addr)
#define arch_ffs(x) ((x) ? 1 + __builtin_ctz(x) : 0)
#define arch_ffsl(x) ((x) ? 1 + __builtin_ctzl(x) : 0)
-#define arch_fls(x) ((x) ? 32 - __builtin_clz(x) : 0)
+#define arch_fls(x) ((x) ? BITS_PER_INT - __builtin_clz(x) : 0)
#define arch_flsl(x) ((x) ? BITS_PER_LONG - __builtin_clzl(x) : 0)
#define arch_hweightl(x) __builtin_popcountl(x)
diff --git a/xen/arch/ppc/include/asm/config.h b/xen/arch/ppc/include/asm/config.h
index 148fb3074d..8e32edd5a5 100644
--- a/xen/arch/ppc/include/asm/config.h
+++ b/xen/arch/ppc/include/asm/config.h
@@ -6,19 +6,12 @@
#include <xen/page-size.h>
#if defined(CONFIG_PPC64)
-#define LONG_BYTEORDER 3
#define ELFSIZE 64
#define MAX_VIRT_CPUS 1024u
#else
#error "Unsupported PowerPC variant"
#endif
-#define BYTES_PER_LONG (1 << LONG_BYTEORDER)
-#define BITS_PER_LONG (BYTES_PER_LONG << 3)
-#define POINTER_ALIGN BYTES_PER_LONG
-
-#define BITS_PER_LLONG 64
-
/* xen_ulong_t is always 64 bits */
#define BITS_PER_XEN_ULONG 64
diff --git a/xen/arch/riscv/include/asm/config.h b/xen/arch/riscv/include/asm/config.h
index 7141bd9e46..314c97c20a 100644
--- a/xen/arch/riscv/include/asm/config.h
+++ b/xen/arch/riscv/include/asm/config.h
@@ -119,25 +119,12 @@
#define HYPERVISOR_VIRT_START XEN_VIRT_START
#if defined(CONFIG_RISCV_64)
-# define INT_BYTEORDER 2
-# define LONG_BYTEORDER 3
# define ELFSIZE 64
# define MAX_VIRT_CPUS 128u
#else
# error "Unsupported RISCV variant"
#endif
-#define BYTES_PER_INT (1 << INT_BYTEORDER)
-#define BITS_PER_INT (BYTES_PER_INT << 3)
-
-#define BYTES_PER_LONG (1 << LONG_BYTEORDER)
-#define BITS_PER_LONG (BYTES_PER_LONG << 3)
-#define POINTER_ALIGN BYTES_PER_LONG
-
-#define BITS_PER_LLONG 64
-
-#define BITS_PER_BYTE 8
-
/* xen_ulong_t is always 64 bits */
#define BITS_PER_XEN_ULONG 64
diff --git a/xen/arch/x86/include/asm/config.h b/xen/arch/x86/include/asm/config.h
index 19746f956e..f0123a7de9 100644
--- a/xen/arch/x86/include/asm/config.h
+++ b/xen/arch/x86/include/asm/config.h
@@ -7,16 +7,8 @@
#ifndef __X86_CONFIG_H__
#define __X86_CONFIG_H__
-#define LONG_BYTEORDER 3
#define CONFIG_PAGING_LEVELS 4
-#define BYTES_PER_LONG (1 << LONG_BYTEORDER)
-#define BITS_PER_LONG (BYTES_PER_LONG << 3)
-#define BITS_PER_BYTE 8
-#define POINTER_ALIGN BYTES_PER_LONG
-
-#define BITS_PER_LLONG 64
-
#define BITS_PER_XEN_ULONG BITS_PER_LONG
#define CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS 1
diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
index d888b2314d..7d43159efb 100644
--- a/xen/include/xen/config.h
+++ b/xen/include/xen/config.h
@@ -98,4 +98,14 @@
#define ZERO_BLOCK_PTR ((void *)-1L)
#endif
+#define BYTES_PER_LONG __SIZEOF_LONG__
+
+#define BITS_PER_BYTE __CHAR_BIT__
+#define BITS_PER_INT (BITS_PER_BYTE * __SIZEOF_INT__)
+#define BITS_PER_LONG (BITS_PER_BYTE * BYTES_PER_LONG)
+#define BITS_PER_LLONG (BITS_PER_BYTE * __SIZEOF_LONG_LONG__)
+
+/* It is assumed that sizeof(void *) == __alignof(void *) */
+#define POINTER_ALIGN __SIZEOF_POINTER__
+
#endif /* __XEN_CONFIG_H__ */
--
2.49.0Hi, On 27/03/2025 17:33, Oleksii Kurochko wrote: > BITS_PER_* values can be defined in a common way using compiler-provided macros. > Thus, these definitions are moved to xen/config.h to reduce duplication across > architectures. > > Additionally, *_BYTEORDER macros are removed, as BITS_PER_* values now come > directly from the compiler environment. > > The arch_fls() implementation for Arm and PPC is updated to use BITS_PER_INT > instead of a hardcoded value of 32. > > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> Acked-by: Julien Grall <jgrall@amazon.com> Cheers, -- Julien Grall
On 27/03/2025 5:33 pm, Oleksii Kurochko wrote: > diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h > index d888b2314d..7d43159efb 100644 > --- a/xen/include/xen/config.h > +++ b/xen/include/xen/config.h > @@ -98,4 +98,14 @@ > #define ZERO_BLOCK_PTR ((void *)-1L) > #endif > > +#define BYTES_PER_LONG __SIZEOF_LONG__ > + > +#define BITS_PER_BYTE __CHAR_BIT__ > +#define BITS_PER_INT (BITS_PER_BYTE * __SIZEOF_INT__) > +#define BITS_PER_LONG (BITS_PER_BYTE * BYTES_PER_LONG) > +#define BITS_PER_LLONG (BITS_PER_BYTE * __SIZEOF_LONG_LONG__) > + > +/* It is assumed that sizeof(void *) == __alignof(void *) */ > +#define POINTER_ALIGN __SIZEOF_POINTER__ > + > #endif /* __XEN_CONFIG_H__ */ As it turns out, this is a prerequisite for the byteswap cleanup. ~Andrew
On 27/03/2025 5:33 pm, Oleksii Kurochko wrote: > As Anrew mentioned here https://lore.kernel.org/xen-devel/f294e2b1-db03-46b2-b46d-61e89b55cef3@suse.com/T/#ma4205392964ff913d9dfa8e044a4af59ed6aef88: > "This patch possibly wants to wait until I've fixed the compiler checks to update to the new baseline, ..." This is now committed. ~Andrew
On 27.03.2025 18:33, Oleksii Kurochko wrote: > BITS_PER_* values can be defined in a common way using compiler-provided macros. > Thus, these definitions are moved to xen/config.h to reduce duplication across > architectures. > > Additionally, *_BYTEORDER macros are removed, as BITS_PER_* values now come > directly from the compiler environment. > > The arch_fls() implementation for Arm and PPC is updated to use BITS_PER_INT > instead of a hardcoded value of 32. > > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> albeit ... > --- a/xen/include/xen/config.h > +++ b/xen/include/xen/config.h > @@ -98,4 +98,14 @@ > #define ZERO_BLOCK_PTR ((void *)-1L) > #endif > > +#define BYTES_PER_LONG __SIZEOF_LONG__ ... I remain unconvinced that we actually need this (yet then not its sibling for int). All uses I can spot could be replaced by sizeof(long). This (and its sibling) would be needed only for #if uses, and there I would then wonder why we couldn't use __SIZEOF_LONG__ directly, now that we assume its universally available. Jan
On 3/28/25 8:49 AM, Jan Beulich wrote: > On 27.03.2025 18:33, Oleksii Kurochko wrote: >> BITS_PER_* values can be defined in a common way using compiler-provided macros. >> Thus, these definitions are moved to xen/config.h to reduce duplication across >> architectures. >> >> Additionally, *_BYTEORDER macros are removed, as BITS_PER_* values now come >> directly from the compiler environment. >> >> The arch_fls() implementation for Arm and PPC is updated to use BITS_PER_INT >> instead of a hardcoded value of 32. >> >> Suggested-by: Andrew Cooper<andrew.cooper3@citrix.com> >> Signed-off-by: Oleksii Kurochko<oleksii.kurochko@gmail.com> > Reviewed-by: Jan Beulich<jbeulich@suse.com> > albeit ... > >> --- a/xen/include/xen/config.h >> +++ b/xen/include/xen/config.h >> @@ -98,4 +98,14 @@ >> #define ZERO_BLOCK_PTR ((void *)-1L) >> #endif >> >> +#define BYTES_PER_LONG __SIZEOF_LONG__ > ... I remain unconvinced that we actually need this (yet then not its sibling > for int). All uses I can spot could be replaced by sizeof(long). This (and its > sibling) would be needed only for #if uses, and there I would then wonder why > we couldn't use __SIZEOF_LONG__ directly, now that we assume its universally > available. I left only because|BYTES_PER_LONG| is used in ARM and x86 code. If no one minds, I can replace|BYTES_PER_LONG| with|__SIZEOF_LONG__|. ~ Oleksii
On 28/03/2025 7:49 am, Jan Beulich wrote: > On 27.03.2025 18:33, Oleksii Kurochko wrote: >> BITS_PER_* values can be defined in a common way using compiler-provided macros. >> Thus, these definitions are moved to xen/config.h to reduce duplication across >> architectures. >> >> Additionally, *_BYTEORDER macros are removed, as BITS_PER_* values now come >> directly from the compiler environment. >> >> The arch_fls() implementation for Arm and PPC is updated to use BITS_PER_INT >> instead of a hardcoded value of 32. >> >> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> > albeit ... > >> --- a/xen/include/xen/config.h >> +++ b/xen/include/xen/config.h >> @@ -98,4 +98,14 @@ >> #define ZERO_BLOCK_PTR ((void *)-1L) >> #endif >> >> +#define BYTES_PER_LONG __SIZEOF_LONG__ > ... I remain unconvinced that we actually need this (yet then not its sibling > for int). All uses I can spot could be replaced by sizeof(long). Perhaps, but that's orthogonal to centralising the definition. It can be deferred. > This (and its > sibling) would be needed only for #if uses, and there I would then wonder why > we couldn't use __SIZEOF_LONG__ directly, now that we assume its universally > available. Looking at the usages, sizeof(long) would be best, and shortest. While we do have a whole bunch of constants which are universally available now, I still want to use the more normal terms where possible. e.g. the difference between __BYTE_ORDER__ and BYTE_ORDER is relevant for the code shared with userspace, and using BYTE_ORDER across Xen is the cleaner option. ~Andrew
© 2016 - 2025 Red Hat, Inc.