| Message ID | c641b981-54b8-d3ca-26a4-28331dc79ca5@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fixed width type adjustments | expand |
On 8/4/23 02:08, 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 - as said, we don't have any uses of these types right now). > > In some Flask sources inclusion of asm/byteorder.h needs moving later. I just did a test build against staging for x86 with all asm/byteorder.h includes removed from FLASK and it built successfully. If there are no object to a small non-functional change patch, I can clean up include ordering as well and submit it to xen-devel. v/r, dps
On 08.08.2023 18:43, Daniel P. Smith wrote: > On 8/4/23 02:08, 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 - as said, we don't have any uses of these types right now). >> >> In some Flask sources inclusion of asm/byteorder.h needs moving later. > > I just did a test build against staging for x86 with all asm/byteorder.h > includes removed from FLASK and it built successfully. If there are no > object to a small non-functional change patch, I can clean up include > ordering as well and submit it to xen-devel. Of course, please go ahead. Simplifies the patch here. Jan
On 04/08/2023 7:08 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 - as said, we don't have any uses of these types right now). > > In some Flask sources inclusion of asm/byteorder.h needs moving later. > > No functional change intended. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Hi Jan, On 04/08/2023 07:08, 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 - as said, we don't have any uses of these types right now). > > In some Flask sources inclusion of asm/byteorder.h needs moving later. > > No functional change intended. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> With one remark: Acked-by: Julien Grall <jgrall@amazon.com> > --- > v2: This is the residual (extended to PPC) of an earlier larger patch > which was partly replaced by introducing xen/stdint.h. > > --- a/xen/arch/arm/include/asm/types.h > +++ b/xen/arch/arm/include/asm/types.h > @@ -1,18 +1,7 @@ > #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(CONFIG_ARM_32) > -typedef signed long long s64; > -typedef unsigned long long u64; > typedef u32 vaddr_t; > #define PRIvaddr PRIx32 > #if defined(CONFIG_PHYS_ADDR_T_32) > @@ -34,9 +23,9 @@ typedef u64 paddr_t; > #endif I find odd that you don't add a newline before/after #endif here but... > 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; > @@ -44,6 +33,7 @@ typedef u64 paddr_t; > #define PRIpaddr "016lx" > typedef u64 register_t; > #define PRIregister "016lx" > + .. you add one here. But this change looks a bit unrelated and IMHO of out context. > #endif > > #endif /* __ARM_TYPES_H__ */ > --- a/xen/arch/ppc/include/asm/types.h Cheers,
On 09.08.2023 12:49, Julien Grall wrote: > On 04/08/2023 07:08, 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 - as said, we don't have any uses of these types right now). >> >> In some Flask sources inclusion of asm/byteorder.h needs moving later. >> >> No functional change intended. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > With one remark: > > Acked-by: Julien Grall <jgrall@amazon.com> Thanks. >> --- a/xen/arch/arm/include/asm/types.h >> +++ b/xen/arch/arm/include/asm/types.h >> @@ -1,18 +1,7 @@ >> #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(CONFIG_ARM_32) >> -typedef signed long long s64; >> -typedef unsigned long long u64; >> typedef u32 vaddr_t; >> #define PRIvaddr PRIx32 >> #if defined(CONFIG_PHYS_ADDR_T_32) >> @@ -34,9 +23,9 @@ typedef u64 paddr_t; >> #endif > > I find odd that you don't add a newline before/after #endif here but... > >> 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; >> @@ -44,6 +33,7 @@ typedef u64 paddr_t; >> #define PRIpaddr "016lx" >> typedef u64 register_t; >> #define PRIregister "016lx" >> + > > .. you add one here. But this change looks a bit unrelated and IMHO of > out context. > >> #endif >> >> #endif /* __ARM_TYPES_H__ */ Hmm, I think I was mistaking the blank line after #if defined(CONFIG_PHYS_ADDR_T_32) as the matching one. Which way would you prefer me to resolve the inconsistency: Add the missing blank line, or drop again the three ones I'm presently adding? Jan
On 09/08/2023 11:58, Jan Beulich wrote: > On 09.08.2023 12:49, Julien Grall wrote: >> On 04/08/2023 07:08, 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 - as said, we don't have any uses of these types right now). >>> >>> In some Flask sources inclusion of asm/byteorder.h needs moving later. >>> >>> No functional change intended. >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> With one remark: >> >> Acked-by: Julien Grall <jgrall@amazon.com> > > Thanks. > >>> --- a/xen/arch/arm/include/asm/types.h >>> +++ b/xen/arch/arm/include/asm/types.h >>> @@ -1,18 +1,7 @@ >>> #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(CONFIG_ARM_32) >>> -typedef signed long long s64; >>> -typedef unsigned long long u64; >>> typedef u32 vaddr_t; >>> #define PRIvaddr PRIx32 >>> #if defined(CONFIG_PHYS_ADDR_T_32) >>> @@ -34,9 +23,9 @@ typedef u64 paddr_t; >>> #endif >> >> I find odd that you don't add a newline before/after #endif here but... >> >>> 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; >>> @@ -44,6 +33,7 @@ typedef u64 paddr_t; >>> #define PRIpaddr "016lx" >>> typedef u64 register_t; >>> #define PRIregister "016lx" >>> + >> >> .. you add one here. But this change looks a bit unrelated and IMHO of >> out context. >> >>> #endif >>> >>> #endif /* __ARM_TYPES_H__ */ > > Hmm, I think I was mistaking the blank line after > > #if defined(CONFIG_PHYS_ADDR_T_32) > > as the matching one. Which way would you prefer me to resolve the > inconsistency: Add the missing blank line, or drop again the three ones > I'm presently adding? I would be ok with adding the missing blank line. Cheers,
--- a/xen/arch/arm/include/asm/types.h +++ b/xen/arch/arm/include/asm/types.h @@ -1,18 +1,7 @@ #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(CONFIG_ARM_32) -typedef signed long long s64; -typedef unsigned long long u64; typedef u32 vaddr_t; #define PRIvaddr PRIx32 #if defined(CONFIG_PHYS_ADDR_T_32) @@ -34,9 +23,9 @@ typedef u64 paddr_t; #endif 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; @@ -44,6 +33,7 @@ typedef u64 paddr_t; #define PRIpaddr "016lx" typedef u64 register_t; #define PRIregister "016lx" + #endif #endif /* __ARM_TYPES_H__ */ --- a/xen/arch/ppc/include/asm/types.h +++ b/xen/arch/ppc/include/asm/types.h @@ -3,17 +3,6 @@ #ifndef _ASM_PPC_TYPES_H #define _ASM_PPC_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; - -typedef signed long s64; -typedef unsigned long u64; typedef unsigned long paddr_t; #define INVALID_PADDR (~0UL) #define PRIpaddr "016lx" --- a/xen/arch/riscv/include/asm/types.h +++ b/xen/arch/riscv/include/asm/types.h @@ -3,19 +3,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(CONFIG_RISCV_32) -typedef signed long long s64; -typedef unsigned long long u64; typedef u32 vaddr_t; #define PRIvaddr PRIx32 typedef u64 paddr_t; @@ -26,8 +15,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,17 +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; - -typedef signed long s64; -typedef unsigned long u64; typedef unsigned long paddr_t; #define INVALID_PADDR (~0UL) #define PRIpaddr "016lx" --- /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 @@ -4,6 +4,16 @@ #include <xen/stdbool.h> #include <xen/stdint.h> +/* 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> typedef __SIZE_TYPE__ size_t; @@ -40,15 +50,6 @@ typedef __PTRDIFF_TYPE__ ptrdiff_t; #define LONG_MIN (-LONG_MAX - 1) #define ULONG_MAX (~0UL) -typedef uint8_t __u8; -typedef int8_t __s8; -typedef uint16_t __u16; -typedef int16_t __s16; -typedef uint32_t __u32; -typedef int32_t __s32; -typedef uint64_t __u64; -typedef int64_t __s64; - 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, <gscoker@alpha.ncsc.mil> */ -#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, <gscoker@alpha.ncsc.mil> */ -#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, <gscoker@alpha.ncsc.mil> */ -#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"
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 - as said, we don't have any uses of these types right now). In some Flask sources inclusion of asm/byteorder.h needs moving later. No functional change intended. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v2: This is the residual (extended to PPC) of an earlier larger patch which was partly replaced by introducing xen/stdint.h.