[2/6] common: move standard C fixed width type declarations to common header

Commit Message

Jan Beulich Feb. 9, 2023, 10:38 a.m. UTC

Have these in one place, for all architectures to use. Also use the C99
types as the "original" ones, and derive the Linux compatible ones
(which we're trying to phase out). For __s<N>, seeing that no uses exist
anymore, move them to a new Linux compatibility header (as an act of
precaution - we don't have any uses of these types right now).

Modern compilers supply __{,U}INT<n>_TYPE__ - use those if available.
Otherwise fall back to using "mode" attributes, but this can be relied
upon only when bytes are 8 bits wide. Should there ever be a port to an
architecture not matching this, it would need to define the fixed width
types locally by some other means.

In a few cases inclusion of asm/types.h needs replacing by xen/types.h.
Further in common/trace.c take the opportunity and also drop the
apparently unused inclusion of asm/io.h at the same time. Finally in
some Flask sources inclusion of asm/byteorder.h needs moving later.

No functional change intended.

Signed-off-by: Jan Beulich <[e-mail]>
---
RFC: I know Andrew would prefer these types to move to a new
     xen/stdint.h, but not being fully convinced of this myself, I was
     hoping for others to voice views.

Comments

Daniel P. Smith Feb. 9, 2023, 2:23 p.m. UTC | #1

On 2/9/23 05:38, Jan Beulich wrote:
> Have these in one place, for all architectures to use. Also use the C99
> types as the "original" ones, and derive the Linux compatible ones
> (which we're trying to phase out). For __s<N>, seeing that no uses exist
> anymore, move them to a new Linux compatibility header (as an act of
> precaution - we don't have any uses of these types right now).
> 
> Modern compilers supply __{,U}INT<n>_TYPE__ - use those if available.
> Otherwise fall back to using "mode" attributes, but this can be relied
> upon only when bytes are 8 bits wide. Should there ever be a port to an
> architecture not matching this, it would need to define the fixed width
> types locally by some other means.
> 
> In a few cases inclusion of asm/types.h needs replacing by xen/types.h.
> Further in common/trace.c take the opportunity and also drop the
> apparently unused inclusion of asm/io.h at the same time. Finally in
> some Flask sources inclusion of asm/byteorder.h needs moving later.
> 
> No functional change intended.
> 
> Signed-off-by: Jan Beulich <[e-mail]>
> ---
> RFC: I know Andrew would prefer these types to move to a new
>       xen/stdint.h, but not being fully convinced of this myself, I was
>       hoping for others to voice views.
> 

IHHO since there is already stdarg.h and stdbool.h, which establishes 
some degree of following C standard library includes, it would seem more 
logical to move to stdint.h. The main point here is not whether 
following the C standard library includes is the right approach, but 
that consistency is more important. Having a mixture of types.h and C 
standard library includes leaves one having to keep track of which C 
standard library includes are present versus what is in type.h.

Andrew Cooper Feb. 15, 2023, 6:54 p.m. UTC | #2

On 09/02/2023 10:38 am, Jan Beulich wrote:
> Have these in one place, for all architectures to use. Also use the C99
> types as the "original" ones, and derive the Linux compatible ones
> (which we're trying to phase out). For __s<N>, seeing that no uses exist
> anymore, move them to a new Linux compatibility header (as an act of
> precaution - we don't have any uses of these types right now).
>
> Modern compilers supply __{,U}INT<n>_TYPE__ - use those if available.
> Otherwise fall back to using "mode" attributes, but this can be relied
> upon only when bytes are 8 bits wide. Should there ever be a port to an
> architecture not matching this, it would need to define the fixed width
> types locally by some other means.

These types were added in GCC 4.5.  It is 12 years old.  We even use a
newer C standard than this compiler...

At this point, it is an unreasonable burden to be continuing to support
compilers this obsolete, not to mention that the mode attributes are
unreadable to anyone who isn't a GCC developer and clearly unnecessary
to begin with.

~Andrew

Jan Beulich Feb. 16, 2023, 7:47 a.m. UTC | #3

On 15.02.2023 19:54, Andrew Cooper wrote:
> On 09/02/2023 10:38 am, Jan Beulich wrote:
>> Have these in one place, for all architectures to use. Also use the C99
>> types as the "original" ones, and derive the Linux compatible ones
>> (which we're trying to phase out). For __s<N>, seeing that no uses exist
>> anymore, move them to a new Linux compatibility header (as an act of
>> precaution - we don't have any uses of these types right now).
>>
>> Modern compilers supply __{,U}INT<n>_TYPE__ - use those if available.
>> Otherwise fall back to using "mode" attributes, but this can be relied
>> upon only when bytes are 8 bits wide. Should there ever be a port to an
>> architecture not matching this, it would need to define the fixed width
>> types locally by some other means.
> 
> These types were added in GCC 4.5.  It is 12 years old.  We even use a
> newer C standard than this compiler...

A newer C standard? We're using C99, don't we? And Xen, at this point,
continues to build fine with gcc 4.3. Furthermore I for one didn't
check when Clang gained support for these pre-defs ...

> At this point, it is an unreasonable burden to be continuing to support
> compilers this obsolete, not to mention that the mode attributes are
> unreadable to anyone who isn't a GCC developer and clearly unnecessary
> to begin with.

I disagree about the rant regarding mode attributes, but that's secondary.
What I'm really unhappy about is for this work to grow a dependency on the
long-standing question of what to update our tool chain baseline to. This
has been discussed many times, but there was never a concrete proposal on
a policy that we could use not only now, but also going forward. As it
stands I can't help the impression that this is going to remain unresolved
for an extended period of time. But there not being an appropriate
solution to that other issue shouldn't block the work here. Once that one
is resolved (and provided it's then acceptable also on the Clang side),
we could easily drop the mode attribute fallbacks again.

Jan

Patch

--- a/xen/arch/arm/include/asm/types.h
+++ b/xen/arch/arm/include/asm/types.h
@@ -1,37 +1,8 @@ 
 #ifndef __ARM_TYPES_H__
 #define __ARM_TYPES_H__
 
-typedef __signed__ char __s8;
-typedef unsigned char __u8;
-
-typedef __signed__ short __s16;
-typedef unsigned short __u16;
-
-typedef __signed__ int __s32;
-typedef unsigned int __u32;
-
-#if defined(__GNUC__) && !defined(__STRICT_ANSI__)
 #if defined(CONFIG_ARM_32)
-typedef __signed__ long long __s64;
-typedef unsigned long long __u64;
-#elif defined (CONFIG_ARM_64)
-typedef __signed__ long __s64;
-typedef unsigned long __u64;
-#endif
-#endif
-
-typedef signed char s8;
-typedef unsigned char u8;
 
-typedef signed short s16;
-typedef unsigned short u16;
-
-typedef signed int s32;
-typedef unsigned int u32;
-
-#if defined(CONFIG_ARM_32)
-typedef signed long long s64;
-typedef unsigned long long u64;
 typedef u32 vaddr_t;
 #define PRIvaddr PRIx32
 typedef u64 paddr_t;
@@ -39,9 +10,9 @@  typedef u64 paddr_t;
 #define PRIpaddr "016llx"
 typedef u32 register_t;
 #define PRIregister "08x"
-#elif defined (CONFIG_ARM_64)
-typedef signed long s64;
-typedef unsigned long u64;
+
+#elif defined(CONFIG_ARM_64)
+
 typedef u64 vaddr_t;
 #define PRIvaddr PRIx64
 typedef u64 paddr_t;
@@ -49,6 +20,7 @@  typedef u64 paddr_t;
 #define PRIpaddr "016lx"
 typedef u64 register_t;
 #define PRIregister "016lx"
+
 #endif
 
 #endif /* __ARM_TYPES_H__ */
--- a/xen/arch/riscv/include/asm/types.h
+++ b/xen/arch/riscv/include/asm/types.h
@@ -1,38 +1,8 @@ 
 #ifndef __RISCV_TYPES_H__
 #define __RISCV_TYPES_H__
 
-typedef __signed__ char __s8;
-typedef unsigned char __u8;
-
-typedef __signed__ short __s16;
-typedef unsigned short __u16;
-
-typedef __signed__ int __s32;
-typedef unsigned int __u32;
-
-#if defined(__GNUC__) && !defined(__STRICT_ANSI__)
-#if defined(CONFIG_RISCV_32)
-typedef __signed__ long long __s64;
-typedef unsigned long long __u64;
-#elif defined (CONFIG_RISCV_64)
-typedef __signed__ long __s64;
-typedef unsigned long __u64;
-#endif
-#endif
-
-typedef signed char s8;
-typedef unsigned char u8;
-
-typedef signed short s16;
-typedef unsigned short u16;
-
-typedef signed int s32;
-typedef unsigned int u32;
-
 #if defined(CONFIG_RISCV_32)
 
-typedef signed long long s64;
-typedef unsigned long long u64;
 typedef u32 vaddr_t;
 #define PRIvaddr PRIx32
 typedef u64 paddr_t;
@@ -43,8 +13,6 @@  typedef u32 register_t;
 
 #elif defined (CONFIG_RISCV_64)
 
-typedef signed long s64;
-typedef unsigned long u64;
 typedef u64 vaddr_t;
 #define PRIvaddr PRIx64
 typedef u64 paddr_t;
--- a/xen/arch/x86/include/asm/types.h
+++ b/xen/arch/x86/include/asm/types.h
@@ -1,31 +1,6 @@ 
 #ifndef __X86_TYPES_H__
 #define __X86_TYPES_H__
 
-typedef __signed__ char __s8;
-typedef unsigned char __u8;
-
-typedef __signed__ short __s16;
-typedef unsigned short __u16;
-
-typedef __signed__ int __s32;
-typedef unsigned int __u32;
-
-#if defined(__GNUC__) && !defined(__STRICT_ANSI__)
-typedef __signed__ long __s64;
-typedef unsigned long __u64;
-#endif
-
-typedef signed char s8;
-typedef unsigned char u8;
-
-typedef signed short s16;
-typedef unsigned short u16;
-
-typedef signed int s32;
-typedef unsigned int u32;
-
-typedef signed long s64;
-typedef unsigned long u64;
 typedef unsigned long paddr_t;
 #define INVALID_PADDR (~0UL)
 #define PRIpaddr "016lx"
--- a/xen/common/trace.c
+++ b/xen/common/trace.c
@@ -16,8 +16,6 @@ 
  * it's possible to reconstruct a chronological record of trace events.
  */
 
-#include <asm/types.h>
-#include <asm/io.h>
 #include <xen/lib.h>
 #include <xen/param.h>
 #include <xen/sched.h>
--- a/xen/include/xen/bitops.h
+++ b/xen/include/xen/bitops.h
@@ -1,6 +1,7 @@ 
 #ifndef _LINUX_BITOPS_H
 #define _LINUX_BITOPS_H
-#include <asm/types.h>
+
+#include <xen/types.h>
 
 /*
  * Create a contiguous bitmask starting at bit position @l and ending at
--- /dev/null
+++ b/xen/include/xen/linux-compat.h
@@ -0,0 +1,19 @@ 
+/******************************************************************************
+ * linux-compat.h
+ *
+ * Container for types and other definitions use in Linux (and hence in files
+ * we "steal" from there), but which shouldn't be used (anymore) in normal Xen
+ * files.
+ */
+
+#ifndef __XEN_LINUX_COMPAT_H__
+#define __XEN_LINUX_COMPAT_H__
+
+#include <xen/types.h>
+
+typedef int8_t  __s8;
+typedef int16_t __s16;
+typedef int32_t __s32;
+typedef int64_t __s64;
+
+#endif /* __XEN_LINUX_COMPAT_H__ */
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -3,6 +3,66 @@ 
 
 #include <xen/stdbool.h>
 
+/* Fixed width types */
+
+#if defined(__INT8_TYPE__)
+typedef __INT8_TYPE__ int8_t;
+#elif BITS_PER_BYTE == 8
+typedef signed int __attribute__((__mode__(__QI__))) int8_t;
+#endif
+
+#if defined(__UINT8_TYPE__)
+typedef __UINT8_TYPE__ uint8_t;
+#elif BITS_PER_BYTE == 8
+typedef unsigned int __attribute__((__mode__(__QI__))) uint8_t;
+#endif
+
+#if defined(__INT16_TYPE__)
+typedef __INT16_TYPE__ int16_t;
+#elif BITS_PER_BYTE == 8
+typedef signed int __attribute__((__mode__(__HI__))) int16_t;
+#endif
+
+#if defined(__UINT16_TYPE__)
+typedef __UINT16_TYPE__ uint16_t;
+#elif BITS_PER_BYTE == 8
+typedef unsigned int __attribute__((__mode__(__HI__))) uint16_t;
+#endif
+
+#if defined(__INT32_TYPE__)
+typedef __INT32_TYPE__ int32_t;
+#elif BITS_PER_BYTE == 8
+typedef signed int __attribute__((__mode__(__SI__))) int32_t;
+#endif
+
+#if defined(__UINT32_TYPE__)
+typedef __UINT32_TYPE__ uint32_t;
+#elif BITS_PER_BYTE == 8
+typedef unsigned int __attribute__((__mode__(__SI__))) uint32_t;
+#endif
+
+#if defined(__INT64_TYPE__)
+typedef __INT64_TYPE__ int64_t;
+#elif BITS_PER_BYTE == 8
+typedef signed int __attribute__((__mode__(__DI__))) int64_t;
+#endif
+
+#if defined(__UINT64_TYPE__)
+typedef __UINT64_TYPE__ uint64_t;
+#elif BITS_PER_BYTE == 8
+typedef unsigned int __attribute__((__mode__(__DI__))) uint64_t;
+#endif
+
+/* Linux inherited types which are being phased out */
+typedef int8_t s8;
+typedef uint8_t u8, __u8;
+typedef int16_t s16;
+typedef uint16_t u16, __u16;
+typedef int32_t s32;
+typedef uint32_t u32, __u32;
+typedef int64_t s64;
+typedef uint64_t u64, __u64;
+
 #include <asm/types.h>
 
 #if defined(__SIZE_TYPE__)
@@ -46,18 +106,6 @@  typedef signed long ptrdiff_t;
 #define LONG_MIN        (-LONG_MAX - 1)
 #define ULONG_MAX       (~0UL)
 
-typedef         __u8            uint8_t;
-typedef         __s8            int8_t;
-
-typedef         __u16           uint16_t;
-typedef         __s16           int16_t;
-
-typedef         __u32           uint32_t;
-typedef         __s32           int32_t;
-
-typedef         __u64           uint64_t;
-typedef         __s64           int64_t;
-
 typedef __u16 __le16;
 typedef __u16 __be16;
 typedef __u32 __le32;
--- a/xen/xsm/flask/ss/conditional.c
+++ b/xen/xsm/flask/ss/conditional.c
@@ -9,7 +9,6 @@ 
 
 /* Ported to Xen 3.0, George Coker, <[e-mail]> */
 
-#include <asm/byteorder.h>
 #include <xen/lib.h>
 #include <xen/types.h>
 #include <xen/errno.h>
@@ -17,6 +16,8 @@ 
 #include <xen/spinlock.h>
 #include <xen/xmalloc.h>
 
+#include <asm/byteorder.h>
+
 #include "security.h"
 #include "conditional.h"
 
--- a/xen/xsm/flask/ss/ebitmap.c
+++ b/xen/xsm/flask/ss/ebitmap.c
@@ -10,12 +10,14 @@ 
 
 /* Ported to Xen 3.0, George Coker, <[e-mail]> */
 
-#include <asm/byteorder.h>
 #include <xen/lib.h>
 #include <xen/xmalloc.h>
 #include <xen/errno.h>
 #include <xen/spinlock.h>
 #include <xen/bitmap.h>
+
+#include <asm/byteorder.h>
+
 #include "ebitmap.h"
 #include "policydb.h"
 
--- a/xen/xsm/flask/ss/policydb.c
+++ b/xen/xsm/flask/ss/policydb.c
@@ -22,12 +22,14 @@ 
 
 /* Ported to Xen 3.0, George Coker, <[e-mail]> */
 
-#include <asm/byteorder.h>
 #include <xen/lib.h>
 #include <xen/types.h>
 #include <xen/xmalloc.h>
 #include <xen/string.h>
 #include <xen/errno.h>
+
+#include <asm/byteorder.h>
+
 #include <conditional.h>
 #include "security.h"