| Message ID | 20230523163811.30792-8-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | build: cleanup build log, avoid user's CFLAGS, avoid too many include of Config.mk | expand |
> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote: > > Whether or not the linker can do build id is only used by the > hypervisor build, so move that there. > > Rename $(build_id_linker) to $(XEN_LDFLAGS_BUILD_ID) as this is a > better name to be exported as to use the "XEN_*" namespace. > > Also update XEN_TREEWIDE_CFLAGS so flags can be used for > arch/x86/boot/ CFLAGS_x86_32 > > Beside a reordering of the command line where CFLAGS is used, there > shouldn't be any other changes. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Seems good to me! Reviewed-by: Luca Fancellu <luca.fancellu@arm.com> Tested-by: Luca Fancellu <luca.fancellu@arm.com>
On 23.05.2023 18:38, Anthony PERARD wrote: > Whether or not the linker can do build id is only used by the > hypervisor build, so move that there. > > Rename $(build_id_linker) to $(XEN_LDFLAGS_BUILD_ID) as this is a > better name to be exported as to use the "XEN_*" namespace. > > Also update XEN_TREEWIDE_CFLAGS so flags can be used for > arch/x86/boot/ CFLAGS_x86_32 > > Beside a reordering of the command line where CFLAGS is used, there > shouldn't be any other changes. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> with one nit: > --- a/xen/scripts/Kbuild.include > +++ b/xen/scripts/Kbuild.include > @@ -91,6 +91,9 @@ cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || e > > clang-ifversion = $(shell [ $(CONFIG_CLANG_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4)) > > +ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \ > + grep -q build-id && echo n || echo y) I realize you only move this line, but I think we want indentation here to improve at this occasion: ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \ grep -q build-id && echo n || echo y) I'll be happy to adjust while committing. Which raises the question: Is there any dependency here on earlier patches in the series? It doesn't look so to me, but I may easily be overlooking something. (Of course first further arch maintainer acks would be needed.) > --- a/xen/test/livepatch/Makefile > +++ b/xen/test/livepatch/Makefile > @@ -37,7 +37,7 @@ $(obj)/modinfo.o: > > # > # This target is only accessible if CONFIG_LIVEPATCH is defined, which > -# depends on $(build_id_linker) being available. Hence we do not > +# depends on $(XEN_LDFLAGS_BUILD_ID) being available. Hence we do not > # need any checks. As an aside, I'm a little confused by "is only accessible" here. I don't see how CONFIG_LIVEPATCH controls reachability. At the very least the parent dir's Makefile doesn't use CONFIG_LIVEPATCH at all. Jan
On Thu, May 25, 2023 at 01:56:53PM +0200, Jan Beulich wrote: > On 23.05.2023 18:38, Anthony PERARD wrote: > > Whether or not the linker can do build id is only used by the > > hypervisor build, so move that there. > > > > Rename $(build_id_linker) to $(XEN_LDFLAGS_BUILD_ID) as this is a > > better name to be exported as to use the "XEN_*" namespace. > > > > Also update XEN_TREEWIDE_CFLAGS so flags can be used for > > arch/x86/boot/ CFLAGS_x86_32 > > > > Beside a reordering of the command line where CFLAGS is used, there > > shouldn't be any other changes. > > > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > with one nit: > > > --- a/xen/scripts/Kbuild.include > > +++ b/xen/scripts/Kbuild.include > > @@ -91,6 +91,9 @@ cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || e > > > > clang-ifversion = $(shell [ $(CONFIG_CLANG_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4)) > > > > +ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \ > > + grep -q build-id && echo n || echo y) > > I realize you only move this line, but I think we want indentation here > to improve at this occasion: > > ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \ > grep -q build-id && echo n || echo y) > > I'll be happy to adjust while committing. Which raises the question: Is > there any dependency here on earlier patches in the series? It doesn't > look so to me, but I may easily be overlooking something. (Of course > first further arch maintainer acks would be needed.) I don't see any dependency on earlier patch, so it looks fine to apply earlier. Thanks. > > --- a/xen/test/livepatch/Makefile > > +++ b/xen/test/livepatch/Makefile > > @@ -37,7 +37,7 @@ $(obj)/modinfo.o: > > > > # > > # This target is only accessible if CONFIG_LIVEPATCH is defined, which > > -# depends on $(build_id_linker) being available. Hence we do not > > +# depends on $(XEN_LDFLAGS_BUILD_ID) being available. Hence we do not > > # need any checks. > > As an aside, I'm a little confused by "is only accessible" here. I don't > see how CONFIG_LIVEPATCH controls reachability. At the very least the > parent dir's Makefile doesn't use CONFIG_LIVEPATCH at all. Yes. `make tests` works just fine without CONFIG_LIVEPATCH. Even when that comment was introduce, it didn't seems like there were anything preventing the target from been run. The only thing is that `make tests` doesn't build the tests if xen-syms was built without build_id, so the status of CONFIG_LIVEPATCH doesn't seems to matter. Thanks,
diff --git a/Config.mk b/Config.mk index d12d4c2b8f..27f48f654a 100644 --- a/Config.mk +++ b/Config.mk @@ -125,18 +125,6 @@ endef check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least gcc-4.1") $(eval $(check-y)) -ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \ - grep -q build-id && echo n || echo y) - -export XEN_HAS_BUILD_ID ?= n -ifeq ($(call ld-ver-build-id,$(LD)),n) -build_id_linker := -else -CFLAGS += -DBUILD_ID -export XEN_HAS_BUILD_ID=y -build_id_linker := --build-id=sha1 -endif - define buildmakevars2shellvars export PREFIX="$(prefix)"; \ export XEN_SCRIPT_DIR="$(XEN_SCRIPT_DIR)"; \ diff --git a/xen/Makefile b/xen/Makefile index 27f70d2200..4dc960df2c 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -286,6 +286,18 @@ CFLAGS += $(CLANG_FLAGS) export CLANG_FLAGS endif +# XEN_HAS_BUILD_ID needed by Kconfig +ifeq ($(call ld-ver-build-id,$(LD)),n) +XEN_LDFLAGS_BUILD_ID := +XEN_HAS_BUILD_ID := n +else +CFLAGS += -DBUILD_ID +XEN_TREEWIDE_CFLAGS += -DBUILD_ID +XEN_HAS_BUILD_ID := y +XEN_LDFLAGS_BUILD_ID := --build-id=sha1 +endif +export XEN_HAS_BUILD_ID XEN_LDFLAGS_BUILD_ID + export XEN_HAS_CHECKPOLICY := $(call success,$(CHECKPOLICY) -h 2>&1 | grep -q xen) # =========================================================================== diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 4d076b278b..1cc57d2cf0 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -102,7 +102,7 @@ $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds $(NM) -pa --format=sysv $(@D)/.$(@F).1 \ | $(objtree)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1.S $(MAKE) $(build)=$(@D) $(@D)/.$(@F).1.o - $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \ + $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(XEN_LDFLAGS_BUILD_ID) \ $(@D)/.$(@F).1.o -o $@ $(NM) -pa --format=sysv $(@D)/$(@F) \ | $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \ diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile index 443f6bf15f..8a0e483c66 100644 --- a/xen/arch/riscv/Makefile +++ b/xen/arch/riscv/Makefile @@ -9,7 +9,7 @@ $(TARGET): $(TARGET)-syms $(OBJCOPY) -O binary -S $< $@ $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds - $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) -o $@ + $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(XEN_LDFLAGS_BUILD_ID) -o $@ $(NM) -pa --format=sysv $(@D)/$(@F) \ | $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \ >$(@D)/$(@F).map diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 2672d7f4ee..c7ec315fe6 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -100,7 +100,7 @@ efi-y := $(shell if [ ! -r $(objtree)/include/xen/compile.h -o \ $(space) efi-$(CONFIG_PV_SHIM_EXCLUSIVE) := -ifneq ($(build_id_linker),) +ifneq ($(XEN_LDFLAGS_BUILD_ID),) notes_phdrs = --notes else ifeq ($(CONFIG_PVH_GUEST),y) @@ -136,19 +136,19 @@ $(TARGET): $(TARGET)-syms $(efi-y) $(obj)/boot/mkelf32 CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds - $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \ + $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(XEN_LDFLAGS_BUILD_ID) \ $(objtree)/common/symbols-dummy.o -o $(@D)/.$(@F).0 $(NM) -pa --format=sysv $(@D)/.$(@F).0 \ | $(objtree)/tools/symbols $(all_symbols) --sysv --sort \ >$(@D)/.$(@F).0.S $(MAKE) $(build)=$(@D) $(@D)/.$(@F).0.o - $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \ + $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(XEN_LDFLAGS_BUILD_ID) \ $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1 $(NM) -pa --format=sysv $(@D)/.$(@F).1 \ | $(objtree)/tools/symbols $(all_symbols) --sysv --sort $(syms-warn-dup-y) \ >$(@D)/.$(@F).1.S $(MAKE) $(build)=$(@D) $(@D)/.$(@F).1.o - $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \ + $(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(XEN_LDFLAGS_BUILD_ID) \ $(orphan-handling-y) $(@D)/.$(@F).1.o -o $@ $(NM) -pa --format=sysv $(@D)/$(@F) \ | $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \ @@ -186,10 +186,10 @@ relocs-dummy := $(obj)/efi/relocs-dummy.o $(TARGET).efi: ALT_BASE = 0x$(shell $(NM) $(obj)/efi/relocs-dummy.o | sed -n 's, A ALT_START$$,,p') endif -ifneq ($(build_id_linker),) +ifneq ($(XEN_LDFLAGS_BUILD_ID),) ifeq ($(call ld-ver-build-id,$(LD) $(filter -m%,$(EFI_LDFLAGS))),y) CFLAGS-y += -DBUILD_ID_EFI -EFI_LDFLAGS += $(build_id_linker) +EFI_LDFLAGS += $(XEN_LDFLAGS_BUILD_ID) note_file := $(obj)/efi/buildid.o # NB: this must be the last input in the linker call, because inputs following # the -b option will all be treated as being in the specified format. diff --git a/xen/scripts/Kbuild.include b/xen/scripts/Kbuild.include index 785a32c32e..d820595e2f 100644 --- a/xen/scripts/Kbuild.include +++ b/xen/scripts/Kbuild.include @@ -91,6 +91,9 @@ cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || e clang-ifversion = $(shell [ $(CONFIG_CLANG_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4)) +ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \ + grep -q build-id && echo n || echo y) + ### # Shorthand for $(Q)$(MAKE) -f scripts/Makefile.build obj= # Usage: diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile index c258ab0b59..c78f3ce06f 100644 --- a/xen/test/livepatch/Makefile +++ b/xen/test/livepatch/Makefile @@ -37,7 +37,7 @@ $(obj)/modinfo.o: # # This target is only accessible if CONFIG_LIVEPATCH is defined, which -# depends on $(build_id_linker) being available. Hence we do not +# depends on $(XEN_LDFLAGS_BUILD_ID) being available. Hence we do not # need any checks. # # N.B. The reason we don't use arch/x86/note.o is that it may @@ -142,7 +142,7 @@ xen_expectations_fail-objs := xen_expectations_fail.o xen_hello_world_func.o not quiet_cmd_livepatch = LD $@ -cmd_livepatch = $(LD) $(XEN_LDFLAGS) $(build_id_linker) -r -o $@ $(real-prereqs) +cmd_livepatch = $(LD) $(XEN_LDFLAGS) $(XEN_LDFLAGS_BUILD_ID) -r -o $@ $(real-prereqs) $(obj)/%.livepatch: FORCE $(call if_changed,livepatch)
Whether or not the linker can do build id is only used by the hypervisor build, so move that there. Rename $(build_id_linker) to $(XEN_LDFLAGS_BUILD_ID) as this is a better name to be exported as to use the "XEN_*" namespace. Also update XEN_TREEWIDE_CFLAGS so flags can be used for arch/x86/boot/ CFLAGS_x86_32 Beside a reordering of the command line where CFLAGS is used, there shouldn't be any other changes. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Config.mk | 12 ------------ xen/Makefile | 12 ++++++++++++ xen/arch/arm/Makefile | 2 +- xen/arch/riscv/Makefile | 2 +- xen/arch/x86/Makefile | 12 ++++++------ xen/scripts/Kbuild.include | 3 +++ xen/test/livepatch/Makefile | 4 ++-- 7 files changed, 25 insertions(+), 22 deletions(-)