| Message ID | 20210921132829.56405-2-kory.maincent@bootlin.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for ISO9660 image compatible with Legacy and EFI BIOS | expand |
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@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.
On 21/09/2021 18:37, Yann E. MORIN wrote: > Köry, All, > > On 2021-09-21 15:28 +0200, Kory Maincent spake thusly: [snip] >> 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? Well, it's a bigger diff compared to what you have now. Especially because you have to keep the GRUB2_IMAGE_i386-pc etc. part, it's only TUPLES that you can use like that. Also, I think the -y approach is good only when you have may options (because it is slightly more difficult to parse) - we have 5 options here, which is a bit borderline. However, this approach becomes a lot more attractive if we eliminate the conditions: 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-$(BR2_TARGET_GRUB2_I386_PC) += i386-pc 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 GRUB2_TUPLES-$(BR2_TARGET_GRUB2_I386_EFI) += i386-efi > > 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) Still feels over-complicated, but I can't think of anything better. But it's definitely an improvement over the BUILTIN_$(tuple). Regards, Arnout [snip]
Arnout, All, On 2021-09-21 19:20 +0200, Arnout Vandecappelle spake thusly: > On 21/09/2021 18:37, Yann E. MORIN wrote: > >On 2021-09-21 15:28 +0200, Kory Maincent spake thusly: > [snip] > >> 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? > > Well, it's a bigger diff compared to what you have now. Especially because > you have to keep the GRUB2_IMAGE_i386-pc etc. part, it's only TUPLES that > you can use like that. > > Also, I think the -y approach is good only when you have may options > (because it is slightly more difficult to parse) - we have 5 options here, > which is a bit borderline. > > However, this approach becomes a lot more attractive if we eliminate the > conditions: Of course! I should have stated that explicitly, but in my mind it was obvious that the conditions would disapear, and that'd we'd keep only the -y assignments. > 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-$(BR2_TARGET_GRUB2_I386_PC) += i386-pc > > 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 > GRUB2_TUPLES-$(BR2_TARGET_GRUB2_I386_EFI) += i386-efi Exactly! > >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) > > Still feels over-complicated, but I can't think of anything better. But > it's definitely an improvement over the BUILTIN_$(tuple). Agreed, it's not a panacea, but it's still more legible. Regards, Yann E. MORIN.
Hello Yann, Thanks for the two reviews. On Tue, 21 Sep 2021 18:37:27 +0200 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > > > -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. Oh sorry, I have forgotten that part. It seems better indeed, I will do that in v3. > > > +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...) True, I did not thought of that solution. > > [--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 [...] His suggestion was to only keep out of GRUB2_CONF_OPTS the two parameterized options "--target" and "--with-platform". If you prefer to drop all the options from GRUB2_CONF_OPTS, I can do that. > > > + $(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? Yes, indeed it will break few post-build script. By default the EFI will look at efi/boot/bootx64.efi or efi/boot/bootia32.efi if the startup.nsh file is not present. So board should boot without that file. > > And I am not sure how to fix that, in the end... Especially the systemd > case (meh). Me neither. Maybe we can let it like v1, and the 64-bit EFI configuration will be the one used if both 32-bit EFI and 64-bit EFI are selected. Regards, Köry
Köry, All, On 2021-09-21 19:49 +0200, Köry Maincent spake thusly: > On Tue, 21 Sep 2021 18:37:27 +0200 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: [--SNIP--] > > > 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 [...] > His suggestion was to only keep out of GRUB2_CONF_OPTS the two parameterized > options "--target" and "--with-platform". If you prefer to drop all the > options from GRUB2_CONF_OPTS, I can do that. Ah, right, I misread what he wrote. But once we move some options out of GRUB2_CONF_OPTS, and since it is used in a single place, does it make sense to keep it now that we no longer are an autotools-package? > > > + $(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? > > Yes, indeed it will break few post-build script. Then those that are broken must be fixed. > By default the EFI will look at efi/boot/bootx64.efi or efi/boot/bootia32.efi > if the startup.nsh file is not present. So board should boot without that file. But then, is startup_{i386,x86_64,arm,aarch64}.nsh even consulted? As I understand it, and without access to the UEFI specifications, it looks like startup.nsh is the only file to be read by a (compliant?) UEFI bios. > > And I am not sure how to fix that, in the end... Especially the systemd > > case (meh). > Me neither. Maybe we can let it like v1, and the 64-bit EFI configuration will > be the one used if both 32-bit EFI and 64-bit EFI are selected. Well, the ultimate goal is to haev a single USB-stick or CDROM that can boot on all three systems: - legacy BIOS - 32-bit UEFI - 64-bit UEFI So, if the startup.nsh script is to be provided, it should cover the two UEFI cases. A 32-bit UEFI bios can't load a 64-bit payload, and conversely, a 65-bit UEFI bios can't load a 32-bit payload. So, if startup.nsh is mandatory, we have an impossible situation... So, I wonder if startup.nsh is even needed at all, and if we should not just drop it altogether, and just rely, s you explained earlier, on the actual naming of the payload: efi/boot/bootia32.efi on 32-bit UEFI, or efi/boot/bootx64.efi on 64-bit UEFI. And drop the creation of startup.nsh in gummiboot and systemd. And fixup the post-build scripts accordingly... But as I said earlier: I do not have access to the UEFI specifications (they are free-as-in-beer to download and read, not implement or use, so by downloading them, I would have to stop reviewing this series as that would count as a use of the specs...) Regards, Yann E. MORIN.
Hello Yann, On Tue, 21 Sep 2021 21:41:05 +0200 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > > > 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 [...] > > His suggestion was to only keep out of GRUB2_CONF_OPTS the two parameterized > > options "--target" and "--with-platform". If you prefer to drop all the > > options from GRUB2_CONF_OPTS, I can do that. > > Ah, right, I misread what he wrote. But once we move some options out of > GRUB2_CONF_OPTS, and since it is used in a single place, does it make > sense to keep it now that we no longer are an autotools-package? Alright > > > And I am not sure how to fix that, in the end... Especially the systemd > > > case (meh). > > Me neither. Maybe we can let it like v1, and the 64-bit EFI configuration > > will be the one used if both 32-bit EFI and 64-bit EFI are selected. > > Well, the ultimate goal is to haev a single USB-stick or CDROM that can > boot on all three systems: > - legacy BIOS > - 32-bit UEFI > - 64-bit UEFI > > So, if the startup.nsh script is to be provided, it should cover the two > UEFI cases. A 32-bit UEFI bios can't load a 64-bit payload, and > conversely, a 65-bit UEFI bios can't load a 32-bit payload. > > So, if startup.nsh is mandatory, we have an impossible situation... > > So, I wonder if startup.nsh is even needed at all, and if we should not > just drop it altogether, and just rely, s you explained earlier, on the > actual naming of the payload: efi/boot/bootia32.efi on 32-bit UEFI, or > efi/boot/bootx64.efi on 64-bit UEFI. > > And drop the creation of startup.nsh in gummiboot and systemd. > > And fixup the post-build scripts accordingly... > > But as I said earlier: I do not have access to the UEFI specifications > (they are free-as-in-beer to download and read, not implement or use, > so by downloading them, I would have to stop reviewing this series as > that would count as a use of the specs...) If I ask someone to read it and confirm the boot files name, is it ok as I have not opened it? ;) Here is the file name convention that need to be install in efi/boot folder 32bit : bootia32.efi x64 : bootx64.efi aarch32 : bootarm.efi aarch64 : bootaa64.efi It seems dropping the creation of the startup.nsh is the best idea. Regards, Köry
diff --git a/Config.in.legacy b/Config.in.legacy index 35a11f4dc6..33cee0202b 100644 --- a/Config.in.legacy +++ b/Config.in.legacy @@ -168,6 +168,30 @@ config BR2_KERNEL_HEADERS_5_12 comment "Legacy options removed in 2021.08" +config BR2_TARGET_GRUB2_BUILTIN_MODULES + string "the grub2 builtin modules has been renamed" + help + This option has been split to separate the builtin modules + between BR2_TARGET_GRUB2_BUILTIN_MODULES_PC and + BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI. + +config BR2_TARGET_GRUB2_BUILTIN_MODULES_WRAP + bool + default y if BR2_TARGET_GRUB2_BUILTIN_MODULES != "" + select BR2_LEGACY + +config BR2_TARGET_GRUB2_BUILTIN_CONFIG + string "the grub2 builtin configuration has been renamed" + help + This option has been split to separate the builtin configuration + between BR2_TARGET_GRUB2_BUILTIN_CONFIG_PC and + BR2_TARGET_GRUB2_BUILTIN_CONFIG_EFI. + +config BR2_TARGET_GRUB2_BUILTIN_CONFIG_WRAP + bool + default y if BR2_TARGET_GRUB2_BUILTIN_CONFIG != "" + select BR2_LEGACY + config BR2_PACKAGE_LIBMCRYPT bool "libmcrypt package was removed" select BR2_LEGACY 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 bool "grub2" depends on BR2_TARGET_GRUB2_ARCH_SUPPORTS depends on BR2_USE_WCHAR + select BR2_TARGET_GRUB2_I386_PC if \ + !BR2_TARGET_GRUB2_HAS_PTF && \ + (BR2_i386 || BR2_x86_64) + select BR2_TARGET_GRUB2_ARM_UBOOT if \ + !BR2_TARGET_GRUB2_HAS_PTF && \ + BR2_arm + select BR2_TARGET_GRUB2_ARM64_EFI if BR2_aarch64 help GNU GRUB is a Multiboot boot loader. It was derived from GRUB, the GRand Unified Bootloader, which was originally @@ -25,10 +32,10 @@ config BR2_TARGET_GRUB2 http://www.gnu.org/software/grub/ -if BR2_TARGET_GRUB2 +config BR2_TARGET_GRUB2_HAS_PTF + bool -choice - prompt "Platform" +if BR2_TARGET_GRUB2 config BR2_TARGET_GRUB2_I386_PC bool "i386-pc" @@ -40,6 +47,7 @@ config BR2_TARGET_GRUB2_I386_PC config BR2_TARGET_GRUB2_I386_EFI bool "i386-efi" depends on BR2_i386 || BR2_x86_64 + select BR2_TARGET_GRUB2_HAS_PTF help Select this option if the platform you're targetting has a 32 bits EFI BIOS. Note that some x86-64 platforms use a 32 @@ -48,6 +56,7 @@ config BR2_TARGET_GRUB2_I386_EFI config BR2_TARGET_GRUB2_X86_64_EFI bool "x86-64-efi" depends on BR2_x86_64 + select BR2_TARGET_GRUB2_HAS_PTF help Select this option if the platform you're targetting has a 64 bits EFI BIOS. @@ -63,6 +72,7 @@ config BR2_TARGET_GRUB2_ARM_UBOOT config BR2_TARGET_GRUB2_ARM_EFI bool "arm-efi" depends on BR2_arm + select BR2_TARGET_GRUB2_HAS_PTF help Select this option if the platform you're targetting is an ARM platform and you want to boot Grub 2 as an EFI @@ -76,10 +86,10 @@ config BR2_TARGET_GRUB2_ARM64_EFI Aarch64 platform and you want to boot Grub 2 as an EFI application. -endchoice - if BR2_TARGET_GRUB2_I386_PC || BR2_TARGET_GRUB2_ARM_UBOOT +comment "Options for the x86 legacy BIOS or ARM U-Boot support" + config BR2_TARGET_GRUB2_BOOT_PARTITION string "boot partition" default "hd0,msdos1" @@ -89,24 +99,43 @@ config BR2_TARGET_GRUB2_BOOT_PARTITION first disk if using a legacy partition table, or 'hd0,gpt1' if using GPT partition table. -endif # BR2_TARGET_GRUB2_I386_PC || BR2_TARGET_GRUB2_ARM_UBOOT - -config BR2_TARGET_GRUB2_BUILTIN_MODULES +config BR2_TARGET_GRUB2_BUILTIN_MODULES_PC string "builtin modules" + default BR2_TARGET_GRUB2_BUILTIN_MODULES if BR2_TARGET_GRUB2_BUILTIN_MODULES != "" # legacy default "boot linux ext2 fat squash4 part_msdos part_gpt normal biosdisk" if BR2_TARGET_GRUB2_I386_PC - default "boot linux ext2 fat squash4 part_msdos part_gpt normal efi_gop" \ - if BR2_TARGET_GRUB2_I386_EFI || BR2_TARGET_GRUB2_X86_64_EFI || \ - BR2_TARGET_GRUB2_ARM_EFI || BR2_TARGET_GRUB2_ARM64_EFI default "linux ext2 fat part_msdos normal" if BR2_TARGET_GRUB2_ARM_UBOOT -config BR2_TARGET_GRUB2_BUILTIN_CONFIG +config BR2_TARGET_GRUB2_BUILTIN_CONFIG_PC + string "builtin config" + default BR2_TARGET_GRUB2_BUILTIN_CONFIG if BR2_TARGET_GRUB2_BUILTIN_CONFIG != "" # legacy + help + Path to a Grub 2 configuration file that will be embedded + into the Grub image itself. This allows to set the root + device and other configuration parameters, but however menu + entries cannot be described in this embedded configuration. + +endif # BR2_TARGET_GRUB2_I386_PC || BR2_TARGET_GRUB2_ARM_UBOOT + +if BR2_TARGET_GRUB2_I386_EFI || BR2_TARGET_GRUB2_X86_64_EFI || \ + BR2_TARGET_GRUB2_ARM_EFI || BR2_TARGET_GRUB2_ARM64_EFI + +comment "Options for the EFI BIOS or ARM EFI support" + +config BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI + string "builtin modules" + default BR2_TARGET_GRUB2_BUILTIN_MODULES if BR2_TARGET_GRUB2_BUILTIN_MODULES != "" # legacy + default "boot linux ext2 fat squash4 part_msdos part_gpt normal efi_gop" + +config BR2_TARGET_GRUB2_BUILTIN_CONFIG_EFI string "builtin config" + default BR2_TARGET_GRUB2_BUILTIN_CONFIG if BR2_TARGET_GRUB2_BUILTIN_CONFIG != "" # legacy help Path to a Grub 2 configuration file that will be embedded into the Grub image itself. This allows to set the root device and other configuration parameters, but however menu entries cannot be described in this embedded configuration. +endif config BR2_TARGET_GRUB2_INSTALL_TOOLS bool "install tools" help 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) -GRUB2_IMAGE = $(BINARIES_DIR)/grub.img -GRUB2_CFG = $(TARGET_DIR)/boot/grub/grub.cfg -GRUB2_PREFIX = ($(GRUB2_BOOT_PARTITION))/boot/grub -GRUB2_TUPLE = i386-pc -GRUB2_TARGET = i386 -GRUB2_PLATFORM = pc -else ifeq ($(BR2_TARGET_GRUB2_I386_EFI),y) -GRUB2_IMAGE = $(BINARIES_DIR)/efi-part/EFI/BOOT/bootia32.efi -GRUB2_CFG = $(BINARIES_DIR)/efi-part/EFI/BOOT/grub.cfg -GRUB2_PREFIX = /EFI/BOOT -GRUB2_TUPLE = i386-efi -GRUB2_TARGET = i386 -GRUB2_PLATFORM = efi -else ifeq ($(BR2_TARGET_GRUB2_X86_64_EFI),y) -GRUB2_IMAGE = $(BINARIES_DIR)/efi-part/EFI/BOOT/bootx64.efi -GRUB2_CFG = $(BINARIES_DIR)/efi-part/EFI/BOOT/grub.cfg -GRUB2_PREFIX = /EFI/BOOT -GRUB2_TUPLE = x86_64-efi -GRUB2_TARGET = x86_64 -GRUB2_PLATFORM = efi -else ifeq ($(BR2_TARGET_GRUB2_ARM_UBOOT),y) -GRUB2_IMAGE = $(BINARIES_DIR)/boot-part/grub/grub.img -GRUB2_CFG = $(BINARIES_DIR)/boot-part/grub/grub.cfg -GRUB2_PREFIX = ($(GRUB2_BOOT_PARTITION))/boot/grub -GRUB2_TUPLE = arm-uboot -GRUB2_TARGET = arm -GRUB2_PLATFORM = uboot -else ifeq ($(BR2_TARGET_GRUB2_ARM_EFI),y) -GRUB2_IMAGE = $(BINARIES_DIR)/efi-part/EFI/BOOT/bootarm.efi -GRUB2_CFG = $(BINARIES_DIR)/efi-part/EFI/BOOT/grub.cfg -GRUB2_PREFIX = /EFI/BOOT -GRUB2_TUPLE = arm-efi -GRUB2_TARGET = arm -GRUB2_PLATFORM = efi -else ifeq ($(BR2_TARGET_GRUB2_ARM64_EFI),y) -GRUB2_IMAGE = $(BINARIES_DIR)/efi-part/EFI/BOOT/bootaa64.efi -GRUB2_CFG = $(BINARIES_DIR)/efi-part/EFI/BOOT/grub.cfg -GRUB2_PREFIX = /EFI/BOOT -GRUB2_TUPLE = arm64-efi -GRUB2_TARGET = aarch64 -GRUB2_PLATFORM = efi +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 +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 +GRUB2_TUPLES += i386-efi +endif +ifeq ($(BR2_TARGET_GRUB2_X86_64_EFI),y) +GRUB2_IMAGE_x86_64-efi = $(BINARIES_DIR)/efi-part/EFI/BOOT/bootx64.efi +GRUB2_CFG_x86_64-efi = $(BINARIES_DIR)/efi-part/EFI/BOOT/grub.cfg +GRUB2_PREFIX_x86_64-efi = /EFI/BOOT +GRUB2_TARGET_x86_64-efi = x86_64 +GRUB2_PLATFORM_x86_64-efi = efi +GRUB2_BUILTIN_x86_64-efi = EFI +GRUB2_TUPLES += x86_64-efi +endif +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 +GRUB2_TUPLES += arm-uboot +endif +ifeq ($(BR2_TARGET_GRUB2_ARM_EFI),y) +GRUB2_IMAGE_arm-efi = $(BINARIES_DIR)/efi-part/EFI/BOOT/bootarm.efi +GRUB2_CFG_arm-efi = $(BINARIES_DIR)/efi-part/EFI/BOOT/grub.cfg +GRUB2_PREFIX_arm-efi = /EFI/BOOT +GRUB2_TARGET_arm-efi = arm +GRUB2_PLATFORM_arm-efi = efi +GRUB2_BUILTIN_arm-efi = EFI +GRUB2_TUPLES += arm-efi +endif +ifeq ($(BR2_TARGET_GRUB2_ARM64_EFI),y) +GRUB2_IMAGE_arm64-efi = $(BINARIES_DIR)/efi-part/EFI/BOOT/bootaa64.efi +GRUB2_CFG_arm64-efi = $(BINARIES_DIR)/efi-part/EFI/BOOT/grub.cfg +GRUB2_PREFIX_arm64-efi = /EFI/BOOT +GRUB2_TARGET_arm64-efi = aarch64 +GRUB2_PLATFORM_arm64-efi = efi +GRUB2_BUILTIN_arm64-efi = EFI +GRUB2_TUPLES += arm64-efi endif # Grub2 is kind of special: it considers CC, LD and so on to be the @@ -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 -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) + ) endef -endif -define GRUB2_INSTALL_IMAGES_CMDS - mkdir -p $(dir $(GRUB2_IMAGE)) - $(HOST_DIR)/usr/bin/grub-mkimage \ - -d $(@D)/grub-core/ \ - -O $(GRUB2_TUPLE) \ - -o $(GRUB2_IMAGE) \ - -p "$(GRUB2_PREFIX)" \ - $(if $(GRUB2_BUILTIN_CONFIG),-c $(GRUB2_BUILTIN_CONFIG)) \ - $(GRUB2_BUILTIN_MODULES) - mkdir -p $(dir $(GRUB2_CFG)) - $(INSTALL) -D -m 0644 boot/grub2/grub.cfg $(GRUB2_CFG) - $(GRUB2_IMAGE_INSTALL_ELTORITO) +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))) ; \ + $(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 \ + ) + ) endef -GRUB2_POST_INSTALL_IMAGES_HOOKS += GRUB2_EFI_STARTUP_NSH -endif -$(eval $(autotools-package)) +$(eval $(generic-package)) $(eval $(host-autotools-package)) diff --git a/support/testing/tests/fs/test_iso9660.py b/support/testing/tests/fs/test_iso9660.py index 68f4840852..1b699e1fef 100644 --- a/support/testing/tests/fs/test_iso9660.py +++ b/support/testing/tests/fs/test_iso9660.py @@ -54,7 +54,7 @@ class TestIso9660Grub2External(infra.basetest.BRTest): # BR2_TARGET_ROOTFS_ISO9660_INITRD is not set BR2_TARGET_GRUB2=y BR2_TARGET_GRUB2_BOOT_PARTITION="cd" - BR2_TARGET_GRUB2_BUILTIN_MODULES="boot linux ext2 fat part_msdos part_gpt normal biosdisk iso9660" + BR2_TARGET_GRUB2_BUILTIN_MODULES_PC="boot linux ext2 fat part_msdos part_gpt normal biosdisk iso9660" BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}" """.format(infra.filepath("conf/grub2.cfg")) @@ -75,7 +75,7 @@ class TestIso9660Grub2ExternalCompress(infra.basetest.BRTest): BR2_TARGET_ROOTFS_ISO9660_TRANSPARENT_COMPRESSION=y BR2_TARGET_GRUB2=y BR2_TARGET_GRUB2_BOOT_PARTITION="cd" - BR2_TARGET_GRUB2_BUILTIN_MODULES="boot linux ext2 fat part_msdos part_gpt normal biosdisk iso9660" + BR2_TARGET_GRUB2_BUILTIN_MODULES_PC="boot linux ext2 fat part_msdos part_gpt normal biosdisk iso9660" BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}" """.format(infra.filepath("conf/grub2.cfg")) @@ -95,7 +95,7 @@ class TestIso9660Grub2Internal(infra.basetest.BRTest): BR2_TARGET_ROOTFS_ISO9660_INITRD=y BR2_TARGET_GRUB2=y BR2_TARGET_GRUB2_BOOT_PARTITION="cd" - BR2_TARGET_GRUB2_BUILTIN_MODULES="boot linux ext2 fat part_msdos part_gpt normal biosdisk iso9660" + BR2_TARGET_GRUB2_BUILTIN_MODULES_PC="boot linux ext2 fat part_msdos part_gpt normal biosdisk iso9660" BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}" """.format(infra.filepath("conf/grub2.cfg"))
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@bootlin.com> --- Changes in v2: Following the review from Yann E. Morin: - Update the tuples configurations to makes the code more readable. - Move "--target" and "--with-platform" parameters into the GRUB2_CONFIGURE_CMD. - Fix startup.nsh installation. - Fix Grub2 configuration to be sure that at least one platform is always enabled. - Fix legacy builtin configurations to string in place of bool. Config.in.legacy | 24 ++++ boot/grub2/Config.in | 53 ++++++-- boot/grub2/grub2.mk | 165 +++++++++++++---------- support/testing/tests/fs/test_iso9660.py | 6 +- 4 files changed, 164 insertions(+), 84 deletions(-)