| Message ID | 20210313160611.18665-2-julien@xen.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Mitigate straight-line speculation | expand |
Hi Julien, > On 13 Mar 2021, at 16:06, Julien Grall <julien@xen.org> wrote: > > From: Julien Grall <jgrall@amazon.com> > > In a follow-up patch we may want to automatically replace some > mnemonics (such as ret) with a different sequence. > > To ensure all the assembly files will include asm/macros.h it is best to > automatically include it on single assembly. This can be done via > config.h. > > It was necessary to include a few more headers as dependency: > - <asm/asm_defns.h> to define sizeof_* > - <xen/page-size.h> which is already a latent issue given STACK_ORDER > rely on PAGE_SIZE. > > Unfortunately the build system will use -D__ASSEMBLY__ when generating > the linker script. A new option -D__LINKER__ is introduceed and used for > the linker script to avoid including headers (such as asm/macros.h) that > may not be compatible with the syntax. > > Lastly, take the opportunity to remove both asm/asm-offsets.h and > asm/macros.h from the various assembly files as they are now > automagically included. > > Signed-off-by: Julien Grall <jgrall@amazon.com> Everything is now building :-) I could not find a better then your define as filtering out or undefining __ASSEMBLY__ is actually not working. So with the fix from offset to defns: Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Cheers Bertrand > --- > xen/arch/arm/Makefile | 2 +- > xen/arch/arm/arm32/entry.S | 1 - > xen/arch/arm/arm32/head.S | 1 - > xen/arch/arm/arm32/proc-v7.S | 1 - > xen/arch/arm/arm64/debug-cadence.inc | 1 - > xen/arch/arm/arm64/debug-pl011.inc | 2 -- > xen/arch/arm/arm64/entry.S | 2 -- > xen/arch/arm/arm64/head.S | 2 -- > xen/arch/arm/arm64/smc.S | 3 --- > xen/include/asm-arm/config.h | 6 ++++++ > 10 files changed, 7 insertions(+), 14 deletions(-) > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 16e6523e2cc6..9ffc3f771c51 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -135,7 +135,7 @@ asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c > $(CC) $(filter-out -flto,$(c_flags)) -S -o $@ $< > > xen.lds: xen.lds.S > - $(CPP) -P $(a_flags) -MQ $@ -o $@ $< > + $(CPP) -P $(a_flags) -D__LINKER__ -MQ $@ -o $@ $< > > dtb.o: $(CONFIG_DTB_FILE) > > diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S > index b228d44b190c..f2f1bc7a3158 100644 > --- a/xen/arch/arm/arm32/entry.S > +++ b/xen/arch/arm/arm32/entry.S > @@ -1,4 +1,3 @@ > -#include <asm/asm_defns.h> > #include <asm/sysregs.h> > #include <asm/regs.h> > #include <asm/alternative.h> > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S > index c404fa973e9b..9084023a6ed9 100644 > --- a/xen/arch/arm/arm32/head.S > +++ b/xen/arch/arm/arm32/head.S > @@ -18,7 +18,6 @@ > */ > > #include <asm/page.h> > -#include <asm/asm_defns.h> > #include <asm/early_printk.h> > > #define ZIMAGE_MAGIC_NUMBER 0x016f2818 > diff --git a/xen/arch/arm/arm32/proc-v7.S b/xen/arch/arm/arm32/proc-v7.S > index 46bfc7a9074c..1efde2d72da0 100644 > --- a/xen/arch/arm/arm32/proc-v7.S > +++ b/xen/arch/arm/arm32/proc-v7.S > @@ -17,7 +17,6 @@ > * GNU General Public License for more details. > */ > > -#include <asm/asm_defns.h> > #include <asm/arm32/processor.h> > #include <asm/sysregs.h> > > diff --git a/xen/arch/arm/arm64/debug-cadence.inc b/xen/arch/arm/arm64/debug-cadence.inc > index 7df0abe4756f..0b6f2e094e18 100644 > --- a/xen/arch/arm/arm64/debug-cadence.inc > +++ b/xen/arch/arm/arm64/debug-cadence.inc > @@ -17,7 +17,6 @@ > * GNU General Public License for more details. > */ > > -#include <asm/asm_defns.h> > #include <asm/cadence-uart.h> > > /* > diff --git a/xen/arch/arm/arm64/debug-pl011.inc b/xen/arch/arm/arm64/debug-pl011.inc > index 385deff49b1b..1928a2e3ffbb 100644 > --- a/xen/arch/arm/arm64/debug-pl011.inc > +++ b/xen/arch/arm/arm64/debug-pl011.inc > @@ -16,8 +16,6 @@ > * GNU General Public License for more details. > */ > > -#include <asm/asm_defns.h> > - > /* > * PL011 UART initialization > * xb: register which containts the UART base address > diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S > index 175ea2981e72..ab9a65fc1475 100644 > --- a/xen/arch/arm/arm64/entry.S > +++ b/xen/arch/arm/arm64/entry.S > @@ -1,6 +1,4 @@ > -#include <asm/asm_defns.h> > #include <asm/current.h> > -#include <asm/macros.h> > #include <asm/regs.h> > #include <asm/alternative.h> > #include <asm/smccc.h> > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S > index 5d44667bd89d..fa7a3ffd2926 100644 > --- a/xen/arch/arm/arm64/head.S > +++ b/xen/arch/arm/arm64/head.S > @@ -21,11 +21,9 @@ > */ > > #include <asm/page.h> > -#include <asm/asm_defns.h> > #include <asm/early_printk.h> > #include <efi/efierr.h> > #include <asm/arm64/efibind.h> > -#include <asm/arm64/macros.h> > > #define PT_PT 0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 P=1 */ > #define PT_MEM 0xf7d /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=0 P=1 */ > diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S > index b0752be57e8f..91bae62dd4d2 100644 > --- a/xen/arch/arm/arm64/smc.S > +++ b/xen/arch/arm/arm64/smc.S > @@ -13,9 +13,6 @@ > * GNU General Public License for more details. > */ > > -#include <asm/asm_defns.h> > -#include <asm/macros.h> > - > /* > * void __arm_smccc_1_0_smc(register_t a0, register_t a1, register_t a2, > * register_t a3, register_t a4, register_t a5, > diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h > index 5c10c755db46..51273b9db1fc 100644 > --- a/xen/include/asm-arm/config.h > +++ b/xen/include/asm-arm/config.h > @@ -69,6 +69,7 @@ > #endif > > #include <xen/const.h> > +#include <xen/page-size.h> > > /* > * Common ARM32 and ARM64 layout: > @@ -190,6 +191,11 @@ extern unsigned long frametable_virt_end; > #define watchdog_disable() ((void)0) > #define watchdog_enable() ((void)0) > > +#if defined(__ASSEMBLY__) && !defined(__LINKER__) > +#include <asm/asm-offsets.h> > +#include <asm/macros.h> > +#endif > + > #endif /* __ARM_CONFIG_H__ */ > /* > * Local variables: > -- > 2.17.1 >
On 17/03/2021 14:38, Bertrand Marquis wrote: > Hi Julien, Hi Bertrand, >> On 13 Mar 2021, at 16:06, Julien Grall <julien@xen.org> wrote: >> >> From: Julien Grall <jgrall@amazon.com> >> >> In a follow-up patch we may want to automatically replace some >> mnemonics (such as ret) with a different sequence. >> >> To ensure all the assembly files will include asm/macros.h it is best to >> automatically include it on single assembly. This can be done via >> config.h. >> >> It was necessary to include a few more headers as dependency: >> - <asm/asm_defns.h> to define sizeof_* >> - <xen/page-size.h> which is already a latent issue given STACK_ORDER >> rely on PAGE_SIZE. >> >> Unfortunately the build system will use -D__ASSEMBLY__ when generating >> the linker script. A new option -D__LINKER__ is introduceed and used for >> the linker script to avoid including headers (such as asm/macros.h) that >> may not be compatible with the syntax. >> >> Lastly, take the opportunity to remove both asm/asm-offsets.h and >> asm/macros.h from the various assembly files as they are now >> automagically included. >> >> Signed-off-by: Julien Grall <jgrall@amazon.com> > > Everything is now building :-) > > I could not find a better then your define as filtering out or undefining __ASSEMBLY__ > is actually not working. Yes, unfortunately the linker is also relying on __ASSEMBLY__ for a few macros and to also remove the definitions of structure/function from headers that can be included either in C or assembly. -D__LINKER__ was the best option I could come up with. > > So with the fix from offset to defns: > > Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Thanks! I will resend a new version with the fix fold in this patch. Cheers,
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 16e6523e2cc6..9ffc3f771c51 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -135,7 +135,7 @@ asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(CC) $(filter-out -flto,$(c_flags)) -S -o $@ $< xen.lds: xen.lds.S - $(CPP) -P $(a_flags) -MQ $@ -o $@ $< + $(CPP) -P $(a_flags) -D__LINKER__ -MQ $@ -o $@ $< dtb.o: $(CONFIG_DTB_FILE) diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S index b228d44b190c..f2f1bc7a3158 100644 --- a/xen/arch/arm/arm32/entry.S +++ b/xen/arch/arm/arm32/entry.S @@ -1,4 +1,3 @@ -#include <asm/asm_defns.h> #include <asm/sysregs.h> #include <asm/regs.h> #include <asm/alternative.h> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index c404fa973e9b..9084023a6ed9 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -18,7 +18,6 @@ */ #include <asm/page.h> -#include <asm/asm_defns.h> #include <asm/early_printk.h> #define ZIMAGE_MAGIC_NUMBER 0x016f2818 diff --git a/xen/arch/arm/arm32/proc-v7.S b/xen/arch/arm/arm32/proc-v7.S index 46bfc7a9074c..1efde2d72da0 100644 --- a/xen/arch/arm/arm32/proc-v7.S +++ b/xen/arch/arm/arm32/proc-v7.S @@ -17,7 +17,6 @@ * GNU General Public License for more details. */ -#include <asm/asm_defns.h> #include <asm/arm32/processor.h> #include <asm/sysregs.h> diff --git a/xen/arch/arm/arm64/debug-cadence.inc b/xen/arch/arm/arm64/debug-cadence.inc index 7df0abe4756f..0b6f2e094e18 100644 --- a/xen/arch/arm/arm64/debug-cadence.inc +++ b/xen/arch/arm/arm64/debug-cadence.inc @@ -17,7 +17,6 @@ * GNU General Public License for more details. */ -#include <asm/asm_defns.h> #include <asm/cadence-uart.h> /* diff --git a/xen/arch/arm/arm64/debug-pl011.inc b/xen/arch/arm/arm64/debug-pl011.inc index 385deff49b1b..1928a2e3ffbb 100644 --- a/xen/arch/arm/arm64/debug-pl011.inc +++ b/xen/arch/arm/arm64/debug-pl011.inc @@ -16,8 +16,6 @@ * GNU General Public License for more details. */ -#include <asm/asm_defns.h> - /* * PL011 UART initialization * xb: register which containts the UART base address diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S index 175ea2981e72..ab9a65fc1475 100644 --- a/xen/arch/arm/arm64/entry.S +++ b/xen/arch/arm/arm64/entry.S @@ -1,6 +1,4 @@ -#include <asm/asm_defns.h> #include <asm/current.h> -#include <asm/macros.h> #include <asm/regs.h> #include <asm/alternative.h> #include <asm/smccc.h> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 5d44667bd89d..fa7a3ffd2926 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -21,11 +21,9 @@ */ #include <asm/page.h> -#include <asm/asm_defns.h> #include <asm/early_printk.h> #include <efi/efierr.h> #include <asm/arm64/efibind.h> -#include <asm/arm64/macros.h> #define PT_PT 0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 P=1 */ #define PT_MEM 0xf7d /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=0 P=1 */ diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S index b0752be57e8f..91bae62dd4d2 100644 --- a/xen/arch/arm/arm64/smc.S +++ b/xen/arch/arm/arm64/smc.S @@ -13,9 +13,6 @@ * GNU General Public License for more details. */ -#include <asm/asm_defns.h> -#include <asm/macros.h> - /* * void __arm_smccc_1_0_smc(register_t a0, register_t a1, register_t a2, * register_t a3, register_t a4, register_t a5, diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h index 5c10c755db46..51273b9db1fc 100644 --- a/xen/include/asm-arm/config.h +++ b/xen/include/asm-arm/config.h @@ -69,6 +69,7 @@ #endif #include <xen/const.h> +#include <xen/page-size.h> /* * Common ARM32 and ARM64 layout: @@ -190,6 +191,11 @@ extern unsigned long frametable_virt_end; #define watchdog_disable() ((void)0) #define watchdog_enable() ((void)0) +#if defined(__ASSEMBLY__) && !defined(__LINKER__) +#include <asm/asm-offsets.h> +#include <asm/macros.h> +#endif + #endif /* __ARM_CONFIG_H__ */ /* * Local variables: