[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [XEN PATCH v3 14/23] xen/build: use new $(c_flags) and $(a_flags) instead of $(CFLAGS)

On Wed, Mar 04, 2020 at 03:42:36PM +0100, Jan Beulich wrote:
> On 26.02.2020 12:33, Anthony PERARD wrote:
> > --- a/xen/scripts/Kbuild.include
> > +++ b/xen/scripts/Kbuild.include
> > @@ -10,7 +10,7 @@ DEPS_INCLUDE = $(addsuffix .d2, $(basename $(wildcard 
> > $(DEPS))))
> >  # as-insn: Check whether assembler supports an instruction.
> >  # Usage: cflags-y += $(call as-insn,CC FLAGS,"insn",option-yes,option-no)
> >  as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
> > -                       | $(filter-out -M% %.d -include 
> > %/include/xen/config.h,$(1)) \
> > +                       | $(filter-out -include 
> > %/include/xen/config.h,$(1)) \
> >                                -c -x c -o /dev/null - 2>&1),$(4),$(3))
> I'm sorry, while it was me to suggest this change - is this
> correct? The variable to modify is a parameter of this macro,
> i.e. things aren't limited to CFLAGS here. If we want to
> disallow use with e.g. c_flags or anything derived from it,
> then we should find some way to actually enforce this (like
> dropping the respective parameter; I'm uncertain though whether
> we wouldn't regret this if we ever got to the point where we
> wanted to use a newer insn in a .S file).

It is probably better to leave the macro as it is for now. I'll drop
this hunk.

I think it would be nice to have the macro use directly CFLAGS (or
XEN_CFLAGS), but since the macro is used within as-option-add, which
needs the flags variable name as parameter, we can't really change the
way as-insn works.

We could come back to that later, remove the use of as-option-add, and
have as-insn use CFLAGS (or AFLAGS) directly. That's how the macro
as-instr of Linux works, the macro always uses KBUILD_AFLAGS.


Anthony PERARD

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.