[XEN PATCH 00/15] build: cleanup build log, avoid user's CFLAGS, avoid too many include of Config.mk

[XEN PATCH 00/15] build: cleanup build log, avoid user's CFLAGS, avoid too many include of Config.mk

[Anthony PERARD]

Patch series available in this git branch:
https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.build-system-xen-removing-config.mk-v1

Hi,

This series of patch cleanup the remaining rules still displaying their command
line.

Then, some change are made in Config.mk to remove build-id stuff only used by
Xen build.

Then, the variable AFLAGS and CFLAGS are been renamed XEN_AFLAGS and XEN_CFLAGS
from the beginning to about inclusion of users CFLAGS as those are usually made
for user space program, not kernel, especially in build environment for distro
packages.

Last patch removes the inclusion of Config.mk from xen/Rules.mk, as this slow
down the build unnecessarily. xen/Makefile should do everything necessary to
setup the build of the hypervisor, and is its only entry point.

Thanks,

Anthony PERARD (15):
  build: hide that we are updating xen/lib/x86
  build: rework asm-offsets.* build step to use kbuild
  build, x86: clean build log for boot/ targets
  build: hide policy.bin commands
  build: introduce a generic command for gzip
  build: quiet for .allconfig.tmp target
  build: move XEN_HAS_BUILD_ID out of Config.mk
  build: use $(filechk, ) for all compat/.xlat/%.lst
  build: hide commands run for kconfig
  build: rename $(AFLAGS) to $(XEN_AFLAGS)
  build: rename CFLAGS to XEN_CFLAGS in xen/Makefile
  build: avoid Config.mk's CFLAGS
  build: fix compile.h compiler version command line
  Config.mk: move $(cc-option, ) to config/compiler-testing.mk
  build: remove Config.mk include from Rules.mk

 Config.mk                   | 39 +------------------
 config/compiler-testing.mk  | 25 +++++++++++++
 xen/Makefile                | 75 +++++++++++++++++++++++++------------
 xen/Rules.mk                |  7 +++-
 xen/arch/arm/Makefile       |  2 +-
 xen/arch/arm/arch.mk        |  8 +++-
 xen/arch/riscv/Makefile     |  2 +-
 xen/arch/riscv/arch.mk      |  4 +-
 xen/arch/x86/Makefile       | 12 +++---
 xen/arch/x86/arch.mk        | 62 +++++++++++++++---------------
 xen/arch/x86/boot/Makefile  | 16 ++++++--
 xen/build.mk                | 24 +++++++-----
 xen/common/Makefile         |  8 ++--
 xen/include/Makefile        | 10 ++---
 xen/scripts/Kbuild.include  | 10 +++++
 xen/test/livepatch/Makefile |  4 +-
 xen/tools/kconfig/Makefile  | 14 +++----
 xen/xsm/flask/Makefile      |  9 ++++-
 18 files changed, 193 insertions(+), 138 deletions(-)
 create mode 100644 config/compiler-testing.mk

-- 
Anthony PERARD

[XEN PATCH 01/15] build: hide that we are updating xen/lib/x86

[Anthony PERARD]

Change of directory to xen/lib/x86 isn't needed to be shown. If
something gets updated, make will print the command line.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/include/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/Makefile b/xen/include/Makefile
index edd5322e88..96d5f6f3c8 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -229,7 +229,7 @@ ifeq ($(XEN_TARGET_ARCH),x86_64)
 .PHONY: lib-x86-all
 lib-x86-all:
 	@mkdir -p $(obj)/xen/lib/x86
-	$(MAKE) -C $(obj)/xen/lib/x86 -f $(abs_srctree)/$(src)/xen/lib/x86/Makefile all
+	$(Q)$(MAKE) -C $(obj)/xen/lib/x86 -f $(abs_srctree)/$(src)/xen/lib/x86/Makefile all
 
 all: lib-x86-all
 endif
-- 
Anthony PERARD

[XEN PATCH 02/15] build: rework asm-offsets.* build step to use kbuild

[Anthony PERARD]

Use $(if_changed_dep, ) macro to generate "asm-offsets.s" and remove
the use of $(move-if-changes,). That mean that "asm-offset.s" will be
changed even when the output doesn't change.

But "asm-offsets.s" is only used to generated "asm-offsets.h". So
instead of regenerating "asm-offsets.h" every time "asm-offsets.s"
change, we will use "$(filechk, )" to only update the ".h" when the
output change. Also, with "$(filechk, )", the file does get
regenerated when the rule change in the makefile.

This changes also result in a cleaner build log.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Instead of having a special $(cmd_asm-offsets.s) command, we could
probably reuse $(cmd_cc_s_c) from Rules.mk, but that would mean that
an hypothetical additional flags "-flto" in CFLAGS would not be
removed anymore, not sure if that matter here.

But then we could write this:

targets += arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s
arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s: CFLAGS-y += -g0
arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s FORCE

instead of having to write a rule for asm-offsets.s
---
 xen/build.mk | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/xen/build.mk b/xen/build.mk
index 758590c68e..e2a78aa806 100644
--- a/xen/build.mk
+++ b/xen/build.mk
@@ -40,13 +40,15 @@ include/xen/compile.h: include/xen/compile.h.in .banner FORCE
 
 targets += include/xen/compile.h
 
--include $(wildcard .asm-offsets.s.d)
-asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c
-	$(CC) $(call cpp_flags,$(c_flags)) -S -g0 -o $@.new -MQ $@ $<
-	$(call move-if-changed,$@.new,$@)
+quiet_cmd_asm-offsets.s = CC      $@
+cmd_asm-offsets.s = $(CC) $(call cpp_flags,$(c_flags)) -S -g0 $< -o $@
 
-arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: asm-offsets.s
-	@(set -e; \
+asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c FORCE
+	$(call if_changed_dep,asm-offsets.s)
+
+targets += asm-offsets.s
+
+define filechk_asm-offsets.h
 	  echo "/*"; \
 	  echo " * DO NOT MODIFY."; \
 	  echo " *"; \
@@ -57,9 +59,13 @@ arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: asm-offsets.s
 	  echo "#ifndef __ASM_OFFSETS_H__"; \
 	  echo "#define __ASM_OFFSETS_H__"; \
 	  echo ""; \
-	  sed -rne "/^[^#].*==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}"; \
+	  sed -rne "/^[^#].*==>/{s:.*==>(.*)<==.*:\1:; s: [\$$#]: :; p;}" $<; \
 	  echo ""; \
-	  echo "#endif") <$< >$@
+	  echo "#endif"
+endef
+
+arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: asm-offsets.s FORCE
+	$(call filechk,asm-offsets.h)
 
 build-dirs := $(patsubst %/built_in.o,%,$(filter %/built_in.o,$(ALL_OBJS) $(ALL_LIBS)))
 
-- 
Anthony PERARD

[XEN PATCH 03/15] build, x86: clean build log for boot/ targets

[Anthony PERARD]

We are adding %.lnk to .PRECIOUS or make treat them as intermediate
targets and remove them.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/arch/x86/boot/Makefile | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
index 03d8ce3a9e..2693b938bd 100644
--- a/xen/arch/x86/boot/Makefile
+++ b/xen/arch/x86/boot/Makefile
@@ -5,6 +5,8 @@ head-bin-objs := cmdline.o reloc.o
 nocov-y   += $(head-bin-objs)
 noubsan-y += $(head-bin-objs)
 targets   += $(head-bin-objs)
+targets   += $(head-bin-objs:.o=.bin)
+targets   += $(head-bin-objs:.o=.lnk)
 
 head-bin-objs := $(addprefix $(obj)/,$(head-bin-objs))
 
@@ -26,10 +28,16 @@ $(head-bin-objs): XEN_CFLAGS := $(CFLAGS_x86_32) -fpic
 LDFLAGS_DIRECT-$(call ld-option,--warn-rwx-segments) := --no-warn-rwx-segments
 LDFLAGS_DIRECT += $(LDFLAGS_DIRECT-y)
 
-%.bin: %.lnk
-	$(OBJCOPY) -j .text -O binary $< $@
+%.bin: OBJCOPYFLAGS := -j .text -O binary
+%.bin: %.lnk FORCE
+	$(call if_changed,objcopy)
 
-%.lnk: %.o $(src)/build32.lds
-	$(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter %.lds,$^) -o $@ $<
+quiet_cmd_ld_lnk_o = LD      $@
+cmd_ld_lnk_o = $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter %.lds,$^) -o $@ $<
+
+%.lnk: %.o $(src)/build32.lds FORCE
+	$(call if_changed,ld_lnk_o)
 
 clean-files := *.lnk *.bin
+
+.PRECIOUS: %.lnk
-- 
Anthony PERARD

[XEN PATCH 04/15] build: hide policy.bin commands

[Anthony PERARD]

Instead, show only when "policy.bin" is been updated.

We still have the full command from the flask/policy Makefile, but we
can't change that Makefile.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/xsm/flask/Makefile | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index 3fdcf7727e..fc44ad684f 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -48,10 +48,15 @@ targets += flask-policy.S
 FLASK_BUILD_DIR := $(abs_objtree)/$(obj)
 POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
 
+policy_chk = \
+    $(Q)if ! cmp -s $(POLICY_SRC) $@; then \
+        $(kecho) '  UPD     $@'; \
+        cp $(POLICY_SRC) $@; \
+    fi
 $(obj)/policy.bin: FORCE
-	$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
+	$(Q)$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
 	        -C $(XEN_ROOT)/tools/flask/policy \
 	        FLASK_BUILD_DIR=$(FLASK_BUILD_DIR) POLICY_FILENAME=$(POLICY_SRC)
-	cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@
+	$(call policy_chk)
 
 clean-files := policy.* $(POLICY_SRC)
-- 
Anthony PERARD

[XEN PATCH 05/15] build: introduce a generic command for gzip

[Anthony PERARD]

Make the gzip command generic and use -9 which wasn't use for
config.gz. (xen.gz does use -9)

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Rules.mk        | 5 +++++
 xen/common/Makefile | 8 ++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 59072ae8df..68b10ca5ef 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -63,6 +63,11 @@ cmd_objcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $< $@
 quiet_cmd_binfile = BINFILE $@
 cmd_binfile = $(SHELL) $(srctree)/tools/binfile $(BINFILE_FLAGS) $@ $(2)
 
+# gzip
+quiet_cmd_gzip = GZIP    $@
+cmd_gzip = \
+    cat $(real-prereqs) | gzip -n -f -9 > $@
+
 # Figure out what we need to build from the various variables
 # ===========================================================================
 
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 46049eac35..f45f19c391 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -78,13 +78,13 @@ obj-$(CONFIG_NEEDS_LIBELF) += libelf/
 obj-$(CONFIG_HAS_DEVICE_TREE) += libfdt/
 
 CONF_FILE := $(if $(patsubst /%,,$(KCONFIG_CONFIG)),$(objtree)/)$(KCONFIG_CONFIG)
-$(obj)/config.gz: $(CONF_FILE)
-	gzip -n -c $< >$@
+$(obj)/config.gz: $(CONF_FILE) FORCE
+	$(call if_changed,gzip)
+
+targets += config.gz
 
 $(obj)/config_data.o: $(obj)/config.gz
 
 $(obj)/config_data.S: $(srctree)/tools/binfile FORCE
 	$(call if_changed,binfile,$(obj)/config.gz xen_config_data)
 targets += config_data.S
-
-clean-files := config.gz
-- 
Anthony PERARD

[XEN PATCH 06/15] build: quiet for .allconfig.tmp target

[Anthony PERARD]

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/Makefile b/xen/Makefile
index e89fc461fc..27f70d2200 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -339,7 +339,7 @@ filechk_kconfig_allconfig = \
     :
 
 .allconfig.tmp: FORCE
-	set -e; { $(call filechk_kconfig_allconfig); } > $@
+	$(Q)set -e; { $(call filechk_kconfig_allconfig); } > $@
 
 config: tools_fixdep outputmakefile FORCE
 	$(Q)$(MAKE) $(build)=tools/kconfig $@
-- 
Anthony PERARD

[XEN PATCH 07/15] build: move XEN_HAS_BUILD_ID out of Config.mk

[Anthony PERARD]

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(-)

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)
-- 
Anthony PERARD

[XEN PATCH 08/15] build: use $(filechk, ) for all compat/.xlat/%.lst

[Anthony PERARD]

Make use of filechk mean that we don't have to use
$(move-if-changed,). It also mean that will have sometime "UPD .." in
the build output when the target changed, rather than having "GEN ..."
all the time when "xlat.lst" happen to have a more recent modification
timestamp.

While there, replace `grep -v` by `sed '//d'` to avoid an extra
fork and pipe when building.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/include/Makefile | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/xen/include/Makefile b/xen/include/Makefile
index 96d5f6f3c8..2e61b50139 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -93,15 +93,13 @@ targets += $(patsubst compat/%, compat/.xlat/%, $(headers-y))
 $(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/compat-xlat-header.py FORCE
 	$(call if_changed,xlat_headers)
 
-quiet_cmd_xlat_lst = GEN     $@
-cmd_xlat_lst = \
-	grep -v '^[[:blank:]]*$(pound)' $< | sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' >$@.new; \
-	$(call move-if-changed,$@.new,$@)
+filechk_xlat_lst = \
+	sed -ne '/^[[:blank:]]*$(pound)/d' -e 's,@arch@,$(compat-arch-y),g' -re 's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' $<
 
 .PRECIOUS: $(obj)/compat/.xlat/%.lst
 targets += $(patsubst compat/%.h, compat/.xlat/%.lst, $(headers-y))
 $(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst FORCE
-	$(call if_changed,xlat_lst)
+	$(call filechk,xlat_lst)
 
 xlat-y := $(shell sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,^[?!][[:blank:]]+[^[:blank:]]+[[:blank:]]+,,p' $(srcdir)/xlat.lst | uniq)
 xlat-y := $(filter $(patsubst compat/%,%,$(headers-y)),$(xlat-y))
-- 
Anthony PERARD

[XEN PATCH 09/15] build: hide commands run for kconfig

[Anthony PERARD]

but still show a log entry for syncconfig. We have to use kecho
instead of $(cmd,) to avoid issue with prompt from kconfig.

linux commits for reference:
    23cd88c91343 ("kbuild: hide commands to run Kconfig, and show short log for syncconfig")
    d952cfaf0cff ("kbuild: do not suppress Kconfig prompts for silent build")

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Makefile               |  1 +
 xen/tools/kconfig/Makefile | 14 +++++++-------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index 4dc960df2c..127c0e40b5 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -382,6 +382,7 @@ $(KCONFIG_CONFIG): tools_fixdep
 # This exploits the 'multi-target pattern rule' trick.
 # The syncconfig should be executed only once to make all the targets.
 include/config/%.conf include/config/%.conf.cmd: $(KCONFIG_CONFIG)
+	$(Q)$(kecho) "  SYNC    $@"
 	$(Q)$(MAKE) $(build)=tools/kconfig syncconfig
 
 ifeq ($(CONFIG_DEBUG),y)
diff --git a/xen/tools/kconfig/Makefile b/xen/tools/kconfig/Makefile
index b7b9a419ad..18c0f5d4f1 100644
--- a/xen/tools/kconfig/Makefile
+++ b/xen/tools/kconfig/Makefile
@@ -24,19 +24,19 @@ endif
 unexport CONFIG_
 
 xconfig: $(obj)/qconf
-	$< $(silent) $(Kconfig)
+	$(Q)$< $(silent) $(Kconfig)
 
 gconfig: $(obj)/gconf
-	$< $(silent) $(Kconfig)
+	$(Q)$< $(silent) $(Kconfig)
 
 menuconfig: $(obj)/mconf
-	$< $(silent) $(Kconfig)
+	$(Q)$< $(silent) $(Kconfig)
 
 config: $(obj)/conf
-	$< $(silent) --oldaskconfig $(Kconfig)
+	$(Q)$< $(silent) --oldaskconfig $(Kconfig)
 
 nconfig: $(obj)/nconf
-	$< $(silent) $(Kconfig)
+	$(Q)$< $(silent) $(Kconfig)
 
 build_menuconfig: $(obj)/mconf
 
@@ -70,12 +70,12 @@ simple-targets := oldconfig allnoconfig allyesconfig allmodconfig \
 PHONY += $(simple-targets)
 
 $(simple-targets): $(obj)/conf
-	$< $(silent) --$@ $(Kconfig)
+	$(Q)$< $(silent) --$@ $(Kconfig)
 
 PHONY += savedefconfig defconfig
 
 savedefconfig: $(obj)/conf
-	$< $(silent) --$@=defconfig $(Kconfig)
+	$(Q)$< $(silent) --$@=defconfig $(Kconfig)
 
 defconfig: $(obj)/conf
 ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/configs/$(KBUILD_DEFCONFIG)),)
-- 
Anthony PERARD

[XEN PATCH 10/15] build: rename $(AFLAGS) to $(XEN_AFLAGS)

[Anthony PERARD]

We don't want the AFLAGS from the environment, they are usually meant
to build user space application and not for the hypervisor.

Config.mk doesn't provied any $(AFLAGS) so we can start a fresh
$(XEN_AFLAGS).

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Makefile         | 10 ++++++----
 xen/arch/x86/arch.mk |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index 127c0e40b5..c4a83fca76 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -258,6 +258,8 @@ export KBUILD_DEFCONFIG := $(ARCH)_defconfig
 # reparsing Config.mk by e.g. arch/x86/boot/.
 export XEN_TREEWIDE_CFLAGS := $(CFLAGS)
 
+XEN_AFLAGS =
+
 # CLANG_FLAGS needs to be calculated before calling Kconfig
 ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
 CLANG_FLAGS :=
@@ -412,9 +414,9 @@ ifneq ($(CONFIG_CC_IS_CLANG),y)
 CFLAGS += -Wa,--strip-local-absolute
 endif
 
-AFLAGS += -D__ASSEMBLY__
+XEN_AFLAGS += -D__ASSEMBLY__
 
-$(call cc-option-add,AFLAGS,CC,-Wa$(comma)--noexecstack)
+$(call cc-option-add,XEN_AFLAGS,CC,-Wa$(comma)--noexecstack)
 
 LDFLAGS-$(call ld-option,--warn-rwx-segments) += --no-warn-rwx-segments
 
@@ -425,7 +427,7 @@ CFLAGS += $(EXTRA_CFLAGS_XEN_CORE)
 # Most CFLAGS are safe for assembly files:
 #  -std=gnu{89,99} gets confused by #-prefixed end-of-line comments
 #  -flto makes no sense and annoys clang
-AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS)) $(AFLAGS-y)
+XEN_AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS)) $(AFLAGS-y)
 
 # LDFLAGS are only passed directly to $(LD)
 LDFLAGS += $(LDFLAGS_DIRECT) $(LDFLAGS-y)
@@ -462,7 +464,7 @@ include $(srctree)/arch/$(TARGET_ARCH)/arch.mk
 
 # define new variables to avoid the ones defined in Config.mk
 export XEN_CFLAGS := $(CFLAGS)
-export XEN_AFLAGS := $(AFLAGS)
+export XEN_AFLAGS := $(XEN_AFLAGS)
 export XEN_LDFLAGS := $(LDFLAGS)
 export CFLAGS_UBSAN
 
diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
index 7b5be9fe46..13ec88a628 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -80,7 +80,7 @@ ifeq ($(CONFIG_LD_IS_GNU),y)
 AFLAGS-$(call ld-option,--print-output-format) += -DHAVE_LD_SORT_BY_INIT_PRIORITY
 else
 # Assume all versions of LLD support this.
-AFLAGS += -DHAVE_LD_SORT_BY_INIT_PRIORITY
+XEN_AFLAGS += -DHAVE_LD_SORT_BY_INIT_PRIORITY
 endif
 
 ifneq ($(CONFIG_PV_SHIM_EXCLUSIVE),y)
-- 
Anthony PERARD

[XEN PATCH 11/15] build: rename CFLAGS to XEN_CFLAGS in xen/Makefile

[Anthony PERARD]

This is a preparatory patch. A future patch will not even use
$(CFLAGS) to seed $(XEN_CFLAGS).

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Makefile           | 41 ++++++++++++++---------------
 xen/arch/arm/arch.mk   |  4 +--
 xen/arch/riscv/arch.mk |  4 +--
 xen/arch/x86/arch.mk   | 58 +++++++++++++++++++++---------------------
 4 files changed, 54 insertions(+), 53 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index c4a83fca76..b3bffe8c6f 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -259,6 +259,7 @@ export KBUILD_DEFCONFIG := $(ARCH)_defconfig
 export XEN_TREEWIDE_CFLAGS := $(CFLAGS)
 
 XEN_AFLAGS =
+XEN_CFLAGS = $(CFLAGS)
 
 # CLANG_FLAGS needs to be calculated before calling Kconfig
 ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
@@ -284,7 +285,7 @@ CLANG_FLAGS += $(call or,$(t1),$(t2),$(t3))
 endif
 
 CLANG_FLAGS += -Werror=unknown-warning-option
-CFLAGS += $(CLANG_FLAGS)
+XEN_CFLAGS += $(CLANG_FLAGS)
 export CLANG_FLAGS
 endif
 
@@ -293,7 +294,7 @@ ifeq ($(call ld-ver-build-id,$(LD)),n)
 XEN_LDFLAGS_BUILD_ID :=
 XEN_HAS_BUILD_ID := n
 else
-CFLAGS += -DBUILD_ID
+XEN_CFLAGS += -DBUILD_ID
 XEN_TREEWIDE_CFLAGS += -DBUILD_ID
 XEN_HAS_BUILD_ID := y
 XEN_LDFLAGS_BUILD_ID := --build-id=sha1
@@ -388,30 +389,30 @@ include/config/%.conf include/config/%.conf.cmd: $(KCONFIG_CONFIG)
 	$(Q)$(MAKE) $(build)=tools/kconfig syncconfig
 
 ifeq ($(CONFIG_DEBUG),y)
-CFLAGS += -O1
+XEN_CFLAGS += -O1
 else
-CFLAGS += -O2
+XEN_CFLAGS += -O2
 endif
 
 ifeq ($(CONFIG_FRAME_POINTER),y)
-CFLAGS += -fno-omit-frame-pointer
+XEN_CFLAGS += -fno-omit-frame-pointer
 else
-CFLAGS += -fomit-frame-pointer
+XEN_CFLAGS += -fomit-frame-pointer
 endif
 
 CFLAGS-$(CONFIG_CC_SPLIT_SECTIONS) += -ffunction-sections -fdata-sections
 
-CFLAGS += -nostdinc -fno-builtin -fno-common
-CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
-$(call cc-option-add,CFLAGS,CC,-Wvla)
-CFLAGS += -pipe -D__XEN__ -include $(srctree)/include/xen/config.h
+XEN_CFLAGS += -nostdinc -fno-builtin -fno-common
+XEN_CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
+$(call cc-option-add,XEN_CFLAGS,CC,-Wvla)
+XEN_CFLAGS += -pipe -D__XEN__ -include $(srctree)/include/xen/config.h
 CFLAGS-$(CONFIG_DEBUG_INFO) += -g
 
 ifneq ($(CONFIG_CC_IS_CLANG),y)
 # Clang doesn't understand this command line argument, and doesn't appear to
 # have a suitable alternative.  The resulting compiled binary does function,
 # but has an excessively large symbol table.
-CFLAGS += -Wa,--strip-local-absolute
+XEN_CFLAGS += -Wa,--strip-local-absolute
 endif
 
 XEN_AFLAGS += -D__ASSEMBLY__
@@ -420,14 +421,14 @@ $(call cc-option-add,XEN_AFLAGS,CC,-Wa$(comma)--noexecstack)
 
 LDFLAGS-$(call ld-option,--warn-rwx-segments) += --no-warn-rwx-segments
 
-CFLAGS += $(CFLAGS-y)
+XEN_CFLAGS += $(CFLAGS-y)
 # allow extra CFLAGS externally via EXTRA_CFLAGS_XEN_CORE
-CFLAGS += $(EXTRA_CFLAGS_XEN_CORE)
+XEN_CFLAGS += $(EXTRA_CFLAGS_XEN_CORE)
 
 # Most CFLAGS are safe for assembly files:
 #  -std=gnu{89,99} gets confused by #-prefixed end-of-line comments
 #  -flto makes no sense and annoys clang
-XEN_AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS)) $(AFLAGS-y)
+XEN_AFLAGS += $(filter-out -std=gnu% -flto,$(XEN_CFLAGS)) $(AFLAGS-y)
 
 # LDFLAGS are only passed directly to $(LD)
 LDFLAGS += $(LDFLAGS_DIRECT) $(LDFLAGS-y)
@@ -439,16 +440,16 @@ CFLAGS_UBSAN :=
 endif
 
 ifeq ($(CONFIG_LTO),y)
-CFLAGS += -flto
+XEN_CFLAGS += -flto
 LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so
 endif
 
 ifdef building_out_of_srctree
-    CFLAGS += -I$(objtree)/include
-    CFLAGS += -I$(objtree)/arch/$(TARGET_ARCH)/include
+    XEN_CFLAGS += -I$(objtree)/include
+    XEN_CFLAGS += -I$(objtree)/arch/$(TARGET_ARCH)/include
 endif
-CFLAGS += -I$(srctree)/include
-CFLAGS += -I$(srctree)/arch/$(TARGET_ARCH)/include
+XEN_CFLAGS += -I$(srctree)/include
+XEN_CFLAGS += -I$(srctree)/arch/$(TARGET_ARCH)/include
 
 # Note that link order matters!
 ALL_OBJS-y                := common/built_in.o
@@ -463,7 +464,7 @@ ALL_LIBS-y                := lib/lib.a
 include $(srctree)/arch/$(TARGET_ARCH)/arch.mk
 
 # define new variables to avoid the ones defined in Config.mk
-export XEN_CFLAGS := $(CFLAGS)
+export XEN_CFLAGS := $(XEN_CFLAGS)
 export XEN_AFLAGS := $(XEN_AFLAGS)
 export XEN_LDFLAGS := $(LDFLAGS)
 export CFLAGS_UBSAN
diff --git a/xen/arch/arm/arch.mk b/xen/arch/arm/arch.mk
index 58db76c4e1..300b8bf7ae 100644
--- a/xen/arch/arm/arch.mk
+++ b/xen/arch/arm/arch.mk
@@ -1,8 +1,8 @@
 ########################################
 # arm-specific definitions
 
-$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
-$(call cc-option-add,CFLAGS,CC,-Wnested-externs)
+$(call cc-options-add,XEN_CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
+$(call cc-option-add,XEN_CFLAGS,CC,-Wnested-externs)
 
 # Prevent floating-point variables from creeping into Xen.
 CFLAGS-$(CONFIG_ARM_32) += -msoft-float
diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
index 7448f759b4..aadf373ce8 100644
--- a/xen/arch/riscv/arch.mk
+++ b/xen/arch/riscv/arch.mk
@@ -1,7 +1,7 @@
 ########################################
 # RISCV-specific definitions
 
-$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
+$(call cc-options-add,XEN_CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 
 CFLAGS-$(CONFIG_RISCV_64) += -mabi=lp64
 
@@ -12,7 +12,7 @@ riscv-march-$(CONFIG_RISCV_ISA_C)       := $(riscv-march-y)c
 # into the upper half _or_ the lower half of the address space.
 # -mcmodel=medlow would force Xen into the lower half.
 
-CFLAGS += -march=$(riscv-march-y) -mstrict-align -mcmodel=medany
+XEN_CFLAGS += -march=$(riscv-march-y) -mstrict-align -mcmodel=medany
 
 # TODO: Drop override when more of the build is working
 override ALL_OBJS-y = arch/$(TARGET_ARCH)/built_in.o
diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
index 13ec88a628..5df3cf6bc3 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -3,42 +3,42 @@
 
 export XEN_IMG_OFFSET := 0x200000
 
-CFLAGS += -I$(srctree)/arch/x86/include/asm/mach-generic
-CFLAGS += -I$(srctree)/arch/x86/include/asm/mach-default
-CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
+XEN_CFLAGS += -I$(srctree)/arch/x86/include/asm/mach-generic
+XEN_CFLAGS += -I$(srctree)/arch/x86/include/asm/mach-default
+XEN_CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
 
 # Prevent floating-point variables from creeping into Xen.
-CFLAGS += -msoft-float
-
-$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
-$(call cc-option-add,CFLAGS,CC,-Wnested-externs)
-$(call as-option-add,CFLAGS,CC,"vmcall",-DHAVE_AS_VMX)
-$(call as-option-add,CFLAGS,CC,"crc32 %eax$(comma)%eax",-DHAVE_AS_SSE4_2)
-$(call as-option-add,CFLAGS,CC,"invept (%rax)$(comma)%rax",-DHAVE_AS_EPT)
-$(call as-option-add,CFLAGS,CC,"rdrand %eax",-DHAVE_AS_RDRAND)
-$(call as-option-add,CFLAGS,CC,"rdfsbase %rax",-DHAVE_AS_FSGSBASE)
-$(call as-option-add,CFLAGS,CC,"xsaveopt (%rax)",-DHAVE_AS_XSAVEOPT)
-$(call as-option-add,CFLAGS,CC,"rdseed %eax",-DHAVE_AS_RDSEED)
-$(call as-option-add,CFLAGS,CC,"clac",-DHAVE_AS_CLAC_STAC)
-$(call as-option-add,CFLAGS,CC,"clwb (%rax)",-DHAVE_AS_CLWB)
-$(call as-option-add,CFLAGS,CC,".equ \"x\"$(comma)1",-DHAVE_AS_QUOTED_SYM)
-$(call as-option-add,CFLAGS,CC,"invpcid (%rax)$(comma)%rax",-DHAVE_AS_INVPCID)
-$(call as-option-add,CFLAGS,CC,"movdiri %rax$(comma)(%rax)",-DHAVE_AS_MOVDIR)
-$(call as-option-add,CFLAGS,CC,"enqcmd (%rax)$(comma)%rax",-DHAVE_AS_ENQCMD)
+XEN_CFLAGS += -msoft-float
+
+$(call cc-options-add,XEN_CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
+$(call cc-option-add,XEN_CFLAGS,CC,-Wnested-externs)
+$(call as-option-add,XEN_CFLAGS,CC,"vmcall",-DHAVE_AS_VMX)
+$(call as-option-add,XEN_CFLAGS,CC,"crc32 %eax$(comma)%eax",-DHAVE_AS_SSE4_2)
+$(call as-option-add,XEN_CFLAGS,CC,"invept (%rax)$(comma)%rax",-DHAVE_AS_EPT)
+$(call as-option-add,XEN_CFLAGS,CC,"rdrand %eax",-DHAVE_AS_RDRAND)
+$(call as-option-add,XEN_CFLAGS,CC,"rdfsbase %rax",-DHAVE_AS_FSGSBASE)
+$(call as-option-add,XEN_CFLAGS,CC,"xsaveopt (%rax)",-DHAVE_AS_XSAVEOPT)
+$(call as-option-add,XEN_CFLAGS,CC,"rdseed %eax",-DHAVE_AS_RDSEED)
+$(call as-option-add,XEN_CFLAGS,CC,"clac",-DHAVE_AS_CLAC_STAC)
+$(call as-option-add,XEN_CFLAGS,CC,"clwb (%rax)",-DHAVE_AS_CLWB)
+$(call as-option-add,XEN_CFLAGS,CC,".equ \"x\"$(comma)1",-DHAVE_AS_QUOTED_SYM)
+$(call as-option-add,XEN_CFLAGS,CC,"invpcid (%rax)$(comma)%rax",-DHAVE_AS_INVPCID)
+$(call as-option-add,XEN_CFLAGS,CC,"movdiri %rax$(comma)(%rax)",-DHAVE_AS_MOVDIR)
+$(call as-option-add,XEN_CFLAGS,CC,"enqcmd (%rax)$(comma)%rax",-DHAVE_AS_ENQCMD)
 
 # GAS's idea of true is -1.  Clang's idea is 1
-$(call as-option-add,CFLAGS,CC,\
+$(call as-option-add,XEN_CFLAGS,CC,\
     ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE)
 
 # Check to see whether the assmbler supports the .nop directive.
-$(call as-option-add,CFLAGS,CC,\
+$(call as-option-add,XEN_CFLAGS,CC,\
     ".L1: .L2: .nops (.L2 - .L1)$(comma)9",-DHAVE_AS_NOPS_DIRECTIVE)
 
-CFLAGS += -mno-red-zone -fpic
+XEN_CFLAGS += -mno-red-zone -fpic
 
 # Xen doesn't use MMX or SSE interally.  If the compiler supports it, also skip
 # the SSE setup for variadic function calls.
-CFLAGS += -mno-mmx -mno-sse $(call cc-option,$(CC),-mskip-rax-setup)
+XEN_CFLAGS += -mno-mmx -mno-sse $(call cc-option,$(CC),-mskip-rax-setup)
 
 ifeq ($(CONFIG_INDIRECT_THUNK),y)
 # Compile with gcc thunk-extern, indirect-branch-register if available.
@@ -54,10 +54,10 @@ ifdef CONFIG_XEN_IBT
 # Force -fno-jump-tables to work around
 #   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104816
 #   https://github.com/llvm/llvm-project/issues/54247
-CFLAGS += -fcf-protection=branch -mmanual-endbr -fno-jump-tables
-$(call cc-option-add,CFLAGS,CC,-fcf-check-attribute=no)
+XEN_CFLAGS += -fcf-protection=branch -mmanual-endbr -fno-jump-tables
+$(call cc-option-add,XEN_CFLAGS,CC,-fcf-check-attribute=no)
 else
-$(call cc-option-add,CFLAGS,CC,-fcf-protection=none)
+$(call cc-option-add,XEN_CFLAGS,CC,-fcf-protection=none)
 endif
 
 # If supported by the compiler, reduce stack alignment to 8 bytes. But allow
@@ -91,7 +91,7 @@ efi-check := arch/x86/efi/check
 $(shell mkdir -p $(dir $(efi-check)))
 
 # Check if the compiler supports the MS ABI.
-XEN_BUILD_EFI := $(call if-success,$(CC) $(CFLAGS) -c $(srctree)/$(efi-check).c -o $(efi-check).o,y)
+XEN_BUILD_EFI := $(call if-success,$(CC) $(XEN_CFLAGS) -c $(srctree)/$(efi-check).c -o $(efi-check).o,y)
 
 # Check if the linker supports PE.
 EFI_LDFLAGS := $(patsubst -m%,-mi386pep,$(LDFLAGS)) --subsystem=10
@@ -129,4 +129,4 @@ export EFI_LDFLAGS
 endif
 
 # Set up the assembler include path properly for older toolchains.
-CFLAGS += -Wa,-I$(objtree)/include -Wa,-I$(srctree)/include
+XEN_CFLAGS += -Wa,-I$(objtree)/include -Wa,-I$(srctree)/include
-- 
Anthony PERARD

[XEN PATCH 12/15] build: avoid Config.mk's CFLAGS

[Anthony PERARD]

The variable $(CFLAGS) is too often set in the environment,
especially when building a package for a distribution. Often, those
CFLAGS are intended to be use to build user spaces binaries, not a
kernel. This mean packager needs to takes extra steps to build Xen by
overriding the CFLAGS provided by the package build environment.

With this patch, we avoid using the variable $(CFLAGS). Also, the
hypervisor's build system have complete control over which CFLAGS are
used.

No change intended to XEN_CFLAGS used, beside some flags which may be
in a different order on the command line.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    There's still $(EXTRA_CFLAGS_XEN_CORE) which allows to add more CFLAGS
    if someone building Xen needs to add more CFLAGS to the hypervisor
    build.

 xen/Makefile         | 11 ++++++++++-
 xen/arch/arm/arch.mk |  4 ++++
 xen/arch/x86/arch.mk |  2 ++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/xen/Makefile b/xen/Makefile
index b3bffe8c6f..4dc3acf2a6 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -259,7 +259,16 @@ export KBUILD_DEFCONFIG := $(ARCH)_defconfig
 export XEN_TREEWIDE_CFLAGS := $(CFLAGS)
 
 XEN_AFLAGS =
-XEN_CFLAGS = $(CFLAGS)
+XEN_CFLAGS =
+ifeq ($(XEN_OS),SunOS)
+    XEN_CFLAGS +=  -Wa,--divide -D_POSIX_C_SOURCE=200112L -D__EXTENSIONS__
+endif
+XEN_CFLAGS += -fno-strict-aliasing
+XEN_CFLAGS += -std=gnu99
+XEN_CFLAGS += -Wall -Wstrict-prototypes
+$(call cc-option-add,XEN_CFLAGS,CC,-Wdeclaration-after-statement)
+$(call cc-option-add,XEN_CFLAGS,CC,-Wno-unused-but-set-variable)
+$(call cc-option-add,XEN_CFLAGS,CC,-Wno-unused-local-typedefs)
 
 # CLANG_FLAGS needs to be calculated before calling Kconfig
 ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
diff --git a/xen/arch/arm/arch.mk b/xen/arch/arm/arch.mk
index 300b8bf7ae..0478fadde2 100644
--- a/xen/arch/arm/arch.mk
+++ b/xen/arch/arm/arch.mk
@@ -1,6 +1,10 @@
 ########################################
 # arm-specific definitions
 
+ifeq ($(XEN_TARGET_ARCH),arm32)
+    XEN_CFLAGS += -marm
+endif
+
 $(call cc-options-add,XEN_CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 $(call cc-option-add,XEN_CFLAGS,CC,-Wnested-externs)
 
diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
index 5df3cf6bc3..fc3b1dc922 100644
--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -1,6 +1,8 @@
 ########################################
 # x86-specific definitions
 
+XEN_CFLAGS += -m64
+
 export XEN_IMG_OFFSET := 0x200000
 
 XEN_CFLAGS += -I$(srctree)/arch/x86/include/asm/mach-generic
-- 
Anthony PERARD

[XEN PATCH 13/15] build: fix compile.h compiler version command line

[Anthony PERARD]

CFLAGS is just from Config.mk, instead use the flags used to build
Xen.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    I don't know if CFLAGS is even useful there, just --version without the
    flags might produce the same result.

 xen/build.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/build.mk b/xen/build.mk
index e2a78aa806..d468bb6e26 100644
--- a/xen/build.mk
+++ b/xen/build.mk
@@ -23,7 +23,7 @@ define cmd_compile.h
 	    -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
 	    -e 's/@@domain@@/$(XEN_DOMAIN)/g' \
 	    -e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \
-	    -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' \
+	    -e 's!@@compiler@@!$(shell $(CC) $(XEN_CFLAGS) --version 2>&1 | head -1)!g' \
 	    -e 's/@@version@@/$(XEN_VERSION)/g' \
 	    -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \
 	    -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \
-- 
Anthony PERARD

[XEN PATCH 14/15] Config.mk: move $(cc-option, ) to config/compiler-testing.mk

[Anthony PERARD]

In xen/, it isn't necessary to include Config.mk in every Makefile in
subdirectories as nearly all necessary variables should be calculated
in xen/Makefile. But some Makefile make use of the macro $(cc-option,)
that is only available in Config.mk.

Extract $(cc-option,) from Config.mk so we can use it without
including Config.mk and thus without having to recalculate some CFLAGS
which would be ignored.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 Config.mk                  | 27 +--------------------------
 config/compiler-testing.mk | 25 +++++++++++++++++++++++++
 xen/Rules.mk               |  1 +
 3 files changed, 27 insertions(+), 26 deletions(-)
 create mode 100644 config/compiler-testing.mk

diff --git a/Config.mk b/Config.mk
index 27f48f654a..ebc8d0dfdd 100644
--- a/Config.mk
+++ b/Config.mk
@@ -18,6 +18,7 @@ realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir $(file)) && echo "
 or       = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(3),$(if $(strip $(4)),$(4)))))
 
 -include $(XEN_ROOT)/.config
+include $(XEN_ROOT)/config/compiler-testing.mk
 
 XEN_COMPILE_ARCH    ?= $(shell uname -m | sed -e s/i.86/x86_32/ \
                          -e s/i86pc/x86_32/ -e s/amd64/x86_64/ \
@@ -79,32 +80,6 @@ PYTHON_PREFIX_ARG ?= --prefix="$(prefix)"
 # to permit the user to set PYTHON_PREFIX_ARG to '' to workaround this bug:
 #  https://bugs.launchpad.net/ubuntu/+bug/362570
 
-# cc-option: Check if compiler supports first option, else fall back to second.
-#
-# This is complicated by the fact that unrecognised -Wno-* options:
-#   (a) are ignored unless the compilation emits a warning; and
-#   (b) even then produce a warning rather than an error
-# To handle this we do a test compile, passing the option-under-test, on a code
-# fragment that will always produce a warning (integer assigned to pointer).
-# We then grep for the option-under-test in the compiler's output, the presence
-# of which would indicate an "unrecognized command-line option" warning/error.
-#
-# Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586)
-cc-option = $(shell if test -z "`echo 'void*p=1;' | \
-              $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2:-Wa$(comma)%=%) -`"; \
-              then echo "$(2)"; else echo "$(3)"; fi ;)
-
-# cc-option-add: Add an option to compilation flags, but only if supported.
-# Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6)
-cc-option-add = $(eval $(call cc-option-add-closure,$(1),$(2),$(3)))
-define cc-option-add-closure
-    ifneq ($$(call cc-option,$$($(2)),$(3),n),n)
-        $(1) += $(3)
-    endif
-endef
-
-cc-options-add = $(foreach o,$(3),$(call cc-option-add,$(1),$(2),$(o)))
-
 # cc-ver: Check compiler against the version requirement. Return boolean 'y'/'n'.
 # Usage: ifeq ($(call cc-ver,$(CC),ge,0x030400),y)
 cc-ver = $(shell if [ $$((`$(1) -dumpversion | awk -F. \
diff --git a/config/compiler-testing.mk b/config/compiler-testing.mk
new file mode 100644
index 0000000000..9677f91abe
--- /dev/null
+++ b/config/compiler-testing.mk
@@ -0,0 +1,25 @@
+# cc-option: Check if compiler supports first option, else fall back to second.
+#
+# This is complicated by the fact that unrecognised -Wno-* options:
+#   (a) are ignored unless the compilation emits a warning; and
+#   (b) even then produce a warning rather than an error
+# To handle this we do a test compile, passing the option-under-test, on a code
+# fragment that will always produce a warning (integer assigned to pointer).
+# We then grep for the option-under-test in the compiler's output, the presence
+# of which would indicate an "unrecognized command-line option" warning/error.
+#
+# Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586)
+cc-option = $(shell if test -z "`echo 'void*p=1;' | \
+              $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2:-Wa$(comma)%=%) -`"; \
+              then echo "$(2)"; else echo "$(3)"; fi ;)
+
+# cc-option-add: Add an option to compilation flags, but only if supported.
+# Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6)
+cc-option-add = $(eval $(call cc-option-add-closure,$(1),$(2),$(3)))
+define cc-option-add-closure
+    ifneq ($$(call cc-option,$$($(2)),$(3),n),n)
+        $(1) += $(3)
+    endif
+endef
+
+cc-options-add = $(foreach o,$(3),$(call cc-option-add,$(1),$(2),$(o)))
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 68b10ca5ef..8177d405c3 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -19,6 +19,7 @@ __build:
 
 include $(XEN_ROOT)/Config.mk
 include $(srctree)/scripts/Kbuild.include
+include $(XEN_ROOT)/config/compiler-testing.mk
 
 # Initialise some variables
 obj-y :=
-- 
Anthony PERARD

[XEN PATCH 15/15] build: remove Config.mk include from Rules.mk

[Anthony PERARD]

Everything needed to build the hypervisor should already be configured
by "xen/Makefile", thus Config.mk shouldn't be needed.

Then, Config.mk keeps on testing support of some CFLAGS with CC, the
result of this testing is not used at this stage so the build is
slowed unnecessarily.

Likewise, GCC is checked to be at the minimum at 4.2 when entering
every sub-directory, so the check have run countless time at this
stage.

We only need to export a few more configuration variables. And add
some variables in Kbuild.include, and macro fallbacks for Make older
than 3.81. (Adding `or` just in case. it's only used in xen/Makefile,
which includes Config.mk and so has already the fallback.)

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    Removing Config.mk benefit:
        Rebuild on my computer is aboud 1 second faster overall.
        Save maybe 3 seconds of user time
        system less loaded

 xen/Makefile               | 4 ++++
 xen/Rules.mk               | 1 -
 xen/scripts/Kbuild.include | 7 +++++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/xen/Makefile b/xen/Makefile
index 4dc3acf2a6..9af7223c66 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -246,10 +246,14 @@ export TARGET_ARCH     := $(shell echo $(XEN_TARGET_ARCH) | \
                             sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \
                                 -e s'/riscv.*/riscv/g')
 
+export XEN_COMPILE_ARCH XEN_TARGET_ARCH
 export CONFIG_SHELL := $(SHELL)
 export CC CXX LD NM OBJCOPY OBJDUMP ADDR2LINE
+export CPP AR
 export YACC = $(if $(BISON),$(BISON),bison)
 export LEX = $(if $(FLEX),$(FLEX),flex)
+export HOSTCC HOSTCXX HOSTCFLAGS
+export EMBEDDED_EXTRA_CFLAGS LDFLAGS_DIRECT
 
 # Default file for 'make defconfig'.
 export KBUILD_DEFCONFIG := $(ARCH)_defconfig
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 8177d405c3..8291e0a573 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -17,7 +17,6 @@ __build:
 
 -include $(objtree)/include/config/auto.conf
 
-include $(XEN_ROOT)/Config.mk
 include $(srctree)/scripts/Kbuild.include
 include $(XEN_ROOT)/config/compiler-testing.mk
 
diff --git a/xen/scripts/Kbuild.include b/xen/scripts/Kbuild.include
index d820595e2f..dfa66f2c8a 100644
--- a/xen/scripts/Kbuild.include
+++ b/xen/scripts/Kbuild.include
@@ -8,6 +8,13 @@ empty   :=
 space   := $(empty) $(empty)
 space_escape := _-_SPACE_-_
 pound   := \#
+comma   := ,
+open    := (
+close   := )
+
+# fallbacks for GNU Make older than 3.81
+realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir $(file)) && echo "$$PWD/$(notdir $(file))")))
+or       = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(3),$(if $(strip $(4)),$(4)))))
 
 ###
 # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
-- 
Anthony PERARD

Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line

[Andrew Cooper]

On 23/05/2023 5:38 pm, Anthony PERARD wrote:
> CFLAGS is just from Config.mk, instead use the flags used to build
> Xen.
>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>
> Notes:
>     I don't know if CFLAGS is even useful there, just --version without the
>     flags might produce the same result.

I can't think of any legitimate reason for CFLAGS to be here.

Any compiler which does differ its output based on CLFAGS is probably
one we don't want to be using...

~Andrew

Re: [XEN PATCH 01/15] build: hide that we are updating xen/lib/x86

[Luca Fancellu]

> On 23 May 2023, at 17:37, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> Change of directory to xen/lib/x86 isn't needed to be shown. If
> something gets updated, make will print the command line.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Hi Anthony,

Looks good to me

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>


> ---
> xen/include/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/include/Makefile b/xen/include/Makefile
> index edd5322e88..96d5f6f3c8 100644
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -229,7 +229,7 @@ ifeq ($(XEN_TARGET_ARCH),x86_64)
> .PHONY: lib-x86-all
> lib-x86-all:
> @mkdir -p $(obj)/xen/lib/x86
> - $(MAKE) -C $(obj)/xen/lib/x86 -f $(abs_srctree)/$(src)/xen/lib/x86/Makefile all
> + $(Q)$(MAKE) -C $(obj)/xen/lib/x86 -f $(abs_srctree)/$(src)/xen/lib/x86/Makefile all
> 
> all: lib-x86-all
> endif
> -- 
> Anthony PERARD
> 
> 

Re: [XEN PATCH 01/15] build: hide that we are updating xen/lib/x86

[Jan Beulich]

On 23.05.2023 18:37, Anthony PERARD wrote:
> Change of directory to xen/lib/x86 isn't needed to be shown. If
> something gets updated, make will print the command line.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [XEN PATCH 04/15] build: hide policy.bin commands

[Jan Beulich]

On 23.05.2023 18:38, Anthony PERARD wrote:
> --- a/xen/xsm/flask/Makefile
> +++ b/xen/xsm/flask/Makefile
> @@ -48,10 +48,15 @@ targets += flask-policy.S
>  FLASK_BUILD_DIR := $(abs_objtree)/$(obj)
>  POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
>  
> +policy_chk = \
> +    $(Q)if ! cmp -s $(POLICY_SRC) $@; then \
> +        $(kecho) '  UPD     $@'; \
> +        cp $(POLICY_SRC) $@; \

Wouldn't this better use move-if-changed? Which, if "UPD ..." output is
desired, would then need overriding from what Config.mk supplies?

In any event, much like move-if-changed itself - please avoid underscores
in names when dashes are fine to use.

> +    fi
>  $(obj)/policy.bin: FORCE

Nit: Blank line above here please.

Jan

> -	$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
> +	$(Q)$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
>  	        -C $(XEN_ROOT)/tools/flask/policy \
>  	        FLASK_BUILD_DIR=$(FLASK_BUILD_DIR) POLICY_FILENAME=$(POLICY_SRC)
> -	cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@
> +	$(call policy_chk)
>  
>  clean-files := policy.* $(POLICY_SRC)

Re: [XEN PATCH 05/15] build: introduce a generic command for gzip

[Jan Beulich]

On 23.05.2023 18:38, Anthony PERARD wrote:
> Make the gzip command generic and use -9 which wasn't use for
> config.gz. (xen.gz does use -9)

You mention xen.gz here, but you don't make its rule use this new
construct. Is that intentional (and if so, why)? (There we also go
through $@.new, and being consistent in that regard would imo be
desirable as well.)

Jan

> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  xen/Rules.mk        | 5 +++++
>  xen/common/Makefile | 8 ++++----
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index 59072ae8df..68b10ca5ef 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -63,6 +63,11 @@ cmd_objcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $< $@
>  quiet_cmd_binfile = BINFILE $@
>  cmd_binfile = $(SHELL) $(srctree)/tools/binfile $(BINFILE_FLAGS) $@ $(2)
>  
> +# gzip
> +quiet_cmd_gzip = GZIP    $@
> +cmd_gzip = \
> +    cat $(real-prereqs) | gzip -n -f -9 > $@
> +
>  # Figure out what we need to build from the various variables
>  # ===========================================================================
>  
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index 46049eac35..f45f19c391 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -78,13 +78,13 @@ obj-$(CONFIG_NEEDS_LIBELF) += libelf/
>  obj-$(CONFIG_HAS_DEVICE_TREE) += libfdt/
>  
>  CONF_FILE := $(if $(patsubst /%,,$(KCONFIG_CONFIG)),$(objtree)/)$(KCONFIG_CONFIG)
> -$(obj)/config.gz: $(CONF_FILE)
> -	gzip -n -c $< >$@
> +$(obj)/config.gz: $(CONF_FILE) FORCE
> +	$(call if_changed,gzip)
> +
> +targets += config.gz
>  
>  $(obj)/config_data.o: $(obj)/config.gz
>  
>  $(obj)/config_data.S: $(srctree)/tools/binfile FORCE
>  	$(call if_changed,binfile,$(obj)/config.gz xen_config_data)
>  targets += config_data.S
> -
> -clean-files := config.gz

Re: [XEN PATCH 06/15] build: quiet for .allconfig.tmp target

[Jan Beulich]

On 23.05.2023 18:38, Anthony PERARD wrote:
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -339,7 +339,7 @@ filechk_kconfig_allconfig = \
>      :
>  
>  .allconfig.tmp: FORCE
> -	set -e; { $(call filechk_kconfig_allconfig); } > $@
> +	$(Q)set -e; { $(call filechk_kconfig_allconfig); } > $@

So this then leaves no trace of the generation of this file. This might
be okay, if only the file wasn't needlessly generated for e.g. "make
syncconfig". So I'd be okay with this complete silencing only if prior
to that (or in the same patch) the dependencies were limited to just
the *config goals which actually need the file (and the setting of
KCONFIG_ALLCONFIG).

Jan

Re: [XEN PATCH 02/15] build: rework asm-offsets.* build step to use kbuild

[Luca Fancellu]

> On 23 May 2023, at 17:37, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> Use $(if_changed_dep, ) macro to generate "asm-offsets.s" and remove
> the use of $(move-if-changes,). That mean that "asm-offset.s" will be
> changed even when the output doesn't change.
> 
> But "asm-offsets.s" is only used to generated "asm-offsets.h". So
> instead of regenerating "asm-offsets.h" every time "asm-offsets.s"
> change, we will use "$(filechk, )" to only update the ".h" when the
> output change. Also, with "$(filechk, )", the file does get
> regenerated when the rule change in the makefile.
> 
> This changes also result in a cleaner build log.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> 
> Instead of having a special $(cmd_asm-offsets.s) command, we could
> probably reuse $(cmd_cc_s_c) from Rules.mk, but that would mean that
> an hypothetical additional flags "-flto" in CFLAGS would not be
> removed anymore, not sure if that matter here.
> 
> But then we could write this:
> 
> targets += arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s
> arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s: CFLAGS-y += -g0
> arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s FORCE
> 
> instead of having to write a rule for asm-offsets.s

The solution above seems clean, maybe I am wrong but -flto should not matter here as we are
not building objects to include in the final build, isn’t it? And gcc documentation states just:

“It is recommended that you compile all the files participating in the same link with the same
options and also specify those options at link time."

I’ve also tested this patch and it works fine, I have to say however that I preferred
a more verbose output, so that people can check how we are invoking the compiler,
but I guess now it’s more consistent with the other invocations that doesn’t print
the compiler invocation.

So if you want to proceed with this one, for me looks fine:

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>

Re: [XEN PATCH 02/15] build: rework asm-offsets.* build step to use kbuild

[Jan Beulich]

On 24.05.2023 09:39, Luca Fancellu wrote:
>> On 23 May 2023, at 17:37, Anthony PERARD <anthony.perard@citrix.com> wrote:
>> Instead of having a special $(cmd_asm-offsets.s) command, we could
>> probably reuse $(cmd_cc_s_c) from Rules.mk, but that would mean that
>> an hypothetical additional flags "-flto" in CFLAGS would not be
>> removed anymore, not sure if that matter here.
>>
>> But then we could write this:
>>
>> targets += arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s
>> arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s: CFLAGS-y += -g0
>> arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s FORCE
>>
>> instead of having to write a rule for asm-offsets.s
> 
> The solution above seems clean, maybe I am wrong but -flto should not matter here as we are
> not building objects to include in the final build, isn’t it? And gcc documentation states just:
> 
> “It is recommended that you compile all the files participating in the same link with the same
> options and also specify those options at link time."
> 
> I’ve also tested this patch and it works fine, I have to say however that I preferred
> a more verbose output, so that people can check how we are invoking the compiler,
> but I guess now it’s more consistent with the other invocations that doesn’t print
> the compiler invocation.

If you want it more verbose, you can pass V=1 on the make command line.
(Of course that'll affect all commands' output.)

Jan

Re: [XEN PATCH 07/15] build: move XEN_HAS_BUILD_ID out of Config.mk

[Luca Fancellu]

> 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>

Re: [XEN PATCH 08/15] build: use $(filechk, ) for all compat/.xlat/%.lst

[Luca Fancellu]

> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> Make use of filechk mean that we don't have to use
> $(move-if-changed,). It also mean that will have sometime "UPD .." in
> the build output when the target changed, rather than having "GEN ..."
> all the time when "xlat.lst" happen to have a more recent modification
> timestamp.
> 
> While there, replace `grep -v` by `sed '//d'` to avoid an extra
> fork and pipe when building.
> 
> 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>

Re: [XEN PATCH 02/15] build: rework asm-offsets.* build step to use kbuild

[Luca Fancellu]

> On 24 May 2023, at 09:01, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 24.05.2023 09:39, Luca Fancellu wrote:
>>> On 23 May 2023, at 17:37, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>> Instead of having a special $(cmd_asm-offsets.s) command, we could
>>> probably reuse $(cmd_cc_s_c) from Rules.mk, but that would mean that
>>> an hypothetical additional flags "-flto" in CFLAGS would not be
>>> removed anymore, not sure if that matter here.
>>> 
>>> But then we could write this:
>>> 
>>> targets += arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s
>>> arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s: CFLAGS-y += -g0
>>> arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s FORCE
>>> 
>>> instead of having to write a rule for asm-offsets.s
>> 
>> The solution above seems clean, maybe I am wrong but -flto should not matter here as we are
>> not building objects to include in the final build, isn’t it? And gcc documentation states just:
>> 
>> “It is recommended that you compile all the files participating in the same link with the same
>> options and also specify those options at link time."
>> 
>> I’ve also tested this patch and it works fine, I have to say however that I preferred
>> a more verbose output, so that people can check how we are invoking the compiler,
>> but I guess now it’s more consistent with the other invocations that doesn’t print
>> the compiler invocation.
> 
> If you want it more verbose, you can pass V=1 on the make command line.
> (Of course that'll affect all commands' output.)

Yes I have to say that after sending the mail, I’ve checked the Makefile and I’ve found the comment

# To put more focus on warnings, be less verbose as default
# Use 'make V=1' to see the full commands

Thank you for pointing that out


> 
> Jan

Re: [XEN PATCH 09/15] build: hide commands run for kconfig

[Luca Fancellu]

> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> but still show a log entry for syncconfig. We have to use kecho
> instead of $(cmd,) to avoid issue with prompt from kconfig.
> 
> linux commits for reference:
>    23cd88c91343 ("kbuild: hide commands to run Kconfig, and show short log for syncconfig")
>    d952cfaf0cff ("kbuild: do not suppress Kconfig prompts for silent build")
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>

Re: [XEN PATCH 10/15] build: rename $(AFLAGS) to $(XEN_AFLAGS)

[Luca Fancellu]

> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> We don't want the AFLAGS from the environment, they are usually meant
> to build user space application and not for the hypervisor.
> 
> Config.mk doesn't provied any $(AFLAGS) so we can start a fresh
> $(XEN_AFLAGS).
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>

Re: [XEN PATCH 11/15] build: rename CFLAGS to XEN_CFLAGS in xen/Makefile

[Luca Fancellu]

> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> This is a preparatory patch. A future patch will not even use
> $(CFLAGS) to seed $(XEN_CFLAGS).
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>

Re: [XEN PATCH 10/15] build: rename $(AFLAGS) to $(XEN_AFLAGS)

[Luca Fancellu]

> On 24 May 2023, at 09:29, Luca Fancellu <Luca.Fancellu@arm.com> wrote:
> 
> 
> 
>> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
>> 
>> We don't want the AFLAGS from the environment, they are usually meant
>> to build user space application and not for the hypervisor.
>> 
>> Config.mk doesn't provied any $(AFLAGS) so we can start a fresh

NIT: there is a typo s/provied/provide/

>> $(XEN_AFLAGS).
>> 
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
> Tested-by: Luca Fancellu <luca.fancellu@arm.com>
> 
> 
> 

Re: [XEN PATCH 00/15] build: cleanup build log, avoid user's CFLAGS, avoid too many include of Config.mk

[Andrew Cooper]

On 23/05/2023 5:37 pm, Anthony PERARD wrote:
> Patch series available in this git branch:
> https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.build-system-xen-removing-config.mk-v1
>
> Hi,
>
> This series of patch cleanup the remaining rules still displaying their command
> line.
>
> Then, some change are made in Config.mk to remove build-id stuff only used by
> Xen build.
>
> Then, the variable AFLAGS and CFLAGS are been renamed XEN_AFLAGS and XEN_CFLAGS
> from the beginning to about inclusion of users CFLAGS as those are usually made
> for user space program, not kernel, especially in build environment for distro
> packages.
>
> Last patch removes the inclusion of Config.mk from xen/Rules.mk, as this slow
> down the build unnecessarily. xen/Makefile should do everything necessary to
> setup the build of the hypervisor, and is its only entry point.

Thankyou for doing this.  I'm tempted to summarily ack it, but lets do
things properly.

One thing though, which I think might be a regression but I'm not sure. 
When doing an incremental build (second build with no change), we get:

...
  UPD     include/xen/compile.h
 __  __            _  _    _  ___                     _        _     _      
 \ \/ /___ _ __   | || |  / |( _ )    _   _ _ __  ___| |_ __ _| |__ | | ___
  \  // _ \ '_ \  | || |_ | |/ _ \ __| | | | '_ \/ __| __/ _` | '_ \| |/ _ \
  /  \  __/ | | | |__   _|| | (_) |__| |_| | | | \__ \ || (_| | |_) | |  __/
 /_/\_\___|_| |_|    |_|(_)_|\___/    \__,_|_| |_|___/\__\__,_|_.__/|_|\___|
                                                                            
make[2]: Nothing to be done for 'all'.
make[4]: Nothing to be done for 'all'.
  CC      common/version.o
  CC      arch/x86/efi/boot.o
...

Where I think those two "nothing to be done for 'all'" are new.  I don't
see them in a build from clean.

~Andrew

P.S. I do have some other notes for further cleanup, but I'm not going
to extend this current series with them.

Re: [XEN PATCH 03/15] build, x86: clean build log for boot/ targets

[Andrew Cooper]

On 23/05/2023 5:37 pm, Anthony PERARD wrote:
> diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
> index 03d8ce3a9e..2693b938bd 100644
> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -26,10 +28,16 @@ $(head-bin-objs): XEN_CFLAGS := $(CFLAGS_x86_32) -fpic
>  LDFLAGS_DIRECT-$(call ld-option,--warn-rwx-segments) := --no-warn-rwx-segments
>  LDFLAGS_DIRECT += $(LDFLAGS_DIRECT-y)
>  
> -%.bin: %.lnk
> -	$(OBJCOPY) -j .text -O binary $< $@
> +%.bin: OBJCOPYFLAGS := -j .text -O binary
> +%.bin: %.lnk FORCE
> +	$(call if_changed,objcopy)
>  
> -%.lnk: %.o $(src)/build32.lds
> -	$(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter %.lds,$^) -o $@ $<
> +quiet_cmd_ld_lnk_o = LD      $@

I'd suggest that this be LD32 because it is different to most other LD's
in the log.

However, this is a definite improvement.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>

Happy to fix up on commit.

Re: [XEN PATCH 12/15] build: avoid Config.mk's CFLAGS

[Luca Fancellu]

> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> The variable $(CFLAGS) is too often set in the environment,
> especially when building a package for a distribution. Often, those
> CFLAGS are intended to be use to build user spaces binaries, not a

NIT: s/use/used/

But I’m not a native speaker so I might be wrong on this

> kernel. This mean packager needs to takes extra steps to build Xen by
> overriding the CFLAGS provided by the package build environment.
> 
> With this patch, we avoid using the variable $(CFLAGS). Also, the
> hypervisor's build system have complete control over which CFLAGS are
> used.
> 
> No change intended to XEN_CFLAGS used, beside some flags which may be
> in a different order on the command line.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> 
> Notes:
>    There's still $(EXTRA_CFLAGS_XEN_CORE) which allows to add more CFLAGS
>    if someone building Xen needs to add more CFLAGS to the hypervisor
>    build.
> 

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>

Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line

[Luca Fancellu]

> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> CFLAGS is just from Config.mk, instead use the flags used to build
> Xen.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> 
> Notes:
>    I don't know if CFLAGS is even useful there, just --version without the
>    flags might produce the same result.
> 
> xen/build.mk | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/build.mk b/xen/build.mk
> index e2a78aa806..d468bb6e26 100644
> --- a/xen/build.mk
> +++ b/xen/build.mk
> @@ -23,7 +23,7 @@ define cmd_compile.h
>    -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
>    -e 's/@@domain@@/$(XEN_DOMAIN)/g' \
>    -e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \
> -    -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' \
> +    -e 's!@@compiler@@!$(shell $(CC) $(XEN_CFLAGS) --version 2>&1 | head -1)!g' \
>    -e 's/@@version@@/$(XEN_VERSION)/g' \
>    -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \
>    -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \
> -- 
> Anthony PERARD
> 
> 

Yes I think Andrew is right, so I guess $(XEN_CFLAGS) can be dropped?

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>

I’ve tested this patch with and without the $(XEN_CFLAGS), so if you drop it you can
retain my r-by if you want.

Re: [XEN PATCH 14/15] Config.mk: move $(cc-option, ) to config/compiler-testing.mk

[Luca Fancellu]

> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> In xen/, it isn't necessary to include Config.mk in every Makefile in
> subdirectories as nearly all necessary variables should be calculated
> in xen/Makefile. But some Makefile make use of the macro $(cc-option,)
> that is only available in Config.mk.
> 
> Extract $(cc-option,) from Config.mk so we can use it without
> including Config.mk and thus without having to recalculate some CFLAGS
> which would be ignored.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>

Re: [XEN PATCH 15/15] build: remove Config.mk include from Rules.mk

[Luca Fancellu]

> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> Everything needed to build the hypervisor should already be configured
> by "xen/Makefile", thus Config.mk shouldn't be needed.
> 
> Then, Config.mk keeps on testing support of some CFLAGS with CC, the
> result of this testing is not used at this stage so the build is
> slowed unnecessarily.
> 
> Likewise, GCC is checked to be at the minimum at 4.2 when entering
> every sub-directory, so the check have run countless time at this
> stage.
> 
> We only need to export a few more configuration variables. And add
> some variables in Kbuild.include, and macro fallbacks for Make older
> than 3.81. (Adding `or` just in case. it's only used in xen/Makefile,
> which includes Config.mk and so has already the fallback.)
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>

Re: [XEN PATCH 02/15] build: rework asm-offsets.* build step to use kbuild

[Jan Beulich]

On 23.05.2023 18:37, Anthony PERARD wrote:
> Use $(if_changed_dep, ) macro to generate "asm-offsets.s" and remove
> the use of $(move-if-changes,). That mean that "asm-offset.s" will be
> changed even when the output doesn't change.
> 
> But "asm-offsets.s" is only used to generated "asm-offsets.h". So
> instead of regenerating "asm-offsets.h" every time "asm-offsets.s"
> change, we will use "$(filechk, )" to only update the ".h" when the
> output change. Also, with "$(filechk, )", the file does get
> regenerated when the rule change in the makefile.
> 
> This changes also result in a cleaner build log.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> 
> Instead of having a special $(cmd_asm-offsets.s) command, we could
> probably reuse $(cmd_cc_s_c) from Rules.mk, but that would mean that
> an hypothetical additional flags "-flto" in CFLAGS would not be
> removed anymore, not sure if that matter here.

There's no real code being generated there, and what we're after are
merely the special .ascii directives. So the presence of -flto should
have no effect there, and hence it would even look more consistent to
me if we didn't use different options (and even rules) for .c -> .s
transformations.

> But then we could write this:
> 
> targets += arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s
> arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s: CFLAGS-y += -g0
> arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s FORCE
> 
> instead of having to write a rule for asm-offsets.s

Ftaod, I'd be happy to ack the patch as it is, but I would favor if
you could do the rework / simplification as outlined.

Jan

Re: [XEN PATCH 03/15] build, x86: clean build log for boot/ targets

[Jan Beulich]

On 23.05.2023 18:37, Anthony PERARD wrote:
> We are adding %.lnk to .PRECIOUS or make treat them as intermediate
> targets and remove them.

What's wrong with them getting removed? Note also that's no different from
today, so it's an orthogonal change in any event (unless I'm overlooking
something). Plus if such behavior was intended, shouldn't $(targets) be
made a prereq of .PRECIOUS in generic makefile logic?

> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  xen/arch/x86/boot/Makefile | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
> index 03d8ce3a9e..2693b938bd 100644
> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -5,6 +5,8 @@ head-bin-objs := cmdline.o reloc.o
>  nocov-y   += $(head-bin-objs)
>  noubsan-y += $(head-bin-objs)
>  targets   += $(head-bin-objs)
> +targets   += $(head-bin-objs:.o=.bin)
> +targets   += $(head-bin-objs:.o=.lnk)

Leaving aside the question of whether .lnk really should be part
of $(targets), don't these two lines also ...

> @@ -26,10 +28,16 @@ $(head-bin-objs): XEN_CFLAGS := $(CFLAGS_x86_32) -fpic
>  LDFLAGS_DIRECT-$(call ld-option,--warn-rwx-segments) := --no-warn-rwx-segments
>  LDFLAGS_DIRECT += $(LDFLAGS_DIRECT-y)
>  
> -%.bin: %.lnk
> -	$(OBJCOPY) -j .text -O binary $< $@
> +%.bin: OBJCOPYFLAGS := -j .text -O binary
> +%.bin: %.lnk FORCE
> +	$(call if_changed,objcopy)
>  
> -%.lnk: %.o $(src)/build32.lds
> -	$(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter %.lds,$^) -o $@ $<
> +quiet_cmd_ld_lnk_o = LD      $@
> +cmd_ld_lnk_o = $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter %.lds,$^) -o $@ $<
> +
> +%.lnk: %.o $(src)/build32.lds FORCE
> +	$(call if_changed,ld_lnk_o)
>  
>  clean-files := *.lnk *.bin

... eliminate the need for this?

Jan

> +
> +.PRECIOUS: %.lnk

Re: [XEN PATCH 02/15] build: rework asm-offsets.* build step to use kbuild

[Anthony PERARD]

On Wed, May 24, 2023 at 04:09:39PM +0200, Jan Beulich wrote:
> On 23.05.2023 18:37, Anthony PERARD wrote:
> > Use $(if_changed_dep, ) macro to generate "asm-offsets.s" and remove
> > the use of $(move-if-changes,). That mean that "asm-offset.s" will be
> > changed even when the output doesn't change.
> > 
> > But "asm-offsets.s" is only used to generated "asm-offsets.h". So
> > instead of regenerating "asm-offsets.h" every time "asm-offsets.s"
> > change, we will use "$(filechk, )" to only update the ".h" when the
> > output change. Also, with "$(filechk, )", the file does get
> > regenerated when the rule change in the makefile.
> > 
> > This changes also result in a cleaner build log.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > ---
> > 
> > Instead of having a special $(cmd_asm-offsets.s) command, we could
> > probably reuse $(cmd_cc_s_c) from Rules.mk, but that would mean that
> > an hypothetical additional flags "-flto" in CFLAGS would not be
> > removed anymore, not sure if that matter here.
> 
> There's no real code being generated there, and what we're after are
> merely the special .ascii directives. So the presence of -flto should
> have no effect there, and hence it would even look more consistent to
> me if we didn't use different options (and even rules) for .c -> .s
> transformations.
> 
> > But then we could write this:
> > 
> > targets += arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s
> > arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s: CFLAGS-y += -g0
> > arch/$(TARGET_ARCH)/include/asm/asm-offsets.h: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.s FORCE
> > 
> > instead of having to write a rule for asm-offsets.s
> 
> Ftaod, I'd be happy to ack the patch as it is, but I would favor if
> you could do the rework / simplification as outlined.

Thanks, I'll do this rework.

-- 
Anthony PERARD

Re: [XEN PATCH 03/15] build, x86: clean build log for boot/ targets

[Anthony PERARD]

On Wed, May 24, 2023 at 04:16:54PM +0200, Jan Beulich wrote:
> On 23.05.2023 18:37, Anthony PERARD wrote:
> > We are adding %.lnk to .PRECIOUS or make treat them as intermediate
> > targets and remove them.
> 
> What's wrong with them getting removed? Note also that's no different from
> today, so it's an orthogonal change in any event (unless I'm overlooking

Indeed, those targets are been removed today. That doesn't cause any
issue because make knows that they are just intermediate and it doesn't
have to rebuilt them if there are just missing.

But, the macro $(if_changed,) doesn't know about intermediates, and if
the target is missing, then it's going to be rebuilt. So with
$(if_changed,) the %.lnk targets are been rebuilt at every incremental
build which cause make to relink "xen" when there's otherwise nothing
to be done.
(I'm using a command like `XEN_BUILD_DATE=today XEN_BUILD_TIME=now make`
to avoid "compile.h" from been regenerated every time)

So, the change isn't orthogonal, but needs a better explanation in the
commit message.

> something). Plus if such behavior was intended, shouldn't $(targets) be
> made a prereq of .PRECIOUS in generic makefile logic?

I think I need to reevaluate what to do here. Maybe .PRECIOUS isn't the
right one to use. But yes, we probably want something generic to tell
make to never delete any $(targets) when they are intermediate.

Linux uses .SECONDARY or .NOTINTERMEDIATE, and .SECONDARY might be
better than .PRECIOUS. PRECIOUS also prevent make from delete a target
when make is interrupted or killed, which might not be desired.

> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > ---
> >  xen/arch/x86/boot/Makefile | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
> > index 03d8ce3a9e..2693b938bd 100644
> > --- a/xen/arch/x86/boot/Makefile
> > +++ b/xen/arch/x86/boot/Makefile
> > @@ -5,6 +5,8 @@ head-bin-objs := cmdline.o reloc.o
> >  nocov-y   += $(head-bin-objs)
> >  noubsan-y += $(head-bin-objs)
> >  targets   += $(head-bin-objs)
> > +targets   += $(head-bin-objs:.o=.bin)
> > +targets   += $(head-bin-objs:.o=.lnk)
> 
> Leaving aside the question of whether .lnk really should be part
> of $(targets), don't these two lines also ...
> 
> > @@ -26,10 +28,16 @@ $(head-bin-objs): XEN_CFLAGS := $(CFLAGS_x86_32) -fpic
> >  LDFLAGS_DIRECT-$(call ld-option,--warn-rwx-segments) := --no-warn-rwx-segments
> >  LDFLAGS_DIRECT += $(LDFLAGS_DIRECT-y)
> >  
> > -%.bin: %.lnk
> > -	$(OBJCOPY) -j .text -O binary $< $@
> > +%.bin: OBJCOPYFLAGS := -j .text -O binary
> > +%.bin: %.lnk FORCE
> > +	$(call if_changed,objcopy)
> >  
> > -%.lnk: %.o $(src)/build32.lds
> > -	$(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter %.lds,$^) -o $@ $<
> > +quiet_cmd_ld_lnk_o = LD      $@
> > +cmd_ld_lnk_o = $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT)) -N -T $(filter %.lds,$^) -o $@ $<
> > +
> > +%.lnk: %.o $(src)/build32.lds FORCE
> > +	$(call if_changed,ld_lnk_o)
> >  
> >  clean-files := *.lnk *.bin
> 
> ... eliminate the need for this?

Yes, but that doesn't seems to work here.

I think there's a "targets:=" missing in "Makefile.clean". So, new patch
to fix that, and then I can eliminate the "clean-files".

Cheers,

-- 
Anthony PERARD

Re: [XEN PATCH 07/15] build: move XEN_HAS_BUILD_ID out of Config.mk

[Jan Beulich]

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

Re: [XEN PATCH 08/15] build: use $(filechk, ) for all compat/.xlat/%.lst

[Jan Beulich]

On 23.05.2023 18:38, Anthony PERARD wrote:
> Make use of filechk mean that we don't have to use

I think you mean "Making use of filechk means ...", or else it reads as
if you're changing how filechk behaves. (I'd again be happy to adjust
while committing, provided you agree; here it looks pretty clear that
there is no dependency on earlier patches, and there's also no need to
wait for further acks.)

> $(move-if-changed,). It also mean that will have sometime "UPD .." in
> the build output when the target changed, rather than having "GEN ..."
> all the time when "xlat.lst" happen to have a more recent modification
> timestamp.
> 
> While there, replace `grep -v` by `sed '//d'` to avoid an extra
> fork and pipe when building.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [XEN PATCH 09/15] build: hide commands run for kconfig

[Jan Beulich]

On 23.05.2023 18:38, Anthony PERARD wrote:
> but still show a log entry for syncconfig. We have to use kecho
> instead of $(cmd,) to avoid issue with prompt from kconfig.

Reading this description I was looking for uses of $(cmd ...) that you
replace. I think this wants wording differently, e.g. "We have to
use kecho, not $(cmd,), to ..."

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -382,6 +382,7 @@ $(KCONFIG_CONFIG): tools_fixdep
>  # This exploits the 'multi-target pattern rule' trick.
>  # The syncconfig should be executed only once to make all the targets.
>  include/config/%.conf include/config/%.conf.cmd: $(KCONFIG_CONFIG)
> +	$(Q)$(kecho) "  SYNC    $@"
>  	$(Q)$(MAKE) $(build)=tools/kconfig syncconfig

The latter of the Linux commits you reference also extends the comment,
to keep people from trying to switch to using $(cmd ...). I think we
should follow suit.

Jan

Re: [XEN PATCH 10/15] build: rename $(AFLAGS) to $(XEN_AFLAGS)

[Jan Beulich]

On 24.05.2023 10:47, Luca Fancellu wrote:
> 
> 
>> On 24 May 2023, at 09:29, Luca Fancellu <Luca.Fancellu@arm.com> wrote:
>>
>>
>>
>>> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>>
>>> We don't want the AFLAGS from the environment, they are usually meant
>>> to build user space application and not for the hypervisor.
>>>
>>> Config.mk doesn't provied any $(AFLAGS) so we can start a fresh
> 
> NIT: there is a typo s/provied/provide/

And preferably with that adjustment ...

>>> $(XEN_AFLAGS).
>>>
>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>> ---
>>
>> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
>> Tested-by: Luca Fancellu <luca.fancellu@arm.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

There is some interaction with the asm-offsets change, but I think the
two patches are still functionally and contextually independent (and
hence the one here could go in ahead of the other one earlier in the
series, with said adjustment made while committing). Please confirm.

Jan

Re: [XEN PATCH 11/15] build: rename CFLAGS to XEN_CFLAGS in xen/Makefile

[Jan Beulich]

On 23.05.2023 18:38, Anthony PERARD wrote:
> This is a preparatory patch. A future patch will not even use
> $(CFLAGS) to seed $(XEN_CFLAGS).
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

I have a question though, albeit not directly related to this patch:

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -259,6 +259,7 @@ export KBUILD_DEFCONFIG := $(ARCH)_defconfig
>  export XEN_TREEWIDE_CFLAGS := $(CFLAGS)
>  
>  XEN_AFLAGS =
> +XEN_CFLAGS = $(CFLAGS)
>  
>  # CLANG_FLAGS needs to be calculated before calling Kconfig
>  ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
> @@ -284,7 +285,7 @@ CLANG_FLAGS += $(call or,$(t1),$(t2),$(t3))
>  endif
>  
>  CLANG_FLAGS += -Werror=unknown-warning-option
> -CFLAGS += $(CLANG_FLAGS)
> +XEN_CFLAGS += $(CLANG_FLAGS)
>  export CLANG_FLAGS
>  endif
>  
> @@ -293,7 +294,7 @@ ifeq ($(call ld-ver-build-id,$(LD)),n)
>  XEN_LDFLAGS_BUILD_ID :=
>  XEN_HAS_BUILD_ID := n
>  else
> -CFLAGS += -DBUILD_ID
> +XEN_CFLAGS += -DBUILD_ID
>  XEN_TREEWIDE_CFLAGS += -DBUILD_ID

Is this redundancy necessary? IOW can't XEN_CFLAGS, at an appopriate
place, simply have $(XEN_TREEWIDE_CFLAGS) appended?

Apart from this the same process question again: Is this independent
of earlier patches (except the immediately preceding one), and could
hence - provided arch maintainer acks arrive - go in ahead of time?

Jan

Re: [XEN PATCH 12/15] build: avoid Config.mk's CFLAGS

[Jan Beulich]

On 23.05.2023 18:38, Anthony PERARD wrote:
> The variable $(CFLAGS) is too often set in the environment,
> especially when building a package for a distribution. Often, those
> CFLAGS are intended to be use to build user spaces binaries, not a
> kernel. This mean packager needs to takes extra steps to build Xen by
> overriding the CFLAGS provided by the package build environment.
> 
> With this patch, we avoid using the variable $(CFLAGS). Also, the
> hypervisor's build system have complete control over which CFLAGS are
> used.

"..., apart from $(EXTRA_CFLAGS_XEN_CORE)", as you say ...

> No change intended to XEN_CFLAGS used, beside some flags which may be
> in a different order on the command line.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> 
> Notes:
>     There's still $(EXTRA_CFLAGS_XEN_CORE) which allows to add more CFLAGS
>     if someone building Xen needs to add more CFLAGS to the hypervisor
>     build.

... only here.

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -259,7 +259,16 @@ export KBUILD_DEFCONFIG := $(ARCH)_defconfig
>  export XEN_TREEWIDE_CFLAGS := $(CFLAGS)
>  
>  XEN_AFLAGS =
> -XEN_CFLAGS = $(CFLAGS)
> +XEN_CFLAGS =
> +ifeq ($(XEN_OS),SunOS)
> +    XEN_CFLAGS +=  -Wa,--divide -D_POSIX_C_SOURCE=200112L -D__EXTENSIONS__

So this (and the arch.mk additions) duplicate stuff we have in config/*.mk.
Such duplication isn't really nice. Setting AS, CC, etc also happens there,
and hence I expect you're going to duplicate that as well in a later patch.
Can't we massage (if necessary) the config/*.mk relevant to the hypervisor
build, so they can be included from xen/Makefile? That way all such basic
settings could remain in a central place, which has been well known for
many years.

Jan

Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line

[Jan Beulich]

On 23.05.2023 20:14, Andrew Cooper wrote:
> On 23/05/2023 5:38 pm, Anthony PERARD wrote:
>> CFLAGS is just from Config.mk, instead use the flags used to build
>> Xen.
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>>
>> Notes:
>>     I don't know if CFLAGS is even useful there, just --version without the
>>     flags might produce the same result.
> 
> I can't think of any legitimate reason for CFLAGS to be here.
> 
> Any compiler which does differ its output based on CLFAGS is probably
> one we don't want to be using...

Well, I wouldn't go quite as far in general, but I agree for the --version
case. Actually at least with gcc it's even "better": I've tried a couple
of 32-bit compilers with "-m64 --version", which would normally choke on
the -m64. But that options is ignored altogether when --version is there.
(Which has up- and downsides of course; the command failing might also be
useful, in telling us that the compiler isn't usable in the first place.)

Jan

Re: [XEN PATCH 04/15] build: hide policy.bin commands

[Daniel P. Smith]

On 5/23/23 12:38, Anthony PERARD wrote:
> Instead, show only when "policy.bin" is been updated.
> 
> We still have the full command from the flask/policy Makefile, but we
> can't change that Makefile.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>   xen/xsm/flask/Makefile | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
> index 3fdcf7727e..fc44ad684f 100644
> --- a/xen/xsm/flask/Makefile
> +++ b/xen/xsm/flask/Makefile
> @@ -48,10 +48,15 @@ targets += flask-policy.S
>   FLASK_BUILD_DIR := $(abs_objtree)/$(obj)
>   POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
>   
> +policy_chk = \
> +    $(Q)if ! cmp -s $(POLICY_SRC) $@; then \
> +        $(kecho) '  UPD     $@'; \
> +        cp $(POLICY_SRC) $@; \
> +    fi
>   $(obj)/policy.bin: FORCE
> -	$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
> +	$(Q)$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common \
>   	        -C $(XEN_ROOT)/tools/flask/policy \
>   	        FLASK_BUILD_DIR=$(FLASK_BUILD_DIR) POLICY_FILENAME=$(POLICY_SRC)
> -	cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@
> +	$(call policy_chk)
>   
>   clean-files := policy.* $(POLICY_SRC)

Outside the suggestion and nit(s) from Jan, all else looks good to me.

Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>

Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line

[Jan Beulich]

On 24.05.2023 11:43, Luca Fancellu wrote:
> 
> 
>> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>
>> CFLAGS is just from Config.mk, instead use the flags used to build
>> Xen.
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>>
>> Notes:
>>    I don't know if CFLAGS is even useful there, just --version without the
>>    flags might produce the same result.
>>
>> xen/build.mk | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/build.mk b/xen/build.mk
>> index e2a78aa806..d468bb6e26 100644
>> --- a/xen/build.mk
>> +++ b/xen/build.mk
>> @@ -23,7 +23,7 @@ define cmd_compile.h
>>    -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
>>    -e 's/@@domain@@/$(XEN_DOMAIN)/g' \
>>    -e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \
>> -    -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' \
>> +    -e 's!@@compiler@@!$(shell $(CC) $(XEN_CFLAGS) --version 2>&1 | head -1)!g' \
>>    -e 's/@@version@@/$(XEN_VERSION)/g' \
>>    -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \
>>    -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \
>> -- 
>> Anthony PERARD
>>
>>
> 
> Yes I think Andrew is right, so I guess $(XEN_CFLAGS) can be dropped?
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
> Tested-by: Luca Fancellu <luca.fancellu@arm.com>
> 
> I’ve tested this patch with and without the $(XEN_CFLAGS), so if you drop it you can
> retain my r-by if you want.

Acked-by: Jan Beulich <jbeulich@suse.com>
preferably with the $(CFLAGS) dropped, which again I'd be happy to do
while committing.

Jan

Re: [XEN PATCH 04/15] build: hide policy.bin commands

[Anthony PERARD]

On Wed, May 24, 2023 at 09:11:10AM +0200, Jan Beulich wrote:
> On 23.05.2023 18:38, Anthony PERARD wrote:
> > --- a/xen/xsm/flask/Makefile
> > +++ b/xen/xsm/flask/Makefile
> > @@ -48,10 +48,15 @@ targets += flask-policy.S
> >  FLASK_BUILD_DIR := $(abs_objtree)/$(obj)
> >  POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
> >  
> > +policy_chk = \
> > +    $(Q)if ! cmp -s $(POLICY_SRC) $@; then \
> > +        $(kecho) '  UPD     $@'; \
> > +        cp $(POLICY_SRC) $@; \
> 
> Wouldn't this better use move-if-changed? Which, if "UPD ..." output is
> desired, would then need overriding from what Config.mk supplies?

I don't like move-if-changed, because it remove the original target. On
incremental build, make will keep building the original target even
when not needed. So we keep seeing the `checkpolicy` command line when
there's otherwise nothing to do.

I could introduce a new generic macro instead, copy-if-changed, which
will do compare and copy (like policy_chk is doing here).

Thanks,

-- 
Anthony PERARD

Re: [XEN PATCH 05/15] build: introduce a generic command for gzip

[Anthony PERARD]

On Wed, May 24, 2023 at 09:17:09AM +0200, Jan Beulich wrote:
> On 23.05.2023 18:38, Anthony PERARD wrote:
> > Make the gzip command generic and use -9 which wasn't use for
> > config.gz. (xen.gz does use -9)
> 
> You mention xen.gz here, but you don't make its rule use this new
> construct. Is that intentional (and if so, why)? (There we also go
> through $@.new, and being consistent in that regard would imo be
> desirable as well.)

It was kind of a justification to say that -9 was ok, because we already
use it.

But I can't use it for xen.gz because the generic command is added to
Rules.mk. But I can probably add cmd_gzip to Kbuild.include instead and
use it for xen.gz, (with simply $(call cmd,) instead of using
if_changed).

Thanks,

-- 
Anthony PERARD

Re: [XEN PATCH 04/15] build: hide policy.bin commands

[Jan Beulich]

On 25.05.2023 15:34, Anthony PERARD wrote:
> On Wed, May 24, 2023 at 09:11:10AM +0200, Jan Beulich wrote:
>> On 23.05.2023 18:38, Anthony PERARD wrote:
>>> --- a/xen/xsm/flask/Makefile
>>> +++ b/xen/xsm/flask/Makefile
>>> @@ -48,10 +48,15 @@ targets += flask-policy.S
>>>  FLASK_BUILD_DIR := $(abs_objtree)/$(obj)
>>>  POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
>>>  
>>> +policy_chk = \
>>> +    $(Q)if ! cmp -s $(POLICY_SRC) $@; then \
>>> +        $(kecho) '  UPD     $@'; \
>>> +        cp $(POLICY_SRC) $@; \
>>
>> Wouldn't this better use move-if-changed? Which, if "UPD ..." output is
>> desired, would then need overriding from what Config.mk supplies?
> 
> I don't like move-if-changed, because it remove the original target. On
> incremental build, make will keep building the original target even
> when not needed. So we keep seeing the `checkpolicy` command line when
> there's otherwise nothing to do.
> 
> I could introduce a new generic macro instead, copy-if-changed, which
> will do compare and copy (like policy_chk is doing here).

Ah, yes, I think I see what you mean. I'd be fine with copy-if-changed,
ideally accompanied by some rule of thumb when to prefer it over
move-if-changed.

Jan

Re: [XEN PATCH 07/15] build: move XEN_HAS_BUILD_ID out of Config.mk

[Anthony PERARD]

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,

-- 
Anthony PERARD

Re: [XEN PATCH 08/15] build: use $(filechk, ) for all compat/.xlat/%.lst

[Anthony PERARD]

On Thu, May 25, 2023 at 02:04:00PM +0200, Jan Beulich wrote:
> On 23.05.2023 18:38, Anthony PERARD wrote:
> > Make use of filechk mean that we don't have to use
> 
> I think you mean "Making use of filechk means ...", or else it reads as
> if you're changing how filechk behaves. (I'd again be happy to adjust
> while committing, provided you agree; here it looks pretty clear that
> there is no dependency on earlier patches, and there's also no need to
> wait for further acks.)

The change sounds good. And yes, no dependency.

> > $(move-if-changed,). It also mean that will have sometime "UPD .." in
> > the build output when the target changed, rather than having "GEN ..."
> > all the time when "xlat.lst" happen to have a more recent modification
> > timestamp.
> > 
> > While there, replace `grep -v` by `sed '//d'` to avoid an extra
> > fork and pipe when building.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>

Thanks,

-- 
Anthony PERARD

Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line

[Jan Beulich]

On 24.05.2023 11:43, Luca Fancellu wrote:
> 
> 
>> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>
>> CFLAGS is just from Config.mk, instead use the flags used to build
>> Xen.
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>>
>> Notes:
>>    I don't know if CFLAGS is even useful there, just --version without the
>>    flags might produce the same result.
>>
>> xen/build.mk | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/build.mk b/xen/build.mk
>> index e2a78aa806..d468bb6e26 100644
>> --- a/xen/build.mk
>> +++ b/xen/build.mk
>> @@ -23,7 +23,7 @@ define cmd_compile.h
>>    -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
>>    -e 's/@@domain@@/$(XEN_DOMAIN)/g' \
>>    -e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \
>> -    -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' \
>> +    -e 's!@@compiler@@!$(shell $(CC) $(XEN_CFLAGS) --version 2>&1 | head -1)!g' \
>>    -e 's/@@version@@/$(XEN_VERSION)/g' \
>>    -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \
>>    -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \
>> -- 
>> Anthony PERARD
>>
>>
> 
> Yes I think Andrew is right, so I guess $(XEN_CFLAGS) can be dropped?
> 
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
> Tested-by: Luca Fancellu <luca.fancellu@arm.com>
> 
> I’ve tested this patch with and without the $(XEN_CFLAGS), so if you drop it you can
> retain my r-by if you want.

I'm sorry, I didn't look back here to spot this extra sentence before
committing the edited patch, which as a result I've now put in without
your tags.

Jan

Re: [XEN PATCH 14/15] Config.mk: move $(cc-option, ) to config/compiler-testing.mk

[Jan Beulich]

On 23.05.2023 18:38, Anthony PERARD wrote:
> In xen/, it isn't necessary to include Config.mk in every Makefile in
> subdirectories as nearly all necessary variables should be calculated
> in xen/Makefile. But some Makefile make use of the macro $(cc-option,)
> that is only available in Config.mk.
> 
> Extract $(cc-option,) from Config.mk so we can use it without
> including Config.mk and thus without having to recalculate some CFLAGS
> which would be ignored.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

When I saw the title and in particular the new file's name which is
already mentioned there, I did expect something entirely different,
perhaps related to the testing of Xen. May I suggest e.g.
compiler-probing.mk or maybe even simply compiler.mk? (I'm also
uncertain about "compiler", tbh - I could e.g. see Kbuild.include's
as-option-add to also be useful outside of xen/, and hence at some
point wanting moving to some common file.)

> --- a/Config.mk
> +++ b/Config.mk
> @@ -18,6 +18,7 @@ realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir $(file)) && echo "
>  or       = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(3),$(if $(strip $(4)),$(4)))))
>  
>  -include $(XEN_ROOT)/.config
> +include $(XEN_ROOT)/config/compiler-testing.mk

Possibly being one of the few users of the top-level .config, I wonder
about the ordering of these includes. This isn't to say that I consider
wrong the order in which you have it now, but I could see the opposite
order as potentially useful, too.

Jan

Re: [XEN PATCH 15/15] build: remove Config.mk include from Rules.mk

[Jan Beulich]

On 23.05.2023 18:38, Anthony PERARD wrote:
> Everything needed to build the hypervisor should already be configured
> by "xen/Makefile", thus Config.mk shouldn't be needed.

"... by xen/Rules.mk." (Or else it sounds as if yo're removing its use
altogether.)

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -246,10 +246,14 @@ export TARGET_ARCH     := $(shell echo $(XEN_TARGET_ARCH) | \
>                              sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \
>                                  -e s'/riscv.*/riscv/g')
>  
> +export XEN_COMPILE_ARCH XEN_TARGET_ARCH
>  export CONFIG_SHELL := $(SHELL)
>  export CC CXX LD NM OBJCOPY OBJDUMP ADDR2LINE
> +export CPP AR

For these two, could I talk you into editing the earlier line instead
of adding a new one?

> --- a/xen/scripts/Kbuild.include
> +++ b/xen/scripts/Kbuild.include
> @@ -8,6 +8,13 @@ empty   :=
>  space   := $(empty) $(empty)
>  space_escape := _-_SPACE_-_
>  pound   := \#
> +comma   := ,
> +open    := (
> +close   := )
> +
> +# fallbacks for GNU Make older than 3.81
> +realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir $(file)) && echo "$$PWD/$(notdir $(file))")))
> +or       = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(3),$(if $(strip $(4)),$(4)))))
>  
>  ###
>  # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o

As long as they're the same, the collision with Config.mk's will be
benign (for xen/Makefile), but I wonder whether, along the lines of
the earlier patch, these wouldn't better be extracted into e.g.
config/fallbacks.mk. (Whether the single-character macros are also
extracted into somewhere is of less importance to me, at least right
now.)

Jan

Re: [XEN PATCH 13/15] build: fix compile.h compiler version command line

[Luca Fancellu]

> On 30 May 2023, at 11:14, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 24.05.2023 11:43, Luca Fancellu wrote:
>> 
>> 
>>> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>> 
>>> CFLAGS is just from Config.mk, instead use the flags used to build
>>> Xen.
>>> 
>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>> ---
>>> 
>>> Notes:
>>>   I don't know if CFLAGS is even useful there, just --version without the
>>>   flags might produce the same result.
>>> 
>>> xen/build.mk | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/xen/build.mk b/xen/build.mk
>>> index e2a78aa806..d468bb6e26 100644
>>> --- a/xen/build.mk
>>> +++ b/xen/build.mk
>>> @@ -23,7 +23,7 @@ define cmd_compile.h
>>>   -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \
>>>   -e 's/@@domain@@/$(XEN_DOMAIN)/g' \
>>>   -e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \
>>> -    -e 's!@@compiler@@!$(shell $(CC) $(CFLAGS) --version 2>&1 | head -1)!g' \
>>> +    -e 's!@@compiler@@!$(shell $(CC) $(XEN_CFLAGS) --version 2>&1 | head -1)!g' \
>>>   -e 's/@@version@@/$(XEN_VERSION)/g' \
>>>   -e 's/@@subversion@@/$(XEN_SUBVERSION)/g' \
>>>   -e 's/@@extraversion@@/$(XEN_EXTRAVERSION)/g' \
>>> -- 
>>> Anthony PERARD
>>> 
>>> 
>> 
>> Yes I think Andrew is right, so I guess $(XEN_CFLAGS) can be dropped?
>> 
>> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
>> Tested-by: Luca Fancellu <luca.fancellu@arm.com>
>> 
>> I’ve tested this patch with and without the $(XEN_CFLAGS), so if you drop it you can
>> retain my r-by if you want.
> 
> I'm sorry, I didn't look back here to spot this extra sentence before
> committing the edited patch, which as a result I've now put in without
> your tags.
> 

No problem!

> Jan