Re: [XEN PATCH v5 04/16] xen/build: have the root Makefile generates the CFLAGS

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

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

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

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

What is av6?

Thanks,

-- 
Anthony PERARD