Re: [PATCH 1/2] x86/EFI: correct compiler probing

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, Dec 07, 2023 at 11:48:12AM +0100, Jan Beulich wrote:
> Passing in $(CFLAGS) means also requesting inclusion of certain headers
> (via -include command line options). That's particularly xen/config.h,
> which in turn requires generated/autoconf.h. This has not caused any
> problems so far only because arch.mk is processed twice, and the missing

The first pass is ignore because "include/config/auto.conf" is missing,
and generating that will also generates "generated/autoconf.h" I think.
So result don't matter, and make restart from scratch once "auto.conf"
is generated.

> header on the 1st pass would be there on the 2nd. Having added an
> inclusion of asm/asm-macros.h to x86'es asm/config.h, the 2nd pass then

I don't see asm-macros.h been included in asm/config.h, is this
in a pending patch?

> also fails on an initial, pristine build.
> 
> As per dd40177c1bc8 ("x86-64/EFI: add CFLAGS to check compile") dropping
> the use of $(CFLAGS) altogether isn't an option, though. Hence remove
> the problematic options only.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

In any case, patch looks fine:
Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

> ---
> This is now the 3rd place where the -include needs dropping. I was half
> decided to introduce a new lazy-expansion variable, yet it's not
> consistently $(CFLAGS) that the options need purging from. Thoughts?

Something like that I guess, we probably want to avoid the "-include"
while testing the compiler. I guess introducing "-include config.h" once
we have all CFLAGS might make more sense, that is at about the time
where XEN_CFLAGS is currently introduced, but I haven't checked if
that's fine to do.

Cheers,

-- 
Anthony PERARD