[Buildroot] [PATCH v2 1/6] boot/grub2: add support to build multiple Grub2 configurations in the same build

Yann E. MORIN yann.morin.1998 at free.fr
Tue Sep 21 16:37:27 UTC 2021


Köry, All,

On 2021-09-21 15:28 +0200, Kory Maincent spake thusly:
> When Grub2 is build it is configured only for one boot set-up, BIOS Legacy,
> EFI 32 bit or EFI 64 bit. It can not deal with several boot set-up on the
> same image.
> 
> This patch allows to build Grub2 for different configurations simultaneously.
> To cover Grub2 configuration of legacy BIOS platforms (32-bit), 32-bit EFI
> BIOS and 64-bit EFI BIOS in the same build, multi-build system felt much more
> reasonable to just extend the grub2 package into 3 packages.
> 
> We can no longer use autotools-package as a consequence of this multi-build, and
> we have to resort to generic-package and a partial duplication of
> the autotools-infra. Grub2 was already using custom option like --prefix or
> --exec-prefix so this won't add much more weirdness.
> 
> We use a GRUB2_TUPLES list to describe all the configurations selected.
> For each boot case described in the GRUB2_TUPLES list, it configures and
> builds Grub2 in a separate folder named build-$(tuple).
> We use a foreach loop to make actions on each tuple selected.
> 
> We have to separate the BR2_TARGET_GRUB2_BUILTIN_MODULES and the
> BR2_TARGET_GRUB2_BUILTIN_CONFIG for each BIOS or EFI boot cases.
> 
> Signed-off-by: Kory Maincent <kory.maincent at bootlin.com>
> ---
[--SNIP--]
> diff --git a/boot/grub2/Config.in b/boot/grub2/Config.in
> index e45133999e..10bba6c5e4 100644
> --- a/boot/grub2/Config.in
> +++ b/boot/grub2/Config.in
> @@ -10,6 +10,13 @@ config BR2_TARGET_GRUB2
[--SNIP--]

Except for very minor formatting issues (mostly, jsut because it hurts
my eyes...), Config.in looks OK now, thanks.

> diff --git a/boot/grub2/grub2.mk b/boot/grub2/grub2.mk
> index 52e9199ae9..a70b8ea1ed 100644
> --- a/boot/grub2/grub2.mk
> +++ b/boot/grub2/grub2.mk
> @@ -57,52 +57,65 @@ GRUB2_INSTALL_TARGET = NO
>  endif
>  GRUB2_CPE_ID_VENDOR = gnu
>  
> -GRUB2_BUILTIN_MODULES = $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_MODULES))
> -GRUB2_BUILTIN_CONFIG = $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_CONFIG))
> +GRUB2_BUILTIN_MODULES_PC = $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_MODULES_PC))
> +GRUB2_BUILTIN_MODULES_EFI = $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI))
> +GRUB2_BUILTIN_CONFIG_PC = $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_CONFIG_PC))
> +GRUB2_BUILTIN_CONFIG_EFI = $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_CONFIG_EFI))
>  GRUB2_BOOT_PARTITION = $(call qstrip,$(BR2_TARGET_GRUB2_BOOT_PARTITION))
>  
>  ifeq ($(BR2_TARGET_GRUB2_I386_PC),y)
[--SNIP removals--]
> +GRUB2_IMAGE_i386-pc = $(BINARIES_DIR)/grub.img
> +GRUB2_CFG_i386-pc = $(TARGET_DIR)/boot/grub/grub.cfg
> +GRUB2_PREFIX_i386-pc = ($(GRUB2_BOOT_PARTITION))/boot/grub
> +GRUB2_TARGET_i386-pc = i386
> +GRUB2_PLATFORM_i386-pc = pc
> +GRUB2_BUILTIN_i386-pc = PC
> +GRUB2_TUPLES += i386-pc
> +endif

Any reason why you have decided not to got with the construct I
suggested:
    GRUB2_TUPLES-$(BR2_TARGET_GRUB2_I386_PC) += i386-pc

and then use $(GRUB2_TUPLES-y) when iterating?

Note: this is used everywhere in the kernel tree, and we already use
that construct in quite a few places, so it would not be totally new
and not totally unkown either:

    $ git grep -E -- '[_-]\$\([^)]+\)[[:space:]]*\+=' '*.mk'

But OK, the multi-conditions work as good if you don't like the -y
stuff.

> +ifeq ($(BR2_TARGET_GRUB2_I386_EFI),y)
> +GRUB2_IMAGE_i386-efi = $(BINARIES_DIR)/efi-part/EFI/BOOT/bootia32.efi
> +GRUB2_CFG_i386-efi = $(BINARIES_DIR)/efi-part/EFI/BOOT/grub.cfg
> +GRUB2_PREFIX_i386-efi = /EFI/BOOT
> +GRUB2_TARGET_i386-efi = i386
> +GRUB2_PLATFORM_i386-efi = efi
> +GRUB2_BUILTIN_i386-efi = EFI

All those GRUB2_BUILTIN_$(tuple) variables are just so that you can do a
double indeirection alter on, which is highly unreadable.

What about assigning the options directly:

    GRUB2_CONFIG_i386-pc = $(GRUB2_BUILTIN_CONFIG_PC)
    GRUB2_MODULES_i386-pc = $(GRUB2_BUILTIN_MODULES_PC)
    GRUB2_CONFIG_i386-efi = $(GRUB2_BUILTIN_CONFIG_EFI)
    GRUB2_MODULES_i386-efi = $(GRUB2_BUILTIN_MODULES_EFI)

[...] and so on [...]

[--SNIP--]
> +ifeq ($(BR2_TARGET_GRUB2_ARM_UBOOT),y)
> +GRUB2_IMAGE_arm-uboot = $(BINARIES_DIR)/boot-part/grub/grub.img
> +GRUB2_CFG_arm-uboot = $(BINARIES_DIR)/boot-part/grub/grub.cfg
> +GRUB2_PREFIX_arm-uboot = ($(GRUB2_BOOT_PARTITION))/boot/grub
> +GRUB2_TARGET_arm-uboot = arm
> +GRUB2_PLATFORM_arm-uboot = uboot
> +GRUB2_BUILTIN_arm-uboot = PC

[...] even for this sole outlier:

    GRUB2_CONFIG_arm-uboot = $(GRUB2_BUILTIN_CONFIG_PC)
    GRUB2_MODULES_arm-uboot = $(GRUB2_BUILTIN_MODULES_PC)

(and where we see that the _PC suffix is not necessarily appropriate.
but naming is really hard...)

[--SNIP--]
> @@ -128,8 +141,8 @@ GRUB2_CONF_ENV = \
>  	TARGET_STRIP="$(TARGET_CROSS)strip"
>  
>  GRUB2_CONF_OPTS = \
> -	--target=$(GRUB2_TARGET) \
> -	--with-platform=$(GRUB2_PLATFORM) \
> +	--host=$(GNU_TARGET_NAME) \
> +	--build=$(GNU_HOST_NAME) \
>  	--prefix=/ \
>  	--exec-prefix=/ \
>  	--disable-grub-mkfont \
> @@ -147,34 +160,48 @@ HOST_GRUB2_CONF_OPTS = \
>  	--enable-libzfs=no \
>  	--disable-werror

As Thomas previously suggested, I think we can just drop GRUB2_CONF_OPTS
and just put the options, even shared , directly in the loop [...]

> -ifeq ($(BR2_TARGET_GRUB2_I386_PC),y)
> -define GRUB2_IMAGE_INSTALL_ELTORITO
> -	cat $(HOST_DIR)/lib/grub/$(GRUB2_TUPLE)/cdboot.img $(GRUB2_IMAGE) > \
> -		$(BINARIES_DIR)/grub-eltorito.img
> +define GRUB2_CONFIGURE_CMDS
> +	$(foreach tuple, $(GRUB2_TUPLES), \
> +		mkdir -p $(@D)/build-$(tuple) ; \
> +		cd $(@D)/build-$(tuple) ; \
> +		$(TARGET_CONFIGURE_OPTS) \
> +		$(TARGET_CONFIGURE_ARGS) \
> +		$(GRUB2_CONF_ENV) \
> +		../configure \
> +			--target=$(GRUB2_TARGET_$(tuple)) \
> +			--with-platform=$(GRUB2_PLATFORM_$(tuple)) \
> +			$(GRUB2_CONF_OPTS)

[...] here.

[--SNIP--]
> +define GRUB2_BUILD_CMDS
> +	$(foreach tuple, $(GRUB2_TUPLES), \
> +		$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/build-$(tuple)
> +	)
>  endef
>  
> -ifeq ($(GRUB2_PLATFORM),efi)
> -define GRUB2_EFI_STARTUP_NSH
> -	echo $(notdir $(GRUB2_IMAGE)) > \
> -		$(BINARIES_DIR)/efi-part/startup.nsh
> +define GRUB2_INSTALL_IMAGES_CMDS
> +	$(foreach tuple, $(GRUB2_TUPLES), \
> +		mkdir -p $(dir $(GRUB2_IMAGE_$(tuple))) ; \
> +		$(HOST_DIR)/usr/bin/grub-mkimage \
> +			-d $(@D)/build-$(tuple)/grub-core/ \
> +			-O $(tuple) \
> +			-o $(GRUB2_IMAGE_$(tuple)) \
> +			-p "$(GRUB2_PREFIX_$(tuple))" \
> +			$(if $(GRUB2_BUILTIN_CONFIG_$(GRUB2_BUILTIN_$(tuple))), \
> +				-c $(GRUB2_BUILTIN_CONFIG_$(GRUB2_BUILTIN_$(tuple)))) \
> +			$(GRUB2_BUILTIN_MODULES_$(GRUB2_BUILTIN_$(tuple))) ; \

Those double indirections are really tricky to read, and I think we can
pretty easily get rid of them (see above).

> +		$(INSTALL) -D -m 0644 boot/grub2/grub.cfg $(GRUB2_CFG_$(tuple)) ; \
> +		$(if $(findstring $(GRUB2_PLATFORM_$(tuple)), pc), \
> +			cat $(HOST_DIR)/lib/grub/$(tuple)/cdboot.img $(GRUB2_IMAGE_$(tuple)) > \
> +				$(BINARIES_DIR)/grub-eltorito.img \
> +		) \
> +		$(if $(findstring $(GRUB2_PLATFORM_$(tuple)), efi), \
> +			echo $(notdir $(GRUB2_IMAGE_$(tuple))) > \
> +				$(BINARIES_DIR)/efi-part/startup_$(GRUB2_TARGET_$(tuple)).nsh \

But the previous path 'efi-part/startup.nsh' is where the post-build
scripts and genimage config files look for:

    $ git grep -E '\.nsh'

So, by changing the place where the .nsh is stored, you are breaking
those boards... Unless I missed something?

And I am not sure how to fix that, in the end... Especially the systemd
case (meh).

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'


More information about the buildroot mailing list