[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCHv6] 02/28] build: build Kconfig and config rules
>>> On 24.11.15 at 18:51, <cardoe@xxxxxxxxxx> wrote: > --- a/.gitignore > +++ b/.gitignore > @@ -217,6 +217,11 @@ tools/xentrace/tbctl > tools/xentrace/xenctx > tools/xentrace/xentrace > xen/.banner > +xen/.config > +xen/.config.old > +xen/defconfig > +xen/**/*.cmd > +xen/**/modules.order The last two seem rather dubious to me in our environment. > @@ -239,6 +244,9 @@ xen/include/headers++.chk > xen/include/asm > xen/include/asm-*/asm-offsets.h > xen/include/compat/* > +xen/include/config.h Linux doesn't seem to generate a file like this. Is this perhaps a stale entry you're copying over? > --- /dev/null > +++ b/xen/Kconfig > @@ -0,0 +1,26 @@ > +# > +# For a description of the syntax of this configuration file, > +# see Documentation/kbuild/kconfig-language.txt from the Linux This path needs fixing. > +# kernel source tree. > +# > +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. > --- 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). > @@ -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? > +# Allow people to just run `make` as before and not force them to configure > +$(BASEDIR)/.config $(BASEDIR)/include/config/auto.conf.cmd: ; > + $(Q)$(MAKE) -f $(BASEDIR)/scripts/kconfig/Makefile > ARCH=$(XEN_TARGET_ARCH) defconfig Is this correct? If I have xen/.config but no xen/include/config/auto.conf.cmd I surely don't want "defconfig" to be run (unless "defconfig" honors xen/.config, in which case the goal's name would be questionable)? Also do you really need all the explicit $(BASEDIR) references in the rules above? > --- /dev/null > +++ b/xen/arch/arm/configs/arm32_defconfig > @@ -0,0 +1 @@ > +CONFIG_64BIT=n > diff --git a/xen/arch/arm/configs/arm64_defconfig > b/xen/arch/arm/configs/arm64_defconfig > new file mode 100644 > index 0000000..e69de29 As said before I'm not really up to speed with defconfigs, but the arm64 variant being empty but the arm32 one disabling 64BIT seems backwards: You don't even have a choice on arm32. > --- /dev/null > +++ b/xen/arch/x86/Kconfig > @@ -0,0 +1,18 @@ > +config X86_64 > + def_bool y > + > +config X86 > + def_bool y > + select HAS_GDBSX Didn't you mean to convert HAS_* in subsequent patches? Also - not 64BIT here, yet this - if added to ARM - should be added here too so it can be used in common code. > +config ARCH_DEFCONFIG > + string > + default "arch/x86/configs/x86_64_defconfig" x86_defconfig perhaps? > --- /dev/null > +++ b/xen/scripts/kconfig/Makefile > @@ -0,0 +1,77 @@ > +# xen/scripts/kconfig > + > +MAKEFLAGS += -rR Why? > +# default rule to do nothing > +all: > + > +XEN_ROOT = $(CURDIR)/.. > + > +# Xen doesn't have a silent build flag > +quiet := silent_ > +Q := @ > +kecho := : > + > +# eventually you'll want to do out of tree builds > +srctree = $(XEN_ROOT)/xen > +objtree = $(srctree) > +src := scripts/kconfig > +obj := $(src) > +KBUILD_SRC := > + > +# handle functions (most of these lifted from different Linux makefiles > +dot-target = $(dir $@).$(notdir $@) > +depfile = $(subst $(comma),,$(dot-target).d) > +basetarget = $(basename $(notdir $@)) > +cmd = $(cmd_$(1)) > +if_changed = $(if y, \ > + @set -e; \ > + $(cmd_$(1)); \ > + ) > + > +if_changed_dep = $(if y, \ > + @set -e; \ > + $(cmd_$(1)); \ > + ) Considering how much you stripped these fro their Linux original, why did you leave the always true $(if ...) in here, and why did you not consolidate things only single lines (helping readability). > +define multi_depend > +$(foreach m, $(notdir $1), \ > + $(eval $(obj)/$m: \ > + $(addprefix $(obj)/, $(foreach s, $3, $($(m:%$(strip $2)=%$(s))))))) > +endef Do you really need this? > +# 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)? Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |