1. Patchew
  2. Xen
  3. View series

[PATCH] tools/symbols: drop asm/types.h inclusion

Jan Beulich posted 1 patch 1 year, 3 months ago
Failed in applying to current master (apply log)
[PATCH] tools/symbols: drop asm/types.h inclusion
Posted by Jan Beulich 1 year, 3 months ago 
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>

--- 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");
Ping: [PATCH] tools/symbols: drop asm/types.h inclusion
Posted by Jan Beulich 1 year, 2 months ago 
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");
>
Re: Ping: [PATCH] tools/symbols: drop asm/types.h inclusion
Posted by Alistair Francis 1 year, 2 months ago 
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");
> >
>
>
Re: [PATCH] tools/symbols: drop asm/types.h inclusion
Posted by Julien Grall 1 year, 3 months ago 
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,

-- 
Julien Grall
Re: [PATCH] tools/symbols: drop asm/types.h inclusion
Posted by Jan Beulich 1 year, 3 months ago 
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
Re: [PATCH] tools/symbols: drop asm/types.h inclusion
Posted by Julien Grall 1 year, 3 months ago 

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,

-- 
Julien Grall
Re: [PATCH] tools/symbols: drop asm/types.h inclusion
Posted by Andrew Cooper 1 year, 3 months ago 
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>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.