| Message ID | 20200226113355.2532224-21-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: Build system improvements | expand |
On Wed, Feb 26, 2020 at 11:33:52AM +0000, 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. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > xen/Rules.mk | 8 ++++++++ > xen/arch/arm/Makefile | 5 ++--- > xen/arch/x86/Makefile | 6 +++--- > 3 files changed, 13 insertions(+), 6 deletions(-) > > diff --git a/xen/Rules.mk b/xen/Rules.mk > index 8c7dba9211d1..02cd37d04054 100644 > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -230,6 +230,14 @@ cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@ > %.s: %.S FORCE > $(call if_changed,cpp_s_S) > > +# Linker scripts, .lds.S -> .lds > +quiet_cmd_cc_lds_S = LDS $@ > +define cmd_cc_lds_S > + $(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<; \ Do you know why the -Ui386 is needed? Also, can this use the CPP rune? I would at least consider naming this CPP, as it's pre-processing the link script, LDS seems quite obscure. Thanks, Roger.
On 26.02.2020 12:33, Anthony PERARD wrote: > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -230,6 +230,14 @@ cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@ > %.s: %.S FORCE > $(call if_changed,cpp_s_S) > > +# Linker scripts, .lds.S -> .lds > +quiet_cmd_cc_lds_S = LDS $@ > +define cmd_cc_lds_S > + $(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<; \ $(CPP)? And then also name the thing cmd_cpp_lds_S? > + sed -e 's/.*\.lds\.o:/$(@F):/g' <$(dot-target).d >$(dot-target).d.new; \ > + mv -f $(dot-target).d.new $(dot-target).d This would benefit from also switching to move-if-changed at this occasion. With you using "define" - is there really a need for adding the trailing "; \" sequence to the first two lines of the macro? > --- a/xen/arch/x86/Makefile > +++ b/xen/arch/x86/Makefile > @@ -75,6 +75,7 @@ obj-y += hpet.o > obj-y += vm_event.o > obj-y += xstate.o > extra-y += asm-macros.i > +extra-y += xen.lds > > x86_emulate.o: x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h > > @@ -197,6 +198,7 @@ endif > note_file_option ?= $(note_file) > > ifeq ($(XEN_BUILD_PE),y) > +extra-y += efi.lds Would be nice if this was moved up using extra-$(XEN_BUILD_PE) += efi.lds Jan
On 27.02.2020 14:14, Roger Pau Monné wrote: > On Wed, Feb 26, 2020 at 11:33:52AM +0000, 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. >> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >> --- >> xen/Rules.mk | 8 ++++++++ >> xen/arch/arm/Makefile | 5 ++--- >> xen/arch/x86/Makefile | 6 +++--- >> 3 files changed, 13 insertions(+), 6 deletions(-) >> >> diff --git a/xen/Rules.mk b/xen/Rules.mk >> index 8c7dba9211d1..02cd37d04054 100644 >> --- a/xen/Rules.mk >> +++ b/xen/Rules.mk >> @@ -230,6 +230,14 @@ cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@ >> %.s: %.S FORCE >> $(call if_changed,cpp_s_S) >> >> +# Linker scripts, .lds.S -> .lds >> +quiet_cmd_cc_lds_S = LDS $@ >> +define cmd_cc_lds_S >> + $(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<; \ > > Do you know why the -Ui386 is needed? It was needed for the 32-bit hypervisor build, to avoid corrupting OUTPUT_ARCH(i386) but it's not needed anymore. Arm shouldn't have had it in the first place. Jan
On Thu, Mar 05, 2020 at 12:05:02PM +0100, Jan Beulich wrote: > On 26.02.2020 12:33, Anthony PERARD wrote: > > --- a/xen/Rules.mk > > +++ b/xen/Rules.mk > > @@ -230,6 +230,14 @@ cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@ > > %.s: %.S FORCE > > $(call if_changed,cpp_s_S) > > > > +# Linker scripts, .lds.S -> .lds > > +quiet_cmd_cc_lds_S = LDS $@ > > +define cmd_cc_lds_S > > + $(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<; \ > > $(CPP)? And then also name the thing cmd_cpp_lds_S? Will do. > > + sed -e 's/.*\.lds\.o:/$(@F):/g' <$(dot-target).d >$(dot-target).d.new; \ > > + mv -f $(dot-target).d.new $(dot-target).d > > This would benefit from also switching to move-if-changed at > this occasion. I don't think using move-if-changed here is a good idea. The *.lds file should be generated if it's dependency *.lds.S changed. move-if-changed might prevent some rebuild, but the *.lds file will be rebuild over and over again. I think move-if-changed is only useful if a file needs to be generated at every make invocation. > With you using "define" - is there really a need for adding the > trailing "; \" sequence to the first two lines of the macro? I think it is, but I'll check again if it's necessary. Maybe just ';' is enough. > > --- a/xen/arch/x86/Makefile > > +++ b/xen/arch/x86/Makefile > > @@ -75,6 +75,7 @@ obj-y += hpet.o > > obj-y += vm_event.o > > obj-y += xstate.o > > extra-y += asm-macros.i > > +extra-y += xen.lds > > > > x86_emulate.o: x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h > > > > @@ -197,6 +198,7 @@ endif > > note_file_option ?= $(note_file) > > > > ifeq ($(XEN_BUILD_PE),y) > > +extra-y += efi.lds > > Would be nice if this was moved up using > > extra-$(XEN_BUILD_PE) += efi.lds I can try, but XEN_BUILD_PE is defined in the middle of the Makefile, so I wouldn't be able to move that with the other obj-y and extra-y definitions. Thanks,
diff --git a/xen/Rules.mk b/xen/Rules.mk index 8c7dba9211d1..02cd37d04054 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -230,6 +230,14 @@ cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@ %.s: %.S FORCE $(call if_changed,cpp_s_S) +# Linker scripts, .lds.S -> .lds +quiet_cmd_cc_lds_S = LDS $@ +define cmd_cc_lds_S + $(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<; \ + sed -e 's/.*\.lds\.o:/$(@F):/g' <$(dot-target).d >$(dot-target).d.new; \ + mv -f $(dot-target).d.new $(dot-target).d +endef + # 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 37ca6d25c08e..b3ee4adb9ac4 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -124,9 +124,8 @@ asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(CC) $(filter-out -flto,$(c_flags)) -S -o $@ $< xen.lds: xen.lds.S - $(CC) -P -E -Ui386 $(a_flags) -o $@ $< - sed -e 's/xen\.lds\.o:/xen\.lds:/g' <.xen.lds.d >.xen.lds.d.new - mv -f .xen.lds.d.new .xen.lds.d + $(call if_changed,cc_lds_S) +extra-y += xen.lds dtb.o: $(CONFIG_DTB_FILE) diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 6fb6cafdf47b..1be94846e11f 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -75,6 +75,7 @@ obj-y += hpet.o obj-y += vm_event.o obj-y += xstate.o extra-y += asm-macros.i +extra-y += xen.lds x86_emulate.o: x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h @@ -197,6 +198,7 @@ endif note_file_option ?= $(note_file) ifeq ($(XEN_BUILD_PE),y) +extra-y += efi.lds $(TARGET).efi: prelink-efi.o $(note_file) efi.lds efi/relocs-dummy.o efi/mkreloc $(foreach base, $(VIRT_BASE) $(ALT_BASE), \ $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< efi/relocs-dummy.o \ @@ -244,9 +246,7 @@ $(BASEDIR)/include/asm-x86/asm-macros.h: asm-macros.i Makefile efi.lds: AFLAGS-y += -DEFI xen.lds efi.lds: xen.lds.S - $(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $< - sed -e 's/.*\.lds\.o:/$(@F):/g' <.$(@F).d >.$(@F).d.new - mv -f .$(@F).d.new .$(@F).d + $(call if_changed,cc_lds_S) boot/mkelf32: boot/mkelf32.c $(HOSTCC) $(HOSTCFLAGS) -o $@ $<
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. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- xen/Rules.mk | 8 ++++++++ xen/arch/arm/Makefile | 5 ++--- xen/arch/x86/Makefile | 6 +++--- 3 files changed, 13 insertions(+), 6 deletions(-)