diff mbox series

[XEN,v3,15/23] xen/build: have the root Makefile generates the CFLAGS

Message ID 20200226113355.2532224-16-anthony.perard@citrix.com (mailing list archive)
State Superseded
Headers show
Series xen: Build system improvements | expand

Commit Message

Anthony PERARD Feb. 26, 2020, 11:33 a.m. UTC
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.

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

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

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

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

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

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

 xen/Makefile               | 60 ++++++++++++++++++++++++
 xen/Rules.mk               | 74 ++++++++----------------------
 xen/arch/arm/Makefile      | 10 ++--
 xen/arch/arm/Rules.mk      | 93 --------------------------------------
 xen/arch/arm/arch.mk       | 88 ++++++++++++++++++++++++++++++++++++
 xen/arch/arm/efi/Makefile  |  2 +-
 xen/arch/x86/Makefile      | 24 +++++-----
 xen/arch/x86/Rules.mk      | 91 +++----------------------------------
 xen/arch/x86/arch.mk       | 84 ++++++++++++++++++++++++++++++++++
 xen/arch/x86/efi/Makefile  |  2 +-
 xen/common/libelf/Makefile |  4 +-
 xen/common/libfdt/Makefile |  4 +-
 xen/include/Makefile       |  2 +-
 xen/xsm/flask/Makefile     |  2 +-
 xen/xsm/flask/ss/Makefile  |  2 +-
 15 files changed, 283 insertions(+), 259 deletions(-)
 create mode 100644 xen/arch/arm/arch.mk
 create mode 100644 xen/arch/x86/arch.mk

Comments

Roger Pau Monné Feb. 27, 2020, 11:05 a.m. UTC | #1
On Wed, Feb 26, 2020 at 11:33:47AM +0000, 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,
                     ^ be generated    ^ ones   ^ depend
I think (albeit I'm not a native speaker).

> but the variable $(c_flags) takes care of that.
> 
> Arch specific CFLAGS are generated by a new file "arch/*/arch.mk"
> which is included by the root Makefile.
> 
> We export the *FLAGS via the environment variables XEN_*FLAGS because
> Rules.mk still includes Config.mk and would add duplicated flags to
> CFLAGS.
> 
> When running Rules.mk in the root directory (xen/), the variable
> `root-make-done' is set, so `need-config' will remain undef and so the
> root Makefile will not generate the cflags again.
> 
> We can't use CFLAGS in subdirectories to add flags to particular
> targets, instead start to use CFLAGS-y. Idem for AFLAGS.
> So there are two different CFLAGS-y, the one in xen/Makefile (and
> arch.mk), and the one in subdirs that Rules.mk is going to use.
> We can't add to XEN_CFLAGS because it is exported, so making change to
> it might be propagated to subdirectory which isn't intended.
> 
> Some style change are introduced in this patch:
>     when LDFLAGS_DIRECT is included in LDFLAGS
>     use of CFLAGS-$(CONFIG_INDIRECT_THUNK) instead of ifeq().
> 
> 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>
> ---
> 
> Notes:
>     v3:
>     - squash "xen/build: introduce ccflags-y and CFLAGS_$@" here, with
>       those changes:
>         - rename ccflags-y to simply CFLAGS-y and start using AFLAGS-y in
>           subdirs.
>         - remove CFLAGS_$@, we don't need it yet.
>         - fix build of xen.lds and efi.lds which needed -D to be a_flags
>     - remove arch_ccflags, and modify c_flags directly
>       with that change, reorder c_flags, so that target specific flags are last.
>     - remove HAVE_AS_QUOTED_SYM from envvar and check XEN_CFLAGS to find if
>       it's there when adding -D__OBJECT_LABEL__.
>     - fix missing some flags in AFLAGS
>       (like -fshort-wchar in xen/arch/x86/efi/Makefile,
>        and -D__OBJECT_LABEL__ and CFLAGS-stack-boundary)
>     - keep COV_FLAGS generation in Rules.mk since it doesn't invovle to
>       call CC
>     - fix clang test for "asm()-s support .include." (in a new patch done
>       ahead)
>     - include Kconfig.include in xen/Makefile because as-option-add is
>       defined there now.
> 
>  xen/Makefile               | 60 ++++++++++++++++++++++++
>  xen/Rules.mk               | 74 ++++++++----------------------
>  xen/arch/arm/Makefile      | 10 ++--
>  xen/arch/arm/Rules.mk      | 93 --------------------------------------
>  xen/arch/arm/arch.mk       | 88 ++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/efi/Makefile  |  2 +-
>  xen/arch/x86/Makefile      | 24 +++++-----
>  xen/arch/x86/Rules.mk      | 91 +++----------------------------------
>  xen/arch/x86/arch.mk       | 84 ++++++++++++++++++++++++++++++++++
>  xen/arch/x86/efi/Makefile  |  2 +-
>  xen/common/libelf/Makefile |  4 +-
>  xen/common/libfdt/Makefile |  4 +-
>  xen/include/Makefile       |  2 +-
>  xen/xsm/flask/Makefile     |  2 +-
>  xen/xsm/flask/ss/Makefile  |  2 +-
>  15 files changed, 283 insertions(+), 259 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 a6120e577e9b..da017dc29d36 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -94,6 +94,8 @@ config: FORCE
>  
>  else # !config-build
>  
> +include scripts/Kbuild.include
> +
>  ifeq ($(need-config),y)
>  include include/config/auto.conf
>  # Read in dependencies to all Kconfig* files, make sure to run syncconfig if
> @@ -113,6 +115,64 @@ $(KCONFIG_CONFIG):
>  include/config/%.conf include/config/%.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

Long term we might want to make the optimization level selectable in
Kconfig IMO.

> +
> +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,
          ^ a
> +# but has an excessively large symbol table.
> +CFLAGS += -Wa,--strip-local-absolute

This is not really related to clang, but to the assembler. If clang is
used with -no-integrated-as it's quite likely that the GNU assembler
will be used, and hence this option would be available.

Can we use cc-option-add here in order to detect whether the build
toolchain support the option?

Ideally this should be done after the integrated assembler tests
performed in x86/Rules.mk.

> +endif
> +
> +AFLAGS += -D__ASSEMBLY__
> +
> +CFLAGS += $(CFLAGS-y)
> +# allow extra CFLAGS externally via EXTRA_CFLAGS_XEN_CORE
> +CFLAGS += $(EXTRA_CFLAGS_XEN_CORE)
> +
> +# Most CFLAGS are safe for assembly files:
> +#  -std=gnu{89,99} gets confused by #-prefixed end-of-line comments
> +#  -flto makes no sense and annoys clang
> +AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS))
> +
> +# LDFLAGS are only passed directly to $(LD)
> +LDFLAGS += $(LDFLAGS_DIRECT) $(LDFLAGS-y)
> +
> +ifeq ($(CONFIG_UBSAN),y)
> +CFLAGS_UBSAN := -fsanitize=undefined
> +else
> +CFLAGS_UBSAN :=

Do you need to define this to empty so it can be exported below? Isn't
it enough to just not set it at all?

> +endif
> +
> +ifeq ($(CONFIG_LTO),y)
> +CFLAGS += -flto
> +LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so
> +endif
> +
> +include $(BASEDIR)/arch/$(TARGET_ARCH)/arch.mk

The strip-local-absolute check should be done after this AFAICT.

> +
> +# define new variables to avoid the ones defines in Config.mk
> +export XEN_CFLAGS := $(CFLAGS)
> +export XEN_AFLAGS := $(AFLAGS)
> +export XEN_LDFLAGS := $(LDFLAGS)
> +export CFLAGS_UBSAN

You might want to rename this to XEN_CFLAGS_UBSAN for coherency with
the rest of the exported variables?

Thanks, Roger.
Jan Beulich March 4, 2020, 3 p.m. UTC | #2
On 26.02.2020 12:33, Anthony PERARD wrote:
> @@ -113,6 +115,64 @@ $(KCONFIG_CONFIG):
>  include/config/%.conf include/config/%.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 += -D__ASSEMBLY__
> +
> +CFLAGS += $(CFLAGS-y)

I can't seem to be able to spot a similar line for AFLAGS.

> @@ -107,7 +65,7 @@ $(foreach o,$(filter-out %/,$(obj-y) $(obj-bin-y) $(extra-y)),$(eval $(call gend
>  subdir-y := $(subdir-y) $(filter %/, $(obj-y))
>  obj-y    := $(patsubst %/, %/built_in.o, $(obj-y))
>  
> -$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
> +$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += -DINIT_SECTIONS_ONLY

While in the description you say "We can't use CFLAGS in
subdirectories to add flags to particular targets, ...", it
remains unclear there why that is, and hence why changes like
this one are necessary. If this is a restriction that's going to
remain, this also needs writing down in a prominent place. After
all if (for example) special compiler options are needed, CFLAGS
would be the natural thing one would want to alter. (Even better
if wrong playing with CFLAGS could be detected and at least
warned about, but I'm completely unclear on how feasible this
would be.)

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

As per the header here you're using git. Can you please arrange for
this file movement (to arch.mk, and also for x86) to actually be
expressed here as a rename, i.e. such that one can see what - if
anything - changes?

Jan
Anthony PERARD March 17, 2020, 6:05 p.m. UTC | #3
On Thu, Feb 27, 2020 at 12:05:04PM +0100, Roger Pau Monné wrote:
> On Wed, Feb 26, 2020 at 11:33:47AM +0000, Anthony PERARD wrote:
> > +ifeq ($(CONFIG_DEBUG),y)
> > +CFLAGS += -O1
> > +else
> > +CFLAGS += -O2
> > +endif
> 
> Long term we might want to make the optimization level selectable in
> Kconfig IMO.

Yep.

> > +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
> 
> This is not really related to clang, but to the assembler. If clang is
> used with -no-integrated-as it's quite likely that the GNU assembler
> will be used, and hence this option would be available.
> 
> Can we use cc-option-add here in order to detect whether the build
> toolchain support the option?

That can be done, but I think I'll do that as a follow up of this patch,
to avoid too many changes when moving the cflags around.

> Ideally this should be done after the integrated assembler tests
> performed in x86/Rules.mk.

> > +ifeq ($(CONFIG_UBSAN),y)
> > +CFLAGS_UBSAN := -fsanitize=undefined
> > +else
> > +CFLAGS_UBSAN :=
> 
> Do you need to define this to empty so it can be exported below? Isn't
> it enough to just not set it at all?

That has two functions, setting the correct flavor for the variable and
reset the value if the value happen to already be in the environment.
The second one is important I think.

> > +# define new variables to avoid the ones defines in Config.mk
> > +export XEN_CFLAGS := $(CFLAGS)
> > +export XEN_AFLAGS := $(AFLAGS)
> > +export XEN_LDFLAGS := $(LDFLAGS)
> > +export CFLAGS_UBSAN
> 
> You might want to rename this to XEN_CFLAGS_UBSAN for coherency with
> the rest of the exported variables?

I don't know, I think it's fine like that. They are other exported
variables, like CFLAGS-stack-boundary for flags. Not adding XEN_ prefix
to all of them mean less modifications, and less errors when doing so.

Thanks,
Anthony PERARD March 17, 2020, 6:35 p.m. UTC | #4
On Wed, Mar 04, 2020 at 04:00:52PM +0100, Jan Beulich wrote:
> On 26.02.2020 12:33, Anthony PERARD wrote:
> > @@ -113,6 +115,64 @@ $(KCONFIG_CONFIG):
> > +AFLAGS += -D__ASSEMBLY__
> > +
> > +CFLAGS += $(CFLAGS-y)
> 
> I can't seem to be able to spot a similar line for AFLAGS.

I didn't add any because it wasn't necessary.

> > @@ -107,7 +65,7 @@ $(foreach o,$(filter-out %/,$(obj-y) $(obj-bin-y) $(extra-y)),$(eval $(call gend
> >  subdir-y := $(subdir-y) $(filter %/, $(obj-y))
> >  obj-y    := $(patsubst %/, %/built_in.o, $(obj-y))
> >  
> > -$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
> > +$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += -DINIT_SECTIONS_ONLY
> 
> While in the description you say "We can't use CFLAGS in
> subdirectories to add flags to particular targets, ...", it
> remains unclear there why that is, and hence why changes like
> this one are necessary. If this is a restriction that's going to
> remain, this also needs writing down in a prominent place. After

Yes, I should probably start writing some documentation, probably
following the kind of documentation that Linux has for kbuild, and I
could put that in docs/misc, along side the kconfig doc. That probably
not a prominent place, but I don't know if there's a better place.

> all if (for example) special compiler options are needed, CFLAGS
> would be the natural thing one would want to alter. (Even better
> if wrong playing with CFLAGS could be detected and at least
> warned about, but I'm completely unclear on how feasible this
> would be.)

The issue with the CFLAGS variable is that it is still set by Config.mk
which Rules.mk includes.
I guess I could reset the CFLAGS variable in Rules.mk after loading
Config.mk, and allow it to be used. I want to stop including Config.mk
in Rules.mk at some point anyway.

I don't think it's possible to check if a variable has been altered,
because it could be changed for a specific target where it's not easy to
check.

> > 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 @@
> 
> As per the header here you're using git. Can you please arrange for
> this file movement (to arch.mk, and also for x86) to actually be
> expressed here as a rename, i.e. such that one can see what - if
> anything - changes?

That should be possible, I'll try to remember to add the right options.

Thanks,
Jan Beulich March 18, 2020, 10:20 a.m. UTC | #5
On 17.03.2020 19:35, Anthony PERARD wrote:
> On Wed, Mar 04, 2020 at 04:00:52PM +0100, Jan Beulich wrote:
>> On 26.02.2020 12:33, Anthony PERARD wrote:
>>> @@ -113,6 +115,64 @@ $(KCONFIG_CONFIG):
>>> +AFLAGS += -D__ASSEMBLY__
>>> +
>>> +CFLAGS += $(CFLAGS-y)
>>
>> I can't seem to be able to spot a similar line for AFLAGS.
> 
> I didn't add any because it wasn't necessary.

Symmetry / consistency would still be desirable imo.

>>> @@ -107,7 +65,7 @@ $(foreach o,$(filter-out %/,$(obj-y) $(obj-bin-y) $(extra-y)),$(eval $(call gend
>>>   subdir-y := $(subdir-y) $(filter %/, $(obj-y))
>>>   obj-y    := $(patsubst %/, %/built_in.o, $(obj-y))
>>>   
>>> -$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
>>> +$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += -DINIT_SECTIONS_ONLY
>>
>> While in the description you say "We can't use CFLAGS in
>> subdirectories to add flags to particular targets, ...", it
>> remains unclear there why that is, and hence why changes like
>> this one are necessary. If this is a restriction that's going to
>> remain, this also needs writing down in a prominent place. After
> 
> Yes, I should probably start writing some documentation, probably
> following the kind of documentation that Linux has for kbuild, and I
> could put that in docs/misc, along side the kconfig doc. That probably
> not a prominent place, but I don't know if there's a better place.

If it's to live under docs/, then I guess xen/Rules.mk should at
least point to that documentation (perhaps in a comment near the
first setting or consumption of CFLAGS-y).

Jan
Anthony PERARD March 19, 2020, 4:24 p.m. UTC | #6
On Tue, Mar 17, 2020 at 06:05:24PM +0000, Anthony PERARD wrote:
> On Thu, Feb 27, 2020 at 12:05:04PM +0100, Roger Pau Monné wrote:
> > On Wed, Feb 26, 2020 at 11:33:47AM +0000, 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
> > 
> > This is not really related to clang, but to the assembler. If clang is
> > used with -no-integrated-as it's quite likely that the GNU assembler
> > will be used, and hence this option would be available.
> > 
> > Can we use cc-option-add here in order to detect whether the build
> > toolchain support the option?
> 
> That can be done, but I think I'll do that as a follow up of this patch,
> to avoid too many changes when moving the cflags around.
> 
> > Ideally this should be done after the integrated assembler tests
> > performed in x86/Rules.mk.

So, testing for the -Wa,--strip-local-absolute flags turns out to be
more complicated than I though it would be.
 - cc-option-add doesn't work because it doesn't test with the current list
   of CFLAGS. And if I add the CFLAGS, clang says the option is unused,
   it doesn't matter if -no-integrated-as is present or not.
 - I tried to use as-option macro from Linux but that comes with issues
   as well. I don't think that enough work as been done to make it work
   well with clang. (I probably need to filter -Wall out of the CFLAGS
   when testing, and they are probably other issues.)

So I think I leave the enhancement for later. Having the flag depends
on GCC is good enough for now.

Thanks,
Roger Pau Monné March 23, 2020, 3:11 p.m. UTC | #7
On Thu, Mar 19, 2020 at 04:24:12PM +0000, Anthony PERARD wrote:
> On Tue, Mar 17, 2020 at 06:05:24PM +0000, Anthony PERARD wrote:
> > On Thu, Feb 27, 2020 at 12:05:04PM +0100, Roger Pau Monné wrote:
> > > On Wed, Feb 26, 2020 at 11:33:47AM +0000, 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
> > > 
> > > This is not really related to clang, but to the assembler. If clang is
> > > used with -no-integrated-as it's quite likely that the GNU assembler
> > > will be used, and hence this option would be available.
> > > 
> > > Can we use cc-option-add here in order to detect whether the build
> > > toolchain support the option?
> > 
> > That can be done, but I think I'll do that as a follow up of this patch,
> > to avoid too many changes when moving the cflags around.
> > 
> > > Ideally this should be done after the integrated assembler tests
> > > performed in x86/Rules.mk.
> 
> So, testing for the -Wa,--strip-local-absolute flags turns out to be
> more complicated than I though it would be.
>  - cc-option-add doesn't work because it doesn't test with the current list
>    of CFLAGS. And if I add the CFLAGS, clang says the option is unused,
>    it doesn't matter if -no-integrated-as is present or not.

Oh, that seems like completely bogus clang behavior. It's my
understanding (from reading the manual) that whatever gets appended to
-Wa,<arg> is just passed to the assembler, in which case the compiler
has no idea whether it's used by it or not.

Which version of clang did you use to test it?

Isn't it fine to just attempt to test if it works, and if the compiler
complains just don't append it? (regardless of whether
-no-integrated-as is used or not)

>  - I tried to use as-option macro from Linux but that comes with issues
>    as well. I don't think that enough work as been done to make it work
>    well with clang. (I probably need to filter -Wall out of the CFLAGS
>    when testing, and they are probably other issues.)
> 
> So I think I leave the enhancement for later. Having the flag depends
> on GCC is good enough for now.

Right, until clang adds support for it there's no pressure.

Thanks, Roger.
Anthony PERARD March 24, 2020, 5:11 p.m. UTC | #8
On Mon, Mar 23, 2020 at 04:11:53PM +0100, Roger Pau Monné wrote:
> On Thu, Mar 19, 2020 at 04:24:12PM +0000, Anthony PERARD wrote:
> > So, testing for the -Wa,--strip-local-absolute flags turns out to be
> > more complicated than I though it would be.
> >  - cc-option-add doesn't work because it doesn't test with the current list
> >    of CFLAGS. And if I add the CFLAGS, clang says the option is unused,
> >    it doesn't matter if -no-integrated-as is present or not.
> 
> Oh, that seems like completely bogus clang behavior. It's my
> understanding (from reading the manual) that whatever gets appended to
> -Wa,<arg> is just passed to the assembler, in which case the compiler
> has no idea whether it's used by it or not.
> 
> Which version of clang did you use to test it?

Probably 9.0.1, I don't think I've upgraded since my tests.
diff mbox series

Patch

diff --git a/xen/Makefile b/xen/Makefile
index a6120e577e9b..da017dc29d36 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -94,6 +94,8 @@  config: FORCE
 
 else # !config-build
 
+include scripts/Kbuild.include
+
 ifeq ($(need-config),y)
 include include/config/auto.conf
 # Read in dependencies to all Kconfig* files, make sure to run syncconfig if
@@ -113,6 +115,64 @@  $(KCONFIG_CONFIG):
 include/config/%.conf include/config/%.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 += -D__ASSEMBLY__
+
+CFLAGS += $(CFLAGS-y)
+# allow extra CFLAGS externally via EXTRA_CFLAGS_XEN_CORE
+CFLAGS += $(EXTRA_CFLAGS_XEN_CORE)
+
+# Most CFLAGS are safe for assembly files:
+#  -std=gnu{89,99} gets confused by #-prefixed end-of-line comments
+#  -flto makes no sense and annoys clang
+AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS))
+
+# LDFLAGS are only passed directly to $(LD)
+LDFLAGS += $(LDFLAGS_DIRECT) $(LDFLAGS-y)
+
+ifeq ($(CONFIG_UBSAN),y)
+CFLAGS_UBSAN := -fsanitize=undefined
+else
+CFLAGS_UBSAN :=
+endif
+
+ifeq ($(CONFIG_LTO),y)
+CFLAGS += -flto
+LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so
+endif
+
+include $(BASEDIR)/arch/$(TARGET_ARCH)/arch.mk
+
+# define new variables to avoid the ones defines in Config.mk
+export XEN_CFLAGS := $(CFLAGS)
+export XEN_AFLAGS := $(AFLAGS)
+export XEN_LDFLAGS := $(LDFLAGS)
+export CFLAGS_UBSAN
+
 endif # need-config
 
 .PHONY: build install uninstall clean distclean MAP
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 4aa119a90c27..f1311c45a372 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -38,59 +38,17 @@  ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
 
 # Initialise some variables
-CFLAGS_UBSAN :=
-
-ifeq ($(CONFIG_DEBUG),y)
-CFLAGS += -O1
-else
-CFLAGS += -O2
-endif
-
-ifeq ($(CONFIG_FRAME_POINTER),y)
-CFLAGS += -fno-omit-frame-pointer
-else
-CFLAGS += -fomit-frame-pointer
-endif
-
-CFLAGS += -nostdinc -fno-builtin -fno-common
-CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
-$(call cc-option-add,CFLAGS,CC,-Wvla)
-CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h
-CFLAGS-$(CONFIG_DEBUG_INFO) += -g
-
-ifneq ($(CONFIG_CC_IS_CLANG),y)
-# Clang doesn't understand this command line argument, and doesn't appear to
-# have an suitable alternative.  The resulting compiled binary does function,
-# but has an excessively large symbol table.
-CFLAGS += -Wa,--strip-local-absolute
-endif
-
-AFLAGS += -D__ASSEMBLY__
+CFLAGS-y :=
+AFLAGS-y :=
 
 ALL_OBJS := $(ALL_OBJS-y)
 
-CFLAGS += $(CFLAGS-y)
-# allow extra CFLAGS externally via EXTRA_CFLAGS_XEN_CORE
-CFLAGS += $(EXTRA_CFLAGS_XEN_CORE)
-
-# Most CFLAGS are safe for assembly files:
-#  -std=gnu{89,99} gets confused by #-prefixed end-of-line comments
-#  -flto makes no sense and annoys clang
-AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS))
-
-# LDFLAGS are only passed directly to $(LD)
-LDFLAGS += $(LDFLAGS_DIRECT)
-
-LDFLAGS += $(LDFLAGS-y)
-
 SPECIAL_DATA_SECTIONS := rodata $(foreach a,1 2 4 8 16, \
                                             $(foreach w,1 2 4, \
                                                         rodata.str$(w).$(a)) \
                                             rodata.cst$(a)) \
                          $(foreach r,rel rel.ro,data.$(r).local)
 
-include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
-
 include Makefile
 
 define gendep
@@ -107,7 +65,7 @@  $(foreach o,$(filter-out %/,$(obj-y) $(obj-bin-y) $(extra-y)),$(eval $(call gend
 subdir-y := $(subdir-y) $(filter %/, $(obj-y))
 obj-y    := $(patsubst %/, %/built_in.o, $(obj-y))
 
-$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
+$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += -DINIT_SECTIONS_ONLY
 
 ifeq ($(CONFIG_COVERAGE),y)
 ifeq ($(CONFIG_CC_IS_CLANG),y)
@@ -115,19 +73,16 @@  ifeq ($(CONFIG_CC_IS_CLANG),y)
 else
     COV_FLAGS := -fprofile-arcs -ftest-coverage
 endif
-$(filter-out %.init.o $(nocov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += $(COV_FLAGS)
+$(filter-out %.init.o $(nocov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += $(COV_FLAGS)
 endif
 
 ifeq ($(CONFIG_UBSAN),y)
-CFLAGS_UBSAN += -fsanitize=undefined
 # Any -fno-sanitize= options need to come after any -fsanitize= options
 $(filter-out %.init.o $(noubsan-y),$(obj-y) $(obj-bin-y) $(extra-y)): \
-CFLAGS += $(filter-out -fno-%,$(CFLAGS_UBSAN)) $(filter -fno-%,$(CFLAGS_UBSAN))
+CFLAGS-y += $(filter-out -fno-%,$(CFLAGS_UBSAN)) $(filter -fno-%,$(CFLAGS_UBSAN))
 endif
 
 ifeq ($(CONFIG_LTO),y)
-CFLAGS += -flto
-LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so
 # Would like to handle all object files as bitcode, but objects made from
 # pure asm are in a different format and have to be collected separately.
 # Mirror the directory tree, collecting them as built_in_bin.o.
@@ -140,10 +95,19 @@  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))
+
+# Calculation of flags, first the generic flags, then the arch specific flags,
+# and last the flags modified for a target or a directory.
+
+c_flags = -MMD -MF $(@D)/.$(@F).d $(XEN_CFLAGS) '-D__OBJECT_FILE__="$@"'
+a_flags = -MMD -MF $(@D)/.$(@F).d $(XEN_AFLAGS)
+
+include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk
 
-c_flags = -MMD -MF $(@D)/.$(@F).d $(CFLAGS) '-D__OBJECT_FILE__="$@"'
-a_flags = -MMD -MF $(@D)/.$(@F).d $(AFLAGS)
+c_flags += $(CFLAGS-y)
+a_flags += $(CFLAGS-y) $(AFLAGS-y)
 
 built_in.o: $(obj-y) $(extra-y)
 ifeq ($(obj-y),)
@@ -152,7 +116,7 @@  else
 ifeq ($(CONFIG_LTO),y)
 	$(LD_LTO) -r -o $@ $(filter-out $(extra-y),$^)
 else
-	$(LD) $(LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out $(extra-y),$^)
 endif
 endif
 
@@ -160,7 +124,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/arm/efi/Makefile b/xen/arch/arm/efi/Makefile
index d34c9168914a..e3ff2c3f283c 100644
--- a/xen/arch/arm/efi/Makefile
+++ b/xen/arch/arm/efi/Makefile
@@ -1,4 +1,4 @@ 
-CFLAGS += -fshort-wchar
+CFLAGS-y += -fshort-wchar
 
 obj-y +=  boot.init.o runtime.o
 obj-$(CONFIG_ACPI) +=  efi-dom0.init.o
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 7fbac8ac525d..5de873cf693e 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)
@@ -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) $(CFLAGS) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
+export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
 # Check if the linker supports PE.
 XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep --subsystem=10 -o efi/check.efi efi/check.o 2>/dev/null && echo y))
 CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
@@ -181,7 +181,7 @@  $(TARGET).efi: ALT_BASE = 0x$(shell $(NM) efi/relocs-dummy.o | sed -n 's, A ALT_
 
 ifneq ($(build_id_linker),)
 ifeq ($(call ld-ver-build-id,$(LD) $(filter -m%,$(EFI_LDFLAGS))),y)
-CFLAGS += -DBUILD_ID_EFI
+CFLAGS-y += -DBUILD_ID_EFI
 EFI_LDFLAGS += $(build_id_linker)
 note_file := efi/buildid.o
 # NB: this must be the last input in the linker call, because inputs following
@@ -228,7 +228,7 @@  efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: ;
 asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h
 	$(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -o $@ $<
 
-asm-macros.i: CFLAGS += -D__ASSEMBLY__ -P
+asm-macros.i: CFLAGS-y += -D__ASSEMBLY__ -P
 
 $(BASEDIR)/include/asm-x86/asm-macros.h: asm-macros.i Makefile
 	echo '#if 0' >$@.new
@@ -241,7 +241,7 @@  $(BASEDIR)/include/asm-x86/asm-macros.h: asm-macros.i Makefile
 	echo '#endif' >>$@.new
 	$(call move-if-changed,$@.new,$@)
 
-efi.lds: AFLAGS += -DEFI
+efi.lds: AFLAGS-y += -DEFI
 xen.lds efi.lds: xen.lds.S
 	$(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(a_flags)) -o $@ $<
 	sed -e 's/.*\.lds\.o:/$(@F):/g' <.$(@F).d >.$(@F).d.new
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 4b7ab784670c..56fe22c979ea 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -1,89 +1,10 @@ 
 ########################################
 # x86-specific definitions
 
-XEN_IMG_OFFSET := 0x200000
-
-CFLAGS += -I$(BASEDIR)/include
-CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
-CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
-CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET)
-CFLAGS += '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))'
-
-# Prevent floating-point variables from creeping into Xen.
-CFLAGS += -msoft-float
-
-ifeq ($(CONFIG_CC_IS_CLANG),y)
-# Note: Any test which adds -no-integrated-as will cause subsequent tests to
-# succeed, and not trigger further additions.
-#
-# The tests to select whether the integrated assembler is usable need to happen
-# before testing any assembler features, or else the result of the tests would
-# be stale if the integrated assembler is not used.
-
-# Older clang's built-in assembler doesn't understand .skip with labels:
-# https://bugs.llvm.org/show_bug.cgi?id=27369
-$(call as-option-add,CFLAGS,CC,".L0: .L1: .skip (.L1 - .L0)",,\
-                     -no-integrated-as)
-
-# Check whether clang asm()-s support .include.
-$(call as-option-add,CFLAGS,CC,".include \"asm-x86/indirect_thunk_asm.h\"",,\
-                     -no-integrated-as)
-
-# Check whether clang keeps .macro-s between asm()-s:
-# https://bugs.llvm.org/show_bug.cgi?id=36110
-$(call as-option-add,CFLAGS,CC,\
-                     ".macro FOO;.endm"$$(close); asm volatile $$(open)".macro FOO;.endm",\
-                     -no-integrated-as)
-endif
-
-$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
-$(call cc-option-add,CFLAGS,CC,-Wnested-externs)
-$(call as-option-add,CFLAGS,CC,"vmcall",-DHAVE_AS_VMX)
-$(call as-option-add,CFLAGS,CC,"crc32 %eax$$(comma)%eax",-DHAVE_AS_SSE4_2)
-$(call as-option-add,CFLAGS,CC,"invept (%rax)$$(comma)%rax",-DHAVE_AS_EPT)
-$(call as-option-add,CFLAGS,CC,"rdrand %eax",-DHAVE_AS_RDRAND)
-$(call as-option-add,CFLAGS,CC,"rdfsbase %rax",-DHAVE_AS_FSGSBASE)
-$(call as-option-add,CFLAGS,CC,"xsaveopt (%rax)",-DHAVE_AS_XSAVEOPT)
-$(call as-option-add,CFLAGS,CC,"rdseed %eax",-DHAVE_AS_RDSEED)
-$(call as-option-add,CFLAGS,CC,"clwb (%rax)",-DHAVE_AS_CLWB)
-$(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1", \
-                     -U__OBJECT_LABEL__ -DHAVE_AS_QUOTED_SYM \
-                     '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$$@')
-$(call as-option-add,CFLAGS,CC,"invpcid (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID)
-
-# GAS's idea of true is -1.  Clang's idea is 1
-$(call as-option-add,CFLAGS,CC,\
-    ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE)
-
-# Check to see whether the assmbler supports the .nop directive.
-$(call as-option-add,CFLAGS,CC,\
-    ".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOPS_DIRECTIVE)
-
-CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables
-
-# Xen doesn't use SSE interally.  If the compiler supports it, also skip the
-# SSE setup for variadic function calls.
-CFLAGS += -mno-sse $(call cc-option,$(CC),-mskip-rax-setup)
-
-# Compile with thunk-extern, indirect-branch-register if avaiable.
-ifeq ($(CONFIG_INDIRECT_THUNK),y)
-CFLAGS += -mindirect-branch=thunk-extern -mindirect-branch-register
-CFLAGS += -fno-jump-tables
+ifneq ($(filter -DHAVE_AS_QUOTED_SYM,$(XEN_CFLAGS)),)
+object_label_flags = '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$@'
+else
+object_label_flags = '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))'
 endif
-
-# If supported by the compiler, reduce stack alignment to 8 bytes. But allow
-# this to be overridden elsewhere.
-$(call cc-option-add,CFLAGS-stack-boundary,CC,-mpreferred-stack-boundary=3)
-CFLAGS += $(CFLAGS-stack-boundary)
-
-ifeq ($(CONFIG_UBSAN),y)
-# Don't enable alignment sanitisation.  x86 has efficient unaligned accesses,
-# and various things (ACPI tables, hypercall pages, stubs, etc) are wont-fix.
-# It also causes an as-yet-unidentified crash on native boot before the
-# console starts.
-$(call cc-option-add,CFLAGS_UBSAN,CC,-fno-sanitize=alignment)
-endif
-
-# Set up the assembler include path properly for older toolchains.
-CFLAGS += -Wa,-I$(BASEDIR)/include
-
+c_flags += $(object_label_flags) $(CFLAGS-stack-boundary)
+a_flags += $(object_label_flags) $(CFLAGS-stack-boundary)
diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
new file mode 100644
index 000000000000..2a51553edb3c
--- /dev/null
+++ b/xen/arch/x86/arch.mk
@@ -0,0 +1,84 @@ 
+########################################
+# 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-x86/indirect_thunk_asm.h\"",,\
+                     -no-integrated-as)
+
+# Check whether clang keeps .macro-s between asm()-s:
+# https://bugs.llvm.org/show_bug.cgi?id=36110
+$(call as-option-add,CFLAGS,CC,\
+                     ".macro FOO;.endm"$$(close); asm volatile $$(open)".macro FOO;.endm",\
+                     -no-integrated-as)
+endif
+
+$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
+$(call cc-option-add,CFLAGS,CC,-Wnested-externs)
+$(call as-option-add,CFLAGS,CC,"vmcall",-DHAVE_AS_VMX)
+$(call as-option-add,CFLAGS,CC,"crc32 %eax$$(comma)%eax",-DHAVE_AS_SSE4_2)
+$(call as-option-add,CFLAGS,CC,"invept (%rax)$$(comma)%rax",-DHAVE_AS_EPT)
+$(call as-option-add,CFLAGS,CC,"rdrand %eax",-DHAVE_AS_RDRAND)
+$(call as-option-add,CFLAGS,CC,"rdfsbase %rax",-DHAVE_AS_FSGSBASE)
+$(call as-option-add,CFLAGS,CC,"xsaveopt (%rax)",-DHAVE_AS_XSAVEOPT)
+$(call as-option-add,CFLAGS,CC,"rdseed %eax",-DHAVE_AS_RDSEED)
+$(call as-option-add,CFLAGS,CC,"clwb (%rax)",-DHAVE_AS_CLWB)
+$(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1",-DHAVE_AS_QUOTED_SYM)
+$(call as-option-add,CFLAGS,CC,"invpcid (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID)
+
+# GAS's idea of true is -1.  Clang's idea is 1
+$(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.
+CFLAGS-$(CONFIG_INDIRECT_THUNK) += -mindirect-branch=thunk-extern
+CFLAGS-$(CONFIG_INDIRECT_THUNK) += -mindirect-branch-register
+CFLAGS-$(CONFIG_INDIRECT_THUNK) += -fno-jump-tables
+
+# If supported by the compiler, reduce stack alignment to 8 bytes. But allow
+# this to be overridden elsewhere.
+$(call cc-option-add,CFLAGS-stack-boundary,CC,-mpreferred-stack-boundary=3)
+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/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index 4bc0a196e9ca..490d791aae2d 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -1,4 +1,4 @@ 
-CFLAGS += -fshort-wchar
+CFLAGS-y += -fshort-wchar
 
 %.o: %.ihex
 	$(OBJCOPY) -I ihex -O binary $< $@
diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile
index 3d9e38f27e65..464c448d9d37 100644
--- a/xen/common/libelf/Makefile
+++ b/xen/common/libelf/Makefile
@@ -3,10 +3,10 @@  nocov-y += libelf.o
 
 SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
 
-CFLAGS += -Wno-pointer-sign
+CFLAGS-y += -Wno-pointer-sign
 
 libelf.o: libelf-temp.o Makefile
 	$(OBJCOPY) $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
 
 libelf-temp.o: libelf-tools.o libelf-loader.o libelf-dominfo.o #libelf-relocate.o
-	$(LD) $(LDFLAGS) -r -o $@ $^
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
diff --git a/xen/common/libfdt/Makefile b/xen/common/libfdt/Makefile
index c075bbf5462a..e2a5e59380a0 100644
--- a/xen/common/libfdt/Makefile
+++ b/xen/common/libfdt/Makefile
@@ -5,10 +5,10 @@  SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
 obj-y += libfdt.o
 nocov-y += libfdt.o
 
-CFLAGS += -I$(BASEDIR)/include/xen/libfdt/
+CFLAGS-y += -I$(BASEDIR)/include/xen/libfdt/
 
 libfdt.o: libfdt-temp.o Makefile
 	$(OBJCOPY) $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
 
 libfdt-temp.o: $(LIBFDT_OBJS)
-	$(LD) $(LDFLAGS) -r -o $@ $^
+	$(LD) $(XEN_LDFLAGS) -r -o $@ $^
diff --git a/xen/include/Makefile b/xen/include/Makefile
index a488a98d8bb7..2a10725d689b 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -64,7 +64,7 @@  compat/%.h: compat/%.i Makefile $(BASEDIR)/tools/compat-build-header.py
 	mv -f $@.new $@
 
 compat/%.i: compat/%.c Makefile
-	$(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
+	$(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(XEN_CFLAGS)) $(cppflags-y) -o $@ $<
 
 compat/%.c: public/%.h xlat.lst Makefile $(BASEDIR)/tools/compat-build-source.py
 	mkdir -p $(@D)
diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index b1fd45421993..011ef5ca91f8 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -4,7 +4,7 @@  obj-y += flask_op.o
 
 obj-y += ss/
 
-CFLAGS += -I./include
+CFLAGS-y += -I./include
 
 AWK = awk
 
diff --git a/xen/xsm/flask/ss/Makefile b/xen/xsm/flask/ss/Makefile
index 046ce8f53326..d32b9e07138e 100644
--- a/xen/xsm/flask/ss/Makefile
+++ b/xen/xsm/flask/ss/Makefile
@@ -8,4 +8,4 @@  obj-y += services.o
 obj-y += conditional.o
 obj-y += mls.o
 
-CFLAGS += -I../include
+CFLAGS-y += -I../include