> On 25 Aug 2020, at 13:37, Jürgen Groß <jgr...@suse.com> wrote: > > On 25.08.20 14:20, Bertrand Marquis wrote: >>> On 25 Aug 2020, at 12:22, Jürgen Groß <jgr...@suse.com> wrote: >>> >>> On 25.08.20 13:16, Bertrand Marquis wrote: >>>>> On 25 Aug 2020, at 12:06, Jürgen Groß <jgr...@suse.com> wrote: >>>>> >>>>> On 25.08.20 12:17, Bertrand Marquis wrote: >>>>>>> On 25 Aug 2020, at 10:49, Jürgen Groß <jgr...@suse.com> wrote: >>>>>>> >>>>>>> On 25.08.20 10:48, Jan Beulich wrote: >>>>>>>> On 25.08.2020 10:08, Jürgen Groß wrote: >>>>>>>>> On 25.08.20 09:48, Jan Beulich wrote: >>>>>>>>>> On 25.08.2020 09:43, Jürgen Groß wrote: >>>>>>>>>>> On 25.08.20 09:34, Jan Beulich wrote: >>>>>>>>>>>> On 25.08.2020 09:12, Jürgen Groß wrote: >>>>>>>>>>>>> I think both problems can be solved at the same time via the >>>>>>>>>>>>> following >>>>>>>>>>>>> approach: >>>>>>>>>>>>> >>>>>>>>>>>>> - collect the data which is reflected in today's CONFIG_ >>>>>>>>>>>>> variables in a >>>>>>>>>>>>> single script and store it in a file, e.g in a format like: >>>>>>>>>>>>> >>>>>>>>>>>>> CC_IS_GCC y >>>>>>>>>>>>> GCC_VERSION 70500 >>>>>>>>>>>>> CLANG_VERSION 0 >>>>>>>>>>>>> CC_HAS_VISIBILITY_ATTRIBUTE y >>>>>>>>>>>>> >>>>>>>>>>>>> - check the tool data at each build to match the contents of that >>>>>>>>>>>>> file >>>>>>>>>>>>> and either fail the build or update the file and rerun >>>>>>>>>>>>> kconfig if they >>>>>>>>>>>>> don't match (I think failing the build and requiring to run a >>>>>>>>>>>>> "make config" would be the better approach) >>>>>>>>>>>>> >>>>>>>>>>>>> - fill the CONFIG_ variable contents from that file in kconfig >>>>>>>>>>>>> instead >>>>>>>>>>>>> of issuing the single shell commands >>>>>>>>>>>> >>>>>>>>>>>> While I agree this is a possible model to use (but still not the >>>>>>>>>>>> one we've inherited from Linux), I fail to see how this addresses >>>>>>>>>>>> my "developers should be aware of what they do (not) build and >>>>>>>>>>>> test" concern: There'd still be dependencies of Kconfig options >>>>>>>>>>>> on the tool chain capabilities, and their prompts therefore would >>>>>>>>>>>> still be invisible without the tool chain having the needed >>>>>>>>>>>> capabilities. IOW I only see this to address 2), but not 1). >>>>>>>>>>> >>>>>>>>>>> Sorry, I fail to see a problem here. >>>>>>>>>>> >>>>>>>>>>> What sense does it make to be able to configure an option which the >>>>>>>>>>> tools don't support? >>>>>>>>>> >>>>>>>>>> Take CET as an example (chosen because that's the one which >>>>>>>>>> already uses the Kconfig approach, despite my disagreement): It's >>>>>>>>>> quite relevant to know whether you're testing Xen with it enabled, >>>>>>>>>> or with it disabled (and hence you potentially missing changes you >>>>>>>>>> need to make to relevant code portions). >>>>>>>>> >>>>>>>>> Correct me if I'm wrong, but assuming my suggested changes being made, >>>>>>>>> wouldn't a .config file setup once with CET enabled (and I assume >>>>>>>>> you'd >>>>>>>>> try to enable CET on purpose when trying to test CET and you'd realize >>>>>>>>> not being able to do so in case your tools don't support CET) ensure >>>>>>>>> you'd never been hit by surprise when some tool updates would remove >>>>>>>>> CET support? >>>>>>>> Probably, but that's not my point. With a CET-incapable tool chain >>>>>>>> you're not prompted whether to enable CET in the first place, when >>>>>>>> creating the initial .config. It is this unawareness of a crucial >>>>>>>> part of code not getting built and tested (and likely unknowingly) >>>>>>>> that I dislike. In fact, after Andrew's patches had gone in, it >>>>>>>> had taken me a while to realize that in certain of my builds I don't >>>>>>>> have CET enabled (despite me having done nothing to disable it), and >>>>>>>> hence those builds working fine are meaningless for any changes >>>>>>>> affecting CET code in any way. >>>>>>> >>>>>>> Yes, this is the result of letting some options depend on others. >>>>>>> >>>>>>> This is what I meant regarding the architecture: there are e.g. multiple >>>>>>> source files in drivers/char/ being built only for ARM or X86, in spite >>>>>>> of being located outside arch/. And yet you don't see this as a problem, >>>>>>> even if you are not able to select those drivers to be built when using >>>>>>> "the other" arch. They are silently disabled. Just like CET in case of >>>>>>> an incapable tool chain. >>>>>>> >>>>>>> So IMO either we ban "depends on" from our Kconfig files (no, I don't >>>>>>> want to do that), or we use it as designed and make it as user friendly >>>>>>> as possible. In case we as developers have a special test case then we >>>>>>> need to check the .config whether the desired settings are really >>>>>>> present. Having those settings depending on tool capabilities in a >>>>>>> specific file will make this check much easier. >>>>>>> >>>>>>> And BTW, I can't see how setting the tolls' capabilities from e.g. >>>>>>> arch/x86/Rules.mk is better in any way (see how CONFIG_INDIRECT_THUNK >>>>>>> got its value in older Xen versions like 4.12). >>>>>>> >>>>>>> We can't have everything and I believe automatically disabling features >>>>>>> which can't work with the current tools is a sane decision. Doing this >>>>>>> via Kconfig is the better approach compared to Makefile sniplets IMO. >>>>>> That sounds like a nice feature to have some compiler or tools options >>>>>> that >>>>>> can be selected or activated in Kconfig. There are some compiler options >>>>>> mandatory to handle some erratas or XSA that one might want to >>>>>> explicitely >>>>>> select. >>>>>> I am bit unsure about the part where some kconfig options would only >>>>>> be available or not depending on some tests with the compiler being doing >>>>>> prior to opening the editor. I would guess the menuconfig process would >>>>>> have to first run some tests and then generated some HAS_ configuration >>>>>> options depending on the result of the tests. >>>>>> Did i got the idea right here ? >>>>>> Is this something somebody tried to do ? >>>>>> As a user I would more expect that the build process would tell me that >>>>>> my >>>>>> configuration is invalid because i selected something that is not >>>>>> supported >>>>>> by my compiler. I might have the chance to just fix my build to use the >>>>>> right >>>>>> compiler (like by mistake using x86 toolchain to compile for arm). >>>>>> We should also be careful not to silently ignore some configuration >>>>>> option if >>>>>> one is changing the compiler and the new one does not support an option. >>>>>> A user would have his configuration and compile using it without >>>>>> passing through the editor interface and might need to be aware that a >>>>>> part >>>>>> of his configuration is not valid anymore because the tools he is using >>>>>> changed. >>>>>> This is something that could occur a lot when using a distribution >>>>>> toolchain. >>>>>> Also there are some compiler option changing so i would more think that >>>>>> there should be generic configuration options so that in the makefiles we >>>>>> could have the opportunity to add different compiler options for >>>>>> different >>>>>> toolchains depending on the version or the type of the toolchain. >>>>>> To be clear i would see something like: >>>>>> in kconfig: >>>>>> COMPILER_OPTION_XXX >>>>>> bool “activate XXX compiler flag >>>>>> in Makefile: >>>>>> ifeq ($(CONFIG_COMPILER_OPTION_XXX), true) >>>>>> test_compiler_cxx: >>>>>> $(CC) -xxx dummy.c -o dummy || $(error Your compiler does not support >>>>>> -xxx) >>>>>> cc-flags += -xxx >>>>>> endif >>>>>> The syntax is wrong here but you get the idea :-) >>>>> >>>>> Ah, okay, this is another approach, which might be even more flexible. >>>>> It would allow to control compiler flags instead of more high level >>>>> features. >>>> We might have both, this would also allow to have more high level features >>>> which are >>>> doing both adding compiler flags and other stuff, >>>>> >>>>> In case we want to go that route we should default COMPILER_OPTION_XXX >>>>> to the current tool capabilities in order to avoid longer try-and-error >>>>> loops. >>>> I am not quite sure how you want to achieve this cleanly. >>> >>> Something like (picked an actual example from x86): >>> >>> config HAS_COMPILER_OPTION_IBR >>> bool "Select compiler option -mindirect-branch-register" >>> default $(cc-option,-mindirect-branch-register) >>> blah blah blah >>> >> Nice :-) >> Definitely i would add a “default y if EXPERT” or something equivalent. > > Uh, rather not. I as a developer don't want to have change the config > manually just because a new HAS_COMPILER_OPTION_ has been added my tools > don't understand (yet). The default action should require no user > intervention, even as expert.
I agree with the argument. Maybe we could have an other option like DISABLE_COMPILER_CHECK for this. I would rather have my test system fail with a make error by setting this then silently discard the option if my compiler is modified. Bertrand
