| Message ID | 1eaa4cce-2ef2-ca38-56d2-5d551c9c1ae9@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools/symbols: drop asm/types.h inclusion | expand |
On 20/01/2023 8:40 am, Jan Beulich wrote: > While this has been there forever, it's not clear to me what it was > (thought to be) needed for. In fact, all three instances of the header > already exclude their entire bodies when __ASSEMBLY__ was defined. > Hence, with no other assembly files including this header, we can at the > same time get rid of those conditionals. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Hi Jan, On 20/01/2023 08:40, Jan Beulich wrote: > While this has been there forever, it's not clear to me what it was > (thought to be) needed for. asm/types.h used to be directly included in assembly x86 file. This was dropped by commit 3f76e83c4cf6 "x86/entry: drop unused header inclusions". > In fact, all three instances of the header > already exclude their entire bodies when __ASSEMBLY__ was defined. > Hence, with no other assembly files including this header, we can at the > same time get rid of those conditionals. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Julien Grall <jgrall@amazon.com> Cheers,
On 20.01.2023 12:24, Julien Grall wrote: > On 20/01/2023 08:40, Jan Beulich wrote: >> While this has been there forever, it's not clear to me what it was >> (thought to be) needed for. > > asm/types.h used to be directly included in assembly x86 file. This was > dropped by commit 3f76e83c4cf6 "x86/entry: drop unused header inclusions". Just to clarify: The statement in the description is about $subject, not ... >> In fact, all three instances of the header >> already exclude their entire bodies when __ASSEMBLY__ was defined. >> Hence, with no other assembly files including this header, we can at the >> same time get rid of those conditionals. ... this further aspect. I can certainly see why the guards may have been there (without having gone look for when the last such use may have disappeared) beyond the bogus use by the tool. >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Acked-by: Julien Grall <jgrall@amazon.com> Thanks. Jan
On 20/01/2023 13:19, Jan Beulich wrote: > On 20.01.2023 12:24, Julien Grall wrote: >> On 20/01/2023 08:40, Jan Beulich wrote: >>> While this has been there forever, it's not clear to me what it was >>> (thought to be) needed for. >> >> asm/types.h used to be directly included in assembly x86 file. This was >> dropped by commit 3f76e83c4cf6 "x86/entry: drop unused header inclusions". > > Just to clarify: The statement in the description is about $subject, > not ... > >>> In fact, all three instances of the header >>> already exclude their entire bodies when __ASSEMBLY__ was defined. >>> Hence, with no other assembly files including this header, we can at the >>> same time get rid of those conditionals. > > ... this further aspect. I can certainly see why the guards may have > been there (without having gone look for when the last such use may > have disappeared) beyond the bogus use by the tool. Ah! Thanks for the clarification. I indeed misinterpreted the first sentence. Cheers,
On 20.01.2023 09:40, Jan Beulich wrote: > While this has been there forever, it's not clear to me what it was > (thought to be) needed for. In fact, all three instances of the header > already exclude their entire bodies when __ASSEMBLY__ was defined. > Hence, with no other assembly files including this header, we can at the > same time get rid of those conditionals. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> May I please ask for a RISC-V side ack (or otherwise) here? Thanks, Jan > --- a/xen/arch/arm/include/asm/types.h > +++ b/xen/arch/arm/include/asm/types.h > @@ -1,9 +1,6 @@ > #ifndef __ARM_TYPES_H__ > #define __ARM_TYPES_H__ > > -#ifndef __ASSEMBLY__ > - > - > typedef __signed__ char __s8; > typedef unsigned char __u8; > > @@ -54,8 +51,6 @@ typedef u64 register_t; > #define PRIregister "016lx" > #endif > > -#endif /* __ASSEMBLY__ */ > - > #endif /* __ARM_TYPES_H__ */ > /* > * Local variables: > --- a/xen/arch/riscv/include/asm/types.h > +++ b/xen/arch/riscv/include/asm/types.h > @@ -1,8 +1,6 @@ > #ifndef __RISCV_TYPES_H__ > #define __RISCV_TYPES_H__ > > -#ifndef __ASSEMBLY__ > - > typedef __signed__ char __s8; > typedef unsigned char __u8; > > @@ -57,8 +55,6 @@ typedef u64 register_t; > > #endif > > -#endif /* __ASSEMBLY__ */ > - > #endif /* __RISCV_TYPES_H__ */ > /* > * Local variables: > --- a/xen/arch/x86/include/asm/types.h > +++ b/xen/arch/x86/include/asm/types.h > @@ -1,8 +1,6 @@ > #ifndef __X86_TYPES_H__ > #define __X86_TYPES_H__ > > -#ifndef __ASSEMBLY__ > - > typedef __signed__ char __s8; > typedef unsigned char __u8; > > @@ -32,6 +30,4 @@ typedef unsigned long paddr_t; > #define INVALID_PADDR (~0UL) > #define PRIpaddr "016lx" > > -#endif /* __ASSEMBLY__ */ > - > #endif /* __X86_TYPES_H__ */ > --- a/xen/tools/symbols.c > +++ b/xen/tools/symbols.c > @@ -302,7 +302,6 @@ static void write_src(void) > return; > } > printf("#include <xen/config.h>\n"); > - printf("#include <asm/types.h>\n"); > printf("#if BITS_PER_LONG == 64 && !defined(SYMBOLS_ORIGIN)\n"); > printf("#define PTR .quad\n"); > printf("#define ALGN .align 8\n"); >
On Mon, Feb 6, 2023 at 5:40 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 20.01.2023 09:40, Jan Beulich wrote: > > While this has been there forever, it's not clear to me what it was > > (thought to be) needed for. In fact, all three instances of the header > > already exclude their entire bodies when __ASSEMBLY__ was defined. > > Hence, with no other assembly files including this header, we can at the > > same time get rid of those conditionals. > > > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > > May I please ask for a RISC-V side ack (or otherwise) here? > > Thanks, Jan > > > --- a/xen/arch/arm/include/asm/types.h > > +++ b/xen/arch/arm/include/asm/types.h > > @@ -1,9 +1,6 @@ > > #ifndef __ARM_TYPES_H__ > > #define __ARM_TYPES_H__ > > > > -#ifndef __ASSEMBLY__ > > - > > - > > typedef __signed__ char __s8; > > typedef unsigned char __u8; > > > > @@ -54,8 +51,6 @@ typedef u64 register_t; > > #define PRIregister "016lx" > > #endif > > > > -#endif /* __ASSEMBLY__ */ > > - > > #endif /* __ARM_TYPES_H__ */ > > /* > > * Local variables: > > --- a/xen/arch/riscv/include/asm/types.h > > +++ b/xen/arch/riscv/include/asm/types.h > > @@ -1,8 +1,6 @@ > > #ifndef __RISCV_TYPES_H__ > > #define __RISCV_TYPES_H__ > > > > -#ifndef __ASSEMBLY__ > > - > > typedef __signed__ char __s8; > > typedef unsigned char __u8; > > > > @@ -57,8 +55,6 @@ typedef u64 register_t; > > > > #endif > > > > -#endif /* __ASSEMBLY__ */ > > - > > #endif /* __RISCV_TYPES_H__ */ > > /* > > * Local variables: > > --- a/xen/arch/x86/include/asm/types.h > > +++ b/xen/arch/x86/include/asm/types.h > > @@ -1,8 +1,6 @@ > > #ifndef __X86_TYPES_H__ > > #define __X86_TYPES_H__ > > > > -#ifndef __ASSEMBLY__ > > - > > typedef __signed__ char __s8; > > typedef unsigned char __u8; > > > > @@ -32,6 +30,4 @@ typedef unsigned long paddr_t; > > #define INVALID_PADDR (~0UL) > > #define PRIpaddr "016lx" > > > > -#endif /* __ASSEMBLY__ */ > > - > > #endif /* __X86_TYPES_H__ */ > > --- a/xen/tools/symbols.c > > +++ b/xen/tools/symbols.c > > @@ -302,7 +302,6 @@ static void write_src(void) > > return; > > } > > printf("#include <xen/config.h>\n"); > > - printf("#include <asm/types.h>\n"); > > printf("#if BITS_PER_LONG == 64 && !defined(SYMBOLS_ORIGIN)\n"); > > printf("#define PTR .quad\n"); > > printf("#define ALGN .align 8\n"); > > > >
--- a/xen/arch/arm/include/asm/types.h +++ b/xen/arch/arm/include/asm/types.h @@ -1,9 +1,6 @@ #ifndef __ARM_TYPES_H__ #define __ARM_TYPES_H__ -#ifndef __ASSEMBLY__ - - typedef __signed__ char __s8; typedef unsigned char __u8; @@ -54,8 +51,6 @@ typedef u64 register_t; #define PRIregister "016lx" #endif -#endif /* __ASSEMBLY__ */ - #endif /* __ARM_TYPES_H__ */ /* * Local variables: --- a/xen/arch/riscv/include/asm/types.h +++ b/xen/arch/riscv/include/asm/types.h @@ -1,8 +1,6 @@ #ifndef __RISCV_TYPES_H__ #define __RISCV_TYPES_H__ -#ifndef __ASSEMBLY__ - typedef __signed__ char __s8; typedef unsigned char __u8; @@ -57,8 +55,6 @@ typedef u64 register_t; #endif -#endif /* __ASSEMBLY__ */ - #endif /* __RISCV_TYPES_H__ */ /* * Local variables: --- a/xen/arch/x86/include/asm/types.h +++ b/xen/arch/x86/include/asm/types.h @@ -1,8 +1,6 @@ #ifndef __X86_TYPES_H__ #define __X86_TYPES_H__ -#ifndef __ASSEMBLY__ - typedef __signed__ char __s8; typedef unsigned char __u8; @@ -32,6 +30,4 @@ typedef unsigned long paddr_t; #define INVALID_PADDR (~0UL) #define PRIpaddr "016lx" -#endif /* __ASSEMBLY__ */ - #endif /* __X86_TYPES_H__ */ --- a/xen/tools/symbols.c +++ b/xen/tools/symbols.c @@ -302,7 +302,6 @@ static void write_src(void) return; } printf("#include <xen/config.h>\n"); - printf("#include <asm/types.h>\n"); printf("#if BITS_PER_LONG == 64 && !defined(SYMBOLS_ORIGIN)\n"); printf("#define PTR .quad\n"); printf("#define ALGN .align 8\n");
While this has been there forever, it's not clear to me what it was (thought to be) needed for. In fact, all three instances of the header already exclude their entire bodies when __ASSEMBLY__ was defined. Hence, with no other assembly files including this header, we can at the same time get rid of those conditionals. Signed-off-by: Jan Beulich <jbeulich@suse.com>