diff mbox series

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

Message ID 20211125134006.1076646-2-anthony.perard@citrix.com (mailing list archive)
State Superseded
Headers show
Series xen: Build system improvements, now with out-of-tree build! | expand

Commit Message

Anthony PERARD Nov. 25, 2021, 1:39 p.m. UTC 
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(-)

Comments

Julien Grall Dec. 14, 2021, 4:54 p.m. UTC | #1 
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,
Anthony PERARD Dec. 14, 2021, 5:09 p.m. UTC | #2 
On Tue, Dec 14, 2021 at 04:54:21PM +0000, Julien Grall wrote:
> On 25/11/2021 13:39, Anthony PERARD wrote:
> > diff --git a/xen/Rules.mk b/xen/Rules.mk
> > +# 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 $@ $<
> > +

> > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> > +xen.lds: xen.lds.S FORCE
> > +	$(call if_changed,cpp_lds_S)
> 
> Sorry, I haven't really followed the build system rework. Could you explain
> why it is necessary to add FORCE?

This new rules make use of a new macro "if_changed", and we need to
execute this new macro every time, even if the source file hasn't been
modified.

if_changed, in addition to checking if the source is newer than the
target that make does, also compare the command line used to generate
the target. If the command line have changed, the macro update the
target.

The command line to execute is stored in "cmd_$1" variables, in this case
"cmd_cpp_lds_S". The macro if_changed store the command line in the file
".${target-name}.cmd".

So, every time the macro if_changed is used, we need to tell make to
always execute the recipe, thus FORCE.

Cheers,
Julien Grall Dec. 14, 2021, 5:12 p.m. UTC | #3 
Hi,

On 14/12/2021 17:09, Anthony PERARD wrote:
> On Tue, Dec 14, 2021 at 04:54:21PM +0000, Julien Grall wrote:
>> On 25/11/2021 13:39, Anthony PERARD wrote:
>>> diff --git a/xen/Rules.mk b/xen/Rules.mk
>>> +# 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 $@ $<
>>> +
> 
>>> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
>>> +xen.lds: xen.lds.S FORCE
>>> +	$(call if_changed,cpp_lds_S)
>>
>> Sorry, I haven't really followed the build system rework. Could you explain
>> why it is necessary to add FORCE?
> 
> This new rules make use of a new macro "if_changed", and we need to
> execute this new macro every time, even if the source file hasn't been
> modified.
> 
> if_changed, in addition to checking if the source is newer than the
> target that make does, also compare the command line used to generate
> the target. If the command line have changed, the macro update the
> target.

Ah, that's the part I was missing. Thanks for the clarification! With that:

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,
diff mbox series

Patch

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
+	$(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 $@ $<