On 2017-04-12 15:17, Henning Schild wrote:
> Am Wed, 12 Apr 2017 14:09:12 +0200
> schrieb Jan Kiszka <[email protected]>:
>
>> On 2017-04-12 12:59, Henning Schild wrote:
>>> Am Tue, 11 Apr 2017 21:47:04 +0200
>>> schrieb Jan Kiszka <[email protected]>:
>>>
>>>> On 2017-04-11 18:56, Henning Schild wrote:
>>>>> Move it up one level so the hypervisor core can make use of it
>>>>> itself.
>>>>>
>>>>> Signed-off-by: Henning Schild <[email protected]>
>>>>>
>>>>> diff --git a/Kbuild b/Kbuild
>>>>> --- a/Kbuild
>>>>> +++ b/Kbuild
>>>>> @@ -1,7 +1,7 @@
>>>>> #
>>>>> # Jailhouse, a Linux-based partitioning hypervisor
>>>>> #
>>>>> -# Copyright (c) Siemens AG, 2013-2015
>>>>> +# Copyright (c) Siemens AG, 2013-2017
>>>>> #
>>>>> # Authors:
>>>>> # Jan Kiszka <[email protected]>
>>>>> @@ -11,12 +11,40 @@
>>>>> # the COPYING file in the top-level directory.
>>>>> #
>>>>>
>>>>> +define filechk_config_mk
>>>>> +(
>>>>> \
>>>>> + echo "\$$(foreach config,\$$(filter
>>>>> CONFIG_%, \
>>>>> + \$$(.VARIABLES)), \$$(eval undefine
>>>>> \$$(config)))"; \
>>>>> + if [ -f $(src)/hypervisor/include/jailhouse/config.h ];
>>>>> then \
>>>>> + sed -e "/^#define
>>>>> \([^[:space:]]*\)[[:space:]]*1/!d" \
>>>>> + -e "s/^#define
>>>>> \([^[:space:]]*\)[[:space:]]*1/\1=y/"\
>>>>> +
>>>>> $(src)/hypervisor/include/jailhouse/config.h; \
>>>>> +
>>>>> fi
>>>>> \ +) +endef
>>>>> +
>>>>> +$(obj)/hypervisor/include/generated/config.mk: $(src)/Makefile
>>>>> FORCE
>>>>> + $(call filechk,config_mk)
>>>>> +
>>>>> +CONFIG_MK := $(obj)/hypervisor/include/generated/config.mk
>>>>
>>>> I've just realized that you can make use of this variable also in
>>>> hypervisor/Makefile if you add
>>>>
>>>> export CONFIG_MK
>>>>
>>>> And then that makefile can re-export the var to the arch makefile
>>>> as it used to do it so far. Nicer than traversing via ../../...
>>>
>>> Arghh ... CONFIG_ is somehow special, could have guessed so. Will
>>> send another round with that and VERSION_H being exported.
>>
>> Yeah, then let's call it GENERATED_CONFIG_MK or so.
>>
>>>
>>>>> +
>>>>> +define filechk_version
>>>>> + $(src)/scripts/gen_version_h $(src)/
>>>>> +endef
>>>>> +
>>>>> +$(obj)/hypervisor/include/generated/version.h: $(src)/Makefile
>>>>> FORCE
>>>>> + $(call filechk,version)
>>>>> +
>>>>> +VERSION_H := $(obj)/hypervisor/include/generated/version.h
>>>>> +
>>>>> +FORCE:
>>>>
>>>> Why this target, BTW?
>>>
>>> That target does not seem to be available at that stage, so i
>>> introduced it myself. It is required because we always want to
>>> generate version.h and config.mk while we can not model deps
>>> correctly. config.h might not exist, jailhouse might not be under
>>> git etc. Basically copied the rules including the dep on FORCE and
>>> needed to implement FORCE.
>>
>> Why? What was the error when it was missing (I cannot reproduce,
>> that's why I'm asking)?
>
> No rule to make target 'FORCE', ...
>
> GNU Make 4.2.1
> Built for x86_64-pc-linux-gnu
Ah, now I got this as well, during make clean. We should be protected by
kbuild which actually provides this target for us, but there seems to be
a gap. Anyway, makes (some) sense now.
>
>> FORCE is an unmet prerequisite that is there to enforce unconditional
>> rule execution.
>>
>>>
>>>>> +
>>>>> subdir-y := driver hypervisor configs inmates tools
>>>>>
>>>>> subdir-ccflags-y := -Werror
>>>>>
>>>>> -# inmates build depends on generated config.mk of the hypervisor,
>>>>> -# and the driver needs version.h from there
>>>>> -$(obj)/inmates $(obj)/driver: $(obj)/hypervisor
>>>>> +# directory dependencies on generated files
>>>>> +$(obj)/driver $(obj)/hypervisor: $(VERSION_H)
>>>>> +$(obj)/hypervisor $(obj)/inmates $(obj)/driver: $(CONFIG_MK)
>>>>>
>>>>> -clean-dirs := Documentation/generated
>>>>> +clean-dirs := Documentation/generated
>>>>> hypervisor/include/generated diff --git a/hypervisor/Makefile
>>>>> b/hypervisor/Makefile --- a/hypervisor/Makefile
>>>>> +++ b/hypervisor/Makefile
>>>>> @@ -1,7 +1,7 @@
>>>>> #
>>>>> # Jailhouse, a Linux-based partitioning hypervisor
>>>>> #
>>>>> -# Copyright (c) Siemens AG, 2013-2016
>>>>> +# Copyright (c) Siemens AG, 2013-2017
>>>>> # Copyright (c) Valentine Sinitsyn, 2014
>>>>> #
>>>>> # Authors:
>>>>> @@ -35,26 +35,7 @@ GCOV_PROFILE := n
>>>>> CORE_OBJECTS = setup.o printk.o paging.o control.o lib.o mmio.o
>>>>> pci.o ivshmem.o CORE_OBJECTS += uart.o uart-8250.o
>>>>>
>>>>> -define filechk_config_mk
>>>>> -(
>>>>> \
>>>>> - echo "\$$(foreach config,\$$(filter
>>>>> CONFIG_%, \
>>>>> - \$$(.VARIABLES)), \$$(eval undefine
>>>>> \$$(config)))"; \
>>>>> - if [ -f $(src)/include/jailhouse/config.h ];
>>>>> then \
>>>>> - sed -e "/^#define
>>>>> \([^[:space:]]*\)[[:space:]]*1/!d" \
>>>>> - -e "s/^#define
>>>>> \([^[:space:]]*\)[[:space:]]*1/\1=y/" \
>>>>> -
>>>>> $(src)/include/jailhouse/config.h; \
>>>>> -
>>>>> fi
>>>>> \ -) -endef
>>>>> -
>>>>> -$(obj)/include/generated/config.mk: Makefile FORCE
>>>>> - $(call filechk,config_mk)
>>>>> -
>>>>> -define filechk_version
>>>>> - $(src)/../scripts/gen_version_h $(src)/..
>>>>> -endef
>>>>> -
>>>>> -clean-dirs := include/generated arch/$(SRCARCH)/include/generated
>>>>> +clean-dirs := arch/$(SRCARCH)/include/generated
>>>>>
>>>>> define sed-y
>>>>> "/^=>/{s:=>#\(.*\):/* \1 */:; \
>>>>> @@ -91,17 +72,13 @@ targets := $(defines-file) arch/$(SRCARC
>>>>> $(Q)mkdir -p $(dir $@)
>>>>> $(call cmd,defines)
>>>>>
>>>>> -$(obj)/include/generated/version.h: $(src)/Makefile FORCE
>>>>> - $(call filechk,version)
>>>>> -
>>>>> $(foreach co,$(CORE_OBJECTS),\
>>>>> $(eval $(obj)/$(co): $(obj)/$(defines-file)))
>>>>>
>>>>> $(obj)/setup.o: $(obj)/include/generated/version.h
>>>>>
>>>>> -arch-builtin: $(obj)/$(defines-file)
>>>>> $(obj)/include/generated/config.mk FORCE
>>>>> - $(Q)$(MAKE) $(build)=$(obj)/arch/$(SRCARCH) \
>>>>> - CONFIG_MK=$(obj)/include/generated/config.mk
>>>>> +arch-builtin: $(obj)/$(defines-file) FORCE
>>>>> + $(Q)$(MAKE) $(build)=$(obj)/arch/$(SRCARCH)
>>>>
>>>> + CONFIG_MK=$(CONFIG_MK), see above.
>>>
>>> Not required after the export.
>>>
>>>>>
>>>>> always :=
>>>>>
>>>>> diff --git a/hypervisor/arch/arm-common/Kbuild
>>>>> b/hypervisor/arch/arm-common/Kbuild ---
>>>>> a/hypervisor/arch/arm-common/Kbuild +++
>>>>> b/hypervisor/arch/arm-common/Kbuild @@ -1,7 +1,7 @@
>>>>> #
>>>>> # Jailhouse, a Linux-based partitioning hypervisor
>>>>> #
>>>>> -# Copyright (c) Siemens AG, 2013-2016
>>>>> +# Copyright (c) Siemens AG, 2013-2017
>>>>> #
>>>>> # Authors:
>>>>> # Jan Kiszka <[email protected]>
>>>>> @@ -10,7 +10,7 @@
>>>>> # the COPYING file in the top-level directory.
>>>>> #
>>>>>
>>>>> -include $(CONFIG_MK)
>>>>> +include $(obj)/../../include/generated/config.mk
>>>>
>>>> And keep this.
>>>>
>>>>>
>>>>> GCOV_PROFILE := n
>>>>>
>>>>> diff --git a/hypervisor/arch/x86/Kbuild
>>>>> b/hypervisor/arch/x86/Kbuild --- a/hypervisor/arch/x86/Kbuild
>>>>> +++ b/hypervisor/arch/x86/Kbuild
>>>>> @@ -1,7 +1,7 @@
>>>>> #
>>>>> # Jailhouse, a Linux-based partitioning hypervisor
>>>>> #
>>>>> -# Copyright (c) Siemens AG, 2013
>>>>> +# Copyright (c) Siemens AG, 2013-2017
>>>>> # Copyright (c) Valentine Sinitsyn, 2014
>>>>> #
>>>>> # Authors:
>>>>> @@ -12,6 +12,8 @@
>>>>> # the COPYING file in the top-level directory.
>>>>> #
>>>>>
>>>>> +include $(obj)/../../include/generated/config.mk
>>>>> +
>>>>> GCOV_PROFILE := n
>>>>>
>>>>> BUILT_IN_OBJECTS := built-in-amd.o built-in-intel.o
>>>>>
>>>>
>>>> The x86 include isn't needed, right? But I agree that it's nicer to
>>>> have it for consistency reasons. Then I would just add it to
>>>> hypervisor/Makefile already in this patch and declare common
>>>> availability.
>>>
>>> It now moved to "hypervisor: introduce GCOV support"
>>
>> Wrong direction: I meant that this patch should make the config system
>> available throughout the whole hypervisor. Then that other patch could
>> simple use it.
>
> Ok, so the way it was. Is that on its own terrible enough to call for
> v7?
That alone not. Some variable names sound a bit weird now, and you
already updated one of the patches - let the dust settle for a while.
Jan
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
