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

> 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?

Henning

> Jan
> 

-- 
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.

Reply via email to