[Buildroot] [PATCH 0/8] Add support for ISO9660 image compatible with Legacy and EFI BIOS

[Buildroot] [PATCH 0/8] Add support for ISO9660 image compatible with Legacy and EFI BIOS

[Kory Maincent]

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 enables zlib to host-xorriso package
 - PATCH 3 updates the generation of ISO9660 image to use xorriso
 - PATCH 4 implements the generation of ISO9660 image booting on a EFI BIOS
 - PATCH 5 implements the generation of hybrid image compatible with Legacy
   and EFI BIOS
 - PATCH 6 updates the encoding of the text return from testing emulator
 - PATCH 7 implements host-efi-bios package to download the EFI binaries
 - PATCH 8 updates iso9660 tests and implements a test for EFI image and
   hybrid image.

Thanks in advance for your review and feedback

Kory Maincent (8):
  boot/grub2: add support to build multiple Grub2 configurations in the
    same build
  package/xorriso: build host variant with zlib support
  fs/iso9660: switch from cdrkit to xorriso to build ISO9660 images
  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
  package/edk2-images: new package
  support/testing/tests/fs/test_iso9660.py: add support to test using
    EFI BIOS

 Config.in.legacy                         |  16 +++
 DEVELOPERS                               |   3 +
 boot/grub2/Config.in                     |  40 ++++--
 boot/grub2/grub2.mk                      | 165 +++++++++++++----------
 fs/iso9660/Config.in                     |  30 ++++-
 fs/iso9660/iso9660.mk                    |  59 ++++++--
 package/Config.in.host                   |   1 +
 package/edk2-images/Config.in.host       |  26 ++++
 package/edk2-images/edk2-images.mk       |  32 +++++
 package/edk2-images/rpm2cpio.sh          |  58 ++++++++
 package/xorriso/xorriso.mk               |   4 +-
 support/testing/conf/grub2-efi.cfg       |   2 +
 support/testing/infra/emulator.py        |   2 +-
 support/testing/tests/fs/test_iso9660.py |  83 +++++++++++-
 14 files changed, 417 insertions(+), 104 deletions(-)
 create mode 100644 package/edk2-images/Config.in.host
 create mode 100644 package/edk2-images/edk2-images.mk
 create mode 100755 package/edk2-images/rpm2cpio.sh
 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

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

[Kory Maincent]

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.
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>
---
 Config.in.legacy                         |  16 +++
 boot/grub2/Config.in                     |  40 ++++--
 boot/grub2/grub2.mk                      | 165 +++++++++++++----------
 support/testing/tests/fs/test_iso9660.py |   9 +-
 4 files changed, 146 insertions(+), 84 deletions(-)

diff --git a/Config.in.legacy b/Config.in.legacy
index a0c1a6898f..519cb830b6 100644
--- a/Config.in.legacy
+++ b/Config.in.legacy
@@ -144,6 +144,22 @@ endif
 
 ###############################################################################
 
+config BR2_TARGET_GRUB2_BUILTIN_MODULES
+	bool "the grub2 builtin modules has been renamed"
+	select BR2_LEGACY
+	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_CONFIG
+	bool "the grub2 builtin configuration has been renamed"
+	select BR2_LEGACY
+	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.
+
 comment "Legacy options removed in 2021.05"
 
 config BR2_PACKAGE_UDISKS_LVM2
diff --git a/boot/grub2/Config.in b/boot/grub2/Config.in
index e45133999e..79046b9c7f 100644
--- a/boot/grub2/Config.in
+++ b/boot/grub2/Config.in
@@ -27,9 +27,6 @@ config BR2_TARGET_GRUB2
 
 if BR2_TARGET_GRUB2
 
-choice
-	prompt "Platform"
-
 config BR2_TARGET_GRUB2_I386_PC
 	bool "i386-pc"
 	depends on BR2_i386 || BR2_x86_64
@@ -76,10 +73,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 +86,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 "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
+	default BR2_TARGET_GRUB2_BUILTIN_MODULES if BR2_TARGET_GRUB2_BUILTIN_MODULES != "" # legacy
+
+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 "boot linux ext2 fat squash4 part_msdos part_gpt normal efi_gop"
+	default BR2_TARGET_GRUB2_BUILTIN_MODULES if BR2_TARGET_GRUB2_BUILTIN_MODULES != "" # legacy
 
-config BR2_TARGET_GRUB2_BUILTIN_CONFIG
+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..04eadd5dc3 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_I386_PC_IMAGE = $(BINARIES_DIR)/grub.img
+GRUB2_I386_PC_CFG = $(TARGET_DIR)/boot/grub/grub.cfg
+GRUB2_I386_PC_PREFIX = ($(GRUB2_BOOT_PARTITION))/boot/grub
+GRUB2_I386_PC_TARGET = i386
+GRUB2_I386_PC_PLATFORM = pc
+GRUB2_I386_PC_BUILTIN = PC
+GRUB2_TUPLES += i386-pc
+endif
+ifeq ($(BR2_TARGET_GRUB2_I386_EFI),y)
+GRUB2_I386_EFI_IMAGE = $(BINARIES_DIR)/efi-part/EFI/BOOT/bootia32.efi
+GRUB2_I386_EFI_CFG = $(BINARIES_DIR)/efi-part/EFI/BOOT/grub.cfg
+GRUB2_I386_EFI_PREFIX = /EFI/BOOT
+GRUB2_I386_EFI_TARGET = i386
+GRUB2_I386_EFI_PLATFORM = efi
+GRUB2_I386_EFI_BUILTIN = EFI
+GRUB2_TUPLES += i386-efi
+endif
+ifeq ($(BR2_TARGET_GRUB2_X86_64_EFI),y)
+GRUB2_X86_64_EFI_IMAGE = $(BINARIES_DIR)/efi-part/EFI/BOOT/bootx64.efi
+GRUB2_X86_64_EFI_CFG = $(BINARIES_DIR)/efi-part/EFI/BOOT/grub.cfg
+GRUB2_X86_64_EFI_PREFIX = /EFI/BOOT
+GRUB2_X86_64_EFI_TARGET = x86_64
+GRUB2_X86_64_EFI_PLATFORM = efi
+GRUB2_X86_64_EFI_BUILTIN = EFI
+GRUB2_TUPLES += x86_64-efi
+endif
+ifeq ($(BR2_TARGET_GRUB2_ARM_UBOOT),y)
+GRUB2_ARM_UBOOT_IMAGE = $(BINARIES_DIR)/boot-part/grub/grub.img
+GRUB2_ARM_UBOOT_CFG = $(BINARIES_DIR)/boot-part/grub/grub.cfg
+GRUB2_ARM_UBOOT_PREFIX = ($(GRUB2_BOOT_PARTITION))/boot/grub
+GRUB2_ARM_UBOOT_TARGET = arm
+GRUB2_ARM_UBOOT_PLATFORM = uboot
+GRUB2_ARM_UBOOT_BUILTIN = PC
+GRUB2_TUPLES += arm-uboot
+endif
+ifeq ($(BR2_TARGET_GRUB2_ARM_EFI),y)
+GRUB2_ARM_EFI_IMAGE = $(BINARIES_DIR)/efi-part/EFI/BOOT/bootarm.efi
+GRUB2_ARM_EFI_CFG = $(BINARIES_DIR)/efi-part/EFI/BOOT/grub.cfg
+GRUB2_ARM_EFI_PREFIX = /EFI/BOOT
+GRUB2_ARM_EFI_TARGET = arm
+GRUB2_ARM_EFI_PLATFORM = efi
+GRUB2_ARM_EFI_BUILTIN = EFI
+GRUB2_TUPLES += arm-efi
+endif
+ifeq ($(BR2_TARGET_GRUB2_ARM64_EFI),y)
+GRUB2_ARM64_EFI_IMAGE = $(BINARIES_DIR)/efi-part/EFI/BOOT/bootaa64.efi
+GRUB2_ARM64_EFI_CFG = $(BINARIES_DIR)/efi-part/EFI/BOOT/grub.cfg
+GRUB2_ARM64_EFI_PREFIX = /EFI/BOOT
+GRUB2_ARM64_EFI_TARGET = aarch64
+GRUB2_ARM64_EFI_PLATFORM = efi
+GRUB2_ARM64_EFI_BUILTIN = EFI
+GRUB2_TUPLES += arm64-efi
 endif
 
 # Grub2 is kind of special: it considers CC, LD and so on to be the
@@ -128,8 +141,10 @@ 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) \
+	--target=$(GRUB2_$(call UPPERCASE,$(tuple))_TARGET) \
+	--with-platform=$(GRUB2_$(call UPPERCASE,$(tuple))_PLATFORM) \
 	--prefix=/ \
 	--exec-prefix=/ \
 	--disable-grub-mkfont \
@@ -147,34 +162,46 @@ 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 \
+			$(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_$(call UPPERCASE,$(tuple))_IMAGE)) ; \
+		$(HOST_DIR)/usr/bin/grub-mkimage \
+			-d $(@D)/build-$(tuple)/grub-core/ \
+			-O $(tuple) \
+			-o $(GRUB2_$(call UPPERCASE,$(tuple))_IMAGE) \
+			-p "$(GRUB2_$(call UPPERCASE,$(tuple))_PREFIX)" \
+			$(if $(GRUB2_BUILTIN_CONFIG_$(GRUB2_$(call UPPERCASE,$(tuple))_BUILTIN)), \
+				-c $(GRUB2_BUILTIN_CONFIG_$(GRUB2_$(call UPPERCASE,$(tuple))_BUILTIN))) \
+			$(GRUB2_BUILTIN_MODULES_$(GRUB2_$(call UPPERCASE,$(tuple))_BUILTIN)) ; \
+		$(INSTALL) -D -m 0644 boot/grub2/grub.cfg $(GRUB2_$(call UPPERCASE,$(tuple))_CFG) ; \
+		$(if $(findstring $(GRUB2_$(call UPPERCASE,$(tuple))_PLATFORM), pc), \
+			cat $(HOST_DIR)/lib/grub/$(tuple)/cdboot.img $(GRUB2_$(call UPPERCASE,$(tuple))_IMAGE) > \
+				$(BINARIES_DIR)/grub-eltorito.img, \
+		) \
+		$(if $(findstring $(GRUB2_$(call UPPERCASE,$(tuple))_PLATFORM), efi), \
+			echo $(notdir $(GRUB2_$(call UPPERCASE,$(tuple))_IMAGE)) > \
+				$(BINARIES_DIR)/efi-part/startup.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..fffa49caf3 100644
--- a/support/testing/tests/fs/test_iso9660.py
+++ b/support/testing/tests/fs/test_iso9660.py
@@ -54,7 +54,8 @@ 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_GRUB2_BUILTIN_CONFIG_PC=""
         BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
         """.format(infra.filepath("conf/grub2.cfg"))
 
@@ -75,7 +76,8 @@ 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_GRUB2_BUILTIN_CONFIG_PC=""
         BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
         """.format(infra.filepath("conf/grub2.cfg"))
 
@@ -95,7 +97,8 @@ 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_GRUB2_BUILTIN_CONFIG_PC=""
         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

[Buildroot] [PATCH 2/8] package/xorriso: build host variant with zlib support

[Kory Maincent]

We will soon use xorriso in the ISO9660 image generation support, and
this requires having zlib support in host-xorriso.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 package/xorriso/xorriso.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/package/xorriso/xorriso.mk b/package/xorriso/xorriso.mk
index 7b38f268bb..ff3414fd8a 100644
--- a/package/xorriso/xorriso.mk
+++ b/package/xorriso/xorriso.mk
@@ -12,14 +12,16 @@ XORRISO_LICENSE_FILES = COPYING COPYRIGHT
 # Disable everything until we actually need those features, and add the correct
 # host libraries
 HOST_XORRISO_CONF_OPTS = \
+	--enable-zlib \
 	--disable-xattr-h-pref-attr \
-	--disable-zlib \
 	--disable-libbz2 \
 	--disable-libcdio \
 	--disable-libreadline \
 	--disable-libedit \
 	--disable-libacl
 
+HOST_XORRISO_DEPENDENCIES = host-zlib
+
 # libcdio doesn't make sense for Linux
 # http://lists.gnu.org/archive/html/bug-xorriso/2017-04/msg00004.html
 XORRISO_CONF_OPTS = --disable-libcdio
-- 
2.25.1

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

[Buildroot] [PATCH 3/8] fs/iso9660: switch from cdrkit to xorriso to build ISO9660 images

[Kory Maincent]

In order to add support for EFI-compatible ISO9660 images in future
patches, this commit switch the ISO9660 logic to use xorriso instead of
cdrkit. Indeed the genimageiso tool from cdrkit doesn't have the
--efi-boot option needed to generate an image compatible with EFI BIOS.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 fs/iso9660/iso9660.mk | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
index a129655ce3..f80b735858 100644
--- a/fs/iso9660/iso9660.mk
+++ b/fs/iso9660/iso9660.mk
@@ -24,7 +24,7 @@
 
 ROOTFS_ISO9660_BOOT_MENU = $(call qstrip,$(BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU))
 
-ROOTFS_ISO9660_DEPENDENCIES = host-cdrkit linux
+ROOTFS_ISO9660_DEPENDENCIES = host-xorriso linux
 
 ifeq ($(BR2_TARGET_ROOTFS_INITRAMFS),y)
 ROOTFS_ISO9660_USE_INITRD = YES
@@ -52,7 +52,7 @@ define ROOTFS_ISO9660_MKZFTREE
 		$(ROOTFS_ISO9660_TMP_TARGET_DIR)
 endef
 ROOTFS_ISO9660_PRE_GEN_HOOKS += ROOTFS_ISO9660_MKZFTREE
-ROOTFS_ISO9660_GENISOIMAGE_OPTS += -z
+ROOTFS_ISO9660_XORRISO_OPTS += -z
 else
 ROOTFS_ISO9660_TMP_TARGET_DIR = $(TARGET_DIR)
 endif
@@ -129,9 +129,9 @@ ROOTFS_ISO9660_PRE_GEN_HOOKS += ROOTFS_ISO9660_DISABLE_EXTERNAL_INITRD
 endif # ROOTFS_ISO9660_USE_INITRD
 
 define ROOTFS_ISO9660_CMD
-	$(HOST_DIR)/bin/genisoimage -J -R -b $(ROOTFS_ISO9660_BOOT_IMAGE) \
+	$(HOST_DIR)/bin/xorriso -as mkisofs -J -R -b $(ROOTFS_ISO9660_BOOT_IMAGE) \
 		-no-emul-boot -boot-load-size 4 -boot-info-table \
-		$(ROOTFS_ISO9660_GENISOIMAGE_OPTS) \
+		$(ROOTFS_ISO9660_XORRISO_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

[Buildroot] [PATCH 4/8] fs/iso9660: add support to Grub EFI bootloader in the image

[Kory Maincent]

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.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 fs/iso9660/Config.in  | 30 ++++++++++++++++++++++++------
 fs/iso9660/iso9660.mk | 33 ++++++++++++++++++++++++++++++---
 2 files changed, 54 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 f80b735858..4e4c5de1d0 100644
--- a/fs/iso9660/iso9660.mk
+++ b/fs/iso9660/iso9660.mk
@@ -57,15 +57,31 @@ else
 ROOTFS_ISO9660_TMP_TARGET_DIR = $(TARGET_DIR)
 endif
 
-ifeq ($(BR2_TARGET_ROOTFS_ISO9660_GRUB2),y)
+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
 ROOTFS_ISO9660_BOOT_IMAGE = boot/grub/grub-eltorito.img
 define ROOTFS_ISO9660_INSTALL_BOOTLOADER
+	mkdir -p $(dir ROOTFS_ISO9660_BOOT_IMAGE)
 	$(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_EFI_PARTITION_PATH)
+	$(HOST_DIR)/bin/mcopy -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 +144,20 @@ 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_XORRISO_BOOTLOADER_OPTS = \
+	-J -R -b $(ROOTFS_ISO9660_BOOT_IMAGE) \
+	$(ROOTFS_ISO9660_XORRISO_BOOTLOADER_COMMON_OPTS) \
+	-boot-load-size 4 -boot-info-table -no-emul-boot
+else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),y)
+ROOTFS_ISO9660_XORRISO_BOOTLOADER_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_XORRISO_BOOTLOADER_OPTS) \
 		$(ROOTFS_ISO9660_XORRISO_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

[Buildroot] [PATCH 5/8] fs/iso9660: add support for hybrid image using Grub bootloader on BIOS and EFI

[Kory Maincent]

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>
---
 fs/iso9660/iso9660.mk | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
index 4e4c5de1d0..d1ce2a1a3f 100644
--- a/fs/iso9660/iso9660.mk
+++ b/fs/iso9660/iso9660.mk
@@ -62,19 +62,21 @@ 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
 	mkdir -p $(dir ROOTFS_ISO9660_BOOT_IMAGE)
 	$(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
@@ -82,12 +84,14 @@ define ROOTFS_ISO9660_INSTALL_BOOTLOADER
 	$(HOST_DIR)/bin/mcopy -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 \
@@ -100,7 +104,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
@@ -144,15 +149,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_XORRISO_BOOTLOADER_OPTS = \
+ROOTFS_ISO9660_XORRISO_BOOTLOADER_OPTS_BIOS = \
 	-J -R -b $(ROOTFS_ISO9660_BOOT_IMAGE) \
-	$(ROOTFS_ISO9660_XORRISO_BOOTLOADER_COMMON_OPTS) \
 	-boot-load-size 4 -boot-info-table -no-emul-boot
-else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),y)
-ROOTFS_ISO9660_XORRISO_BOOTLOADER_OPTS = \
+
+ROOTFS_ISO9660_XORRISO_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_XORRISO_BOOTLOADER_OPTS = $(ROOTFS_ISO9660_XORRISO_BOOTLOADER_OPTS_BIOS)
+ROOTFS_ISO9660_XORRISO_BOOTLOADER_OPTS += -eltorito-alt-boot
+ROOTFS_ISO9660_XORRISO_BOOTLOADER_OPTS += $(ROOTFS_ISO9660_XORRISO_BOOTLOADER_OPTS_EFI)
+else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_BIOS_BOOTLOADER),y)
+ROOTFS_ISO9660_XORRISO_BOOTLOADER_OPTS = $(ROOTFS_ISO9660_XORRISO_BOOTLOADER_OPTS_BIOS)
+else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),y)
+ROOTFS_ISO9660_XORRISO_BOOTLOADER_OPTS = $(ROOTFS_ISO9660_XORRISO_BOOTLOADER_OPTS_EFI)
 endif
 
 define ROOTFS_ISO9660_CMD
-- 
2.25.1

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

[Buildroot] [PATCH 6/8] support/testing/infra/emulator.py: update encoding when calling qemu

[Kory Maincent]

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 5611ec96e8..06b3840b75 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

[Buildroot] [PATCH 7/8] package/edk2-images: new package

[Kory Maincent]

In order to add support for EFI boot from the emulator, this patch add a
package to download the EFI BIOS prebuilt binary from:
https://www.kraxel.org/repos/jenkins/edk2/

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 DEVELOPERS                         |  3 ++
 package/Config.in.host             |  1 +
 package/edk2-images/Config.in.host | 26 ++++++++++++++
 package/edk2-images/edk2-images.mk | 32 +++++++++++++++++
 package/edk2-images/rpm2cpio.sh    | 58 ++++++++++++++++++++++++++++++
 5 files changed, 120 insertions(+)
 create mode 100644 package/edk2-images/Config.in.host
 create mode 100644 package/edk2-images/edk2-images.mk
 create mode 100755 package/edk2-images/rpm2cpio.sh

diff --git a/DEVELOPERS b/DEVELOPERS
index 42b5becb7e..a69703f6da 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -1503,6 +1503,9 @@ N:	Koen Martens <gmc@sonologic.nl>
 F:	package/capnproto/
 F:	package/linuxconsoletools/
 
+N:	Köry Maincent <kory.maincent@bootlin.com>
+F:	package/edk2-images
+
 N:	Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be>
 F:	package/bcusdk/
 F:	package/libpthsem/
diff --git a/package/Config.in.host b/package/Config.in.host
index 064c98af3d..9e3c15c5d3 100644
--- a/package/Config.in.host
+++ b/package/Config.in.host
@@ -20,6 +20,7 @@ menu "Host utilities"
 	source "package/dtc/Config.in.host"
 	source "package/e2fsprogs/Config.in.host"
 	source "package/e2tools/Config.in.host"
+	source "package/edk2-images/Config.in.host"
 	source "package/environment-setup/Config.in.host"
 	source "package/erofs-utils/Config.in.host"
 	source "package/eudev/Config.in.host"
diff --git a/package/edk2-images/Config.in.host b/package/edk2-images/Config.in.host
new file mode 100644
index 0000000000..ca105c757c
--- /dev/null
+++ b/package/edk2-images/Config.in.host
@@ -0,0 +1,26 @@
+config BR2_PACKAGE_HOST_EDK2_IMAGES
+	bool "host edk2-images"
+	help
+	  Downloads and installs pre-compiled EDK2 EFI BIOS images.
+	  They are typically useful to allow Qemu to emulate an x86 or
+	  x86-64 system with an EFI-compliant BIOS.
+
+	  https://www.kraxel.org/repos/jenkins/edk2/
+
+if BR2_PACKAGE_HOST_EDK2_IMAGES
+
+choice
+	prompt "EFI Architecture"
+	default BR2_PACKAGE_HOST_EDK2_IMAGES_IA32
+	help
+	  select the desired architecture
+
+config BR2_PACKAGE_HOST_EDK2_IMAGES_IA32
+	bool "ia32"
+
+config BR2_PACKAGE_HOST_EDK2_IMAGES_X64
+	bool "x64"
+
+endchoice
+
+endif
diff --git a/package/edk2-images/edk2-images.mk b/package/edk2-images/edk2-images.mk
new file mode 100644
index 0000000000..067bb4fcf7
--- /dev/null
+++ b/package/edk2-images/edk2-images.mk
@@ -0,0 +1,32 @@
+################################################################################
+#
+# edk2-images
+#
+################################################################################
+
+EDK2_IMAGES_VERSION = 20210804.42.gcf7c650592
+ifeq ($(BR2_PACKAGE_HOST_EDK2_IMAGES_IA32),y)
+EDK2_IMAGES_ARCH = ia32
+else
+EDK2_IMAGES_ARCH = x64
+endif
+EDK2_IMAGES_SITE = https://www.kraxel.org/repos/jenkins/edk2
+EDK2_IMAGES_SOURCE = edk2.git-ovmf-$(EDK2_IMAGES_ARCH)-0-$(EDK2_IMAGES_VERSION).noarch.rpm
+EDK2_IMAGES_LICENSE = BSD-2-Clause-Patent
+
+HOST_EDK2_IMAGES_INSTALL_DIR = $(HOST_DIR)/usr/share/ovmf-$(EDK2_IMAGES_ARCH)
+
+HOST_EDK2_IMAGES_DEPENDENCIES = host-cpio
+
+define HOST_EDK2_IMAGES_EXTRACT_CMDS
+	$(HOST_EDK2_IMAGES_PKGDIR)/rpm2cpio.sh $(HOST_EDK2_IMAGES_DL_DIR)/$(HOST_EDK2_IMAGES_SOURCE) | \
+		$(HOST_DIR)/bin/cpio -dimv -D $(@D)
+endef
+
+define HOST_EDK2_IMAGES_INSTALL_CMDS
+	rm -rf $(HOST_EDK2_IMAGES_INSTALL_DIR)
+	mkdir -p $(HOST_EDK2_IMAGES_INSTALL_DIR)
+	cp -af $(@D)/usr/share/edk2.git/ovmf-$(EDK2_IMAGES_ARCH)/* $(HOST_EDK2_IMAGES_INSTALL_DIR)/
+endef
+
+$(eval $(host-generic-package))
diff --git a/package/edk2-images/rpm2cpio.sh b/package/edk2-images/rpm2cpio.sh
new file mode 100755
index 0000000000..acedaefa22
--- /dev/null
+++ b/package/edk2-images/rpm2cpio.sh
@@ -0,0 +1,58 @@
+#!/bin/sh -efu
+
+# Script imported from the rpm Github repository.
+# https://github.com/rpm-software-management/rpm/blob/master/scripts/rpm2cpio.sh
+
+fatal() {
+	echo "$*" >&2
+	exit 1
+}
+
+pkg="$1"
+[ -n "$pkg" ] && [ -e "$pkg" ] ||
+	fatal "No package supplied"
+
+_dd() {
+	local o="$1"; shift
+	dd if="$pkg" skip="$o" iflag=skip_bytes status=none $*
+}
+
+calcsize() {
+	offset=$(($1 + 8))
+
+	local i b b0 b1 b2 b3 b4 b5 b6 b7
+
+	i=0
+	while [ $i -lt 8 ]; do
+		b="$(_dd $(($offset + $i)) bs=1 count=1)"
+		[ -z "$b" ] &&
+			b="0" ||
+			b="$(exec printf '%u\n' "'$b")"
+		eval "b$i=\$b"
+		i=$(($i + 1))
+	done
+
+	rsize=$((8 + ((($b0 << 24) + ($b1 << 16) + ($b2 << 8) + $b3) << 4) + ($b4 << 24) + ($b5 << 16) + ($b6 << 8) + $b7))
+	offset=$(($offset + $rsize))
+}
+
+case "$(_dd 0 bs=8 count=1)" in
+	"$(printf '\355\253\356\333')"*) ;; # '\xed\xab\xee\xdb'
+	*) fatal "File doesn't look like rpm: $pkg" ;;
+esac
+
+calcsize 96
+sigsize=$rsize
+
+calcsize $(($offset + (8 - ($sigsize % 8)) % 8))
+hdrsize=$rsize
+
+case "$(_dd $offset bs=3 count=1)" in
+	"$(printf '\102\132')"*) _dd $offset | bunzip2 ;; # '\x42\x5a'
+	"$(printf '\037\213')"*) _dd $offset | gunzip  ;; # '\x1f\x8b'
+	"$(printf '\375\067')"*) _dd $offset | xzcat   ;; # '\xfd\x37'
+	"$(printf '\135\000')"*) _dd $offset | unlzma  ;; # '\x5d\x00'
+	"$(printf '\050\265')"*) _dd $offset | unzstd  ;; # '\x28\xb5'
+	*) fatal "Unrecognized rpm file: $pkg" ;;
+esac
+
-- 
2.25.1

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

[Buildroot] [PATCH 8/8] support/testing/tests/fs/test_iso9660.py: add support to test using EFI BIOS

[Kory Maincent]

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 fffa49caf3..c986f044b3 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, "host/usr/share/ovmf-ia32/OVMF-pure-efi.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_GRUB2_BUILTIN_CONFIG_PC=""
@@ -75,6 +80,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_GRUB2_BUILTIN_CONFIG_PC=""
@@ -96,6 +102,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_GRUB2_BUILTIN_CONFIG_PC=""
@@ -110,6 +117,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_PACKAGE_HOST_EDK2_IMAGES=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_PACKAGE_HOST_EDK2_IMAGES=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

Re: [Buildroot] [PATCH 7/8] package/edk2-images: new package

[D. Olsson via buildroot]

[-- Attachment #1.1: Type: text/plain, Size: 559 bytes --]

Hi Kory,

On Tue, Sep 14, 2021 at 11:34, Kory Maincent <kory.maincent@bootlin.com> wrote:

> In order to add support for EFI boot from the emulator, this patch add a
> package to download the EFI BIOS prebuilt binary from:
> https://www.kraxel.org/repos/jenkins/edk2/

Since a few months[1] it's been possible to build EDK2 from source in Buildroot. Would it be possible to extend the existing package instead of duplicating the effort by maintaining two packages?

[1]: https://git.buildroot.net/buildroot/log/boot/edk2/edk2.hash?showmsg=1

Cheers

D. Olsson

[-- Attachment #1.2: Type: text/html, Size: 945 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

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

Re: [Buildroot] [PATCH 7/8] package/edk2-images: new package

[Thomas Petazzoni]

Hello,

On Fri, 17 Sep 2021 13:43:38 +0000
"D. Olsson" <hi@senzilla.io> wrote:

> On Tue, Sep 14, 2021 at 11:34, Kory Maincent
> <kory.maincent@bootlin.com> wrote:
> 
> > In order to add support for EFI boot from the emulator, this patch
> > add a package to download the EFI BIOS prebuilt binary from:
> > https://www.kraxel.org/repos/jenkins/edk2/  
> 
> Since a few months[1] it's been possible to build EDK2 from source in
> Buildroot. Would it be possible to extend the existing package
> instead of duplicating the effort by maintaining two packages?
> 
> [1]:
> https://git.buildroot.net/buildroot/log/boot/edk2/edk2.hash?showmsg=1

When Köry packaged edk2-images, I indeed thought of the fact that we
can now build EDK2 from source. That being said, to quickly get an EFI
BIOS image to do tests under Qemu, a pre-built EDK2 image is quite nice.

I'll let others comment on this.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 7/8] package/edk2-images: new package

[Yann E. MORIN]

Thomas, Dick, All,

On 2021-09-17 16:14 +0200, Thomas Petazzoni spake thusly:
> On Fri, 17 Sep 2021 13:43:38 +0000
> "D. Olsson" <hi@senzilla.io> wrote:
> > On Tue, Sep 14, 2021 at 11:34, Kory Maincent
> > <kory.maincent@bootlin.com> wrote:
> > > In order to add support for EFI boot from the emulator, this patch
> > > add a package to download the EFI BIOS prebuilt binary from:
> > > https://www.kraxel.org/repos/jenkins/edk2/  
> > Since a few months[1] it's been possible to build EDK2 from source in
> > Buildroot. Would it be possible to extend the existing package
> > instead of duplicating the effort by maintaining two packages?
> > 
> > [1]:
> > https://git.buildroot.net/buildroot/log/boot/edk2/edk2.hash?showmsg=1
> 
> When Köry packaged edk2-images, I indeed thought of the fact that we
> can now build EDK2 from source. That being said, to quickly get an EFI
> BIOS image to do tests under Qemu, a pre-built EDK2 image is quite nice.
> 
> I'll let others comment on this.

I see no reason to have a pre-built version of edk2.

So I agree with Dick: we should be using the edk2 boot package we
already have in-tree.

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

Re: [Buildroot] [PATCH 2/8] package/xorriso: build host variant with zlib support

[Yann E. MORIN]

Köry, All,

On 2021-09-14 11:34 +0200, Kory Maincent spake thusly:
> We will soon use xorriso in the ISO9660 image generation support, and
> this requires having zlib support in host-xorriso.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

Applied to master, thanks.

Regards,
Yann E. MORIN.

> ---
>  package/xorriso/xorriso.mk | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/package/xorriso/xorriso.mk b/package/xorriso/xorriso.mk
> index 7b38f268bb..ff3414fd8a 100644
> --- a/package/xorriso/xorriso.mk
> +++ b/package/xorriso/xorriso.mk
> @@ -12,14 +12,16 @@ XORRISO_LICENSE_FILES = COPYING COPYRIGHT
>  # Disable everything until we actually need those features, and add the correct
>  # host libraries
>  HOST_XORRISO_CONF_OPTS = \
> +	--enable-zlib \
>  	--disable-xattr-h-pref-attr \
> -	--disable-zlib \
>  	--disable-libbz2 \
>  	--disable-libcdio \
>  	--disable-libreadline \
>  	--disable-libedit \
>  	--disable-libacl
>  
> +HOST_XORRISO_DEPENDENCIES = host-zlib
> +
>  # libcdio doesn't make sense for Linux
>  # http://lists.gnu.org/archive/html/bug-xorriso/2017-04/msg00004.html
>  XORRISO_CONF_OPTS = --disable-libcdio
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@lists.buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

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

Re: [Buildroot] [PATCH 3/8] fs/iso9660: switch from cdrkit to xorriso to build ISO9660 images

[Yann E. MORIN]

Köry, All,

On 2021-09-14 11:34 +0200, Kory Maincent spake thusly:
> In order to add support for EFI-compatible ISO9660 images in future
> patches, this commit switch the ISO9660 logic to use xorriso instead of
> cdrkit. Indeed the genimageiso tool from cdrkit doesn't have the
> --efi-boot option needed to generate an image compatible with EFI BIOS.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
>  fs/iso9660/iso9660.mk | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
> index a129655ce3..f80b735858 100644
> --- a/fs/iso9660/iso9660.mk
> +++ b/fs/iso9660/iso9660.mk
> @@ -24,7 +24,7 @@
>  
>  ROOTFS_ISO9660_BOOT_MENU = $(call qstrip,$(BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU))
>  
> -ROOTFS_ISO9660_DEPENDENCIES = host-cdrkit linux
> +ROOTFS_ISO9660_DEPENDENCIES = host-xorriso linux
>  
>  ifeq ($(BR2_TARGET_ROOTFS_INITRAMFS),y)
>  ROOTFS_ISO9660_USE_INITRD = YES
> @@ -52,7 +52,7 @@ define ROOTFS_ISO9660_MKZFTREE
>  		$(ROOTFS_ISO9660_TMP_TARGET_DIR)
>  endef
>  ROOTFS_ISO9660_PRE_GEN_HOOKS += ROOTFS_ISO9660_MKZFTREE
> -ROOTFS_ISO9660_GENISOIMAGE_OPTS += -z
> +ROOTFS_ISO9660_XORRISO_OPTS += -z

Having the tool name in the variable name is totally superfluous; for
example. squashfs or ext2 do not have it. So, I've dropped that when
applying.

Applied to master, thanks.

Note: ext2 is however a bad example: its variables are not properly
namespace-prefixed with ROOTFS_...

Regards,
Yann E. MORIN.

>  else
>  ROOTFS_ISO9660_TMP_TARGET_DIR = $(TARGET_DIR)
>  endif
> @@ -129,9 +129,9 @@ ROOTFS_ISO9660_PRE_GEN_HOOKS += ROOTFS_ISO9660_DISABLE_EXTERNAL_INITRD
>  endif # ROOTFS_ISO9660_USE_INITRD
>  
>  define ROOTFS_ISO9660_CMD
> -	$(HOST_DIR)/bin/genisoimage -J -R -b $(ROOTFS_ISO9660_BOOT_IMAGE) \
> +	$(HOST_DIR)/bin/xorriso -as mkisofs -J -R -b $(ROOTFS_ISO9660_BOOT_IMAGE) \
>  		-no-emul-boot -boot-load-size 4 -boot-info-table \
> -		$(ROOTFS_ISO9660_GENISOIMAGE_OPTS) \
> +		$(ROOTFS_ISO9660_XORRISO_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

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

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

[Yann E. MORIN]

Köry, All,

On 2021-09-14 11:34 +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.
> 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.

What you forgot to state in bold fat characters here, is that we can no
longer use autotools-package as a consequence of this multi-build, and
that we have to resort to generic-pacakge and a partial duplication of
the autotools-infra.

This is not very nice... :-(

IIRC, we had a similar problem with barbox, where we had to build two
images for it, so we ended up duplicating the package, where the
top-most boot/barebox/barebox.mk defines a minimalist barebox-package
infra, and the barebox and barebox-aux are two barebox-package:

    boot/barebox/
    ├── barebox/
    │   ├── barebox.hash -> ../barebox.hash
    │   ├── barebox.mk
    │   └── Config.in
    ├── barebox-aux/
    │   ├── barebox-aux.hash -> ../barebox.hash
    │   ├── barebox-aux.mk
    │   └── Config.in
    ├── barebox.hash
    ├── barebox.mk
    └── Config.in

If I understand correctly, we are in a similar situation with grub2, and
there are only ever at most two "platforms" we can build, and it does
not look like it will ever change:

    i386:
        BR2_TARGET_GRUB2_I386_PC
        BR2_TARGET_GRUB2_I386_EFI

    x86_64:
        BR2_TARGET_GRUB2_I386_EFI
        BR2_TARGET_GRUB2_X86_64_EFI

    arm:
        BR2_TARGET_GRUB2_ARM_UBOOT
        BR2_TARGET_GRUB2_ARM_EFI

    aarch64:
        BR2_TARGET_GRUB2_ARM64_EFI

So, can't we do the same "duplication" of grub2?

Note that, in menuconfig, we can keep things as-is, so tht it looks
nicer to the user, and then we have a bit of logic to decide if we need
grub2-aux (or whatever it's named):

    config BR2_PACKAGE_GRUB2_NEEDS_AUX
        bool
        select BR2_PACKAGE_GRUB2_AUX
        default y if BR2_TARGET_GRUB2_I386_PC && BR2_TARGET_GRUB2_I386_EFI
        default y if BR2_TARGET_GRUB2_I386_EFI && BR2_TARGET_GRUB2_X86_64_EFI
        default y if BR2_TARGET_GRUB2_ARM_UBOOT && BR2_TARGET_GRUB2_ARM_EFI

And this means the user is not even aware (in menuconfig) that there is
a secondary grub2-aux package.

Anyway, see some review below...

> 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>
> ---
>  Config.in.legacy                         |  16 +++
>  boot/grub2/Config.in                     |  40 ++++--
>  boot/grub2/grub2.mk                      | 165 +++++++++++++----------
>  support/testing/tests/fs/test_iso9660.py |   9 +-
>  4 files changed, 146 insertions(+), 84 deletions(-)
> 
> diff --git a/Config.in.legacy b/Config.in.legacy
> index a0c1a6898f..519cb830b6 100644
> --- a/Config.in.legacy
> +++ b/Config.in.legacy
> @@ -144,6 +144,22 @@ endif
>  
>  ###############################################################################
>  
> +config BR2_TARGET_GRUB2_BUILTIN_MODULES
> +	bool "the grub2 builtin modules has been renamed"

BR2_TARGET_GRUB2_BUILTIN_MODULES was a string, so it can't be a boolean
in legacy.

> +	select BR2_LEGACY
> +	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_CONFIG
> +	bool "the grub2 builtin configuration has been renamed"

Ditto: a string migrates to a legacy string not a legacy bool.

> +	select BR2_LEGACY
> +	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.
> +
>  comment "Legacy options removed in 2021.05"
>  
>  config BR2_PACKAGE_UDISKS_LVM2
> diff --git a/boot/grub2/Config.in b/boot/grub2/Config.in
> index e45133999e..79046b9c7f 100644
> --- a/boot/grub2/Config.in
> +++ b/boot/grub2/Config.in
> @@ -27,9 +27,6 @@ config BR2_TARGET_GRUB2
>  
>  if BR2_TARGET_GRUB2
>  
> -choice
> -	prompt "Platform"

Before, this was a choice, whichj would guarantee that exactly one
platform would be enabled.

Now, with your patch, there is nothing that ensures that there is at
least one platform enabled.

When there are more than two options, like is the case here, what we
usually do is:

    config BR2_TARGET_GRUB2
        bool "grub2"
        select BR2_TARGET_GRUB2_I386_PC if !BR2_TARGET_GRUB2_HAS_PTF

    config BR2_TARGET_GRUB2_HAS_PTF
        bool

    config BR2_TARGET_GRUB2_I386_PC
        bool "i386-pc"

    config BR2_TARGET_GRUB2_I386_EFI
        bool "i386-efi"
        select BR2_TARGET_GRUB2_HAS_PTF

    config BR2_TARGET_GRUB2_X86_64_EFI
        bool "x86-64-efi"
        select BR2_TARGET_GRUB2_HAS_PTF

    # and so on...

This ensures that at least one platform is enabled.

>  config BR2_TARGET_GRUB2_I386_PC
>  	bool "i386-pc"
>  	depends on BR2_i386 || BR2_x86_64
> @@ -76,10 +73,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 +86,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 "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
> +	default BR2_TARGET_GRUB2_BUILTIN_MODULES if BR2_TARGET_GRUB2_BUILTIN_MODULES != "" # legacy

Kconfig uses the first default in order, which condition is valid, and
we want the legacy default to take precedence over the per-platform
default. So we want the legacy default to come first in the list:

    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 "linux ext2 fat part_msdos normal" if BR2_TARGET_GRUB2_ARM_UBOOT

> +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 "boot linux ext2 fat squash4 part_msdos part_gpt normal efi_gop"
> +	default BR2_TARGET_GRUB2_BUILTIN_MODULES if BR2_TARGET_GRUB2_BUILTIN_MODULES != "" # legacy

Ditto: legacy default comes first.

> -config BR2_TARGET_GRUB2_BUILTIN_CONFIG
> +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..04eadd5dc3 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))

Ah, wee're getting into the ugly part... ;-]

>  ifeq ($(BR2_TARGET_GRUB2_I386_PC),y)
[--SNIP removals--]
> +GRUB2_I386_PC_IMAGE = $(BINARIES_DIR)/grub.img
> +GRUB2_I386_PC_CFG = $(TARGET_DIR)/boot/grub/grub.cfg
> +GRUB2_I386_PC_PREFIX = ($(GRUB2_BOOT_PARTITION))/boot/grub
> +GRUB2_I386_PC_TARGET = i386
> +GRUB2_I386_PC_PLATFORM = pc
> +GRUB2_I386_PC_BUILTIN = PC
> +GRUB2_TUPLES += i386-pc
> +endif
> +ifeq ($(BR2_TARGET_GRUB2_I386_EFI),y)
> +GRUB2_I386_EFI_IMAGE = $(BINARIES_DIR)/efi-part/EFI/BOOT/bootia32.efi
> +GRUB2_I386_EFI_CFG = $(BINARIES_DIR)/efi-part/EFI/BOOT/grub.cfg
> +GRUB2_I386_EFI_PREFIX = /EFI/BOOT
> +GRUB2_I386_EFI_TARGET = i386
> +GRUB2_I386_EFI_PLATFORM = efi
> +GRUB2_I386_EFI_BUILTIN = EFI
> +GRUB2_TUPLES += i386-efi
> +endif
[--SNIP--]

I think it is more digestible if written as:

    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

    [...]

    $(foreach tuple, $(GRUB2_TUPLES_y), blablabla...)

That way you don;t have to play tricks with $(call UPPERCASE,$(tuple)),
and the conditional blocks are gone away which makes for a more legible
code (IMHO).

>  # Grub2 is kind of special: it considers CC, LD and so on to be the
> @@ -128,8 +141,10 @@ 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) \
> +	--target=$(GRUB2_$(call UPPERCASE,$(tuple))_TARGET) \
> +	--with-platform=$(GRUB2_$(call UPPERCASE,$(tuple))_PLATFORM) \

    --target=$(GRUB2_TARGET_$(tuple))
    --with-platform=$(GRUB2_PLATFORM_$(tuple)) \

However, I don't like macros that are implicitly parameterised. We've
gone in quite some changes in the past to fix similar cases (e.g. in the
download infra).

So, I'd write a real parameterised marco:

    # $(1): tuple blablabla...
    GRUB2_CONF_OPTS = \
        --host=$(GNU_TARGET_NAME) \
        --target=$(GRUB2_TARGET_$(1)) \
        --with-platform=$(GRUB2_PLATFORM_$(1)) \
        [...]

    $(foreach tuple, $(GRUB2_TUPLES_y), \
        [...] \
        ../configure $(call GRUB2_CONF_OPTS,$(tuple))
    )

    # And ditto for the _BUILD_CMDS...

>  	--prefix=/ \
>  	--exec-prefix=/ \
>  	--disable-grub-mkfont \
> @@ -147,34 +162,46 @@ 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 \
> +			$(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_$(call UPPERCASE,$(tuple))_IMAGE)) ; \
> +		$(HOST_DIR)/usr/bin/grub-mkimage \
> +			-d $(@D)/build-$(tuple)/grub-core/ \
> +			-O $(tuple) \
> +			-o $(GRUB2_$(call UPPERCASE,$(tuple))_IMAGE) \
> +			-p "$(GRUB2_$(call UPPERCASE,$(tuple))_PREFIX)" \
> +			$(if $(GRUB2_BUILTIN_CONFIG_$(GRUB2_$(call UPPERCASE,$(tuple))_BUILTIN)), \
> +				-c $(GRUB2_BUILTIN_CONFIG_$(GRUB2_$(call UPPERCASE,$(tuple))_BUILTIN))) \
> +			$(GRUB2_BUILTIN_MODULES_$(GRUB2_$(call UPPERCASE,$(tuple))_BUILTIN)) ; \
> +		$(INSTALL) -D -m 0644 boot/grub2/grub.cfg $(GRUB2_$(call UPPERCASE,$(tuple))_CFG) ; \
> +		$(if $(findstring $(GRUB2_$(call UPPERCASE,$(tuple))_PLATFORM), pc), \
> +			cat $(HOST_DIR)/lib/grub/$(tuple)/cdboot.img $(GRUB2_$(call UPPERCASE,$(tuple))_IMAGE) > \
> +				$(BINARIES_DIR)/grub-eltorito.img, \

Spurious trailing comma. You're lucky, this is interpreted as a
separator between the two 'if' clauses, providing an empty 'else'
clause. Still, please drop it.

> +		) \
> +		$(if $(findstring $(GRUB2_$(call UPPERCASE,$(tuple))_PLATFORM), efi), \
> +			echo $(notdir $(GRUB2_$(call UPPERCASE,$(tuple))_IMAGE)) > \
> +				$(BINARIES_DIR)/efi-part/startup.nsh \

For x86_64, you can now have two platforms providing the EFI stuff:
both BR2_TARGET_GRUB2_I386_EFI and BR2_TARGET_GRUB2_X86_64_EFI can now
be enabled at the same time, so the latter will replcae the former in
that startup.nsh file. This is probably not good...

> +		)
> +	)
>  endef
> -GRUB2_POST_INSTALL_IMAGES_HOOKS += GRUB2_EFI_STARTUP_NSH
> -endif
>  
> -$(eval $(autotools-package))
> +$(eval $(generic-package))

This is my main problem: we're mostly duplicating bits and pieces of the
autotolls-package infra, which means we *will* fall behind very quickly
when we have to update said infra, and those changes will not percolate
to this grub2 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..fffa49caf3 100644
> --- a/support/testing/tests/fs/test_iso9660.py
> +++ b/support/testing/tests/fs/test_iso9660.py
> @@ -54,7 +54,8 @@ 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_GRUB2_BUILTIN_CONFIG_PC=""

No need to provide an empty string here: the default *is* an empty
string.

>          BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
>          """.format(infra.filepath("conf/grub2.cfg"))
>  
> @@ -75,7 +76,8 @@ 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_GRUB2_BUILTIN_CONFIG_PC=""

Ditto.

>          BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
>          """.format(infra.filepath("conf/grub2.cfg"))
>  
> @@ -95,7 +97,8 @@ 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_GRUB2_BUILTIN_CONFIG_PC=""

Ditto.

Given the above, and given that the rest of the series )(except patches
2-3 already applied) depend on this first patch, I've marked the series
as Changes Requested in Patchwork.

Regards,
Yann E. MORIN.

>          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

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

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

[Thomas Petazzoni]

Hello Yann,

On Fri, 17 Sep 2021 23:24:29 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> What you forgot to state in bold fat characters here, is that we can no
> longer use autotools-package as a consequence of this multi-build, and
> that we have to resort to generic-pacakge and a partial duplication of
> the autotools-infra.
> 
> This is not very nice... :-(

Indeed, but...

> IIRC, we had a similar problem with barbox, where we had to build two
> images for it, so we ended up duplicating the package, where the
> top-most boot/barebox/barebox.mk defines a minimalist barebox-package
> infra, and the barebox and barebox-aux are two barebox-package:
> 
>     boot/barebox/
>     ├── barebox/
>     │   ├── barebox.hash -> ../barebox.hash
>     │   ├── barebox.mk
>     │   └── Config.in
>     ├── barebox-aux/
>     │   ├── barebox-aux.hash -> ../barebox.hash
>     │   ├── barebox-aux.mk
>     │   └── Config.in
>     ├── barebox.hash
>     ├── barebox.mk
>     └── Config.in
> 
> If I understand correctly, we are in a similar situation with grub2, and
> there are only ever at most two "platforms" we can build, and it does
> not look like it will ever change:

Nope, that is not correct.

>     i386:
>         BR2_TARGET_GRUB2_I386_PC
>         BR2_TARGET_GRUB2_I386_EFI
> 
>     x86_64:
>         BR2_TARGET_GRUB2_I386_EFI
>         BR2_TARGET_GRUB2_X86_64_EFI

On x86-64, we can have:

	BR2_TARGET_GRUB2_I386_PC
	BR2_TARGET_GRUB2_I386_EFI
	BR2_TARGET_GRUB2_X86_64_EFI

to cover legacy BIOS platforms (32-bit), 32-bit EFI BIOS and 64-bit EFI
BIOS. Dans this is precisely the use-case we are in fact interested in.

So that means we would need 3 Grub packages, which is the reason why it
felt much more reasonable to just extend the grub2 package itself.

> >  GRUB2_CONF_OPTS = \
> > -	--target=$(GRUB2_TARGET) \
> > -	--with-platform=$(GRUB2_PLATFORM) \
> > +	--host=$(GNU_TARGET_NAME) \
> > +	--build=$(GNU_HOST_NAME) \
> > +	--target=$(GRUB2_$(call UPPERCASE,$(tuple))_TARGET) \
> > +	--with-platform=$(GRUB2_$(call UPPERCASE,$(tuple))_PLATFORM) \  
> 
>     --target=$(GRUB2_TARGET_$(tuple))
>     --with-platform=$(GRUB2_PLATFORM_$(tuple)) \
> 
> However, I don't like macros that are implicitly parameterised. We've
> gone in quite some changes in the past to fix similar cases (e.g. in the
> download infra).

Ah, yes, I missed that during the review, indeed the fact that we
reference $(tuple) in GRUB2_CONF_OPTS without it being an argument is a
bit annoying.

> So, I'd write a real parameterised marco:
> 
>     # $(1): tuple blablabla...
>     GRUB2_CONF_OPTS = \
>         --host=$(GNU_TARGET_NAME) \
>         --target=$(GRUB2_TARGET_$(1)) \
>         --with-platform=$(GRUB2_PLATFORM_$(1)) \
>         [...]
> 
>     $(foreach tuple, $(GRUB2_TUPLES_y), \
>         [...] \
>         ../configure $(call GRUB2_CONF_OPTS,$(tuple))

Or, just do:

	  ../configure $(GRUB2_CONF_OPTS) \
		--target=... \
		--with-platform=...

I.e, keep out of GRUB2_CONF_OPTS the options that are parameterized.


> > -$(eval $(autotools-package))
> > +$(eval $(generic-package))  
> 
> This is my main problem: we're mostly duplicating bits and pieces of the
> autotolls-package infra, which means we *will* fall behind very quickly
> when we have to update said infra, and those changes will not percolate
> to this grub2 package.

That is true, but on the other hand, grub2 is not the typical
user-space autotools package either.

See the stack of crap that we have to pass to make it work:

GRUB2_CONF_ENV = \
        CPP="$(TARGET_CC) -E" \
        TARGET_CC="$(TARGET_CC)" \
        CFLAGS="$(TARGET_CFLAGS) -Os" \
        TARGET_CFLAGS="$(TARGET_CFLAGS) -Os" \
        CPPFLAGS="$(TARGET_CPPFLAGS) -Os -fno-stack-protector" \
        TARGET_CPPFLAGS="$(TARGET_CPPFLAGS) -Os -fno-stack-protector" \
        TARGET_LDFLAGS="$(TARGET_LDFLAGS) -Os" \
        TARGET_NM="$(TARGET_NM)" \
        TARGET_OBJCOPY="$(TARGET_OBJCOPY)" \
        TARGET_STRIP="$(TARGET_CROSS)strip"

GRUB2_CONF_OPTS = \
        --target=$(GRUB2_TARGET) \
        --with-platform=$(GRUB2_PLATFORM) \
        --prefix=/ \
        --exec-prefix=/ \
        --disable-grub-mkfont \
        --enable-efiemu=no \
        ac_cv_lib_lzma_lzma_code=no \
        --enable-device-mapper=no \
        --enable-libzfs=no \
        --disable-werror

Including the passing of --target, custom --prefix, custom
--exec-prefix.

So even though we do use autotools-package, it's not like we don't
already have to play quite a few tricks to get it to do the right thing
with the weirdness of grub2.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Yann E. MORIN]

Thomas, Köry, All,

On 2021-09-18 10:50 +0200, Thomas Petazzoni spake thusly:
> On Fri, 17 Sep 2021 23:24:29 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> > What you forgot to state in bold fat characters here, is that we can no
> > longer use autotools-package as a consequence of this multi-build, and
> > that we have to resort to generic-pacakge and a partial duplication of
> > the autotools-infra.
[--SNIP--]
> > If I understand correctly, we are in a similar situation with grub2, and
> > there are only ever at most two "platforms" we can build, and it does
> > not look like it will ever change:
[--SNIP--]
> >     x86_64:
> >         BR2_TARGET_GRUB2_I386_EFI
> >         BR2_TARGET_GRUB2_X86_64_EFI
> On x86-64, we can have:
> 	BR2_TARGET_GRUB2_I386_PC
> 	BR2_TARGET_GRUB2_I386_EFI
> 	BR2_TARGET_GRUB2_X86_64_EFI
> 
> to cover legacy BIOS platforms (32-bit), 32-bit EFI BIOS and 64-bit EFI
> BIOS. Dans this is precisely the use-case we are in fact interested in.
> 
> So that means we would need 3 Grub packages, which is the reason why it
> felt much more reasonable to just extend the grub2 package itself.

Arg, I indeed missed the BR2_TARGET_GRUB2_I386_PC for x86-64, which
indeed basically invalidates the two-package solution, and the
three-package alternative is not a viable solution either...

[--SNIP--]
> > > -$(eval $(autotools-package))
> > > +$(eval $(generic-package))  
> > This is my main problem: we're mostly duplicating bits and pieces of the
> > autotolls-package infra, which means we *will* fall behind very quickly
> > when we have to update said infra, and those changes will not percolate
> > to this grub2 package.
> That is true, but on the other hand, grub2 is not the typical
> user-space autotools package either.
[--SNIP--]
> So even though we do use autotools-package, it's not like we don't
> already have to play quite a few tricks to get it to do the right thing
> with the weirdness of grub2.

That's true.

Köry: when you respin, in addition to the other points, can you extend
the commit log with a summary of the explanations from Thomas, on why a
multi-package solution like barebox is not viable, and that grub2 is
already pretty special anyway, please?

Thanks!

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

Re: [Buildroot] [PATCH 4/8] fs/iso9660: add support to Grub EFI bootloader in the image

[Yann E. MORIN]

Köry, All,

On 2021-09-14 11:34 +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.

I am not sure I got that last sentence. Did you refer to using 'cd' or
'root=(cd0)' as explain in the config help?

Also see further review below...

> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
>  fs/iso9660/Config.in  | 30 ++++++++++++++++++++++++------
>  fs/iso9660/iso9660.mk | 33 ++++++++++++++++++++++++++++++---
>  2 files changed, 54 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 f80b735858..4e4c5de1d0 100644
> --- a/fs/iso9660/iso9660.mk
> +++ b/fs/iso9660/iso9660.mk
> @@ -57,15 +57,31 @@ else
>  ROOTFS_ISO9660_TMP_TARGET_DIR = $(TARGET_DIR)
>  endif
>  
> -ifeq ($(BR2_TARGET_ROOTFS_ISO9660_GRUB2),y)
> +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
>  ROOTFS_ISO9660_BOOT_IMAGE = boot/grub/grub-eltorito.img
>  define ROOTFS_ISO9660_INSTALL_BOOTLOADER
> +	mkdir -p $(dir ROOTFS_ISO9660_BOOT_IMAGE)

This looks really dubious. The CWD is always Buidroot's top directory,
so this would create boot/grub/ in Buildroot's tree. Weird...

>  	$(INSTALL) -D -m 0644 $(BINARIES_DIR)/grub-eltorito.img \
>  		$(ROOTFS_ISO9660_TMP_TARGET_DIR)/boot/grub/grub-eltorito.img

... as this directory should normally be created with this "install -D".

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

We probably want to use --invariant with BR2_REPRODUCIBLE...

> +	$(HOST_DIR)/bin/mcopy -i $(ROOTFS_ISO9660_EFI_PARTITION_PATH) -s \
> +		$(ROOTFS_ISO9660_EFI_PARTITION_CONTENT)/* ::/

We may also need at least -m (Preserve the file modification time), and
maybe also -p (Preserves the attributes of the copied files), again with
BR2_REPRODUCIBLE (or always).

> +endef
>  else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_ISOLINUX),y)
>  ROOTFS_ISO9660_DEPENDENCIES += syslinux
>  ROOTFS_ISO9660_BOOTLOADER_CONFIG_PATH = \
> @@ -128,9 +144,20 @@ 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_XORRISO_BOOTLOADER_OPTS = \
> +	-J -R -b $(ROOTFS_ISO9660_BOOT_IMAGE) \
> +	$(ROOTFS_ISO9660_XORRISO_BOOTLOADER_COMMON_OPTS) \
> +	-boot-load-size 4 -boot-info-table -no-emul-boot

No need to introduce another variable, just use the existing one (notice
I have renamed it when applying previous patches) (I also tend to prefer
cionstant options first, then configurable ones last, unless order is
important), and for multi-line assignments, put one "option" per line:

    ROOTFS_ISO9660_OPTS += \
        -J \
        -R \
        -boot-load-size 4 \
        -boot-info-table \
        -no-emul-boot \
        -b $(ROOTFS_ISO9660_BOOT_IMAGE)

ROOTFS_ISO9660_XORRISO_BOOTLOADER_COMMON_OPTS is never set, and indeed
you removed it in patch 5. Just do not introduce it to begin with, I
guess? ;-)

> +else ifeq ($(BR2_TARGET_ROOTFS_ISO9660_EFI_BOOTLOADER),y)
> +ROOTFS_ISO9660_XORRISO_BOOTLOADER_OPTS = \
> +	--efi-boot $(ROOTFS_ISO9660_EFI_PARTITION) \
> +	-no-emul-boot

Ditto.

> +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_XORRISO_BOOTLOADER_OPTS) \
>  		$(ROOTFS_ISO9660_XORRISO_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

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