All of lore.kernel.org
 help / color / mirror / Atom feed
* [XEN PATCH v5 00/16] xen: Build system improvements
@ 2020-04-21 16:11 Anthony PERARD
  2020-04-21 16:11 ` [XEN PATCH v5 01/16] build,xsm: Fix multiple call Anthony PERARD
                   ` (15 more replies)
  0 siblings, 16 replies; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:11 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Tim Deegan, Jan Beulich,
	Anthony PERARD, Daniel De Graaf, Volodymyr Babchuk,
	Roger Pau Monné

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

v5:
- few changes detailed in patch notes.
- 1 new patch

v4:
- some patch already applied.
- Have added patches from "xen/arm: Configure early printk via Kconfig" series.
- Some new patch to add documentation or fix issues, and patch to improve
  compat header generation.
Other changes are detailed in patches.

v3:
- new patches that do some cleanup or fix issues
- have rework most patches, to have better commit message or change the coding
  style, or fix issues that I've seen. There were some cases where CFLAGS were
  missing.
  See patch notes for details
- introduce if_changed*. That plenty of new patches on top of what we had in v2.
  (those changes ignore CONFIG_LTO=y, I'll see about fixing that later)

(There is more to come in order to use fixdep from Linux, but that's not ready)

Hi,

I have work toward building Xen (the hypervisor) with Linux's build system,
Kbuild.

The main reason for that is to be able to have out-of-tree build. It's annoying
when a build fail because of the pvshim. Other benefit is a much faster
rebuild, and `make clean` doesn't take ages, and better dependencies to figure
out what needs to be rebuild.

So, we are not there yet, but the series already contain quite a few
improvement and cleanup. More patches are going to be added to the series.

Cheers,

Anthony PERARD (16):
  build,xsm: Fix multiple call
  xen/build: include include/config/auto.conf in main Makefile
  xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS)
  xen/build: have the root Makefile generates the CFLAGS
  build: Introduce documentation for xen Makefiles
  xen/build: introduce if_changed and if_changed_rule
  xen/build: Start using if_changed
  build: Introduce $(cpp_flags)
  xen/build: use if_changed on built_in.o
  xen/build: Use if_changed_rules with %.o:%.c targets
  xen/build: factorise generation of the linker scripts
  xen/build: Use if_changed for prelink*.o
  xen,symbols: rework file symbols selection
  build: use if_changed to build mm/*/guest_%.o
  build,include: rework compat-build-source.py
  build,include: rework compat-build-header.py

 .gitignore                            |   1 +
 docs/misc/xen-makefiles/makefiles.rst | 185 ++++++++++++++++++++
 xen/Makefile                          | 212 ++++++++++++++++++++---
 xen/Rules.mk                          | 235 ++++++++++++++++----------
 xen/arch/arm/Makefile                 |  22 +--
 xen/arch/arm/Rules.mk                 |  23 ---
 xen/arch/arm/{Rules.mk => arch.mk}    |   5 -
 xen/arch/arm/efi/Makefile             |   2 +-
 xen/arch/x86/Makefile                 |  41 ++---
 xen/arch/x86/Rules.mk                 |  91 +---------
 xen/arch/x86/{Rules.mk => arch.mk}    |  17 +-
 xen/arch/x86/efi/Makefile             |   9 +-
 xen/arch/x86/mm/Makefile              |  14 +-
 xen/arch/x86/mm/hap/Makefile          |  15 +-
 xen/arch/x86/mm/shadow/Makefile       |  14 +-
 xen/common/libelf/Makefile            |  14 +-
 xen/common/libfdt/Makefile            |  13 +-
 xen/include/Makefile                  |  16 +-
 xen/scripts/Kbuild.include            | 107 ++++++++++++
 xen/tools/compat-build-header.py      |  52 +++++-
 xen/tools/compat-build-source.py      |   8 +-
 xen/tools/symbols.c                   |  20 ++-
 xen/xsm/flask/Makefile                |  19 ++-
 xen/xsm/flask/ss/Makefile             |   2 +-
 24 files changed, 809 insertions(+), 328 deletions(-)
 create mode 100644 docs/misc/xen-makefiles/makefiles.rst
 copy xen/arch/arm/{Rules.mk => arch.mk} (85%)
 copy xen/arch/x86/{Rules.mk => arch.mk} (87%)

-- 
Anthony PERARD



^ permalink raw reply	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 01/16] build,xsm: Fix multiple call
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
@ 2020-04-21 16:11 ` Anthony PERARD
  2020-04-21 16:11 ` [XEN PATCH v5 02/16] xen/build: include include/config/auto.conf in main Makefile Anthony PERARD
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:11 UTC (permalink / raw)
  To: xen-devel; +Cc: Anthony PERARD, Daniel De Graaf, Jan Beulich

Both script mkflask.sh and mkaccess_vector.sh generates multiple
files. Exploits the 'multi-target pattern rule' trick to call each
scripts only once.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---

Notes:
    v5:
    - Use simpler $(subst ) instead of $(patsubst )
    - moved the patch ahead in the series
    
    v4:
    - new patch

 xen/xsm/flask/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index b1fd45421993..f001bb18d4ed 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -21,10 +21,10 @@ ALL_H_FILES = $(FLASK_H_FILES) $(AV_H_FILES)
 
 $(obj-y) ss/built_in.o: $(ALL_H_FILES)
 
-$(FLASK_H_FILES): $(FLASK_H_DEPEND)
+$(subst include/,%/,$(FLASK_H_FILES)): $(FLASK_H_DEPEND)
 	$(CONFIG_SHELL) policy/mkflask.sh $(AWK) include $(FLASK_H_DEPEND)
 
-$(AV_H_FILES): $(AV_H_DEPEND)
+$(subst include/,%/,$(AV_H_FILES)): $(AV_H_DEPEND)
 	$(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)
 
 obj-bin-$(CONFIG_XSM_FLASK_POLICY) += flask-policy.o
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 02/16] xen/build: include include/config/auto.conf in main Makefile
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
  2020-04-21 16:11 ` [XEN PATCH v5 01/16] build,xsm: Fix multiple call Anthony PERARD
@ 2020-04-21 16:11 ` Anthony PERARD
  2020-04-21 16:11 ` [XEN PATCH v5 03/16] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS) Anthony PERARD
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:11 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Jan Beulich, Anthony PERARD

We are going to generate the CFLAGS early from "xen/Makefile" instead
of in "Rules.mk", but we need to include "config/auto.conf", so
include it in "Makefile".

Before including "config/auto.conf" we check which make target a user
is calling, as some targets don't need "auto.conf". For targets that
needs auto.conf, make will generate it (and a default .config if
missing).

root-make-done is to avoid doing the calculation again once Rules.mk
takes over and is been executed with the root Makefile. When Rules.mk
is including xen/Makefile, `config-build' and `need-config' are
undefined so auto.conf will not be included again (it is already
included by Rules.mk) and kconfig target are out of reach of Rules.mk.

We are introducing a target %config to catch all targets for kconfig.
So we need an extra target %/.config to prevent make from trying to
regenerate $(XEN_ROOT)/.config that is included in Config.mk.

The way targets are filtered is inspired by Kbuild, with some code
imported from Linux. That's why there is PHONY variable that isn't
used yet, for example.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---

Notes:
    v5:
    - move kconfig shorthand to the only file that uses it.
    
    v4:
    - check that root-make-done hasn't been set to an expected value
      instead of checking if it has been set at all.
    - Add a shorthand $(kconfig) to run kconfig targets.
    
    v3:
    - filter only for %config instead of both config %config
    - keep the multi-target pattern rule trick for include/config/auto.conf
      instead of using Linux's newer pattern (we dont have tristate.conf so
      don't need to change it)
    - use y/n for root-make-done, config-build, need-config instead of
      relying on ifdef and ifndef and on assigning an empty value meaning
      undef
    - use space for indentation
    - explain why %/.config is suddenly needed.

 xen/Makefile | 101 +++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 78 insertions(+), 23 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index e5f7b1ae13bc..643c689658f3 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -49,7 +49,76 @@ default: build
 .PHONY: dist
 dist: install
 
-build install:: include/config/auto.conf
+
+ifneq ($(root-make-done),y)
+# section to run before calling Rules.mk, but only once.
+#
+# To make sure we do not include .config for any of the *config targets
+# catch them early, and hand them over to tools/kconfig/Makefile
+
+clean-targets := %clean
+no-dot-config-targets := $(clean-targets) \
+                         uninstall debug cloc \
+                         cscope TAGS tags MAP gtags \
+                         xenversion
+
+config-build    := n
+need-config     := y
+
+ifneq ($(filter $(no-dot-config-targets), $(MAKECMDGOALS)),)
+    ifeq ($(filter-out $(no-dot-config-targets), $(MAKECMDGOALS)),)
+        need-config := n
+    endif
+endif
+
+ifneq ($(filter %config,$(MAKECMDGOALS)),)
+    config-build := y
+endif
+
+export root-make-done := y
+endif # root-make-done
+
+include scripts/Kbuild.include
+
+# Shorthand for kconfig
+kconfig = -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)"
+
+ifeq ($(config-build),y)
+# ===========================================================================
+# *config targets only - make sure prerequisites are updated, and descend
+# in tools/kconfig to make the *config target
+
+config: FORCE
+	$(MAKE) $(kconfig) $@
+
+# Config.mk tries to include .config file, don't try to remake it
+%/.config: ;
+
+%config: FORCE
+	$(MAKE) $(kconfig) $@
+
+else # !config-build
+
+ifeq ($(need-config),y)
+include include/config/auto.conf
+# Read in dependencies to all Kconfig* files, make sure to run syncconfig if
+# changes are detected.
+include include/config/auto.conf.cmd
+
+# Allow people to just run `make` as before and not force them to configure
+$(KCONFIG_CONFIG):
+	$(MAKE) $(kconfig) defconfig
+
+# The actual configuration files used during the build are stored in
+# include/generated/ and include/config/. Update them if .config is newer than
+# include/config/auto.conf (which mirrors .config).
+#
+# 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)
+	$(MAKE) $(kconfig) syncconfig
+
+endif # need-config
 
 .PHONY: build install uninstall clean distclean MAP
 build install uninstall debug clean distclean MAP::
@@ -254,9 +323,6 @@ cscope:
 _MAP:
 	$(NM) -n $(TARGET)-syms | grep -v '\(compiled\)\|\(\.o$$\)\|\( [aUw] \)\|\(\.\.ng$$\)\|\(LASH[RL]DI\)' > System.map
 
-.PHONY: FORCE
-FORCE:
-
 %.o %.i %.s: %.c FORCE
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C $(*D) $(@F)
 
@@ -277,25 +343,6 @@ $(foreach base,arch/x86/mm/guest_walk_% \
                arch/x86/mm/shadow/guest_%, \
     $(foreach ext,o i s,$(call build-intermediate,$(base).$(ext))))
 
-kconfig := oldconfig config menuconfig defconfig allyesconfig allnoconfig \
-	nconfig xconfig gconfig savedefconfig listnewconfig olddefconfig \
-	randconfig $(notdir $(wildcard arch/$(SRCARCH)/configs/*_defconfig))
-.PHONY: $(kconfig)
-$(kconfig):
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
-
-include/config/%.conf: include/config/auto.conf.cmd $(KCONFIG_CONFIG)
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" syncconfig
-
-# Allow people to just run `make` as before and not force them to configure
-$(KCONFIG_CONFIG):
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" defconfig
-
-# Break the dependency chain for the first run
-include/config/auto.conf.cmd: ;
-
--include $(BASEDIR)/include/config/auto.conf.cmd
-
 .PHONY: cloc
 cloc:
 	$(eval tmpfile := $(shell mktemp))
@@ -307,3 +354,11 @@ cloc:
 	cloc --list-file=$(tmpfile)
 	rm $(tmpfile)
 
+endif #config-build
+
+PHONY += FORCE
+FORCE:
+
+# Declare the contents of the PHONY variable as phony.  We keep that
+# information in a variable so we can use it in if_changed and friends.
+.PHONY: $(PHONY)
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 03/16] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS)
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
  2020-04-21 16:11 ` [XEN PATCH v5 01/16] build,xsm: Fix multiple call Anthony PERARD
  2020-04-21 16:11 ` [XEN PATCH v5 02/16] xen/build: include include/config/auto.conf in main Makefile Anthony PERARD
@ 2020-04-21 16:11 ` Anthony PERARD
  2020-04-24  9:20   ` Julien Grall
  2020-04-21 16:11 ` [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS Anthony PERARD
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:11 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Tim Deegan, Jan Beulich,
	Anthony PERARD, Volodymyr Babchuk, Roger Pau Monné

In a later patch ("xen/build: have the root Makefile generates the
CFLAGS), we want to generate the CFLAGS in xen/Makefile, then export
it and have Rules.mk use a CFLAGS from the environment variables. That
changes the flavor of the CFLAGS and flags intended for one target
(like -D__OBJECT_FILE__ and -M%) gets propagated and duplicated. So we
start by moving such flags out of $(CFLAGS) and into $(c_flags) which
is to be modified by only Rules.mk.

__OBJECT_FILE__ is only used by arch/x86/mm/*.c files, so having it in
$(c_flags) is enough, we don't need it in $(a_flags).

For include/Makefile and as-insn we can keep using CFLAGS, but since
it doesn't have -M* flags anymore there is no need to filter them out.

The XEN_BUILD_EFI tests in arch/x86/Makefile was filtering out
CFLAGS-y, but according to dd40177c1bc8 ("x86-64/EFI: add CFLAGS to
check compile"), it was done to filter out -MF. CFLAGS doesn't
have those flags anymore, so no filtering is needed.

This is inspired by the way Kbuild generates CFLAGS for each targets.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---

Notes:
    v4:
    - drop change in as-insn macro, and keep filtering-out -M% %.d
    
    v3:
    - include/Makefile: Keep using CFLAGS, but since it doesn't have -M*
      flags anymore, no need to filter it.
    - Write c_flags and a_flags on a single line.
    - arch/x86/Makefile: remove the filter-out of dependency flags
      they are remove from CFLAGS anyway.
      (was intended to be done in xen/build: have the root Makefile
      generates the CFLAGS originally, move the change to this patch).
    - also modify as-insn as it is now xen/ only.

 xen/Rules.mk                    | 23 +++++++++++------------
 xen/arch/arm/Makefile           |  4 ++--
 xen/arch/x86/Makefile           |  6 +++---
 xen/arch/x86/mm/Makefile        |  6 +++---
 xen/arch/x86/mm/hap/Makefile    |  6 +++---
 xen/arch/x86/mm/shadow/Makefile |  6 +++---
 xen/include/Makefile            |  2 +-
 7 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 9079df7978a7..3408a35dbf53 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -57,7 +57,6 @@ CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
 $(call cc-option-add,CFLAGS,CC,-Wvla)
 CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h
 CFLAGS-$(CONFIG_DEBUG_INFO) += -g
-CFLAGS += '-D__OBJECT_FILE__="$@"'
 
 ifneq ($(CONFIG_CC_IS_CLANG),y)
 # Clang doesn't understand this command line argument, and doesn't appear to
@@ -70,9 +69,6 @@ AFLAGS += -D__ASSEMBLY__
 
 ALL_OBJS := $(ALL_OBJS-y)
 
-# Get gcc to generate the dependencies for us.
-CFLAGS-y += -MMD -MP -MF $(@D)/.$(@F).d
-
 CFLAGS += $(CFLAGS-y)
 # allow extra CFLAGS externally via EXTRA_CFLAGS_XEN_CORE
 CFLAGS += $(EXTRA_CFLAGS_XEN_CORE)
@@ -146,9 +142,12 @@ endif
 # Always build obj-bin files as binary even if they come from C source. 
 $(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS))
 
+c_flags = -MMD -MP -MF $(@D)/.$(@F).d $(CFLAGS) '-D__OBJECT_FILE__="$@"'
+a_flags = -MMD -MP -MF $(@D)/.$(@F).d $(AFLAGS)
+
 built_in.o: $(obj-y) $(extra-y)
 ifeq ($(obj-y),)
-	$(CC) $(CFLAGS) -c -x c /dev/null -o $@
+	$(CC) $(c_flags) -c -x c /dev/null -o $@
 else
 ifeq ($(CONFIG_LTO),y)
 	$(LD_LTO) -r -o $@ $(filter-out $(extra-y),$^)
@@ -159,7 +158,7 @@ endif
 
 built_in_bin.o: $(obj-bin-y) $(extra-y)
 ifeq ($(obj-bin-y),)
-	$(CC) $(AFLAGS) -c -x assembler /dev/null -o $@
+	$(CC) $(a_flags) -c -x assembler /dev/null -o $@
 else
 	$(LD) $(LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
 endif
@@ -178,7 +177,7 @@ SRCPATH := $(patsubst $(BASEDIR)/%,%,$(CURDIR))
 
 %.o: %.c Makefile
 ifeq ($(CONFIG_ENFORCE_UNIQUE_SYMBOLS),y)
-	$(CC) $(CFLAGS) -c $< -o $(@D)/.$(@F).tmp -MQ $@
+	$(CC) $(c_flags) -c $< -o $(@D)/.$(@F).tmp -MQ $@
 ifeq ($(CONFIG_CC_IS_CLANG),y)
 	$(OBJCOPY) --redefine-sym $<=$(SRCPATH)/$< $(@D)/.$(@F).tmp $@
 else
@@ -186,11 +185,11 @@ else
 endif
 	rm -f $(@D)/.$(@F).tmp
 else
-	$(CC) $(CFLAGS) -c $< -o $@
+	$(CC) $(c_flags) -c $< -o $@
 endif
 
 %.o: %.S Makefile
-	$(CC) $(AFLAGS) -c $< -o $@
+	$(CC) $(a_flags) -c $< -o $@
 
 $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o Makefile
 	$(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p;}' | while read idx name sz rest; do \
@@ -205,12 +204,12 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o Makefile
 	$(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
 
 %.i: %.c Makefile
-	$(CPP) $(filter-out -Wa$(comma)%,$(CFLAGS)) $< -o $@
+	$(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) $< -o $@
 
 %.s: %.c Makefile
-	$(CC) $(filter-out -Wa$(comma)%,$(CFLAGS)) -S $< -o $@
+	$(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@
 
 %.s: %.S Makefile
-	$(CPP) $(filter-out -Wa$(comma)%,$(AFLAGS)) $< -o $@
+	$(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@
 
 -include $(DEPS_INCLUDE)
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 7273f356f190..913f6cdeed3f 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -120,10 +120,10 @@ $(TARGET)-syms: prelink.o xen.lds
 	rm -f $(@D)/.$(@F).[0-9]*
 
 asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c
-	$(CC) $(filter-out -flto,$(CFLAGS)) -S -o $@ $<
+	$(CC) $(filter-out -flto,$(c_flags)) -S -o $@ $<
 
 xen.lds: xen.lds.S
-	$(CC) -P -E -Ui386 $(AFLAGS) -o $@ $<
+	$(CC) -P -E -Ui386 $(a_flags) -o $@ $<
 	sed -e 's/xen\.lds\.o:/xen\.lds:/g' <.xen.lds.d >.xen.lds.d.new
 	mv -f .xen.lds.d.new .xen.lds.d
 
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index e954edbc2e0a..1405525105d9 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -168,7 +168,7 @@ EFI_LDFLAGS += --major-os-version=2 --minor-os-version=0
 EFI_LDFLAGS += --major-subsystem-version=2 --minor-subsystem-version=0
 
 # Check if the compiler supports the MS ABI.
-export XEN_BUILD_EFI := $(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
+export XEN_BUILD_EFI := $(shell $(CC) $(CFLAGS) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
 # Check if the linker supports PE.
 XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep --subsystem=10 -o efi/check.efi efi/check.o 2>/dev/null && echo y))
 CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
@@ -223,7 +223,7 @@ efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: $(B
 efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: ;
 
 asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h
-	$(CC) $(filter-out -Wa$(comma)% -flto,$(CFLAGS)) -S -o $@ $<
+	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -o $@ $<
 
 asm-macros.i: CFLAGS += -D__ASSEMBLY__ -P
 
@@ -240,7 +240,7 @@ $(BASEDIR)/include/asm-x86/asm-macros.h: asm-macros.i Makefile
 
 efi.lds: AFLAGS += -DEFI
 xen.lds efi.lds: xen.lds.S
-	$(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(AFLAGS)) -o $@ $<
+	$(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<
 	sed -e 's/.*\.lds\.o:/$(@F):/g' <.$(@F).d >.$(@F).d.new
 	mv -f .$(@F).d.new .$(@F).d
 
diff --git a/xen/arch/x86/mm/Makefile b/xen/arch/x86/mm/Makefile
index d87dc0aa6eeb..a2431fde6bb4 100644
--- a/xen/arch/x86/mm/Makefile
+++ b/xen/arch/x86/mm/Makefile
@@ -12,10 +12,10 @@ obj-$(CONFIG_HVM) += p2m-ept.o p2m-pod.o
 obj-y += paging.o
 
 guest_walk_%.o: guest_walk.c Makefile
-	$(CC) $(CFLAGS) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
+	$(CC) $(c_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
 
 guest_walk_%.i: guest_walk.c Makefile
-	$(CPP) $(filter-out -Wa$(comma)%,$(CFLAGS)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
+	$(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
 
 guest_walk_%.s: guest_walk.c Makefile
-	$(CC) $(filter-out -Wa$(comma)%,$(CFLAGS)) -DGUEST_PAGING_LEVELS=$* -S $< -o $@
+	$(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -S $< -o $@
diff --git a/xen/arch/x86/mm/hap/Makefile b/xen/arch/x86/mm/hap/Makefile
index b14a9aff93d2..22e7ad54bd33 100644
--- a/xen/arch/x86/mm/hap/Makefile
+++ b/xen/arch/x86/mm/hap/Makefile
@@ -6,10 +6,10 @@ obj-y += nested_hap.o
 obj-y += nested_ept.o
 
 guest_walk_%level.o: guest_walk.c Makefile
-	$(CC) $(CFLAGS) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
+	$(CC) $(c_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
 
 guest_walk_%level.i: guest_walk.c Makefile
-	$(CPP) $(filter-out -Wa$(comma)%,$(CFLAGS)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
+	$(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
 
 guest_walk_%level.s: guest_walk.c Makefile
-	$(CC) $(filter-out -Wa$(comma)%,$(CFLAGS)) -DGUEST_PAGING_LEVELS=$* -S $< -o $@
+	$(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -S $< -o $@
diff --git a/xen/arch/x86/mm/shadow/Makefile b/xen/arch/x86/mm/shadow/Makefile
index ff03a9937f9b..23d3ff10802c 100644
--- a/xen/arch/x86/mm/shadow/Makefile
+++ b/xen/arch/x86/mm/shadow/Makefile
@@ -7,10 +7,10 @@ obj-y += none.o
 endif
 
 guest_%.o: multi.c Makefile
-	$(CC) $(CFLAGS) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
+	$(CC) $(c_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
 
 guest_%.i: multi.c Makefile
-	$(CPP) $(filter-out -Wa$(comma)%,$(CFLAGS)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
+	$(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
 
 guest_%.s: multi.c Makefile
-	$(CC) $(filter-out -Wa$(comma)%,$(CFLAGS)) -DGUEST_PAGING_LEVELS=$* -S $< -o $@
+	$(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -S $< -o $@
diff --git a/xen/include/Makefile b/xen/include/Makefile
index 433bad9055b2..a488a98d8bb7 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -64,7 +64,7 @@ compat/%.h: compat/%.i Makefile $(BASEDIR)/tools/compat-build-header.py
 	mv -f $@.new $@
 
 compat/%.i: compat/%.c Makefile
-	$(CPP) $(filter-out -Wa$(comma)% -M% %.d -include %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
+	$(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
 
 compat/%.c: public/%.h xlat.lst Makefile $(BASEDIR)/tools/compat-build-source.py
 	mkdir -p $(@D)
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
                   ` (2 preceding siblings ...)
  2020-04-21 16:11 ` [XEN PATCH v5 03/16] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS) Anthony PERARD
@ 2020-04-21 16:11 ` Anthony PERARD
  2020-04-23 16:40   ` Jan Beulich
                     ` (2 more replies)
  2020-04-21 16:11 ` [XEN PATCH v5 05/16] build: Introduce documentation for xen Makefiles Anthony PERARD
                   ` (11 subsequent siblings)
  15 siblings, 3 replies; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:11 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Jan Beulich, Anthony PERARD,
	Daniel De Graaf, Volodymyr Babchuk, Roger Pau Monné

Instead of generating the CFLAGS in Rules.mk everytime we enter a new
subdirectory, we are going to generate most of them a single time, and
export the result in the environment so that Rules.mk can use it.  The
only flags left to be generated are the ones that depend on the
targets, but the variable $(c_flags) takes care of that.

Arch specific CFLAGS are generated by a new file "arch/*/arch.mk"
which is included by the root Makefile.

We export the *FLAGS via the environment variables XEN_*FLAGS because
Rules.mk still includes Config.mk and would add duplicated flags to
CFLAGS.

When running Rules.mk in the root directory (xen/), the variable
`root-make-done' is set, so `need-config' will remain undef and so the
root Makefile will not generate the cflags again.

We can't use CFLAGS in subdirectories to add flags to particular
targets, instead start to use CFLAGS-y. Idem for AFLAGS.
So there are two different CFLAGS-y, the one in xen/Makefile (and
arch.mk), and the one in subdirs that Rules.mk is going to use.
We can't add to XEN_CFLAGS because it is exported, so making change to
it might be propagated to subdirectory which isn't intended.

Some style change are introduced in this patch:
    when LDFLAGS_DIRECT is included in LDFLAGS
    use of CFLAGS-$(CONFIG_INDIRECT_THUNK) instead of ifeq().

The LTO change hasn't been tested properly, as LTO is marked as
broken.

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

Notes:
    v5:
    - typos
    - remove -flto from XEN_CFLAGS instead of calling lto broken and
      commenting out lto stuff.
    
    v4:
    - typos
    - Adding $(AFLAGS-y) to $(AFLAGS)
    
    v3:
    - squash "xen/build: introduce ccflags-y and CFLAGS_$@" here, with
      those changes:
        - rename ccflags-y to simply CFLAGS-y and start using AFLAGS-y in
          subdirs.
        - remove CFLAGS_$@, we don't need it yet.
        - fix build of xen.lds and efi.lds which needed -D to be a_flags
    - remove arch_ccflags, and modify c_flags directly
      with that change, reorder c_flags, so that target specific flags are last.
    - remove HAVE_AS_QUOTED_SYM from envvar and check XEN_CFLAGS to find if
      it's there when adding -D__OBJECT_LABEL__.
    - fix missing some flags in AFLAGS
      (like -fshort-wchar in xen/arch/x86/efi/Makefile,
       and -D__OBJECT_LABEL__ and CFLAGS-stack-boundary)
    - keep COV_FLAGS generation in Rules.mk since it doesn't invovle to
      call CC
    - fix clang test for "asm()-s support .include." (in a new patch done
      ahead)
    - include Kconfig.include in xen/Makefile because as-option-add is
      defined there now.

 xen/Makefile                       | 58 +++++++++++++++++++
 xen/Rules.mk                       | 73 ++++++------------------
 xen/arch/arm/Makefile              | 10 ++--
 xen/arch/arm/Rules.mk              | 23 --------
 xen/arch/arm/{Rules.mk => arch.mk} |  5 --
 xen/arch/arm/efi/Makefile          |  2 +-
 xen/arch/x86/Makefile              | 24 ++++----
 xen/arch/x86/Rules.mk              | 91 ++----------------------------
 xen/arch/x86/{Rules.mk => arch.mk} | 17 ++----
 xen/arch/x86/efi/Makefile          |  2 +-
 xen/common/libelf/Makefile         |  4 +-
 xen/common/libfdt/Makefile         |  4 +-
 xen/include/Makefile               |  2 +-
 xen/xsm/flask/Makefile             |  2 +-
 xen/xsm/flask/ss/Makefile          |  2 +-
 15 files changed, 114 insertions(+), 205 deletions(-)
 copy xen/arch/arm/{Rules.mk => arch.mk} (85%)
 copy xen/arch/x86/{Rules.mk => arch.mk} (87%)

diff --git a/xen/Makefile b/xen/Makefile
index 643c689658f3..2a689b26a2ba 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -118,6 +118,64 @@ $(KCONFIG_CONFIG):
 include/config/%.conf include/config/%.conf.cmd: $(KCONFIG_CONFIG)
 	$(MAKE) $(kconfig) syncconfig
 
+ifeq ($(CONFIG_DEBUG),y)
+CFLAGS += -O1
+else
+CFLAGS += -O2
+endif
+
+ifeq ($(CONFIG_FRAME_POINTER),y)
+CFLAGS += -fno-omit-frame-pointer
+else
+CFLAGS += -fomit-frame-pointer
+endif
+
+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 $(BASEDIR)/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
+endif
+
+AFLAGS += -D__ASSEMBLY__
+
+CFLAGS += $(CFLAGS-y)
+# allow extra CFLAGS externally via EXTRA_CFLAGS_XEN_CORE
+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)
+
+# LDFLAGS are only passed directly to $(LD)
+LDFLAGS += $(LDFLAGS_DIRECT) $(LDFLAGS-y)
+
+ifeq ($(CONFIG_UBSAN),y)
+CFLAGS_UBSAN := -fsanitize=undefined
+else
+CFLAGS_UBSAN :=
+endif
+
+ifeq ($(CONFIG_LTO),y)
+CFLAGS += -flto
+LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so
+endif
+
+include $(BASEDIR)/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_LDFLAGS := $(LDFLAGS)
+export CFLAGS_UBSAN
+
 endif # need-config
 
 .PHONY: build install uninstall clean distclean MAP
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 3408a35dbf53..8e4b9e3a4a82 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -38,59 +38,17 @@ ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
 
 # Initialise some variables
-CFLAGS_UBSAN :=
-
-ifeq ($(CONFIG_DEBUG),y)
-CFLAGS += -O1
-else
-CFLAGS += -O2
-endif
-
-ifeq ($(CONFIG_FRAME_POINTER),y)
-CFLAGS += -fno-omit-frame-pointer
-else
-CFLAGS += -fomit-frame-pointer
-endif
-
-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 $(BASEDIR)/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 an suitable alternative.  The resulting compiled binary does function,
-# but has an excessively large symbol table.
-CFLAGS += -Wa,--strip-local-absolute
-endif
-
-AFLAGS += -D__ASSEMBLY__
+CFLAGS-y :=
+AFLAGS-y :=
 
 ALL_OBJS := $(ALL_OBJS-y)
 
-CFLAGS += $(CFLAGS-y)
-# allow extra CFLAGS externally via EXTRA_CFLAGS_XEN_CORE
-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))
-
-# LDFLAGS are only passed directly to $(LD)
-LDFLAGS += $(LDFLAGS_DIRECT)
-
-LDFLAGS += $(LDFLAGS-y)
-
 SPECIAL_DATA_SECTIONS := rodata $(foreach a,1 2 4 8 16, \
                                             $(foreach w,1 2 4, \
                                                         rodata.str$(w).$(a)) \
                                             rodata.cst$(a)) \
                          $(foreach r,rel rel.ro,data.$(r).local)
 
-include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
-
 include Makefile
 
 define gendep
@@ -107,7 +65,7 @@ $(foreach o,$(filter-out %/,$(obj-y) $(obj-bin-y) $(extra-y)),$(eval $(call gend
 subdir-y := $(subdir-y) $(filter %/, $(obj-y))
 obj-y    := $(patsubst %/, %/built_in.o, $(obj-y))
 
-$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
+$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += -DINIT_SECTIONS_ONLY
 
 ifeq ($(CONFIG_COVERAGE),y)
 ifeq ($(CONFIG_CC_IS_CLANG),y)
@@ -115,19 +73,16 @@ ifeq ($(CONFIG_CC_IS_CLANG),y)
 else
     COV_FLAGS := -fprofile-arcs -ftest-coverage
 endif
-$(filter-out %.init.o $(nocov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += $(COV_FLAGS)
+$(filter-out %.init.o $(nocov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += $(COV_FLAGS)
 endif
 
 ifeq ($(CONFIG_UBSAN),y)
-CFLAGS_UBSAN += -fsanitize=undefined
 # Any -fno-sanitize= options need to come after any -fsanitize= options
 $(filter-out %.init.o $(noubsan-y),$(obj-y) $(obj-bin-y) $(extra-y)): \
-CFLAGS += $(filter-out -fno-%,$(CFLAGS_UBSAN)) $(filter -fno-%,$(CFLAGS_UBSAN))
+CFLAGS-y += $(filter-out -fno-%,$(CFLAGS_UBSAN)) $(filter -fno-%,$(CFLAGS_UBSAN))
 endif
 
 ifeq ($(CONFIG_LTO),y)
-CFLAGS += -flto
-LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so
 # Would like to handle all object files as bitcode, but objects made from
 # pure asm are in a different format and have to be collected separately.
 # Mirror the directory tree, collecting them as built_in_bin.o.
@@ -140,10 +95,18 @@ obj-bin-y :=
 endif
 
 # Always build obj-bin files as binary even if they come from C source. 
-$(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS))
+$(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS))
+
+# Calculation of flags, first the generic flags, then the arch specific flags,
+# and last the flags modified for a target or a directory.
+
+c_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_CFLAGS) '-D__OBJECT_FILE__="$@"'
+a_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_AFLAGS)
+
+include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
 
-c_flags = -MMD -MP -MF $(@D)/.$(@F).d $(CFLAGS) '-D__OBJECT_FILE__="$@"'
-a_flags = -MMD -MP -MF $(@D)/.$(@F).d $(AFLAGS)
+c_flags += $(CFLAGS-y)
+a_flags += $(CFLAGS-y) $(AFLAGS-y)
 
 built_in.o: $(obj-y) $(extra-y)
 ifeq ($(obj-y),)
@@ -152,7 +115,7 @@ else
 ifeq ($(CONFIG_LTO),y)
 	$(LD_LTO) -r -o $@ $(filter-out $(extra-y),$^)
 else
-	$(LD) $(LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
 endif
 endif
 
@@ -160,7 +123,7 @@ built_in_bin.o: $(obj-bin-y) $(extra-y)
 ifeq ($(obj-bin-y),)
 	$(CC) $(a_flags) -c -x assembler /dev/null -o $@
 else
-	$(LD) $(LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
 endif
 
 # Force execution of pattern rules (for which PHONY cannot be directly used).
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 913f6cdeed3f..9f1ab2335756 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -95,24 +95,24 @@ prelink_lto.o: $(ALL_OBJS)
 
 # Link it with all the binary objects
 prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
-	$(LD) $(LDFLAGS) -r -o $@ $^
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
 else
 prelink.o: $(ALL_OBJS)
-	$(LD) $(LDFLAGS) -r -o $@ $^
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
 endif
 
 $(TARGET)-syms: prelink.o xen.lds
-	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
+	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o \
 	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
 	$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
 		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0.S
 	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
-	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
+	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o \
 	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
 	$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
 		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1.S
 	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
-	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
+	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
 	    $(@D)/.$(@F).1.o -o $@
 	$(NM) -pa --format=sysv $(@D)/$(@F) \
 		| $(BASEDIR)/tools/symbols --xensyms --sysv --sort \
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index 3ad284aa71a4..e69de29bb2d1 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -1,23 +0,0 @@
-########################################
-# arm-specific definitions
-
-#
-# If you change any of these configuration options then you must
-# 'make clean' before rebuilding.
-#
-
-CFLAGS += -I$(BASEDIR)/include
-
-$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
-$(call cc-option-add,CFLAGS,CC,-Wnested-externs)
-
-# Prevent floating-point variables from creeping into Xen.
-CFLAGS-$(CONFIG_ARM_32) += -msoft-float
-CFLAGS-$(CONFIG_ARM_32) += -mcpu=cortex-a15
-
-CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
-CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
-
-ifneq ($(filter command line environment,$(origin CONFIG_EARLY_PRINTK)),)
-    $(error You must use 'make menuconfig' to enable/disable early printk now)
-endif
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/arch.mk
similarity index 85%
copy from xen/arch/arm/Rules.mk
copy to xen/arch/arm/arch.mk
index 3ad284aa71a4..c8186f58288d 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/arch.mk
@@ -1,11 +1,6 @@
 ########################################
 # arm-specific definitions
 
-#
-# If you change any of these configuration options then you must
-# 'make clean' before rebuilding.
-#
-
 CFLAGS += -I$(BASEDIR)/include
 
 $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile
index d34c9168914a..e3ff2c3f283c 100644
--- a/xen/arch/arm/efi/Makefile
+++ b/xen/arch/arm/efi/Makefile
@@ -1,4 +1,4 @@
-CFLAGS += -fshort-wchar
+CFLAGS-y += -fshort-wchar
 
 obj-y +=  boot.init.o runtime.o
 obj-$(CONFIG_ACPI) +=  efi-dom0.init.o
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 1405525105d9..a805e9982e85 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -121,32 +121,32 @@ prelink-efi_lto.o: $(ALL_OBJS) efi/runtime.o efi/compat.o
 
 # Link it with all the binary objects
 prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
-	$(LD) $(LDFLAGS) -r -o $@ $^
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
 
 prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink-efi_lto.o efi/boot.init.o
-	$(LD) $(LDFLAGS) -r -o $@ $^
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
 else
 prelink.o: $(ALL_OBJS)
-	$(LD) $(LDFLAGS) -r -o $@ $^
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
 
 prelink-efi.o: $(ALL_OBJS) efi/boot.init.o efi/runtime.o efi/compat.o
-	$(LD) $(LDFLAGS) -r -o $@ $(filter-out %/efi/built_in.o,$^)
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out %/efi/built_in.o,$^)
 endif
 
 $(TARGET)-syms: prelink.o xen.lds
-	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
+	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
 	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
 	$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
 		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort \
 		>$(@D)/.$(@F).0.S
 	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
-	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
+	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
 	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
 	$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
 		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort $(syms-warn-dup-y) \
 		>$(@D)/.$(@F).1.S
 	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
-	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
+	$(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
 	    $(@D)/.$(@F).1.o -o $@
 	$(NM) -pa --format=sysv $(@D)/$(@F) \
 		| $(BASEDIR)/tools/symbols --xensyms --sysv --sort \
@@ -159,7 +159,7 @@ note.o: $(TARGET)-syms
 		--rename-section=.data=.note.gnu.build-id -S $@.bin $@
 	rm -f $@.bin
 
-EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(LDFLAGS)) --subsystem=10
+EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(XEN_LDFLAGS)) --subsystem=10
 EFI_LDFLAGS += --image-base=$(1) --stack=0,0 --heap=0,0 --strip-debug
 EFI_LDFLAGS += --section-alignment=0x200000 --file-alignment=0x20
 EFI_LDFLAGS += --major-image-version=$(XEN_VERSION)
@@ -168,7 +168,7 @@ EFI_LDFLAGS += --major-os-version=2 --minor-os-version=0
 EFI_LDFLAGS += --major-subsystem-version=2 --minor-subsystem-version=0
 
 # Check if the compiler supports the MS ABI.
-export XEN_BUILD_EFI := $(shell $(CC) $(CFLAGS) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
+export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
 # Check if the linker supports PE.
 XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep --subsystem=10 -o efi/check.efi efi/check.o 2>/dev/null && echo y))
 CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
@@ -178,7 +178,7 @@ $(TARGET).efi: ALT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A ALT_
 
 ifneq ($(build_id_linker),)
 ifeq ($(call ld-ver-build-id,$(LD) $(filter -m%,$(EFI_LDFLAGS))),y)
-CFLAGS += -DBUILD_ID_EFI
+CFLAGS-y += -DBUILD_ID_EFI
 EFI_LDFLAGS += $(build_id_linker)
 note_file := efi/buildid.o
 # NB: this must be the last input in the linker call, because inputs following
@@ -225,7 +225,7 @@ efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: ;
 asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h
 	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -o $@ $<
 
-asm-macros.i: CFLAGS += -D__ASSEMBLY__ -P
+asm-macros.i: CFLAGS-y += -D__ASSEMBLY__ -P
 
 $(BASEDIR)/include/asm-x86/asm-macros.h: asm-macros.i Makefile
 	echo '#if 0' >$@.new
@@ -238,7 +238,7 @@ $(BASEDIR)/include/asm-x86/asm-macros.h: asm-macros.i Makefile
 	echo '#endif' >>$@.new
 	$(call move-if-changed,$@.new,$@)
 
-efi.lds: AFLAGS += -DEFI
+efi.lds: AFLAGS-y += -DEFI
 xen.lds efi.lds: xen.lds.S
 	$(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<
 	sed -e 's/.*\.lds\.o:/$(@F):/g' <.$(@F).d >.$(@F).d.new
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 4b7ab784670c..56fe22c979ea 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -1,89 +1,10 @@
 ########################################
 # x86-specific definitions
 
-XEN_IMG_OFFSET := 0x200000
-
-CFLAGS += -I$(BASEDIR)/include
-CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
-CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
-CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
-CFLAGS += '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))'
-
-# Prevent floating-point variables from creeping into Xen.
-CFLAGS += -msoft-float
-
-ifeq ($(CONFIG_CC_IS_CLANG),y)
-# Note: Any test which adds -no-integrated-as will cause subsequent tests to
-# succeed, and not trigger further additions.
-#
-# The tests to select whether the integrated assembler is usable need to happen
-# before testing any assembler features, or else the result of the tests would
-# be stale if the integrated assembler is not used.
-
-# Older clang's built-in assembler doesn't understand .skip with labels:
-# https://bugs.llvm.org/show_bug.cgi?id=27369
-$(call as-option-add,CFLAGS,CC,".L0: .L1: .skip (.L1 - .L0)",,\
-                     -no-integrated-as)
-
-# Check whether clang asm()-s support .include.
-$(call as-option-add,CFLAGS,CC,".include \"asm-x86/indirect_thunk_asm.h\"",,\
-                     -no-integrated-as)
-
-# Check whether clang keeps .macro-s between asm()-s:
-# https://bugs.llvm.org/show_bug.cgi?id=36110
-$(call as-option-add,CFLAGS,CC,\
-                     ".macro FOO;.endm"$$(close); asm volatile $$(open)".macro FOO;.endm",\
-                     -no-integrated-as)
-endif
-
-$(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,"clwb (%rax)",-DHAVE_AS_CLWB)
-$(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1", \
-                     -U__OBJECT_LABEL__ -DHAVE_AS_QUOTED_SYM \
-                     '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$$@')
-$(call as-option-add,CFLAGS,CC,"invpcid (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID)
-
-# GAS's idea of true is -1.  Clang's idea is 1
-$(call as-option-add,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,\
-    ".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOPS_DIRECTIVE)
-
-CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables
-
-# Xen doesn't use SSE interally.  If the compiler supports it, also skip the
-# SSE setup for variadic function calls.
-CFLAGS += -mno-sse $(call cc-option,$(CC),-mskip-rax-setup)
-
-# Compile with thunk-extern, indirect-branch-register if avaiable.
-ifeq ($(CONFIG_INDIRECT_THUNK),y)
-CFLAGS += -mindirect-branch=thunk-extern -mindirect-branch-register
-CFLAGS += -fno-jump-tables
+ifneq ($(filter -DHAVE_AS_QUOTED_SYM,$(XEN_CFLAGS)),)
+object_label_flags = '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$@'
+else
+object_label_flags = '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))'
 endif
-
-# If supported by the compiler, reduce stack alignment to 8 bytes. But allow
-# this to be overridden elsewhere.
-$(call cc-option-add,CFLAGS-stack-boundary,CC,-mpreferred-stack-boundary=3)
-CFLAGS += $(CFLAGS-stack-boundary)
-
-ifeq ($(CONFIG_UBSAN),y)
-# Don't enable alignment sanitisation.  x86 has efficient unaligned accesses,
-# and various things (ACPI tables, hypercall pages, stubs, etc) are wont-fix.
-# It also causes an as-yet-unidentified crash on native boot before the
-# console starts.
-$(call cc-option-add,CFLAGS_UBSAN,CC,-fno-sanitize=alignment)
-endif
-
-# Set up the assembler include path properly for older toolchains.
-CFLAGS += -Wa,-I$(BASEDIR)/include
-
+c_flags += $(object_label_flags) $(CFLAGS-stack-boundary)
+a_flags += $(object_label_flags) $(CFLAGS-stack-boundary)
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/arch.mk
similarity index 87%
copy from xen/arch/x86/Rules.mk
copy to xen/arch/x86/arch.mk
index 4b7ab784670c..2a51553edb3c 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/arch.mk
@@ -1,13 +1,12 @@
 ########################################
 # x86-specific definitions
 
-XEN_IMG_OFFSET := 0x200000
+export XEN_IMG_OFFSET := 0x200000
 
 CFLAGS += -I$(BASEDIR)/include
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
 CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
-CFLAGS += '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))'
 
 # Prevent floating-point variables from creeping into Xen.
 CFLAGS += -msoft-float
@@ -46,9 +45,7 @@ $(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,"clwb (%rax)",-DHAVE_AS_CLWB)
-$(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1", \
-                     -U__OBJECT_LABEL__ -DHAVE_AS_QUOTED_SYM \
-                     '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$$@')
+$(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)
 
 # GAS's idea of true is -1.  Clang's idea is 1
@@ -66,15 +63,14 @@ CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables
 CFLAGS += -mno-sse $(call cc-option,$(CC),-mskip-rax-setup)
 
 # Compile with thunk-extern, indirect-branch-register if avaiable.
-ifeq ($(CONFIG_INDIRECT_THUNK),y)
-CFLAGS += -mindirect-branch=thunk-extern -mindirect-branch-register
-CFLAGS += -fno-jump-tables
-endif
+CFLAGS-$(CONFIG_INDIRECT_THUNK) += -mindirect-branch=thunk-extern
+CFLAGS-$(CONFIG_INDIRECT_THUNK) += -mindirect-branch-register
+CFLAGS-$(CONFIG_INDIRECT_THUNK) += -fno-jump-tables
 
 # If supported by the compiler, reduce stack alignment to 8 bytes. But allow
 # this to be overridden elsewhere.
 $(call cc-option-add,CFLAGS-stack-boundary,CC,-mpreferred-stack-boundary=3)
-CFLAGS += $(CFLAGS-stack-boundary)
+export CFLAGS-stack-boundary
 
 ifeq ($(CONFIG_UBSAN),y)
 # Don't enable alignment sanitisation.  x86 has efficient unaligned accesses,
@@ -86,4 +82,3 @@ endif
 
 # Set up the assembler include path properly for older toolchains.
 CFLAGS += -Wa,-I$(BASEDIR)/include
-
diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index 4bc0a196e9ca..490d791aae2d 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -1,4 +1,4 @@
-CFLAGS += -fshort-wchar
+CFLAGS-y += -fshort-wchar
 
 %.o: %.ihex
 	$(OBJCOPY) -I ihex -O binary $< $@
diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile
index 3d9e38f27e65..464c448d9d37 100644
--- a/xen/common/libelf/Makefile
+++ b/xen/common/libelf/Makefile
@@ -3,10 +3,10 @@ nocov-y += libelf.o
 
 SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
 
-CFLAGS += -Wno-pointer-sign
+CFLAGS-y += -Wno-pointer-sign
 
 libelf.o: libelf-temp.o Makefile
 	$(OBJCOPY) $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
 
 libelf-temp.o: libelf-tools.o libelf-loader.o libelf-dominfo.o #libelf-relocate.o
-	$(LD) $(LDFLAGS) -r -o $@ $^
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
diff --git a/xen/common/libfdt/Makefile b/xen/common/libfdt/Makefile
index c075bbf5462a..e2a5e59380a0 100644
--- a/xen/common/libfdt/Makefile
+++ b/xen/common/libfdt/Makefile
@@ -5,10 +5,10 @@ SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
 obj-y += libfdt.o
 nocov-y += libfdt.o
 
-CFLAGS += -I$(BASEDIR)/include/xen/libfdt/
+CFLAGS-y += -I$(BASEDIR)/include/xen/libfdt/
 
 libfdt.o: libfdt-temp.o Makefile
 	$(OBJCOPY) $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
 
 libfdt-temp.o: $(LIBFDT_OBJS)
-	$(LD) $(LDFLAGS) -r -o $@ $^
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
diff --git a/xen/include/Makefile b/xen/include/Makefile
index a488a98d8bb7..2a10725d689b 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -64,7 +64,7 @@ compat/%.h: compat/%.i Makefile $(BASEDIR)/tools/compat-build-header.py
 	mv -f $@.new $@
 
 compat/%.i: compat/%.c Makefile
-	$(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
+	$(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(XEN_CFLAGS)) $(cppflags-y) -o $@ $<
 
 compat/%.c: public/%.h xlat.lst Makefile $(BASEDIR)/tools/compat-build-source.py
 	mkdir -p $(@D)
diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index f001bb18d4ed..6db396347b1f 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -4,7 +4,7 @@ obj-y += flask_op.o
 
 obj-y += ss/
 
-CFLAGS += -I./include
+CFLAGS-y += -I./include
 
 AWK = awk
 
diff --git a/xen/xsm/flask/ss/Makefile b/xen/xsm/flask/ss/Makefile
index 046ce8f53326..d32b9e07138e 100644
--- a/xen/xsm/flask/ss/Makefile
+++ b/xen/xsm/flask/ss/Makefile
@@ -8,4 +8,4 @@ obj-y += services.o
 obj-y += conditional.o
 obj-y += mls.o
 
-CFLAGS += -I../include
+CFLAGS-y += -I../include
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 05/16] build: Introduce documentation for xen Makefiles
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
                   ` (3 preceding siblings ...)
  2020-04-21 16:11 ` [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS Anthony PERARD
@ 2020-04-21 16:11 ` Anthony PERARD
  2020-04-21 16:11 ` [XEN PATCH v5 06/16] xen/build: introduce if_changed and if_changed_rule Anthony PERARD
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:11 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Jan Beulich, Anthony PERARD

This start explainning the variables that can be used in the many
Makefiles in xen/.

Most of the document copies and modifies text from Linux v5.4 document
linux.git/Documentation/kbuild/makefiles.rst. Modification are mostly
to avoid mentioning kbuild. Thus I've added the SPDX tag which was
only in index.rst in linux.git.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---

Notes:
    v4:
    - new patch

 docs/misc/xen-makefiles/makefiles.rst | 87 +++++++++++++++++++++++++++
 xen/Rules.mk                          |  4 ++
 2 files changed, 91 insertions(+)
 create mode 100644 docs/misc/xen-makefiles/makefiles.rst

diff --git a/docs/misc/xen-makefiles/makefiles.rst b/docs/misc/xen-makefiles/makefiles.rst
new file mode 100644
index 000000000000..a86e3a612d61
--- /dev/null
+++ b/docs/misc/xen-makefiles/makefiles.rst
@@ -0,0 +1,87 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============
+Xen Makefiles
+=============
+
+Documentation for the build system of Xen, found in xen.git/xen/.
+
+Makefile files
+==============
+
+Description of the syntax that can be used in most Makefiles named
+'Makefile'. ('xen/Makefile' isn't part of the description.)
+
+'Makefile's are consumed by 'Rules.mk' when building.
+
+Goal definitions
+----------------
+
+	Goal definitions are the main part (heart) of the Makefile.
+	These lines define the files to be built, any special compilation
+	options, and any subdirectories to be entered recursively.
+
+	The most simple makefile contains one line:
+
+	Example::
+
+		obj-y += foo.o
+
+	This tells the build system that there is one object in that
+	directory, named foo.o. foo.o will be built from foo.c or foo.S.
+
+	The following pattern is often used to have object selected
+	depending on the configuration:
+
+	Example::
+
+		obj-$(CONFIG_FOO) += foo.o
+
+	$(CONFIG_FOO) can evaluates to y.
+	If CONFIG_FOO is not y, then the file will not be compiled nor linked.
+
+Descending down in directories
+------------------------------
+
+	A Makefile is only responsible for building objects in its own
+	directory. Files in subdirectories should be taken care of by
+	Makefiles in these subdirs. The build system will automatically
+	invoke make recursively in subdirectories, provided you let it know of
+	them.
+
+	To do so, obj-y is used.
+	acpi lives in a separate directory, and the Makefile present in
+	drivers/ tells the build system to descend down using the following
+	assignment.
+
+	Example::
+
+		#drivers/Makefile
+		obj-$(CONFIG_ACPI) += acpi/
+
+	If CONFIG_ACPI is set to 'y'
+	the corresponding obj- variable will be set, and the build system
+	will descend down in the apci directory.
+	The build system only uses this information to decide that it needs
+	to visit the directory, it is the Makefile in the subdirectory that
+	specifies what is modular and what is built-in.
+
+	It is good practice to use a `CONFIG_` variable when assigning directory
+	names. This allows the build system to totally skip the directory if the
+	corresponding `CONFIG_` option is 'y'.
+
+Compilation flags
+-----------------
+
+    CFLAGS-y and AFLAGS-y
+	These two flags apply only to the makefile in which they
+	are assigned. They are used for all the normal cc, as and ld
+	invocations happening during a recursive build.
+
+	$(CFLAGS-y) is necessary because the top Makefile owns the
+	variable $(XEN_CFLAGS) and uses it for compilation flags for the
+	entire tree. And the variable $(CFLAGS) is modified by Config.mk
+	which evaluated in every subdirs.
+
+	CFLAGS-y specifies options for compiling with $(CC).
+	AFLAGS-y specifies assembler options.
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 8e4b9e3a4a82..9d1e1ebf5e44 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -1,3 +1,7 @@
+#
+# See docs/misc/xen-makefiles/makefiles.rst on variables that can be used in
+# Makefile and are consumed by Rules.mk
+#
 
 -include $(BASEDIR)/include/config/auto.conf
 
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 06/16] xen/build: introduce if_changed and if_changed_rule
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
                   ` (4 preceding siblings ...)
  2020-04-21 16:11 ` [XEN PATCH v5 05/16] build: Introduce documentation for xen Makefiles Anthony PERARD
@ 2020-04-21 16:11 ` Anthony PERARD
  2020-04-21 16:11 ` [XEN PATCH v5 07/16] xen/build: Start using if_changed Anthony PERARD
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:11 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Jan Beulich, Anthony PERARD

The if_changed macro from Linux, in addition to check if any files
needs an update, check if the command line has changed since the last
invocation. The latter will force a rebuild if any options to the
executable have changed.

if_changed_rule checks dependencies like if_changed, but execute
rule_$(1) instead of cmd_$(1) when a target needs to be rebuilt. A rule_
macro can call more than one cmd_ macro. One of the cmd_ macro in a
rule need to be call using a macro that record the command line, so
cmd_and_record is introduced. It is similar to cmd_and_fixup from
Linux but without a call to fixdep which we don't have yet. (We will
later replace cmd_and_record by cmd_and_fixup.)

Example of a rule_ macro:
define rule_cc_o_c
    $(call cmd_and_record,cc_o_o)
    $(call cmd,objcopy)
endef

This needs one of the call to use cmd_and_record, otherwise no .*.cmd
file will be created, and the target will keep been rebuilt.

In order for if_changed to works correctly, we need to load the .%.cmd
files that the macro generates, this is done by adding targets in to
the $(targets) variable. We use intermediate_targets to add %.init.o
dependency %.o to target since there aren't in obj-y.

We also add $(MAKECMDGOALS) to targets so that when running for
example `make common/memory.i`, make will load the associated .%.cmd
dependency file.

Beside the if_changed*, we import the machinery used for a "beautify
output". The important one is when running make with V=2 which help to
debug the makefiles by printing why a target is been rebuilt, via the
$(echo-why) macro.

if_changed and if_changed_rule aren't used yet.

Most of this code is copied from Linux v5.4, including the
documentation.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---

Notes:
    v5:
    - In documentation, fix some mix tab/space used for indentation,
      convert to all tab.
    - and fix a "note" in "if_changed" where the paragraph was cut in two.
    
    v4:
    - Use := in make whenever possible (instead of =)
    - insert new string in .gitignore somewhere more plausible.
    - import documentation from Linux

 .gitignore                            |   1 +
 docs/misc/xen-makefiles/makefiles.rst |  98 +++++++++++++++++++++++
 xen/Makefile                          |  53 ++++++++++++-
 xen/Rules.mk                          |  33 +++++++-
 xen/scripts/Kbuild.include            | 107 ++++++++++++++++++++++++++
 5 files changed, 290 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index 4ca679ddbc9a..bfa53723b38b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,4 +1,5 @@
 .hg
+.*.cmd
 .*.tmp
 *.orig
 *~
diff --git a/docs/misc/xen-makefiles/makefiles.rst b/docs/misc/xen-makefiles/makefiles.rst
index a86e3a612d61..04bc72601c31 100644
--- a/docs/misc/xen-makefiles/makefiles.rst
+++ b/docs/misc/xen-makefiles/makefiles.rst
@@ -85,3 +85,101 @@ Compilation flags
 
 	CFLAGS-y specifies options for compiling with $(CC).
 	AFLAGS-y specifies assembler options.
+
+
+Build system infrastructure
+===========================
+
+This chapter describe some of the macro used when building Xen.
+
+Macros
+------
+
+
+    if_changed
+	if_changed is the infrastructure used for the following commands.
+
+	Usage::
+
+		target: source(s) FORCE
+			$(call if_changed,ld/objcopy/...)
+
+	When the rule is evaluated, it is checked to see if any files
+	need an update, or the command line has changed since the last
+	invocation. The latter will force a rebuild if any options
+	to the executable have changed.
+	Any target that utilises if_changed must be listed in $(targets),
+	otherwise the command line check will fail, and the target will
+	always be built.
+	if_changed may be used in conjunction with custom commands as
+	defined in "Custom commands".
+
+	Note: It is a typical mistake to forget the FORCE prerequisite.
+	Another common pitfall is that whitespace is sometimes
+	significant; for instance, the below will fail (note the extra space
+	after the comma)::
+
+		target: source(s) FORCE
+
+	**WRONG!**	$(call if_changed, ld/objcopy/...)
+
+	Note:
+		if_changed should not be used more than once per target.
+		It stores the executed command in a corresponding .cmd file
+		and multiple calls would result in overwrites and unwanted
+		results when the target is up to date and only the tests on
+		changed commands trigger execution of commands.
+
+    ld
+	Link target.
+
+	Example::
+
+		targets += setup setup.o bootsect bootsect.o
+		$(obj)/setup $(obj)/bootsect: %: %.o FORCE
+			$(call if_changed,ld)
+
+	$(targets) are assigned all potential targets, by which the build
+	system knows the targets and will:
+
+		1) check for commandline changes
+
+	The ": %: %.o" part of the prerequisite is a shorthand that
+	frees us from listing the setup.o and bootsect.o files.
+
+	Note:
+		It is a common mistake to forget the "targets :=" assignment,
+		resulting in the target file being recompiled for no
+		obvious reason.
+
+    objcopy
+	Copy binary. Uses OBJCOPYFLAGS usually specified in
+	arch/$(ARCH)/Makefile.
+
+Custom commands
+---------------
+
+	When the build system is executing with V=0, then only
+	a shorthand of a command is normally displayed.
+	To enable this behaviour for custom commands, two variables are
+	required to be set::
+
+		quiet_cmd_<command>	- what shall be echoed
+		      cmd_<command>	- the command to execute
+
+	Example::
+
+		# xsm/flask/Makefile
+		mkflask := policy/mkflask.sh
+		quiet_cmd_mkflask = MKFLASK $@
+		cmd_mkflask = $(CONFIG_SHELL) $(mkflask) $(AWK) include \
+			$(FLASK_H_DEPEND)
+
+		include/flask.h: $(FLASK_H_DEPEND) $(mkflask) FORCE
+			$(call if_changed,mkflask)
+
+	When updating the include/flask.h target, the line:
+
+		MKFLASK include/flask.h
+
+	will be displayed with "make V=0". (V=0 is the default)
diff --git a/xen/Makefile b/xen/Makefile
index 2a689b26a2ba..07f8ef808743 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -52,7 +52,57 @@ dist: install
 
 ifneq ($(root-make-done),y)
 # section to run before calling Rules.mk, but only once.
+
+# Beautify output
+# ---------------------------------------------------------------------------
+#
+# Normally, we echo the whole command before executing it. By making
+# that echo $($(quiet)$(cmd)), we now have the possibility to set
+# $(quiet) to choose other forms of output instead, e.g.
+#
+#         quiet_cmd_cc_o_c = Compiling $(RELDIR)/$@
+#         cmd_cc_o_c       = $(CC) $(c_flags) -c -o $@ $<
+#
+# If $(quiet) is empty, the whole command will be printed.
+# If it is set to "quiet_", only the short version will be printed.
+# If it is set to "silent_", nothing will be printed at all, since
+# the variable $(silent_cmd_cc_o_c) doesn't exist.
+#
+# A simple variant is to prefix commands with $(Q) - that's useful
+# for commands that shall be hidden in non-verbose mode.
 #
+#	$(Q)ln $@ :<
+#
+# If KBUILD_VERBOSE equals 0 then the above command will be hidden.
+# If KBUILD_VERBOSE equals 1 then the above command is displayed.
+#
+# To put more focus on warnings, be less verbose as default
+# Use 'make V=1' to see the full commands
+
+ifeq ("$(origin V)", "command line")
+    KBUILD_VERBOSE := $(V)
+endif
+ifndef KBUILD_VERBOSE
+    KBUILD_VERBOSE := 0
+endif
+
+ifeq ($(KBUILD_VERBOSE),1)
+    quiet :=
+    Q :=
+else
+    quiet := quiet_
+    Q := @
+endif
+
+# If the user is running make -s (silent mode), suppress echoing of
+# commands
+
+ifneq ($(findstring s,$(filter-out --%,$(MAKEFLAGS))),)
+    quiet := silent_
+endif
+
+export quiet Q KBUILD_VERBOSE
+
 # To make sure we do not include .config for any of the *config targets
 # catch them early, and hand them over to tools/kconfig/Makefile
 
@@ -261,7 +311,8 @@ _clean: delete-unfresh-files
 	$(MAKE) $(clean) arch/x86
 	$(MAKE) $(clean) test
 	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) clean
-	find . \( -name "*.o" -o -name ".*.d" -o -name ".*.d2" -o -name "*.gcno" \) -exec rm -f {} \;
+	find . \( -name "*.o" -o -name ".*.d" -o -name ".*.d2" \
+		-o -name "*.gcno" -o -name ".*.cmd" \) -exec rm -f {} \;
 	rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map *~ core
 	rm -f include/asm-*/asm-offsets.h
 	rm -f .banner
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 9d1e1ebf5e44..21cac7f5f51a 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -42,6 +42,7 @@ ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
 
 # Initialise some variables
+targets :=
 CFLAGS-y :=
 AFLAGS-y :=
 
@@ -69,6 +70,10 @@ $(foreach o,$(filter-out %/,$(obj-y) $(obj-bin-y) $(extra-y)),$(eval $(call gend
 subdir-y := $(subdir-y) $(filter %/, $(obj-y))
 obj-y    := $(patsubst %/, %/built_in.o, $(obj-y))
 
+# $(subdir-obj-y) is the list of objects in $(obj-y) which uses dir/ to
+# tell kbuild to descend
+subdir-obj-y := $(filter %/built_in.o, $(obj-y))
+
 $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += -DINIT_SECTIONS_ONLY
 
 ifeq ($(CONFIG_COVERAGE),y)
@@ -123,6 +128,10 @@ else
 endif
 endif
 
+targets += built_in.o
+targets += $(filter-out $(subdir-obj-y), $(obj-y)) $(extra-y)
+targets += $(MAKECMDGOALS)
+
 built_in_bin.o: $(obj-bin-y) $(extra-y)
 ifeq ($(obj-bin-y),)
 	$(CC) $(a_flags) -c -x assembler /dev/null -o $@
@@ -131,7 +140,7 @@ else
 endif
 
 # Force execution of pattern rules (for which PHONY cannot be directly used).
-.PHONY: FORCE
+PHONY += FORCE
 FORCE:
 
 %/built_in.o: FORCE
@@ -179,4 +188,26 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o Makefile
 %.s: %.S Makefile
 	$(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@
 
+# Add intermediate targets:
+# When building objects with specific suffix patterns, add intermediate
+# targets that the final targets are derived from.
+intermediate_targets = $(foreach sfx, $(2), \
+				$(patsubst %$(strip $(1)),%$(sfx), \
+					$(filter %$(strip $(1)), $(targets))))
+# %.init.o <- %.o
+targets += $(call intermediate_targets, .init.o, .o)
+
 -include $(DEPS_INCLUDE)
+
+# Read all saved command lines and dependencies for the $(targets) we
+# may be building above, using $(if_changed{,_dep}). As an
+# optimization, we don't need to read them if the target does not
+# exist, we will rebuild anyway in that case.
+
+existing-targets := $(wildcard $(sort $(targets)))
+
+-include $(foreach f,$(existing-targets),$(dir $(f)).$(notdir $(f)).cmd)
+
+# Declare the contents of the PHONY variable as phony.  We keep that
+# information in a variable so we can use it in if_changed and friends.
+.PHONY: $(PHONY)
diff --git a/xen/scripts/Kbuild.include b/xen/scripts/Kbuild.include
index 806c68824ed5..e62eddc3654c 100644
--- a/xen/scripts/Kbuild.include
+++ b/xen/scripts/Kbuild.include
@@ -2,11 +2,30 @@
 ####
 # kbuild: Generic definitions
 
+# Convenient variables
+squote  := '
+empty   :=
+space   := $(empty) $(empty)
+space_escape := _-_SPACE_-_
+pound   := \#
+
+###
+# Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
+dot-target = $(@D)/.$(@F)
+
 ###
 # dependencies
 DEPS = .*.d
 DEPS_INCLUDE = $(addsuffix .d2, $(basename $(wildcard $(DEPS))))
 
+###
+# real prerequisites without phony targets
+real-prereqs = $(filter-out $(PHONY), $^)
+
+###
+# Escape single quote for use in echo statements
+escsq = $(subst $(squote),'\$(squote)',$1)
+
 # as-insn: Check whether assembler supports an instruction.
 # Usage: cflags-y += $(call as-insn,CC FLAGS,"insn",option-yes,option-no)
 as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
@@ -32,3 +51,91 @@ cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || e
 # Usage:
 # $(MAKE) $(clean) dir
 clean := -f $(BASEDIR)/scripts/Makefile.clean clean -C
+
+# echo command.
+# Short version is used, if $(quiet) equals `quiet_', otherwise full one.
+echo-cmd = $(if $($(quiet)cmd_$(1)),\
+        echo '  $(call escsq,$($(quiet)cmd_$(1)))$(echo-why)';)
+
+# printing commands
+cmd = @set -e; $(echo-cmd) $(cmd_$(1))
+
+###
+# if_changed      - execute command if any prerequisite is newer than
+#                   target, or command line has changed
+# if_changed_rule - as if_changed but execute rule instead
+
+ifneq ($(KBUILD_NOCMDDEP),1)
+# Check if both commands are the same including their order. Result is empty
+# string if equal. User may override this check using make KBUILD_NOCMDDEP=1
+cmd-check = $(filter-out $(subst $(space),$(space_escape),$(strip $(cmd_$@))), \
+                         $(subst $(space),$(space_escape),$(strip $(cmd_$1))))
+else
+cmd-check = $(if $(strip $(cmd_$@)),,1)
+endif
+
+# Replace >$< with >$$< to preserve $ when reloading the .cmd file
+# (needed for make)
+# Replace >#< with >$(pound)< to avoid starting a comment in the .cmd file
+# (needed for make)
+# Replace >'< with >'\''< to be able to enclose the whole string in '...'
+# (needed for the shell)
+make-cmd = $(call escsq,$(subst $(pound),$$(pound),$(subst $$,$$$$,$(cmd_$(1)))))
+
+# Find any prerequisites that is newer than target or that does not exist.
+# PHONY targets skipped in both cases.
+any-prereq = $(filter-out $(PHONY),$?)$(filter-out $(PHONY) $(wildcard $^),$^)
+
+# Execute command if command has changed or prerequisite(s) are updated.
+if_changed = $(if $(any-prereq)$(cmd-check),                                 \
+        $(cmd);                                                              \
+        printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
+
+# Usage: $(call if_changed_rule,foo)
+# Will check if $(cmd_foo) or any of the prerequisites changed,
+# and if so will execute $(rule_foo).
+if_changed_rule = $(if $(any-prereq)$(cmd-check),$(rule_$(1)),@:)
+
+cmd_and_record =                                                             \
+        $(cmd);                                                              \
+        printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd
+
+###
+# why - tell why a target got built
+#       enabled by make V=2
+#       Output (listed in the order they are checked):
+#          (1) - due to target is PHONY
+#          (2) - due to target missing
+#          (3) - due to: file1.h file2.h
+#          (4) - due to command line change
+#          (5) - due to missing .cmd file
+#          (6) - due to target not in $(targets)
+# (1) PHONY targets are always build
+# (2) No target, so we better build it
+# (3) Prerequisite is newer than target
+# (4) The command line stored in the file named dir/.target.cmd
+#     differed from actual command line. This happens when compiler
+#     options changes
+# (5) No dir/.target.cmd file (used to store command line)
+# (6) No dir/.target.cmd file and target not listed in $(targets)
+#     This is a good hint that there is a bug in the kbuild file
+ifeq ($(KBUILD_VERBOSE),2)
+why =                                                                        \
+    $(if $(filter $@, $(PHONY)),- due to target is PHONY,                    \
+        $(if $(wildcard $@),                                                 \
+            $(if $(any-prereq),- due to: $(any-prereq),                      \
+                $(if $(cmd-check),                                           \
+                    $(if $(cmd_$@),- due to command line change,             \
+                        $(if $(filter $@, $(targets)),                       \
+                            - due to missing .cmd file,                      \
+                            - due to $(notdir $@) not in $$(targets)         \
+                         )                                                   \
+                     )                                                       \
+                 )                                                           \
+             ),                                                              \
+             - due to target missing                                         \
+         )                                                                   \
+     )
+
+echo-why = $(call escsq, $(strip $(why)))
+endif
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 07/16] xen/build: Start using if_changed
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
                   ` (5 preceding siblings ...)
  2020-04-21 16:11 ` [XEN PATCH v5 06/16] xen/build: introduce if_changed and if_changed_rule Anthony PERARD
@ 2020-04-21 16:11 ` Anthony PERARD
  2020-04-24  9:28   ` Julien Grall
  2020-04-21 16:12 ` [XEN PATCH v5 08/16] build: Introduce $(cpp_flags) Anthony PERARD
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:11 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Jan Beulich, Anthony PERARD,
	Daniel De Graaf, Volodymyr Babchuk, Roger Pau Monné

This patch start to use if_changed introduced in a previous commit.

Whenever if_changed is called, the target must have FORCE as
dependency so that if_changed can check if the command line to be
run has changed, so the macro $(real-prereqs) must be used to
discover the dependencies without "FORCE".

Whenever a target isn't in obj-y, it should be added to extra-y so the
.*.cmd dependency file associated with the target can be loaded. This
is done for xsm/flask/ and both common/lib{elf,fdt}/ and
arch/x86/Makefile.

For the targets that generate .*.d dependency files, there's going to
be two dependency files (.*.d and .*.cmd) until we can merge them
together in a later patch via fixdep from Linux.

One cleanup, libelf-relocate.o doesn't exist anymore.

We import cmd_ld and cmd_objcopy from Linux v5.4.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---

Notes:
    v4:
    - typos
    - fix missing FORCE in libfdt/Makefile
    - typo flask vs flash in xsm
    - in xsm, use *_H_DEPEND in command lines of mkaccess and mkflask
      instead of $(real-prereq) to avoid including the script as argument of
      itself.

 xen/Rules.mk               | 68 +++++++++++++++++++++++++++-----------
 xen/arch/arm/Makefile      |  4 +--
 xen/arch/x86/Makefile      |  1 +
 xen/arch/x86/efi/Makefile  |  7 ++--
 xen/common/libelf/Makefile | 12 ++++---
 xen/common/libfdt/Makefile | 11 +++---
 xen/xsm/flask/Makefile     | 17 +++++++---
 7 files changed, 85 insertions(+), 35 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 21cac7f5f51a..2e28c572305a 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -56,6 +56,18 @@ SPECIAL_DATA_SECTIONS := rodata $(foreach a,1 2 4 8 16, \
 
 include Makefile
 
+# Linking
+# ---------------------------------------------------------------------------
+
+quiet_cmd_ld = LD      $@
+cmd_ld = $(LD) $(XEN_LDFLAGS) -r -o $@ $(real-prereqs)
+
+# Objcopy
+# ---------------------------------------------------------------------------
+
+quiet_cmd_objcopy = OBJCOPY $@
+cmd_objcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $< $@
+
 define gendep
     ifneq ($(1),$(subst /,:,$(1)))
         DEPS += $(dir $(1)).$(notdir $(1)).d
@@ -164,29 +176,47 @@ else
 	$(CC) $(c_flags) -c $< -o $@
 endif
 
-%.o: %.S Makefile
-	$(CC) $(a_flags) -c $< -o $@
+quiet_cmd_cc_o_S = CC      $@
+cmd_cc_o_S = $(CC) $(a_flags) -c $< -o $@
+
+%.o: %.S FORCE
+	$(call if_changed,cc_o_S)
+
+
+quiet_cmd_obj_init_o = INIT_O  $@
+define cmd_obj_init_o
+    $(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p;}' | while read idx name sz rest; do \
+        case "$$name" in \
+        .*.local) ;; \
+        .text|.text.*|.data|.data.*|.bss) \
+            test $$sz != 0 || continue; \
+            echo "Error: size of $<:$$name is 0x$$sz" >&2; \
+            exit $$(expr $$idx + 1);; \
+        esac; \
+    done; \
+    $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
+endef
+
+$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o FORCE
+	$(call if_changed,obj_init_o)
+
+quiet_cmd_cpp_i_c = CPP     $@
+cmd_cpp_i_c = $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) $< -o $@
+
+quiet_cmd_cc_s_c = CC      $@
+cmd_cc_s_c = $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@
 
-$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o Makefile
-	$(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p;}' | while read idx name sz rest; do \
-		case "$$name" in \
-		.*.local) ;; \
-		.text|.text.*|.data|.data.*|.bss) \
-			test $$sz != 0 || continue; \
-			echo "Error: size of $<:$$name is 0x$$sz" >&2; \
-			exit $$(expr $$idx + 1);; \
-		esac; \
-	done
-	$(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
+quiet_cmd_s_S = CPP     $@
+cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@
 
-%.i: %.c Makefile
-	$(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) $< -o $@
+%.i: %.c FORCE
+	$(call if_changed,cpp_i_c)
 
-%.s: %.c Makefile
-	$(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@
+%.s: %.c FORCE
+	$(call if_changed,cc_s_c)
 
-%.s: %.S Makefile
-	$(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@
+%.s: %.S FORCE
+	$(call if_changed,cpp_s_S)
 
 # Add intermediate targets:
 # When building objects with specific suffix patterns, add intermediate
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 9f1ab2335756..b79ad55646bd 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -97,8 +97,8 @@ prelink_lto.o: $(ALL_OBJS)
 prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
 	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
 else
-prelink.o: $(ALL_OBJS)
-	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
+prelink.o: $(ALL_OBJS) FORCE
+	$(call if_changed,ld)
 endif
 
 $(TARGET)-syms: prelink.o xen.lds
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index a805e9982e85..44137d919b66 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -71,6 +71,7 @@ obj-$(CONFIG_TBOOT) += tboot.o
 obj-y += hpet.o
 obj-y += vm_event.o
 obj-y += xstate.o
+extra-y += asm-macros.i
 
 x86_emulate.o: x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
 
diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index 490d791aae2d..3e4c395b7535 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -1,7 +1,10 @@
 CFLAGS-y += -fshort-wchar
 
-%.o: %.ihex
-	$(OBJCOPY) -I ihex -O binary $< $@
+quiet_cmd_objcopy_o_ihex = OBJCOPY $@
+cmd_objcopy_o_ihex = $(OBJCOPY) -I ihex -O binary $< $@
+
+%.o: %.ihex FORCE
+	$(call if_changed,objcopy_o_ihex)
 
 boot.init.o: buildid.o
 
diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile
index 464c448d9d37..a92326c982e9 100644
--- a/xen/common/libelf/Makefile
+++ b/xen/common/libelf/Makefile
@@ -1,12 +1,16 @@
 obj-bin-y := libelf.o
 nocov-y += libelf.o
+libelf-objs := libelf-tools.o libelf-loader.o libelf-dominfo.o
 
 SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
+OBJCOPYFLAGS := $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s))
 
 CFLAGS-y += -Wno-pointer-sign
 
-libelf.o: libelf-temp.o Makefile
-	$(OBJCOPY) $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
+libelf.o: libelf-temp.o FORCE
+	$(call if_changed,objcopy)
 
-libelf-temp.o: libelf-tools.o libelf-loader.o libelf-dominfo.o #libelf-relocate.o
-	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
+libelf-temp.o: $(libelf-objs) FORCE
+	$(call if_changed,ld)
+
+extra-y += libelf-temp.o $(libelf-objs)
diff --git a/xen/common/libfdt/Makefile b/xen/common/libfdt/Makefile
index e2a5e59380a0..6bd207cf8ffa 100644
--- a/xen/common/libfdt/Makefile
+++ b/xen/common/libfdt/Makefile
@@ -1,14 +1,17 @@
 include Makefile.libfdt
 
 SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
+OBJCOPYFLAGS := $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s))
 
 obj-y += libfdt.o
 nocov-y += libfdt.o
 
 CFLAGS-y += -I$(BASEDIR)/include/xen/libfdt/
 
-libfdt.o: libfdt-temp.o Makefile
-	$(OBJCOPY) $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
+libfdt.o: libfdt-temp.o FORCE
+	$(call if_changed,objcopy)
 
-libfdt-temp.o: $(LIBFDT_OBJS)
-	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
+libfdt-temp.o: $(LIBFDT_OBJS) FORCE
+	$(call if_changed,ld)
+
+extra-y += libfdt-temp.o $(LIBFDT_OBJS)
diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index 6db396347b1f..eebfceecc5fb 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -20,12 +20,21 @@ AV_H_FILES = include/av_perm_to_string.h include/av_permissions.h
 ALL_H_FILES = $(FLASK_H_FILES) $(AV_H_FILES)
 
 $(obj-y) ss/built_in.o: $(ALL_H_FILES)
+extra-y += $(ALL_H_FILES)
 
-$(subst include/,%/,$(FLASK_H_FILES)): $(FLASK_H_DEPEND)
-	$(CONFIG_SHELL) policy/mkflask.sh $(AWK) include $(FLASK_H_DEPEND)
+mkflask := policy/mkflask.sh
+quiet_cmd_mkflask = MKFLASK $@
+cmd_mkflask = $(CONFIG_SHELL) $(mkflask) $(AWK) include $(FLASK_H_DEPEND)
 
-$(subst include/,%/,$(AV_H_FILES)): $(AV_H_DEPEND)
-	$(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)
+$(subst include/,%/,$(FLASK_H_FILES)): $(FLASK_H_DEPEND) $(mkflask) FORCE
+	$(call if_changed,mkflask)
+
+mkaccess := policy/mkaccess_vector.sh
+quiet_cmd_mkaccess = MKACCESS VECTOR $@
+cmd_mkaccess = $(CONFIG_SHELL) $(mkaccess) $(AWK) $(AV_H_DEPEND)
+
+$(subst include/,%/,$(AV_H_FILES)): $(AV_H_DEPEND) $(mkaccess) FORCE
+	$(call if_changed,mkaccess)
 
 obj-bin-$(CONFIG_XSM_FLASK_POLICY) += flask-policy.o
 flask-policy.o: policy.bin
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 08/16] build: Introduce $(cpp_flags)
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
                   ` (6 preceding siblings ...)
  2020-04-21 16:11 ` [XEN PATCH v5 07/16] xen/build: Start using if_changed Anthony PERARD
@ 2020-04-21 16:12 ` Anthony PERARD
  2020-04-23 16:48   ` Jan Beulich
  2020-04-21 16:12 ` [XEN PATCH v5 09/16] xen/build: use if_changed on built_in.o Anthony PERARD
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:12 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Jan Beulich, Anthony PERARD

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

Notes:
    v5:
    - new patch

 xen/Rules.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 2e28c572305a..25bcf45612bd 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -123,6 +123,7 @@ $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS))
 
 c_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_CFLAGS) '-D__OBJECT_FILE__="$@"'
 a_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_AFLAGS)
+cpp_flags = $(filter-out -Wa$(comma)%,$(a_flags))
 
 include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
 
@@ -207,7 +208,7 @@ quiet_cmd_cc_s_c = CC      $@
 cmd_cc_s_c = $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@
 
 quiet_cmd_s_S = CPP     $@
-cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@
+cmd_s_S = $(CPP) $(cpp_flags) $< -o $@
 
 %.i: %.c FORCE
 	$(call if_changed,cpp_i_c)
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 09/16] xen/build: use if_changed on built_in.o
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
                   ` (7 preceding siblings ...)
  2020-04-21 16:12 ` [XEN PATCH v5 08/16] build: Introduce $(cpp_flags) Anthony PERARD
@ 2020-04-21 16:12 ` Anthony PERARD
  2020-04-28 13:48   ` Jan Beulich
  2020-04-21 16:12 ` [XEN PATCH v5 10/16] xen/build: Use if_changed_rules with %.o:%.c targets Anthony PERARD
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:12 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Jan Beulich, Anthony PERARD

In the case where $(obj-y) is empty, we also replace $(c_flags) by
$(XEN_CFLAGS) to avoid generating an .%.d dependency file. This avoid
make trying to include %.h file in the ld command if $(obj-y) isn't
empty anymore on a second run.

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

Notes:
    v4:
    - Have cmd_ld_builtin depends on CONFIG_LTO, which simplify built_in.o
      rule.

 xen/Rules.mk | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 25bcf45612bd..5e08e14455d7 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -130,15 +130,24 @@ include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
 c_flags += $(CFLAGS-y)
 a_flags += $(CFLAGS-y) $(AFLAGS-y)
 
-built_in.o: $(obj-y) $(extra-y)
-ifeq ($(obj-y),)
-	$(CC) $(c_flags) -c -x c /dev/null -o $@
-else
+quiet_cmd_ld_builtin = LD      $@
 ifeq ($(CONFIG_LTO),y)
-	$(LD_LTO) -r -o $@ $(filter-out $(extra-y),$^)
+cmd_ld_builtin = \
+    $(LD_LTO) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
 else
-	$(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
+cmd_ld_builtin = \
+    $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
 endif
+
+quiet_cmd_cc_builtin = LD      $@
+cmd_cc_builtin = \
+    $(CC) $(XEN_CFLAGS) -c -x c /dev/null -o $@
+
+built_in.o: $(obj-y) $(extra-y) FORCE
+ifeq ($(obj-y),)
+	$(call if_changed,cc_builtin)
+else
+	$(call if_changed,ld_builtin)
 endif
 
 targets += built_in.o
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 10/16] xen/build: Use if_changed_rules with %.o:%.c targets
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
                   ` (8 preceding siblings ...)
  2020-04-21 16:12 ` [XEN PATCH v5 09/16] xen/build: use if_changed on built_in.o Anthony PERARD
@ 2020-04-21 16:12 ` Anthony PERARD
  2020-04-21 16:12 ` [XEN PATCH v5 11/16] xen/build: factorise generation of the linker scripts Anthony PERARD
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:12 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Jan Beulich, Anthony PERARD

Use $(dot-target) to have the target name prefix with a dot.

Now, when the CC command has run, it is recorded in .*.cmd
file, then if_changed_rules will compare it on subsequent runs.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
 xen/Rules.mk | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 5e08e14455d7..9150911296de 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -173,19 +173,27 @@ FORCE:
 
 SRCPATH := $(patsubst $(BASEDIR)/%,%,$(CURDIR))
 
-%.o: %.c Makefile
+quiet_cmd_cc_o_c = CC      $@
 ifeq ($(CONFIG_ENFORCE_UNIQUE_SYMBOLS),y)
-	$(CC) $(c_flags) -c $< -o $(@D)/.$(@F).tmp -MQ $@
-ifeq ($(CONFIG_CC_IS_CLANG),y)
-	$(OBJCOPY) --redefine-sym $<=$(SRCPATH)/$< $(@D)/.$(@F).tmp $@
-else
-	$(OBJCOPY) --redefine-sym $(<F)=$(SRCPATH)/$< $(@D)/.$(@F).tmp $@
-endif
-	rm -f $(@D)/.$(@F).tmp
+    cmd_cc_o_c = $(CC) $(c_flags) -c $< -o $(dot-target).tmp -MQ $@
+    ifeq ($(CONFIG_CC_IS_CLANG),y)
+        cmd_objcopy_fix_sym = $(OBJCOPY) --redefine-sym $<=$(SRCPATH)/$< $(dot-target).tmp $@
+    else
+        cmd_objcopy_fix_sym = $(OBJCOPY) --redefine-sym $(<F)=$(SRCPATH)/$< $(dot-target).tmp $@
+    endif
+    cmd_objcopy_fix_sym += && rm -f $(dot-target).tmp
 else
-	$(CC) $(c_flags) -c $< -o $@
+    cmd_cc_o_c = $(CC) $(c_flags) -c $< -o $@
 endif
 
+define rule_cc_o_c
+    $(call cmd_and_record,cc_o_c)
+    $(call cmd,objcopy_fix_sym)
+endef
+
+%.o: %.c FORCE
+	$(call if_changed_rule,cc_o_c)
+
 quiet_cmd_cc_o_S = CC      $@
 cmd_cc_o_S = $(CC) $(a_flags) -c $< -o $@
 
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 11/16] xen/build: factorise generation of the linker scripts
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
                   ` (9 preceding siblings ...)
  2020-04-21 16:12 ` [XEN PATCH v5 10/16] xen/build: Use if_changed_rules with %.o:%.c targets Anthony PERARD
@ 2020-04-21 16:12 ` Anthony PERARD
  2020-04-24  9:29   ` Julien Grall
  2020-04-28 13:50   ` Jan Beulich
  2020-04-21 16:12 ` [XEN PATCH v5 12/16] xen/build: Use if_changed for prelink*.o Anthony PERARD
                   ` (4 subsequent siblings)
  15 siblings, 2 replies; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:12 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Jan Beulich, Anthony PERARD,
	Volodymyr Babchuk, Roger Pau Monné

In Arm and X86 makefile, generating the linker script is the same, so
we can simply have both call the same macro.

We need to add *.lds files into extra-y so that Rules.mk can find the
.*.cmd dependency file and load it.

Change made to the command line:
- Use of $(CPP) instead of $(CC) -E
- Remove -Ui386.
  We don't compile Xen for i386 anymore, so that macro is never
  defined. Also it doesn't make sense on Arm.
- Use $(cpp_flags) which simply filter -Wa,% options from $(a_flags).

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

Notes:
    v5:
    - rename cc_lds_S to cpp_lds_S as the binary runned is now CPP
    - Use new cpp_flags instead of the open-coded filter of a_flags.
    
    v4:
    - fix rebuild by adding FORCE as dependency
    - Use $(CPP)
    - remove -Ui386
    - avoid using "define" for cmd_cc_lds_S, as adding '; \' on each line is
      still mandatory for if_changed (or cmd) macro to work.

 xen/Rules.mk          | 6 ++++++
 xen/arch/arm/Makefile | 8 ++++----
 xen/arch/x86/Makefile | 8 ++++----
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 9150911296de..877f52d871fa 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -236,6 +236,12 @@ cmd_s_S = $(CPP) $(cpp_flags) $< -o $@
 %.s: %.S FORCE
 	$(call if_changed,cpp_s_S)
 
+# Linker scripts, .lds.S -> .lds
+quiet_cmd_cpp_lds_S = LDS     $@
+cmd_cpp_lds_S = $(CPP) -P $(cpp_flags) -o $@ $<; \
+    sed -e 's/.*\.lds\.o:/$(@F):/g' <$(dot-target).d >$(dot-target).d.new; \
+    mv -f $(dot-target).d.new $(dot-target).d
+
 # Add intermediate targets:
 # When building objects with specific suffix patterns, add intermediate
 # targets that the final targets are derived from.
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index b79ad55646bd..68cb258870eb 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -64,6 +64,8 @@ obj-y += vpsci.o
 obj-y += vuart.o
 extra-y += $(TARGET_SUBARCH)/head.o
 
+extra-y += xen.lds
+
 #obj-bin-y += ....o
 
 ifdef CONFIG_DTB_FILE
@@ -122,10 +124,8 @@ $(TARGET)-syms: prelink.o xen.lds
 asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c
 	$(CC) $(filter-out -flto,$(c_flags)) -S -o $@ $<
 
-xen.lds: xen.lds.S
-	$(CC) -P -E -Ui386 $(a_flags) -o $@ $<
-	sed -e 's/xen\.lds\.o:/xen\.lds:/g' <.xen.lds.d >.xen.lds.d.new
-	mv -f .xen.lds.d.new .xen.lds.d
+xen.lds: xen.lds.S FORCE
+	$(call if_changed,cpp_lds_S)
 
 dtb.o: $(CONFIG_DTB_FILE)
 
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 44137d919b66..92430192a74e 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -72,6 +72,7 @@ obj-y += hpet.o
 obj-y += vm_event.o
 obj-y += xstate.o
 extra-y += asm-macros.i
+extra-y += xen.lds
 
 x86_emulate.o: x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
 
@@ -173,6 +174,7 @@ export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o efi/check.
 # Check if the linker supports PE.
 XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep --subsystem=10 -o efi/check.efi efi/check.o 2>/dev/null && echo y))
 CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
+extra-$(XEN_BUILD_PE) += efi.lds
 
 $(TARGET).efi: VIRT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A VIRT_START$$,,p')
 $(TARGET).efi: ALT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A ALT_START$$,,p')
@@ -240,10 +242,8 @@ $(BASEDIR)/include/asm-x86/asm-macros.h: asm-macros.i Makefile
 	$(call move-if-changed,$@.new,$@)
 
 efi.lds: AFLAGS-y += -DEFI
-xen.lds efi.lds: xen.lds.S
-	$(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<
-	sed -e 's/.*\.lds\.o:/$(@F):/g' <.$(@F).d >.$(@F).d.new
-	mv -f .$(@F).d.new .$(@F).d
+xen.lds efi.lds: xen.lds.S FORCE
+	$(call if_changed,cpp_lds_S)
 
 boot/mkelf32: boot/mkelf32.c
 	$(HOSTCC) $(HOSTCFLAGS) -o $@ $<
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 12/16] xen/build: Use if_changed for prelink*.o
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
                   ` (10 preceding siblings ...)
  2020-04-21 16:12 ` [XEN PATCH v5 11/16] xen/build: factorise generation of the linker scripts Anthony PERARD
@ 2020-04-21 16:12 ` Anthony PERARD
  2020-04-21 16:12 ` [XEN PATCH v5 13/16] xen,symbols: rework file symbols selection Anthony PERARD
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:12 UTC (permalink / raw)
  To: xen-devel
  Cc: Anthony PERARD, Andrew Cooper, Wei Liu, Jan Beulich,
	Roger Pau Monné

We change the dependencies of prelink-efi.o so that we can use the
same command line. The dependency on efi/built_in.o isn't needed
because, we already have:
    efi/*.o: efi/built_in.o
to build efi/*.o files that prelink-efi.o needs.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---

Notes:
    v4:
    - fix rebuild, prelink.o and prelink-efi.o needs to be in targets

 xen/arch/x86/Makefile | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 92430192a74e..33e9ebf25074 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -128,11 +128,13 @@ prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
 prelink-efi.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink-efi_lto.o efi/boot.init.o
 	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
 else
-prelink.o: $(ALL_OBJS)
-	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
+prelink.o: $(ALL_OBJS) FORCE
+	$(call if_changed,ld)
+
+prelink-efi.o: $(filter-out %/efi/built_in.o,$(ALL_OBJS)) efi/boot.init.o efi/runtime.o efi/compat.o FORCE
+	$(call if_changed,ld)
 
-prelink-efi.o: $(ALL_OBJS) efi/boot.init.o efi/runtime.o efi/compat.o
-	$(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out %/efi/built_in.o,$^)
+targets += prelink.o prelink-efi.o
 endif
 
 $(TARGET)-syms: prelink.o xen.lds
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 13/16] xen,symbols: rework file symbols selection
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
                   ` (11 preceding siblings ...)
  2020-04-21 16:12 ` [XEN PATCH v5 12/16] xen/build: Use if_changed for prelink*.o Anthony PERARD
@ 2020-04-21 16:12 ` Anthony PERARD
  2020-04-28 14:05   ` Jan Beulich
  2020-04-21 16:12 ` [XEN PATCH v5 14/16] build: use if_changed to build mm/*/guest_%.o Anthony PERARD
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:12 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Jan Beulich, Anthony PERARD

We want to use the same rune to build mm/*/guest_*.o as the one use to
build every other *.o object. The consequence it that file symbols that
the program ./symbols prefer changes with CONFIG_ENFORCE_UNIQUE_SYMBOLS=y.

For example, when building arch/x86/mm/guest_walk_2.o from guest_walk.c,
this would be the difference of file symbol present in the object when
building with CONFIG_ENFORCE_UNIQUE_SYMBOLS=y:

(1) Currently we have those two file symbols:
    guest_walk.c
    guest_walk_2.o
(2) When building with the same rune, we will have:
    arch/x86/mm/guest_walk.c
    guest_walk_2.o

The order in which those symbols are present may be different. Building
without CONFIG_ENFORCE_UNIQUE_SYMBOLS will result in (1).

Currently, in case (1) ./symbols chooses the *.o symbol (object file
name). But in case (2), may choose the *.c symbol (source file name with
path component) if it is first

We want to have ./symbols choose the object file name symbol in both
cases. So this patch changes that ./symbols prefer the "object file
name" symbol over the "source file name with path component" symbols.

The new intended order of preference is:
    - first object file name symbol (present only in object files
      produced from multiply-compiled sources)
    - first source file name with path components symbol
    - last source file name without any path component symbol

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

Notes:
    v5:
    - reword part of the commit message
    
    v4:
    - rescope enum symbol_type
    - remove setting values to enums, as it's not needed.
    - rename the enumeration symbols

 xen/tools/symbols.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/xen/tools/symbols.c b/xen/tools/symbols.c
index 9f9e2c990061..b3a9465b32d3 100644
--- a/xen/tools/symbols.c
+++ b/xen/tools/symbols.c
@@ -84,7 +84,12 @@ static int read_symbol(FILE *in, struct sym_entry *s)
 {
 	char str[500], type[20] = "";
 	char *sym, stype;
-	static enum { symbol, single_source, multi_source } last;
+	static enum symbol_type {
+		symbol,
+		file_source,
+		path_source,
+		obj_file,
+	} last;
 	static char *filename;
 	int rc = -1;
 
@@ -125,13 +130,20 @@ static int read_symbol(FILE *in, struct sym_entry *s)
 		 * prefer the first one if that names an object file or has a
 		 * directory component (to cover multiply compiled files).
 		 */
-		bool multi = strchr(str, '/') || (sym && sym[1] == 'o');
+		enum symbol_type current;
 
-		if (multi || last != multi_source) {
+		if (sym && sym[1] == 'o')
+		    current = obj_file;
+		else if (strchr(str, '/'))
+		    current = path_source;
+		else
+		    current = file_source;
+
+		if (current > last || last == file_source) {
 			free(filename);
 			filename = *str ? strdup(str) : NULL;
+			last = current;
 		}
-		last = multi ? multi_source : single_source;
 		goto skip_tail;
 	}
 
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 14/16] build: use if_changed to build mm/*/guest_%.o
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
                   ` (12 preceding siblings ...)
  2020-04-21 16:12 ` [XEN PATCH v5 13/16] xen,symbols: rework file symbols selection Anthony PERARD
@ 2020-04-21 16:12 ` Anthony PERARD
  2020-04-28 14:07   ` Jan Beulich
  2020-04-21 16:12 ` [XEN PATCH v5 15/16] build,include: rework compat-build-source.py Anthony PERARD
  2020-04-21 16:12 ` [XEN PATCH v5 16/16] build,include: rework compat-build-header.py Anthony PERARD
  15 siblings, 1 reply; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:12 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Andrew Cooper, Tim Deegan, George Dunlap, Jan Beulich,
	Anthony PERARD, Roger Pau Monné

Use if_changed for building all guest_%.o objects, and make use of
command that already exist.

The current command only runs `CC`, but the runes to build every other
object in Xen also runs `objcopy` (when CONFIG_ENFORCE_UNIQUE_SYMBOLS=y)
which modify the file symbol. But with patch
"xen,symbols: rework file symbols selection", ./symbols should still
select the file symbols directive intended to be used for guest_%.o
objects.

The goal here is to reduce the number of commands written in
makefiles.

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

Notes:
    v5:
    - reword commit message
    
    v4:
    - remove the introduction of Kbuild's CFLAGS_$@
      and simply use make's per-target variable customization.
      Mostly to avoid using $(eval ) which might not work as expected on
      make 3.80.

 xen/arch/x86/mm/Makefile        | 14 ++++++++------
 xen/arch/x86/mm/hap/Makefile    | 15 +++++++++------
 xen/arch/x86/mm/shadow/Makefile | 14 ++++++++------
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/mm/Makefile b/xen/arch/x86/mm/Makefile
index a2431fde6bb4..a66a57314489 100644
--- a/xen/arch/x86/mm/Makefile
+++ b/xen/arch/x86/mm/Makefile
@@ -11,11 +11,13 @@ obj-y += p2m.o p2m-pt.o
 obj-$(CONFIG_HVM) += p2m-ept.o p2m-pod.o
 obj-y += paging.o
 
-guest_walk_%.o: guest_walk.c Makefile
-	$(CC) $(c_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
+guest_walk_%.o guest_walk_%.i guest_walk_%.s: CFLAGS-y += -DGUEST_PAGING_LEVELS=$*
 
-guest_walk_%.i: guest_walk.c Makefile
-	$(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
+guest_walk_%.o: guest_walk.c FORCE
+	$(call if_changed_rule,cc_o_c)
 
-guest_walk_%.s: guest_walk.c Makefile
-	$(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -S $< -o $@
+guest_walk_%.i: guest_walk.c FORCE
+	$(call if_changed,cpp_i_c)
+
+guest_walk_%.s: guest_walk.c FORCE
+	$(call if_changed,cc_s_c)
diff --git a/xen/arch/x86/mm/hap/Makefile b/xen/arch/x86/mm/hap/Makefile
index 22e7ad54bd33..34720b2fbe2e 100644
--- a/xen/arch/x86/mm/hap/Makefile
+++ b/xen/arch/x86/mm/hap/Makefile
@@ -5,11 +5,14 @@ obj-y += guest_walk_4level.o
 obj-y += nested_hap.o
 obj-y += nested_ept.o
 
-guest_walk_%level.o: guest_walk.c Makefile
-	$(CC) $(c_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
+guest_walk_%level.o guest_walk_%level.i guest_walk_%level.s: \
+    CFLAGS-y += -DGUEST_PAGING_LEVELS=$*
 
-guest_walk_%level.i: guest_walk.c Makefile
-	$(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
+guest_walk_%level.o: guest_walk.c FORCE
+	$(call if_changed_rule,cc_o_c)
 
-guest_walk_%level.s: guest_walk.c Makefile
-	$(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -S $< -o $@
+guest_walk_%level.i: guest_walk.c FORCE
+	$(call if_changed,cpp_i_c)
+
+guest_walk_%level.s: guest_walk.c FORCE
+	$(call if_changed,cc_s_c)
diff --git a/xen/arch/x86/mm/shadow/Makefile b/xen/arch/x86/mm/shadow/Makefile
index 23d3ff10802c..e00f9cb1aaba 100644
--- a/xen/arch/x86/mm/shadow/Makefile
+++ b/xen/arch/x86/mm/shadow/Makefile
@@ -6,11 +6,13 @@ else
 obj-y += none.o
 endif
 
-guest_%.o: multi.c Makefile
-	$(CC) $(c_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
+guest_%.o guest_%.i guest_%.s: CFLAGS-y += -DGUEST_PAGING_LEVELS=$*
 
-guest_%.i: multi.c Makefile
-	$(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
+guest_%.o: multi.c FORCE
+	$(call if_changed_rule,cc_o_c)
 
-guest_%.s: multi.c Makefile
-	$(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -S $< -o $@
+guest_%.i: multi.c FORCE
+	$(call if_changed,cpp_i_c)
+
+guest_%.s: multi.c FORCE
+	$(call if_changed,cc_s_c)
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 15/16] build,include: rework compat-build-source.py
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
                   ` (13 preceding siblings ...)
  2020-04-21 16:12 ` [XEN PATCH v5 14/16] build: use if_changed to build mm/*/guest_%.o Anthony PERARD
@ 2020-04-21 16:12 ` Anthony PERARD
  2020-04-28 14:37   ` [XEN PATCH v5 15/16] build, include: " Jan Beulich
  2020-04-21 16:12 ` [XEN PATCH v5 16/16] build,include: rework compat-build-header.py Anthony PERARD
  15 siblings, 1 reply; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:12 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Jan Beulich, Anthony PERARD

Improvement are:
- give the path to xlat.lst as argument
- include `grep -v` in compat-build-source.py script, we don't need to
  write this in several scripted language.

No changes in final compat/%.h headers.

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

Notes:
    v5:
    - removed "have 'xlat.lst' path as a variable" from the patch.
    
    v4:
    - new patch

 xen/include/Makefile             | 3 +--
 xen/tools/compat-build-source.py | 8 +++++++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/xen/include/Makefile b/xen/include/Makefile
index 2a10725d689b..537085b82793 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -68,8 +68,7 @@ compat/%.i: compat/%.c Makefile
 
 compat/%.c: public/%.h xlat.lst Makefile $(BASEDIR)/tools/compat-build-source.py
 	mkdir -p $(@D)
-	grep -v 'DEFINE_XEN_GUEST_HANDLE(long)' $< | \
-	$(PYTHON) $(BASEDIR)/tools/compat-build-source.py >$@.new
+	$(PYTHON) $(BASEDIR)/tools/compat-build-source.py xlat.lst <$< >$@.new
 	mv -f $@.new $@
 
 compat/.xlat/%.h: compat/%.h compat/.xlat/%.lst $(BASEDIR)/tools/get-fields.sh Makefile
diff --git a/xen/tools/compat-build-source.py b/xen/tools/compat-build-source.py
index c664eb85e633..c7fc2c53db61 100755
--- a/xen/tools/compat-build-source.py
+++ b/xen/tools/compat-build-source.py
@@ -12,7 +12,11 @@ pats = [
  [ r"XEN_GUEST_HANDLE(_[0-9A-Fa-f]+)?", r"COMPAT_HANDLE" ],
 ];
 
-xlatf = open('xlat.lst', 'r')
+try:
+    xlatf = open(sys.argv[1], 'r')
+except IndexError:
+    print('missing path to xlat.lst argument')
+    sys.exit(1)
 for line in xlatf.readlines():
     match = re.subn(r"^\s*\?\s+(\w*)\s.*", r"\1", line.rstrip())
     if match[1]:
@@ -24,6 +28,8 @@ for pat in pats:
     pat[0] = re.compile(pat[0])
 
 for line in sys.stdin.readlines():
+    if 'DEFINE_XEN_GUEST_HANDLE(long)' in line:
+        continue
     for pat in pats:
         line = re.sub(pat[0], pat[1], line)
     print(line.rstrip())
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [XEN PATCH v5 16/16] build,include: rework compat-build-header.py
  2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
                   ` (14 preceding siblings ...)
  2020-04-21 16:12 ` [XEN PATCH v5 15/16] build,include: rework compat-build-source.py Anthony PERARD
@ 2020-04-21 16:12 ` Anthony PERARD
  2020-04-28 14:39   ` [XEN PATCH v5 16/16] build, include: " Jan Beulich
  2020-04-28 14:54   ` Wei Liu
  15 siblings, 2 replies; 41+ messages in thread
From: Anthony PERARD @ 2020-04-21 16:12 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, Jan Beulich, Anthony PERARD

Replace a mix of shell script and python script by all python script.

No change to the final generated headers.

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

Notes:
    v5:
    - Removed -P from CPP when generating compat/%.i
      -> keep removing linemarkers and keep de-duplicating empty lines.
      So that all the blank line that currently exist in the generated
      headers stays in place.
    
    v4:
    - new patch

 xen/include/Makefile             | 11 +------
 xen/tools/compat-build-header.py | 52 ++++++++++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/xen/include/Makefile b/xen/include/Makefile
index 537085b82793..12660625119e 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -51,16 +51,7 @@ public-$(CONFIG_ARM) := $(wildcard public/arch-arm/*.h public/arch-arm/*/*.h)
 all: $(headers-y)
 
 compat/%.h: compat/%.i Makefile $(BASEDIR)/tools/compat-build-header.py
-	set -e; id=_$$(echo $@ | tr '[:lower:]-/.' '[:upper:]___'); \
-	echo "#ifndef $$id" >$@.new; \
-	echo "#define $$id" >>$@.new; \
-	echo "#include <xen/compat.h>" >>$@.new; \
-	$(if $(filter-out compat/arch-%.h,$@),echo "#include <$(patsubst compat/%,public/%,$@)>" >>$@.new;) \
-	$(if $(prefix-y),echo "$(prefix-y)" >>$@.new;) \
-	grep -v '^# [0-9]' $< | \
-	$(PYTHON) $(BASEDIR)/tools/compat-build-header.py | uniq >>$@.new; \
-	$(if $(suffix-y),echo "$(suffix-y)" >>$@.new;) \
-	echo "#endif /* $$id */" >>$@.new
+	$(PYTHON) $(BASEDIR)/tools/compat-build-header.py <$< $@ "$(prefix-y)" "$(suffix-y)" >>$@.new; \
 	mv -f $@.new $@
 
 compat/%.i: compat/%.c Makefile
diff --git a/xen/tools/compat-build-header.py b/xen/tools/compat-build-header.py
index b85c43f13faf..34d5343c3dd9 100755
--- a/xen/tools/compat-build-header.py
+++ b/xen/tools/compat-build-header.py
@@ -2,6 +2,12 @@
 
 import re,sys
 
+try:
+    maketrans = str.maketrans
+except AttributeError:
+    # For python2
+    from string import maketrans
+
 pats = [
  [ r"__InClUdE__(.*)", r"#include\1\n#pragma pack(4)" ],
  [ r"__IfDeF__ (XEN_HAVE.*)", r"#ifdef \1" ],
@@ -20,7 +26,49 @@ pats = [
  [ r"(^|[^\w])long([^\w]|$$)", r"\1int\2" ]
 ];
 
+output_filename = sys.argv[1]
+
+# tr '[:lower:]-/.' '[:upper:]___'
+header_id = '_' + \
+    output_filename.upper().translate(maketrans('-/.','___'))
+
+header = """#ifndef {0}
+#define {0}
+#include <xen/compat.h>""".format(header_id)
+
+print(header)
+
+if not re.match("compat/arch-.*.h$", output_filename):
+    x = output_filename.replace("compat/","public/")
+    print('#include <%s>' % x)
+
+def print_if_nonempty(s):
+    if len(s):
+        print(s)
+
+print_if_nonempty(sys.argv[2])
+
+last_line_empty = False
 for line in sys.stdin.readlines():
+    line = line.rstrip()
+
+    # Remove linemarkers generated by the preprocessor.
+    if re.match(r"^# \d", line):
+        continue
+
+    # De-duplicate empty lines.
+    if len(line) == 0:
+        if not last_line_empty:
+            print(line)
+            last_line_empty = True
+        continue
+    else:
+        last_line_empty = False
+
     for pat in pats:
-        line = re.subn(pat[0], pat[1], line)[0]
-    print(line.rstrip())
+        line = re.sub(pat[0], pat[1], line)
+    print(line)
+
+print_if_nonempty(sys.argv[3])
+
+print("#endif /* %s */" % header_id)
-- 
Anthony PERARD



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS
  2020-04-21 16:11 ` [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS Anthony PERARD
@ 2020-04-23 16:40   ` Jan Beulich
  2020-04-24  9:45     ` Anthony PERARD
  2020-04-24  9:26   ` Julien Grall
  2020-04-24 13:01   ` Jan Beulich
  2 siblings, 1 reply; 41+ messages in thread
From: Jan Beulich @ 2020-04-23 16:40 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel, Daniel De Graaf,
	Volodymyr Babchuk, Roger Pau Monné

On 21.04.2020 18:11, Anthony PERARD wrote:
> Instead of generating the CFLAGS in Rules.mk everytime we enter a new
> subdirectory, we are going to generate most of them a single time, and
> export the result in the environment so that Rules.mk can use it.  The
> only flags left to be generated are the ones that depend on the
> targets, but the variable $(c_flags) takes care of that.
> 
> Arch specific CFLAGS are generated by a new file "arch/*/arch.mk"
> which is included by the root Makefile.
> 
> We export the *FLAGS via the environment variables XEN_*FLAGS because
> Rules.mk still includes Config.mk and would add duplicated flags to
> CFLAGS.
> 
> When running Rules.mk in the root directory (xen/), the variable
> `root-make-done' is set, so `need-config' will remain undef and so the
> root Makefile will not generate the cflags again.
> 
> We can't use CFLAGS in subdirectories to add flags to particular
> targets, instead start to use CFLAGS-y. Idem for AFLAGS.
> So there are two different CFLAGS-y, the one in xen/Makefile (and
> arch.mk), and the one in subdirs that Rules.mk is going to use.
> We can't add to XEN_CFLAGS because it is exported, so making change to
> it might be propagated to subdirectory which isn't intended.
> 
> Some style change are introduced in this patch:
>     when LDFLAGS_DIRECT is included in LDFLAGS
>     use of CFLAGS-$(CONFIG_INDIRECT_THUNK) instead of ifeq().
> 
> The LTO change hasn't been tested properly, as LTO is marked as
> broken.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with one further question:

> --- a/xen/arch/x86/Rules.mk
> +++ b/xen/arch/x86/Rules.mk
> @@ -1,89 +1,10 @@
>  ########################################
>  # x86-specific definitions
>  
> -XEN_IMG_OFFSET := 0x200000
> -
> -CFLAGS += -I$(BASEDIR)/include
> -CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
> -CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
> -CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
> -CFLAGS += '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))'
> -
> -# Prevent floating-point variables from creeping into Xen.
> -CFLAGS += -msoft-float
> -
> -ifeq ($(CONFIG_CC_IS_CLANG),y)
> -# Note: Any test which adds -no-integrated-as will cause subsequent tests to
> -# succeed, and not trigger further additions.
> -#
> -# The tests to select whether the integrated assembler is usable need to happen
> -# before testing any assembler features, or else the result of the tests would
> -# be stale if the integrated assembler is not used.
> -
> -# Older clang's built-in assembler doesn't understand .skip with labels:
> -# https://bugs.llvm.org/show_bug.cgi?id=27369
> -$(call as-option-add,CFLAGS,CC,".L0: .L1: .skip (.L1 - .L0)",,\
> -                     -no-integrated-as)
> -
> -# Check whether clang asm()-s support .include.
> -$(call as-option-add,CFLAGS,CC,".include \"asm-x86/indirect_thunk_asm.h\"",,\
> -                     -no-integrated-as)
> -
> -# Check whether clang keeps .macro-s between asm()-s:
> -# https://bugs.llvm.org/show_bug.cgi?id=36110
> -$(call as-option-add,CFLAGS,CC,\
> -                     ".macro FOO;.endm"$$(close); asm volatile $$(open)".macro FOO;.endm",\
> -                     -no-integrated-as)
> -endif
> -
> -$(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,"clwb (%rax)",-DHAVE_AS_CLWB)
> -$(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1", \
> -                     -U__OBJECT_LABEL__ -DHAVE_AS_QUOTED_SYM \
> -                     '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$$@')
> -$(call as-option-add,CFLAGS,CC,"invpcid (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID)
> -
> -# GAS's idea of true is -1.  Clang's idea is 1
> -$(call as-option-add,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,\
> -    ".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOPS_DIRECTIVE)
> -
> -CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables
> -
> -# Xen doesn't use SSE interally.  If the compiler supports it, also skip the
> -# SSE setup for variadic function calls.
> -CFLAGS += -mno-sse $(call cc-option,$(CC),-mskip-rax-setup)
> -
> -# Compile with thunk-extern, indirect-branch-register if avaiable.
> -ifeq ($(CONFIG_INDIRECT_THUNK),y)
> -CFLAGS += -mindirect-branch=thunk-extern -mindirect-branch-register
> -CFLAGS += -fno-jump-tables
> +ifneq ($(filter -DHAVE_AS_QUOTED_SYM,$(XEN_CFLAGS)),)
> +object_label_flags = '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$@'
> +else
> +object_label_flags = '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))'
>  endif
> -
> -# If supported by the compiler, reduce stack alignment to 8 bytes. But allow
> -# this to be overridden elsewhere.
> -$(call cc-option-add,CFLAGS-stack-boundary,CC,-mpreferred-stack-boundary=3)
> -CFLAGS += $(CFLAGS-stack-boundary)
> -
> -ifeq ($(CONFIG_UBSAN),y)
> -# Don't enable alignment sanitisation.  x86 has efficient unaligned accesses,
> -# and various things (ACPI tables, hypercall pages, stubs, etc) are wont-fix.
> -# It also causes an as-yet-unidentified crash on native boot before the
> -# console starts.
> -$(call cc-option-add,CFLAGS_UBSAN,CC,-fno-sanitize=alignment)
> -endif
> -
> -# Set up the assembler include path properly for older toolchains.
> -CFLAGS += -Wa,-I$(BASEDIR)/include
> -
> +c_flags += $(object_label_flags) $(CFLAGS-stack-boundary)
> +a_flags += $(object_label_flags) $(CFLAGS-stack-boundary)

Why are you also adding these to a_flags? It probably doesn't hurt,
but I'd prefer if it was removed (could be done while committing if
all acks arrive and other other need for av6 arises) if there's no
clear need.

Jan


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 08/16] build: Introduce $(cpp_flags)
  2020-04-21 16:12 ` [XEN PATCH v5 08/16] build: Introduce $(cpp_flags) Anthony PERARD
@ 2020-04-23 16:48   ` Jan Beulich
  2020-04-28 14:01     ` Anthony PERARD
  0 siblings, 1 reply; 41+ messages in thread
From: Jan Beulich @ 2020-04-23 16:48 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel

On 21.04.2020 18:12, Anthony PERARD wrote:
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -123,6 +123,7 @@ $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS))
>  
>  c_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_CFLAGS) '-D__OBJECT_FILE__="$@"'
>  a_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_AFLAGS)
> +cpp_flags = $(filter-out -Wa$(comma)%,$(a_flags))

I can see this happening to be this way right now, but in principle
I could see a_flags to hold items applicable to assembly files only,
but not to (the preprocessing of) C files. Hence while this is fine
for now, ...

> @@ -207,7 +208,7 @@ quiet_cmd_cc_s_c = CC      $@
>  cmd_cc_s_c = $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@
>  
>  quiet_cmd_s_S = CPP     $@
> -cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@
> +cmd_s_S = $(CPP) $(cpp_flags) $< -o $@

... this one is a trap waiting for someone to fall in imo. Instead
where I'd expect this patch to use $(cpp_flags) is e.g. in
xen/arch/x86/mm/Makefile:

guest_walk_%.i: guest_walk.c Makefile
	$(CPP) $(cpp_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@

And note how this currently uses $(c_flags), not $(a_flags), which
suggests that your deriving from $(a_flags) isn't correct either.

Jan


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 03/16] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS)
  2020-04-21 16:11 ` [XEN PATCH v5 03/16] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS) Anthony PERARD
@ 2020-04-24  9:20   ` Julien Grall
  0 siblings, 0 replies; 41+ messages in thread
From: Julien Grall @ 2020-04-24  9:20 UTC (permalink / raw)
  To: Anthony PERARD, xen-devel
  Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
	George Dunlap, Tim Deegan, Jan Beulich, Volodymyr Babchuk,
	Roger Pau Monné

Hi,

On 21/04/2020 17:11, Anthony PERARD wrote:
> In a later patch ("xen/build: have the root Makefile generates the
> CFLAGS), we want to generate the CFLAGS in xen/Makefile, then export
> it and have Rules.mk use a CFLAGS from the environment variables. That
> changes the flavor of the CFLAGS and flags intended for one target
> (like -D__OBJECT_FILE__ and -M%) gets propagated and duplicated. So we
> start by moving such flags out of $(CFLAGS) and into $(c_flags) which
> is to be modified by only Rules.mk.
> 
> __OBJECT_FILE__ is only used by arch/x86/mm/*.c files, so having it in
> $(c_flags) is enough, we don't need it in $(a_flags).
> 
> For include/Makefile and as-insn we can keep using CFLAGS, but since
> it doesn't have -M* flags anymore there is no need to filter them out.
> 
> The XEN_BUILD_EFI tests in arch/x86/Makefile was filtering out
> CFLAGS-y, but according to dd40177c1bc8 ("x86-64/EFI: add CFLAGS to
> check compile"), it was done to filter out -MF. CFLAGS doesn't
> have those flags anymore, so no filtering is needed.
> 
> This is inspired by the way Kbuild generates CFLAGS for each targets.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

For the Arm bits:

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

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS
  2020-04-21 16:11 ` [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS Anthony PERARD
  2020-04-23 16:40   ` Jan Beulich
@ 2020-04-24  9:26   ` Julien Grall
  2020-04-24 13:01   ` Jan Beulich
  2 siblings, 0 replies; 41+ messages in thread
From: Julien Grall @ 2020-04-24  9:26 UTC (permalink / raw)
  To: Anthony PERARD, xen-devel
  Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
	George Dunlap, Jan Beulich, Daniel De Graaf, Volodymyr Babchuk,
	Roger Pau Monné

Hi Anthony,

On 21/04/2020 17:11, Anthony PERARD wrote:
> Instead of generating the CFLAGS in Rules.mk everytime we enter a new
> subdirectory, we are going to generate most of them a single time, and
> export the result in the environment so that Rules.mk can use it.  The
> only flags left to be generated are the ones that depend on the
> targets, but the variable $(c_flags) takes care of that.
> 
> Arch specific CFLAGS are generated by a new file "arch/*/arch.mk"
> which is included by the root Makefile.
> 
> We export the *FLAGS via the environment variables XEN_*FLAGS because
> Rules.mk still includes Config.mk and would add duplicated flags to
> CFLAGS.
> 
> When running Rules.mk in the root directory (xen/), the variable
> `root-make-done' is set, so `need-config' will remain undef and so the
> root Makefile will not generate the cflags again.
> 
> We can't use CFLAGS in subdirectories to add flags to particular
> targets, instead start to use CFLAGS-y. Idem for AFLAGS.
> So there are two different CFLAGS-y, the one in xen/Makefile (and
> arch.mk), and the one in subdirs that Rules.mk is going to use.
> We can't add to XEN_CFLAGS because it is exported, so making change to
> it might be propagated to subdirectory which isn't intended.
> 
> Some style change are introduced in this patch:
>      when LDFLAGS_DIRECT is included in LDFLAGS
>      use of CFLAGS-$(CONFIG_INDIRECT_THUNK) instead of ifeq().
> 
> The LTO change hasn't been tested properly, as LTO is marked as
> broken.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

For the Arm bits:

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

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 07/16] xen/build: Start using if_changed
  2020-04-21 16:11 ` [XEN PATCH v5 07/16] xen/build: Start using if_changed Anthony PERARD
@ 2020-04-24  9:28   ` Julien Grall
  0 siblings, 0 replies; 41+ messages in thread
From: Julien Grall @ 2020-04-24  9:28 UTC (permalink / raw)
  To: Anthony PERARD, xen-devel
  Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
	George Dunlap, Jan Beulich, Daniel De Graaf, Volodymyr Babchuk,
	Roger Pau Monné

Hi,

On 21/04/2020 17:11, Anthony PERARD wrote:
> This patch start to use if_changed introduced in a previous commit.
> 
> Whenever if_changed is called, the target must have FORCE as
> dependency so that if_changed can check if the command line to be
> run has changed, so the macro $(real-prereqs) must be used to
> discover the dependencies without "FORCE".
> 
> Whenever a target isn't in obj-y, it should be added to extra-y so the
> .*.cmd dependency file associated with the target can be loaded. This
> is done for xsm/flask/ and both common/lib{elf,fdt}/ and
> arch/x86/Makefile.
> 
> For the targets that generate .*.d dependency files, there's going to
> be two dependency files (.*.d and .*.cmd) until we can merge them
> together in a later patch via fixdep from Linux.
> 
> One cleanup, libelf-relocate.o doesn't exist anymore.
> 
> We import cmd_ld and cmd_objcopy from Linux v5.4.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>

For the Arm bits:

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

Cheers,

> ---
> 
> Notes:
>      v4:
>      - typos
>      - fix missing FORCE in libfdt/Makefile
>      - typo flask vs flash in xsm
>      - in xsm, use *_H_DEPEND in command lines of mkaccess and mkflask
>        instead of $(real-prereq) to avoid including the script as argument of
>        itself.
> 
>   xen/Rules.mk               | 68 +++++++++++++++++++++++++++-----------
>   xen/arch/arm/Makefile      |  4 +--
>   xen/arch/x86/Makefile      |  1 +
>   xen/arch/x86/efi/Makefile  |  7 ++--
>   xen/common/libelf/Makefile | 12 ++++---
>   xen/common/libfdt/Makefile | 11 +++---
>   xen/xsm/flask/Makefile     | 17 +++++++---
>   7 files changed, 85 insertions(+), 35 deletions(-)
> 
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index 21cac7f5f51a..2e28c572305a 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -56,6 +56,18 @@ SPECIAL_DATA_SECTIONS := rodata $(foreach a,1 2 4 8 16, \
>   
>   include Makefile
>   
> +# Linking
> +# ---------------------------------------------------------------------------
> +
> +quiet_cmd_ld = LD      $@
> +cmd_ld = $(LD) $(XEN_LDFLAGS) -r -o $@ $(real-prereqs)
> +
> +# Objcopy
> +# ---------------------------------------------------------------------------
> +
> +quiet_cmd_objcopy = OBJCOPY $@
> +cmd_objcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $< $@
> +
>   define gendep
>       ifneq ($(1),$(subst /,:,$(1)))
>           DEPS += $(dir $(1)).$(notdir $(1)).d
> @@ -164,29 +176,47 @@ else
>   	$(CC) $(c_flags) -c $< -o $@
>   endif
>   
> -%.o: %.S Makefile
> -	$(CC) $(a_flags) -c $< -o $@
> +quiet_cmd_cc_o_S = CC      $@
> +cmd_cc_o_S = $(CC) $(a_flags) -c $< -o $@
> +
> +%.o: %.S FORCE
> +	$(call if_changed,cc_o_S)
> +
> +
> +quiet_cmd_obj_init_o = INIT_O  $@
> +define cmd_obj_init_o
> +    $(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p;}' | while read idx name sz rest; do \
> +        case "$$name" in \
> +        .*.local) ;; \
> +        .text|.text.*|.data|.data.*|.bss) \
> +            test $$sz != 0 || continue; \
> +            echo "Error: size of $<:$$name is 0x$$sz" >&2; \
> +            exit $$(expr $$idx + 1);; \
> +        esac; \
> +    done; \
> +    $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
> +endef
> +
> +$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o FORCE
> +	$(call if_changed,obj_init_o)
> +
> +quiet_cmd_cpp_i_c = CPP     $@
> +cmd_cpp_i_c = $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) $< -o $@
> +
> +quiet_cmd_cc_s_c = CC      $@
> +cmd_cc_s_c = $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@
>   
> -$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o Makefile
> -	$(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p;}' | while read idx name sz rest; do \
> -		case "$$name" in \
> -		.*.local) ;; \
> -		.text|.text.*|.data|.data.*|.bss) \
> -			test $$sz != 0 || continue; \
> -			echo "Error: size of $<:$$name is 0x$$sz" >&2; \
> -			exit $$(expr $$idx + 1);; \
> -		esac; \
> -	done
> -	$(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
> +quiet_cmd_s_S = CPP     $@
> +cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@
>   
> -%.i: %.c Makefile
> -	$(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) $< -o $@
> +%.i: %.c FORCE
> +	$(call if_changed,cpp_i_c)
>   
> -%.s: %.c Makefile
> -	$(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@
> +%.s: %.c FORCE
> +	$(call if_changed,cc_s_c)
>   
> -%.s: %.S Makefile
> -	$(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@
> +%.s: %.S FORCE
> +	$(call if_changed,cpp_s_S)
>   
>   # Add intermediate targets:
>   # When building objects with specific suffix patterns, add intermediate
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 9f1ab2335756..b79ad55646bd 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -97,8 +97,8 @@ prelink_lto.o: $(ALL_OBJS)
>   prelink.o: $(patsubst %/built_in.o,%/built_in_bin.o,$(ALL_OBJS)) prelink_lto.o
>   	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
>   else
> -prelink.o: $(ALL_OBJS)
> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
> +prelink.o: $(ALL_OBJS) FORCE
> +	$(call if_changed,ld)
>   endif
>   
>   $(TARGET)-syms: prelink.o xen.lds
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index a805e9982e85..44137d919b66 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -71,6 +71,7 @@ obj-$(CONFIG_TBOOT) += tboot.o
>   obj-y += hpet.o
>   obj-y += vm_event.o
>   obj-y += xstate.o
> +extra-y += asm-macros.i
>   
>   x86_emulate.o: x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
>   
> diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
> index 490d791aae2d..3e4c395b7535 100644
> --- a/xen/arch/x86/efi/Makefile
> +++ b/xen/arch/x86/efi/Makefile
> @@ -1,7 +1,10 @@
>   CFLAGS-y += -fshort-wchar
>   
> -%.o: %.ihex
> -	$(OBJCOPY) -I ihex -O binary $< $@
> +quiet_cmd_objcopy_o_ihex = OBJCOPY $@
> +cmd_objcopy_o_ihex = $(OBJCOPY) -I ihex -O binary $< $@
> +
> +%.o: %.ihex FORCE
> +	$(call if_changed,objcopy_o_ihex)
>   
>   boot.init.o: buildid.o
>   
> diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile
> index 464c448d9d37..a92326c982e9 100644
> --- a/xen/common/libelf/Makefile
> +++ b/xen/common/libelf/Makefile
> @@ -1,12 +1,16 @@
>   obj-bin-y := libelf.o
>   nocov-y += libelf.o
> +libelf-objs := libelf-tools.o libelf-loader.o libelf-dominfo.o
>   
>   SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
> +OBJCOPYFLAGS := $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s))
>   
>   CFLAGS-y += -Wno-pointer-sign
>   
> -libelf.o: libelf-temp.o Makefile
> -	$(OBJCOPY) $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
> +libelf.o: libelf-temp.o FORCE
> +	$(call if_changed,objcopy)
>   
> -libelf-temp.o: libelf-tools.o libelf-loader.o libelf-dominfo.o #libelf-relocate.o
> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
> +libelf-temp.o: $(libelf-objs) FORCE
> +	$(call if_changed,ld)
> +
> +extra-y += libelf-temp.o $(libelf-objs)
> diff --git a/xen/common/libfdt/Makefile b/xen/common/libfdt/Makefile
> index e2a5e59380a0..6bd207cf8ffa 100644
> --- a/xen/common/libfdt/Makefile
> +++ b/xen/common/libfdt/Makefile
> @@ -1,14 +1,17 @@
>   include Makefile.libfdt
>   
>   SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
> +OBJCOPYFLAGS := $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s))
>   
>   obj-y += libfdt.o
>   nocov-y += libfdt.o
>   
>   CFLAGS-y += -I$(BASEDIR)/include/xen/libfdt/
>   
> -libfdt.o: libfdt-temp.o Makefile
> -	$(OBJCOPY) $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
> +libfdt.o: libfdt-temp.o FORCE
> +	$(call if_changed,objcopy)
>   
> -libfdt-temp.o: $(LIBFDT_OBJS)
> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
> +libfdt-temp.o: $(LIBFDT_OBJS) FORCE
> +	$(call if_changed,ld)
> +
> +extra-y += libfdt-temp.o $(LIBFDT_OBJS)
> diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
> index 6db396347b1f..eebfceecc5fb 100644
> --- a/xen/xsm/flask/Makefile
> +++ b/xen/xsm/flask/Makefile
> @@ -20,12 +20,21 @@ AV_H_FILES = include/av_perm_to_string.h include/av_permissions.h
>   ALL_H_FILES = $(FLASK_H_FILES) $(AV_H_FILES)
>   
>   $(obj-y) ss/built_in.o: $(ALL_H_FILES)
> +extra-y += $(ALL_H_FILES)
>   
> -$(subst include/,%/,$(FLASK_H_FILES)): $(FLASK_H_DEPEND)
> -	$(CONFIG_SHELL) policy/mkflask.sh $(AWK) include $(FLASK_H_DEPEND)
> +mkflask := policy/mkflask.sh
> +quiet_cmd_mkflask = MKFLASK $@
> +cmd_mkflask = $(CONFIG_SHELL) $(mkflask) $(AWK) include $(FLASK_H_DEPEND)
>   
> -$(subst include/,%/,$(AV_H_FILES)): $(AV_H_DEPEND)
> -	$(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)
> +$(subst include/,%/,$(FLASK_H_FILES)): $(FLASK_H_DEPEND) $(mkflask) FORCE
> +	$(call if_changed,mkflask)
> +
> +mkaccess := policy/mkaccess_vector.sh
> +quiet_cmd_mkaccess = MKACCESS VECTOR $@
> +cmd_mkaccess = $(CONFIG_SHELL) $(mkaccess) $(AWK) $(AV_H_DEPEND)
> +
> +$(subst include/,%/,$(AV_H_FILES)): $(AV_H_DEPEND) $(mkaccess) FORCE
> +	$(call if_changed,mkaccess)
>   
>   obj-bin-$(CONFIG_XSM_FLASK_POLICY) += flask-policy.o
>   flask-policy.o: policy.bin
> 

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 11/16] xen/build: factorise generation of the linker scripts
  2020-04-21 16:12 ` [XEN PATCH v5 11/16] xen/build: factorise generation of the linker scripts Anthony PERARD
@ 2020-04-24  9:29   ` Julien Grall
  2020-04-28 13:50   ` Jan Beulich
  1 sibling, 0 replies; 41+ messages in thread
From: Julien Grall @ 2020-04-24  9:29 UTC (permalink / raw)
  To: Anthony PERARD, xen-devel
  Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
	George Dunlap, Jan Beulich, Volodymyr Babchuk,
	Roger Pau Monné



On 21/04/2020 17:12, Anthony PERARD wrote:
> In Arm and X86 makefile, generating the linker script is the same, so
> we can simply have both call the same macro.
> 
> We need to add *.lds files into extra-y so that Rules.mk can find the
> .*.cmd dependency file and load it.
> 
> Change made to the command line:
> - Use of $(CPP) instead of $(CC) -E
> - Remove -Ui386.
>    We don't compile Xen for i386 anymore, so that macro is never
>    defined. Also it doesn't make sense on Arm.
> - Use $(cpp_flags) which simply filter -Wa,% options from $(a_flags).
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

For the Arm bits:

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

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS
  2020-04-23 16:40   ` Jan Beulich
@ 2020-04-24  9:45     ` Anthony PERARD
  2020-04-24 11:20       ` Jan Beulich
  0 siblings, 1 reply; 41+ messages in thread
From: Anthony PERARD @ 2020-04-24  9:45 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel, Daniel De Graaf,
	Volodymyr Babchuk, Roger Pau Monné

On Thu, Apr 23, 2020 at 06:40:33PM +0200, Jan Beulich wrote:
> On 21.04.2020 18:11, Anthony PERARD wrote:
> > Instead of generating the CFLAGS in Rules.mk everytime we enter a new
> > subdirectory, we are going to generate most of them a single time, and
> > export the result in the environment so that Rules.mk can use it.  The
> > only flags left to be generated are the ones that depend on the
> > targets, but the variable $(c_flags) takes care of that.
> > 
> > Arch specific CFLAGS are generated by a new file "arch/*/arch.mk"
> > which is included by the root Makefile.
> > 
> > We export the *FLAGS via the environment variables XEN_*FLAGS because
> > Rules.mk still includes Config.mk and would add duplicated flags to
> > CFLAGS.
> > 
> > When running Rules.mk in the root directory (xen/), the variable
> > `root-make-done' is set, so `need-config' will remain undef and so the
> > root Makefile will not generate the cflags again.
> > 
> > We can't use CFLAGS in subdirectories to add flags to particular
> > targets, instead start to use CFLAGS-y. Idem for AFLAGS.
> > So there are two different CFLAGS-y, the one in xen/Makefile (and
> > arch.mk), and the one in subdirs that Rules.mk is going to use.
> > We can't add to XEN_CFLAGS because it is exported, so making change to
> > it might be propagated to subdirectory which isn't intended.
> > 
> > Some style change are introduced in this patch:
> >     when LDFLAGS_DIRECT is included in LDFLAGS
> >     use of CFLAGS-$(CONFIG_INDIRECT_THUNK) instead of ifeq().
> > 
> > The LTO change hasn't been tested properly, as LTO is marked as
> > broken.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with one further question:
> 
> > --- a/xen/arch/x86/Rules.mk
> > +++ b/xen/arch/x86/Rules.mk
[..]
> > +c_flags += $(object_label_flags) $(CFLAGS-stack-boundary)
> > +a_flags += $(object_label_flags) $(CFLAGS-stack-boundary)
> 
> Why are you also adding these to a_flags? It probably doesn't hurt,
> but I'd prefer if it was removed (could be done while committing if
> all acks arrive and other other need for av6 arises) if there's no
> clear need.

Those flags are already in a_flags (or previously AFLAGS). I've tried to
be careful to avoid changing the list of *flags in an already
complicated enough patch. I would like to keep this patch that way.

If the -D__OBJECT_LABEL__ and the stack-bondary flags aren't needed in
a_flags, it would be better to remove them in a separated patch, with
some explanation on why they are removed.

What is av6?

Thanks,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS
  2020-04-24  9:45     ` Anthony PERARD
@ 2020-04-24 11:20       ` Jan Beulich
  0 siblings, 0 replies; 41+ messages in thread
From: Jan Beulich @ 2020-04-24 11:20 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel, Daniel De Graaf,
	Volodymyr Babchuk, Roger Pau Monné

On 24.04.2020 11:45, Anthony PERARD wrote:
> On Thu, Apr 23, 2020 at 06:40:33PM +0200, Jan Beulich wrote:
>> On 21.04.2020 18:11, Anthony PERARD wrote:
>>> --- a/xen/arch/x86/Rules.mk
>>> +++ b/xen/arch/x86/Rules.mk
> [..]
>>> +c_flags += $(object_label_flags) $(CFLAGS-stack-boundary)
>>> +a_flags += $(object_label_flags) $(CFLAGS-stack-boundary)
>>
>> Why are you also adding these to a_flags? It probably doesn't hurt,
>> but I'd prefer if it was removed (could be done while committing if
>> all acks arrive and other other need for av6 arises) if there's no
>> clear need.
> 
> Those flags are already in a_flags (or previously AFLAGS). I've tried to
> be careful to avoid changing the list of *flags in an already
> complicated enough patch. I would like to keep this patch that way.
> 
> If the -D__OBJECT_LABEL__ and the stack-bondary flags aren't needed in
> a_flags, it would be better to remove them in a separated patch, with
> some explanation on why they are removed.

Ah, fair point - previously we had

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

and hence implicitly inherited all kinds of inapplicable flags.
I'm fine with this (and probably other things) getting taken care
of later then.

> What is av6?

I was missing a blank: "a v6". Also s/other other/no other".

Jan


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS
  2020-04-21 16:11 ` [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS Anthony PERARD
  2020-04-23 16:40   ` Jan Beulich
  2020-04-24  9:26   ` Julien Grall
@ 2020-04-24 13:01   ` Jan Beulich
  2020-04-24 13:30     ` Anthony PERARD
  2 siblings, 1 reply; 41+ messages in thread
From: Jan Beulich @ 2020-04-24 13:01 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel, Daniel De Graaf,
	Volodymyr Babchuk, Roger Pau Monné

On 21.04.2020 18:11, Anthony PERARD wrote:
> Instead of generating the CFLAGS in Rules.mk everytime we enter a new
> subdirectory, we are going to generate most of them a single time, and
> export the result in the environment so that Rules.mk can use it.  The
> only flags left to be generated are the ones that depend on the
> targets, but the variable $(c_flags) takes care of that.
> 
> Arch specific CFLAGS are generated by a new file "arch/*/arch.mk"
> which is included by the root Makefile.
> 
> We export the *FLAGS via the environment variables XEN_*FLAGS because
> Rules.mk still includes Config.mk and would add duplicated flags to
> CFLAGS.
> 
> When running Rules.mk in the root directory (xen/), the variable
> `root-make-done' is set, so `need-config' will remain undef and so the
> root Makefile will not generate the cflags again.
> 
> We can't use CFLAGS in subdirectories to add flags to particular
> targets, instead start to use CFLAGS-y. Idem for AFLAGS.
> So there are two different CFLAGS-y, the one in xen/Makefile (and
> arch.mk), and the one in subdirs that Rules.mk is going to use.
> We can't add to XEN_CFLAGS because it is exported, so making change to
> it might be propagated to subdirectory which isn't intended.
> 
> Some style change are introduced in this patch:
>     when LDFLAGS_DIRECT is included in LDFLAGS
>     use of CFLAGS-$(CONFIG_INDIRECT_THUNK) instead of ifeq().
> 
> The LTO change hasn't been tested properly, as LTO is marked as
> broken.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

While committing this, in my pre-push build test I noticed that
presumably an earlier change of yours has caused

Makefile:103: include/config/auto.conf: No such file or directory
Makefile:106: include/config/auto.conf.cmd: No such file or directory

for a build in a completely fresh tree.

Jan


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS
  2020-04-24 13:01   ` Jan Beulich
@ 2020-04-24 13:30     ` Anthony PERARD
  2020-04-24 14:07       ` Anthony PERARD
  0 siblings, 1 reply; 41+ messages in thread
From: Anthony PERARD @ 2020-04-24 13:30 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel, Daniel De Graaf,
	Volodymyr Babchuk, Roger Pau Monné

On Fri, Apr 24, 2020 at 03:01:32PM +0200, Jan Beulich wrote:
> While committing this, in my pre-push build test I noticed that
> presumably an earlier change of yours has caused
> 
> Makefile:103: include/config/auto.conf: No such file or directory
> Makefile:106: include/config/auto.conf.cmd: No such file or directory
> 
> for a build in a completely fresh tree.

Are those presumably "warning" an issue? Are the files still missing
after make has run? Didn't the build managed to build xen.gz?
There is maybe a line saying that make will re-execute.

I've seen those error before, on older version of make. But it's just
make complaining before even trying to update/create those files. But it
just create those files and start over.

Also, that would be patch "build: include include/config/auto.conf in
main Makefile" which introduce this behavior.

Thanks,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS
  2020-04-24 13:30     ` Anthony PERARD
@ 2020-04-24 14:07       ` Anthony PERARD
  0 siblings, 0 replies; 41+ messages in thread
From: Anthony PERARD @ 2020-04-24 14:07 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel, Daniel De Graaf,
	Volodymyr Babchuk, Roger Pau Monné

On Fri, Apr 24, 2020 at 02:30:56PM +0100, Anthony PERARD wrote:
> On Fri, Apr 24, 2020 at 03:01:32PM +0200, Jan Beulich wrote:
> > While committing this, in my pre-push build test I noticed that
> > presumably an earlier change of yours has caused
> > 
> > Makefile:103: include/config/auto.conf: No such file or directory
> > Makefile:106: include/config/auto.conf.cmd: No such file or directory
> > 
> > for a build in a completely fresh tree.
> 
> Are those presumably "warning" an issue? Are the files still missing
> after make has run? Didn't the build managed to build xen.gz?
> There is maybe a line saying that make will re-execute.
> 
> I've seen those error before, on older version of make. But it's just
> make complaining before even trying to update/create those files. But it
> just create those files and start over.
> 
> Also, that would be patch "build: include include/config/auto.conf in
> main Makefile" which introduce this behavior.

I'll prepare a patch to use "-include" instead of simply "include". That
will silent the warnings and nothing else should change. Linux's Kbuild
doesn't have this issue because we have to run `make menuconfig` or
equivalent which also generates the two auto.conf* files, which I didn't
notice until now.

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 09/16] xen/build: use if_changed on built_in.o
  2020-04-21 16:12 ` [XEN PATCH v5 09/16] xen/build: use if_changed on built_in.o Anthony PERARD
@ 2020-04-28 13:48   ` Jan Beulich
  2020-05-01 14:42     ` Anthony PERARD
  0 siblings, 1 reply; 41+ messages in thread
From: Jan Beulich @ 2020-04-28 13:48 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel

On 21.04.2020 18:12, Anthony PERARD wrote:
> In the case where $(obj-y) is empty, we also replace $(c_flags) by
> $(XEN_CFLAGS) to avoid generating an .%.d dependency file. This avoid
> make trying to include %.h file in the ld command if $(obj-y) isn't
> empty anymore on a second run.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

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

Personally I'd prefer ...

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -130,15 +130,24 @@ include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
>  c_flags += $(CFLAGS-y)
>  a_flags += $(CFLAGS-y) $(AFLAGS-y)
>  
> -built_in.o: $(obj-y) $(extra-y)
> -ifeq ($(obj-y),)
> -	$(CC) $(c_flags) -c -x c /dev/null -o $@
> -else
> +quiet_cmd_ld_builtin = LD      $@
>  ifeq ($(CONFIG_LTO),y)
> -	$(LD_LTO) -r -o $@ $(filter-out $(extra-y),$^)
> +cmd_ld_builtin = \
> +    $(LD_LTO) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
>  else
> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
> +cmd_ld_builtin = \
> +    $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
>  endif
> +
> +quiet_cmd_cc_builtin = LD      $@
> +cmd_cc_builtin = \
> +    $(CC) $(XEN_CFLAGS) -c -x c /dev/null -o $@
> +
> +built_in.o: $(obj-y) $(extra-y) FORCE
> +ifeq ($(obj-y),)
> +	$(call if_changed,cc_builtin)
> +else
> +	$(call if_changed,ld_builtin)
>  endif

...

   $(call if_changed,$(if $(obj-y),ld,cc)_builtin)

but perhaps I'm the only one.

Jan


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 11/16] xen/build: factorise generation of the linker scripts
  2020-04-21 16:12 ` [XEN PATCH v5 11/16] xen/build: factorise generation of the linker scripts Anthony PERARD
  2020-04-24  9:29   ` Julien Grall
@ 2020-04-28 13:50   ` Jan Beulich
  1 sibling, 0 replies; 41+ messages in thread
From: Jan Beulich @ 2020-04-28 13:50 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel, Volodymyr Babchuk,
	Roger Pau Monné

On 21.04.2020 18:12, Anthony PERARD wrote:
> In Arm and X86 makefile, generating the linker script is the same, so
> we can simply have both call the same macro.
> 
> We need to add *.lds files into extra-y so that Rules.mk can find the
> .*.cmd dependency file and load it.
> 
> Change made to the command line:
> - Use of $(CPP) instead of $(CC) -E
> - Remove -Ui386.
>   We don't compile Xen for i386 anymore, so that macro is never
>   defined. Also it doesn't make sense on Arm.
> - Use $(cpp_flags) which simply filter -Wa,% options from $(a_flags).
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

For the non-Arm bits
Acked-by: Jan Beulich <jbeulich@suse.com>



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 08/16] build: Introduce $(cpp_flags)
  2020-04-23 16:48   ` Jan Beulich
@ 2020-04-28 14:01     ` Anthony PERARD
  2020-04-28 14:20       ` Jan Beulich
  0 siblings, 1 reply; 41+ messages in thread
From: Anthony PERARD @ 2020-04-28 14:01 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel

On Thu, Apr 23, 2020 at 06:48:51PM +0200, Jan Beulich wrote:
> On 21.04.2020 18:12, Anthony PERARD wrote:
> > --- a/xen/Rules.mk
> > +++ b/xen/Rules.mk
> > @@ -123,6 +123,7 @@ $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS))
> >  
> >  c_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_CFLAGS) '-D__OBJECT_FILE__="$@"'
> >  a_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_AFLAGS)
> > +cpp_flags = $(filter-out -Wa$(comma)%,$(a_flags))
> 
> I can see this happening to be this way right now, but in principle
> I could see a_flags to hold items applicable to assembly files only,
> but not to (the preprocessing of) C files. Hence while this is fine
> for now, ...
> 
> > @@ -207,7 +208,7 @@ quiet_cmd_cc_s_c = CC      $@
> >  cmd_cc_s_c = $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@
> >  
> >  quiet_cmd_s_S = CPP     $@
> > -cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@
> > +cmd_s_S = $(CPP) $(cpp_flags) $< -o $@
> 
> ... this one is a trap waiting for someone to fall in imo. Instead
> where I'd expect this patch to use $(cpp_flags) is e.g. in
> xen/arch/x86/mm/Makefile:
> 
> guest_walk_%.i: guest_walk.c Makefile
> 	$(CPP) $(cpp_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
> 
> And note how this currently uses $(c_flags), not $(a_flags), which
> suggests that your deriving from $(a_flags) isn't correct either.

I think we can drop this patch for now, and change patch "xen/build:
factorise generation of the linker scripts" to not use $(cpp_flags).

If we derive $(cpp_flags) from $(c_flags) instead, we would need to
find out if CPP commands using a_flags can use c_flags instead.

On the other hand, I've looked at Linux source code, and they use
$(cpp_flags) for only a few targets, only to generate the .lds scripts.
For other rules, they use either a_flags or c_flags, for example:
    %.i: %.c ; uses $(c_flags)
    %.i: %.S ; uses $(a_flags)
    %.s: %.S ; uses $(a_flags)

(Also, they use -Qunused-arguments clang's options, so they don't need
to filter out -Wa,* arguments, I think.)

So, maybe having a single $(cpp_flags) when running the CPP command
isn't such a good idea.

So, would dropping $(cpp_flags) for now, and rework the *FLAGS later, be
good enough?

Thanks,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 13/16] xen,symbols: rework file symbols selection
  2020-04-21 16:12 ` [XEN PATCH v5 13/16] xen,symbols: rework file symbols selection Anthony PERARD
@ 2020-04-28 14:05   ` Jan Beulich
  0 siblings, 0 replies; 41+ messages in thread
From: Jan Beulich @ 2020-04-28 14:05 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel

On 21.04.2020 18:12, Anthony PERARD wrote:
> We want to use the same rune to build mm/*/guest_*.o as the one use to
> build every other *.o object. The consequence it that file symbols that
> the program ./symbols prefer changes with CONFIG_ENFORCE_UNIQUE_SYMBOLS=y.
> 
> For example, when building arch/x86/mm/guest_walk_2.o from guest_walk.c,
> this would be the difference of file symbol present in the object when
> building with CONFIG_ENFORCE_UNIQUE_SYMBOLS=y:
> 
> (1) Currently we have those two file symbols:
>     guest_walk.c
>     guest_walk_2.o
> (2) When building with the same rune, we will have:
>     arch/x86/mm/guest_walk.c
>     guest_walk_2.o

So I had to go to the v4 discussion to understand this again. As said
there, it may be obvious to you but despite having been the one to
introduce the objcopy step there, it is not to me. Hence my continued
desire to have this at least briefly mentioned here, as it's not
otherwise noticeable from looking at the patch itself.

The code change itself looks okay to me, but I'll want to ack this
change only once I can follow the description from a single pass of
reading through it. I'm sorry for the extra trouble.

Jan


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 14/16] build: use if_changed to build mm/*/guest_%.o
  2020-04-21 16:12 ` [XEN PATCH v5 14/16] build: use if_changed to build mm/*/guest_%.o Anthony PERARD
@ 2020-04-28 14:07   ` Jan Beulich
  0 siblings, 0 replies; 41+ messages in thread
From: Jan Beulich @ 2020-04-28 14:07 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Wei Liu, Andrew Cooper, Tim Deegan, George Dunlap, xen-devel,
	Roger Pau Monné

On 21.04.2020 18:12, Anthony PERARD wrote:
> Use if_changed for building all guest_%.o objects, and make use of
> command that already exist.
> 
> The current command only runs `CC`, but the runes to build every other
> object in Xen also runs `objcopy` (when CONFIG_ENFORCE_UNIQUE_SYMBOLS=y)
> which modify the file symbol. But with patch
> "xen,symbols: rework file symbols selection", ./symbols should still
> select the file symbols directive intended to be used for guest_%.o
> objects.
> 
> The goal here is to reduce the number of commands written in
> makefiles.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

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



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 08/16] build: Introduce $(cpp_flags)
  2020-04-28 14:01     ` Anthony PERARD
@ 2020-04-28 14:20       ` Jan Beulich
  2020-05-01 14:32         ` Anthony PERARD
  0 siblings, 1 reply; 41+ messages in thread
From: Jan Beulich @ 2020-04-28 14:20 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel

On 28.04.2020 16:01, Anthony PERARD wrote:
> On Thu, Apr 23, 2020 at 06:48:51PM +0200, Jan Beulich wrote:
>> On 21.04.2020 18:12, Anthony PERARD wrote:
>>> --- a/xen/Rules.mk
>>> +++ b/xen/Rules.mk
>>> @@ -123,6 +123,7 @@ $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS))
>>>  
>>>  c_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_CFLAGS) '-D__OBJECT_FILE__="$@"'
>>>  a_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_AFLAGS)
>>> +cpp_flags = $(filter-out -Wa$(comma)%,$(a_flags))
>>
>> I can see this happening to be this way right now, but in principle
>> I could see a_flags to hold items applicable to assembly files only,
>> but not to (the preprocessing of) C files. Hence while this is fine
>> for now, ...
>>
>>> @@ -207,7 +208,7 @@ quiet_cmd_cc_s_c = CC      $@
>>>  cmd_cc_s_c = $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@
>>>  
>>>  quiet_cmd_s_S = CPP     $@
>>> -cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@
>>> +cmd_s_S = $(CPP) $(cpp_flags) $< -o $@
>>
>> ... this one is a trap waiting for someone to fall in imo. Instead
>> where I'd expect this patch to use $(cpp_flags) is e.g. in
>> xen/arch/x86/mm/Makefile:
>>
>> guest_walk_%.i: guest_walk.c Makefile
>> 	$(CPP) $(cpp_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
>>
>> And note how this currently uses $(c_flags), not $(a_flags), which
>> suggests that your deriving from $(a_flags) isn't correct either.
> 
> I think we can drop this patch for now, and change patch "xen/build:
> factorise generation of the linker scripts" to not use $(cpp_flags).
> 
> If we derive $(cpp_flags) from $(c_flags) instead, we would need to
> find out if CPP commands using a_flags can use c_flags instead.
> 
> On the other hand, I've looked at Linux source code, and they use
> $(cpp_flags) for only a few targets, only to generate the .lds scripts.
> For other rules, they use either a_flags or c_flags, for example:
>     %.i: %.c ; uses $(c_flags)
>     %.i: %.S ; uses $(a_flags)
>     %.s: %.S ; uses $(a_flags)

The first on really ought to be use cpp_flags. I couldn't find the
middle one. The last one clearly has to do something about -Wa,
options, but apart from this I'd consider a_flags appropriate to
use there.

> (Also, they use -Qunused-arguments clang's options, so they don't need
> to filter out -Wa,* arguments, I think.)

Maybe we should do so too then?

> So, maybe having a single $(cpp_flags) when running the CPP command
> isn't such a good idea.

Right - after all in particular the use of CPP to produce .lds is
an abuse, as the source file (named .lds.S) isn't really what its
name says.

> So, would dropping $(cpp_flags) for now, and rework the *FLAGS later, be
> good enough?

I don't think so, no, I'm sorry. cpp_flags should be there for its
real purpose. Whether the .lds.S -> .lds rule can use it, or should
use a_flags, or yet something else is a different thing.

Jan


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 15/16] build, include: rework compat-build-source.py
  2020-04-21 16:12 ` [XEN PATCH v5 15/16] build,include: rework compat-build-source.py Anthony PERARD
@ 2020-04-28 14:37   ` Jan Beulich
  0 siblings, 0 replies; 41+ messages in thread
From: Jan Beulich @ 2020-04-28 14:37 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel

On 21.04.2020 18:12, Anthony PERARD wrote:
> Improvement are:
> - give the path to xlat.lst as argument
> - include `grep -v` in compat-build-source.py script, we don't need to
>   write this in several scripted language.
> 
> No changes in final compat/%.h headers.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

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



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 16/16] build, include: rework compat-build-header.py
  2020-04-21 16:12 ` [XEN PATCH v5 16/16] build,include: rework compat-build-header.py Anthony PERARD
@ 2020-04-28 14:39   ` Jan Beulich
  2020-04-28 14:54   ` Wei Liu
  1 sibling, 0 replies; 41+ messages in thread
From: Jan Beulich @ 2020-04-28 14:39 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel

On 21.04.2020 18:12, Anthony PERARD wrote:
> Replace a mix of shell script and python script by all python script.
> 
> No change to the final generated headers.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> 
> Notes:
>     v5:
>     - Removed -P from CPP when generating compat/%.i
>       -> keep removing linemarkers and keep de-duplicating empty lines.
>       So that all the blank line that currently exist in the generated
>       headers stays in place.

Thanks for doing these adjustments. However, my Python isn't good
enough to actually ack this patch, I'm afraid.

Jan


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 16/16] build, include: rework compat-build-header.py
  2020-04-21 16:12 ` [XEN PATCH v5 16/16] build,include: rework compat-build-header.py Anthony PERARD
  2020-04-28 14:39   ` [XEN PATCH v5 16/16] build, include: " Jan Beulich
@ 2020-04-28 14:54   ` Wei Liu
  1 sibling, 0 replies; 41+ messages in thread
From: Wei Liu @ 2020-04-28 14:54 UTC (permalink / raw)
  To: Anthony PERARD, xen-devel
  Cc: Stefano Stabellini, Julien Grall, wl, Andrew Cooper, Ian Jackson,
	George Dunlap, Jan Beulich



On 21/04/2020 17:12, Anthony PERARD wrote:
> Replace a mix of shell script and python script by all python script.
> 
> No change to the final generated headers.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

To the best of my knowledge this patch is doing the right transformation.

Acked-by: Wei Liu <wl@xen.org>



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 08/16] build: Introduce $(cpp_flags)
  2020-04-28 14:20       ` Jan Beulich
@ 2020-05-01 14:32         ` Anthony PERARD
  2020-05-04  9:09           ` Jan Beulich
  0 siblings, 1 reply; 41+ messages in thread
From: Anthony PERARD @ 2020-05-01 14:32 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel

On Tue, Apr 28, 2020 at 04:20:57PM +0200, Jan Beulich wrote:
> On 28.04.2020 16:01, Anthony PERARD wrote:
> > On Thu, Apr 23, 2020 at 06:48:51PM +0200, Jan Beulich wrote:
> >> On 21.04.2020 18:12, Anthony PERARD wrote:
> >>> --- a/xen/Rules.mk
> >>> +++ b/xen/Rules.mk
> >>> @@ -123,6 +123,7 @@ $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS))
> >>>  
> >>>  c_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_CFLAGS) '-D__OBJECT_FILE__="$@"'
> >>>  a_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_AFLAGS)
> >>> +cpp_flags = $(filter-out -Wa$(comma)%,$(a_flags))
> >>
> >> I can see this happening to be this way right now, but in principle
> >> I could see a_flags to hold items applicable to assembly files only,
> >> but not to (the preprocessing of) C files. Hence while this is fine
> >> for now, ...
> >>
> >>> @@ -207,7 +208,7 @@ quiet_cmd_cc_s_c = CC      $@
> >>>  cmd_cc_s_c = $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@
> >>>  
> >>>  quiet_cmd_s_S = CPP     $@
> >>> -cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@
> >>> +cmd_s_S = $(CPP) $(cpp_flags) $< -o $@
> >>
> >> ... this one is a trap waiting for someone to fall in imo. Instead
> >> where I'd expect this patch to use $(cpp_flags) is e.g. in
> >> xen/arch/x86/mm/Makefile:
> >>
> >> guest_walk_%.i: guest_walk.c Makefile
> >> 	$(CPP) $(cpp_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
> >>
> >> And note how this currently uses $(c_flags), not $(a_flags), which
> >> suggests that your deriving from $(a_flags) isn't correct either.
> > 
> > I think we can drop this patch for now, and change patch "xen/build:
> > factorise generation of the linker scripts" to not use $(cpp_flags).
> > 
> > If we derive $(cpp_flags) from $(c_flags) instead, we would need to
> > find out if CPP commands using a_flags can use c_flags instead.
> > 
> > On the other hand, I've looked at Linux source code, and they use
> > $(cpp_flags) for only a few targets, only to generate the .lds scripts.
> > For other rules, they use either a_flags or c_flags, for example:
> >     %.i: %.c ; uses $(c_flags)
> >     %.i: %.S ; uses $(a_flags)
> >     %.s: %.S ; uses $(a_flags)
> 
> The first on really ought to be use cpp_flags. I couldn't find the
> middle one. The last one clearly has to do something about -Wa,
> options, but apart from this I'd consider a_flags appropriate to
> use there.
> 
> > (Also, they use -Qunused-arguments clang's options, so they don't need
> > to filter out -Wa,* arguments, I think.)
> 
> Maybe we should do so too then?
> 
> > So, maybe having a single $(cpp_flags) when running the CPP command
> > isn't such a good idea.
> 
> Right - after all in particular the use of CPP to produce .lds is
> an abuse, as the source file (named .lds.S) isn't really what its
> name says.
> 
> > So, would dropping $(cpp_flags) for now, and rework the *FLAGS later, be
> > good enough?
> 
> I don't think so, no, I'm sorry. cpp_flags should be there for its
> real purpose. Whether the .lds.S -> .lds rule can use it, or should
> use a_flags, or yet something else is a different thing.


OK. I think we can rework the patch to derive cpp_flags from c_flags,
use this new cpp_flags for %.i:%.c; but keep using a_flags for %.s:%.S.

As for the .lds, we could use this new cpp_flags, the only think I saw
missing was -D__ASSEMBLY__, which can be added to the command line.
(There would also be an extra -std=gnu99, but I don't think it matters.)

Does that sounds good?
(Just two patch to change, this one and the one reworking .lds
generation.)


As for using -Qunused-arguments with clang, I didn't managed to find the
documentation of clang's command line argument for clang 3.5 on llvm
website, but I found it for clang 5.0 and the option is listed there.
I've tested building Xen on our gitlab CI, which as debian jessie which
seems to have clang 3.5, and Xen built just fine. So that might be an
option we can use later, but probably only for CPP flags.

Thanks,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 09/16] xen/build: use if_changed on built_in.o
  2020-04-28 13:48   ` Jan Beulich
@ 2020-05-01 14:42     ` Anthony PERARD
  2020-05-04  9:11       ` Jan Beulich
  0 siblings, 1 reply; 41+ messages in thread
From: Anthony PERARD @ 2020-05-01 14:42 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel

On Tue, Apr 28, 2020 at 03:48:13PM +0200, Jan Beulich wrote:
> On 21.04.2020 18:12, Anthony PERARD wrote:
> > In the case where $(obj-y) is empty, we also replace $(c_flags) by
> > $(XEN_CFLAGS) to avoid generating an .%.d dependency file. This avoid
> > make trying to include %.h file in the ld command if $(obj-y) isn't
> > empty anymore on a second run.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> Personally I'd prefer ...
> 
> > --- a/xen/Rules.mk
> > +++ b/xen/Rules.mk
> > @@ -130,15 +130,24 @@ include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
> >  c_flags += $(CFLAGS-y)
> >  a_flags += $(CFLAGS-y) $(AFLAGS-y)
> >  
> > -built_in.o: $(obj-y) $(extra-y)
> > -ifeq ($(obj-y),)
> > -	$(CC) $(c_flags) -c -x c /dev/null -o $@
> > -else
> > +quiet_cmd_ld_builtin = LD      $@
> >  ifeq ($(CONFIG_LTO),y)
> > -	$(LD_LTO) -r -o $@ $(filter-out $(extra-y),$^)
> > +cmd_ld_builtin = \
> > +    $(LD_LTO) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
> >  else
> > -	$(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
> > +cmd_ld_builtin = \
> > +    $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
> >  endif
> > +
> > +quiet_cmd_cc_builtin = LD      $@
> > +cmd_cc_builtin = \
> > +    $(CC) $(XEN_CFLAGS) -c -x c /dev/null -o $@
> > +
> > +built_in.o: $(obj-y) $(extra-y) FORCE
> > +ifeq ($(obj-y),)
> > +	$(call if_changed,cc_builtin)
> > +else
> > +	$(call if_changed,ld_builtin)
> >  endif
> 
> ...
> 
>    $(call if_changed,$(if $(obj-y),ld,cc)_builtin)
> 
> but perhaps I'm the only one.

I think so. Spelling the full name of the command makes it easier to
look for where it is used, or for where it is defined.

Linux doesn't have this issue about checking $(obj-y) as they use 'ar'
to make archives of objects, an archive with 0 object is fine. But that
is something I'll look at later, to find out if it is better and why.

Thanks,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 08/16] build: Introduce $(cpp_flags)
  2020-05-01 14:32         ` Anthony PERARD
@ 2020-05-04  9:09           ` Jan Beulich
  0 siblings, 0 replies; 41+ messages in thread
From: Jan Beulich @ 2020-05-04  9:09 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel

On 01.05.2020 16:32, Anthony PERARD wrote:
> On Tue, Apr 28, 2020 at 04:20:57PM +0200, Jan Beulich wrote:
>> On 28.04.2020 16:01, Anthony PERARD wrote:
>>> On Thu, Apr 23, 2020 at 06:48:51PM +0200, Jan Beulich wrote:
>>>> On 21.04.2020 18:12, Anthony PERARD wrote:
>>>>> --- a/xen/Rules.mk
>>>>> +++ b/xen/Rules.mk
>>>>> @@ -123,6 +123,7 @@ $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS))
>>>>>  
>>>>>  c_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_CFLAGS) '-D__OBJECT_FILE__="$@"'
>>>>>  a_flags = -MMD -MP -MF $(@D)/.$(@F).d $(XEN_AFLAGS)
>>>>> +cpp_flags = $(filter-out -Wa$(comma)%,$(a_flags))
>>>>
>>>> I can see this happening to be this way right now, but in principle
>>>> I could see a_flags to hold items applicable to assembly files only,
>>>> but not to (the preprocessing of) C files. Hence while this is fine
>>>> for now, ...
>>>>
>>>>> @@ -207,7 +208,7 @@ quiet_cmd_cc_s_c = CC      $@
>>>>>  cmd_cc_s_c = $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@
>>>>>  
>>>>>  quiet_cmd_s_S = CPP     $@
>>>>> -cmd_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) $< -o $@
>>>>> +cmd_s_S = $(CPP) $(cpp_flags) $< -o $@
>>>>
>>>> ... this one is a trap waiting for someone to fall in imo. Instead
>>>> where I'd expect this patch to use $(cpp_flags) is e.g. in
>>>> xen/arch/x86/mm/Makefile:
>>>>
>>>> guest_walk_%.i: guest_walk.c Makefile
>>>> 	$(CPP) $(cpp_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@
>>>>
>>>> And note how this currently uses $(c_flags), not $(a_flags), which
>>>> suggests that your deriving from $(a_flags) isn't correct either.
>>>
>>> I think we can drop this patch for now, and change patch "xen/build:
>>> factorise generation of the linker scripts" to not use $(cpp_flags).
>>>
>>> If we derive $(cpp_flags) from $(c_flags) instead, we would need to
>>> find out if CPP commands using a_flags can use c_flags instead.
>>>
>>> On the other hand, I've looked at Linux source code, and they use
>>> $(cpp_flags) for only a few targets, only to generate the .lds scripts.
>>> For other rules, they use either a_flags or c_flags, for example:
>>>     %.i: %.c ; uses $(c_flags)
>>>     %.i: %.S ; uses $(a_flags)
>>>     %.s: %.S ; uses $(a_flags)
>>
>> The first on really ought to be use cpp_flags. I couldn't find the
>> middle one. The last one clearly has to do something about -Wa,
>> options, but apart from this I'd consider a_flags appropriate to
>> use there.
>>
>>> (Also, they use -Qunused-arguments clang's options, so they don't need
>>> to filter out -Wa,* arguments, I think.)
>>
>> Maybe we should do so too then?
>>
>>> So, maybe having a single $(cpp_flags) when running the CPP command
>>> isn't such a good idea.
>>
>> Right - after all in particular the use of CPP to produce .lds is
>> an abuse, as the source file (named .lds.S) isn't really what its
>> name says.
>>
>>> So, would dropping $(cpp_flags) for now, and rework the *FLAGS later, be
>>> good enough?
>>
>> I don't think so, no, I'm sorry. cpp_flags should be there for its
>> real purpose. Whether the .lds.S -> .lds rule can use it, or should
>> use a_flags, or yet something else is a different thing.
> 
> 
> OK. I think we can rework the patch to derive cpp_flags from c_flags,
> use this new cpp_flags for %.i:%.c; but keep using a_flags for %.s:%.S.
> 
> As for the .lds, we could use this new cpp_flags, the only think I saw
> missing was -D__ASSEMBLY__, which can be added to the command line.
> (There would also be an extra -std=gnu99, but I don't think it matters.)
> 
> Does that sounds good?

Yes. I had another thought though in the meantime: What if cpp_flags
became a macro to be used with $(call ), with c_flags or a_flags (or
whatever else) passed in by the use sites?

> As for using -Qunused-arguments with clang, I didn't managed to find the
> documentation of clang's command line argument for clang 3.5 on llvm
> website, but I found it for clang 5.0 and the option is listed there.
> I've tested building Xen on our gitlab CI, which as debian jessie which
> seems to have clang 3.5, and Xen built just fine. So that might be an
> option we can use later, but probably only for CPP flags.

Okay, thanks for checking.

Jan


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [XEN PATCH v5 09/16] xen/build: use if_changed on built_in.o
  2020-05-01 14:42     ` Anthony PERARD
@ 2020-05-04  9:11       ` Jan Beulich
  0 siblings, 0 replies; 41+ messages in thread
From: Jan Beulich @ 2020-05-04  9:11 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
	Ian Jackson, George Dunlap, xen-devel

On 01.05.2020 16:42, Anthony PERARD wrote:
> On Tue, Apr 28, 2020 at 03:48:13PM +0200, Jan Beulich wrote:
>> On 21.04.2020 18:12, Anthony PERARD wrote:
>>> In the case where $(obj-y) is empty, we also replace $(c_flags) by
>>> $(XEN_CFLAGS) to avoid generating an .%.d dependency file. This avoid
>>> make trying to include %.h file in the ld command if $(obj-y) isn't
>>> empty anymore on a second run.
>>>
>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>
>> Personally I'd prefer ...
>>
>>> --- a/xen/Rules.mk
>>> +++ b/xen/Rules.mk
>>> @@ -130,15 +130,24 @@ include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
>>>  c_flags += $(CFLAGS-y)
>>>  a_flags += $(CFLAGS-y) $(AFLAGS-y)
>>>  
>>> -built_in.o: $(obj-y) $(extra-y)
>>> -ifeq ($(obj-y),)
>>> -	$(CC) $(c_flags) -c -x c /dev/null -o $@
>>> -else
>>> +quiet_cmd_ld_builtin = LD      $@
>>>  ifeq ($(CONFIG_LTO),y)
>>> -	$(LD_LTO) -r -o $@ $(filter-out $(extra-y),$^)
>>> +cmd_ld_builtin = \
>>> +    $(LD_LTO) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
>>>  else
>>> -	$(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
>>> +cmd_ld_builtin = \
>>> +    $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$(real-prereqs))
>>>  endif
>>> +
>>> +quiet_cmd_cc_builtin = LD      $@
>>> +cmd_cc_builtin = \
>>> +    $(CC) $(XEN_CFLAGS) -c -x c /dev/null -o $@
>>> +
>>> +built_in.o: $(obj-y) $(extra-y) FORCE
>>> +ifeq ($(obj-y),)
>>> +	$(call if_changed,cc_builtin)
>>> +else
>>> +	$(call if_changed,ld_builtin)
>>>  endif
>>
>> ...
>>
>>    $(call if_changed,$(if $(obj-y),ld,cc)_builtin)
>>
>> but perhaps I'm the only one.
> 
> I think so. Spelling the full name of the command makes it easier to
> look for where it is used, or for where it is defined.

   $(call if_changed,$(if $(obj-y),ld_builtin,cc_builtin))

then?

Jan



^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2020-05-04  9:11 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-21 16:11 [XEN PATCH v5 00/16] xen: Build system improvements Anthony PERARD
2020-04-21 16:11 ` [XEN PATCH v5 01/16] build,xsm: Fix multiple call Anthony PERARD
2020-04-21 16:11 ` [XEN PATCH v5 02/16] xen/build: include include/config/auto.conf in main Makefile Anthony PERARD
2020-04-21 16:11 ` [XEN PATCH v5 03/16] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS) Anthony PERARD
2020-04-24  9:20   ` Julien Grall
2020-04-21 16:11 ` [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS Anthony PERARD
2020-04-23 16:40   ` Jan Beulich
2020-04-24  9:45     ` Anthony PERARD
2020-04-24 11:20       ` Jan Beulich
2020-04-24  9:26   ` Julien Grall
2020-04-24 13:01   ` Jan Beulich
2020-04-24 13:30     ` Anthony PERARD
2020-04-24 14:07       ` Anthony PERARD
2020-04-21 16:11 ` [XEN PATCH v5 05/16] build: Introduce documentation for xen Makefiles Anthony PERARD
2020-04-21 16:11 ` [XEN PATCH v5 06/16] xen/build: introduce if_changed and if_changed_rule Anthony PERARD
2020-04-21 16:11 ` [XEN PATCH v5 07/16] xen/build: Start using if_changed Anthony PERARD
2020-04-24  9:28   ` Julien Grall
2020-04-21 16:12 ` [XEN PATCH v5 08/16] build: Introduce $(cpp_flags) Anthony PERARD
2020-04-23 16:48   ` Jan Beulich
2020-04-28 14:01     ` Anthony PERARD
2020-04-28 14:20       ` Jan Beulich
2020-05-01 14:32         ` Anthony PERARD
2020-05-04  9:09           ` Jan Beulich
2020-04-21 16:12 ` [XEN PATCH v5 09/16] xen/build: use if_changed on built_in.o Anthony PERARD
2020-04-28 13:48   ` Jan Beulich
2020-05-01 14:42     ` Anthony PERARD
2020-05-04  9:11       ` Jan Beulich
2020-04-21 16:12 ` [XEN PATCH v5 10/16] xen/build: Use if_changed_rules with %.o:%.c targets Anthony PERARD
2020-04-21 16:12 ` [XEN PATCH v5 11/16] xen/build: factorise generation of the linker scripts Anthony PERARD
2020-04-24  9:29   ` Julien Grall
2020-04-28 13:50   ` Jan Beulich
2020-04-21 16:12 ` [XEN PATCH v5 12/16] xen/build: Use if_changed for prelink*.o Anthony PERARD
2020-04-21 16:12 ` [XEN PATCH v5 13/16] xen,symbols: rework file symbols selection Anthony PERARD
2020-04-28 14:05   ` Jan Beulich
2020-04-21 16:12 ` [XEN PATCH v5 14/16] build: use if_changed to build mm/*/guest_%.o Anthony PERARD
2020-04-28 14:07   ` Jan Beulich
2020-04-21 16:12 ` [XEN PATCH v5 15/16] build,include: rework compat-build-source.py Anthony PERARD
2020-04-28 14:37   ` [XEN PATCH v5 15/16] build, include: " Jan Beulich
2020-04-21 16:12 ` [XEN PATCH v5 16/16] build,include: rework compat-build-header.py Anthony PERARD
2020-04-28 14:39   ` [XEN PATCH v5 16/16] build, include: " Jan Beulich
2020-04-28 14:54   ` Wei Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.