All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v2 0/6] Add support for ISO9660 image compatible with Legacy and EFI BIOS
@ 2021-09-21 13:28 Kory Maincent
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 1/6] boot/grub2: add support to build multiple Grub2 configurations in the same build Kory Maincent
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Kory Maincent @ 2021-09-21 13:28 UTC (permalink / raw)
  To: buildroot; +Cc: yann.morin.1998, thomas.petazzoni

This series of patches aims to support the generation of an ISO9660 hybrid
image compatible with Legacy BIOS and EFI BIOS. To implement this, we need
to improve the grub2 package and modify the ISO9660 image support

Grub2 was written to build only one configuration at a time. For the hybrid
image we need to have several configuration of Grub2 in the same image.
For example we might want to have Grub2 built for BIOS, EFI 32 bits and EFI
64 bits in the same image. To support this, we chose to fill a list of
configuration name tuples, and then build each Grub2 configuration in a
separate build-$(tuple) folder. It seems simpler than having multiple
duplicated grub2 packages for each possible configuration.

The generation of ISO9660 image was only supporting bootloaders based on
Legacy BIOS boot. We first change the ISO9660 image generation to use
xorriso instead of genimageiso, in order to be able to build an image
compatible with both legacy and EFI BIOS. Then we add the generation of an
EFI System Partition in iso9660 so that we can install the EFI-compatible
bootloader."

In detail:

 - PATCH 1 implements simultaneous build of GRUB for different configurations
 - PATCH 2 implements the generation of ISO9660 image booting on a EFI BIOS
 - PATCH 3 implements the generation of hybrid image compatible with Legacy
   and EFI BIOS
 - PATCH 4 updates the encoding of the text return from testing emulator
 - PATCH 5 add support to i386 architecture to edk2 package
 - PATCH 6 updates iso9660 tests and implements a test for EFI image and
   hybrid image.

Changes in v2:
- Update the typo of Grub2 configuration tuples to make more legible code
- Expand explanation in few commit messages.
- Fix typos.
- Fix Grub2 Legacy builtin configurations.
- Add mkfs and mcopy parameters in iso9660 package to build reproducible
  images.
- Remove the implementation of host-efi-bios package.
- Add support for i386 architecture to edk2 package.


Thanks in advance for your review and feedback

Kory Maincent (6):
  boot/grub2: add support to build multiple Grub2 configurations in the
    same build
  fs/iso9660: add support to Grub EFI bootloader in the image
  fs/iso9660: add support for hybrid image using Grub bootloader on BIOS
    and EFI
  support/testing/infra/emulator.py: update encoding when calling qemu
  boot/edk2: add support to i386 architecture
  support/testing/tests/fs/test_iso9660.py: add support to test using
    EFI BIOS

 Config.in.legacy                         |  24 ++++
 boot/edk2/Config.in                      |  12 +-
 boot/edk2/edk2.mk                        |  12 +-
 boot/grub2/Config.in                     |  53 ++++++--
 boot/grub2/grub2.mk                      | 165 +++++++++++++----------
 fs/iso9660/Config.in                     |  30 ++++-
 fs/iso9660/iso9660.mk                    |  60 ++++++++-
 support/testing/conf/grub2-efi.cfg       |   2 +
 support/testing/infra/emulator.py        |   2 +-
 support/testing/tests/fs/test_iso9660.py |  80 ++++++++++-
 10 files changed, 337 insertions(+), 103 deletions(-)
 create mode 100644 support/testing/conf/grub2-efi.cfg

-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 1/6] boot/grub2: add support to build multiple Grub2 configurations in the same build
  2021-09-21 13:28 [Buildroot] [PATCH v2 0/6] Add support for ISO9660 image compatible with Legacy and EFI BIOS Kory Maincent
@ 2021-09-21 13:28 ` Kory Maincent
  2021-09-21 16:37   ` Yann E. MORIN
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 2/6] fs/iso9660: add support to Grub EFI bootloader in the image Kory Maincent
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Kory Maincent @ 2021-09-21 13:28 UTC (permalink / raw)
  To: buildroot; +Cc: yann.morin.1998, thomas.petazzoni

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(-)

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"))
 
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 2/6] fs/iso9660: add support to Grub EFI bootloader in the image
  2021-09-21 13:28 [Buildroot] [PATCH v2 0/6] Add support for ISO9660 image compatible with Legacy and EFI BIOS Kory Maincent
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 1/6] boot/grub2: add support to build multiple Grub2 configurations in the same build Kory Maincent
@ 2021-09-21 13:28 ` Kory Maincent
  2021-09-21 15:36   ` Yann E. MORIN
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 3/6] fs/iso9660: add support for hybrid image using Grub bootloader on BIOS and EFI Kory Maincent
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Kory Maincent @ 2021-09-21 13:28 UTC (permalink / raw)
  To: buildroot; +Cc: yann.morin.1998, thomas.petazzoni

Add support to boot the Grub bootloader from an EFI BIOS in the ISO9660
image.
For that we need to create EFI System Partition (ESP). The ESP is a vfat
partition which contain the Grub2 binary at the /EFI/BOOT/ location.
xorriso command will generate the iso image including the ESP.

We notice Grub can not read and mount the ESP, therefore we place the Grub
configuration file in the ISO9660 partition. A Grub2 builtin configuration
need to be used to tell Grub2 to search automatically its configuration
file in the ISO9660 partition. Use 'set root=(cd0)' in the configuration
file passed to BR2_TARGET_GRUB2_BUILTIN_CONFIG_EFI.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Changes in v2:
Following the review from Yann E. Morin:
- Add parameter to mkfs and mcopy to allow reproducible builds
- Fix typos and variable multi-line assignement

 fs/iso9660/Config.in  | 30 ++++++++++++++++++++++++------
 fs/iso9660/iso9660.mk | 38 +++++++++++++++++++++++++++++++++++---
 2 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/fs/iso9660/Config.in b/fs/iso9660/Config.in
index 6f001c0640..4fc96db28c 100644
--- a/fs/iso9660/Config.in
+++ b/fs/iso9660/Config.in
@@ -3,6 +3,7 @@ config BR2_TARGET_ROOTFS_ISO9660
 	depends on (BR2_i386 || BR2_x86_64)
 	depends on BR2_LINUX_KERNEL
 	depends on BR2_TARGET_GRUB2_I386_PC || \
+		BR2_TARGET_GRUB2_I386_EFI || BR2_TARGET_GRUB2_X86_64_EFI || \
 		BR2_TARGET_SYSLINUX_ISOLINUX
 	select BR2_LINUX_KERNEL_INSTALL_TARGET \
 	       if (!BR2_TARGET_ROOTFS_ISO9660_INITRD && !BR2_TARGET_ROOTFS_INITRAMFS)
@@ -27,22 +28,38 @@ choice
 
 config BR2_TARGET_ROOTFS_ISO9660_GRUB2
 	bool "grub2"
-	depends on BR2_TARGET_GRUB2_I386_PC
+	depends on BR2_TARGET_GRUB2_I386_PC || BR2_TARGET_GRUB2_I386_EFI || \
+		BR2_TARGET_GRUB2_X86_64_EFI
+	select BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER \
+	       if BR2_TARGET_GRUB2_I386_PC
+	select BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER \
+	       if (BR2_TARGET_GRUB2_I386_EFI || BR2_TARGET_GRUB2_X86_64_EFI)
 	help
 	  Use Grub 2 as the bootloader for the ISO9660 image. Make
 	  sure to enable the 'iso9660' module in
-	  BR2_TARGET_GRUB2_BUILTIN_MODULES and to use 'cd' as the boot
-	  partition in BR2_TARGET_GRUB2_BOOT_PARTITION=.
+	  BR2_TARGET_GRUB2_BUILTIN_MODULES_PC or
+	  BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI. Use 'cd' as the boot
+	  partition in BR2_TARGET_GRUB2_BOOT_PARTITION= for GRUB on BIOS
+	  or 'set root=(cd0)' in the configuration file passed to
+	  BR2_TARGET_GRUB2_BUILTIN_CONFIG_EFI for GRUB on EFI.
 
 config BR2_TARGET_ROOTFS_ISO9660_ISOLINUX
 	bool "isolinux"
 	depends on BR2_TARGET_SYSLINUX_ISOLINUX
+	select BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER
 
 endchoice
 
+config BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER
+	bool
+
+config BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER
+	bool
+
 config BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU
 	string "Boot menu config file"
-	default "fs/iso9660/grub.cfg" if BR2_TARGET_ROOTFS_ISO9660_GRUB2
+	default "fs/iso9660/grub.cfg" if BR2_TARGET_ROOTFS_ISO9660_GRUB2_PC || \
+					BR2_TARGET_ROOTFS_ISO9660_GRUB2_EFI
 	default "fs/iso9660/isolinux.cfg" if BR2_TARGET_ROOTFS_ISO9660_ISOLINUX
 	help
 	  Use this option to provide a custom bootloader configuration
@@ -83,7 +100,8 @@ config BR2_TARGET_ROOTFS_ISO9660_HYBRID
 
 endif
 
-comment "iso image needs a Linux kernel and either grub2 i386-pc or isolinux to be built"
+comment "iso image needs a Linux kernel and either grub2 or isolinux to be built"
 	depends on BR2_i386 || BR2_x86_64
 	depends on !BR2_LINUX_KERNEL || \
-		!(BR2_TARGET_GRUB2_I386_PC || BR2_TARGET_SYSLINUX_ISOLINUX)
+		!(BR2_TARGET_GRUB2_I386_PC || BR2_TARGET_GRUB2_I386_EFI || \
+		BR2_TARGET_GRUB2_X86_64_EFI || BR2_TARGET_SYSLINUX_ISOLINUX)
diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
index 23421a9a5c..d46aec002b 100644
--- a/fs/iso9660/iso9660.mk
+++ b/fs/iso9660/iso9660.mk
@@ -57,7 +57,11 @@ else
 ROOTFS_ISO9660_TMP_TARGET_DIR = $(TARGET_DIR)
 endif
 
-ifeq ($(BR2_TARGET_ROOTFS_ISO9660_GRUB2),y)
+ifeq ($(BR2_REPRODUCIBLE),y)
+ROOTFS_ISO9660_MKFS_OPTS = --invariant
+endif
+
+ifeq ($(BR2_TARGET_ROOTFS_ISO9660_GRUB2)$(BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER),yy)
 ROOTFS_ISO9660_DEPENDENCIES += grub2
 ROOTFS_ISO9660_BOOTLOADER_CONFIG_PATH = \
 	$(ROOTFS_ISO9660_TMP_TARGET_DIR)/boot/grub/grub.cfg
@@ -66,6 +70,21 @@ define ROOTFS_ISO9660_INSTALL_BOOTLOADER
 	$(INSTALL) -D -m 0644 $(BINARIES_DIR)/grub-eltorito.img \
 		$(ROOTFS_ISO9660_TMP_TARGET_DIR)/boot/grub/grub-eltorito.img
 endef
+else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_GRUB2)$(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),yy)
+ROOTFS_ISO9660_DEPENDENCIES += grub2 host-dosfstools host-mtools
+ROOTFS_ISO9660_EFI_PARTITION = boot/fat.efi
+ROOTFS_ISO9660_EFI_PARTITION_PATH = $(ROOTFS_ISO9660_TMP_TARGET_DIR)/$(ROOTFS_ISO9660_EFI_PARTITION)
+ROOTFS_ISO9660_EFI_PARTITION_CONTENT = $(BINARIES_DIR)/efi-part
+ROOTFS_ISO9660_BOOTLOADER_CONFIG_PATH = \
+	$(ROOTFS_ISO9660_TMP_TARGET_DIR)/boot/grub/grub.cfg
+define ROOTFS_ISO9660_INSTALL_BOOTLOADER
+	rm -rf $(ROOTFS_ISO9660_EFI_PARTITION_PATH)
+	mkdir -p $(dir $(ROOTFS_ISO9660_EFI_PARTITION_PATH))
+	dd if=/dev/zero of=$(ROOTFS_ISO9660_EFI_PARTITION_PATH) bs=1M count=1
+	$(HOST_DIR)/sbin/mkfs.vfat $(ROOTFS_ISO9660_MKFS_OPTS) $(ROOTFS_ISO9660_EFI_PARTITION_PATH)
+	$(HOST_DIR)/bin/mcopy -p -m -i $(ROOTFS_ISO9660_EFI_PARTITION_PATH) -s \
+		$(ROOTFS_ISO9660_EFI_PARTITION_CONTENT)/* ::/
+endef
 else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_ISOLINUX),y)
 ROOTFS_ISO9660_DEPENDENCIES += syslinux
 ROOTFS_ISO9660_BOOTLOADER_CONFIG_PATH = \
@@ -128,9 +147,22 @@ ROOTFS_ISO9660_PRE_GEN_HOOKS += ROOTFS_ISO9660_DISABLE_EXTERNAL_INITRD
 
 endif # ROOTFS_ISO9660_USE_INITRD
 
+ifeq ($(BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER),y)
+ROOTFS_ISO9660_OPTS += \
+	-J \
+	-R \
+	-boot-load-size 4 \
+	-boot-info-table \
+	-no-emul-boot \
+	-b $(ROOTFS_ISO9660_BOOT_IMAGE)
+else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),y)
+ROOTFS_ISO9660_OPTS += \
+	--efi-boot $(ROOTFS_ISO9660_EFI_PARTITION) \
+	-no-emul-boot
+endif
+
 define ROOTFS_ISO9660_CMD
-	$(HOST_DIR)/bin/xorriso -as mkisofs -J -R -b $(ROOTFS_ISO9660_BOOT_IMAGE) \
-		-no-emul-boot -boot-load-size 4 -boot-info-table \
+	$(HOST_DIR)/bin/xorriso -as mkisofs \
 		$(ROOTFS_ISO9660_OPTS) \
 		-o $@ $(ROOTFS_ISO9660_TMP_TARGET_DIR)
 endef
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 3/6] fs/iso9660: add support for hybrid image using Grub bootloader on BIOS and EFI
  2021-09-21 13:28 [Buildroot] [PATCH v2 0/6] Add support for ISO9660 image compatible with Legacy and EFI BIOS Kory Maincent
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 1/6] boot/grub2: add support to build multiple Grub2 configurations in the same build Kory Maincent
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 2/6] fs/iso9660: add support to Grub EFI bootloader in the image Kory Maincent
@ 2021-09-21 13:28 ` Kory Maincent
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 4/6] support/testing/infra/emulator.py: update encoding when calling qemu Kory Maincent
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Kory Maincent @ 2021-09-21 13:28 UTC (permalink / raw)
  To: buildroot; +Cc: yann.morin.1998, thomas.petazzoni

Add support for building an hybrid ISO9660 image compatible with legacy
and UEFI BIOS.
The option -eltorito-alt-boot need to be used in the xorriso command
to generate the hybrid image.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Changes in v2:
- Fix typos

 fs/iso9660/iso9660.mk | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
index d46aec002b..499d19c9bb 100644
--- a/fs/iso9660/iso9660.mk
+++ b/fs/iso9660/iso9660.mk
@@ -66,18 +66,20 @@ ROOTFS_ISO9660_DEPENDENCIES += grub2
 ROOTFS_ISO9660_BOOTLOADER_CONFIG_PATH = \
 	$(ROOTFS_ISO9660_TMP_TARGET_DIR)/boot/grub/grub.cfg
 ROOTFS_ISO9660_BOOT_IMAGE = boot/grub/grub-eltorito.img
-define ROOTFS_ISO9660_INSTALL_BOOTLOADER
+define ROOTFS_ISO9660_INSTALL_BOOTLOADER_BIOS
 	$(INSTALL) -D -m 0644 $(BINARIES_DIR)/grub-eltorito.img \
 		$(ROOTFS_ISO9660_TMP_TARGET_DIR)/boot/grub/grub-eltorito.img
 endef
-else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_GRUB2)$(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),yy)
+endif
+
+ifeq ($(BR2_TARGET_ROOTFS_ISO9660_GRUB2)$(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),yy)
 ROOTFS_ISO9660_DEPENDENCIES += grub2 host-dosfstools host-mtools
 ROOTFS_ISO9660_EFI_PARTITION = boot/fat.efi
 ROOTFS_ISO9660_EFI_PARTITION_PATH = $(ROOTFS_ISO9660_TMP_TARGET_DIR)/$(ROOTFS_ISO9660_EFI_PARTITION)
 ROOTFS_ISO9660_EFI_PARTITION_CONTENT = $(BINARIES_DIR)/efi-part
 ROOTFS_ISO9660_BOOTLOADER_CONFIG_PATH = \
 	$(ROOTFS_ISO9660_TMP_TARGET_DIR)/boot/grub/grub.cfg
-define ROOTFS_ISO9660_INSTALL_BOOTLOADER
+define ROOTFS_ISO9660_INSTALL_BOOTLOADER_EFI
 	rm -rf $(ROOTFS_ISO9660_EFI_PARTITION_PATH)
 	mkdir -p $(dir $(ROOTFS_ISO9660_EFI_PARTITION_PATH))
 	dd if=/dev/zero of=$(ROOTFS_ISO9660_EFI_PARTITION_PATH) bs=1M count=1
@@ -85,12 +87,14 @@ define ROOTFS_ISO9660_INSTALL_BOOTLOADER
 	$(HOST_DIR)/bin/mcopy -p -m -i $(ROOTFS_ISO9660_EFI_PARTITION_PATH) -s \
 		$(ROOTFS_ISO9660_EFI_PARTITION_CONTENT)/* ::/
 endef
-else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_ISOLINUX),y)
+endif
+
+ifeq ($(BR2_TARGET_ROOTFS_ISO9660_ISOLINUX),y)
 ROOTFS_ISO9660_DEPENDENCIES += syslinux
 ROOTFS_ISO9660_BOOTLOADER_CONFIG_PATH = \
 	$(ROOTFS_ISO9660_TMP_TARGET_DIR)/isolinux/isolinux.cfg
 ROOTFS_ISO9660_BOOT_IMAGE = isolinux/isolinux.bin
-define ROOTFS_ISO9660_INSTALL_BOOTLOADER
+define ROOTFS_ISO9660_INSTALL_BOOTLOADER_BIOS
 	$(INSTALL) -D -m 0644 $(BINARIES_DIR)/syslinux/* \
 		$(ROOTFS_ISO9660_TMP_TARGET_DIR)/isolinux/
 	$(INSTALL) -D -m 0644 $(HOST_DIR)/share/syslinux/ldlinux.c32 \
@@ -103,7 +107,8 @@ define ROOTFS_ISO9660_PREPARATION
 		$(ROOTFS_ISO9660_BOOTLOADER_CONFIG_PATH)
 	$(SED) "s%__KERNEL_PATH__%/boot/$(LINUX_IMAGE_NAME)%" \
 		$(ROOTFS_ISO9660_BOOTLOADER_CONFIG_PATH)
-	$(ROOTFS_ISO9660_INSTALL_BOOTLOADER)
+	$(ROOTFS_ISO9660_INSTALL_BOOTLOADER_BIOS)
+	$(ROOTFS_ISO9660_INSTALL_BOOTLOADER_EFI)
 endef
 
 ROOTFS_ISO9660_PRE_GEN_HOOKS += ROOTFS_ISO9660_PREPARATION
@@ -147,23 +152,32 @@ ROOTFS_ISO9660_PRE_GEN_HOOKS += ROOTFS_ISO9660_DISABLE_EXTERNAL_INITRD
 
 endif # ROOTFS_ISO9660_USE_INITRD
 
-ifeq ($(BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER),y)
-ROOTFS_ISO9660_OPTS += \
+ROOTFS_ISO9660_BOOTLOADER_OPTS_BIOS = \
 	-J \
 	-R \
 	-boot-load-size 4 \
 	-boot-info-table \
 	-no-emul-boot \
 	-b $(ROOTFS_ISO9660_BOOT_IMAGE)
-else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),y)
-ROOTFS_ISO9660_OPTS += \
+
+ROOTFS_ISO9660_BOOTLOADER_OPTS_EFI = \
 	--efi-boot $(ROOTFS_ISO9660_EFI_PARTITION) \
 	-no-emul-boot
+
+ifeq ($(BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER)$(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),yy)
+ROOTFS_ISO9660_BOOTLOADER_OPTS = $(ROOTFS_ISO9660_BOOTLOADER_OPTS_BIOS)
+ROOTFS_ISO9660_BOOTLOADER_OPTS += -eltorito-alt-boot
+ROOTFS_ISO9660_BOOTLOADER_OPTS += $(ROOTFS_ISO9660_BOOTLOADER_OPTS_EFI)
+else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER),y)
+ROOTFS_ISO9660_BOOTLOADER_OPTS = $(ROOTFS_ISO9660_BOOTLOADER_OPTS_BIOS)
+else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),y)
+ROOTFS_ISO9660_BOOTLOADER_OPTS = $(ROOTFS_ISO9660_BOOTLOADER_OPTS_EFI)
 endif
 
 define ROOTFS_ISO9660_CMD
 	$(HOST_DIR)/bin/xorriso -as mkisofs \
 		$(ROOTFS_ISO9660_OPTS) \
+		$(ROOTFS_ISO9660_BOOTLOADER_OPTS) \
 		-o $@ $(ROOTFS_ISO9660_TMP_TARGET_DIR)
 endef
 
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 4/6] support/testing/infra/emulator.py: update encoding when calling qemu
  2021-09-21 13:28 [Buildroot] [PATCH v2 0/6] Add support for ISO9660 image compatible with Legacy and EFI BIOS Kory Maincent
                   ` (2 preceding siblings ...)
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 3/6] fs/iso9660: add support for hybrid image using Grub bootloader on BIOS and EFI Kory Maincent
@ 2021-09-21 13:28 ` Kory Maincent
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 5/6] boot/edk2: add support to i386 architecture Kory Maincent
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 6/6] support/testing/tests/fs/test_iso9660.py: add support to test using EFI BIOS Kory Maincent
  5 siblings, 0 replies; 16+ messages in thread
From: Kory Maincent @ 2021-09-21 13:28 UTC (permalink / raw)
  To: buildroot; +Cc: yann.morin.1998, thomas.petazzoni

With UTF-8 got issue with wrong character returned by Qemu when using EFI
BIOS. This breaks the test process with the following error. Update to
ISO-8859-1 encoding to avoid it.

    emulator.login()
  File "/home/kmaincent/Documents/projects/ariane-groupe/buildroot/support/testing/infra/emulator.py", line 89, in login
    index = self.qemu.expect(["buildroot login:", pexpect.TIMEOUT],
  File "/usr/lib/python3/dist-packages/pexpect/spawnbase.py", line 340, in expect
    return self.expect_list(compiled_pattern_list,
  File "/usr/lib/python3/dist-packages/pexpect/spawnbase.py", line 369, in expect_list
    return exp.expect_loop(timeout)
  File "/usr/lib/python3/dist-packages/pexpect/expect.py", line 111, in expect_loop
    incoming = spawn.read_nonblocking(spawn.maxread, timeout)
  File "/usr/lib/python3/dist-packages/pexpect/pty_spawn.py", line 485, in read_nonblocking
    return super(spawn, self).read_nonblocking(size)
  File "/usr/lib/python3/dist-packages/pexpect/spawnbase.py", line 178, in read_nonblocking
    s = self._decoder.decode(s, final=False)
  File "/usr/lib/python3.8/codecs.py", line 322, in decode
    (result, consumed) = self._buffer_decode(data, self.errors, final)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xda in position 0: invalid continuation byte

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 support/testing/infra/emulator.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
index 0a77eb80fc..1fab9caad8 100644
--- a/support/testing/infra/emulator.py
+++ b/support/testing/infra/emulator.py
@@ -76,7 +76,7 @@ class Emulator(object):
         self.logfile.write("> starting qemu with '%s'\n" % " ".join(qemu_cmd))
         self.qemu = pexpect.spawn(qemu_cmd[0], qemu_cmd[1:],
                                   timeout=5 * self.timeout_multiplier,
-                                  encoding='utf-8',
+                                  encoding='ISO-8859-1',
                                   env={"QEMU_AUDIO_DRV": "none"})
         # We want only stdout into the log to avoid double echo
         self.qemu.logfile_read = self.logfile
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 5/6] boot/edk2: add support to i386 architecture
  2021-09-21 13:28 [Buildroot] [PATCH v2 0/6] Add support for ISO9660 image compatible with Legacy and EFI BIOS Kory Maincent
                   ` (3 preceding siblings ...)
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 4/6] support/testing/infra/emulator.py: update encoding when calling qemu Kory Maincent
@ 2021-09-21 13:28 ` Kory Maincent
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 6/6] support/testing/tests/fs/test_iso9660.py: add support to test using EFI BIOS Kory Maincent
  5 siblings, 0 replies; 16+ messages in thread
From: Kory Maincent @ 2021-09-21 13:28 UTC (permalink / raw)
  To: buildroot; +Cc: yann.morin.1998, thomas.petazzoni

Add support the build the firmware for QEMU i386 pc machine

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 boot/edk2/Config.in | 12 +++++++++++-
 boot/edk2/edk2.mk   | 12 ++++++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/boot/edk2/Config.in b/boot/edk2/Config.in
index 150806899f..9f8988bcca 100644
--- a/boot/edk2/Config.in
+++ b/boot/edk2/Config.in
@@ -1,6 +1,6 @@
 config BR2_TARGET_EDK2
 	bool "EDK2"
-	depends on BR2_x86_64 || BR2_aarch64
+	depends on BR2_x86_64 || BR2_aarch64 || BR2_i386
 	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_5
 	select BR2_PACKAGE_EDK2_PLATFORMS
 	help
@@ -13,9 +13,18 @@ if BR2_TARGET_EDK2
 
 choice
 	prompt "Platform"
+	default BR2_TARGET_EDK2_PLATFORM_OVMF_I386 if BR2_i386
 	default BR2_TARGET_EDK2_PLATFORM_OVMF_X64 if BR2_x86_64
 	default BR2_TARGET_EDK2_PLATFORM_ARM_VIRT_QEMU if BR2_aarch64
 
+config BR2_TARGET_EDK2_PLATFORM_OVMF_I386
+	bool "i386"
+	depends on BR2_i386 || BR2_x86_64
+	help
+	  Platform configuration for a generic i386 target.
+	  This platform will boot from flash address 0x0.
+	  It should therefore be used as the first bootloader.
+
 config BR2_TARGET_EDK2_PLATFORM_OVMF_X64
 	bool "x86-64"
 	depends on BR2_x86_64
@@ -94,6 +103,7 @@ endchoice
 
 config BR2_TARGET_EDK2_FD_NAME
 	string
+	default "OVMF" if BR2_TARGET_EDK2_PLATFORM_OVMF_I386
 	default "OVMF" if BR2_TARGET_EDK2_PLATFORM_OVMF_X64
 	default "QEMU_EFI" if BR2_TARGET_EDK2_PLATFORM_ARM_VIRT_QEMU
 	default "QEMU_EFI" if BR2_TARGET_EDK2_PLATFORM_ARM_VIRT_QEMU_KERNEL
diff --git a/boot/edk2/edk2.mk b/boot/edk2/edk2.mk
index fabd0c5b45..ab3cdad464 100644
--- a/boot/edk2/edk2.mk
+++ b/boot/edk2/edk2.mk
@@ -14,7 +14,9 @@ EDK2_DEPENDENCIES = edk2-platforms host-python3 host-acpica host-util-linux
 EDK2_INSTALL_TARGET = NO
 EDK2_INSTALL_IMAGES = YES
 
-ifeq ($(BR2_x86_64),y)
+ifeq ($(BR2_i386),y)
+EDK2_ARCH = IA32
+else ifeq ($(BR2_x86_64),y)
 EDK2_ARCH = X64
 else ifeq ($(BR2_aarch64),y)
 EDK2_ARCH = AARCH64
@@ -55,7 +57,13 @@ EDK2_GIT_SUBMODULES = YES
 EDK2_BUILD_PACKAGES = $(@D)/Build/Buildroot
 EDK2_PACKAGES_PATH = $(@D):$(EDK2_BUILD_PACKAGES):$(STAGING_DIR)/usr/share/edk2-platforms
 
-ifeq ($(BR2_TARGET_EDK2_PLATFORM_OVMF_X64),y)
+ifeq ($(BR2_TARGET_EDK2_PLATFORM_OVMF_I386),y)
+EDK2_DEPENDENCIES += host-nasm
+EDK2_PACKAGE_NAME = OvmfPkg
+EDK2_PLATFORM_NAME = OvmfPkgIa32
+EDK2_BUILD_DIR = OvmfIa32
+
+else ifeq ($(BR2_TARGET_EDK2_PLATFORM_OVMF_X64),y)
 EDK2_DEPENDENCIES += host-nasm
 EDK2_PACKAGE_NAME = OvmfPkg
 EDK2_PLATFORM_NAME = OvmfPkgX64
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Buildroot] [PATCH v2 6/6] support/testing/tests/fs/test_iso9660.py: add support to test using EFI BIOS
  2021-09-21 13:28 [Buildroot] [PATCH v2 0/6] Add support for ISO9660 image compatible with Legacy and EFI BIOS Kory Maincent
                   ` (4 preceding siblings ...)
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 5/6] boot/edk2: add support to i386 architecture Kory Maincent
@ 2021-09-21 13:28 ` Kory Maincent
  5 siblings, 0 replies; 16+ messages in thread
From: Kory Maincent @ 2021-09-21 13:28 UTC (permalink / raw)
  To: buildroot; +Cc: yann.morin.1998, thomas.petazzoni

The ISO9660 tests are only testing BIOS Legacy.
Add support to test an ISO9660 image based on EFI BIOS.
Add support to test an ISO9660 hybrid image based on Legacy and EFI BIOS.
Add dedicated Grub2 builtin config for the EFI compatible cases.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 support/testing/conf/grub2-efi.cfg       |  2 +
 support/testing/tests/fs/test_iso9660.py | 74 +++++++++++++++++++++++-
 2 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 support/testing/conf/grub2-efi.cfg

diff --git a/support/testing/conf/grub2-efi.cfg b/support/testing/conf/grub2-efi.cfg
new file mode 100644
index 0000000000..11c32e880e
--- /dev/null
+++ b/support/testing/conf/grub2-efi.cfg
@@ -0,0 +1,2 @@
+set root=(cd0)
+set prefix=/boot/grub
diff --git a/support/testing/tests/fs/test_iso9660.py b/support/testing/tests/fs/test_iso9660.py
index 1b699e1fef..8315442604 100644
--- a/support/testing/tests/fs/test_iso9660.py
+++ b/support/testing/tests/fs/test_iso9660.py
@@ -25,9 +25,13 @@ BASIC_CONFIG = \
     """.format(infra.filepath("conf/minimal-x86-qemu-kernel.config"))
 
 
-def test_mount_internal_external(emulator, builddir, internal=True):
+def test_mount_internal_external(emulator, builddir, internal=True, efi=False):
     img = os.path.join(builddir, "images", "rootfs.iso9660")
-    emulator.boot(arch="i386", options=["-cdrom", img])
+    if efi:
+        efi_img = os.path.join(builddir, "images", "OVMF.fd")
+        emulator.boot(arch="i386", options=["-cdrom", img, "-bios", efi_img])
+    else:
+        emulator.boot(arch="i386", options=["-cdrom", img])
     emulator.login()
 
     if internal:
@@ -53,6 +57,7 @@ class TestIso9660Grub2External(infra.basetest.BRTest):
         BR2_TARGET_ROOTFS_ISO9660=y
         # BR2_TARGET_ROOTFS_ISO9660_INITRD is not set
         BR2_TARGET_GRUB2=y
+        BR2_TARGET_GRUB2_I386_PC=y
         BR2_TARGET_GRUB2_BOOT_PARTITION="cd"
         BR2_TARGET_GRUB2_BUILTIN_MODULES_PC="boot linux ext2 fat part_msdos part_gpt normal biosdisk iso9660"
         BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
@@ -74,6 +79,7 @@ class TestIso9660Grub2ExternalCompress(infra.basetest.BRTest):
         # BR2_TARGET_ROOTFS_ISO9660_INITRD is not set
         BR2_TARGET_ROOTFS_ISO9660_TRANSPARENT_COMPRESSION=y
         BR2_TARGET_GRUB2=y
+        BR2_TARGET_GRUB2_I386_PC=y
         BR2_TARGET_GRUB2_BOOT_PARTITION="cd"
         BR2_TARGET_GRUB2_BUILTIN_MODULES_PC="boot linux ext2 fat part_msdos part_gpt normal biosdisk iso9660"
         BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
@@ -94,6 +100,7 @@ class TestIso9660Grub2Internal(infra.basetest.BRTest):
         BR2_TARGET_ROOTFS_ISO9660=y
         BR2_TARGET_ROOTFS_ISO9660_INITRD=y
         BR2_TARGET_GRUB2=y
+        BR2_TARGET_GRUB2_I386_PC=y
         BR2_TARGET_GRUB2_BOOT_PARTITION="cd"
         BR2_TARGET_GRUB2_BUILTIN_MODULES_PC="boot linux ext2 fat part_msdos part_gpt normal biosdisk iso9660"
         BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
@@ -107,6 +114,69 @@ class TestIso9660Grub2Internal(infra.basetest.BRTest):
         exit_code = test_touch_file(self.emulator)
         self.assertEqual(exit_code, 0)
 
+
+class TestIso9660Grub2EFI(infra.basetest.BRTest):
+    config = BASIC_CONFIG + \
+        """
+        BR2_TARGET_ROOTFS_ISO9660=y
+        BR2_TARGET_ROOTFS_ISO9660_INITRD=y
+        BR2_TARGET_GRUB2=y
+        BR2_TARGET_GRUB2_I386_EFI=y
+        BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI="boot linux ext2 fat part_msdos part_gpt normal iso9660"
+        BR2_TARGET_GRUB2_BUILTIN_CONFIG_EFI="{}"
+        BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
+        BR2_TARGET_EDK2=y
+        """.format(infra.filepath("conf/grub2-efi.cfg"),
+                   infra.filepath("conf/grub2.cfg"))
+
+    def test_run(self):
+        exit_code = test_mount_internal_external(self.emulator,
+                                                 self.builddir, internal=True,
+                                                 efi=True)
+        self.assertEqual(exit_code, 0)
+
+        exit_code = test_touch_file(self.emulator)
+        self.assertEqual(exit_code, 0)
+
+
+class TestIso9660Grub2Hybrid(infra.basetest.BRTest):
+    config = BASIC_CONFIG + \
+        """
+        BR2_TARGET_ROOTFS_ISO9660=y
+        BR2_TARGET_ROOTFS_ISO9660_INITRD=y
+        BR2_TARGET_GRUB2=y
+        BR2_TARGET_GRUB2_I386_PC=y
+        BR2_TARGET_GRUB2_I386_EFI=y
+        BR2_TARGET_GRUB2_BOOT_PARTITION="cd"
+        BR2_TARGET_GRUB2_BUILTIN_MODULES_PC="boot linux ext2 fat squash4 part_msdos part_gpt normal iso9660 biosdisk"
+        BR2_TARGET_GRUB2_BUILTIN_CONFIG_PC=""
+        BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI="boot linux ext2 fat squash4 part_msdos part_gpt normal iso9660 efi_gop"
+        BR2_TARGET_GRUB2_BUILTIN_CONFIG_EFI="{}"
+        BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
+        BR2_TARGET_EDK2=y
+        """.format(infra.filepath("conf/grub2-efi.cfg"),
+                   infra.filepath("conf/grub2.cfg"))
+
+    def test_run(self):
+        exit_code = test_mount_internal_external(self.emulator,
+                                                 self.builddir, internal=True,
+                                                 efi=False)
+        self.assertEqual(exit_code, 0)
+
+        exit_code = test_touch_file(self.emulator)
+        self.assertEqual(exit_code, 0)
+
+        self.emulator.stop()
+
+        exit_code = test_mount_internal_external(self.emulator,
+                                                 self.builddir, internal=True,
+                                                 efi=True)
+        self.assertEqual(exit_code, 0)
+
+        exit_code = test_touch_file(self.emulator)
+        self.assertEqual(exit_code, 0)
+
+
 #
 # Syslinux
 
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Buildroot] [PATCH v2 2/6] fs/iso9660: add support to Grub EFI bootloader in the image
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 2/6] fs/iso9660: add support to Grub EFI bootloader in the image Kory Maincent
@ 2021-09-21 15:36   ` Yann E. MORIN
  2021-09-21 18:41     ` Köry Maincent
  0 siblings, 1 reply; 16+ messages in thread
From: Yann E. MORIN @ 2021-09-21 15:36 UTC (permalink / raw)
  To: Kory Maincent; +Cc: thomas.petazzoni, buildroot

Köry, All,

On 2021-09-21 15:28 +0200, Kory Maincent spake thusly:
> Add support to boot the Grub bootloader from an EFI BIOS in the ISO9660
> image.
> For that we need to create EFI System Partition (ESP). The ESP is a vfat
> partition which contain the Grub2 binary at the /EFI/BOOT/ location.
> xorriso command will generate the iso image including the ESP.
> 
> We notice Grub can not read and mount the ESP, therefore we place the Grub
> configuration file in the ISO9660 partition. A Grub2 builtin configuration
> need to be used to tell Grub2 to search automatically its configuration
> file in the ISO9660 partition. Use 'set root=(cd0)' in the configuration
> file passed to BR2_TARGET_GRUB2_BUILTIN_CONFIG_EFI.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
> 
> Changes in v2:
> Following the review from Yann E. Morin:
> - Add parameter to mkfs and mcopy to allow reproducible builds
> - Fix typos and variable multi-line assignement
> 
>  fs/iso9660/Config.in  | 30 ++++++++++++++++++++++++------
>  fs/iso9660/iso9660.mk | 38 +++++++++++++++++++++++++++++++++++---
>  2 files changed, 59 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/iso9660/Config.in b/fs/iso9660/Config.in
> index 6f001c0640..4fc96db28c 100644
> --- a/fs/iso9660/Config.in
> +++ b/fs/iso9660/Config.in
> @@ -3,6 +3,7 @@ config BR2_TARGET_ROOTFS_ISO9660
>  	depends on (BR2_i386 || BR2_x86_64)
>  	depends on BR2_LINUX_KERNEL
>  	depends on BR2_TARGET_GRUB2_I386_PC || \
> +		BR2_TARGET_GRUB2_I386_EFI || BR2_TARGET_GRUB2_X86_64_EFI || \

iso9660 already depends on (BR2_i386 || BR2_x86_64), and thanks to the
select of a default platform for i386 and x86_64, we know that at least
one of the three i386-pc, i386-efi, or x86_64-efi is set, so we can
simplify the condition to just:

    depends on BR2_TARGET_GRUB2 || BR2_TARGET_SYSLINUX_ISOLINUX

No?

>  		BR2_TARGET_SYSLINUX_ISOLINUX
>  	select BR2_LINUX_KERNEL_INSTALL_TARGET \
>  	       if (!BR2_TARGET_ROOTFS_ISO9660_INITRD && !BR2_TARGET_ROOTFS_INITRAMFS)
> @@ -27,22 +28,38 @@ choice
>  
>  config BR2_TARGET_ROOTFS_ISO9660_GRUB2
>  	bool "grub2"
> -	depends on BR2_TARGET_GRUB2_I386_PC
> +	depends on BR2_TARGET_GRUB2_I386_PC || BR2_TARGET_GRUB2_I386_EFI || \
> +		BR2_TARGET_GRUB2_X86_64_EFI

Similarly here, the condition can be simplified to just:

    depends on BR2_TARGET_GRUB2

> +	select BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER \
> +	       if BR2_TARGET_GRUB2_I386_PC
> +	select BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER \
> +	       if (BR2_TARGET_GRUB2_I386_EFI || BR2_TARGET_GRUB2_X86_64_EFI)
>  	help
>  	  Use Grub 2 as the bootloader for the ISO9660 image. Make
>  	  sure to enable the 'iso9660' module in
> -	  BR2_TARGET_GRUB2_BUILTIN_MODULES and to use 'cd' as the boot
> -	  partition in BR2_TARGET_GRUB2_BOOT_PARTITION=.
> +	  BR2_TARGET_GRUB2_BUILTIN_MODULES_PC or
> +	  BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI. Use 'cd' as the boot
> +	  partition in BR2_TARGET_GRUB2_BOOT_PARTITION= for GRUB on BIOS
> +	  or 'set root=(cd0)' in the configuration file passed to
> +	  BR2_TARGET_GRUB2_BUILTIN_CONFIG_EFI for GRUB on EFI.
>  
>  config BR2_TARGET_ROOTFS_ISO9660_ISOLINUX
>  	bool "isolinux"
>  	depends on BR2_TARGET_SYSLINUX_ISOLINUX
> +	select BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER
>  
>  endchoice
>  
> +config BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER
> +	bool
> +
> +config BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER
> +	bool
> +
>  config BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU
>  	string "Boot menu config file"
> -	default "fs/iso9660/grub.cfg" if BR2_TARGET_ROOTFS_ISO9660_GRUB2
> +	default "fs/iso9660/grub.cfg" if BR2_TARGET_ROOTFS_ISO9660_GRUB2_PC || \
> +					BR2_TARGET_ROOTFS_ISO9660_GRUB2_EFI

I forgot to state so in the previous review, but I think it is more
appropriate to duplicate the default for each case; that's nicer to
read:

    default "fs/iso9660/grub.cfg" if BR2_TARGET_ROOTFS_ISO9660_GRUB2_PC
    default "fs/iso9660/grub.cfg" if BR2_TARGET_ROOTFS_ISO9660_GRUB2_EFI

However, BR2_TARGET_ROOTFS_ISO9660_GRUB2_PC and BR2_TARGET_ROOTFS_ISO9660_GRUB2_EFI
are defined nowhere...

Did you mean BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER or
BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER by chance?

>  	default "fs/iso9660/isolinux.cfg" if BR2_TARGET_ROOTFS_ISO9660_ISOLINUX
>  	help
>  	  Use this option to provide a custom bootloader configuration
> @@ -83,7 +100,8 @@ config BR2_TARGET_ROOTFS_ISO9660_HYBRID
>  
>  endif
>  
> -comment "iso image needs a Linux kernel and either grub2 i386-pc or isolinux to be built"
> +comment "iso image needs a Linux kernel and either grub2 or isolinux to be built"
>  	depends on BR2_i386 || BR2_x86_64
>  	depends on !BR2_LINUX_KERNEL || \
> -		!(BR2_TARGET_GRUB2_I386_PC || BR2_TARGET_SYSLINUX_ISOLINUX)
> +		!(BR2_TARGET_GRUB2_I386_PC || BR2_TARGET_GRUB2_I386_EFI || \
> +		BR2_TARGET_GRUB2_X86_64_EFI || BR2_TARGET_SYSLINUX_ISOLINUX)

Ditto, the condition can be simplified, as in the comment iself:

    depends on !BR2_LINUX_KERNEL || \
            !(BR2_TARGET_GRUB2 || BR2_TARGET_SYSLINUX_ISOLINUX)

> diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
> index 23421a9a5c..d46aec002b 100644
> --- a/fs/iso9660/iso9660.mk
> +++ b/fs/iso9660/iso9660.mk
> @@ -57,7 +57,11 @@ else
>  ROOTFS_ISO9660_TMP_TARGET_DIR = $(TARGET_DIR)
>  endif
>  
> -ifeq ($(BR2_TARGET_ROOTFS_ISO9660_GRUB2),y)
> +ifeq ($(BR2_REPRODUCIBLE),y)
> +ROOTFS_ISO9660_MKFS_OPTS = --invariant

'MKFS' is ambiguous: it seems to imply it applies to the iso9660 mkfs,
while in fact it applies to the vfat mkfs. So, maybe (yes, bikesheding,
and naming is hard): ROOTFS_ISO9660_VFAT_OPTS ?

> +endif
> +
> +ifeq ($(BR2_TARGET_ROOTFS_ISO9660_GRUB2)$(BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER),yy)
>  ROOTFS_ISO9660_DEPENDENCIES += grub2
>  ROOTFS_ISO9660_BOOTLOADER_CONFIG_PATH = \
>  	$(ROOTFS_ISO9660_TMP_TARGET_DIR)/boot/grub/grub.cfg
> @@ -66,6 +70,21 @@ define ROOTFS_ISO9660_INSTALL_BOOTLOADER
>  	$(INSTALL) -D -m 0644 $(BINARIES_DIR)/grub-eltorito.img \
>  		$(ROOTFS_ISO9660_TMP_TARGET_DIR)/boot/grub/grub-eltorito.img
>  endef
> +else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_GRUB2)$(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),yy)
> +ROOTFS_ISO9660_DEPENDENCIES += grub2 host-dosfstools host-mtools
> +ROOTFS_ISO9660_EFI_PARTITION = boot/fat.efi
> +ROOTFS_ISO9660_EFI_PARTITION_PATH = $(ROOTFS_ISO9660_TMP_TARGET_DIR)/$(ROOTFS_ISO9660_EFI_PARTITION)
> +ROOTFS_ISO9660_EFI_PARTITION_CONTENT = $(BINARIES_DIR)/efi-part
> +ROOTFS_ISO9660_BOOTLOADER_CONFIG_PATH = \
> +	$(ROOTFS_ISO9660_TMP_TARGET_DIR)/boot/grub/grub.cfg
> +define ROOTFS_ISO9660_INSTALL_BOOTLOADER
> +	rm -rf $(ROOTFS_ISO9660_EFI_PARTITION_PATH)
> +	mkdir -p $(dir $(ROOTFS_ISO9660_EFI_PARTITION_PATH))
> +	dd if=/dev/zero of=$(ROOTFS_ISO9660_EFI_PARTITION_PATH) bs=1M count=1
> +	$(HOST_DIR)/sbin/mkfs.vfat $(ROOTFS_ISO9660_MKFS_OPTS) $(ROOTFS_ISO9660_EFI_PARTITION_PATH)
> +	$(HOST_DIR)/bin/mcopy -p -m -i $(ROOTFS_ISO9660_EFI_PARTITION_PATH) -s \
> +		$(ROOTFS_ISO9660_EFI_PARTITION_CONTENT)/* ::/

Ah, but the whole point of reproducible, and thus --invariant and -p -m,
is that the generated filesystem is reproducible. Here, the VFAT fs
itself is reproducible thanks to --invariant, but you copy a file with
the date/time the build occurs, which by definition is not reproducible.
You should touch the file to $(SOURCE_DATE_EPOCH) before copying into
the vfat.

Yes, reproducibility is hard, but not as much as naming is. ;-]

> +endef
>  else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_ISOLINUX),y)
>  ROOTFS_ISO9660_DEPENDENCIES += syslinux
>  ROOTFS_ISO9660_BOOTLOADER_CONFIG_PATH = \
> @@ -128,9 +147,22 @@ ROOTFS_ISO9660_PRE_GEN_HOOKS += ROOTFS_ISO9660_DISABLE_EXTERNAL_INITRD
>  
>  endif # ROOTFS_ISO9660_USE_INITRD
>  
> +ifeq ($(BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER),y)
> +ROOTFS_ISO9660_OPTS += \
> +	-J \
> +	-R \
> +	-boot-load-size 4 \
> +	-boot-info-table \
> +	-no-emul-boot \

--no-emul-boot is in both cases, so maybe it can be left in the constant
options, in _CMDS below, no?

> +	-b $(ROOTFS_ISO9660_BOOT_IMAGE)
> +else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),y)

Note that BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER and
BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER are mutually exclusive, by
definition, but we also know at least one will be set, also by
definition.

So, the else clause can be a simple one, without the ifeq.

/methink.

> +ROOTFS_ISO9660_OPTS += \
> +	--efi-boot $(ROOTFS_ISO9660_EFI_PARTITION) \
> +	-no-emul-boot

So with an EFI bootloader, we do not need to generate an ISO with Joliet
and RockRidge externsions? Or is that implied by the mere fact that it
is using an efi-boot?

Regards,
Yann E. MORIN.

> +endif
> +
>  define ROOTFS_ISO9660_CMD
> -	$(HOST_DIR)/bin/xorriso -as mkisofs -J -R -b $(ROOTFS_ISO9660_BOOT_IMAGE) \
> -		-no-emul-boot -boot-load-size 4 -boot-info-table \
> +	$(HOST_DIR)/bin/xorriso -as mkisofs \
>  		$(ROOTFS_ISO9660_OPTS) \
>  		-o $@ $(ROOTFS_ISO9660_TMP_TARGET_DIR)
>  endef
> -- 
> 2.25.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Buildroot] [PATCH v2 1/6] boot/grub2: add support to build multiple Grub2 configurations in the same build
  2021-09-21 13:28 ` [Buildroot] [PATCH v2 1/6] boot/grub2: add support to build multiple Grub2 configurations in the same build Kory Maincent
@ 2021-09-21 16:37   ` Yann E. MORIN
  2021-09-21 17:20     ` Arnout Vandecappelle
  2021-09-21 17:49     ` Köry Maincent
  0 siblings, 2 replies; 16+ messages in thread
From: Yann E. MORIN @ 2021-09-21 16:37 UTC (permalink / raw)
  To: Kory Maincent; +Cc: thomas.petazzoni, buildroot

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Buildroot] [PATCH v2 1/6] boot/grub2: add support to build multiple Grub2 configurations in the same build
  2021-09-21 16:37   ` Yann E. MORIN
@ 2021-09-21 17:20     ` Arnout Vandecappelle
  2021-09-21 17:26       ` Yann E. MORIN
  2021-09-21 17:49     ` Köry Maincent
  1 sibling, 1 reply; 16+ messages in thread
From: Arnout Vandecappelle @ 2021-09-21 17:20 UTC (permalink / raw)
  To: Yann E. MORIN, Kory Maincent; +Cc: thomas.petazzoni, buildroot



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]

_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Buildroot] [PATCH v2 1/6] boot/grub2: add support to build multiple Grub2 configurations in the same build
  2021-09-21 17:20     ` Arnout Vandecappelle
@ 2021-09-21 17:26       ` Yann E. MORIN
  0 siblings, 0 replies; 16+ messages in thread
From: Yann E. MORIN @ 2021-09-21 17:26 UTC (permalink / raw)
  To: Arnout Vandecappelle; +Cc: Kory Maincent, thomas.petazzoni, buildroot

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Buildroot] [PATCH v2 1/6] boot/grub2: add support to build multiple Grub2 configurations in the same build
  2021-09-21 16:37   ` Yann E. MORIN
  2021-09-21 17:20     ` Arnout Vandecappelle
@ 2021-09-21 17:49     ` Köry Maincent
  2021-09-21 19:41       ` Yann E. MORIN
  1 sibling, 1 reply; 16+ messages in thread
From: Köry Maincent @ 2021-09-21 17:49 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: thomas.petazzoni, buildroot

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
_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Buildroot] [PATCH v2 2/6] fs/iso9660: add support to Grub EFI bootloader in the image
  2021-09-21 15:36   ` Yann E. MORIN
@ 2021-09-21 18:41     ` Köry Maincent
  2021-09-21 19:52       ` Yann E. MORIN
  0 siblings, 1 reply; 16+ messages in thread
From: Köry Maincent @ 2021-09-21 18:41 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: thomas.petazzoni, buildroot

Yann,

On Tue, 21 Sep 2021 17:36:09 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> > --- a/fs/iso9660/Config.in
> > +++ b/fs/iso9660/Config.in
> > @@ -3,6 +3,7 @@ config BR2_TARGET_ROOTFS_ISO9660
> >  	depends on (BR2_i386 || BR2_x86_64)
> >  	depends on BR2_LINUX_KERNEL
> >  	depends on BR2_TARGET_GRUB2_I386_PC || \
> > +		BR2_TARGET_GRUB2_I386_EFI || BR2_TARGET_GRUB2_X86_64_EFI
> > || \  
> 
> iso9660 already depends on (BR2_i386 || BR2_x86_64), and thanks to the
> select of a default platform for i386 and x86_64, we know that at least
> one of the three i386-pc, i386-efi, or x86_64-efi is set, so we can
> simplify the condition to just:
> 
>     depends on BR2_TARGET_GRUB2 || BR2_TARGET_SYSLINUX_ISOLINUX
> 
> No?

Yes, nice catch!

> >  config BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU
> >  	string "Boot menu config file"
> > -	default "fs/iso9660/grub.cfg" if BR2_TARGET_ROOTFS_ISO9660_GRUB2
> > +	default "fs/iso9660/grub.cfg" if
> > BR2_TARGET_ROOTFS_ISO9660_GRUB2_PC || \
> > +
> > BR2_TARGET_ROOTFS_ISO9660_GRUB2_EFI  
> 
> I forgot to state so in the previous review, but I think it is more
> appropriate to duplicate the default for each case; that's nicer to
> read:
> 
>     default "fs/iso9660/grub.cfg" if BR2_TARGET_ROOTFS_ISO9660_GRUB2_PC
>     default "fs/iso9660/grub.cfg" if BR2_TARGET_ROOTFS_ISO9660_GRUB2_EFI
> 
> However, BR2_TARGET_ROOTFS_ISO9660_GRUB2_PC and
> BR2_TARGET_ROOTFS_ISO9660_GRUB2_EFI are defined nowhere...
> 
> Did you mean BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER or
> BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER by chance?

meh, how does these config are still there !!

> > -ifeq ($(BR2_TARGET_ROOTFS_ISO9660_GRUB2),y)
> > +ifeq ($(BR2_REPRODUCIBLE),y)
> > +ROOTFS_ISO9660_MKFS_OPTS = --invariant  
> 
> 'MKFS' is ambiguous: it seems to imply it applies to the iso9660 mkfs,
> while in fact it applies to the vfat mkfs. So, maybe (yes, bikesheding,
> and naming is hard): ROOTFS_ISO9660_VFAT_OPTS ?

Let's go for it.

> > +define ROOTFS_ISO9660_INSTALL_BOOTLOADER
> > +	rm -rf $(ROOTFS_ISO9660_EFI_PARTITION_PATH)
> > +	mkdir -p $(dir $(ROOTFS_ISO9660_EFI_PARTITION_PATH))
> > +	dd if=/dev/zero of=$(ROOTFS_ISO9660_EFI_PARTITION_PATH) bs=1M
> > count=1
> > +	$(HOST_DIR)/sbin/mkfs.vfat $(ROOTFS_ISO9660_MKFS_OPTS)
> > $(ROOTFS_ISO9660_EFI_PARTITION_PATH)
> > +	$(HOST_DIR)/bin/mcopy -p -m -i
> > $(ROOTFS_ISO9660_EFI_PARTITION_PATH) -s \
> > +		$(ROOTFS_ISO9660_EFI_PARTITION_CONTENT)/* ::/  
> 
> Ah, but the whole point of reproducible, and thus --invariant and -p -m,
> is that the generated filesystem is reproducible. Here, the VFAT fs
> itself is reproducible thanks to --invariant, but you copy a file with
> the date/time the build occurs, which by definition is not reproducible.
> You should touch the file to $(SOURCE_DATE_EPOCH) before copying into
> the vfat.

Ack

> > +ifeq ($(BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER),y)
> > +ROOTFS_ISO9660_OPTS += \
> > +	-J \
> > +	-R \
> > +	-boot-load-size 4 \
> > +	-boot-info-table \
> > +	-no-emul-boot \  
> 
> --no-emul-boot is in both cases, so maybe it can be left in the constant
> options, in _CMDS below, no?

True it could. -no-emul-boot need to be set in both boot cases, if fact I have
added it like that to not need to separate it in the next patch. ;)
I will add it to constant options.

> 
> > +	-b $(ROOTFS_ISO9660_BOOT_IMAGE)
> > +else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),y)  
> 
> Note that BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER and
> BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER are mutually exclusive, by
> definition, but we also know at least one will be set, also by
> definition.
> 
> So, the else clause can be a simple one, without the ifeq.

doh! you raise a point. In fact in the current configuration they are not
mutually exclusive. I need to fix that with a Kconfig menu.

> 
> /methink.
> 
> > +ROOTFS_ISO9660_OPTS += \
> > +	--efi-boot $(ROOTFS_ISO9660_EFI_PARTITION) \
> > +	-no-emul-boot  
> 
> So with an EFI bootloader, we do not need to generate an ISO with Joliet
> and RockRidge externsions? Or is that implied by the mere fact that it
> is using an efi-boot?

No, you are right I should add them to constant options.

Thanks,
Regards,

Köry
_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Buildroot] [PATCH v2 1/6] boot/grub2: add support to build multiple Grub2 configurations in the same build
  2021-09-21 17:49     ` Köry Maincent
@ 2021-09-21 19:41       ` Yann E. MORIN
  2021-09-22  7:45         ` Köry Maincent
  0 siblings, 1 reply; 16+ messages in thread
From: Yann E. MORIN @ 2021-09-21 19:41 UTC (permalink / raw)
  To: Köry Maincent; +Cc: thomas.petazzoni, buildroot

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Buildroot] [PATCH v2 2/6] fs/iso9660: add support to Grub EFI bootloader in the image
  2021-09-21 18:41     ` Köry Maincent
@ 2021-09-21 19:52       ` Yann E. MORIN
  0 siblings, 0 replies; 16+ messages in thread
From: Yann E. MORIN @ 2021-09-21 19:52 UTC (permalink / raw)
  To: Köry Maincent; +Cc: thomas.petazzoni, buildroot

Köry, All,

On 2021-09-21 20:41 +0200, Köry Maincent spake thusly:
> On Tue, 21 Sep 2021 17:36:09 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
[--SNIP--]
> > > +	-b $(ROOTFS_ISO9660_BOOT_IMAGE)
> > > +else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),y)  
> > Note that BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER and
> > BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER are mutually exclusive, by
> > definition, but we also know at least one will be set, also by
> > definition.
> > So, the else clause can be a simple one, without the ifeq.
> 
> doh! you raise a point. In fact in the current configuration they are not
> mutually exclusive. I need to fix that with a Kconfig menu.

Ah, right! And this patch is not (yet) about making a fully hybrid
image, so you *have* to have the user choose what boot payload to use,
probably something like:

    choice
        bool "Boot payload"

    config BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER
        bool "legacy bios"
        depends on something I guess...

    config BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER
        bool "UEFI"
        depends on something else I guess...

    endchoice

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.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Buildroot] [PATCH v2 1/6] boot/grub2: add support to build multiple Grub2 configurations in the same build
  2021-09-21 19:41       ` Yann E. MORIN
@ 2021-09-22  7:45         ` Köry Maincent
  0 siblings, 0 replies; 16+ messages in thread
From: Köry Maincent @ 2021-09-22  7:45 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: thomas.petazzoni, buildroot

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
_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-09-22  7:45 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-21 13:28 [Buildroot] [PATCH v2 0/6] Add support for ISO9660 image compatible with Legacy and EFI BIOS Kory Maincent
2021-09-21 13:28 ` [Buildroot] [PATCH v2 1/6] boot/grub2: add support to build multiple Grub2 configurations in the same build Kory Maincent
2021-09-21 16:37   ` Yann E. MORIN
2021-09-21 17:20     ` Arnout Vandecappelle
2021-09-21 17:26       ` Yann E. MORIN
2021-09-21 17:49     ` Köry Maincent
2021-09-21 19:41       ` Yann E. MORIN
2021-09-22  7:45         ` Köry Maincent
2021-09-21 13:28 ` [Buildroot] [PATCH v2 2/6] fs/iso9660: add support to Grub EFI bootloader in the image Kory Maincent
2021-09-21 15:36   ` Yann E. MORIN
2021-09-21 18:41     ` Köry Maincent
2021-09-21 19:52       ` Yann E. MORIN
2021-09-21 13:28 ` [Buildroot] [PATCH v2 3/6] fs/iso9660: add support for hybrid image using Grub bootloader on BIOS and EFI Kory Maincent
2021-09-21 13:28 ` [Buildroot] [PATCH v2 4/6] support/testing/infra/emulator.py: update encoding when calling qemu Kory Maincent
2021-09-21 13:28 ` [Buildroot] [PATCH v2 5/6] boot/edk2: add support to i386 architecture Kory Maincent
2021-09-21 13:28 ` [Buildroot] [PATCH v2 6/6] support/testing/tests/fs/test_iso9660.py: add support to test using EFI BIOS Kory Maincent

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.