Re: [XEN PATCH v8 01/47] build: factorise generation of the linker scripts

[Julien Grall <julien@xen.org>]

Hi Anthony,

On 25/11/2021 13:39, Anthony PERARD wrote:
> In Arm and X86 makefile, generating the linker script is the same, so
> we can simply have both call the same macro.
> 
> We need to add *.lds files into extra-y so that Rules.mk can find the
> .*.cmd dependency file and load it.
> 
> Change made to the command line:
> - Use cpp_flags macro which simply filter -Wa,% options from $(a_flags).
> - Added -D__LINKER__ even it is only used by Arm's lds.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> ---
> 
> Notes:
>      v6:
>      - CPP already used instead of CC -E
>      - -Ui386 already removed
>      - cpp_flags is now a macro
>      - rebased
>      
>      v5:
>      - rename cc_lds_S to cpp_lds_S as the binary runned is now CPP
>      - Use new cpp_flags instead of the open-coded filter of a_flags.
>      
>      v4:
>      - fix rebuild by adding FORCE as dependency
>      - Use $(CPP)
>      - remove -Ui386
>      - avoid using "define" for cmd_cc_lds_S, as adding '; \' on each line is
>        still mandatory for if_changed (or cmd) macro to work.
> 
>   xen/Rules.mk          | 4 ++++
>   xen/arch/arm/Makefile | 6 ++++--
>   xen/arch/x86/Makefile | 6 ++++--
>   3 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index 5e0699e58b2b..d21930a7bf71 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -238,6 +238,10 @@ cmd_cpp_s_S = $(CPP) $(call cpp_flags,$(a_flags)) -MQ $@ -o $@ $<
>   %.s: %.S FORCE
>   	$(call if_changed,cpp_s_S)
>   
> +# Linker scripts, .lds.S -> .lds
> +quiet_cmd_cpp_lds_S = LDS     $@
> +cmd_cpp_lds_S = $(CPP) -P $(call cpp_flags,$(a_flags)) -D__LINKER__ -MQ $@ -o $@ $<
> +
>   # Add intermediate targets:
>   # When building objects with specific suffix patterns, add intermediate
>   # targets that the final targets are derived from.
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 07f634508eee..a3a497bafe89 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -67,6 +67,8 @@ obj-y += vsmc.o
>   obj-y += vpsci.o
>   obj-y += vuart.o
>   
> +extra-y += xen.lds
> +
>   #obj-bin-y += ....o
>   
>   ifneq ($(CONFIG_DTB_FILE),"")
> @@ -132,8 +134,8 @@ $(TARGET)-syms: prelink.o xen.lds
>   .PHONY: include
>   include:
>   
> -xen.lds: xen.lds.S
> -	$(CPP) -P $(a_flags) -D__LINKER__ -MQ $@ -o $@ $<
> +xen.lds: xen.lds.S FORCE

Sorry, I haven't really followed the build system rework. Could you 
explain why it is necessary to add FORCE?

> +	$(call if_changed,cpp_lds_S)
>   
>   dtb.o: $(patsubst "%",%,$(CONFIG_DTB_FILE))
>   
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index 69b6cfaded25..669e16e72690 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -78,6 +78,7 @@ obj-y += sysctl.o
>   endif
>   
>   extra-y += asm-macros.i
> +extra-y += xen.lds
>   
>   ifneq ($(CONFIG_HVM),y)
>   x86_emulate.o: CFLAGS-y += -Wno-unused-label
> @@ -238,6 +239,7 @@ endif
>   note_file_option ?= $(note_file)
>   
>   ifeq ($(XEN_BUILD_PE),y)
> +extra-y += efi.lds
>   $(TARGET).efi: prelink.o $(note_file) efi.lds efi/relocs-dummy.o efi/mkreloc
>   ifeq ($(CONFIG_DEBUG_INFO),y)
>   	$(if $(filter --strip-debug,$(EFI_LDFLAGS)),echo,:) "Will strip debug info from $(@F)"
> @@ -290,8 +292,8 @@ $(BASEDIR)/include/asm-x86/asm-macros.h: asm-macros.i Makefile
>   	$(call move-if-changed,$@.new,$@)
>   
>   efi.lds: AFLAGS-y += -DEFI
> -xen.lds efi.lds: xen.lds.S
> -	$(CPP) -P $(call cpp_flags,$(a_flags)) -MQ $@ -o $@ $<
> +xen.lds efi.lds: xen.lds.S FORCE
> +	$(call if_changed,cpp_lds_S)
>   
>   boot/mkelf32: boot/mkelf32.c
>   	$(HOSTCC) $(HOSTCFLAGS) -o $@ $<

Cheers,

-- 
Julien Grall