| Message ID | 2c83b876-6fd8-1315-3b28-b45e877187aa@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86emul: further work | expand |
On 24/03/2020 12:33, Jan Beulich wrote: > With the exception of HAVE_AS_QUOTED_SYM, populate the results into a > generated header instead of (at least once per [sub]directory) into > CFLAGS. This results in proper rebuilds (via make dependencies) in case > the compiler used changes between builds. It additionally eases > inspection of which assembler features were actually found usable. > > Some trickery is needed to avoid header generation itself to try to > include the to-be/not-yet-generated header. > > Since the definitions in generated/config.h, previously having been > command line options, might even affect xen/config.h or its descendants, > move adding of the -include option for the latter after inclusion of the > per-arch Rules.mk. Use the occasion to also move the most general -I > option to the common Rules.mk. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Given the work of Anthony's which is already committed in staging, I'd really prefer this patch to look something like https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=WIP.x86/asm&id=95ef9f80ed6359e89f988121521c421b7e9528de That avoids all fragile games with includes, and is the position we want to be in, longterm. All the requisite infrastructure looks to be already present. ~Andrew
On 25.03.2020 22:12, Andrew Cooper wrote: > On 24/03/2020 12:33, Jan Beulich wrote: >> With the exception of HAVE_AS_QUOTED_SYM, populate the results into a >> generated header instead of (at least once per [sub]directory) into >> CFLAGS. This results in proper rebuilds (via make dependencies) in case >> the compiler used changes between builds. It additionally eases >> inspection of which assembler features were actually found usable. >> >> Some trickery is needed to avoid header generation itself to try to >> include the to-be/not-yet-generated header. >> >> Since the definitions in generated/config.h, previously having been >> command line options, might even affect xen/config.h or its descendants, >> move adding of the -include option for the latter after inclusion of the >> per-arch Rules.mk. Use the occasion to also move the most general -I >> option to the common Rules.mk. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Given the work of Anthony's which is already committed in staging, I'd > really prefer this patch to look something like > https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=WIP.x86/asm&id=95ef9f80ed6359e89f988121521c421b7e9528de > > That avoids all fragile games with includes, and is the position we want > to be in, longterm. Ah, so they already have something going in that direction. Looks okay to me, albeit ... > All the requisite infrastructure looks to be already present. ... there's the one open prereq question of what happens upon tool chain updates. It's not clear to me if/how kconfig would get invoked despite none of the recorded dependencies having changed in such a case. (I'm sure you realize there's no issue with this when the determination occurs out of a makefile.) Jan
On Thu, Mar 26, 2020 at 10:50:48AM +0100, Jan Beulich wrote: > On 25.03.2020 22:12, Andrew Cooper wrote: > > All the requisite infrastructure looks to be already present. > > ... there's the one open prereq question of what happens upon > tool chain updates. It's not clear to me if/how kconfig would > get invoked despite none of the recorded dependencies having > changed in such a case. (I'm sure you realize there's no issue > with this when the determination occurs out of a makefile.) We might need one small change for this to happen, it is to add a comment in .config which display the output of `$(CC) --version | head -1`. Simple :-). If the output of `$(CC) --version` changes, kconfig will run again. That would be enough to detect tool chain updates, right? Have a look at "include/config/auto.conf.cmd" to find out how kconfig is forced to run again. I'll prepare a patch.
On 26.03.2020 14:42, Anthony PERARD wrote: > On Thu, Mar 26, 2020 at 10:50:48AM +0100, Jan Beulich wrote: >> On 25.03.2020 22:12, Andrew Cooper wrote: >>> All the requisite infrastructure looks to be already present. >> >> ... there's the one open prereq question of what happens upon >> tool chain updates. It's not clear to me if/how kconfig would >> get invoked despite none of the recorded dependencies having >> changed in such a case. (I'm sure you realize there's no issue >> with this when the determination occurs out of a makefile.) > > We might need one small change for this to happen, it is to add a > comment in .config which display the output of `$(CC) --version | head > -1`. Simple :-). > If the output of `$(CC) --version` changes, kconfig will run again. That > would be enough to detect tool chain updates, right? I'm afraid it's not that simple: For one I'm not sure that line would indeed change when a distro issues a gcc update. Even the minor version may not change in this case; recall as an example the backport of the compiler support backing INDIRECT_THUNK. And then gcc isn't the tool chain - it may well be that e.g. gas gets updated (supporting new insns or directives) without gcc getting touched at all. Plus finally I don't think a comment like you suggest would do - while processing it kconfig would find that $(CC) gets used, but aiui it would then record just $(CC) as needing tracking, not the output of the command. But maybe I'm lacking some further detail here. > Have a look at "include/config/auto.conf.cmd" to find out how kconfig is > forced to run again. Oh, that's good to know. Thanks for the pointer. Jan
On 25.03.2020 22:12, Andrew Cooper wrote: > On 24/03/2020 12:33, Jan Beulich wrote: >> With the exception of HAVE_AS_QUOTED_SYM, populate the results into a >> generated header instead of (at least once per [sub]directory) into >> CFLAGS. This results in proper rebuilds (via make dependencies) in case >> the compiler used changes between builds. It additionally eases >> inspection of which assembler features were actually found usable. >> >> Some trickery is needed to avoid header generation itself to try to >> include the to-be/not-yet-generated header. >> >> Since the definitions in generated/config.h, previously having been >> command line options, might even affect xen/config.h or its descendants, >> move adding of the -include option for the latter after inclusion of the >> per-arch Rules.mk. Use the occasion to also move the most general -I >> option to the common Rules.mk. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Given the work of Anthony's which is already committed in staging, I'd > really prefer this patch to look something like > https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=WIP.x86/asm&id=95ef9f80ed6359e89f988121521c421b7e9528de > > That avoids all fragile games with includes, and is the position we want > to be in, longterm. I've thought about this some more, and not just because of the issue with tool chain updates (or downgrades) I'm not convinced this is the way to go, irrespective of whether Linux does. I've gone through Linux'es Kconfig documentation again, and I couldn't find a hint that this is supposed to cover other than user choices. Determining tool chain capabilities at build (rather than configure) time seems quite a bit more reliable to me. IOW there ought to be a clear distinction between "what kind of kernel [or hypervisor] do I want" and "how does the kernel [hypervisor] get built". When it comes to certain user choices requiring certain tool chain capabilities, then the help text should point this out and the build should probably be made fail if the prereqs aren't met (alternatively behavior like that of our xen.efi building could be chosen, with a warning issued during the build process). If we (and perhaps also they) clearly stated that the intention is to also latch system properties (like userland ./configure would do), and if the intended behavior was clearly spelled out when it comes to any of those latched properties changing, then I might change my opinion. Jan
--- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -55,7 +55,7 @@ endif CFLAGS += -nostdinc -fno-builtin -fno-common CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith $(call cc-option-add,CFLAGS,CC,-Wvla) -CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h +CFLAGS += -pipe -D__XEN__ -I$(BASEDIR)/include CFLAGS-$(CONFIG_DEBUG_INFO) += -g CFLAGS += '-D__OBJECT_FILE__="$@"' @@ -95,6 +95,9 @@ SPECIAL_DATA_SECTIONS := rodata $(foreac include $(BASEDIR)/arch/$(TARGET_ARCH)/Rules.mk +# Allow the arch to use -include ahead of this one. +CFLAGS += -include xen/config.h + include Makefile define gendep --- a/xen/arch/arm/Rules.mk +++ b/xen/arch/arm/Rules.mk @@ -6,8 +6,6 @@ # 'make clean' before rebuilding. # -CFLAGS += -I$(BASEDIR)/include - $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS)) $(call cc-option-add,CFLAGS,CC,-Wnested-externs) --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -225,7 +225,8 @@ endif efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: $(BASEDIR)/arch/x86/efi/built_in.o efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o efi/relocs-dummy.o: ; -asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h +asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c $(BASEDIR)/include/asm-x86/asm-macros.h \ + $(BASEDIR)/include/generated/config.h $(CC) $(filter-out -Wa$(comma)% -flto,$(CFLAGS)) -S -o $@ $< asm-macros.i: CFLAGS += -D__ASSEMBLY__ -P @@ -241,6 +242,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) + +# GAS's idea of true is -1. Clang's idea is 1. +NEGATIVE_TRUE-insn := .if ((1 > 0) > 0); .error \"\"; .endif + +# Check to see whether the assembler supports the .nops directive. +NOPS_DIRECTIVE-insn := .L1: .L2: .nops (.L2 - .L1),9 + +as-features-list := $(as-ISA-list) NEGATIVE_TRUE NOPS_DIRECTIVE + +$(BASEDIR)/include/generated/config.h: FORCE + echo '/* Generated header, do not edit. */' >$@.new + $(foreach f,$(as-features-list), \ + $(if $($(f)-insn),,$(error $(f)-insn is empty)) \ + echo '#$(call as-insn,$(CC) $(CFLAGS),"$($(f)-insn)", \ + define, \ + undef) HAVE_AS_$(f) /* $($(f)-insn) */' >>$@.new;) + $(call move-if-changed,$@.new,$@) + +endif + efi.lds: AFLAGS += -DEFI xen.lds efi.lds: xen.lds.S $(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(AFLAGS)) -o $@ $< --- a/xen/arch/x86/Rules.mk +++ b/xen/arch/x86/Rules.mk @@ -3,7 +3,7 @@ XEN_IMG_OFFSET := 0x200000 -CFLAGS += -I$(BASEDIR)/include +CFLAGS += $(if $(filter asm-macros.% %/generated/config.h,$@),,-include generated/config.h) CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default CFLAGS += -DXEN_IMG_OFFSET=$(XEN_IMG_OFFSET) @@ -38,26 +38,9 @@ endif $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS)) $(call cc-option-add,CFLAGS,CC,-Wnested-externs) -$(call as-option-add,CFLAGS,CC,"vmcall",-DHAVE_AS_VMX) -$(call as-option-add,CFLAGS,CC,"crc32 %eax$$(comma)%eax",-DHAVE_AS_SSE4_2) -$(call as-option-add,CFLAGS,CC,"invept (%rax)$$(comma)%rax",-DHAVE_AS_EPT) -$(call as-option-add,CFLAGS,CC,"rdrand %eax",-DHAVE_AS_RDRAND) -$(call as-option-add,CFLAGS,CC,"rdfsbase %rax",-DHAVE_AS_FSGSBASE) -$(call as-option-add,CFLAGS,CC,"xsaveopt (%rax)",-DHAVE_AS_XSAVEOPT) -$(call as-option-add,CFLAGS,CC,"rdseed %eax",-DHAVE_AS_RDSEED) -$(call as-option-add,CFLAGS,CC,"clwb (%rax)",-DHAVE_AS_CLWB) $(call as-option-add,CFLAGS,CC,".equ \"x\"$$(comma)1", \ -U__OBJECT_LABEL__ -DHAVE_AS_QUOTED_SYM \ '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$$@') -$(call as-option-add,CFLAGS,CC,"invpcid (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID) - -# GAS's idea of true is -1. Clang's idea is 1 -$(call as-option-add,CFLAGS,CC,\ - ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE) - -# Check to see whether the assmbler supports the .nop directive. -$(call as-option-add,CFLAGS,CC,\ - ".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOPS_DIRECTIVE) CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables --- a/xen/include/Makefile +++ b/xen/include/Makefile @@ -64,7 +64,7 @@ compat/%.h: compat/%.i Makefile $(BASEDI mv -f $@.new $@ compat/%.i: compat/%.c Makefile - $(CPP) $(filter-out -Wa$(comma)% -M% %.d -include %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $< + $(CPP) $(filter-out -Wa$(comma)% -M% %.d -include %/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $< compat/%.c: public/%.h xlat.lst Makefile $(BASEDIR)/tools/compat-build-source.py mkdir -p $(@D) --- a/xen/scripts/Kbuild.include +++ b/xen/scripts/Kbuild.include @@ -10,7 +10,7 @@ DEPS_INCLUDE = $(addsuffix .d2, $(basena # 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)) \ -c -x c -o /dev/null - 2>&1),$(4),$(3)) # as-option-add: Conditionally add options to flags
With the exception of HAVE_AS_QUOTED_SYM, populate the results into a generated header instead of (at least once per [sub]directory) into CFLAGS. This results in proper rebuilds (via make dependencies) in case the compiler used changes between builds. It additionally eases inspection of which assembler features were actually found usable. Some trickery is needed to avoid header generation itself to try to include the to-be/not-yet-generated header. Since the definitions in generated/config.h, previously having been command line options, might even affect xen/config.h or its descendants, move adding of the -include option for the latter after inclusion of the per-arch Rules.mk. Use the occasion to also move the most general -I option to the common Rules.mk. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v5: Re-base. v4: New. --- An alternative to the $(MAKECMDGOALS) trickery would be to make generation of generated/config.h part of the asm-offsets.s rule, instead of adding it as a dependency there. Not sure whether either is truly better than the other.