[Xen-devel] [XEN PATCH v2 00/12] xen: Build system improvements

[Xen-devel] [XEN PATCH v2 00/12] xen: Build system improvements

[Anthony PERARD]

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

series is based on "[XEN PATCH v3 0/6] xen: Kconfig update with few extra"

v2:
Rather than taking Kbuild and making it work with Xen, the v2 takes the opposite
approach of slowly transforming our current build system into Kbuild. That have
the advantage of keeping all the feature we have and making the patches much
easier to review. Kconfig update is done in an other patch series.

v1:
https://lists.xenproject.org/archives/html/xen-devel/2019-10/msg01609.html

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.

XXX Known issue
- make dist-tests is broken. I'll fix that latter.
- efi build maybe broken (xen doesn't boot on albana which looks like to be one
  of the uefi host)

Cheers,

Anthony PERARD (12):
  xen/build: Remove left over -DMAX_PHYS_IRQS
  xen/build: Use obj-y += subdir/ instead of subdir-y
  xen/build: use $(clean) shorthand for clean targets
  xen/build: extract clean target from Rules.mk
  xen/include: remove include of Config.mk
  xen/test/livepatch: remove include of Config.mk
  xen/build: run targets csopes,tags,.. without Rules.mk
  xen/build: make tests in test/ directly
  xen/build: include include/config/auto.conf in main Makefile
  xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS)
  xen/build: introduce ccflags-y and CFLAGS_$@
  xen/build: have the root Makefile generates the CFLAGS

 xen/Makefile                         | 211 +++++++++++++++++++++------
 xen/Rules.mk                         | 135 +++++------------
 xen/arch/arm/Makefile                |  28 ++--
 xen/arch/arm/Rules.mk                |  93 ------------
 xen/arch/arm/arch.mk                 |  88 +++++++++++
 xen/arch/arm/arm32/Makefile          |   2 +-
 xen/arch/arm/arm64/Makefile          |   2 +-
 xen/arch/arm/efi/Makefile            |   2 +-
 xen/arch/x86/Makefile                |  46 +++---
 xen/arch/x86/Rules.mk                |  91 +-----------
 xen/arch/x86/acpi/Makefile           |   2 +-
 xen/arch/x86/arch.mk                 |  87 +++++++++++
 xen/arch/x86/cpu/Makefile            |   4 +-
 xen/arch/x86/efi/Makefile            |   2 +-
 xen/arch/x86/guest/Makefile          |   4 +-
 xen/arch/x86/hvm/Makefile            |   6 +-
 xen/arch/x86/mm/Makefile             |  10 +-
 xen/arch/x86/mm/hap/Makefile         |   6 +-
 xen/arch/x86/mm/shadow/Makefile      |   6 +-
 xen/arch/x86/x86_64/Makefile         |   2 +-
 xen/common/Makefile                  |   8 +-
 xen/common/libelf/Makefile           |   4 +-
 xen/common/libfdt/Makefile           |   4 +-
 xen/drivers/Makefile                 |  14 +-
 xen/drivers/acpi/Makefile            |   6 +-
 xen/drivers/passthrough/Makefile     |   8 +-
 xen/drivers/passthrough/vtd/Makefile |   2 +-
 xen/include/Makefile                 |   4 +-
 xen/lib/Makefile                     |   2 +-
 xen/scripts/Kbuild.include           |  10 ++
 xen/scripts/Makefile.clean           |  33 +++++
 xen/test/livepatch/Makefile          |   2 -
 xen/xsm/Makefile                     |   2 +-
 xen/xsm/flask/Makefile               |   4 +-
 xen/xsm/flask/ss/Makefile            |   2 +-
 35 files changed, 519 insertions(+), 413 deletions(-)
 create mode 100644 xen/arch/arm/arch.mk
 create mode 100644 xen/arch/x86/arch.mk
 create mode 100644 xen/scripts/Makefile.clean

-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2 01/12] xen/build: Remove left over -DMAX_PHYS_IRQS

[Anthony PERARD]

From: Anthony PERARD <anthony.perard@gmail.com>

The use of MAX_PHYS_IRQS have been removed in cf5e6f2d3441 ("x86:
eliminate hard-coded NR_IRQS"), so remove the left over CFLAGS.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Rules.mk | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index fcdafd029342..22f25c5b2be8 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -71,10 +71,6 @@ ifneq ($(CONFIG_CC_IS_CLANG),y)
 CFLAGS += -Wa,--strip-local-absolute
 endif
 
-ifneq ($(max_phys_irqs),)
-CFLAGS-y                += -DMAX_PHYS_IRQS=$(max_phys_irqs)
-endif
-
 AFLAGS-y                += -D__ASSEMBLY__
 
 ALL_OBJS := $(ALL_OBJS-y)
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 01/12] xen/build: Remove left over -DMAX_PHYS_IRQS

[Jan Beulich]

On 17.01.2020 11:53, Anthony PERARD wrote:
> From: Anthony PERARD <anthony.perard@gmail.com>
> 
> The use of MAX_PHYS_IRQS have been removed in cf5e6f2d3441 ("x86:
> eliminate hard-coded NR_IRQS"), so remove the left over CFLAGS.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2 02/12] xen/build: Use obj-y += subdir/ instead of subdir-y

[Anthony PERARD]

This is part of upgrading our build system and import more of Linux's
one.

In Linux, subdir-y in Makefiles is only used to descend into
subdirectory when there are no object to build, Xen doesn't have that
and all subdir have object to be included in the final binary.

To allow the new syntax, the "obj-y" and "subdir-*" calculation in
Rules.mk is changed and partially imported from Linux's Kbuild.

The command used to modify the Makefile was:
    sed -i -r 's#^subdir-(.*)#obj-\1/#;' **/Makefile

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Rules.mk                         | 21 ++++++++++-----------
 xen/arch/arm/Makefile                | 14 +++++++-------
 xen/arch/arm/arm32/Makefile          |  2 +-
 xen/arch/arm/arm64/Makefile          |  2 +-
 xen/arch/x86/Makefile                | 18 +++++++++---------
 xen/arch/x86/acpi/Makefile           |  2 +-
 xen/arch/x86/cpu/Makefile            |  4 ++--
 xen/arch/x86/guest/Makefile          |  4 ++--
 xen/arch/x86/hvm/Makefile            |  6 +++---
 xen/arch/x86/mm/Makefile             |  4 ++--
 xen/arch/x86/x86_64/Makefile         |  2 +-
 xen/common/Makefile                  |  8 ++++----
 xen/drivers/Makefile                 | 14 +++++++-------
 xen/drivers/acpi/Makefile            |  6 +++---
 xen/drivers/passthrough/Makefile     |  8 ++++----
 xen/drivers/passthrough/vtd/Makefile |  2 +-
 xen/lib/Makefile                     |  2 +-
 xen/xsm/Makefile                     |  2 +-
 xen/xsm/flask/Makefile               |  2 +-
 19 files changed, 61 insertions(+), 62 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 22f25c5b2be8..8b04cbdd24ca 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -105,17 +105,16 @@ define gendep
 endef
 $(foreach o,$(filter-out %/,$(obj-y) $(obj-bin-y) $(extra-y)),$(eval $(call gendep,$(o))))
 
-# Ensure each subdirectory has exactly one trailing slash.
-subdir-n := $(patsubst %,%/,$(patsubst %/,%,$(subdir-n) $(subdir-)))
-subdir-y := $(patsubst %,%/,$(patsubst %/,%,$(subdir-y)))
-
-# Add explicitly declared subdirectories to the object lists.
-obj-y += $(patsubst %/,%/built_in.o,$(subdir-y))
-
-# Add implicitly declared subdirectories (in the object lists) to the
-# subdirectory list, and rewrite the object-list entry.
-subdir-y += $(filter %/,$(obj-y))
-obj-y    := $(patsubst %/,%/built-in.o,$(obj-y))
+# Handle objects in subdirs
+# ---------------------------------------------------------------------------
+# o if we encounter foo/ in $(obj-y), replace it by foo/built_in.o
+#   and add the directory to the list of dirs to descend into: $(subdir-y)
+__subdir-y	:= $(filter %/, $(obj-y))
+subdir-y	+= $(__subdir-y)
+obj-y		:= $(patsubst %/, %/built_in.o, $(obj-y))
+
+subdir-n := $(subdir-n) $(subdir-) \
+		$(filter %/, $(obj-n) $(obj-))
 
 subdir-all := $(subdir-y) $(subdir-n)
 
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 70f532e42a06..1044c2298a05 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -1,11 +1,11 @@
-subdir-$(CONFIG_ARM_32) += arm32
-subdir-$(CONFIG_ARM_64) += arm64
-subdir-$(CONFIG_ARM_64) += efi
-subdir-$(CONFIG_ACPI) += acpi
+obj-$(CONFIG_ARM_32) += arm32/
+obj-$(CONFIG_ARM_64) += arm64/
+obj-$(CONFIG_ARM_64) += efi/
+obj-$(CONFIG_ACPI) += acpi/
 ifneq ($(CONFIG_NO_PLAT),y)
-subdir-y += platforms
+obj-y += platforms/
 endif
-subdir-$(CONFIG_TEE) += tee
+obj-$(CONFIG_TEE) += tee/
 
 obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
 obj-y += bootfdt.init.o
@@ -48,7 +48,7 @@ obj-y += sysctl.o
 obj-y += time.o
 obj-y += traps.o
 obj-y += vcpreg.o
-subdir-$(CONFIG_NEW_VGIC) += vgic
+obj-$(CONFIG_NEW_VGIC) += vgic/
 ifneq ($(CONFIG_NEW_VGIC),y)
 obj-y += gic-vgic.o
 obj-y += vgic.o
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index 0ac254f34714..539bbef298a7 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -1,4 +1,4 @@
-subdir-y += lib
+obj-y += lib/
 
 obj-$(EARLY_PRINTK) += debug.o
 obj-y += domctl.o
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index c4f3a28a0d0b..db8565b71a33 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -1,4 +1,4 @@
-subdir-y += lib
+obj-y += lib/
 
 obj-y += cache.o
 obj-$(CONFIG_HARDEN_BRANCH_PREDICTOR) += bpi.o
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 6783688b00be..461d1f3dc2a6 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -1,12 +1,12 @@
-subdir-y += acpi
-subdir-y += cpu
-subdir-y += genapic
-subdir-$(CONFIG_GUEST) += guest
-subdir-$(CONFIG_HVM) += hvm
-subdir-y += mm
-subdir-$(CONFIG_XENOPROF) += oprofile
-subdir-$(CONFIG_PV) += pv
-subdir-y += x86_64
+obj-y += acpi/
+obj-y += cpu/
+obj-y += genapic/
+obj-$(CONFIG_GUEST) += guest/
+obj-$(CONFIG_HVM) += hvm/
+obj-y += mm/
+obj-$(CONFIG_XENOPROF) += oprofile/
+obj-$(CONFIG_PV) += pv/
+obj-y += x86_64/
 
 alternative-y := alternative.init.o
 alternative-$(CONFIG_LIVEPATCH) :=
diff --git a/xen/arch/x86/acpi/Makefile b/xen/arch/x86/acpi/Makefile
index 27b4aa30b0ca..1b9e62571301 100644
--- a/xen/arch/x86/acpi/Makefile
+++ b/xen/arch/x86/acpi/Makefile
@@ -1,4 +1,4 @@
-subdir-y += cpufreq
+obj-y += cpufreq/
 
 obj-y += lib.o power.o suspend.o cpu_idle.o cpuidle_menu.o
 obj-bin-y += boot.init.o wakeup_prot.o
diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
index 466acc8b10e5..de983006a1b1 100644
--- a/xen/arch/x86/cpu/Makefile
+++ b/xen/arch/x86/cpu/Makefile
@@ -1,5 +1,5 @@
-subdir-y += mcheck
-subdir-y += mtrr
+obj-y += mcheck/
+obj-y += mtrr/
 
 obj-y += amd.o
 obj-y += centaur.o
diff --git a/xen/arch/x86/guest/Makefile b/xen/arch/x86/guest/Makefile
index f164196772e8..a1e370d69df8 100644
--- a/xen/arch/x86/guest/Makefile
+++ b/xen/arch/x86/guest/Makefile
@@ -1,4 +1,4 @@
 obj-y += hypervisor.o
 
-subdir-$(CONFIG_HYPERV_GUEST) += hyperv
-subdir-$(CONFIG_XEN_GUEST) += xen
+obj-$(CONFIG_HYPERV_GUEST) += hyperv/
+obj-$(CONFIG_XEN_GUEST) += xen/
diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
index 43e5f3a21f8b..346419154460 100644
--- a/xen/arch/x86/hvm/Makefile
+++ b/xen/arch/x86/hvm/Makefile
@@ -1,6 +1,6 @@
-subdir-y += svm
-subdir-y += vmx
-subdir-y += viridian
+obj-y += svm/
+obj-y += vmx/
+obj-y += viridian/
 
 obj-y += asid.o
 obj-y += dm.o
diff --git a/xen/arch/x86/mm/Makefile b/xen/arch/x86/mm/Makefile
index 5010a29d6cb0..d87dc0aa6eeb 100644
--- a/xen/arch/x86/mm/Makefile
+++ b/xen/arch/x86/mm/Makefile
@@ -1,5 +1,5 @@
-subdir-y += shadow
-subdir-$(CONFIG_HVM) += hap
+obj-y += shadow/
+obj-$(CONFIG_HVM) += hap/
 
 obj-$(CONFIG_HVM) += altp2m.o
 obj-$(CONFIG_HVM) += guest_walk_2.o guest_walk_3.o guest_walk_4.o
diff --git a/xen/arch/x86/x86_64/Makefile b/xen/arch/x86/x86_64/Makefile
index 4bfa1480eb7e..2bb1eb0a8131 100644
--- a/xen/arch/x86/x86_64/Makefile
+++ b/xen/arch/x86/x86_64/Makefile
@@ -1,4 +1,4 @@
-subdir-$(CONFIG_PV) += compat
+obj-$(CONFIG_PV) += compat/
 
 obj-bin-y += entry.o
 obj-y += traps.o
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 62b34e69e95c..d4db0a6d466a 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -73,8 +73,8 @@ obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o kernel.o memory.o multicall
 
 extra-y := symbols-dummy.o
 
-subdir-$(CONFIG_COVERAGE) += coverage
-subdir-$(CONFIG_UBSAN) += ubsan
+obj-$(CONFIG_COVERAGE) += coverage/
+obj-$(CONFIG_UBSAN) += ubsan/
 
-subdir-$(CONFIG_NEEDS_LIBELF) += libelf
-subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt
+obj-$(CONFIG_NEEDS_LIBELF) += libelf/
+obj-$(CONFIG_HAS_DEVICE_TREE) += libfdt/
diff --git a/xen/drivers/Makefile b/xen/drivers/Makefile
index 30bab3cfdb36..2a1ae8ad130a 100644
--- a/xen/drivers/Makefile
+++ b/xen/drivers/Makefile
@@ -1,7 +1,7 @@
-subdir-y += char
-subdir-$(CONFIG_HAS_CPUFREQ) += cpufreq
-subdir-$(CONFIG_HAS_PCI) += pci
-subdir-$(CONFIG_HAS_VPCI) += vpci
-subdir-$(CONFIG_HAS_PASSTHROUGH) += passthrough
-subdir-$(CONFIG_ACPI) += acpi
-subdir-$(CONFIG_VIDEO) += video
+obj-y += char/
+obj-$(CONFIG_HAS_CPUFREQ) += cpufreq/
+obj-$(CONFIG_HAS_PCI) += pci/
+obj-$(CONFIG_HAS_VPCI) += vpci/
+obj-$(CONFIG_HAS_PASSTHROUGH) += passthrough/
+obj-$(CONFIG_ACPI) += acpi/
+obj-$(CONFIG_VIDEO) += video/
diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile
index 444b11d5839d..4f8e97228ee2 100644
--- a/xen/drivers/acpi/Makefile
+++ b/xen/drivers/acpi/Makefile
@@ -1,6 +1,6 @@
-subdir-y += tables
-subdir-y += utilities
-subdir-$(CONFIG_X86) += apei
+obj-y += tables/
+obj-y += utilities/
+obj-$(CONFIG_X86) += apei/
 
 obj-bin-y += tables.init.o
 obj-$(CONFIG_NUMA) += numa.o
diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile
index d50ab188c83c..e973e16c7484 100644
--- a/xen/drivers/passthrough/Makefile
+++ b/xen/drivers/passthrough/Makefile
@@ -1,7 +1,7 @@
-subdir-$(CONFIG_X86) += vtd
-subdir-$(CONFIG_X86) += amd
-subdir-$(CONFIG_X86) += x86
-subdir-$(CONFIG_ARM) += arm
+obj-$(CONFIG_X86) += vtd/
+obj-$(CONFIG_X86) += amd/
+obj-$(CONFIG_X86) += x86/
+obj-$(CONFIG_ARM) += arm/
 
 obj-y += iommu.o
 obj-$(CONFIG_HAS_PCI) += pci.o
diff --git a/xen/drivers/passthrough/vtd/Makefile b/xen/drivers/passthrough/vtd/Makefile
index f302653858a0..fde7555fac07 100644
--- a/xen/drivers/passthrough/vtd/Makefile
+++ b/xen/drivers/passthrough/vtd/Makefile
@@ -1,4 +1,4 @@
-subdir-$(CONFIG_X86) += x86
+obj-$(CONFIG_X86) += x86/
 
 obj-y += iommu.o
 obj-y += dmar.o
diff --git a/xen/lib/Makefile b/xen/lib/Makefile
index dcdb75931378..7019ca00e8fd 100644
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -1 +1 @@
-subdir-$(CONFIG_X86) += x86
+obj-$(CONFIG_X86) += x86/
diff --git a/xen/xsm/Makefile b/xen/xsm/Makefile
index e4d581e065f8..cf0a728f1c96 100644
--- a/xen/xsm/Makefile
+++ b/xen/xsm/Makefile
@@ -3,4 +3,4 @@ obj-$(CONFIG_XSM) += xsm_policy.o
 obj-$(CONFIG_XSM) += dummy.o
 obj-$(CONFIG_XSM_SILO) += silo.o
 
-subdir-$(CONFIG_XSM_FLASK) += flask
+obj-$(CONFIG_XSM_FLASK) += flask/
diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index 7c3f381287be..b1fd45421993 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -2,7 +2,7 @@ obj-y += avc.o
 obj-y += hooks.o
 obj-y += flask_op.o
 
-subdir-y += ss
+obj-y += ss/
 
 CFLAGS += -I./include
 
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 02/12] xen/build: Use obj-y += subdir/ instead of subdir-y

[Jan Beulich]

On 17.01.2020 11:53, Anthony PERARD wrote:
> This is part of upgrading our build system and import more of Linux's
> one.
> 
> In Linux, subdir-y in Makefiles is only used to descend into
> subdirectory when there are no object to build, Xen doesn't have that
> and all subdir have object to be included in the final binary.
> 
> To allow the new syntax, the "obj-y" and "subdir-*" calculation in
> Rules.mk is changed and partially imported from Linux's Kbuild.
> 
> The command used to modify the Makefile was:
>     sed -i -r 's#^subdir-(.*)#obj-\1/#;' **/Makefile
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

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

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -105,17 +105,16 @@ define gendep
>  endef
>  $(foreach o,$(filter-out %/,$(obj-y) $(obj-bin-y) $(extra-y)),$(eval $(call gendep,$(o))))
>  
> -# Ensure each subdirectory has exactly one trailing slash.
> -subdir-n := $(patsubst %,%/,$(patsubst %/,%,$(subdir-n) $(subdir-)))
> -subdir-y := $(patsubst %,%/,$(patsubst %/,%,$(subdir-y)))
> -
> -# Add explicitly declared subdirectories to the object lists.
> -obj-y += $(patsubst %/,%/built_in.o,$(subdir-y))
> -
> -# Add implicitly declared subdirectories (in the object lists) to the
> -# subdirectory list, and rewrite the object-list entry.
> -subdir-y += $(filter %/,$(obj-y))
> -obj-y    := $(patsubst %/,%/built-in.o,$(obj-y))
> +# Handle objects in subdirs
> +# ---------------------------------------------------------------------------
> +# o if we encounter foo/ in $(obj-y), replace it by foo/built_in.o
> +#   and add the directory to the list of dirs to descend into: $(subdir-y)
> +__subdir-y	:= $(filter %/, $(obj-y))
> +subdir-y	+= $(__subdir-y)

I realize I'll be called guilty of bike-shedding again, and I also
realize this is the way Linux does it, but what use is the
intermediate __subdir-y? Linux has no 2nd use, and hence I also
don't see why we would gain one. I further think according to our
style there should be no use of tabs here.

> +obj-y		:= $(patsubst %/, %/built_in.o, $(obj-y))
> +
> +subdir-n := $(subdir-n) $(subdir-) \
> +		$(filter %/, $(obj-n) $(obj-))

This will easily fit on one line (and isn't anything cloned from
Linux).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 02/12] xen/build: Use obj-y += subdir/ instead of subdir-y

[Anthony PERARD]

On Wed, Jan 29, 2020 at 03:19:05PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
> > +# Handle objects in subdirs
> > +# ---------------------------------------------------------------------------
> > +# o if we encounter foo/ in $(obj-y), replace it by foo/built_in.o
> > +#   and add the directory to the list of dirs to descend into: $(subdir-y)
> > +__subdir-y	:= $(filter %/, $(obj-y))
> > +subdir-y	+= $(__subdir-y)
> 
> I realize I'll be called guilty of bike-shedding again, and I also
> realize this is the way Linux does it, but what use is the
> intermediate __subdir-y? Linux has no 2nd use, and hence I also
> don't see why we would gain one. I further think according to our
> style there should be no use of tabs here.

I though the extra __subdir-y that Linux does was to do the filtering on
obj-y right way and not at a later time when subdir-y is used. But in
Linux (now that I look more closely at it), subdir-y is initialised with
:= to have the right type, so the extra __subdir-y doesn't appear to be
useful. (And I didn't find any subdir-y=)

So, I'll add a "subdir-y :=" somewhere and remove the need for
__subdir-y. (And hopefully, no one will add a subdir-y=dir somewhere and
break the build.)

As for using space instead of tabs, I can do that. I just need to figure
out how to configure my editor properly to use tab only when needed.

> > +obj-y		:= $(patsubst %/, %/built_in.o, $(obj-y))
> > +
> > +subdir-n := $(subdir-n) $(subdir-) \
> > +		$(filter %/, $(obj-n) $(obj-))
> 
> This will easily fit on one line (and isn't anything cloned from
> Linux).

Will do.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 02/12] xen/build: Use obj-y += subdir/ instead of subdir-y

[Jan Beulich]

On 30.01.2020 17:54, Anthony PERARD wrote:
> On Wed, Jan 29, 2020 at 03:19:05PM +0100, Jan Beulich wrote:
>> On 17.01.2020 11:53, Anthony PERARD wrote:
>>> +# Handle objects in subdirs
>>> +# ---------------------------------------------------------------------------
>>> +# o if we encounter foo/ in $(obj-y), replace it by foo/built_in.o
>>> +#   and add the directory to the list of dirs to descend into: $(subdir-y)
>>> +__subdir-y	:= $(filter %/, $(obj-y))
>>> +subdir-y	+= $(__subdir-y)
>>
>> I realize I'll be called guilty of bike-shedding again, and I also
>> realize this is the way Linux does it, but what use is the
>> intermediate __subdir-y? Linux has no 2nd use, and hence I also
>> don't see why we would gain one. I further think according to our
>> style there should be no use of tabs here.
> 
> I though the extra __subdir-y that Linux does was to do the filtering on
> obj-y right way and not at a later time when subdir-y is used. But in
> Linux (now that I look more closely at it), subdir-y is initialised with
> := to have the right type, so the extra __subdir-y doesn't appear to be
> useful. (And I didn't find any subdir-y=)
> 
> So, I'll add a "subdir-y :=" somewhere and remove the need for
> __subdir-y. (And hopefully, no one will add a subdir-y=dir somewhere and
> break the build.)

Alternatively, to retain this "latching" effect, how about

subdir-y := $(subdir-y) $(filter %/, $(obj-y))

?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 02/12] xen/build: Use obj-y += subdir/ instead of subdir-y

[Anthony PERARD]

On Fri, Jan 31, 2020 at 09:35:16AM +0100, Jan Beulich wrote:
> Alternatively, to retain this "latching" effect, how about
> 
> subdir-y := $(subdir-y) $(filter %/, $(obj-y))

Sounds good.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2 03/12] xen/build: use $(clean) shorthand for clean targets

[Anthony PERARD]

From: Anthony PERARD <anthony.perard@gmail.com>

Collect all the clean targets as we are going to modify it shortly.
Also, this is inspired by Linux's Kbuild.

"Kbuild.include" isn't included by "Makefile", but the "_clean" target
is only used by Rules.mk which include Kbuild.include.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Makefile               | 16 ++++++++--------
 xen/Rules.mk               |  2 +-
 xen/scripts/Kbuild.include |  5 +++++
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index c326fee5880e..814011175fd8 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -117,14 +117,14 @@ _debug:
 .PHONY: _clean
 _clean: delete-unfresh-files
 	$(MAKE) -C tools clean
-	$(MAKE) -f $(BASEDIR)/Rules.mk -C include clean
-	$(MAKE) -f $(BASEDIR)/Rules.mk -C common clean
-	$(MAKE) -f $(BASEDIR)/Rules.mk -C drivers clean
-	$(MAKE) -f $(BASEDIR)/Rules.mk -C xsm clean
-	$(MAKE) -f $(BASEDIR)/Rules.mk -C crypto clean
-	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/arm clean
-	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/x86 clean
-	$(MAKE) -f $(BASEDIR)/Rules.mk -C test clean
+	$(MAKE) $(clean) include
+	$(MAKE) $(clean) common
+	$(MAKE) $(clean) drivers
+	$(MAKE) $(clean) xsm
+	$(MAKE) $(clean) crypto
+	$(MAKE) $(clean) arch/arm
+	$(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 {} \;
 	rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map *~ core
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 8b04cbdd24ca..120323717d87 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -185,7 +185,7 @@ FORCE:
 clean:: $(addprefix _clean_, $(subdir-all))
 	rm -f *.o .*.o.tmp *~ core $(DEPS_RM)
 _clean_%/: FORCE
-	$(MAKE) -f $(BASEDIR)/Rules.mk -C $* clean
+	$(MAKE) $(clean) $*
 
 SRCPATH := $(patsubst $(BASEDIR)/%,%,$(CURDIR))
 
diff --git a/xen/scripts/Kbuild.include b/xen/scripts/Kbuild.include
index a5c462fd9777..2465cc4060c3 100644
--- a/xen/scripts/Kbuild.include
+++ b/xen/scripts/Kbuild.include
@@ -5,3 +5,8 @@
 # cc-ifversion
 # Usage:  EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
 cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4))
+
+# Shorthand for $(MAKE) clean
+# Usage:
+# $(MAKE) $(clean) dir
+clean := -f $(BASEDIR)/Rules.mk clean -C
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 03/12] xen/build: use $(clean) shorthand for clean targets

[Jan Beulich]

On 17.01.2020 11:53, Anthony PERARD wrote:
> From: Anthony PERARD <anthony.perard@gmail.com>
> 
> Collect all the clean targets as we are going to modify it shortly.
> Also, this is inspired by Linux's Kbuild.
> 
> "Kbuild.include" isn't included by "Makefile", but the "_clean" target
> is only used by Rules.mk which include Kbuild.include.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2 04/12] xen/build: extract clean target from Rules.mk

[Anthony PERARD]

From: Anthony PERARD <anthony.perard@gmail.com>

Most of the code executed by Rules.mk isn't necessary for the clean
target, especially not the CFLAGS. This make running make clean much
faster.

This extract the code into a different Makefile. It doesn't want to
include Config.mk either so variables DEPS_RM and DEPS_INCLUDE are
extracted from Config.mk as well. DEPS_INCLUDE is put into
Kbuild.include so it could be use by other Makefiles.

This is inspired by Kbuild, with Makefile.clean partially copied from
Linux v5.4.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Rules.mk               | 13 -------------
 xen/scripts/Kbuild.include |  7 ++++++-
 xen/scripts/Makefile.clean | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 39 insertions(+), 14 deletions(-)
 create mode 100644 xen/scripts/Makefile.clean

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 120323717d87..deab0abd63e1 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -94,8 +94,6 @@ LDFLAGS += $(LDFLAGS-y)
 
 include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
 
-DEPS = .*.d
-
 include Makefile
 
 define gendep
@@ -113,11 +111,6 @@ __subdir-y	:= $(filter %/, $(obj-y))
 subdir-y	+= $(__subdir-y)
 obj-y		:= $(patsubst %/, %/built_in.o, $(obj-y))
 
-subdir-n := $(subdir-n) $(subdir-) \
-		$(filter %/, $(obj-n) $(obj-))
-
-subdir-all := $(subdir-y) $(subdir-n)
-
 $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
 
 ifeq ($(CONFIG_COVERAGE),y)
@@ -181,12 +174,6 @@ FORCE:
 %/built_in_bin.o: FORCE
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C $* built_in_bin.o
 
-.PHONY: clean
-clean:: $(addprefix _clean_, $(subdir-all))
-	rm -f *.o .*.o.tmp *~ core $(DEPS_RM)
-_clean_%/: FORCE
-	$(MAKE) $(clean) $*
-
 SRCPATH := $(patsubst $(BASEDIR)/%,%,$(CURDIR))
 
 %.o: %.c Makefile
diff --git a/xen/scripts/Kbuild.include b/xen/scripts/Kbuild.include
index 2465cc4060c3..6a9b0c39da53 100644
--- a/xen/scripts/Kbuild.include
+++ b/xen/scripts/Kbuild.include
@@ -2,6 +2,11 @@
 ####
 # kbuild: Generic definitions
 
+###
+# dependencies
+DEPS = .*.d
+DEPS_INCLUDE = $(addsuffix .d2, $(basename $(wildcard $(DEPS))))
+
 # cc-ifversion
 # Usage:  EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
 cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || echo $(4))
@@ -9,4 +14,4 @@ cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || e
 # Shorthand for $(MAKE) clean
 # Usage:
 # $(MAKE) $(clean) dir
-clean := -f $(BASEDIR)/Rules.mk clean -C
+clean := -f $(BASEDIR)/scripts/Makefile.clean clean -C
diff --git a/xen/scripts/Makefile.clean b/xen/scripts/Makefile.clean
new file mode 100644
index 000000000000..31cf2b59594e
--- /dev/null
+++ b/xen/scripts/Makefile.clean
@@ -0,0 +1,33 @@
+# SPDX-License-Identifier: GPL-2.0
+# ==========================================================================
+# Cleaning up
+# ==========================================================================
+
+clean::
+
+include $(BASEDIR)/scripts/Kbuild.include
+
+include Makefile
+
+# Figure out what we need to build from the various variables
+# ==========================================================================
+__subdir-y	:= $(filter %/, $(obj-y))
+subdir-y	+= $(__subdir-y)
+subdir-n := $(subdir-n) $(subdir-) \
+		$(filter %/, $(obj-n) $(obj-))
+subdir-all := $(subdir-y) $(subdir-n)
+
+DEPS_RM = $(DEPS) $(DEPS_INCLUDE)
+.PHONY: clean
+clean:: $(addprefix _clean_, $(subdir-all))
+	rm -f *.o .*.o.tmp *~ core $(DEPS_RM)
+
+# Descending
+# ---------------------------------------------------------------------------
+
+_clean_%/: FORCE
+	$(MAKE) $(clean) $*
+
+# Force execution of pattern rules (for which PHONY cannot be directly used).
+.PHONY: FORCE
+FORCE:
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 04/12] xen/build: extract clean target from Rules.mk

[Jan Beulich]

On 17.01.2020 11:53, Anthony PERARD wrote:
> From: Anthony PERARD <anthony.perard@gmail.com>
> 
> Most of the code executed by Rules.mk isn't necessary for the clean
> target, especially not the CFLAGS. This make running make clean much
> faster.
> 
> This extract the code into a different Makefile. It doesn't want to
> include Config.mk either so variables DEPS_RM and DEPS_INCLUDE are
> extracted from Config.mk as well. DEPS_INCLUDE is put into
> Kbuild.include so it could be use by other Makefiles.

"extracted" makes it sound as if the intention was to move things,
yet ...

> ---
>  xen/Rules.mk               | 13 -------------
>  xen/scripts/Kbuild.include |  7 ++++++-
>  xen/scripts/Makefile.clean | 33 +++++++++++++++++++++++++++++++++
>  3 files changed, 39 insertions(+), 14 deletions(-)

... ./Config.mk doesn't get touched at all. I guess there are reasons
for this, but I consider it dangerous to leave independent definitions
of the same variables in disconnected places. What if one side gets
updated without noticing the other?

> --- /dev/null
> +++ b/xen/scripts/Makefile.clean
> @@ -0,0 +1,33 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# ==========================================================================
> +# Cleaning up
> +# ==========================================================================
> +
> +clean::
> +
> +include $(BASEDIR)/scripts/Kbuild.include
> +
> +include Makefile
> +
> +# Figure out what we need to build from the various variables

s/build/clean/ ?

> +# ==========================================================================
> +__subdir-y	:= $(filter %/, $(obj-y))
> +subdir-y	+= $(__subdir-y)
> +subdir-n := $(subdir-n) $(subdir-) \
> +		$(filter %/, $(obj-n) $(obj-))
> +subdir-all := $(subdir-y) $(subdir-n)

Same remark as for the earlier patch regarding __subdir-y and the
use of tabs. Additionally please use consistent indentation of
:= / += within this block.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 04/12] xen/build: extract clean target from Rules.mk

[Anthony PERARD]

On Wed, Jan 29, 2020 at 03:30:19PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
> > From: Anthony PERARD <anthony.perard@gmail.com>
> > 
> > Most of the code executed by Rules.mk isn't necessary for the clean
> > target, especially not the CFLAGS. This make running make clean much
> > faster.
> > 
> > This extract the code into a different Makefile. It doesn't want to
> > include Config.mk either so variables DEPS_RM and DEPS_INCLUDE are
> > extracted from Config.mk as well. DEPS_INCLUDE is put into
> > Kbuild.include so it could be use by other Makefiles.
> 
> "extracted" makes it sound as if the intention was to move things,
> yet ...
> 
> > ---
> >  xen/Rules.mk               | 13 -------------
> >  xen/scripts/Kbuild.include |  7 ++++++-
> >  xen/scripts/Makefile.clean | 33 +++++++++++++++++++++++++++++++++
> >  3 files changed, 39 insertions(+), 14 deletions(-)
> 
> ... ./Config.mk doesn't get touched at all. I guess there are reasons
> for this, but I consider it dangerous to leave independent definitions
> of the same variables in disconnected places. What if one side gets
> updated without noticing the other?

I guess the word "extracted" is the wrong one. I'll need to rewrite the
patch commentary.

As for why Config.mk isn't change, it's because it is used by both the
hypervisor makefiles and the tools makefiles. I would like for recursive
makefiles to not include Config.mk anymore, so having only xen/Makefile
doing that include. (I would like to go further and not used Config.mk
anymore, but that might not be necessary.)

As for the last point, the variables DEPS_RM and DEPS_INCLUDE are copied
because Makefile.clean doesn't have them and at some point Rules.mk (no
patch yet) isn't going to have them either, so there will be a single
location which is Kbuild.include. Currently with this patch, both
variables from Kbuild.include are the one used by Rules.mk, so it
doesn't matter if Config.mk is modified.

Things doesn't look great yet, but it doesn't feel like there are better
way to refactor the build system.

> > --- /dev/null
> > +++ b/xen/scripts/Makefile.clean
> > +# Figure out what we need to build from the various variables
> 
> s/build/clean/ ?

Yep, I just copy the typo from Linux. But I can fix it in our repo.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 04/12] xen/build: extract clean target from Rules.mk

[Jan Beulich]

On 30.01.2020 19:10, Anthony PERARD wrote:
> On Wed, Jan 29, 2020 at 03:30:19PM +0100, Jan Beulich wrote:
>> On 17.01.2020 11:53, Anthony PERARD wrote:
>>> From: Anthony PERARD <anthony.perard@gmail.com>
>>>
>>> Most of the code executed by Rules.mk isn't necessary for the clean
>>> target, especially not the CFLAGS. This make running make clean much
>>> faster.
>>>
>>> This extract the code into a different Makefile. It doesn't want to
>>> include Config.mk either so variables DEPS_RM and DEPS_INCLUDE are
>>> extracted from Config.mk as well. DEPS_INCLUDE is put into
>>> Kbuild.include so it could be use by other Makefiles.
>>
>> "extracted" makes it sound as if the intention was to move things,
>> yet ...
>>
>>> ---
>>>  xen/Rules.mk               | 13 -------------
>>>  xen/scripts/Kbuild.include |  7 ++++++-
>>>  xen/scripts/Makefile.clean | 33 +++++++++++++++++++++++++++++++++
>>>  3 files changed, 39 insertions(+), 14 deletions(-)
>>
>> ... ./Config.mk doesn't get touched at all. I guess there are reasons
>> for this, but I consider it dangerous to leave independent definitions
>> of the same variables in disconnected places. What if one side gets
>> updated without noticing the other?
> 
> I guess the word "extracted" is the wrong one. I'll need to rewrite the
> patch commentary.
> 
> As for why Config.mk isn't change, it's because it is used by both the
> hypervisor makefiles and the tools makefiles. I would like for recursive
> makefiles to not include Config.mk anymore, so having only xen/Makefile
> doing that include. (I would like to go further and not used Config.mk
> anymore, but that might not be necessary.)
> 
> As for the last point, the variables DEPS_RM and DEPS_INCLUDE are copied
> because Makefile.clean doesn't have them and at some point Rules.mk (no
> patch yet) isn't going to have them either, so there will be a single
> location which is Kbuild.include. Currently with this patch, both
> variables from Kbuild.include are the one used by Rules.mk, so it
> doesn't matter if Config.mk is modified.

But with Config.mk still getting included from elsewhere underneath
xen/, this is going to be confusing. Changing the Kbuild.include
instances should really affect the entire xen/ tree then, at which
point the Config.mk instances could be declared "for everything
else" (and eventually be moved into the subtrees). I agree it's
not very helpful that Config.mk contains not only common
definitions, but also ones actually controlling how certain parts
of the build process work (like the two DEPS_*; going through the
file I wonder whether these two are actually the only outliers).

> Things doesn't look great yet, but it doesn't feel like there are better
> way to refactor the build system.

Right, an incremental switch of the build machinery is going to
run into oddities. Calling them out explicitly (including what
the plan is to resolve them by the end of the transformation
process) would be helpful, though.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2 05/12] xen/include: remove include of Config.mk

[Anthony PERARD]

It isn't necessary to include Config.mk here because this Makefile is
only used by xen/Rules.mk which already includes Config.mk.

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

diff --git a/xen/include/Makefile b/xen/include/Makefile
index fde0ca013121..433bad9055b2 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -1,5 +1,3 @@
-include $(XEN_ROOT)/Config.mk
-
 ifneq ($(CONFIG_COMPAT),)
 
 compat-arch-$(CONFIG_X86) := x86_32
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 05/12] xen/include: remove include of Config.mk

[Jan Beulich]

On 17.01.2020 11:53, Anthony PERARD wrote:
> It isn't necessary to include Config.mk here because this Makefile is
> only used by xen/Rules.mk which already includes Config.mk.

And so is xen/test/livepatch/Makefile afaics from its parent dir
Makefile. With this also adjusted (or it explained why I'm seeing
things incorrectly) ...

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

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 05/12] xen/include: remove include of Config.mk

[Jan Beulich]

On 29.01.2020 16:28, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
>> It isn't necessary to include Config.mk here because this Makefile is
>> only used by xen/Rules.mk which already includes Config.mk.
> 
> And so is xen/test/livepatch/Makefile afaics from its parent dir
> Makefile. With this also adjusted (or it explained why I'm seeing
> things incorrectly) ...
> 
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

And now I've seen that patch 6 does just this. I think such
common theme changes are, unless patches are overly large
already, better put all in on patch. Anyway - the R-b then
is unconditional.

Another question: The cover letter doesn't say anything about
some (or most) patches here being independent of one another,
and hence the option of them going in out of order. The one
here looks to be entirely standalone, for example.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 05/12] xen/include: remove include of Config.mk

[Anthony PERARD]

On Wed, Jan 29, 2020 at 04:33:02PM +0100, Jan Beulich wrote:
> On 29.01.2020 16:28, Jan Beulich wrote:
> > On 17.01.2020 11:53, Anthony PERARD wrote:
> >> It isn't necessary to include Config.mk here because this Makefile is
> >> only used by xen/Rules.mk which already includes Config.mk.
> > 
> > And so is xen/test/livepatch/Makefile afaics from its parent dir
> > Makefile. With this also adjusted (or it explained why I'm seeing
> > things incorrectly) ...
> > 
> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > 
> > Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> And now I've seen that patch 6 does just this. I think such
> common theme changes are, unless patches are overly large
> already, better put all in on patch. Anyway - the R-b then
> is unconditional.

I don't like squashing unrelated changes together. I though both changes
deserved there own explanation in this case. They don't touch the same
subsystem, they don't have the same set of maintainers.

> Another question: The cover letter doesn't say anything about
> some (or most) patches here being independent of one another,
> and hence the option of them going in out of order. The one
> here looks to be entirely standalone, for example.

It is extra work to figure out which patch could be applied out of
order. I would have independent patch at the beginning of the series,
but if there aren't, it is probably because I haven't though they were
important enough to think about applying them independently. I might try
to reorder some patches in later version of a series to have them
applied earlier.

As for this series, I do think applying most patches in order is
important, changing the order may lead to unexpected breakage. That
might not be true, but I don't want to spend time on checking that.

Cheers,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 05/12] xen/include: remove include of Config.mk

[Jan Beulich]

On 30.01.2020 19:34, Anthony PERARD wrote:
> On Wed, Jan 29, 2020 at 04:33:02PM +0100, Jan Beulich wrote:
>> On 29.01.2020 16:28, Jan Beulich wrote:
>>> On 17.01.2020 11:53, Anthony PERARD wrote:
>>>> It isn't necessary to include Config.mk here because this Makefile is
>>>> only used by xen/Rules.mk which already includes Config.mk.
>>>
>>> And so is xen/test/livepatch/Makefile afaics from its parent dir
>>> Makefile. With this also adjusted (or it explained why I'm seeing
>>> things incorrectly) ...
>>>
>>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>>
>>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>
>> And now I've seen that patch 6 does just this. I think such
>> common theme changes are, unless patches are overly large
>> already, better put all in on patch. Anyway - the R-b then
>> is unconditional.
> 
> I don't like squashing unrelated changes together. I though both changes
> deserved there own explanation in this case. They don't touch the same
> subsystem, they don't have the same set of maintainers.

Yes, the issue was in part because I noticed too late that
there was a 2nd, similar patch (and hence I went and checked
whether you've caught all instances where removal would be
possible). I understand your concerns, yet I think these two
aren't unrelated. Under a title "remove unnecessary includes
of Config.mk" both would have fit. But don't get me wrong,
I'm fine with them remaining split. A post-commit-message
remark clarifying this doesn't cover all instances would
have helped review nevertheless.

>> Another question: The cover letter doesn't say anything about
>> some (or most) patches here being independent of one another,
>> and hence the option of them going in out of order. The one
>> here looks to be entirely standalone, for example.
> 
> It is extra work to figure out which patch could be applied out of
> order. I would have independent patch at the beginning of the series,
> but if there aren't, it is probably because I haven't though they were
> important enough to think about applying them independently. I might try
> to reorder some patches in later version of a series to have them
> applied earlier.
> 
> As for this series, I do think applying most patches in order is
> important, changing the order may lead to unexpected breakage. That
> might not be true, but I don't want to spend time on checking that.

Fair enough. With my committer hat on, I just typically try
to spot opportunities of committing pieces, to reduce the
overall volume of to-be-resent patches.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2 06/12] xen/test/livepatch: remove include of Config.mk

[Anthony PERARD]

livepatch/Makefile seems to only be used via Rules.mk, which already
includes Config.mk, avoid the second include.

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

diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile
index 82a076aac1eb..148dddb90473 100644
--- a/xen/test/livepatch/Makefile
+++ b/xen/test/livepatch/Makefile
@@ -1,5 +1,3 @@
-include $(XEN_ROOT)/Config.mk
-
 ifeq ($(XEN_TARGET_ARCH),x86_64)
 OBJCOPY_MAGIC := -I binary -O elf64-x86-64 -B i386:x86-64
 endif
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 06/12] xen/test/livepatch: remove include of Config.mk

[Ross Lagerwall]

On 1/17/20 10:53 AM, Anthony PERARD wrote:
> livepatch/Makefile seems to only be used via Rules.mk, which already
> includes Config.mk, avoid the second include.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2 07/12] xen/build: run targets csopes, tags, .. without Rules.mk

[Anthony PERARD]

Those targets make use of $(all_sources) which depends on TARGET_ARCH,
so we just need to set TARGET_ARCH earlier and once.

XEN_TARGET_ARCH isn't expected to change during the build, so
TARGET_SUBARCH and TARGET_ARCH aren't going to change either. Set them
once and for all in the Xen root Makefile. This allow to run more
targets without Rules.mk.

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

diff --git a/xen/Makefile b/xen/Makefile
index 814011175fd8..0e589de7755e 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -35,6 +35,11 @@ SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
 # we need XEN_TARGET_ARCH to generate the proper config
 include $(XEN_ROOT)/Config.mk
 
+# Set ARCH/SUBARCH appropriately.
+export TARGET_SUBARCH  := $(XEN_TARGET_ARCH)
+export TARGET_ARCH     := $(shell echo $(XEN_TARGET_ARCH) | \
+                            sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
+
 # Allow someone to change their config file
 export KCONFIG_CONFIG ?= .config
 
@@ -46,8 +51,8 @@ dist: install
 
 build install:: include/config/auto.conf
 
-.PHONY: build install uninstall clean distclean cscope TAGS tags MAP gtags tests
-build install uninstall debug clean distclean cscope TAGS tags MAP gtags tests::
+.PHONY: build install uninstall clean distclean MAP tests
+build install uninstall debug clean distclean MAP tests::
 ifneq ($(XEN_TARGET_ARCH),x86_32)
 	$(MAKE) -f Rules.mk _$@
 else
@@ -220,25 +225,25 @@ endef
 xenversion:
 	@echo $(XEN_FULLVERSION)
 
-.PHONY: _TAGS
-_TAGS: 
+.PHONY: TAGS
+TAGS:
 	set -e; rm -f TAGS; \
 	$(call set_exuberant_flags,etags); \
 	$(all_sources) | xargs etags $$exuberant_flags -a
 
-.PHONY: _tags
-_tags: 
+.PHONY: tags
+tags:
 	set -e; rm -f tags; \
 	$(call set_exuberant_flags,ctags); \
 	$(all_sources) | xargs ctags $$exuberant_flags -a
 
-.PHONY: _gtags
-_gtags:
+.PHONY: gtags
+gtags:
 	set -e; rm -f GTAGS GSYMS GPATH GRTAGS
 	$(all_sources) | gtags -f -
 
-.PHONY: _cscope
-_cscope:
+.PHONY: cscope
+cscope:
 	$(all_sources) > cscope.files
 	cscope -k -b -q
 
diff --git a/xen/Rules.mk b/xen/Rules.mk
index deab0abd63e1..d20521cc9ec1 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -27,11 +27,6 @@ ifneq ($(origin verbose),undefined)
 $(error "You must use 'make menuconfig' to enable/disable verbose now.")
 endif
 
-# Set ARCH/SUBARCH appropriately.
-override TARGET_SUBARCH  := $(XEN_TARGET_ARCH)
-override TARGET_ARCH     := $(shell echo $(XEN_TARGET_ARCH) | \
-                              sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
-
 TARGET := $(BASEDIR)/xen
 
 # Note that link order matters!
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 07/12] xen/build: run targets csopes, tags, .. without Rules.mk

[Jan Beulich]

On 17.01.2020 11:53, Anthony PERARD wrote:
> Those targets make use of $(all_sources) which depends on TARGET_ARCH,
> so we just need to set TARGET_ARCH earlier and once.
> 
> XEN_TARGET_ARCH isn't expected to change during the build, so
> TARGET_SUBARCH and TARGET_ARCH aren't going to change either.

In principle yes, but there's an exception which may be worth
mentioning here that it doesn't conflict: arch/x86/boot/build32.mk
overrides XEN_TARGET_ARCH (but doesn't use the remaining make
machinery).

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -35,6 +35,11 @@ SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
>  # we need XEN_TARGET_ARCH to generate the proper config
>  include $(XEN_ROOT)/Config.mk
>  
> +# Set ARCH/SUBARCH appropriately.
> +export TARGET_SUBARCH  := $(XEN_TARGET_ARCH)
> +export TARGET_ARCH     := $(shell echo $(XEN_TARGET_ARCH) | \
> +                            sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')

Seeing this, ...

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -27,11 +27,6 @@ ifneq ($(origin verbose),undefined)
>  $(error "You must use 'make menuconfig' to enable/disable verbose now.")
>  endif
>  
> -# Set ARCH/SUBARCH appropriately.
> -override TARGET_SUBARCH  := $(XEN_TARGET_ARCH)
> -override TARGET_ARCH     := $(shell echo $(XEN_TARGET_ARCH) | \
> -                              sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')

... where did the "override"s go?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2 08/12] xen/build: make tests in test/ directly

[Anthony PERARD]

It is unnecessary to make _tests via Rules.mk because the target
use Rules.mk as well.

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

diff --git a/xen/Makefile b/xen/Makefile
index 0e589de7755e..5d72d82be260 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -51,8 +51,8 @@ dist: install
 
 build install:: include/config/auto.conf
 
-.PHONY: build install uninstall clean distclean MAP tests
-build install uninstall debug clean distclean MAP tests::
+.PHONY: build install uninstall clean distclean MAP
+build install uninstall debug clean distclean MAP::
 ifneq ($(XEN_TARGET_ARCH),x86_32)
 	$(MAKE) -f Rules.mk _$@
 else
@@ -92,8 +92,8 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
 		fi; \
 	fi
 
-.PHONY: _tests
-_tests:
+.PHONY: tests
+tests:
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C test tests
 
 .PHONY: _uninstall
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 08/12] xen/build: make tests in test/ directly

[Jan Beulich]

On 17.01.2020 11:53, Anthony PERARD wrote:
> It is unnecessary to make _tests via Rules.mk because the target
> use Rules.mk as well.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2 09/12] xen/build: include include/config/auto.conf in main Makefile

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

The way targets are filtered is inspireds 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>
---
 xen/Makefile | 96 +++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 73 insertions(+), 23 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index 5d72d82be260..80679b4a6b17 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -49,7 +49,71 @@ default: build
 .PHONY: dist
 dist: install
 
-build install:: include/config/auto.conf
+
+ifndef root-make-done
+# 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	:=
+need-config	:= 1
+
+ifneq ($(filter $(no-dot-config-targets), $(MAKECMDGOALS)),)
+	ifeq ($(filter-out $(no-dot-config-targets), $(MAKECMDGOALS)),)
+		need-config :=
+	endif
+endif
+
+ifneq ($(filter config %config,$(MAKECMDGOALS)),)
+	config-build := 1
+endif
+
+export root-make-done := 1
+endif # root-make-done
+
+ifdef config-build
+# ===========================================================================
+# *config targets only - make sure prerequisites are updated, and descend
+# in tools/kconfig to make the *config target
+
+config: FORCE
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
+
+# Config.mk tries to include .config file, don't try to remake it
+%/.config: ;
+
+%config: FORCE
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
+
+else # !config-build
+
+ifdef need-config
+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) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" 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.
+%/auto.conf %/auto.conf.cmd: $(KCONFIG_CONFIG)
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" syncconfig
+
+endif # need-config
 
 .PHONY: build install uninstall clean distclean MAP
 build install uninstall debug clean distclean MAP::
@@ -251,9 +315,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)
 
@@ -274,25 +335,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 \
-	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))
@@ -304,3 +346,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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 09/12] xen/build: include include/config/auto.conf in main Makefile

[Jan Beulich]

On 17.01.2020 11:53, Anthony PERARD wrote:
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -49,7 +49,71 @@ default: build
>  .PHONY: dist
>  dist: install
>  
> -build install:: include/config/auto.conf
> +
> +ifndef root-make-done
> +# 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	:=

Is this actually needed? While correct (afaict) together with the
ifdef further down, I find this aspect of make behavior a little
confusing, and hence it would seem slightly better if there was
no empty definition of this symbol.

> +need-config	:= 1

Here and below, would it be possible to use y instead of 1, to
match how "true" gets expressed in various places elsewhere?
Or would there again be deviation-from-Linux concerns?

> +ifneq ($(filter $(no-dot-config-targets), $(MAKECMDGOALS)),)
> +	ifeq ($(filter-out $(no-dot-config-targets), $(MAKECMDGOALS)),)
> +		need-config :=
> +	endif
> +endif
> +
> +ifneq ($(filter config %config,$(MAKECMDGOALS)),)

Just $(filter %config, ...) suffices here, afaict, similar to
above "%clean" also being used to cover "clean".

> +	config-build := 1
> +endif
> +
> +export root-make-done := 1
> +endif # root-make-done
> +
> +ifdef config-build
> +# ===========================================================================
> +# *config targets only - make sure prerequisites are updated, and descend
> +# in tools/kconfig to make the *config target
> +
> +config: FORCE
> +	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
> +
> +# Config.mk tries to include .config file, don't try to remake it
> +%/.config: ;

This didn't exist before - why is it needed all of the sudden?

> +%config: FORCE
> +	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
> +
> +else # !config-build
> +
> +ifdef need-config
> +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) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" 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.
> +%/auto.conf %/auto.conf.cmd: $(KCONFIG_CONFIG)
> +	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" syncconfig

Previously the target pattern was include/config/%.conf. Is there a
particular reason for the switch?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 09/12] xen/build: include include/config/auto.conf in main Makefile

[Anthony PERARD]

On Thu, Jan 30, 2020 at 02:06:18PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -49,7 +49,71 @@ default: build
> >  .PHONY: dist
> >  dist: install
> >  
> > -build install:: include/config/auto.conf
> > +
> > +ifndef root-make-done
> > +# 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	:=
> 
> Is this actually needed? While correct (afaict) together with the
> ifdef further down, I find this aspect of make behavior a little
> confusing, and hence it would seem slightly better if there was
> no empty definition of this symbol.

That's actually a very recent change in Linux source code. They used to
use ifeq($(config-build),1) and ifeq($(config-build),0). I can certainly
change back to use ifeq instead of ifdef.

> > +need-config	:= 1
> 
> Here and below, would it be possible to use y instead of 1, to
> match how "true" gets expressed in various places elsewhere?
> Or would there again be deviation-from-Linux concerns?

It's probably fine to use "y". I don't think it matter, we need to make
quite a lot of changes compare to Linux anyway. I'll use "n" for the
negative.

> > +ifneq ($(filter $(no-dot-config-targets), $(MAKECMDGOALS)),)
> > +	ifeq ($(filter-out $(no-dot-config-targets), $(MAKECMDGOALS)),)
> > +		need-config :=
> > +	endif
> > +endif
> > +
> > +ifneq ($(filter config %config,$(MAKECMDGOALS)),)
> 
> Just $(filter %config, ...) suffices here, afaict, similar to
> above "%clean" also being used to cover "clean".

Yes, I'll remove the extra "config".

> > +	config-build := 1
> > +endif
> > +
> > +export root-make-done := 1
> > +endif # root-make-done
> > +
> > +ifdef config-build
> > +# ===========================================================================
> > +# *config targets only - make sure prerequisites are updated, and descend
> > +# in tools/kconfig to make the *config target
> > +
> > +config: FORCE
> > +	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
> > +
> > +# Config.mk tries to include .config file, don't try to remake it
> > +%/.config: ;
> 
> This didn't exist before - why is it needed all of the sudden?

It's because I'm introducing a new target "%config". So when make
"-include $(XEN_ROOT)/.config" (as found in Config.mk) it check if the
file needs to be rebuilt, and find %config and thus run kconfig to build
.config.

Currently, Makefile list all the targets that needs to be built with
kconfig.

> > +%config: FORCE
> > +	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
> > +
> > +else # !config-build
> > +
> > +ifdef need-config
> > +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) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" 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.
> > +%/auto.conf %/auto.conf.cmd: $(KCONFIG_CONFIG)
> > +	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" syncconfig
> 
> Previously the target pattern was include/config/%.conf. Is there a
> particular reason for the switch?

That change was needed in Linux because the full target is:
    %/auto.conf %/auto.conf.cmd %/tristate.conf:
But since we don't need tristate.conf in Xen, I can go back to what we have.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 09/12] xen/build: include include/config/auto.conf in main Makefile

[Jan Beulich]

On 03.02.2020 12:45, Anthony PERARD wrote:
> On Thu, Jan 30, 2020 at 02:06:18PM +0100, Jan Beulich wrote:
>> On 17.01.2020 11:53, Anthony PERARD wrote:
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -49,7 +49,71 @@ default: build
>>>  .PHONY: dist
>>>  dist: install
>>>  
>>> -build install:: include/config/auto.conf
>>> +
>>> +ifndef root-make-done
>>> +# 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	:=
>>
>> Is this actually needed? While correct (afaict) together with the
>> ifdef further down, I find this aspect of make behavior a little
>> confusing, and hence it would seem slightly better if there was
>> no empty definition of this symbol.
> 
> That's actually a very recent change in Linux source code. They used to
> use ifeq($(config-build),1) and ifeq($(config-build),0). I can certainly
> change back to use ifeq instead of ifdef.

Then perhaps, along the lines of ...

>>> +need-config	:= 1
>>
>> Here and below, would it be possible to use y instead of 1, to
>> match how "true" gets expressed in various places elsewhere?
>> Or would there again be deviation-from-Linux concerns?
> 
> It's probably fine to use "y". I don't think it matter, we need to make
> quite a lot of changes compare to Linux anyway. I'll use "n" for the
> negative.

... this, also use y/n?

>>> +ifdef config-build
>>> +# ===========================================================================
>>> +# *config targets only - make sure prerequisites are updated, and descend
>>> +# in tools/kconfig to make the *config target
>>> +
>>> +config: FORCE
>>> +	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
>>> +
>>> +# Config.mk tries to include .config file, don't try to remake it
>>> +%/.config: ;
>>
>> This didn't exist before - why is it needed all of the sudden?
> 
> It's because I'm introducing a new target "%config". So when make
> "-include $(XEN_ROOT)/.config" (as found in Config.mk) it check if the
> file needs to be rebuilt, and find %config and thus run kconfig to build
> .config.
> 
> Currently, Makefile list all the targets that needs to be built with
> kconfig.

Ah, I see - we didn't have a %config target anywhere at all.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2 10/12] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS)

[Anthony PERARD]

From: Anthony PERARD <anthony.perard@gmail.com>

We would like to calculate CFLAGS once and before calling Rules.mk,
so the variable CFLAGS needs to have the same value across the whole
build. Thus we need a new variable where some flags can change
depending on the target name.

Both the dependency and __OBJECT_FILE__ are such flags that change
depending on the target, so there are move out of $(CFLAGS).

__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).

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

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Rules.mk                    | 27 +++++++++++++++------------
 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, 30 insertions(+), 27 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index d20521cc9ec1..c98d5372f3db 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-y                += -D__ASSEMBLY__
 
 ALL_OBJS := $(ALL_OBJS-y)
 
-# Get gcc to generate the dependencies for us.
-CFLAGS-y += -MMD -MF $(@D)/.$(@F).d
-
 CFLAGS += $(CFLAGS-y)
 # allow extra CFLAGS externally via EXTRA_CFLAGS_XEN_CORE
 CFLAGS += $(EXTRA_CFLAGS_XEN_CORE)
@@ -141,9 +137,16 @@ 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 -MF $(@D)/.$(@F).d \
+          $(CFLAGS) \
+          '-D__OBJECT_FILE__="$@"'
+
+a_flags = -MMD -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),$^)
@@ -154,7 +157,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
@@ -173,7 +176,7 @@ SRCPATH := $(patsubst $(BASEDIR)/%,%,$(CURDIR))
 
 %.o: %.c Makefile
 ifeq ($(CONFIG_ENFORCE_UNIQUE_SYMBOLS),y)
-	$(CC) $(CFLAGS) -c $< -o $(@D)/.$(@F).tmp
+	$(CC) $(c_flags) -c $< -o $(@D)/.$(@F).tmp
 ifeq ($(CONFIG_CC_IS_CLANG),y)
 	$(OBJCOPY) --redefine-sym $<=$(SRCPATH)/$< $(@D)/.$(@F).tmp $@
 else
@@ -181,11 +184,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 $@
 
 SPECIAL_DATA_SECTIONS := rodata $(foreach a,1 2 4 8 16, \
 					    $(foreach w,1 2 4, \
@@ -206,13 +209,13 @@ $(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 $@
 
 # -std=gnu{89,99} gets confused by # as an end-of-line comment marker
 %.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 1044c2298a05..7f1427630b96 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -121,10 +121,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 461d1f3dc2a6..472e3fadb719 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -225,7 +225,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
 
@@ -241,12 +241,12 @@ $(BASEDIR)/include/asm-x86/asm-macros.h: asm-macros.i Makefile
 	$(call move-if-changed,$@.new,$@)
 
 xen.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
 
 efi.lds: xen.lds.S
-	$(CC) -P -E -Ui386 -DEFI $(filter-out -Wa$(comma)%,$(AFLAGS)) -o $@ $<
+	$(CC) -P -E -Ui386 -DEFI $(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..83131c2472f5 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)% -M% %.d -include %/include/xen/config.h,$(c_flags)) $(cppflags-y) -o $@ $<
 
 compat/%.c: public/%.h xlat.lst Makefile $(BASEDIR)/tools/compat-build-source.py
 	mkdir -p $(@D)
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 10/12] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS)

[Jan Beulich]

On 17.01.2020 11:53, Anthony PERARD wrote:
> We would like to calculate CFLAGS once and before calling Rules.mk,
> so the variable CFLAGS needs to have the same value across the whole
> build. Thus we need a new variable where some flags can change
> depending on the target name.
> 
> Both the dependency and __OBJECT_FILE__ are such flags that change
> depending on the target, so there are move out of $(CFLAGS).

I'm afraid I don't understand: Being a delayed expansion (or
"recursively expanded") variable, what problem is there when CFLAGS
references $@? Is there a plan to change the variable's flavor? If
so, I'd like to ask for this to be mentioned here. "Calculate once",
at least to me, doesn't imply this.

> @@ -141,9 +137,16 @@ 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 -MF $(@D)/.$(@F).d \
> +          $(CFLAGS) \
> +          '-D__OBJECT_FILE__="$@"'
> +
> +a_flags = -MMD -MF $(@D)/.$(@F).d \
> +          $(AFLAGS)

Is there a reason both get extended over multiple lines?

> --- 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)% -M% %.d -include %/include/xen/config.h,$(c_flags)) $(cppflags-y) -o $@ $<

I think this wants to continue to reference $(CFLAGS) and instead have
the -M% and %.d patterns dropped. Similarly I guess as-insn in Config.mk
could then have these two patterns dropped.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 10/12] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS)

[Anthony PERARD]

On Thu, Jan 30, 2020 at 02:29:42PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
> > We would like to calculate CFLAGS once and before calling Rules.mk,
> > so the variable CFLAGS needs to have the same value across the whole
> > build. Thus we need a new variable where some flags can change
> > depending on the target name.
> > 
> > Both the dependency and __OBJECT_FILE__ are such flags that change
> > depending on the target, so there are move out of $(CFLAGS).
> 
> I'm afraid I don't understand: Being a delayed expansion (or
> "recursively expanded") variable, what problem is there when CFLAGS
> references $@? Is there a plan to change the variable's flavor? If
> so, I'd like to ask for this to be mentioned here. "Calculate once",
> at least to me, doesn't imply this.

If I rewrite the first paragraph thus, would that be better?

    In a later patch, we want to calculate the CFLAGS in xen/Makefile,
    then export it. So have Rules.mk use a CFLAGS from the environment
    variables. This will mean that if Rules.mk or a Makefile modify
    CFLAGS for a target, the modification propagates to other targets.
    Thus we will need a different variable name than the one from the
    environment which can have a different value for each target.


> > @@ -141,9 +137,16 @@ 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 -MF $(@D)/.$(@F).d \
> > +          $(CFLAGS) \
> > +          '-D__OBJECT_FILE__="$@"'
> > +
> > +a_flags = -MMD -MF $(@D)/.$(@F).d \
> > +          $(AFLAGS)
> 
> Is there a reason both get extended over multiple lines?

Beside that it looks cleaner to me, not really.

> > --- 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)% -M% %.d -include %/include/xen/config.h,$(c_flags)) $(cppflags-y) -o $@ $<
> 
> I think this wants to continue to reference $(CFLAGS) and instead have
> the -M% and %.d patterns dropped. Similarly I guess as-insn in Config.mk
> could then have these two patterns dropped.

It's probably a good idea to keep using CFLAGS, I'll look into it.
As to change as-insn, I can move it out of Config.mk and then change it.
I'll look into that as well.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 10/12] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS)

[Jan Beulich]

On 03.02.2020 13:17, Anthony PERARD wrote:
> On Thu, Jan 30, 2020 at 02:29:42PM +0100, Jan Beulich wrote:
>> On 17.01.2020 11:53, Anthony PERARD wrote:
>>> We would like to calculate CFLAGS once and before calling Rules.mk,
>>> so the variable CFLAGS needs to have the same value across the whole
>>> build. Thus we need a new variable where some flags can change
>>> depending on the target name.
>>>
>>> Both the dependency and __OBJECT_FILE__ are such flags that change
>>> depending on the target, so there are move out of $(CFLAGS).
>>
>> I'm afraid I don't understand: Being a delayed expansion (or
>> "recursively expanded") variable, what problem is there when CFLAGS
>> references $@? Is there a plan to change the variable's flavor? If
>> so, I'd like to ask for this to be mentioned here. "Calculate once",
>> at least to me, doesn't imply this.
> 
> If I rewrite the first paragraph thus, would that be better?
> 
>     In a later patch, we want to calculate the CFLAGS in xen/Makefile,
>     then export it. So have Rules.mk use a CFLAGS from the environment
>     variables. This will mean that if Rules.mk or a Makefile modify
>     CFLAGS for a target, the modification propagates to other targets.
>     Thus we will need a different variable name than the one from the
>     environment which can have a different value for each target.

I think this is better, yes, albeit I'm still a little unhappy
about the uses of "target" here: It makes it look to me as if
this was primarily about things like

abc.o: CFLAGS += ...

where, unless its rule invokes make, I don't think the adjustment
would propagate anywhere. Instead aiui what you refer to is solely
an effect from the subdir traversal the build system does?

>>> @@ -141,9 +137,16 @@ 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 -MF $(@D)/.$(@F).d \
>>> +          $(CFLAGS) \
>>> +          '-D__OBJECT_FILE__="$@"'
>>> +
>>> +a_flags = -MMD -MF $(@D)/.$(@F).d \
>>> +          $(AFLAGS)
>>
>> Is there a reason both get extended over multiple lines?
> 
> Beside that it looks cleaner to me, not really.

Apparently a matter of taste then. I generally consider trailing
backslashes a little "unclean", and hence would prefer to avoid
them if I can.

>>> --- 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)% -M% %.d -include %/include/xen/config.h,$(c_flags)) $(cppflags-y) -o $@ $<
>>
>> I think this wants to continue to reference $(CFLAGS) and instead have
>> the -M% and %.d patterns dropped. Similarly I guess as-insn in Config.mk
>> could then have these two patterns dropped.
> 
> It's probably a good idea to keep using CFLAGS, I'll look into it.
> As to change as-insn, I can move it out of Config.mk and then change it.
> I'll look into that as well.

Thank you.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2 11/12] xen/build: introduce ccflags-y and CFLAGS_$@

[Anthony PERARD]

We are going to want $(CFLAGS) to be static and never change during
the build, so introduce new variables that can be use to change the
flags of a single target or of a whole directory.

Those two variables are taken from Kbuild, in Linux v5.4.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Rules.mk               | 7 ++++++-
 xen/arch/arm/efi/Makefile  | 2 +-
 xen/arch/x86/Makefile      | 6 +++---
 xen/arch/x86/efi/Makefile  | 2 +-
 xen/common/libelf/Makefile | 2 +-
 xen/common/libfdt/Makefile | 2 +-
 xen/xsm/flask/Makefile     | 2 +-
 xen/xsm/flask/ss/Makefile  | 2 +-
 8 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index c98d5372f3db..f0111f2bc1b4 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -39,6 +39,7 @@ ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
 
 # Initialise some variables
 CFLAGS_UBSAN :=
+ccflags-y :=
 
 ifeq ($(CONFIG_DEBUG),y)
 CFLAGS += -O1
@@ -137,9 +138,13 @@ endif
 # Always build obj-bin files as binary even if they come from C source. 
 $(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS))
 
+# target with its suffix stripped
+target-stem = $(basename $@)
+
 c_flags = -MMD -MF $(@D)/.$(@F).d \
           $(CFLAGS) \
-          '-D__OBJECT_FILE__="$@"'
+          '-D__OBJECT_FILE__="$@"' \
+          $(ccflags-y) $(CFLAGS_$(target-stem).o)
 
 a_flags = -MMD -MF $(@D)/.$(@F).d \
           $(AFLAGS)
diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile
index d34c9168914a..e4aaba3e074b 100644
--- a/xen/arch/arm/efi/Makefile
+++ b/xen/arch/arm/efi/Makefile
@@ -1,4 +1,4 @@
-CFLAGS += -fshort-wchar
+ccflags-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 472e3fadb719..acf4c145c896 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -174,14 +174,14 @@ EFI_LDFLAGS += --major-subsystem-version=2 --minor-subsystem-version=0
 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)
 # 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
+ccflags-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
 
 $(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')
 
 ifneq ($(build_id_linker),)
 ifeq ($(call ld-ver-build-id,$(LD) $(filter -m%,$(EFI_LDFLAGS))),y)
-CFLAGS += -DBUILD_ID_EFI
+ccflags-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
@@ -227,7 +227,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
+CFLAGS_asm-macros.o += -D__ASSEMBLY__ -P
 
 $(BASEDIR)/include/asm-x86/asm-macros.h: asm-macros.i Makefile
 	echo '#if 0' >$@.new
diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index 4bc0a196e9ca..2cbb3de3a8ab 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -1,4 +1,4 @@
-CFLAGS += -fshort-wchar
+ccflags-y += -fshort-wchar
 
 %.o: %.ihex
 	$(OBJCOPY) -I ihex -O binary $< $@
diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile
index 3d9e38f27e65..9a433f01fbd4 100644
--- a/xen/common/libelf/Makefile
+++ b/xen/common/libelf/Makefile
@@ -3,7 +3,7 @@ nocov-y += libelf.o
 
 SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
 
-CFLAGS += -Wno-pointer-sign
+ccflags-y += -Wno-pointer-sign
 
 libelf.o: libelf-temp.o Makefile
 	$(OBJCOPY) $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
diff --git a/xen/common/libfdt/Makefile b/xen/common/libfdt/Makefile
index c075bbf5462a..9ea5c696d52a 100644
--- a/xen/common/libfdt/Makefile
+++ b/xen/common/libfdt/Makefile
@@ -5,7 +5,7 @@ SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
 obj-y += libfdt.o
 nocov-y += libfdt.o
 
-CFLAGS += -I$(BASEDIR)/include/xen/libfdt/
+ccflags-y += -I$(BASEDIR)/include/xen/libfdt/
 
 libfdt.o: libfdt-temp.o Makefile
 	$(OBJCOPY) $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index b1fd45421993..3bf0a6fa0456 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
+ccflags-y += -I./include
 
 AWK = awk
 
diff --git a/xen/xsm/flask/ss/Makefile b/xen/xsm/flask/ss/Makefile
index 046ce8f53326..30f910a9c9c1 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
+ccflags-y += -I../include
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 11/12] xen/build: introduce ccflags-y and CFLAGS_$@

[Jan Beulich]

On 17.01.2020 11:53, Anthony PERARD wrote:
> We are going to want $(CFLAGS) to be static and never change during
> the build, so introduce new variables that can be use to change the
> flags of a single target or of a whole directory.

I'm again struggling with the "why": What problem is there with e.g.

CFLAGS += -fshort-wchar

in a particular Makefile? This doesn't alter the when-to-expand
aspect of $(CFLAGS) at all.

I have to admit that I'm also a little puzzled by the naming, no
matter that it's taken from Linux. It doesn't really seem to fit
CFLAGS/AFLAGS and c_flags/a_flags.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 11/12] xen/build: introduce ccflags-y and CFLAGS_$@

[Anthony PERARD]

On Thu, Jan 30, 2020 at 02:39:43PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, Anthony PERARD wrote:
> I have to admit that I'm also a little puzzled by the naming, no
> matter that it's taken from Linux. It doesn't really seem to fit
> CFLAGS/AFLAGS and c_flags/a_flags.

So I've look into the history of Linux, ccflags-y was introduce to get
rid of EXTRA_CFLAGS and especially to have the -y part in the name of
the variable.

So, instead of ccflags-y and the like, we could use CFLAGS-y in Makefile
of subdirectories.

For makefiles in subdir, Linux has:
    CFLAGS_$@
    CFLAGS_REMOVE_$@
    ccflags-y
    subdir-ccflags-y
so CFLAGS-y would be better (and we can think about the subdir one
later).

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 11/12] xen/build: introduce ccflags-y and CFLAGS_$@

[Jan Beulich]

On 03.02.2020 15:23, Anthony PERARD wrote:
> On Thu, Jan 30, 2020 at 02:39:43PM +0100, Jan Beulich wrote:
>> On 17.01.2020 11:53, Anthony PERARD wrote:
>> I have to admit that I'm also a little puzzled by the naming, no
>> matter that it's taken from Linux. It doesn't really seem to fit
>> CFLAGS/AFLAGS and c_flags/a_flags.
> 
> So I've look into the history of Linux, ccflags-y was introduce to get
> rid of EXTRA_CFLAGS and especially to have the -y part in the name of
> the variable.
> 
> So, instead of ccflags-y and the like, we could use CFLAGS-y in Makefile
> of subdirectories.
> 
> For makefiles in subdir, Linux has:
>     CFLAGS_$@
>     CFLAGS_REMOVE_$@
>     ccflags-y
>     subdir-ccflags-y
> so CFLAGS-y would be better (and we can think about the subdir one
> later).

In case this doesn't conflict with uses of CFLAGS-y anywhere else
(not sure if any are left by this point of the series) - sounds
good.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2 12/12] xen/build: have the root Makefile generates the CFLAGS

[Anthony PERARD]

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 generates are the one that depends 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.

In order to allow some variable that are specific to one arch and
needs to be regenerated for each target, there's a new variable
$(arch_ccflags).

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.

There is on FIXME added about LTO build, but since LTO is marked as
BROKEN, this commit doesn't attempt to filter -flto flags out of the
CFLAGS.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Makefile               | 70 ++++++++++++++++++++++++++++
 xen/Rules.mk               | 70 +++++-----------------------
 xen/arch/arm/Makefile      | 10 ++--
 xen/arch/arm/Rules.mk      | 93 --------------------------------------
 xen/arch/arm/arch.mk       | 88 ++++++++++++++++++++++++++++++++++++
 xen/arch/x86/Makefile      | 16 +++----
 xen/arch/x86/Rules.mk      | 91 +++----------------------------------
 xen/arch/x86/arch.mk       | 87 +++++++++++++++++++++++++++++++++++
 xen/common/libelf/Makefile |  2 +-
 xen/common/libfdt/Makefile |  2 +-
 10 files changed, 278 insertions(+), 251 deletions(-)
 create mode 100644 xen/arch/arm/arch.mk
 create mode 100644 xen/arch/x86/arch.mk

diff --git a/xen/Makefile b/xen/Makefile
index 80679b4a6b17..22ca963994c8 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -113,6 +113,76 @@ $(KCONFIG_CONFIG):
 %/auto.conf %/auto.conf.cmd: $(KCONFIG_CONFIG)
 	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" 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 an suitable alternative.  The resulting compiled binary does function,
+# but has an excessively large symbol table.
+CFLAGS += -Wa,--strip-local-absolute
+endif
+
+AFLAGS-y                += -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 += $(AFLAGS-y) $(filter-out -std=gnu% -flto,$(CFLAGS))
+
+# LDFLAGS are only passed directly to $(LD)
+LDFLAGS += $(LDFLAGS_DIRECT)
+
+LDFLAGS += $(LDFLAGS-y)
+
+ifeq ($(CONFIG_COVERAGE),y)
+ifeq ($(CONFIG_CC_IS_CLANG),y)
+    COV_FLAGS := -fprofile-instr-generate -fcoverage-mapping
+else
+    COV_FLAGS := -fprofile-arcs -ftest-coverage
+endif
+else
+COV_FLAGS :=
+endif
+
+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 defines in Config.mk
+export XEN_CFLAGS := $(CFLAGS)
+export XEN_AFLAGS := $(AFLAGS)
+export XEN_LDFLAGS := $(LDFLAGS)
+export COV_FLAGS CFLAGS_UBSAN
+
 endif # need-config
 
 .PHONY: build install uninstall clean distclean MAP
diff --git a/xen/Rules.mk b/xen/Rules.mk
index f0111f2bc1b4..faf462a0178a 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -38,52 +38,12 @@ ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
 
 # Initialise some variables
-CFLAGS_UBSAN :=
 ccflags-y :=
-
-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-y                += -D__ASSEMBLY__
+# Allow arch specific cflags, to be calculated for each objects.
+arch_ccflags =
 
 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 += $(AFLAGS-y) $(filter-out -std=gnu% -flto,$(CFLAGS))
-
-# LDFLAGS are only passed directly to $(LD)
-LDFLAGS += $(LDFLAGS_DIRECT)
-
-LDFLAGS += $(LDFLAGS-y)
-
 include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
 
 include Makefile
@@ -103,27 +63,19 @@ __subdir-y	:= $(filter %/, $(obj-y))
 subdir-y	+= $(__subdir-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)): ccflags-y += -DINIT_SECTIONS_ONLY
 
 ifeq ($(CONFIG_COVERAGE),y)
-ifeq ($(CONFIG_CC_IS_CLANG),y)
-    COV_FLAGS := -fprofile-instr-generate -fcoverage-mapping
-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)): ccflags-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))
+ccflags-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.
@@ -136,18 +88,20 @@ 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))
+# FIXME LTO broken, but we would need a different way to filter -flto out
+# $(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS))
 
 # target with its suffix stripped
 target-stem = $(basename $@)
 
 c_flags = -MMD -MF $(@D)/.$(@F).d \
-          $(CFLAGS) \
+          $(XEN_CFLAGS) \
+          $(arch_ccflags) \
           '-D__OBJECT_FILE__="$@"' \
           $(ccflags-y) $(CFLAGS_$(target-stem).o)
 
 a_flags = -MMD -MF $(@D)/.$(@F).d \
-          $(AFLAGS)
+          $(XEN_AFLAGS)
 
 built_in.o: $(obj-y) $(extra-y)
 ifeq ($(obj-y),)
@@ -156,7 +110,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
 
@@ -164,7 +118,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 7f1427630b96..1599e2ba4058 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -96,24 +96,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 022a3a6f82ba..e69de29bb2d1 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -1,93 +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
-
-EARLY_PRINTK := n
-
-ifeq ($(CONFIG_DEBUG),y)
-
-# See docs/misc/arm/early-printk.txt for syntax
-
-EARLY_PRINTK_brcm           := 8250,0xF040AB00,2
-EARLY_PRINTK_dra7           := 8250,0x4806A000,2
-EARLY_PRINTK_fastmodel      := pl011,0x1c090000,115200
-EARLY_PRINTK_exynos5250     := exynos4210,0x12c20000
-EARLY_PRINTK_hikey960       := pl011,0xfff32000
-EARLY_PRINTK_juno           := pl011,0x7ff80000
-EARLY_PRINTK_lager          := scif,0xe6e60000
-EARLY_PRINTK_midway         := pl011,0xfff36000
-EARLY_PRINTK_mvebu          := mvebu,0xd0012000
-EARLY_PRINTK_omap5432       := 8250,0x48020000,2
-EARLY_PRINTK_rcar3          := scif,0xe6e88000
-EARLY_PRINTK_seattle        := pl011,0xe1010000
-EARLY_PRINTK_sun6i          := 8250,0x01c28000,2
-EARLY_PRINTK_sun7i          := 8250,0x01c28000,2
-EARLY_PRINTK_thunderx       := pl011,0x87e024000000
-EARLY_PRINTK_vexpress       := pl011,0x1c090000
-EARLY_PRINTK_xgene-mcdivitt := 8250,0x1c021000,2
-EARLY_PRINTK_xgene-storm    := 8250,0x1c020000,2
-EARLY_PRINTK_zynqmp         := cadence,0xff000000
-
-ifneq ($(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)),)
-EARLY_PRINTK_CFG := $(subst $(comma), ,$(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)))
-else
-EARLY_PRINTK_CFG := $(subst $(comma), ,$(CONFIG_EARLY_PRINTK))
-endif
-
-# Extract configuration from string
-EARLY_PRINTK_INC := $(word 1,$(EARLY_PRINTK_CFG))
-EARLY_UART_BASE_ADDRESS := $(word 2,$(EARLY_PRINTK_CFG))
-
-# UART specific options
-ifeq ($(EARLY_PRINTK_INC),8250)
-EARLY_UART_REG_SHIFT := $(word 3,$(EARLY_PRINTK_CFG))
-endif
-ifeq ($(EARLY_PRINTK_INC),pl011)
-ifneq ($(word 3,$(EARLY_PRINTK_CFG)),)
-EARLY_PRINTK_INIT_UART := y
-EARLY_PRINTK_BAUD := $(word 3,$(EARLY_PRINTK_CFG))
-endif
-endif
-ifeq ($(EARLY_PRINTK_INC),scif)
-ifneq ($(word 3,$(EARLY_PRINTK_CFG)),)
-CFLAGS-y += -DEARLY_PRINTK_VERSION_$(word 3,$(EARLY_PRINTK_CFG))
-else
-CFLAGS-y += -DEARLY_PRINTK_VERSION_NONE
-endif
-endif
-
-ifneq ($(EARLY_PRINTK_INC),)
-EARLY_PRINTK := y
-endif
-
-CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK
-CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DEARLY_PRINTK_INIT_UART
-CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
-CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK_BAUD=$(EARLY_PRINTK_BAUD)
-CFLAGS-$(EARLY_PRINTK) += -DEARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
-CFLAGS-$(EARLY_PRINTK) += -DEARLY_UART_REG_SHIFT=$(EARLY_UART_REG_SHIFT)
-
-else # !CONFIG_DEBUG
-
-ifneq ($(CONFIG_EARLY_PRINTK),)
-# Early printk is dependant on a debug build.
-$(error CONFIG_EARLY_PRINTK enabled for non-debug build)
-endif
-
-endif
diff --git a/xen/arch/arm/arch.mk b/xen/arch/arm/arch.mk
new file mode 100644
index 000000000000..296d6c6cf526
--- /dev/null
+++ b/xen/arch/arm/arch.mk
@@ -0,0 +1,88 @@
+########################################
+# arm-specific definitions
+
+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
+
+EARLY_PRINTK := n
+
+ifeq ($(CONFIG_DEBUG),y)
+
+# See docs/misc/arm/early-printk.txt for syntax
+
+EARLY_PRINTK_brcm           := 8250,0xF040AB00,2
+EARLY_PRINTK_dra7           := 8250,0x4806A000,2
+EARLY_PRINTK_fastmodel      := pl011,0x1c090000,115200
+EARLY_PRINTK_exynos5250     := exynos4210,0x12c20000
+EARLY_PRINTK_hikey960       := pl011,0xfff32000
+EARLY_PRINTK_juno           := pl011,0x7ff80000
+EARLY_PRINTK_lager          := scif,0xe6e60000
+EARLY_PRINTK_midway         := pl011,0xfff36000
+EARLY_PRINTK_mvebu          := mvebu,0xd0012000
+EARLY_PRINTK_omap5432       := 8250,0x48020000,2
+EARLY_PRINTK_rcar3          := scif,0xe6e88000
+EARLY_PRINTK_seattle        := pl011,0xe1010000
+EARLY_PRINTK_sun6i          := 8250,0x01c28000,2
+EARLY_PRINTK_sun7i          := 8250,0x01c28000,2
+EARLY_PRINTK_thunderx       := pl011,0x87e024000000
+EARLY_PRINTK_vexpress       := pl011,0x1c090000
+EARLY_PRINTK_xgene-mcdivitt := 8250,0x1c021000,2
+EARLY_PRINTK_xgene-storm    := 8250,0x1c020000,2
+EARLY_PRINTK_zynqmp         := cadence,0xff000000
+
+ifneq ($(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)),)
+EARLY_PRINTK_CFG := $(subst $(comma), ,$(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)))
+else
+EARLY_PRINTK_CFG := $(subst $(comma), ,$(CONFIG_EARLY_PRINTK))
+endif
+
+# Extract configuration from string
+EARLY_PRINTK_INC := $(word 1,$(EARLY_PRINTK_CFG))
+EARLY_UART_BASE_ADDRESS := $(word 2,$(EARLY_PRINTK_CFG))
+
+# UART specific options
+ifeq ($(EARLY_PRINTK_INC),8250)
+EARLY_UART_REG_SHIFT := $(word 3,$(EARLY_PRINTK_CFG))
+endif
+ifeq ($(EARLY_PRINTK_INC),pl011)
+ifneq ($(word 3,$(EARLY_PRINTK_CFG)),)
+EARLY_PRINTK_INIT_UART := y
+EARLY_PRINTK_BAUD := $(word 3,$(EARLY_PRINTK_CFG))
+endif
+endif
+ifeq ($(EARLY_PRINTK_INC),scif)
+ifneq ($(word 3,$(EARLY_PRINTK_CFG)),)
+CFLAGS-y += -DEARLY_PRINTK_VERSION_$(word 3,$(EARLY_PRINTK_CFG))
+else
+CFLAGS-y += -DEARLY_PRINTK_VERSION_NONE
+endif
+endif
+
+ifneq ($(EARLY_PRINTK_INC),)
+EARLY_PRINTK := y
+endif
+
+CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK
+CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DEARLY_PRINTK_INIT_UART
+CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
+CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK_BAUD=$(EARLY_PRINTK_BAUD)
+CFLAGS-$(EARLY_PRINTK) += -DEARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
+CFLAGS-$(EARLY_PRINTK) += -DEARLY_UART_REG_SHIFT=$(EARLY_UART_REG_SHIFT)
+
+else # !CONFIG_DEBUG
+
+ifneq ($(CONFIG_EARLY_PRINTK),)
+# Early printk is dependant on a debug build.
+$(error CONFIG_EARLY_PRINTK enabled for non-debug build)
+endif
+
+endif
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index acf4c145c896..6724572eb84b 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -124,32 +124,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 \
@@ -162,7 +162,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)
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index e69b8e697cc0..7867f9ccef83 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/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
+ifdef HAVE_AS_QUOTED_SYM
+arch_ccflags += -DHAVE_AS_QUOTED_SYM \
+		'-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$@'
+else
+arch_ccflags += '-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
-
+arch_ccflags += $(CFLAGS-stack-boundary)
diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
new file mode 100644
index 000000000000..1073f6cc99e9
--- /dev/null
+++ b/xen/arch/x86/arch.mk
@@ -0,0 +1,87 @@
+########################################
+# x86-specific definitions
+
+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)
+
+# 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/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)
+ifeq ($(call as-insn,$(CC) $(CFLAGS),".equ \"x\"$(comma)1",y),y)
+  export HAVE_AS_QUOTED_SYM := y
+endif
+$(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
+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)
+export 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
diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile
index 9a433f01fbd4..8f8508dd842e 100644
--- a/xen/common/libelf/Makefile
+++ b/xen/common/libelf/Makefile
@@ -9,4 +9,4 @@ 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 9ea5c696d52a..42986e118ebf 100644
--- a/xen/common/libfdt/Makefile
+++ b/xen/common/libfdt/Makefile
@@ -11,4 +11,4 @@ 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 $@ $^
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 12/12] xen/build: have the root Makefile generates the CFLAGS

[Jan Beulich]

On 17.01.2020 11:53, 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 generates are the one that depends 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.
> 
> In order to allow some variable that are specific to one arch and
> needs to be regenerated for each target, there's a new variable
> $(arch_ccflags).

And simply adding to c_flags is considered bad? Or does not work?

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -113,6 +113,76 @@ $(KCONFIG_CONFIG):
>  %/auto.conf %/auto.conf.cmd: $(KCONFIG_CONFIG)
>  	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" syncconfig
>  
> +ifeq ($(CONFIG_DEBUG),y)
> +CFLAGS += -O1
> +else
> +CFLAGS += -O2
> +endif

Why does this start with +=, not := (or = )?

> +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-y                += -D__ASSEMBLY__

Why not just AFLAGS? I think in a overhaul like what you do,
anomalies like this one would better be eliminated. The -y
forms should be added into the base variables (like you do ...

> +CFLAGS += $(CFLAGS-y)

... here), but be added to only via CFLAGS-$(variable)
constructs. Or otherwise there should be only CFLAGS-y, and
no plain CFLAGS at all.

> +# 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 += $(AFLAGS-y) $(filter-out -std=gnu% -flto,$(CFLAGS))
> +
> +# LDFLAGS are only passed directly to $(LD)
> +LDFLAGS += $(LDFLAGS_DIRECT)
> +
> +LDFLAGS += $(LDFLAGS-y)

These two could be folded.

> +ifeq ($(CONFIG_COVERAGE),y)
> +ifeq ($(CONFIG_CC_IS_CLANG),y)
> +    COV_FLAGS := -fprofile-instr-generate -fcoverage-mapping
> +else
> +    COV_FLAGS := -fprofile-arcs -ftest-coverage
> +endif
> +else
> +COV_FLAGS :=
> +endif

COV_FLAGS gets propagated through the environment, despite being
invariant. Can't this stay in Rules.mk?

> --- 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/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
> +ifdef HAVE_AS_QUOTED_SYM
> +arch_ccflags += -DHAVE_AS_QUOTED_SYM \
> +		'-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$@'
> +else
> +arch_ccflags += '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))'
>  endif

Why does HAVE_AS_QUOTED_SYM need a make / environment variable to
propagate? Can't this be as-option-add against arch_ccflags (or
c_flags), in arch.mk or Rules.mk? Or can't arch.mk have

$(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1",-DHAVE_AS_QUOTED_SYM)

and then here you simply check CFLAGS for this specific -D option?

> --- /dev/null
> +++ b/xen/arch/x86/arch.mk
> @@ -0,0 +1,87 @@
> +########################################
> +# x86-specific definitions
> +
> +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)
> +
> +# 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/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)
> +ifeq ($(call as-insn,$(CC) $(CFLAGS),".equ \"x\"$(comma)1",y),y)
> +  export HAVE_AS_QUOTED_SYM := y
> +endif
> +$(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
> +endif

CFLAGS-$(CONFIG_INDIRECT_THUNK) += ... ?

> +# 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)
> +export CFLAGS-stack-boundary

I find such random export unfortunate, but I can see why - within
the targeted model - this is the least bad alternative.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 12/12] xen/build: have the root Makefile generates the CFLAGS

[Anthony PERARD]

On Thu, Jan 30, 2020 at 03:33:15PM +0100, Jan Beulich wrote:
> On 17.01.2020 11:53, 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 generates are the one that depends 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.
> > 
> > In order to allow some variable that are specific to one arch and
> > needs to be regenerated for each target, there's a new variable
> > $(arch_ccflags).
> 
> And simply adding to c_flags is considered bad? Or does not work?

I could try to add directly to c_flags, it might not be that bad.
arch_ccflags is my invention, Linux doesn't have something similar.

> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -113,6 +113,76 @@ $(KCONFIG_CONFIG):
> >  %/auto.conf %/auto.conf.cmd: $(KCONFIG_CONFIG)
> >  	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" syncconfig
> >  
> > +ifeq ($(CONFIG_DEBUG),y)
> > +CFLAGS += -O1
> > +else
> > +CFLAGS += -O2
> > +endif
> 
> Why does this start with +=, not := (or = )?

Makefile includes Config.mk before these lines, so having += simply add
to the CFLAGS already generated by Config.mk.

> > +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-y                += -D__ASSEMBLY__
> 
> Why not just AFLAGS? I think in a overhaul like what you do,
> anomalies like this one would better be eliminated. The -y
> forms should be added into the base variables (like you do ...

I wanted to avoid too much modification, and mostly want to move the code
around. So it would be easier to check that the commit doesn't introduce
errors.  So, if your are fine with patch that move code and modify it,
I'll fix some of the oddities. (Or I can have another patch for it.)

> > +CFLAGS += $(CFLAGS-y)
> 
> ... here), but be added to only via CFLAGS-$(variable)
> constructs. Or otherwise there should be only CFLAGS-y, and
> no plain CFLAGS at all.
> 
> > +# 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 += $(AFLAGS-y) $(filter-out -std=gnu% -flto,$(CFLAGS))
> > +
> > +# LDFLAGS are only passed directly to $(LD)
> > +LDFLAGS += $(LDFLAGS_DIRECT)
> > +
> > +LDFLAGS += $(LDFLAGS-y)
> 
> These two could be folded.
> 
> > +ifeq ($(CONFIG_COVERAGE),y)
> > +ifeq ($(CONFIG_CC_IS_CLANG),y)
> > +    COV_FLAGS := -fprofile-instr-generate -fcoverage-mapping
> > +else
> > +    COV_FLAGS := -fprofile-arcs -ftest-coverage
> > +endif
> > +else
> > +COV_FLAGS :=
> > +endif
> 
> COV_FLAGS gets propagated through the environment, despite being
> invariant. Can't this stay in Rules.mk?

I guess that's fine.

> > --- 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/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
> > +ifdef HAVE_AS_QUOTED_SYM
> > +arch_ccflags += -DHAVE_AS_QUOTED_SYM \
> > +		'-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$@'
> > +else
> > +arch_ccflags += '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))'
> >  endif
> 
> Why does HAVE_AS_QUOTED_SYM need a make / environment variable to
> propagate? Can't this be as-option-add against arch_ccflags (or
> c_flags), in arch.mk or Rules.mk? Or can't arch.mk have
> 
> $(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1",-DHAVE_AS_QUOTED_SYM)
> 
> and then here you simply check CFLAGS for this specific -D option?

Sounds good, I'll do that.

> > +ifeq ($(CONFIG_INDIRECT_THUNK),y)
> > +CFLAGS += -mindirect-branch=thunk-extern -mindirect-branch-register
> > +CFLAGS += -fno-jump-tables
> > +endif
> 
> CFLAGS-$(CONFIG_INDIRECT_THUNK) += ... ?

Will change.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 12/12] xen/build: have the root Makefile generates the CFLAGS

[Jan Beulich]

On 03.02.2020 14:57, Anthony PERARD wrote:
> On Thu, Jan 30, 2020 at 03:33:15PM +0100, Jan Beulich wrote:
>> On 17.01.2020 11:53, Anthony PERARD wrote:
>>> +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-y                += -D__ASSEMBLY__
>>
>> Why not just AFLAGS? I think in a overhaul like what you do,
>> anomalies like this one would better be eliminated. The -y
>> forms should be added into the base variables (like you do ...
> 
> I wanted to avoid too much modification, and mostly want to move the code
> around. So it would be easier to check that the commit doesn't introduce
> errors.  So, if your are fine with patch that move code and modify it,
> I'll fix some of the oddities. (Or I can have another patch for it.)

With such extra adjustments at least briefly mentioned in the
description, folding in is fine as far as I'm concerned.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2.1 13/12] Makefile: Fix install-tests

[Anthony PERARD]

The top-level makefile make uses of internal implementation detail of
the xen build system. Avoid that by creating a new target
"install-tests" in xen/Makefile, and by fixing the top-level Makefile
to not call xen/Rules.mk anymore.

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

diff --git a/Makefile b/Makefile
index 512d6b73c898..9ad2602f63f0 100644
--- a/Makefile
+++ b/Makefile
@@ -155,13 +155,11 @@ install-docs:
 # We only have build-tests install-tests, not uninstall-tests etc.
 .PHONY: build-tests
 build-tests: build-xen
-	export BASEDIR=$(XEN_ROOT)/xen; \
-	$(MAKE) -f $$BASEDIR/Rules.mk -C xen/test build
+	$(MAKE) -C xen tests
 
 .PHONY: install-tests
 install-tests: install-xen
-	export BASEDIR=$(XEN_ROOT)/xen; \
-	$(MAKE) -f $$BASEDIR/Rules.mk -C xen/test install
+	$(MAKE) -C xen $@
 
 # build xen and the tools and place them in the install
 # directory. 'make install' should then copy them to the normal system
diff --git a/xen/Makefile b/xen/Makefile
index c326fee5880e..72bc89924622 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -90,6 +90,9 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
 .PHONY: _tests
 _tests:
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C test tests
+.PHONY: install-tests
+install-tests:
+	$(MAKE) -f $(BASEDIR)/Rules.mk -C test install
 
 .PHONY: _uninstall
 _uninstall: D=$(DESTDIR)
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2.1 13/12] Makefile: Fix install-tests

[Jan Beulich]

On 21.01.2020 14:59, Anthony PERARD wrote:
> The top-level makefile make uses of internal implementation detail of
> the xen build system. Avoid that by creating a new target
> "install-tests" in xen/Makefile, and by fixing the top-level Makefile
> to not call xen/Rules.mk anymore.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

This in principle could have my R-b, but ...

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -90,6 +90,9 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
>  .PHONY: _tests
>  _tests:
>  	$(MAKE) -f $(BASEDIR)/Rules.mk -C test tests
> +.PHONY: install-tests
> +install-tests:
> +	$(MAKE) -f $(BASEDIR)/Rules.mk -C test install

... I'm irritated by the patch context here: Patch 8 changed
_tests to tests, and by the numbering this patch goes on top
of patch 8. Could you clarify what's going on here, please?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2.1 13/12] Makefile: Fix install-tests

[Anthony PERARD]

On Thu, Jan 30, 2020 at 12:37:17PM +0100, Jan Beulich wrote:
> On 21.01.2020 14:59, Anthony PERARD wrote:
> > The top-level makefile make uses of internal implementation detail of
> > the xen build system. Avoid that by creating a new target
> > "install-tests" in xen/Makefile, and by fixing the top-level Makefile
> > to not call xen/Rules.mk anymore.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> This in principle could have my R-b, but ...
> 
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -90,6 +90,9 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
> >  .PHONY: _tests
> >  _tests:
> >  	$(MAKE) -f $(BASEDIR)/Rules.mk -C test tests
> > +.PHONY: install-tests
> > +install-tests:
> > +	$(MAKE) -f $(BASEDIR)/Rules.mk -C test install
> 
> ... I'm irritated by the patch context here: Patch 8 changed
> _tests to tests, and by the numbering this patch goes on top
> of patch 8. Could you clarify what's going on here, please?

I wanted to have this patch earlier in the series. I could probably have
put in the subject something like "[PATCH 1.5/12]" to make this clearer.

Cheers,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2.1 13/12] Makefile: Fix install-tests

[Jan Beulich]

On 03.02.2020 15:29, Anthony PERARD wrote:
> On Thu, Jan 30, 2020 at 12:37:17PM +0100, Jan Beulich wrote:
>> On 21.01.2020 14:59, Anthony PERARD wrote:
>>> The top-level makefile make uses of internal implementation detail of
>>> the xen build system. Avoid that by creating a new target
>>> "install-tests" in xen/Makefile, and by fixing the top-level Makefile
>>> to not call xen/Rules.mk anymore.
>>>
>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>
>> This in principle could have my R-b, but ...
>>
>>> --- a/xen/Makefile
>>> +++ b/xen/Makefile
>>> @@ -90,6 +90,9 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
>>>  .PHONY: _tests
>>>  _tests:
>>>  	$(MAKE) -f $(BASEDIR)/Rules.mk -C test tests
>>> +.PHONY: install-tests
>>> +install-tests:
>>> +	$(MAKE) -f $(BASEDIR)/Rules.mk -C test install
>>
>> ... I'm irritated by the patch context here: Patch 8 changed
>> _tests to tests, and by the numbering this patch goes on top
>> of patch 8. Could you clarify what's going on here, please?
> 
> I wanted to have this patch earlier in the series. I could probably have
> put in the subject something like "[PATCH 1.5/12]" to make this clearer.

I see. In which case, as indicated,
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2.1 14/12] squash! xen/build: introduce ccflags-y and CFLAGS_$@

[Anthony PERARD]

-DXEN_BUILD_EFI and -DBUILD_ID_EFI seems to only be used in xen.lds.S
which is build using the AFLAGS, so add those flags only to asflags-y.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
That fix build of xen.efi.

Should add asflags-y into the patch title.
---
 xen/Rules.mk          | 4 +++-
 xen/arch/x86/Makefile | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 4051d60addb5..add7cad93e4e 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -39,6 +39,7 @@ ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
 
 # Initialise some variables
 ccflags-y :=
+asflags-y :=
 # Allow arch specific cflags, to be calculated for each objects.
 arch_ccflags =
 
@@ -101,7 +102,8 @@ c_flags = -MMD -MF $(@D)/.$(@F).d \
           $(ccflags-y) $(CFLAGS_$(target-stem).o)
 
 a_flags = -MMD -MF $(@D)/.$(@F).d \
-          $(XEN_AFLAGS)
+          $(XEN_AFLAGS) \
+          $(asflags-y)
 
 built_in.o: $(obj-y) $(extra-y)
 ifeq ($(obj-y),)
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 191e2173315a..1e013ee31131 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -174,14 +174,14 @@ EFI_LDFLAGS += --major-subsystem-version=2 --minor-subsystem-version=0
 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)
 # 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))
-ccflags-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
+asflags-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
 
 $(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')
 
 ifneq ($(build_id_linker),)
 ifeq ($(call ld-ver-build-id,$(LD) $(filter -m%,$(EFI_LDFLAGS))),y)
-ccflags-y += -DBUILD_ID_EFI
+asflags-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
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH v2.1 15/12] squash! xen/build: have the root Makefile generates the CFLAGS

[Anthony PERARD]

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. XEN_CFLAGS doesn't
have those flags anymore, so no filtering is needed.

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

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 1e013ee31131..3f7702f42a9d 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -171,7 +171,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) $(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))
 asflags-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
-- 
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2.1 15/12] squash! xen/build: have the root Makefile generates the CFLAGS

[Jan Beulich]

On 21.01.2020 14:59, Anthony PERARD wrote:
> 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. XEN_CFLAGS doesn't
> have those flags anymore, so no filtering is needed.

But this should then be part of patch 10, not 12, I would think.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2.1 15/12] squash! xen/build: have the root Makefile generates the CFLAGS

[Anthony PERARD]

On Thu, Jan 30, 2020 at 02:32:13PM +0100, Jan Beulich wrote:
> On 21.01.2020 14:59, Anthony PERARD wrote:
> > 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. XEN_CFLAGS doesn't
> > have those flags anymore, so no filtering is needed.
> 
> But this should then be part of patch 10, not 12, I would think.

Sound good, I'll check if that's fine.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [XEN PATCH v2 00/12] xen: Build system improvements

[Anthony PERARD]

(Actually CCing all that are CCed on patches)

On Fri, Jan 17, 2020 at 10:53:46AM +0000, Anthony PERARD wrote:
> Patch series available in this git branch:
> https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.build-system-xen-v2
> 
> series is based on "[XEN PATCH v3 0/6] xen: Kconfig update with few extra"
> 
> v2:
> Rather than taking Kbuild and making it work with Xen, the v2 takes the opposite
> approach of slowly transforming our current build system into Kbuild. That have
> the advantage of keeping all the feature we have and making the patches much
> easier to review. Kconfig update is done in an other patch series.
> 
> v1:
> https://lists.xenproject.org/archives/html/xen-devel/2019-10/msg01609.html
> 
> 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.
> 
> XXX Known issue
> - make dist-tests is broken. I'll fix that latter.
> - efi build maybe broken (xen doesn't boot on albana which looks like to be one
>   of the uefi host)

With the new patch, and both squash! of v2.1, those two issues are fixed.
    Makefile: Fix install-tests
    squash! xen/build: introduce ccflags-y and CFLAGS_$@
    squash! xen/build: have the root Makefile generates the CFLAGS

I've created a new branch:
https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.build-system-xen-v2.1

And a link to an osstest run:
http://logs.test-lab.xenproject.org/osstest/logs/146338/

Cheers,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel