Re: [Xen-devel] [XEN PATCH v3 15/23] xen/build: have the root Makefile generates the CFLAGS

From: Anthony PERARD <anthony.perard@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>, "Wei Liu" <wl@xen.org>,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"George Dunlap" <George.Dunlap@eu.citrix.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	xen-devel@lists.xenproject.org,
	"Daniel De Graaf" <dgdegra@tycho.nsa.gov>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [XEN PATCH v3 15/23] xen/build: have the root Makefile generates the CFLAGS
Date: Tue, 17 Mar 2020 18:35:03 +0000	[thread overview]
Message-ID: <20200317183503.GB4088@perard.uk.xensource.com> (raw)
In-Reply-To: <dd7bf903-6f51-7610-3963-2b572a6bde49@suse.com>

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,

-- 
Anthony PERARD

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