|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: Kconfig vs tool chain capabilities
> On 25 Aug 2020, at 13:51, Jürgen Groß <jgross@xxxxxxxx> wrote: > > On 25.08.20 14:44, Bertrand Marquis wrote: >>> On 25 Aug 2020, at 13:37, Jürgen Groß <jgross@xxxxxxxx> wrote: >>> >>> On 25.08.20 14:20, Bertrand Marquis wrote: >>>>> On 25 Aug 2020, at 12:22, Jürgen Groß <jgross@xxxxxxxx> wrote: >>>>> >>>>> On 25.08.20 13:16, Bertrand Marquis wrote: >>>>>>> On 25 Aug 2020, at 12:06, Jürgen Groß <jgross@xxxxxxxx> wrote: >>>>>>> >>>>>>> On 25.08.20 12:17, Bertrand Marquis wrote: >>>>>>>>> On 25 Aug 2020, at 10:49, Jürgen Groß <jgross@xxxxxxxx> 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. > > But this would exactly be the behavior. > > A new HAS_COMPILER_OPTION_ added would default to your tool capabilities > and you could change it manually afterwards. If you change it to "y" in > spite of your tools not supporting it the build would fail. And if it > would be "y" per default initially the .config file would be created > with the option enabled, so a tool change removing support of the option > would result in a failed build, too. > Make sense, it seems i did not think that through :-) So that works for me. Bertrand
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |