Re: [Xen-devel] [PATCH v3 1/8] x86: determine HAVE_AS_* just once

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.01.2020 13:04, Roger Pau Monné wrote:
> On Mon, Jan 06, 2020 at 05:34:45PM +0100, Jan Beulich wrote:
>> --- a/Config.mk
>> +++ b/Config.mk
>> @@ -151,7 +151,7 @@ endif
>>  # 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 -M% %.d -include %/config.h,$(1)) \
> 
> Don't you need to filter out -include xen/config.h as added to CLFAGS
> below?

The whole point of the change is to filter out _both_ config.h (as
well as any future one) - the one under include/xen/ and the one
under include/generated/. Hence the widening of what the pattern
would match.

>> @@ -240,6 +241,45 @@ $(BASEDIR)/include/asm-x86/asm-macros.h:
>>      echo '#endif' >>$@.new
>>      $(call move-if-changed,$@.new,$@)
>>  
>> +# There are multiple invocations of this Makefile, one each for 
>> asm-offset.s,
>> +# $(TARGET), built_in.o, and several more from the rules building $(TARGET)
>> +# and $(TARGET).efi. The 2nd and 3rd may race with one another, and we don't
>> +# want to re-generate config.h in that case anyway, so guard the logic
>> +# accordingly. (We do want to have the FORCE dependency on the rule, to be
>> +# sure we pick up changes when the compiler used has changed.)
>> +ifeq ($(MAKECMDGOALS),asm-offsets.s)
>> +
>> +as-ISA-list := CLWB EPT FSGSBASE INVPCID RDRAND RDSEED SSE4_2 VMX XSAVEOPT
>> +
>> +CLWB-insn   := clwb (%rax)
>> +EPT-insn    := invept (%rax),%rax
>> +FSGSBASE-insn       := rdfsbase %rax
>> +INVPCID-insn        := invpcid (%rax),%rax
>> +RDRAND-insn := rdrand %eax
>> +RDSEED-insn := rdseed %eax
>> +SSE4_2-insn := crc32 %eax,%eax
>> +VMX-insn    := vmcall
>> +XSAVEOPT-insn       := xsaveopt (%rax)
>> +
>> +as-features-list := $(as-ISA-list) NEGATIVE_TRUE NOPS_DIRECTIVE
> 
> I think it would be clearer to place this below the NEGATIVE_TRUE and
> NOPS_DIRECTIVE definitions below? So that all FOO-insn are together.

If your remark was about just the last line - yes, perhaps (and
looking at it again I don't even know why it ended up in the
place it's in right now). But I'm told the original mechanism is
going to be replaced now by a Kconfig one anyway. If this is
going to happen soon, the patch here would be of no further
interest.

Jan

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