diff mbox series

[4/4] stubdom: add fine grained library config items to Mini-OS configs

Message ID 20241005151548.29184-5-jgross@suse.com (mailing list archive)
State New
Headers show
Series stubdom: prepare more fine grained Xen library usage | expand

Commit Message

Jürgen Groß Oct. 5, 2024, 3:15 p.m. UTC 
Today Mini-OS can only be configured to use all or none Xen library.
In order to prepare a more fine grained configuration scheme, add per
library config items to the Mini-OS config files.

As some libraries pull in others, the config files need to be
extended at build time to reflect those indirect library uses.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 stubdom/.gitignore |  1 +
 stubdom/Makefile   | 49 ++++++++++++++++++++++++++++++++++------------
 2 files changed, 38 insertions(+), 12 deletions(-)

Comments

Anthony PERARD Oct. 7, 2024, 2:15 p.m. UTC | #1 
On Sat, Oct 05, 2024 at 05:15:48PM +0200, Juergen Gross wrote:
> diff --git a/stubdom/Makefile b/stubdom/Makefile
> index 8c503c2bf8..3b501a0710 100644
> --- a/stubdom/Makefile
> +++ b/stubdom/Makefile
> @@ -340,6 +340,14 @@ endef
>  
>  $(foreach lib,$(STUB_LIBS),$(eval $(call BUILD_lib,$(lib))))
>  
> +define BUILD_config
> + cp $< $@
> + for i in $(sort $(APP_LIBS) $(call xenlibs-dependencies,$(APP_LIBS))); do \
> +   u=`echo $$i | tr a-z A-Z`; \
> +   echo "CONFIG_LIBXEN$$u=y"; \
> + done >> $@
> +endef

I don't think I like having a recipe hidden like that in a variable,
maybe if it was a full rule it would be a bit less annoying to me. But
how about something slightly different:

First, the name, "GEN_config" would be a bit better, then we could have
it only do the output and not writing any file:

define GEN_config
 (cat '$<' && \
 for i in $(sort $(APP_LIBS) $(call xenlibs-dependencies,$(APP_LIBS))); do \
   u=`echo $$i | tr a-z A-Z`; \
   echo "CONFIG_LIBXEN$$u=y"; \
 done)
endef

The this can be used in rules as:
    $(GEN_config) > $@

Would that be ok?

(It might be better to have the macro not depends on the environment
have take parameter explicitly which could be used as $(call
GEN_config,$<,evtchn gnttab) > $@ or take a variable if it's useful
elsewhere, but I'm already fine if $@ is taken out of the macro.)

> +
>  xenstore/stamp: $(XEN_ROOT)/tools/xenstored/Makefile.common
>  	$(do_links)
>  
> @@ -373,8 +381,12 @@ $(TARGETS_MINIOS): mini-os-%:
>  # ioemu
>  #######
>  
> -ioemu-minios-config.mk: $(CURDIR)/ioemu-minios.cfg
> -	MINIOS_CONFIG="$<" CONFIG_FILE="$(CURDIR)/$@" $(MAKE) DESTDIR= -C $(MINI_OS) config
> +ioemu-minios.out.cfg: APP_LIBS = evtchn gnttab ctrl guest
> +ioemu-minios.out.cfg: $(CURDIR)/ioemu-minios.cfg Makefile

Could you change the suffix to ".gen.cfg"? ".out.cfg" is a bit generic
while "generated" is more common for the kind of file that are
automatically generated by the build system for it's own use.

BTW, in the first prerequisite, $(CURDIR) isn't necessary anymore, it
was only to be used in "MINIOS_CONFIG" just below.

> +	$(BUILD_config)
> +
> +ioemu-minios-config.mk: ioemu-minios.out.cfg
> +	MINIOS_CONFIG="$(CURDIR)/$<" CONFIG_FILE="$(CURDIR)/$@" $(MAKE) DESTDIR= -C $(MINI_OS) config

Thanks,
Jürgen Groß Oct. 7, 2024, 3:08 p.m. UTC | #2 
On 07.10.24 16:15, Anthony PERARD wrote:
> On Sat, Oct 05, 2024 at 05:15:48PM +0200, Juergen Gross wrote:
>> diff --git a/stubdom/Makefile b/stubdom/Makefile
>> index 8c503c2bf8..3b501a0710 100644
>> --- a/stubdom/Makefile
>> +++ b/stubdom/Makefile
>> @@ -340,6 +340,14 @@ endef
>>   
>>   $(foreach lib,$(STUB_LIBS),$(eval $(call BUILD_lib,$(lib))))
>>   
>> +define BUILD_config
>> + cp $< $@
>> + for i in $(sort $(APP_LIBS) $(call xenlibs-dependencies,$(APP_LIBS))); do \
>> +   u=`echo $$i | tr a-z A-Z`; \
>> +   echo "CONFIG_LIBXEN$$u=y"; \
>> + done >> $@
>> +endef
> 
> I don't think I like having a recipe hidden like that in a variable,
> maybe if it was a full rule it would be a bit less annoying to me. But
> how about something slightly different:
> 
> First, the name, "GEN_config" would be a bit better, then we could have
> it only do the output and not writing any file:
> 
> define GEN_config
>   (cat '$<' && \
>   for i in $(sort $(APP_LIBS) $(call xenlibs-dependencies,$(APP_LIBS))); do \
>     u=`echo $$i | tr a-z A-Z`; \
>     echo "CONFIG_LIBXEN$$u=y"; \
>   done)
> endef
> 
> The this can be used in rules as:
>      $(GEN_config) > $@
> 
> Would that be ok?

Absolutely fine with me.

> (It might be better to have the macro not depends on the environment
> have take parameter explicitly which could be used as $(call
> GEN_config,$<,evtchn gnttab) > $@ or take a variable if it's useful
> elsewhere, but I'm already fine if $@ is taken out of the macro.)
> 
>> +
>>   xenstore/stamp: $(XEN_ROOT)/tools/xenstored/Makefile.common
>>   	$(do_links)
>>   
>> @@ -373,8 +381,12 @@ $(TARGETS_MINIOS): mini-os-%:
>>   # ioemu
>>   #######
>>   
>> -ioemu-minios-config.mk: $(CURDIR)/ioemu-minios.cfg
>> -	MINIOS_CONFIG="$<" CONFIG_FILE="$(CURDIR)/$@" $(MAKE) DESTDIR= -C $(MINI_OS) config
>> +ioemu-minios.out.cfg: APP_LIBS = evtchn gnttab ctrl guest
>> +ioemu-minios.out.cfg: $(CURDIR)/ioemu-minios.cfg Makefile
> 
> Could you change the suffix to ".gen.cfg"? ".out.cfg" is a bit generic
> while "generated" is more common for the kind of file that are
> automatically generated by the build system for it's own use.

Okay.

> 
> BTW, in the first prerequisite, $(CURDIR) isn't necessary anymore, it
> was only to be used in "MINIOS_CONFIG" just below.

Ah, right. Will remove it.


Juergen
diff mbox series

Patch

diff --git a/stubdom/.gitignore b/stubdom/.gitignore
index 10e2547a22..c6a88467ae 100644
--- a/stubdom/.gitignore
+++ b/stubdom/.gitignore
@@ -1,3 +1,4 @@ 
+*.out.cfg
 /*.tar.gz
 /*-minios-config.mk
 /autom4te.cache/
diff --git a/stubdom/Makefile b/stubdom/Makefile
index 8c503c2bf8..3b501a0710 100644
--- a/stubdom/Makefile
+++ b/stubdom/Makefile
@@ -340,6 +340,14 @@  endef
 
 $(foreach lib,$(STUB_LIBS),$(eval $(call BUILD_lib,$(lib))))
 
+define BUILD_config
+ cp $< $@
+ for i in $(sort $(APP_LIBS) $(call xenlibs-dependencies,$(APP_LIBS))); do \
+   u=`echo $$i | tr a-z A-Z`; \
+   echo "CONFIG_LIBXEN$$u=y"; \
+ done >> $@
+endef
+
 xenstore/stamp: $(XEN_ROOT)/tools/xenstored/Makefile.common
 	$(do_links)
 
@@ -373,8 +381,12 @@  $(TARGETS_MINIOS): mini-os-%:
 # ioemu
 #######
 
-ioemu-minios-config.mk: $(CURDIR)/ioemu-minios.cfg
-	MINIOS_CONFIG="$<" CONFIG_FILE="$(CURDIR)/$@" $(MAKE) DESTDIR= -C $(MINI_OS) config
+ioemu-minios.out.cfg: APP_LIBS = evtchn gnttab ctrl guest
+ioemu-minios.out.cfg: $(CURDIR)/ioemu-minios.cfg Makefile
+	$(BUILD_config)
+
+ioemu-minios-config.mk: ioemu-minios.out.cfg
+	MINIOS_CONFIG="$(CURDIR)/$<" CONFIG_FILE="$(CURDIR)/$@" $(MAKE) DESTDIR= -C $(MINI_OS) config
 
 .PHONY: ioemu
 ioemu: cross-zlib cross-libpci libxenguest ioemu-minios-config.mk
@@ -435,8 +447,12 @@  grub-upstream: grub-$(GRUB_VERSION).tar.gz
 		patch -d $@ -p1 < $$i || exit 1; \
 	done
 
-grub-$(XEN_TARGET_ARCH)-minios-config.mk: $(CURDIR)/grub/minios.cfg
-	MINIOS_CONFIG="$<" CONFIG_FILE="$(CURDIR)/$@" $(MAKE) DESTDIR= -C $(MINI_OS) config
+grub/minios.out.cfg: APP_LIBS = guest ctrl toollog
+grub/minios.out.cfg: $(CURDIR)/grub/minios.cfg Makefile
+	$(BUILD_config)
+
+grub-$(XEN_TARGET_ARCH)-minios-config.mk: grub/minios.out.cfg
+	MINIOS_CONFIG="$(CURDIR)/$<" CONFIG_FILE="$(CURDIR)/$@" $(MAKE) DESTDIR= -C $(MINI_OS) config
 
 .PHONY: grub
 grub: cross-polarssl grub-upstream $(CROSS_ROOT) grub-$(XEN_TARGET_ARCH)-minios-config.mk
@@ -447,8 +463,12 @@  grub: cross-polarssl grub-upstream $(CROSS_ROOT) grub-$(XEN_TARGET_ARCH)-minios-
 # xenstore
 ##########
 
-xenstore-minios-config.mk: $(CURDIR)/xenstore-minios.cfg
-	MINIOS_CONFIG="$<" CONFIG_FILE="$(CURDIR)/$@" $(MAKE) DESTDIR= -C $(MINI_OS) config
+xenstore-minios.out.cfg: APP_LIBS = gnttab evtchn toollog ctrl
+xenstore-minios.out.cfg: $(CURDIR)/xenstore-minios.cfg Makefile
+	$(BUILD_config)
+
+xenstore-minios-config.mk: xenstore-minios.out.cfg
+	MINIOS_CONFIG="$(CURDIR)/$<" CONFIG_FILE="$(CURDIR)/$@" $(MAKE) DESTDIR= -C $(MINI_OS) config
 
 .PHONY: xenstore
 xenstore: $(CROSS_ROOT) xenstore-minios-config.mk
@@ -458,8 +478,12 @@  xenstore: $(CROSS_ROOT) xenstore-minios-config.mk
 # xenstorepvh
 #############
 
-xenstorepvh-minios-config.mk: $(CURDIR)/xenstorepvh-minios.cfg
-	MINIOS_CONFIG="$<" CONFIG_FILE="$(CURDIR)/$@" $(MAKE) DESTDIR= -C $(MINI_OS) config
+xenstorepvh-minios.out.cfg: APP_LIBS = gnttab evtchn toollog ctrl
+xenstorepvh-minios.out.cfg: $(CURDIR)/xenstorepvh-minios.cfg Makefile
+	$(BUILD_config)
+
+xenstorepvh-minios-config.mk: xenstorepvh-minios.out.cfg
+	MINIOS_CONFIG="$(CURDIR)/$<" CONFIG_FILE="$(CURDIR)/$@" $(MAKE) DESTDIR= -C $(MINI_OS) config
 
 .PHONY: xenstorepvh
 xenstorepvh: $(CROSS_ROOT) xenstorepvh-minios-config.mk
@@ -472,7 +496,7 @@  xenstorepvh: $(CROSS_ROOT) xenstorepvh-minios-config.mk
 .PHONY: ioemu-stubdom
 ioemu-stubdom: APP_OBJS=$(CURDIR)/ioemu/i386-stubdom/qemu.a $(CURDIR)/ioemu/i386-stubdom/libqemu.a $(CURDIR)/ioemu/libqemu_common.a
 ioemu-stubdom: mini-os-$(XEN_TARGET_ARCH)-ioemu lwip-$(XEN_TARGET_ARCH) libxenguest ioemu
-	DEF_CPPFLAGS="$(TARGET_CPPFLAGS)" DEF_CFLAGS="$(TARGET_CFLAGS)" DEF_LDFLAGS="$(TARGET_LDFLAGS)" MINIOS_CONFIG="$(CURDIR)/ioemu-minios.cfg" $(MAKE) DESTDIR= -C $(MINI_OS) OBJ_DIR=$(CURDIR)/$< LWIPDIR=$(CURDIR)/lwip-$(XEN_TARGET_ARCH) APP_OBJS="$(APP_OBJS)"
+	DEF_CPPFLAGS="$(TARGET_CPPFLAGS)" DEF_CFLAGS="$(TARGET_CFLAGS)" DEF_LDFLAGS="$(TARGET_LDFLAGS)" MINIOS_CONFIG="$(CURDIR)/ioemu-minios.out.cfg" $(MAKE) DESTDIR= -C $(MINI_OS) OBJ_DIR=$(CURDIR)/$< LWIPDIR=$(CURDIR)/lwip-$(XEN_TARGET_ARCH) APP_OBJS="$(APP_OBJS)"
 
 .PHONY: c-stubdom
 c-stubdom: mini-os-$(XEN_TARGET_ARCH)-c lwip-$(XEN_TARGET_ARCH) libxenguest c
@@ -488,7 +512,7 @@  vtpmmgr-stubdom: mini-os-$(XEN_TARGET_ARCH)-vtpmmgr vtpmmgr
 
 .PHONY: pv-grub
 pv-grub: mini-os-$(XEN_TARGET_ARCH)-grub libxenguest grub
-	DEF_CPPFLAGS="$(TARGET_CPPFLAGS)" DEF_CFLAGS="$(TARGET_CFLAGS)" DEF_LDFLAGS="$(TARGET_LDFLAGS)" MINIOS_CONFIG="$(CURDIR)/grub/minios.cfg" $(MAKE) DESTDIR= -C $(MINI_OS) OBJ_DIR=$(CURDIR)/$< APP_OBJS=$(CURDIR)/grub-$(XEN_TARGET_ARCH)/main.a
+	DEF_CPPFLAGS="$(TARGET_CPPFLAGS)" DEF_CFLAGS="$(TARGET_CFLAGS)" DEF_LDFLAGS="$(TARGET_LDFLAGS)" MINIOS_CONFIG="$(CURDIR)/grub/minios.out.cfg" $(MAKE) DESTDIR= -C $(MINI_OS) OBJ_DIR=$(CURDIR)/$< APP_OBJS=$(CURDIR)/grub-$(XEN_TARGET_ARCH)/main.a
 
 .PHONY: pv-grub-if-enabled
 ifneq ($(filter grub,$(STUBDOM_TARGETS)),)
@@ -499,11 +523,11 @@  endif
 
 .PHONY: xenstore-stubdom
 xenstore-stubdom: mini-os-$(XEN_TARGET_ARCH)-xenstore libxenguest xenstore
-	DEF_CPPFLAGS="$(TARGET_CPPFLAGS)" DEF_CFLAGS="$(TARGET_CFLAGS)" DEF_LDFLAGS="$(TARGET_LDFLAGS)" MINIOS_CONFIG="$(CURDIR)/xenstore-minios.cfg" $(MAKE) DESTDIR= -C $(MINI_OS) OBJ_DIR=$(CURDIR)/$< APP_OBJS=$(CURDIR)/xenstore/xenstored.a
+	DEF_CPPFLAGS="$(TARGET_CPPFLAGS)" DEF_CFLAGS="$(TARGET_CFLAGS)" DEF_LDFLAGS="$(TARGET_LDFLAGS)" MINIOS_CONFIG="$(CURDIR)/xenstore-minios.out.cfg" $(MAKE) DESTDIR= -C $(MINI_OS) OBJ_DIR=$(CURDIR)/$< APP_OBJS=$(CURDIR)/xenstore/xenstored.a
 
 .PHONY: xenstorepvh-stubdom
 xenstorepvh-stubdom: mini-os-$(XEN_TARGET_ARCH)-xenstorepvh libxenguest xenstorepvh
-	DEF_CPPFLAGS="$(TARGET_CPPFLAGS)" DEF_CFLAGS="$(TARGET_CFLAGS)" DEF_LDFLAGS="$(TARGET_LDFLAGS)" MINIOS_CONFIG="$(CURDIR)/xenstorepvh-minios.cfg" $(MAKE) DESTDIR= -C $(MINI_OS) OBJ_DIR=$(CURDIR)/$< APP_OBJS=$(CURDIR)/xenstorepvh/xenstored.a
+	DEF_CPPFLAGS="$(TARGET_CPPFLAGS)" DEF_CFLAGS="$(TARGET_CFLAGS)" DEF_LDFLAGS="$(TARGET_LDFLAGS)" MINIOS_CONFIG="$(CURDIR)/xenstorepvh-minios.out.cfg" $(MAKE) DESTDIR= -C $(MINI_OS) OBJ_DIR=$(CURDIR)/$< APP_OBJS=$(CURDIR)/xenstorepvh/xenstored.a
 
 #########
 # install
@@ -605,6 +629,7 @@  clean:
 	rm -fr grub-$(XEN_TARGET_ARCH)
 	rm -f $(STUBDOMPATH)
 	rm -f *-minios-config.mk
+	rm -f *.out.cfg
 	rm -fr pkg-config
 	-[ ! -d ioemu ] || $(MAKE) DESTDIR= -C ioemu clean
 	-[ ! -d xenstore ] || $(MAKE) -f $(CURDIR)/xenlibs.mk -C xenstore clean