Re: [Xen-devel] [PATCHv6] 02/28] build: build Kconfig and config rules

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.12.15 at 01:34, <cardoe@xxxxxxxxxx> wrote:
> On 12/1/15 5:22 AM, Jan Beulich wrote:
>>>>> On 30.11.15 at 18:53, <cardoe@xxxxxxxxxx> wrote:
>>> On 11/30/15 8:36 AM, Jan Beulich wrote:
>>>>>>> On 24.11.15 at 18:51, <cardoe@xxxxxxxxxx> wrote:
>>>>> +mainmenu "Xen/$SRCARCH $XEN_FULLVERSION Configuration"
>>>>> +
>>>>> +config SRCARCH
>>>>> + string
>>>>> + option env="SRCARCH"
>>>>> +
>>>>> +config ARCH
>>>>> + string
>>>>> + option env="ARCH"
>>>>> +
>>>>> +source "arch/$SRCARCH/Kconfig"
>>>>> +
>>>>> +config XEN_FULLVERSION
>>>>> + string
>>>>> + option env="XEN_FULLVERSION"
>>>>> +
>>>>> +config DEFCONFIG_LIST
>>>>> + string
>>>>> + option defconfig_list
>>>>> + default "$ARCH_DEFCONFIG"
>>>>> + default "arch/$SRCARCH/defconfig"
>>>>
>>>> Linux'es variant has just SRCARCH and the source directive. Why
>>>> do we need so much more here? In any even I again think you
>>>> should at least briefly explain any extensions you do in the commit
>>>> message.
>>>
>>> I'm not understanding the question here. Linux has 5 items in their
>>> list. The last two are identical to the 2 that I retained. Given other
>>> evolutions of the patchset I can drop the last one and keep it as just
>>> $ARCH_DEFCONFIG since I've always got that defined for arm and x86.
>> 
>> Oh, the context was ambiguous: The comment related to everything
>> quoted, not just the last option. I.e. I'm being irritated by there being
>> quite a few more config options here compared to Linux.
> 
> So all these items are in Linux. And they seem to actually be necessary.
> You'll just find them in different files (e.g. init/Kconfig) that I
> didn't import because we really didn't need them for anything.

Okay, this makes sense. Remains the question on the
XEN_FULLVERSION name: Doesn't this collide with the identically
named make variable? Or is it intentionally named the same? In
which case - what's the purpose? There's no
CONFIG_KERNELVERSION resulting from this in Linux (which seems
in line with documentation).

>>>>> --- a/xen/Makefile
>>>>> +++ b/xen/Makefile
>>>>> @@ -17,6 +17,9 @@ export XEN_ROOT := $(BASEDIR)/..
>>>>>  
>>>>>  EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
>>>>>  
>>>>> +# Don't break if the build process wasn't called from the top level
>>>>> +XEN_TARGET_ARCH ?= $(shell uname -m)
>>>>
>>>> I don't see the need for this. We already require setting this on the
>>>> command line when invoking a build process bypassing the top level
>>>> directory. Later in the series, when actually using the produced
>>>> xen/.config, I could see this becoming a nice enhancement (by
>>>> reading the value from the file).
>>>
>>> One of the early on reviews told me it was not required (and I can
>>> confirm that I can run "git clean -xdf && cd xen && make" and it will
>>> build). This change makes that behavior still work.
>> 
>> Yes, non-cross builds of course have been working without setting
>> that variable. I was (not clearly enough) referring to cross builds,
>> including hypervisor builds on ix86 distros (which I consider at best
>> kind of cross, but would really deem to be native with there not
>> being any 32-bit hypervisor anymore).
>> 
>> So can you clarify what exactly breaks in which way without this
>> change?
> 
> The ability to "git clone git://xen && cd xen/xen && make" because I
> couldn't pick the correct defconfig to use. See the rules just below
> this reply in xen/Makefile where they are used.

Okay. But - does "uname -m" really produce e.g. "arm32" and "arm64"
respectively, and not, as I would expect, "armv..." and "aarch64"?
See the setup of XEN_COMPILE_ARCH in ./Config.mk.

>>>>> @@ -216,3 +220,16 @@ FORCE:
>>>>>  
>>>>>  %/: FORCE
>>>>>   $(MAKE) -f $(BASEDIR)/Rules.mk -C $* built_in.o built_in_bin.o
>>>>> +
>>>>> +kconfig := silentoldconfig oldconfig config menuconfig defconfig \
>>>>> + nconfig xconfig gconfig savedefconfig listnewconfig olddefconfig
>>>>> +.PHONY: $(kconfig)
>>>>> +$(kconfig):
>>>>> + $(MAKE) -f $(BASEDIR)/scripts/kconfig/Makefile ARCH=$(XEN_TARGET_ARCH) 
>>>>> $@
>>>>> +
>>>>> +$(BASEDIR)/include/config/%.conf: $(BASEDIR)/include/config/auto.conf.cmd
>>>>> + $(Q)$(MAKE) -f $(BASEDIR)/scripts/kconfig/Makefile 
>>>>> ARCH=$(XEN_TARGET_ARCH) silentoldconfig
>>>>
>>>> Okay, I have found the Linux original to this and it's the same there,
>>>> but I'd like to ask anyway: How can this work without $* getting
>>>> passed on?
>>>
>>> silentoldconfig is the step that actually takes the .config and
>>> generates out all the files like auto.conf and generated/config.h It
>>> should be in xen/scripts/kconfig/Makefile.linux I believe.
>> 
>> Not sure what you're trying to tell me, but in any event I don't see
>> this to answer the question.
> 
> So maybe I'm not understanding your question. I don't understand why $*
> would need to be passed on. The target "silentoldconfig" in the kconfig
> Makefile takes care of building all of the files that the rule could
> match. That's what I tried to say in my original response. I believe
> your point is that I would have to pass the target file on like the
> below implicit case?

Yes, that's what one would expect from a pattern rule. And yes, now
I understand your answer to the original question.

>>>>> +config ARCH_DEFCONFIG
>>>>> + string
>>>>> + default "arch/x86/configs/x86_64_defconfig"
>>>>
>>>> x86_defconfig perhaps?
>>>
>>> No. I was told to drop support for x86 entirely in an earlier review.
>>> Its not possible to configure for 32-bit x86 in v6.
>> 
>> x86 != 32-bit. I think you're mixing this up with ix86 or x86-32.
>> Here I consider x86 as to basic architecture without any
>> particular bit width in mind.
> 
> ok. Well the syntax is still "arch/SUBARCH/configs/ARCH_defconfig" so
> the original is correct. There is no defconfig for the ambiguous x86
> family. You're either building for x86_64 or x86_32 (which I referred to
> as x86 in my original response).
> 
> This defconfig is for the 64-bit architecture of x86 (x86_64) and there
> for its named correctly.

But there is no x86_32 architecture form the hypervisor build's
point of view, and hence x86 isn't ambiguous. In fact the mid-term
plan is to remove leftovers of references to x86_64 (like the
arch/x86/x86_64/ or include/asm-x86/x86_64/ directories) where
possible. The only place they need to be kept are in the public
interface.

>>>>> +# Set our default defconfig file
>>>>> +KBUILD_DEFCONFIG := $(ARCH)_defconfig
>>>>> +
>>>>> +# provide our shell
>>>>> +CONFIG_SHELL := $(SHELL)
>>>>> +
>>>>> +# provide the host compiler
>>>>> +HOSTCC := gcc
>>>>> +HOSTCXX := g++
>>>>
>>>> Why is the C setting from ./Config.mk not good enough (the C++
>>>> one should then be added there if indeed needed, which I doubt)?
>>>
>>> From an earlier review I dropped the usage of Config.mk because I was
>>> told the hypervisor (xen/) should build without ./configure being run.
>>> The C++ is needed for the kconfig frontends.
>> 
>> But Config.mk has nothing to do with top level configure, and is e.g.
>> being included from xen/Rules.mk.
> 
> I'm not including Config.mk in those build files because it includes a
> lot and many of them are redefinitions of what kconfig does. All I would
> get out of it would be HOSTCC.
> 
> $ wc -l config/x86_64.mk config/Linux.mk config/StdGNU.mk Config.mk
>    32 config/x86_64.mk
>     3 config/Linux.mk
>    50 config/StdGNU.mk
>   287 Config.mk
>   372 total
> 
> If there was a bit more granularity (config/host.mk) it wouldn't be so bad.

Indeed introducing that would seem like a useful intermediate step.
No multiple points of defining the same thing please (because of
someone actually needing to fiddle with them will quite likely touch
only one instance, resulting in time wasted figuring out why things
no longer work uniformly).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel