| Message ID | 20210824105038.1257926-22-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: Build system improvements, now with out-of-tree build! | expand |
On 24.08.2021 12:50, Anthony PERARD wrote: > --- a/xen/Makefile > +++ b/xen/Makefile > @@ -271,8 +271,21 @@ CFLAGS += -flto > LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so > endif > > +# Note that link order matters! > +ALL_OBJS-y := common/built_in.o > +ALL_OBJS-y += drivers/built_in.o > +ALL_OBJS-y += lib/built_in.o > +ALL_OBJS-y += xsm/built_in.o > +ALL_OBJS-y += arch/$(TARGET_ARCH)/built_in.o > +ALL_OBJS-$(CONFIG_CRYPTO) += crypto/built_in.o > + > +ALL_LIBS-y := lib/lib.a > + > include $(BASEDIR)/arch/$(TARGET_ARCH)/arch.mk > > +export ALL_OBJS := $(ALL_OBJS-y) > +export ALL_LIBS := $(ALL_LIBS-y) > + > # define new variables to avoid the ones defined in Config.mk > export XEN_CFLAGS := $(CFLAGS) > export XEN_AFLAGS := $(AFLAGS) > @@ -393,7 +406,7 @@ $(TARGET): FORCE > $(MAKE) -f $(BASEDIR)/Rules.mk -C include > $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) include > $(MAKE) -f $(BASEDIR)/Rules.mk arch/$(TARGET_ARCH)/include/asm/asm-offsets.h > - $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) $@ > + $(MAKE) -f $(BASEDIR)/Rules.mk $@ This merely results in what was previously invoked from here now getting invoked from the very bottom of build.mk. I'm afraid I don't see why this is a useful change to make. > --- a/xen/build.mk > +++ b/xen/build.mk > @@ -56,3 +56,27 @@ arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: asm-offsets.s > sed -rne "/^[^#].*==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}"; \ > echo ""; \ > echo "#endif") <$< >$@ > + > +# head.o is built by descending into arch/arm/$(TARGET_SUBARCH), depends on the > +# part of $(ALL_OBJS) that will eventually recurse into $(TARGET_SUBARCH)/ and > +# build head.o > +arch/arm/$(TARGET_SUBARCH)/head.o: arch/arm/built_in.o > +arch/arm/$(TARGET_SUBARCH)/head.o: ; This previously lived in an Arm-specific file. Moving this here in the given, still Arm-specific form is imo a no-go when done alongside all the other good changes you're making. Is there a reason this can't go into xen/arch/arm/arch.mk? Jan
On Mon, Oct 11, 2021 at 01:31:59PM +0200, Jan Beulich wrote: > On 24.08.2021 12:50, Anthony PERARD wrote: > > --- a/xen/Makefile > > +++ b/xen/Makefile > > @@ -271,8 +271,21 @@ CFLAGS += -flto > > LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so > > endif > > > > +# Note that link order matters! > > +ALL_OBJS-y := common/built_in.o > > +ALL_OBJS-y += drivers/built_in.o > > +ALL_OBJS-y += lib/built_in.o > > +ALL_OBJS-y += xsm/built_in.o > > +ALL_OBJS-y += arch/$(TARGET_ARCH)/built_in.o > > +ALL_OBJS-$(CONFIG_CRYPTO) += crypto/built_in.o > > + > > +ALL_LIBS-y := lib/lib.a > > + > > include $(BASEDIR)/arch/$(TARGET_ARCH)/arch.mk > > > > +export ALL_OBJS := $(ALL_OBJS-y) > > +export ALL_LIBS := $(ALL_LIBS-y) > > + > > # define new variables to avoid the ones defined in Config.mk > > export XEN_CFLAGS := $(CFLAGS) > > export XEN_AFLAGS := $(AFLAGS) > > @@ -393,7 +406,7 @@ $(TARGET): FORCE > > $(MAKE) -f $(BASEDIR)/Rules.mk -C include > > $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) include > > $(MAKE) -f $(BASEDIR)/Rules.mk arch/$(TARGET_ARCH)/include/asm/asm-offsets.h > > - $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) $@ > > + $(MAKE) -f $(BASEDIR)/Rules.mk $@ > > This merely results in what was previously invoked from here now getting > invoked from the very bottom of build.mk. I'm afraid I don't see why this > is a useful change to make. Would you rather have this following change? @@ -393,7 +406,8 @@ $(TARGET): FORCE $(MAKE) -f $(BASEDIR)/Rules.mk -C include $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) include $(MAKE) -f $(BASEDIR)/Rules.mk arch/$(TARGET_ARCH)/include/asm/asm-offsets.h + $(MAKE) -f $(BASEDIR)/Rules.mk prelink.o $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) $@ That would probably be fine. > > --- a/xen/build.mk > > +++ b/xen/build.mk > > @@ -56,3 +56,27 @@ arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: asm-offsets.s > > sed -rne "/^[^#].*==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}"; \ > > echo ""; \ > > echo "#endif") <$< >$@ > > + > > +# head.o is built by descending into arch/arm/$(TARGET_SUBARCH), depends on the > > +# part of $(ALL_OBJS) that will eventually recurse into $(TARGET_SUBARCH)/ and > > +# build head.o > > +arch/arm/$(TARGET_SUBARCH)/head.o: arch/arm/built_in.o > > +arch/arm/$(TARGET_SUBARCH)/head.o: ; > > This previously lived in an Arm-specific file. Moving this here in the > given, still Arm-specific form is imo a no-go when done alongside all > the other good changes you're making. Is there a reason this can't go > into xen/arch/arm/arch.mk? This is temporary and it is removed in patch "build: build everything from the root dir, use obj=$subdir" but I could move it to "arch/arm/Rules.mk" I think. Thanks,
On 13.10.2021 14:30, Anthony PERARD wrote: > On Mon, Oct 11, 2021 at 01:31:59PM +0200, Jan Beulich wrote: >> On 24.08.2021 12:50, Anthony PERARD wrote: >>> @@ -393,7 +406,7 @@ $(TARGET): FORCE >>> $(MAKE) -f $(BASEDIR)/Rules.mk -C include >>> $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) include >>> $(MAKE) -f $(BASEDIR)/Rules.mk arch/$(TARGET_ARCH)/include/asm/asm-offsets.h >>> - $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) $@ >>> + $(MAKE) -f $(BASEDIR)/Rules.mk $@ >> >> This merely results in what was previously invoked from here now getting >> invoked from the very bottom of build.mk. I'm afraid I don't see why this >> is a useful change to make. > > Would you rather have this following change? > > @@ -393,7 +406,8 @@ $(TARGET): FORCE > $(MAKE) -f $(BASEDIR)/Rules.mk -C include > $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) include > $(MAKE) -f $(BASEDIR)/Rules.mk arch/$(TARGET_ARCH)/include/asm/asm-offsets.h > + $(MAKE) -f $(BASEDIR)/Rules.mk prelink.o > $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) $@ > > That would probably be fine. Hmm, perhaps I'd prefer to avoid yet another $(MAKE) invocation. But your reply made me understand why you make the change: You need to add the prelink.o dependency to $(TARGET). That wasn't obvious to me when first reviewing the change. >>> --- a/xen/build.mk >>> +++ b/xen/build.mk >>> @@ -56,3 +56,27 @@ arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: asm-offsets.s >>> sed -rne "/^[^#].*==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}"; \ >>> echo ""; \ >>> echo "#endif") <$< >$@ >>> + >>> +# head.o is built by descending into arch/arm/$(TARGET_SUBARCH), depends on the >>> +# part of $(ALL_OBJS) that will eventually recurse into $(TARGET_SUBARCH)/ and >>> +# build head.o >>> +arch/arm/$(TARGET_SUBARCH)/head.o: arch/arm/built_in.o >>> +arch/arm/$(TARGET_SUBARCH)/head.o: ; >> >> This previously lived in an Arm-specific file. Moving this here in the >> given, still Arm-specific form is imo a no-go when done alongside all >> the other good changes you're making. Is there a reason this can't go >> into xen/arch/arm/arch.mk? > > This is temporary and it is removed in patch > "build: build everything from the root dir, use obj=$subdir" > but I could move it to "arch/arm/Rules.mk" I think. Moving there would be preferred; if that somehow doesn't work out, please mention the temporary nature in the description, or else I (or perhaps others) would ask the same question again on a future version of the series. Jan
diff --git a/xen/Makefile b/xen/Makefile index 7f100845cdd0..1dad20a95be6 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -271,8 +271,21 @@ CFLAGS += -flto LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so endif +# Note that link order matters! +ALL_OBJS-y := common/built_in.o +ALL_OBJS-y += drivers/built_in.o +ALL_OBJS-y += lib/built_in.o +ALL_OBJS-y += xsm/built_in.o +ALL_OBJS-y += arch/$(TARGET_ARCH)/built_in.o +ALL_OBJS-$(CONFIG_CRYPTO) += crypto/built_in.o + +ALL_LIBS-y := lib/lib.a + include $(BASEDIR)/arch/$(TARGET_ARCH)/arch.mk +export ALL_OBJS := $(ALL_OBJS-y) +export ALL_LIBS := $(ALL_LIBS-y) + # define new variables to avoid the ones defined in Config.mk export XEN_CFLAGS := $(CFLAGS) export XEN_AFLAGS := $(AFLAGS) @@ -393,7 +406,7 @@ $(TARGET): FORCE $(MAKE) -f $(BASEDIR)/Rules.mk -C include $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) include $(MAKE) -f $(BASEDIR)/Rules.mk arch/$(TARGET_ARCH)/include/asm/asm-offsets.h - $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) $@ + $(MAKE) -f $(BASEDIR)/Rules.mk $@ SUBDIRS = xsm arch/$(TARGET_ARCH) common drivers lib test define all_sources diff --git a/xen/Rules.mk b/xen/Rules.mk index a49ca4ceca39..2db13a8f9c54 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -8,26 +8,12 @@ include $(XEN_ROOT)/Config.mk include $(BASEDIR)/scripts/Kbuild.include - -# Note that link order matters! -ALL_OBJS-y += $(BASEDIR)/common/built_in.o -ALL_OBJS-y += $(BASEDIR)/drivers/built_in.o -ALL_OBJS-y += $(BASEDIR)/lib/built_in.o -ALL_OBJS-y += $(BASEDIR)/xsm/built_in.o -ALL_OBJS-y += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o -ALL_OBJS-$(CONFIG_CRYPTO) += $(BASEDIR)/crypto/built_in.o - -ALL_LIBS-y := $(BASEDIR)/lib/lib.a - # Initialise some variables lib-y := targets := CFLAGS-y := AFLAGS-y := -ALL_OBJS := $(ALL_OBJS-y) -ALL_LIBS := $(ALL_LIBS-y) - SPECIAL_DATA_SECTIONS := rodata $(foreach a,1 2 4 8 16, \ $(foreach w,1 2 4, \ rodata.str$(w).$(a)) \ diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index dc1d09c8b429..067c0d9844e4 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -73,14 +73,6 @@ ifneq ($(CONFIG_DTB_FILE),"") obj-y += dtb.o endif -ALL_OBJS := $(TARGET_SUBARCH)/head.o $(ALL_OBJS) - -# head.o is built by descending into the sub-directory, depends on the part of -# $(ALL_OBJS) that will eventually recurse into $(TARGET_SUBARCH)/ and build -# head.o -$(TARGET_SUBARCH)/head.o: $(BASEDIR)/arch/arm/built_in.o -$(TARGET_SUBARCH)/head.o: ; - ifdef CONFIG_LIVEPATCH all_symbols = --all-symbols ifdef CONFIG_FAST_SYMBOL_LOOKUP @@ -96,33 +88,18 @@ ifeq ($(CONFIG_ARM_64),y) ln -sf $(@F) $@.efi endif -ifeq ($(CONFIG_LTO),y) -# Gather all LTO objects together -prelink_lto.o: $(ALL_OBJS) $(ALL_LIBS) - $(LD_LTO) -r -o $@ $(filter-out %.a,$^) --start-group $(filter %.a,$^) --end-group - -# Link it with all the binary objects -prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o - $(call if_changed,ld) -else -prelink.o: $(ALL_OBJS) $(ALL_LIBS) FORCE - $(call if_changed,ld) -endif - -targets += prelink.o - -$(TARGET)-syms: prelink.o xen.lds - $(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o \ +$(TARGET)-syms: $(BASEDIR)/prelink.o xen.lds + $(LD) $(XEN_LDFLAGS) -T xen.lds -N $< \ $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0 $(NM) -pa --format=sysv $(@D)/.$(@F).0 \ | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0.S $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o - $(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o \ + $(LD) $(XEN_LDFLAGS) -T xen.lds -N $< \ $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1 $(NM) -pa --format=sysv $(@D)/.$(@F).1 \ | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1.S $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o - $(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \ + $(LD) $(XEN_LDFLAGS) -T xen.lds -N $< $(build_id_linker) \ $(@D)/.$(@F).1.o -o $@ $(NM) -pa --format=sysv $(@D)/$(@F) \ | $(BASEDIR)/tools/symbols --all-symbols --xensyms --sysv --sort \ diff --git a/xen/arch/arm/arch.mk b/xen/arch/arm/arch.mk index c3ac443b3788..ba3f140e2ea7 100644 --- a/xen/arch/arm/arch.mk +++ b/xen/arch/arm/arch.mk @@ -26,3 +26,5 @@ ifeq ($(CONFIG_ARM64_ERRATUM_843419),y) LDFLAGS += --fix-cortex-a53-843419 endif endif + +ALL_OBJS-y := arch/arm/$(TARGET_SUBARCH)/head.o $(ALL_OBJS-y) diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 29aa67ea371e..823f8fed4144 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -126,37 +126,20 @@ ifneq ($(efi-y),) CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI endif # $(efi-y) -ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS) - -ifeq ($(CONFIG_LTO),y) -# Gather all LTO objects together -prelink_lto.o: $(ALL_OBJS) $(ALL_LIBS) - $(LD_LTO) -r -o $@ $(filter-out %.a,$^) --start-group $(filter %.a,$^) --end-group - -# Link it with all the binary objects -prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o FORCE - $(call if_changed,ld) -else -prelink.o: $(ALL_OBJS) $(ALL_LIBS) FORCE - $(call if_changed,ld) -endif - -targets += prelink.o - -$(TARGET)-syms: prelink.o xen.lds - $(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \ +$(TARGET)-syms: $(BASEDIR)/prelink.o xen.lds + $(LD) $(XEN_LDFLAGS) -T xen.lds -N $< $(build_id_linker) \ $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0 $(NM) -pa --format=sysv $(@D)/.$(@F).0 \ | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort \ >$(@D)/.$(@F).0.S $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o - $(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \ + $(LD) $(XEN_LDFLAGS) -T xen.lds -N $< $(build_id_linker) \ $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1 $(NM) -pa --format=sysv $(@D)/.$(@F).1 \ | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort $(syms-warn-dup-y) \ >$(@D)/.$(@F).1.S $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o - $(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \ + $(LD) $(XEN_LDFLAGS) -T xen.lds -N $< $(build_id_linker) \ $(@D)/.$(@F).1.o -o $@ $(NM) -pa --format=sysv $(@D)/$(@F) \ | $(BASEDIR)/tools/symbols --all-symbols --xensyms --sysv --sort \ @@ -209,7 +192,7 @@ 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 +$(TARGET).efi: $(BASEDIR)/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)" endif @@ -238,7 +221,7 @@ $(TARGET).efi: FORCE echo '$(if $(filter y,$(XEN_BUILD_EFI)),xen.efi generation,EFI support) disabled' endif -efi/buildid.o efi/relocs-dummy.o: $(BASEDIR)/arch/x86/efi/built_in.o +# These should already have been rebuilt when building the prerequisite of "prelink.o" efi/buildid.o efi/relocs-dummy.o: ; .PHONY: include diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk index 98dd41d32118..61e0222f4a08 100644 --- a/xen/arch/x86/arch.mk +++ b/xen/arch/x86/arch.mk @@ -104,3 +104,5 @@ endif # Set up the assembler include path properly for older toolchains. CFLAGS += -Wa,-I$(BASEDIR)/include + +ALL_OBJS-y := arch/x86/boot/built_in.o arch/x86/efi/built_in.o $(ALL_OBJS-y) diff --git a/xen/build.mk b/xen/build.mk index 369e1fe5c698..9093d9f493bc 100644 --- a/xen/build.mk +++ b/xen/build.mk @@ -56,3 +56,27 @@ arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: asm-offsets.s sed -rne "/^[^#].*==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}"; \ echo ""; \ echo "#endif") <$< >$@ + +# head.o is built by descending into arch/arm/$(TARGET_SUBARCH), depends on the +# part of $(ALL_OBJS) that will eventually recurse into $(TARGET_SUBARCH)/ and +# build head.o +arch/arm/$(TARGET_SUBARCH)/head.o: arch/arm/built_in.o +arch/arm/$(TARGET_SUBARCH)/head.o: ; + +ifeq ($(CONFIG_LTO),y) +# Gather all LTO objects together +prelink_lto.o: $(ALL_OBJS) $(ALL_LIBS) + $(LD_LTO) -r -o $@ $(filter-out %.a,$^) --start-group $(filter %.a,$^) --end-group + +# Link it with all the binary objects +prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o FORCE + $(call if_changed,ld) +else +prelink.o: $(ALL_OBJS) $(ALL_LIBS) FORCE + $(call if_changed,ld) +endif + +targets += prelink.o + +$(TARGET): prelink.o FORCE + $(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) $@
This is to avoid arch/$arch/Makefile having to recurse into parents directories. This avoid duplication of the logic to build prelink.o between arches. In order to do that, we cut the $(TARGET) target in the main Makefile in two, there is a "prepare" phase/target runned before starting to build "prelink.o" which will prepare "include/" among other things, the all the $(ALL_OBJS) will be generated in order to build "prelink.o" and finally $(TARGET) will be generated by calling into "arch/*/" to make $(TARGET). Now we don't need to prefix $(ALL_OBJS) with $(BASEDIR) as it is now only used from the main Makefile. Other changes is using "$<" instead of spelling "prelink.o" in the target "$(TARGET)" in both arch/*/Makefile. Beside "prelink.o" been at a different location, no other functional change intended. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Notes: v7: - change, now things are in build.mk: no more prepare phase needed xen/Makefile | 15 ++++++++++++++- xen/Rules.mk | 14 -------------- xen/arch/arm/Makefile | 31 ++++--------------------------- xen/arch/arm/arch.mk | 2 ++ xen/arch/x86/Makefile | 29 ++++++----------------------- xen/arch/x86/arch.mk | 2 ++ xen/build.mk | 24 ++++++++++++++++++++++++ 7 files changed, 52 insertions(+), 65 deletions(-)