[PATCH] RFC: Support an EFI-loader bootflow

[PATCH] RFC: Support an EFI-loader bootflow

[Simon Glass]

This is just a demonstration of how to support EFI loader using bootflow.
Various things need cleaning up, not least that the naming needs to be
finalised. I will deal with that in the v2 series.

In order to support multiple methods of booting from the same device, we
should probably separate out the different implementations (syslinux,
EFI loader and soon bootmgr, Chromium OS, Android, VBE) into pluggable
drivers and number them as we do with partitions. For now the sequence
number is used to determine both the partition number and the
implementation to use.

The same boot command is used as before ('bootflow scan -lb') so there is
no change to that. It can boot both Fedora 31 and 34, for example.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
See u-boot-dm/bmea for the tree containing this patch and the series
that it relies on:

  https://patchwork.ozlabs.org/project/uboot/list/?series=258654&state=*

As an aside, the hack to call efi_set_bootdev() provides another example
of why the EFI implementation should have been written using driver model,
instead of independently of it. I hope that someone can take up this
challenge and reduce the amount of duplication between the EFI
implementation and the rest of U-Boot.

Some relevant threads on that are below. The first two show (I believe)
why this is was all so unnecessary if it had been done correctly from the
start:

https://lists.denx.de/pipermail/u-boot/2016-May/254804.html

https://lists.denx.de/pipermail/u-boot/2016-August/263501.html

http://patchwork.ozlabs.org/project/uboot/patch/1471374529-61610-2-git-send-email-agraf@suse.de/#1433754

https://lists.denx.de/pipermail/u-boot/2019-March/362190.html

https://yhbt.net/lore/all/20210628134827.GA9516@bill-the-cat/

https://lists.denx.de/pipermail/u-boot/2018-February/321463.html

Sample log on rpi_3_32b:

U-Boot 2021.10-rc2-00043-gccd453aa918-dirty (Aug 28 2021 - 13:58:46 -0600)

DRAM:  992 MiB
RPI 3 Model B (0xa22082)
MMC:   mmc@7e202000: 0, sdhci@7e300000: 1
Loading Environment from FAT... Unable to read "uboot.env" from mmc0:1... In:    serial
Out:   vidconsole
Err:   vidconsole
Net:   No ethernet found.
starting USB...
Bus usb@7e980000: USB DWC2
scanning bus usb@7e980000 for devices... usb_kbd usb_kbd: Timeout poll on interrupt endpoint
Failed to get keyboard state from device 0c40:8000
4 USB Device(s) found
       scanning usb for storage devices... 0 Storage Device(s) found
Hit any key to stop autoboot:  0
Scanning for bootflows in all bootmethods
Seq  Type         State   Uclass    Part  Name                      Filename
---  -----------  ------  --------  ----  ------------------------  ----------------
Scanning bootmethod 'mmc@7e202000.bootmethod':
  0  efi-loader   loaded  mmc          1  mmc@7e202000.bootmethod.p efi/boot/bootarm.efi
** Booting bootflow 'mmc@7e202000.bootmethod.part_1'
Scanning disk mmc@7e202000.blk...
** Unrecognized filesystem type **
Card did not respond to voltage select! : -110
Scanning disk sdhci@7e300000.blk...
Disk sdhci@7e300000.blk not ready
Found 4 disks
No EFI system partition
Booting /efi\boot
Waiting for Ethernet connection... done.

      Fedora (5.11.12-300.fc34.armv7hl) 34 (Workstation Edition)
      UEFI Firmware Settings

      Use the ▲ and ▼ keys to change the selection.
      Press 'e' to edit the selected item, or 'c' for a command prompt. Press Escape to return to the previous menu.
   The selected entry will be started automatically in 0s.

 boot/Kconfig         |  21 +++++++
 boot/Makefile        |   1 +
 boot/bootmethod.c    |  73 ++++++++++++++++++----
 boot/efiloader.c     | 141 +++++++++++++++++++++++++++++++++++++++++++
 include/bm_efi.h     |  42 +++++++++++++
 include/bootmethod.h |   1 +
 6 files changed, 266 insertions(+), 13 deletions(-)
 create mode 100644 boot/efiloader.c
 create mode 100644 include/bm_efi.h

diff --git a/boot/Kconfig b/boot/Kconfig
index a1beb182f60..6339ace9413 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -310,6 +310,27 @@ config BOOTMETHOD_DISTRO
 
 	  This provides a way to try out bootmethod on an existing boot flow.
 
+config BOOTMETHOD_EFILOADER
+	bool "Bootmethod support for EFI boot"
+	depends on BOOTMETHOD && EFI_LOADER
+	default y
+	help
+	  Enables support for EFI boot using bootmethods. This makes the
+	  bootmethods look for a 'boot<arch>.efi' on each filesystem
+	  they scan. The resulting file is booted after enabling U-Boot's
+	  EFI loader support.
+
+	  The <arch> depends on the architecture of the board:
+
+	     aa64      - aarch64 (ARM 64-bit)
+	     arm       - ARM 32-bit
+	     ia32      - x86 32-bit
+	     x64       - x86 64-bit
+	     riscv32   - RISC-V 32-bit
+	     riscv64   - RISC-V 64-bit
+
+	  This provides a way to try out bootmethod on an existing boot flow.
+
 config LEGACY_IMAGE_FORMAT
 	bool "Enable support for the legacy image format"
 	default y if !FIT_SIGNATURE
diff --git a/boot/Makefile b/boot/Makefile
index 4ce721242b0..7a4e882a805 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_ANDROID_AB) += android_ab.o
 obj-$(CONFIG_ANDROID_BOOT_IMAGE) += image-android.o image-android-dt.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETHOD) += bootmethod.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTMETHOD_DISTRO) += distro.o
+obj-$(CONFIG_$(SPL_TPL_)BOOTMETHOD_EFILOADER) += efiloader.o
 obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += image-fdt.o
 obj-$(CONFIG_$(SPL_TPL_)FIT_SIGNATURE) += fdt_region.o
 obj-$(CONFIG_$(SPL_TPL_)FIT) += image-fit.o
diff --git a/boot/bootmethod.c b/boot/bootmethod.c
index b9752f75e54..33b3c7d4a39 100644
--- a/boot/bootmethod.c
+++ b/boot/bootmethod.c
@@ -9,6 +9,7 @@
 #include <bootmethod.h>
 #include <distro.h>
 #include <dm.h>
+#include <bm_efi.h>
 #include <fs.h>
 #include <log.h>
 #include <malloc.h>
@@ -17,12 +18,15 @@
 #include <dm/uclass-internal.h>
 
 enum {
+	/* So far we only support distroboot and EFI_LOADER */
+	MAX_BOOTMETHODS			= 2,
+
 	/*
 	 * Set some sort of limit on the number of bootflows a bootmethod can
 	 * return. Note that for disks this limits the partitions numbers that
-	 * are scanned to 1..MAX_BOOTFLOWS_PER_BOOTMETHOD
+	 * are scanned to 1..MAX_BOOTFLOWS_PER_BOOTMETHOD / MAX_BOOTMETHODS
 	 */
-	MAX_BOOTFLOWS_PER_BOOTMETHOD	= 20,
+	MAX_BOOTFLOWS_PER_BOOTMETHOD	= 20 * MAX_BOOTMETHODS,
 };
 
 static const char *const bootmethod_state[BOOTFLOWST_COUNT] = {
@@ -36,6 +40,7 @@ static const char *const bootmethod_state[BOOTFLOWST_COUNT] = {
 
 static const char *const bootmethod_type[BOOTFLOWT_COUNT] = {
 	"distro-boot",
+	"efi-loader",
 };
 
 int bootmethod_get_state(struct bootflow_state **statep)
@@ -279,14 +284,23 @@ int bootmethod_scan_next_bootflow(struct bootmethod_iter *iter,
 
 		/*
 		 * Unless there are no more partitions or no bootflow support,
-		 * try the next partition
+		 * try the next partition. If we run out of partitions, fall
+		 * through to select the next device.
 		 */
 		else if (ret != -ESHUTDOWN && ret != -ENOSYS) {
 			log_debug("Bootmethod '%s' seq %d: Error %d\n",
 				  dev->name, iter->seq, ret);
-			if ((iter->seq++ != MAX_BOOTFLOWS_PER_BOOTMETHOD) &&
-			    (iter->flags & BOOTFLOWF_ALL))
-				return log_msg_ret("all", ret);
+			if (iter->seq++ != MAX_BOOTFLOWS_PER_BOOTMETHOD) {
+				/*
+				 * For 'all' we return all bootflows, even
+				 * those with errors
+				 */
+				if (iter->flags & BOOTFLOWF_ALL)
+					return log_msg_ret("all", ret);
+
+				/* Try the next partition */
+				continue;
+			}
 		}
 
 		/* we got to the end of that bootmethod, try the next */
@@ -327,12 +341,19 @@ int bootmethod_find_in_blk(struct udevice *dev, struct udevice *blk, int seq,
 {
 	struct blk_desc *desc = dev_get_uclass_plat(blk);
 	struct disk_partition info;
+	bool done = false;
 	char name[60];
-	int partnum = seq + 1;
+
+	/*
+	 * TODO(sjg@chromium.org): Add a suitable parameter for the method
+	 * number. Needs to consider the renaming suggested in the cover letter
+	 */
+	int methodnum = seq % MAX_BOOTMETHODS;
+	int partnum = seq / MAX_BOOTMETHODS + 1;
 	int ret;
 
 	if (seq >= MAX_BOOTFLOWS_PER_BOOTMETHOD)
-		return -ESHUTDOWN;
+		return log_msg_ret("max", -ESHUTDOWN);
 
 	bflow->blk = blk;
 	bflow->seq = seq;
@@ -344,7 +365,11 @@ int bootmethod_find_in_blk(struct udevice *dev, struct udevice *blk, int seq,
 	bflow->state = BOOTFLOWST_BASE;
 	ret = part_get_info(desc, partnum, &info);
 
-	/* This error indicates the media is not present */
+	/*
+	 * This error indicates the media is not present. Otherwise we just
+	 * blindly scan the next partition. We could be more intelligent here
+	 * and check which partition numbers actually exist.
+	 */
 	if (ret != -EOPNOTSUPP)
 		bflow->state = BOOTFLOWST_MEDIA;
 	if (ret)
@@ -362,11 +387,27 @@ int bootmethod_find_in_blk(struct udevice *dev, struct udevice *blk, int seq,
 
 	bflow->state = BOOTFLOWST_FS;
 
-	if (CONFIG_IS_ENABLED(BOOTMETHOD_DISTRO)) {
-		ret = distro_boot_setup(desc, partnum, bflow);
-		if (ret)
-			return log_msg_ret("distro", ret);
+	switch (methodnum) {
+	case 0:
+		if (CONFIG_IS_ENABLED(BOOTMETHOD_DISTRO)) {
+			done = true;
+			ret = distro_boot_setup(desc, partnum, bflow);
+			if (ret)
+				return log_msg_ret("distro", ret);
+		}
+		break;
+
+	case 1:
+		if (CONFIG_IS_ENABLED(BOOTMETHOD_EFILOADER)) {
+			done = true;
+			ret = efiloader_boot_setup(desc, partnum, bflow);
+			if (ret)
+				return log_msg_ret("efi_loader", ret);
+		}
+		break;
 	}
+	if (!done)
+		return log_msg_ret("supp", -ENOTSUPP);
 
 	return 0;
 }
@@ -386,6 +427,12 @@ int bootflow_boot(struct bootflow *bflow)
 			ret = distro_boot(bflow);
 		}
 		break;
+	case BOOTFLOWT_EFILOADER:
+		if (CONFIG_IS_ENABLED(BOOTMETHOD_EFILOADER)) {
+			done = true;
+			ret = efiloader_boot(bflow);
+		}
+		break;
 	case BOOTFLOWT_COUNT:
 		break;
 	}
diff --git a/boot/efiloader.c b/boot/efiloader.c
new file mode 100644
index 00000000000..7e874bc8134
--- /dev/null
+++ b/boot/efiloader.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * EFI loader implementation for bootflow
+ *
+ * Copyright 2021 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <common.h>
+#include <blk.h>
+#include <bootmethod.h>
+#include <command.h>
+#include <distro.h>
+#include <dm.h>
+#include <efi_loader.h>
+#include <fs.h>
+#include <malloc.h>
+#include <mapmem.h>
+#include <net.h>
+#include <pxe_utils.h>
+#include <vsprintf.h>
+
+/* This could be written in C perhaps - taken from config_distro_bootcmd.h */
+#if defined(CONFIG_ARM64)
+#define BOOTEFI_NAME "bootaa64.efi"
+#elif defined(CONFIG_ARM)
+#define BOOTEFI_NAME "bootarm.efi"
+#elif defined(CONFIG_X86_RUN_32BIT)
+#define BOOTEFI_NAME "bootia32.efi"
+#elif defined(CONFIG_X86_RUN_64BIT)
+#define BOOTEFI_NAME "bootx64.efi"
+#elif defined(CONFIG_ARCH_RV32I)
+#define BOOTEFI_NAME "bootriscv32.efi"
+#elif defined(CONFIG_ARCH_RV64I)
+#define BOOTEFI_NAME "bootriscv64.efi"
+#elif defined(CONFIG_SANDBOX)
+#define BOOTEFI_NAME "bootsbox.efi"
+#else
+#error "Not supported for this architecture"
+#endif
+
+#define EFI_FNAME	"efi/boot/" BOOTEFI_NAME
+
+static int efiload_read_file(struct blk_desc *desc, int partnum,
+			     struct bootflow *bflow)
+{
+	const struct udevice *media_dev;
+	int size = bflow->size;
+	char devnum_str[9];
+	char dirname[200];
+	loff_t bytes_read;
+	char *last_slash;
+	ulong addr;
+	char *buf;
+	int ret;
+
+	/* Sadly FS closes the file after fs_size() so we must redo this */
+	ret = fs_set_blk_dev_with_part(desc, partnum);
+	if (ret)
+		return log_msg_ret("set", ret);
+
+	buf = malloc(size + 1);
+	if (!buf)
+		return log_msg_ret("buf", -ENOMEM);
+	addr = map_to_sysmem(buf);
+
+	ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
+	if (ret) {
+		free(buf);
+		return log_msg_ret("read", ret);
+	}
+	if (size != bytes_read)
+		return log_msg_ret("bread", -EINVAL);
+	buf[size] = '\0';
+	bflow->state = BOOTFLOWST_LOADED;
+	bflow->buf = buf;
+
+	/*
+	 * This is a horrible hack to tell EFI about this boot device. Once we
+	 * unify EFI with the rest of U-Boot we can clean this up. The same hack
+	 * exists in multiple places, e.g. in the fs, tftp and load commands.
+	 *
+	 * Once we can clean up the EFI code to make proper use of driver model,
+	 * this can go away.
+	 */
+	media_dev = dev_get_parent(bflow->dev);
+	snprintf(devnum_str, sizeof(devnum_str), "%x", dev_seq(media_dev));
+
+	strlcpy(dirname, bflow->fname, sizeof(dirname));
+	last_slash = strrchr(dirname, '/');
+	if (last_slash)
+		*last_slash = '\0';
+
+	efi_set_bootdev(dev_get_uclass_name(media_dev), devnum_str, dirname,
+			bflow->buf, size);
+
+	return 0;
+}
+
+int efiloader_boot_setup(struct blk_desc *desc, int partnum,
+			 struct bootflow *bflow)
+{
+	loff_t size;
+	int ret;
+
+	bflow->type = BOOTFLOWT_EFILOADER;
+	bflow->fname = strdup(EFI_FNAME);
+	if (!bflow->fname)
+		return log_msg_ret("name", -ENOMEM);
+	ret = fs_size(bflow->fname, &size);
+	bflow->size = size;
+	if (ret)
+		return log_msg_ret("size", ret);
+	bflow->state = BOOTFLOWST_FILE;
+	log_debug("   - distro file size %x\n", (uint)size);
+	if (size > 0x2000000)
+		return log_msg_ret("chk", -E2BIG);
+
+	ret = efiload_read_file(desc, partnum, bflow);
+	if (ret)
+		return log_msg_ret("read", -EINVAL);
+
+	return 0;
+}
+
+int efiloader_boot(struct bootflow *bflow)
+{
+	char cmd[50];
+
+	/*
+	 * At some point we can add a real interface to bootefi so we can call
+	 * this directly. For now, go through the CLI like distro boot.
+	 */
+	snprintf(cmd, sizeof(cmd), "bootefi %lx %lx",
+		 (ulong)map_to_sysmem(bflow->buf),
+		 (ulong)map_to_sysmem(gd->fdt_blob));
+	if (run_command(cmd, 0))
+		return log_msg_ret("run", -EINVAL);
+
+	return 0;
+}
diff --git a/include/bm_efi.h b/include/bm_efi.h
new file mode 100644
index 00000000000..836b2c17f22
--- /dev/null
+++ b/include/bm_efi.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2021 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#ifndef __bootmethod_efi_h
+#define __bootmethod_efi_h
+
+struct blk_desc;
+
+/**
+ * efiloader_boot_setup() - Set up a bootflow for EFI boot from a block device
+ *
+ * This fills out a bootflow for a particular boot device and partition. It
+ * scans for a filesystem and suitable file, updating the bootflow accordingly.
+ *
+ * This sets the following fields in @bflow:
+ *
+ *	type, size, fname, state, subdir, buf
+ *
+ * The caller mast have already set the other fields.
+ *
+ * @desc: Block-device descriptor
+ * @partnum: Partition number (1..)
+ * @bflow: Partial bootflow to be completed by this function
+ * @return 0 on success (bootflow got to 'loaded' state), -ve on error
+ */
+int efiloader_boot_setup(struct blk_desc *desc, int partnum,
+			 struct bootflow *bflow);
+
+/**
+ * efiloader_boot() - Boot an EFI binary
+ *
+ * Boots a bootflow of type BOOTFLOWT_EFI_LOADER. This boots an EFI application
+ * which takes care of the boot from then on.
+ *
+ * @bflow: Bootflow to boot
+ */
+int efiloader_boot(struct bootflow *bflow);
+
+#endif
diff --git a/include/bootmethod.h b/include/bootmethod.h
index d80be556b8a..d6cc486c43c 100644
--- a/include/bootmethod.h
+++ b/include/bootmethod.h
@@ -25,6 +25,7 @@ enum bootflow_state_t {
 
 enum bootflow_type_t {
 	BOOTFLOWT_DISTRO,	/**< Distro boot */
+	BOOTFLOWT_EFILOADER,	/**< EFI loader boot */
 
 	BOOTFLOWT_COUNT,
 };
-- 
2.33.0.259.gc128427fd7-goog

Re: [PATCH] RFC: Support an EFI-loader bootflow

[AKASHI Takahiro]

Simon,

On Sat, Aug 28, 2021 at 02:35:21PM -0600, Simon Glass wrote:
> This is just a demonstration of how to support EFI loader using bootflow.
> Various things need cleaning up, not least that the naming needs to be
> finalised. I will deal with that in the v2 series.
> 
> In order to support multiple methods of booting from the same device, we
> should probably separate out the different implementations (syslinux,
> EFI loader

I still believe that we'd better add "removable media" support
to UEFI boot manager first (and then probably call this functionality
from bootflow?).

I admit that, in this case, we will have an issue that we will not
recognize any device which is plugged in dynamically after UEFI
subsystem is initialized. But this issue will potentially exist 
even with your approach.

> and soon bootmgr,

What will you expect in UEFI boot manager case?
Boot parameters (options) as well as the boot order are well defined
by BootXXXX and BootOrder variables. How are they fit into your scheme?

But anyway, we can use the following commands to run a specific
boot flow in UEFI world:
=> efidebug boot next 1(or whatever else); bootefi bootmgr

> Chromium OS, Android, VBE) into pluggable
> drivers and number them as we do with partitions. For now the sequence
> number is used to determine both the partition number and the
> implementation to use.
> 
> The same boot command is used as before ('bootflow scan -lb') so there is
> no change to that. It can boot both Fedora 31 and 34, for example.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> See u-boot-dm/bmea for the tree containing this patch and the series
> that it relies on:
> 
>   https://patchwork.ozlabs.org/project/uboot/list/?series=258654&state=*
> 
> As an aside, the hack to call efi_set_bootdev() provides another example
> of why the EFI implementation should have been written using driver model,
> instead of independently of it. I hope that someone can take up this
> challenge and reduce the amount of duplication between the EFI
> implementation and the rest of U-Boot.
> 
> Some relevant threads on that are below. The first two show (I believe)
> why this is was all so unnecessary if it had been done correctly from the
> start:
> 
> https://lists.denx.de/pipermail/u-boot/2016-May/254804.html
> 
> https://lists.denx.de/pipermail/u-boot/2016-August/263501.html
> 
> http://patchwork.ozlabs.org/project/uboot/patch/1471374529-61610-2-git-send-email-agraf@suse.de/#1433754
> 
> https://lists.denx.de/pipermail/u-boot/2019-March/362190.html
> 
> https://yhbt.net/lore/all/20210628134827.GA9516@bill-the-cat/
> 
> https://lists.denx.de/pipermail/u-boot/2018-February/321463.html
> 
> Sample log on rpi_3_32b:
> 
> U-Boot 2021.10-rc2-00043-gccd453aa918-dirty (Aug 28 2021 - 13:58:46 -0600)
> 
> DRAM:  992 MiB
> RPI 3 Model B (0xa22082)
> MMC:   mmc@7e202000: 0, sdhci@7e300000: 1
> Loading Environment from FAT... Unable to read "uboot.env" from mmc0:1... In:    serial
> Out:   vidconsole
> Err:   vidconsole
> Net:   No ethernet found.
> starting USB...
> Bus usb@7e980000: USB DWC2
> scanning bus usb@7e980000 for devices... usb_kbd usb_kbd: Timeout poll on interrupt endpoint
> Failed to get keyboard state from device 0c40:8000
> 4 USB Device(s) found
>        scanning usb for storage devices... 0 Storage Device(s) found
> Hit any key to stop autoboot:  0
> Scanning for bootflows in all bootmethods
> Seq  Type         State   Uclass    Part  Name                      Filename
> ---  -----------  ------  --------  ----  ------------------------  ----------------
> Scanning bootmethod 'mmc@7e202000.bootmethod':
>   0  efi-loader   loaded  mmc          1  mmc@7e202000.bootmethod.p efi/boot/bootarm.efi
> ** Booting bootflow 'mmc@7e202000.bootmethod.part_1'
> Scanning disk mmc@7e202000.blk...
> ** Unrecognized filesystem type **
> Card did not respond to voltage select! : -110
> Scanning disk sdhci@7e300000.blk...
> Disk sdhci@7e300000.blk not ready
> Found 4 disks
> No EFI system partition
> Booting /efi\boot
> Waiting for Ethernet connection... done.
> 
>       Fedora (5.11.12-300.fc34.armv7hl) 34 (Workstation Edition)
>       UEFI Firmware Settings
> 
>       Use the ▲ and ▼ keys to change the selection.
>       Press 'e' to edit the selected item, or 'c' for a command prompt. Press Escape to return to the previous menu.
>    The selected entry will be started automatically in 0s.
> 
>  boot/Kconfig         |  21 +++++++
>  boot/Makefile        |   1 +
>  boot/bootmethod.c    |  73 ++++++++++++++++++----
>  boot/efiloader.c     | 141 +++++++++++++++++++++++++++++++++++++++++++
>  include/bm_efi.h     |  42 +++++++++++++
>  include/bootmethod.h |   1 +
>  6 files changed, 266 insertions(+), 13 deletions(-)
>  create mode 100644 boot/efiloader.c
>  create mode 100644 include/bm_efi.h
> 
> diff --git a/boot/Kconfig b/boot/Kconfig
> index a1beb182f60..6339ace9413 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -310,6 +310,27 @@ config BOOTMETHOD_DISTRO
>  
>  	  This provides a way to try out bootmethod on an existing boot flow.
>  
> +config BOOTMETHOD_EFILOADER
> +	bool "Bootmethod support for EFI boot"
> +	depends on BOOTMETHOD && EFI_LOADER
> +	default y
> +	help
> +	  Enables support for EFI boot using bootmethods. This makes the
> +	  bootmethods look for a 'boot<arch>.efi' on each filesystem
> +	  they scan. The resulting file is booted after enabling U-Boot's
> +	  EFI loader support.
> +
> +	  The <arch> depends on the architecture of the board:
> +
> +	     aa64      - aarch64 (ARM 64-bit)
> +	     arm       - ARM 32-bit
> +	     ia32      - x86 32-bit
> +	     x64       - x86 64-bit
> +	     riscv32   - RISC-V 32-bit
> +	     riscv64   - RISC-V 64-bit
> +
> +	  This provides a way to try out bootmethod on an existing boot flow.
> +
>  config LEGACY_IMAGE_FORMAT
>  	bool "Enable support for the legacy image format"
>  	default y if !FIT_SIGNATURE
> diff --git a/boot/Makefile b/boot/Makefile
> index 4ce721242b0..7a4e882a805 100644
> --- a/boot/Makefile
> +++ b/boot/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_ANDROID_AB) += android_ab.o
>  obj-$(CONFIG_ANDROID_BOOT_IMAGE) += image-android.o image-android-dt.o
>  obj-$(CONFIG_$(SPL_TPL_)BOOTMETHOD) += bootmethod.o
>  obj-$(CONFIG_$(SPL_TPL_)BOOTMETHOD_DISTRO) += distro.o
> +obj-$(CONFIG_$(SPL_TPL_)BOOTMETHOD_EFILOADER) += efiloader.o
>  obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += image-fdt.o
>  obj-$(CONFIG_$(SPL_TPL_)FIT_SIGNATURE) += fdt_region.o
>  obj-$(CONFIG_$(SPL_TPL_)FIT) += image-fit.o
> diff --git a/boot/bootmethod.c b/boot/bootmethod.c
> index b9752f75e54..33b3c7d4a39 100644
> --- a/boot/bootmethod.c
> +++ b/boot/bootmethod.c
> @@ -9,6 +9,7 @@
>  #include <bootmethod.h>
>  #include <distro.h>
>  #include <dm.h>
> +#include <bm_efi.h>
>  #include <fs.h>
>  #include <log.h>
>  #include <malloc.h>
> @@ -17,12 +18,15 @@
>  #include <dm/uclass-internal.h>
>  
>  enum {
> +	/* So far we only support distroboot and EFI_LOADER */
> +	MAX_BOOTMETHODS			= 2,
> +
>  	/*
>  	 * Set some sort of limit on the number of bootflows a bootmethod can
>  	 * return. Note that for disks this limits the partitions numbers that
> -	 * are scanned to 1..MAX_BOOTFLOWS_PER_BOOTMETHOD
> +	 * are scanned to 1..MAX_BOOTFLOWS_PER_BOOTMETHOD / MAX_BOOTMETHODS
>  	 */
> -	MAX_BOOTFLOWS_PER_BOOTMETHOD	= 20,
> +	MAX_BOOTFLOWS_PER_BOOTMETHOD	= 20 * MAX_BOOTMETHODS,
>  };
>  
>  static const char *const bootmethod_state[BOOTFLOWST_COUNT] = {
> @@ -36,6 +40,7 @@ static const char *const bootmethod_state[BOOTFLOWST_COUNT] = {
>  
>  static const char *const bootmethod_type[BOOTFLOWT_COUNT] = {
>  	"distro-boot",
> +	"efi-loader",
>  };
>  
>  int bootmethod_get_state(struct bootflow_state **statep)
> @@ -279,14 +284,23 @@ int bootmethod_scan_next_bootflow(struct bootmethod_iter *iter,
>  
>  		/*
>  		 * Unless there are no more partitions or no bootflow support,
> -		 * try the next partition
> +		 * try the next partition. If we run out of partitions, fall
> +		 * through to select the next device.
>  		 */
>  		else if (ret != -ESHUTDOWN && ret != -ENOSYS) {
>  			log_debug("Bootmethod '%s' seq %d: Error %d\n",
>  				  dev->name, iter->seq, ret);
> -			if ((iter->seq++ != MAX_BOOTFLOWS_PER_BOOTMETHOD) &&
> -			    (iter->flags & BOOTFLOWF_ALL))
> -				return log_msg_ret("all", ret);
> +			if (iter->seq++ != MAX_BOOTFLOWS_PER_BOOTMETHOD) {
> +				/*
> +				 * For 'all' we return all bootflows, even
> +				 * those with errors
> +				 */
> +				if (iter->flags & BOOTFLOWF_ALL)
> +					return log_msg_ret("all", ret);
> +
> +				/* Try the next partition */
> +				continue;
> +			}
>  		}
>  
>  		/* we got to the end of that bootmethod, try the next */
> @@ -327,12 +341,19 @@ int bootmethod_find_in_blk(struct udevice *dev, struct udevice *blk, int seq,
>  {
>  	struct blk_desc *desc = dev_get_uclass_plat(blk);
>  	struct disk_partition info;
> +	bool done = false;
>  	char name[60];
> -	int partnum = seq + 1;
> +
> +	/*
> +	 * TODO(sjg@chromium.org): Add a suitable parameter for the method
> +	 * number. Needs to consider the renaming suggested in the cover letter
> +	 */
> +	int methodnum = seq % MAX_BOOTMETHODS;
> +	int partnum = seq / MAX_BOOTMETHODS + 1;
>  	int ret;
>  
>  	if (seq >= MAX_BOOTFLOWS_PER_BOOTMETHOD)
> -		return -ESHUTDOWN;
> +		return log_msg_ret("max", -ESHUTDOWN);
>  
>  	bflow->blk = blk;
>  	bflow->seq = seq;
> @@ -344,7 +365,11 @@ int bootmethod_find_in_blk(struct udevice *dev, struct udevice *blk, int seq,
>  	bflow->state = BOOTFLOWST_BASE;
>  	ret = part_get_info(desc, partnum, &info);
>  
> -	/* This error indicates the media is not present */
> +	/*
> +	 * This error indicates the media is not present. Otherwise we just
> +	 * blindly scan the next partition. We could be more intelligent here
> +	 * and check which partition numbers actually exist.
> +	 */
>  	if (ret != -EOPNOTSUPP)
>  		bflow->state = BOOTFLOWST_MEDIA;
>  	if (ret)
> @@ -362,11 +387,27 @@ int bootmethod_find_in_blk(struct udevice *dev, struct udevice *blk, int seq,
>  
>  	bflow->state = BOOTFLOWST_FS;
>  
> -	if (CONFIG_IS_ENABLED(BOOTMETHOD_DISTRO)) {
> -		ret = distro_boot_setup(desc, partnum, bflow);
> -		if (ret)
> -			return log_msg_ret("distro", ret);
> +	switch (methodnum) {
> +	case 0:
> +		if (CONFIG_IS_ENABLED(BOOTMETHOD_DISTRO)) {
> +			done = true;
> +			ret = distro_boot_setup(desc, partnum, bflow);
> +			if (ret)
> +				return log_msg_ret("distro", ret);
> +		}
> +		break;
> +
> +	case 1:
> +		if (CONFIG_IS_ENABLED(BOOTMETHOD_EFILOADER)) {
> +			done = true;
> +			ret = efiloader_boot_setup(desc, partnum, bflow);
> +			if (ret)
> +				return log_msg_ret("efi_loader", ret);
> +		}
> +		break;
>  	}
> +	if (!done)
> +		return log_msg_ret("supp", -ENOTSUPP);
>  
>  	return 0;
>  }
> @@ -386,6 +427,12 @@ int bootflow_boot(struct bootflow *bflow)
>  			ret = distro_boot(bflow);
>  		}
>  		break;
> +	case BOOTFLOWT_EFILOADER:
> +		if (CONFIG_IS_ENABLED(BOOTMETHOD_EFILOADER)) {
> +			done = true;
> +			ret = efiloader_boot(bflow);
> +		}
> +		break;
>  	case BOOTFLOWT_COUNT:
>  		break;
>  	}
> diff --git a/boot/efiloader.c b/boot/efiloader.c
> new file mode 100644
> index 00000000000..7e874bc8134
> --- /dev/null
> +++ b/boot/efiloader.c
> @@ -0,0 +1,141 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * EFI loader implementation for bootflow
> + *
> + * Copyright 2021 Google LLC
> + * Written by Simon Glass <sjg@chromium.org>
> + */
> +
> +#include <common.h>
> +#include <blk.h>
> +#include <bootmethod.h>
> +#include <command.h>
> +#include <distro.h>
> +#include <dm.h>
> +#include <efi_loader.h>
> +#include <fs.h>
> +#include <malloc.h>
> +#include <mapmem.h>
> +#include <net.h>
> +#include <pxe_utils.h>
> +#include <vsprintf.h>
> +
> +/* This could be written in C perhaps - taken from config_distro_bootcmd.h */
> +#if defined(CONFIG_ARM64)
> +#define BOOTEFI_NAME "bootaa64.efi"
> +#elif defined(CONFIG_ARM)
> +#define BOOTEFI_NAME "bootarm.efi"
> +#elif defined(CONFIG_X86_RUN_32BIT)
> +#define BOOTEFI_NAME "bootia32.efi"
> +#elif defined(CONFIG_X86_RUN_64BIT)
> +#define BOOTEFI_NAME "bootx64.efi"
> +#elif defined(CONFIG_ARCH_RV32I)
> +#define BOOTEFI_NAME "bootriscv32.efi"
> +#elif defined(CONFIG_ARCH_RV64I)
> +#define BOOTEFI_NAME "bootriscv64.efi"
> +#elif defined(CONFIG_SANDBOX)
> +#define BOOTEFI_NAME "bootsbox.efi"
> +#else
> +#error "Not supported for this architecture"
> +#endif
> +
> +#define EFI_FNAME	"efi/boot/" BOOTEFI_NAME
> +
> +static int efiload_read_file(struct blk_desc *desc, int partnum,
> +			     struct bootflow *bflow)
> +{
> +	const struct udevice *media_dev;
> +	int size = bflow->size;
> +	char devnum_str[9];
> +	char dirname[200];
> +	loff_t bytes_read;
> +	char *last_slash;
> +	ulong addr;
> +	char *buf;
> +	int ret;
> +
> +	/* Sadly FS closes the file after fs_size() so we must redo this */
> +	ret = fs_set_blk_dev_with_part(desc, partnum);
> +	if (ret)
> +		return log_msg_ret("set", ret);
> +
> +	buf = malloc(size + 1);
> +	if (!buf)
> +		return log_msg_ret("buf", -ENOMEM);
> +	addr = map_to_sysmem(buf);
> +
> +	ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
> +	if (ret) {
> +		free(buf);
> +		return log_msg_ret("read", ret);
> +	}
> +	if (size != bytes_read)
> +		return log_msg_ret("bread", -EINVAL);
> +	buf[size] = '\0';
> +	bflow->state = BOOTFLOWST_LOADED;
> +	bflow->buf = buf;
> +
> +	/*
> +	 * This is a horrible hack to tell EFI about this boot device. Once we
> +	 * unify EFI with the rest of U-Boot we can clean this up. The same hack
> +	 * exists in multiple places, e.g. in the fs, tftp and load commands.

Which part do you call a "horrible hack"? efi_set_bootdev()?
In fact, there are a couple of reason why we need to call this function:
1. to remember a device to create a dummy device path for the loaded
   image later,
2. to remember a size of loaded image which is used for sanity check and
   image authentication later,
3. to avoid those parameters being remembered accidentally by "loading" dtb
   and/or other binaries than the image itself,

I hope that (1) and (2) will be avoidable if we modify the current
implementation (and bootefi syntax), and then we won't need (3).

> +	 * Once we can clean up the EFI code to make proper use of driver model,
> +	 * this can go away.

My point is, however, that this kind of cleanup is irrelevant to
whether we use driver model or not.

-Takahiro Akashi

> +	 */
> +	media_dev = dev_get_parent(bflow->dev);
> +	snprintf(devnum_str, sizeof(devnum_str), "%x", dev_seq(media_dev));
> +
> +	strlcpy(dirname, bflow->fname, sizeof(dirname));
> +	last_slash = strrchr(dirname, '/');
> +	if (last_slash)
> +		*last_slash = '\0';
> +
> +	efi_set_bootdev(dev_get_uclass_name(media_dev), devnum_str, dirname,
> +			bflow->buf, size);
> +
> +	return 0;
> +}
> +
> +int efiloader_boot_setup(struct blk_desc *desc, int partnum,
> +			 struct bootflow *bflow)
> +{
> +	loff_t size;
> +	int ret;
> +
> +	bflow->type = BOOTFLOWT_EFILOADER;
> +	bflow->fname = strdup(EFI_FNAME);
> +	if (!bflow->fname)
> +		return log_msg_ret("name", -ENOMEM);
> +	ret = fs_size(bflow->fname, &size);
> +	bflow->size = size;
> +	if (ret)
> +		return log_msg_ret("size", ret);
> +	bflow->state = BOOTFLOWST_FILE;
> +	log_debug("   - distro file size %x\n", (uint)size);
> +	if (size > 0x2000000)
> +		return log_msg_ret("chk", -E2BIG);
> +
> +	ret = efiload_read_file(desc, partnum, bflow);
> +	if (ret)
> +		return log_msg_ret("read", -EINVAL);
> +
> +	return 0;
> +}
> +
> +int efiloader_boot(struct bootflow *bflow)
> +{
> +	char cmd[50];
> +
> +	/*
> +	 * At some point we can add a real interface to bootefi so we can call
> +	 * this directly. For now, go through the CLI like distro boot.
> +	 */
> +	snprintf(cmd, sizeof(cmd), "bootefi %lx %lx",
> +		 (ulong)map_to_sysmem(bflow->buf),
> +		 (ulong)map_to_sysmem(gd->fdt_blob));
> +	if (run_command(cmd, 0))
> +		return log_msg_ret("run", -EINVAL);
> +
> +	return 0;
> +}
> diff --git a/include/bm_efi.h b/include/bm_efi.h
> new file mode 100644
> index 00000000000..836b2c17f22
> --- /dev/null
> +++ b/include/bm_efi.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2021 Google LLC
> + * Written by Simon Glass <sjg@chromium.org>
> + */
> +
> +#ifndef __bootmethod_efi_h
> +#define __bootmethod_efi_h
> +
> +struct blk_desc;
> +
> +/**
> + * efiloader_boot_setup() - Set up a bootflow for EFI boot from a block device
> + *
> + * This fills out a bootflow for a particular boot device and partition. It
> + * scans for a filesystem and suitable file, updating the bootflow accordingly.
> + *
> + * This sets the following fields in @bflow:
> + *
> + *	type, size, fname, state, subdir, buf
> + *
> + * The caller mast have already set the other fields.
> + *
> + * @desc: Block-device descriptor
> + * @partnum: Partition number (1..)
> + * @bflow: Partial bootflow to be completed by this function
> + * @return 0 on success (bootflow got to 'loaded' state), -ve on error
> + */
> +int efiloader_boot_setup(struct blk_desc *desc, int partnum,
> +			 struct bootflow *bflow);
> +
> +/**
> + * efiloader_boot() - Boot an EFI binary
> + *
> + * Boots a bootflow of type BOOTFLOWT_EFI_LOADER. This boots an EFI application
> + * which takes care of the boot from then on.
> + *
> + * @bflow: Bootflow to boot
> + */
> +int efiloader_boot(struct bootflow *bflow);
> +
> +#endif
> diff --git a/include/bootmethod.h b/include/bootmethod.h
> index d80be556b8a..d6cc486c43c 100644
> --- a/include/bootmethod.h
> +++ b/include/bootmethod.h
> @@ -25,6 +25,7 @@ enum bootflow_state_t {
>  
>  enum bootflow_type_t {
>  	BOOTFLOWT_DISTRO,	/**< Distro boot */
> +	BOOTFLOWT_EFILOADER,	/**< EFI loader boot */
>  
>  	BOOTFLOWT_COUNT,
>  };
> -- 
> 2.33.0.259.gc128427fd7-goog
> 

Re: [PATCH] RFC: Support an EFI-loader bootflow

[Simon Glass]

Hi Takahiro,

On Tue, 31 Aug 2021 at 00:14, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> Simon,
>
> On Sat, Aug 28, 2021 at 02:35:21PM -0600, Simon Glass wrote:
> > This is just a demonstration of how to support EFI loader using bootflow.
> > Various things need cleaning up, not least that the naming needs to be
> > finalised. I will deal with that in the v2 series.
> >
> > In order to support multiple methods of booting from the same device, we
> > should probably separate out the different implementations (syslinux,
> > EFI loader
>
> I still believe that we'd better add "removable media" support
> to UEFI boot manager first (and then probably call this functionality
> from bootflow?).
>
> I admit that, in this case, we will have an issue that we will not
> recognize any device which is plugged in dynamically after UEFI
> subsystem is initialized. But this issue will potentially exist
> even with your approach.

That can be fixed by dropping the UEFI tables and using driver model
instead. I may have mentioned that :-)

>
> > and soon bootmgr,
>
> What will you expect in UEFI boot manager case?
> Boot parameters (options) as well as the boot order are well defined
> by BootXXXX and BootOrder variables. How are they fit into your scheme?

I haven't looked at boot manager yet, but I can't imagine it
presenting an insurmountable challenge.

>
> But anyway, we can use the following commands to run a specific
> boot flow in UEFI world:
> => efidebug boot next 1(or whatever else); bootefi bootmgr

OK.

As you probably noticed I was trying to have bootflow connect directly
to the code that does the booting so that 'CONFIG_CMDLINE' can be
disabled (e.g. for security reasons) and the boot will still work.

>
> > Chromium OS, Android, VBE) into pluggable
> > drivers and number them as we do with partitions. For now the sequence
> > number is used to determine both the partition number and the
> > implementation to use.
> >
> > The same boot command is used as before ('bootflow scan -lb') so there is
> > no change to that. It can boot both Fedora 31 and 34, for example.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> > See u-boot-dm/bmea for the tree containing this patch and the series
> > that it relies on:
> >
> >   https://patchwork.ozlabs.org/project/uboot/list/?series=258654&state=*
> >

[..]

> > +static int efiload_read_file(struct blk_desc *desc, int partnum,
> > +                          struct bootflow *bflow)
> > +{
> > +     const struct udevice *media_dev;
> > +     int size = bflow->size;
> > +     char devnum_str[9];
> > +     char dirname[200];
> > +     loff_t bytes_read;
> > +     char *last_slash;
> > +     ulong addr;
> > +     char *buf;
> > +     int ret;
> > +
> > +     /* Sadly FS closes the file after fs_size() so we must redo this */
> > +     ret = fs_set_blk_dev_with_part(desc, partnum);
> > +     if (ret)
> > +             return log_msg_ret("set", ret);
> > +
> > +     buf = malloc(size + 1);
> > +     if (!buf)
> > +             return log_msg_ret("buf", -ENOMEM);
> > +     addr = map_to_sysmem(buf);
> > +
> > +     ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
> > +     if (ret) {
> > +             free(buf);
> > +             return log_msg_ret("read", ret);
> > +     }
> > +     if (size != bytes_read)
> > +             return log_msg_ret("bread", -EINVAL);
> > +     buf[size] = '\0';
> > +     bflow->state = BOOTFLOWST_LOADED;
> > +     bflow->buf = buf;
> > +
> > +     /*
> > +      * This is a horrible hack to tell EFI about this boot device. Once we
> > +      * unify EFI with the rest of U-Boot we can clean this up. The same hack
> > +      * exists in multiple places, e.g. in the fs, tftp and load commands.
>
> Which part do you call a "horrible hack"? efi_set_bootdev()?
> In fact, there are a couple of reason why we need to call this function:
> 1. to remember a device to create a dummy device path for the loaded
>    image later,
> 2. to remember a size of loaded image which is used for sanity check and
>    image authentication later,
> 3. to avoid those parameters being remembered accidentally by "loading" dtb
>    and/or other binaries than the image itself,
>
> I hope that (1) and (2) will be avoidable if we modify the current
> implementation (and bootefi syntax), and then we won't need (3).

Yes thank you...I do understand why it is needed now, but it is
basically due to the the  fat that EFI has its own driver structures.
Once we stop those, it will go away.

>
> > +      * Once we can clean up the EFI code to make proper use of driver model,
> > +      * this can go away.
>
> My point is, however, that this kind of cleanup is irrelevant to
> whether we use driver model or not.

Are you sure? Without driver model how are you going to reference a
udevice? If not that, how are you going to reference a device? The
tables in the UEFI implementation were specifically added to avoid
relying on driver model. It is a crying shame that I did not push back
harder on this at the time.

Regards,
Simon

Re: [PATCH] RFC: Support an EFI-loader bootflow

[AKASHI Takahiro]

Simon,

On Thu, Sep 02, 2021 at 10:40:57AM -0600, Simon Glass wrote:
> Hi Takahiro,
> 
> On Tue, 31 Aug 2021 at 00:14, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > Simon,
> >
> > On Sat, Aug 28, 2021 at 02:35:21PM -0600, Simon Glass wrote:
> > > This is just a demonstration of how to support EFI loader using bootflow.
> > > Various things need cleaning up, not least that the naming needs to be
> > > finalised. I will deal with that in the v2 series.
> > >
> > > In order to support multiple methods of booting from the same device, we
> > > should probably separate out the different implementations (syslinux,
> > > EFI loader
> >
> > I still believe that we'd better add "removable media" support
> > to UEFI boot manager first (and then probably call this functionality
    ^^^^^^^^^^^^^^^^^^^^^^^^^^

> > from bootflow?).
> >
> > I admit that, in this case, we will have an issue that we will not
> > recognize any device which is plugged in dynamically after UEFI
> > subsystem is initialized. But this issue will potentially exist
> > even with your approach.
> 
> That can be fixed by dropping the UEFI tables and using driver model
> instead. I may have mentioned that :-)

I'm afraid that you don't get my point above.

> >
> > > and soon bootmgr,
> >
> > What will you expect in UEFI boot manager case?
> > Boot parameters (options) as well as the boot order are well defined
> > by BootXXXX and BootOrder variables. How are they fit into your scheme?
> 
> I haven't looked at boot manager yet, but I can't imagine it
> presenting an insurmountable challenge.

I don't say it's challenging.
Since you have not yet explained your idea about how to specify
the *boot order* in your scheme, I wonder how "BootXXXX"/"BootOrder"
be treated and honored.
There might be a parallel world again.

> >
> > But anyway, we can use the following commands to run a specific
> > boot flow in UEFI world:
> > => efidebug boot next 1(or whatever else); bootefi bootmgr
> 
> OK.
> 
> As you probably noticed I was trying to have bootflow connect directly
> to the code that does the booting so that 'CONFIG_CMDLINE' can be
> disabled (e.g. for security reasons) and the boot will still work.

# Maybe, it sounds kinda chicken and egg.

Even now, you can code this way :)

   efi_set_variable(u"BootNext", ..., u"Boot0001");
   do_efibootmgr();

That's it. My concern is what I mentioned above.

Just a note:
In the current distro_bootcmd, UEFI boot manager is also called
*every time* one of boot media in "boot_targets" is scanned/enumerated.
But it will make little sense because the current boot manager only
allows/requires users to specify both the boot device and the image file
path explicitly in a boot option, i.e. "BootXXXX" variable, and tries
all the boot options in "BootOrder" until it successfully launches
one of those images.

> >
> > > Chromium OS, Android, VBE) into pluggable
> > > drivers and number them as we do with partitions. For now the sequence
> > > number is used to determine both the partition number and the
> > > implementation to use.
> > >
> > > The same boot command is used as before ('bootflow scan -lb') so there is
> > > no change to that. It can boot both Fedora 31 and 34, for example.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > > See u-boot-dm/bmea for the tree containing this patch and the series
> > > that it relies on:
> > >
> > >   https://patchwork.ozlabs.org/project/uboot/list/?series=258654&state=*
> > >
> 
> [..]
> 
> > > +static int efiload_read_file(struct blk_desc *desc, int partnum,
> > > +                          struct bootflow *bflow)
> > > +{
> > > +     const struct udevice *media_dev;
> > > +     int size = bflow->size;
> > > +     char devnum_str[9];
> > > +     char dirname[200];
> > > +     loff_t bytes_read;
> > > +     char *last_slash;
> > > +     ulong addr;
> > > +     char *buf;
> > > +     int ret;
> > > +
> > > +     /* Sadly FS closes the file after fs_size() so we must redo this */
> > > +     ret = fs_set_blk_dev_with_part(desc, partnum);
> > > +     if (ret)
> > > +             return log_msg_ret("set", ret);
> > > +
> > > +     buf = malloc(size + 1);
> > > +     if (!buf)
> > > +             return log_msg_ret("buf", -ENOMEM);
> > > +     addr = map_to_sysmem(buf);
> > > +
> > > +     ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
> > > +     if (ret) {
> > > +             free(buf);
> > > +             return log_msg_ret("read", ret);
> > > +     }
> > > +     if (size != bytes_read)
> > > +             return log_msg_ret("bread", -EINVAL);
> > > +     buf[size] = '\0';
> > > +     bflow->state = BOOTFLOWST_LOADED;
> > > +     bflow->buf = buf;
> > > +
> > > +     /*
> > > +      * This is a horrible hack to tell EFI about this boot device. Once we
> > > +      * unify EFI with the rest of U-Boot we can clean this up. The same hack
> > > +      * exists in multiple places, e.g. in the fs, tftp and load commands.
> >
> > Which part do you call a "horrible hack"? efi_set_bootdev()?
> > In fact, there are a couple of reason why we need to call this function:
> > 1. to remember a device to create a dummy device path for the loaded
> >    image later,
> > 2. to remember a size of loaded image which is used for sanity check and
> >    image authentication later,
> > 3. to avoid those parameters being remembered accidentally by "loading" dtb
> >    and/or other binaries than the image itself,
> >
> > I hope that (1) and (2) will be avoidable if we modify the current
> > implementation (and bootefi syntax), and then we won't need (3).
> 
> Yes thank you...I do understand why it is needed now, but it is
> basically due to the the  fat that EFI has its own driver structures.
> Once we stop those, it will go away.

Here, my point is, even under the current implementation, we will
be able to eliminate efi_set_bootdev() with some tweaks.

In other words, even you could integrate UEFI into the device model,
the issue (2), for example, would still remain unsolved.
In case of (2), we use the *size* information for sanity check against
image's header information as well as calculating a hash value for
UEFI secure boot when efi_load_image() is called. Even if the integration
is done, we need to pass on the size information to "bootefi <addr>"
command implicitly or explicitly.

> >
> > > +      * Once we can clean up the EFI code to make proper use of driver model,
> > > +      * this can go away.
> >
> > My point is, however, that this kind of cleanup is irrelevant to
> > whether we use driver model or not.
> 
> Are you sure? Without driver model how are you going to reference a
> udevice? If not that, how are you going to reference a device? The
> tables in the UEFI implementation were specifically added to avoid
> relying on driver model. It is a crying shame that I did not push back
> harder on this at the time.

I hope you will get my point in the previous comment now.

Thanks,
-Takahiro Akashi


> Regards,
> Simon

Re: [PATCH] RFC: Support an EFI-loader bootflow

[Simon Glass]

Hi Takahiro,

On Thu, 2 Sept 2021 at 20:27, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> Simon,
>
> On Thu, Sep 02, 2021 at 10:40:57AM -0600, Simon Glass wrote:
> > Hi Takahiro,
> >
> > On Tue, 31 Aug 2021 at 00:14, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> > >
> > > Simon,
> > >
> > > On Sat, Aug 28, 2021 at 02:35:21PM -0600, Simon Glass wrote:
> > > > This is just a demonstration of how to support EFI loader using bootflow.
> > > > Various things need cleaning up, not least that the naming needs to be
> > > > finalised. I will deal with that in the v2 series.
> > > >
> > > > In order to support multiple methods of booting from the same device, we
> > > > should probably separate out the different implementations (syslinux,
> > > > EFI loader
> > >
> > > I still believe that we'd better add "removable media" support
> > > to UEFI boot manager first (and then probably call this functionality
>     ^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> > > from bootflow?).
> > >
> > > I admit that, in this case, we will have an issue that we will not
> > > recognize any device which is plugged in dynamically after UEFI
> > > subsystem is initialized. But this issue will potentially exist
> > > even with your approach.
> >
> > That can be fixed by dropping the UEFI tables and using driver model
> > instead. I may have mentioned that :-)
>
> I'm afraid that you don't get my point above.
>
> > >
> > > > and soon bootmgr,
> > >
> > > What will you expect in UEFI boot manager case?
> > > Boot parameters (options) as well as the boot order are well defined
> > > by BootXXXX and BootOrder variables. How are they fit into your scheme?
> >
> > I haven't looked at boot manager yet, but I can't imagine it
> > presenting an insurmountable challenge.
>
> I don't say it's challenging.
> Since you have not yet explained your idea about how to specify
> the *boot order* in your scheme, I wonder how "BootXXXX"/"BootOrder"
> be treated and honored.
> There might be a parallel world again.

Well as I mentioned, I haven't looked at it yet. The original question
was how to do EFI LOADER and I did a patch  to show that.

Are we likely to see mixed-boot environments, that use distro boot for
some OSes and EFI for others? I hope not as it would be confusing. EFI
is the parallel world, as I see it.

It should be easy enough for the 'bootmgr' bootflow to read the EFI
variables and select the correct ordering. As I understand it, EFI
does not support lazy boot, so it is acceptable to probe all the
devices before selecting one?

>
> > >
> > > But anyway, we can use the following commands to run a specific
> > > boot flow in UEFI world:
> > > => efidebug boot next 1(or whatever else); bootefi bootmgr
> >
> > OK.
> >
> > As you probably noticed I was trying to have bootflow connect directly
> > to the code that does the booting so that 'CONFIG_CMDLINE' can be
> > disabled (e.g. for security reasons) and the boot will still work.
>
> # Maybe, it sounds kinda chicken and egg.
>
> Even now, you can code this way :)
>
>    efi_set_variable(u"BootNext", ..., u"Boot0001");
>    do_efibootmgr();
>
> That's it. My concern is what I mentioned above.

OK. But then you would need to export those functions. I think it
would be better to split up the logic a bit and move things out of the
cmd/ directory (at some point).

>
> Just a note:
> In the current distro_bootcmd, UEFI boot manager is also called
> *every time* one of boot media in "boot_targets" is scanned/enumerated.
> But it will make little sense because the current boot manager only
> allows/requires users to specify both the boot device and the image file
> path explicitly in a boot option, i.e. "BootXXXX" variable, and tries
> all the boot options in "BootOrder" until it successfully launches
> one of those images.

Yes, is the idea of lazy boot entirely impossible? Or is it still
possible to do that to some extent, e.g. by scanning until you find
the first thing in the boot order?

>
> > >
> > > > Chromium OS, Android, VBE) into pluggable
> > > > drivers and number them as we do with partitions. For now the sequence
> > > > number is used to determine both the partition number and the
> > > > implementation to use.
> > > >
> > > > The same boot command is used as before ('bootflow scan -lb') so there is
> > > > no change to that. It can boot both Fedora 31 and 34, for example.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > > See u-boot-dm/bmea for the tree containing this patch and the series
> > > > that it relies on:
> > > >
> > > >   https://patchwork.ozlabs.org/project/uboot/list/?series=258654&state=*
> > > >
> >
> > [..]
> >
> > > > +static int efiload_read_file(struct blk_desc *desc, int partnum,
> > > > +                          struct bootflow *bflow)
> > > > +{
> > > > +     const struct udevice *media_dev;
> > > > +     int size = bflow->size;
> > > > +     char devnum_str[9];
> > > > +     char dirname[200];
> > > > +     loff_t bytes_read;
> > > > +     char *last_slash;
> > > > +     ulong addr;
> > > > +     char *buf;
> > > > +     int ret;
> > > > +
> > > > +     /* Sadly FS closes the file after fs_size() so we must redo this */
> > > > +     ret = fs_set_blk_dev_with_part(desc, partnum);
> > > > +     if (ret)
> > > > +             return log_msg_ret("set", ret);
> > > > +
> > > > +     buf = malloc(size + 1);
> > > > +     if (!buf)
> > > > +             return log_msg_ret("buf", -ENOMEM);
> > > > +     addr = map_to_sysmem(buf);
> > > > +
> > > > +     ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
> > > > +     if (ret) {
> > > > +             free(buf);
> > > > +             return log_msg_ret("read", ret);
> > > > +     }
> > > > +     if (size != bytes_read)
> > > > +             return log_msg_ret("bread", -EINVAL);
> > > > +     buf[size] = '\0';
> > > > +     bflow->state = BOOTFLOWST_LOADED;
> > > > +     bflow->buf = buf;
> > > > +
> > > > +     /*
> > > > +      * This is a horrible hack to tell EFI about this boot device. Once we
> > > > +      * unify EFI with the rest of U-Boot we can clean this up. The same hack
> > > > +      * exists in multiple places, e.g. in the fs, tftp and load commands.
> > >
> > > Which part do you call a "horrible hack"? efi_set_bootdev()?
> > > In fact, there are a couple of reason why we need to call this function:
> > > 1. to remember a device to create a dummy device path for the loaded
> > >    image later,
> > > 2. to remember a size of loaded image which is used for sanity check and
> > >    image authentication later,
> > > 3. to avoid those parameters being remembered accidentally by "loading" dtb
> > >    and/or other binaries than the image itself,
> > >
> > > I hope that (1) and (2) will be avoidable if we modify the current
> > > implementation (and bootefi syntax), and then we won't need (3).
> >
> > Yes thank you...I do understand why it is needed now, but it is
> > basically due to the the  fat that EFI has its own driver structures.
> > Once we stop those, it will go away.
>
> Here, my point is, even under the current implementation, we will
> be able to eliminate efi_set_bootdev() with some tweaks.
>
> In other words, even you could integrate UEFI into the device model,
> the issue (2), for example, would still remain unsolved.
> In case of (2), we use the *size* information for sanity check against
> image's header information as well as calculating a hash value for
> UEFI secure boot when efi_load_image() is called. Even if the integration
> is done, we need to pass on the size information to "bootefi <addr>"
> command implicitly or explicitly.

If you look at 'struct bootflow' you can see that it stores the size.
It can easily store other info as needed by EFI. So I don't think we
need that hack in the end, when things are using bootflow.

>
> > >
> > > > +      * Once we can clean up the EFI code to make proper use of driver model,
> > > > +      * this can go away.
> > >
> > > My point is, however, that this kind of cleanup is irrelevant to
> > > whether we use driver model or not.
> >
> > Are you sure? Without driver model how are you going to reference a
> > udevice? If not that, how are you going to reference a device? The
> > tables in the UEFI implementation were specifically added to avoid
> > relying on driver model. It is a crying shame that I did not push back
> > harder on this at the time.
>
> I hope you will get my point in the previous comment now.

Well yes I do, but I don't understand why there is such resistance to
sorting out the EFI implementation? It is quite a mess at the moment.
I think the first step is to drop the non-DM code, but the second is
to drop the parallel tables that EFI keeps about each device.

Regards,
Simon

Re: [PATCH] RFC: Support an EFI-loader bootflow

[Heinrich Schuchardt]

On 9/3/21 10:53 AM, Simon Glass wrote:
> Hi Takahiro,
>
> On Thu, 2 Sept 2021 at 20:27, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
>>
>> Simon,
>>
>> On Thu, Sep 02, 2021 at 10:40:57AM -0600, Simon Glass wrote:
>>> Hi Takahiro,
>>>
>>> On Tue, 31 Aug 2021 at 00:14, AKASHI Takahiro
>>> <takahiro.akashi@linaro.org> wrote:
>>>>
>>>> Simon,
>>>>
>>>> On Sat, Aug 28, 2021 at 02:35:21PM -0600, Simon Glass wrote:
>>>>> This is just a demonstration of how to support EFI loader using bootflow.
>>>>> Various things need cleaning up, not least that the naming needs to be
>>>>> finalised. I will deal with that in the v2 series.
>>>>>
>>>>> In order to support multiple methods of booting from the same device, we
>>>>> should probably separate out the different implementations (syslinux,
>>>>> EFI loader
>>>>
>>>> I still believe that we'd better add "removable media" support
>>>> to UEFI boot manager first (and then probably call this functionality
>>      ^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>>>> from bootflow?).
>>>>
>>>> I admit that, in this case, we will have an issue that we will not
>>>> recognize any device which is plugged in dynamically after UEFI
>>>> subsystem is initialized. But this issue will potentially exist
>>>> even with your approach.
>>>
>>> That can be fixed by dropping the UEFI tables and using driver model
>>> instead. I may have mentioned that :-)
>>
>> I'm afraid that you don't get my point above.
>>
>>>>
>>>>> and soon bootmgr,
>>>>
>>>> What will you expect in UEFI boot manager case?
>>>> Boot parameters (options) as well as the boot order are well defined
>>>> by BootXXXX and BootOrder variables. How are they fit into your scheme?
>>>
>>> I haven't looked at boot manager yet, but I can't imagine it
>>> presenting an insurmountable challenge.
>>
>> I don't say it's challenging.
>> Since you have not yet explained your idea about how to specify
>> the *boot order* in your scheme, I wonder how "BootXXXX"/"BootOrder"
>> be treated and honored.
>> There might be a parallel world again.
>
> Well as I mentioned, I haven't looked at it yet. The original question
> was how to do EFI LOADER and I did a patch  to show that.
>
> Are we likely to see mixed-boot environments, that use distro boot for
> some OSes and EFI for others? I hope not as it would be confusing. EFI
> is the parallel world, as I see it.
>
> It should be easy enough for the 'bootmgr' bootflow to read the EFI
> variables and select the correct ordering. As I understand it, EFI
> does not support lazy boot, so it is acceptable to probe all the
> devices before selecting one?
>
>>
>>>>
>>>> But anyway, we can use the following commands to run a specific
>>>> boot flow in UEFI world:
>>>> => efidebug boot next 1(or whatever else); bootefi bootmgr
>>>
>>> OK.
>>>
>>> As you probably noticed I was trying to have bootflow connect directly
>>> to the code that does the booting so that 'CONFIG_CMDLINE' can be
>>> disabled (e.g. for security reasons) and the boot will still work.
>>
>> # Maybe, it sounds kinda chicken and egg.
>>
>> Even now, you can code this way :)
>>
>>     efi_set_variable(u"BootNext", ..., u"Boot0001");
>>     do_efibootmgr();
>>
>> That's it. My concern is what I mentioned above.
>
> OK. But then you would need to export those functions. I think it
> would be better to split up the logic a bit and move things out of the
> cmd/ directory (at some point).
>
>>
>> Just a note:
>> In the current distro_bootcmd, UEFI boot manager is also called
>> *every time* one of boot media in "boot_targets" is scanned/enumerated.
>> But it will make little sense because the current boot manager only
>> allows/requires users to specify both the boot device and the image file
>> path explicitly in a boot option, i.e. "BootXXXX" variable, and tries
>> all the boot options in "BootOrder" until it successfully launches
>> one of those images.
>
> Yes, is the idea of lazy boot entirely impossible? Or is it still
> possible to do that to some extent, e.g. by scanning until you find
> the first thing in the boot order?
>
>>
>>>>
>>>>> Chromium OS, Android, VBE) into pluggable
>>>>> drivers and number them as we do with partitions. For now the sequence
>>>>> number is used to determine both the partition number and the
>>>>> implementation to use.
>>>>>
>>>>> The same boot command is used as before ('bootflow scan -lb') so there is
>>>>> no change to that. It can boot both Fedora 31 and 34, for example.
>>>>>
>>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>>> ---
>>>>> See u-boot-dm/bmea for the tree containing this patch and the series
>>>>> that it relies on:
>>>>>
>>>>>    https://patchwork.ozlabs.org/project/uboot/list/?series=258654&state=*
>>>>>
>>>
>>> [..]
>>>
>>>>> +static int efiload_read_file(struct blk_desc *desc, int partnum,
>>>>> +                          struct bootflow *bflow)
>>>>> +{
>>>>> +     const struct udevice *media_dev;
>>>>> +     int size = bflow->size;
>>>>> +     char devnum_str[9];
>>>>> +     char dirname[200];
>>>>> +     loff_t bytes_read;
>>>>> +     char *last_slash;
>>>>> +     ulong addr;
>>>>> +     char *buf;
>>>>> +     int ret;
>>>>> +
>>>>> +     /* Sadly FS closes the file after fs_size() so we must redo this */
>>>>> +     ret = fs_set_blk_dev_with_part(desc, partnum);
>>>>> +     if (ret)
>>>>> +             return log_msg_ret("set", ret);
>>>>> +
>>>>> +     buf = malloc(size + 1);
>>>>> +     if (!buf)
>>>>> +             return log_msg_ret("buf", -ENOMEM);
>>>>> +     addr = map_to_sysmem(buf);
>>>>> +
>>>>> +     ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
>>>>> +     if (ret) {
>>>>> +             free(buf);
>>>>> +             return log_msg_ret("read", ret);
>>>>> +     }
>>>>> +     if (size != bytes_read)
>>>>> +             return log_msg_ret("bread", -EINVAL);
>>>>> +     buf[size] = '\0';
>>>>> +     bflow->state = BOOTFLOWST_LOADED;
>>>>> +     bflow->buf = buf;
>>>>> +
>>>>> +     /*
>>>>> +      * This is a horrible hack to tell EFI about this boot device. Once we
>>>>> +      * unify EFI with the rest of U-Boot we can clean this up. The same hack
>>>>> +      * exists in multiple places, e.g. in the fs, tftp and load commands.
>>>>
>>>> Which part do you call a "horrible hack"? efi_set_bootdev()?
>>>> In fact, there are a couple of reason why we need to call this function:
>>>> 1. to remember a device to create a dummy device path for the loaded
>>>>     image later,
>>>> 2. to remember a size of loaded image which is used for sanity check and
>>>>     image authentication later,
>>>> 3. to avoid those parameters being remembered accidentally by "loading" dtb
>>>>     and/or other binaries than the image itself,
>>>>
>>>> I hope that (1) and (2) will be avoidable if we modify the current
>>>> implementation (and bootefi syntax), and then we won't need (3).
>>>
>>> Yes thank you...I do understand why it is needed now, but it is
>>> basically due to the the  fat that EFI has its own driver structures.
>>> Once we stop those, it will go away.
>>
>> Here, my point is, even under the current implementation, we will
>> be able to eliminate efi_set_bootdev() with some tweaks.
>>
>> In other words, even you could integrate UEFI into the device model,
>> the issue (2), for example, would still remain unsolved.
>> In case of (2), we use the *size* information for sanity check against
>> image's header information as well as calculating a hash value for
>> UEFI secure boot when efi_load_image() is called. Even if the integration
>> is done, we need to pass on the size information to "bootefi <addr>"
>> command implicitly or explicitly.
>
> If you look at 'struct bootflow' you can see that it stores the size.
> It can easily store other info as needed by EFI. So I don't think we
> need that hack in the end, when things are using bootflow.
>
>>
>>>>
>>>>> +      * Once we can clean up the EFI code to make proper use of driver model,
>>>>> +      * this can go away.
>>>>
>>>> My point is, however, that this kind of cleanup is irrelevant to
>>>> whether we use driver model or not.
>>>
>>> Are you sure? Without driver model how are you going to reference a
>>> udevice? If not that, how are you going to reference a device? The
>>> tables in the UEFI implementation were specifically added to avoid
>>> relying on driver model. It is a crying shame that I did not push back
>>> harder on this at the time.
>>
>> I hope you will get my point in the previous comment now.
>
> Well yes I do, but I don't understand why there is such resistance to
> sorting out the EFI implementation? It is quite a mess at the moment.
> I think the first step is to drop the non-DM code, but the second is
> to drop the parallel tables that EFI keeps about each device.


Concerning these "parallel" tables. Please, have a look at the UEFI spec
to understand what a handle, a protocol interface, and an event are.

I also gave as short overview in
https://archive.fosdem.org/2020/schedule/event/firmware_duwu/ starting
at slide 10 or 08:00 of the video.

Handles may refer to devices, to drivers or anything else.
Handles are both to be created by U-Boot and by EFI binaries that we
execute.

On these handles protocol interfaces can be installed. These may be
those defined in the UEFI spec or something completely independent. The
protocol interfaces contain pointers to functions and additional data.

Further objects are events. These may contain references to an event
handler function that is called when the event is triggered.

None of the above objects matches one to one to objects in the driver
model. But we can use some integration:

When a U-Boot device is probed we must create the UEFI handle and
install the expected protocols on it.

When a U-Boot device is removed we must destroy the handle. There are
certain conditions under which a protocol must not be removed from a
handle and the handle cannot be destroyed. In this case removing a
device should not be possible.

Best regards

Heinrich

Re: [PATCH] RFC: Support an EFI-loader bootflow

[AKASHI Takahiro]

Hi Simon,

On Fri, Sep 03, 2021 at 02:53:52AM -0600, Simon Glass wrote:
> Hi Takahiro,
> 
> On Thu, 2 Sept 2021 at 20:27, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > Simon,
> >
> > On Thu, Sep 02, 2021 at 10:40:57AM -0600, Simon Glass wrote:
> > > Hi Takahiro,
> > >
> > > On Tue, 31 Aug 2021 at 00:14, AKASHI Takahiro
> > > <takahiro.akashi@linaro.org> wrote:
> > > >
> > > > Simon,
> > > >
> > > > On Sat, Aug 28, 2021 at 02:35:21PM -0600, Simon Glass wrote:
> > > > > This is just a demonstration of how to support EFI loader using bootflow.
> > > > > Various things need cleaning up, not least that the naming needs to be
> > > > > finalised. I will deal with that in the v2 series.
> > > > >
> > > > > In order to support multiple methods of booting from the same device, we
> > > > > should probably separate out the different implementations (syslinux,
> > > > > EFI loader
> > > >
> > > > I still believe that we'd better add "removable media" support
> > > > to UEFI boot manager first (and then probably call this functionality
> >     ^^^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > > > from bootflow?).
> > > >
> > > > I admit that, in this case, we will have an issue that we will not
> > > > recognize any device which is plugged in dynamically after UEFI
> > > > subsystem is initialized. But this issue will potentially exist
> > > > even with your approach.
> > >
> > > That can be fixed by dropping the UEFI tables and using driver model
> > > instead. I may have mentioned that :-)
> >
> > I'm afraid that you don't get my point above.
> >
> > > >
> > > > > and soon bootmgr,
> > > >
> > > > What will you expect in UEFI boot manager case?
> > > > Boot parameters (options) as well as the boot order are well defined
> > > > by BootXXXX and BootOrder variables. How are they fit into your scheme?
> > >
> > > I haven't looked at boot manager yet, but I can't imagine it
> > > presenting an insurmountable challenge.
> >
> > I don't say it's challenging.
> > Since you have not yet explained your idea about how to specify
> > the *boot order* in your scheme, I wonder how "BootXXXX"/"BootOrder"
> > be treated and honored.
> > There might be a parallel world again.
> 
> Well as I mentioned, I haven't looked at it yet. The original question
> was how to do EFI LOADER and I did a patch  to show that.
> 
> Are we likely to see mixed-boot environments, that use distro boot for
> some OSes and EFI for others? I hope not as it would be confusing. EFI
> is the parallel world, as I see it.

Yeah, I absolutely agree here.
That is why I constantly insist "removable media" support should be
implemented in UEFI boot manager in the first place. Please remember
that "removable media" support is part of UEFI specification,
UEFI boot manager and hence "EFI LOADER".

# By "removable media" support, I mean that the image to be loaded
# is determined by searching for the default file path, or /EFI/BOOT/xxx.efi,
# in the system partition. The media can be, say, a fixed (non-removable)
# HD on the system.
# This is definitely part of UEFI environment.

> It should be easy enough for the 'bootmgr' bootflow to read the EFI
> variables and select the correct ordering. As I understand it, EFI
> does not support lazy boot, so it is acceptable to probe all the
> devices before selecting one?
> >
> > > >
> > > > But anyway, we can use the following commands to run a specific
> > > > boot flow in UEFI world:
> > > > => efidebug boot next 1(or whatever else); bootefi bootmgr
> > >
> > > OK.
> > >
> > > As you probably noticed I was trying to have bootflow connect directly
> > > to the code that does the booting so that 'CONFIG_CMDLINE' can be
> > > disabled (e.g. for security reasons) and the boot will still work.
> >
> > # Maybe, it sounds kinda chicken and egg.
> >
> > Even now, you can code this way :)
> >
> >    efi_set_variable(u"BootNext", ..., u"Boot0001");
> >    do_efibootmgr();
> >
> > That's it. My concern is what I mentioned above.
> 
> OK. But then you would need to export those functions. I think it
> would be better to split up the logic a bit and move things out of the
> cmd/ directory (at some point).

If we don't take "mixed-boot environments", adding efibootmger to
bootflow mechanism, or implementing efibootmger using bootflow,
makes little sense.

> >
> > Just a note:
> > In the current distro_bootcmd, UEFI boot manager is also called
> > *every time* one of boot media in "boot_targets" is scanned/enumerated.
> > But it will make little sense because the current boot manager only
> > allows/requires users to specify both the boot device and the image file
> > path explicitly in a boot option, i.e. "BootXXXX" variable, and tries
> > all the boot options in "BootOrder" until it successfully launches
> > one of those images.
> 
> Yes, is the idea of lazy boot entirely impossible? Or is it still
> possible to do that to some extent, e.g. by scanning until you find
> the first thing in the boot order?

If I correctly understand what 'lazy boot' means here, you can do
all the enumeration/scanning first, then call UEFI boot manager:
=> scsi rescan; usb start; mmc rescan; (and whatever else); bootefi bootmger

The order of scanning commands doesn't matter as "BootOrder" determine
the priorities among BootXXXX (boot options).
What the current UEFI cannot provides is to detect a new device that
is attached to the board once UEFI subsystem is initialized.

That issue, which I have mentioned in the ML from time to time, should be
separately resolved as part of "integration" efforts between UEFI objects
and driver model.

> >
> > > >
> > > > > Chromium OS, Android, VBE) into pluggable
> > > > > drivers and number them as we do with partitions. For now the sequence
> > > > > number is used to determine both the partition number and the
> > > > > implementation to use.
> > > > >
> > > > > The same boot command is used as before ('bootflow scan -lb') so there is
> > > > > no change to that. It can boot both Fedora 31 and 34, for example.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > > See u-boot-dm/bmea for the tree containing this patch and the series
> > > > > that it relies on:
> > > > >
> > > > >   https://patchwork.ozlabs.org/project/uboot/list/?series=258654&state=*
> > > > >
> > >
> > > [..]
> > >
> > > > > +static int efiload_read_file(struct blk_desc *desc, int partnum,
> > > > > +                          struct bootflow *bflow)
> > > > > +{
> > > > > +     const struct udevice *media_dev;
> > > > > +     int size = bflow->size;
> > > > > +     char devnum_str[9];
> > > > > +     char dirname[200];
> > > > > +     loff_t bytes_read;
> > > > > +     char *last_slash;
> > > > > +     ulong addr;
> > > > > +     char *buf;
> > > > > +     int ret;
> > > > > +
> > > > > +     /* Sadly FS closes the file after fs_size() so we must redo this */
> > > > > +     ret = fs_set_blk_dev_with_part(desc, partnum);
> > > > > +     if (ret)
> > > > > +             return log_msg_ret("set", ret);
> > > > > +
> > > > > +     buf = malloc(size + 1);
> > > > > +     if (!buf)
> > > > > +             return log_msg_ret("buf", -ENOMEM);
> > > > > +     addr = map_to_sysmem(buf);
> > > > > +
> > > > > +     ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
> > > > > +     if (ret) {
> > > > > +             free(buf);
> > > > > +             return log_msg_ret("read", ret);
> > > > > +     }
> > > > > +     if (size != bytes_read)
> > > > > +             return log_msg_ret("bread", -EINVAL);
> > > > > +     buf[size] = '\0';
> > > > > +     bflow->state = BOOTFLOWST_LOADED;
> > > > > +     bflow->buf = buf;
> > > > > +
> > > > > +     /*
> > > > > +      * This is a horrible hack to tell EFI about this boot device. Once we
> > > > > +      * unify EFI with the rest of U-Boot we can clean this up. The same hack
> > > > > +      * exists in multiple places, e.g. in the fs, tftp and load commands.
> > > >
> > > > Which part do you call a "horrible hack"? efi_set_bootdev()?
> > > > In fact, there are a couple of reason why we need to call this function:
> > > > 1. to remember a device to create a dummy device path for the loaded
> > > >    image later,
> > > > 2. to remember a size of loaded image which is used for sanity check and
> > > >    image authentication later,
> > > > 3. to avoid those parameters being remembered accidentally by "loading" dtb
> > > >    and/or other binaries than the image itself,
> > > >
> > > > I hope that (1) and (2) will be avoidable if we modify the current
> > > > implementation (and bootefi syntax), and then we won't need (3).
> > >
> > > Yes thank you...I do understand why it is needed now, but it is
> > > basically due to the the  fat that EFI has its own driver structures.
> > > Once we stop those, it will go away.
> >
> > Here, my point is, even under the current implementation, we will
> > be able to eliminate efi_set_bootdev() with some tweaks.
> >
> > In other words, even you could integrate UEFI into the device model,
> > the issue (2), for example, would still remain unsolved.
> > In case of (2), we use the *size* information for sanity check against
> > image's header information as well as calculating a hash value for
> > UEFI secure boot when efi_load_image() is called. Even if the integration
> > is done, we need to pass on the size information to "bootefi <addr>"
> > command implicitly or explicitly.
> 
> If you look at 'struct bootflow' you can see that it stores the size.
> It can easily store other info as needed by EFI. So I don't think we
> need that hack in the end, when things are using bootflow.

I'm afraid that you miss the context of my discussion.
We support a command sequence like the following:
=> load scsi 0:1 50000000 /hellowworld.efi
=> bootefi 50000000

distro_bootcmd does use this method to add "removable media" support,
but even if the distro script is re-implemented using bootflow machanism,
we cannot remove "horrible hack" in load command as long as we continue
to use load command.

> >
> > > >
> > > > > +      * Once we can clean up the EFI code to make proper use of driver model,
> > > > > +      * this can go away.
> > > >
> > > > My point is, however, that this kind of cleanup is irrelevant to
> > > > whether we use driver model or not.
> > >
> > > Are you sure? Without driver model how are you going to reference a
> > > udevice? If not that, how are you going to reference a device? The
> > > tables in the UEFI implementation were specifically added to avoid
> > > relying on driver model. It is a crying shame that I did not push back
> > > harder on this at the time.
> >
> > I hope you will get my point in the previous comment now.
> 
> Well yes I do, but I don't understand why there is such resistance to
> sorting out the EFI implementation? It is quite a mess at the moment.

As you know, I have a positive opinion to the integration.
What I object to here is to integrate UEFI boot manager into bootflow.

> I think the first step is to drop the non-DM code, but the second is
> to drop the parallel tables that EFI keeps about each device.

As Heinrich suggested in his reply, there are many concepts that might
not be easily mapped to U-Boot counterparts. If our only concern is on
the boot flow, it would be easier, but we would also like to support
a lot of API, either boot/runtime services or protocols. Filling
semantic gaps would make it harder to completely drop some sort of
"parallel tables".

I think that the first thing that we should tackle is to eliminate
the issue, as I mentioned above, regarding "block devices".
It would shed light on how we could go further on this issue.

-Takahiro Akashi

> Regards,
> Simon

Re: [PATCH] RFC: Support an EFI-loader bootflow

[Simon Glass]

Hi Heinrich,

On Fri, 3 Sept 2021 at 10:30, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> On 9/3/21 10:53 AM, Simon Glass wrote:
> > Hi Takahiro,
> >
> > On Thu, 2 Sept 2021 at 20:27, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> >>
> >> Simon,
> >>
> >> On Thu, Sep 02, 2021 at 10:40:57AM -0600, Simon Glass wrote:
> >>> Hi Takahiro,
> >>>
> >>> On Tue, 31 Aug 2021 at 00:14, AKASHI Takahiro
> >>> <takahiro.akashi@linaro.org> wrote:
> >>>>
> >>>> Simon,
> >>>>
> >>>> On Sat, Aug 28, 2021 at 02:35:21PM -0600, Simon Glass wrote:
> >>>>> This is just a demonstration of how to support EFI loader using bootflow.
> >>>>> Various things need cleaning up, not least that the naming needs to be
> >>>>> finalised. I will deal with that in the v2 series.
> >>>>>
> >>>>> In order to support multiple methods of booting from the same device, we
> >>>>> should probably separate out the different implementations (syslinux,
> >>>>> EFI loader
> >>>>
> >>>> I still believe that we'd better add "removable media" support
> >>>> to UEFI boot manager first (and then probably call this functionality
> >>      ^^^^^^^^^^^^^^^^^^^^^^^^^^
> >>
> >>>> from bootflow?).
> >>>>
> >>>> I admit that, in this case, we will have an issue that we will not
> >>>> recognize any device which is plugged in dynamically after UEFI
> >>>> subsystem is initialized. But this issue will potentially exist
> >>>> even with your approach.
> >>>
> >>> That can be fixed by dropping the UEFI tables and using driver model
> >>> instead. I may have mentioned that :-)
> >>
> >> I'm afraid that you don't get my point above.
> >>
> >>>>
> >>>>> and soon bootmgr,
> >>>>
> >>>> What will you expect in UEFI boot manager case?
> >>>> Boot parameters (options) as well as the boot order are well defined
> >>>> by BootXXXX and BootOrder variables. How are they fit into your scheme?
> >>>
> >>> I haven't looked at boot manager yet, but I can't imagine it
> >>> presenting an insurmountable challenge.
> >>
> >> I don't say it's challenging.
> >> Since you have not yet explained your idea about how to specify
> >> the *boot order* in your scheme, I wonder how "BootXXXX"/"BootOrder"
> >> be treated and honored.
> >> There might be a parallel world again.
> >
> > Well as I mentioned, I haven't looked at it yet. The original question
> > was how to do EFI LOADER and I did a patch  to show that.
> >
> > Are we likely to see mixed-boot environments, that use distro boot for
> > some OSes and EFI for others? I hope not as it would be confusing. EFI
> > is the parallel world, as I see it.
> >
> > It should be easy enough for the 'bootmgr' bootflow to read the EFI
> > variables and select the correct ordering. As I understand it, EFI
> > does not support lazy boot, so it is acceptable to probe all the
> > devices before selecting one?
> >
> >>
> >>>>
> >>>> But anyway, we can use the following commands to run a specific
> >>>> boot flow in UEFI world:
> >>>> => efidebug boot next 1(or whatever else); bootefi bootmgr
> >>>
> >>> OK.
> >>>
> >>> As you probably noticed I was trying to have bootflow connect directly
> >>> to the code that does the booting so that 'CONFIG_CMDLINE' can be
> >>> disabled (e.g. for security reasons) and the boot will still work.
> >>
> >> # Maybe, it sounds kinda chicken and egg.
> >>
> >> Even now, you can code this way :)
> >>
> >>     efi_set_variable(u"BootNext", ..., u"Boot0001");
> >>     do_efibootmgr();
> >>
> >> That's it. My concern is what I mentioned above.
> >
> > OK. But then you would need to export those functions. I think it
> > would be better to split up the logic a bit and move things out of the
> > cmd/ directory (at some point).
> >
> >>
> >> Just a note:
> >> In the current distro_bootcmd, UEFI boot manager is also called
> >> *every time* one of boot media in "boot_targets" is scanned/enumerated.
> >> But it will make little sense because the current boot manager only
> >> allows/requires users to specify both the boot device and the image file
> >> path explicitly in a boot option, i.e. "BootXXXX" variable, and tries
> >> all the boot options in "BootOrder" until it successfully launches
> >> one of those images.
> >
> > Yes, is the idea of lazy boot entirely impossible? Or is it still
> > possible to do that to some extent, e.g. by scanning until you find
> > the first thing in the boot order?
> >
> >>
> >>>>
> >>>>> Chromium OS, Android, VBE) into pluggable
> >>>>> drivers and number them as we do with partitions. For now the sequence
> >>>>> number is used to determine both the partition number and the
> >>>>> implementation to use.
> >>>>>
> >>>>> The same boot command is used as before ('bootflow scan -lb') so there is
> >>>>> no change to that. It can boot both Fedora 31 and 34, for example.
> >>>>>
> >>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
> >>>>> ---
> >>>>> See u-boot-dm/bmea for the tree containing this patch and the series
> >>>>> that it relies on:
> >>>>>
> >>>>>    https://patchwork.ozlabs.org/project/uboot/list/?series=258654&state=*
> >>>>>
> >>>
> >>> [..]
> >>>
> >>>>> +static int efiload_read_file(struct blk_desc *desc, int partnum,
> >>>>> +                          struct bootflow *bflow)
> >>>>> +{
> >>>>> +     const struct udevice *media_dev;
> >>>>> +     int size = bflow->size;
> >>>>> +     char devnum_str[9];
> >>>>> +     char dirname[200];
> >>>>> +     loff_t bytes_read;
> >>>>> +     char *last_slash;
> >>>>> +     ulong addr;
> >>>>> +     char *buf;
> >>>>> +     int ret;
> >>>>> +
> >>>>> +     /* Sadly FS closes the file after fs_size() so we must redo this */
> >>>>> +     ret = fs_set_blk_dev_with_part(desc, partnum);
> >>>>> +     if (ret)
> >>>>> +             return log_msg_ret("set", ret);
> >>>>> +
> >>>>> +     buf = malloc(size + 1);
> >>>>> +     if (!buf)
> >>>>> +             return log_msg_ret("buf", -ENOMEM);
> >>>>> +     addr = map_to_sysmem(buf);
> >>>>> +
> >>>>> +     ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
> >>>>> +     if (ret) {
> >>>>> +             free(buf);
> >>>>> +             return log_msg_ret("read", ret);
> >>>>> +     }
> >>>>> +     if (size != bytes_read)
> >>>>> +             return log_msg_ret("bread", -EINVAL);
> >>>>> +     buf[size] = '\0';
> >>>>> +     bflow->state = BOOTFLOWST_LOADED;
> >>>>> +     bflow->buf = buf;
> >>>>> +
> >>>>> +     /*
> >>>>> +      * This is a horrible hack to tell EFI about this boot device. Once we
> >>>>> +      * unify EFI with the rest of U-Boot we can clean this up. The same hack
> >>>>> +      * exists in multiple places, e.g. in the fs, tftp and load commands.
> >>>>
> >>>> Which part do you call a "horrible hack"? efi_set_bootdev()?
> >>>> In fact, there are a couple of reason why we need to call this function:
> >>>> 1. to remember a device to create a dummy device path for the loaded
> >>>>     image later,
> >>>> 2. to remember a size of loaded image which is used for sanity check and
> >>>>     image authentication later,
> >>>> 3. to avoid those parameters being remembered accidentally by "loading" dtb
> >>>>     and/or other binaries than the image itself,
> >>>>
> >>>> I hope that (1) and (2) will be avoidable if we modify the current
> >>>> implementation (and bootefi syntax), and then we won't need (3).
> >>>
> >>> Yes thank you...I do understand why it is needed now, but it is
> >>> basically due to the the  fat that EFI has its own driver structures.
> >>> Once we stop those, it will go away.
> >>
> >> Here, my point is, even under the current implementation, we will
> >> be able to eliminate efi_set_bootdev() with some tweaks.
> >>
> >> In other words, even you could integrate UEFI into the device model,
> >> the issue (2), for example, would still remain unsolved.
> >> In case of (2), we use the *size* information for sanity check against
> >> image's header information as well as calculating a hash value for
> >> UEFI secure boot when efi_load_image() is called. Even if the integration
> >> is done, we need to pass on the size information to "bootefi <addr>"
> >> command implicitly or explicitly.
> >
> > If you look at 'struct bootflow' you can see that it stores the size.
> > It can easily store other info as needed by EFI. So I don't think we
> > need that hack in the end, when things are using bootflow.
> >
> >>
> >>>>
> >>>>> +      * Once we can clean up the EFI code to make proper use of driver model,
> >>>>> +      * this can go away.
> >>>>
> >>>> My point is, however, that this kind of cleanup is irrelevant to
> >>>> whether we use driver model or not.
> >>>
> >>> Are you sure? Without driver model how are you going to reference a
> >>> udevice? If not that, how are you going to reference a device? The
> >>> tables in the UEFI implementation were specifically added to avoid
> >>> relying on driver model. It is a crying shame that I did not push back
> >>> harder on this at the time.
> >>
> >> I hope you will get my point in the previous comment now.
> >
> > Well yes I do, but I don't understand why there is such resistance to
> > sorting out the EFI implementation? It is quite a mess at the moment.
> > I think the first step is to drop the non-DM code, but the second is
> > to drop the parallel tables that EFI keeps about each device.
>
>
> Concerning these "parallel" tables. Please, have a look at the UEFI spec
> to understand what a handle, a protocol interface, and an event are.
>
> I also gave as short overview in
> https://archive.fosdem.org/2020/schedule/event/firmware_duwu/ starting
> at slide 10 or 08:00 of the video.
>
> Handles may refer to devices, to drivers or anything else.
> Handles are both to be created by U-Boot and by EFI binaries that we
> execute.
>
> On these handles protocol interfaces can be installed. These may be
> those defined in the UEFI spec or something completely independent. The
> protocol interfaces contain pointers to functions and additional data.
>
> Further objects are events. These may contain references to an event
> handler function that is called when the event is triggered.

Thank you for the info. I did read the entire spec some years ago
(about 6 I think) and have dug into bits of it since. I do find the
naming confusing so I cannot claim to understand it. The use of void *
everywhere is a pain.. I will doubtless take another look.

U-Boot does have devices, which have a (single) uclass and API. It
also has drivers. After all, the EFI tables are created from U-Boot
data structures, so presumably there is some correspondence.

>
> None of the above objects matches one to one to objects in the driver
> model. But we can use some integration:
>
> When a U-Boot device is probed we must create the UEFI handle and
> install the expected protocols on it.
>
> When a U-Boot device is removed we must destroy the handle. There are
> certain conditions under which a protocol must not be removed from a
> handle and the handle cannot be destroyed. In this case removing a
> device should not be possible.

I wish this had been done from the beginning, but we may as well start
now. It is quite a complicated task at this point.

Can we pick one data structure that we can make ephemeral, using only
the DM structs and features? Can we look at what features need to be
added (such as partition devices?) and add those to U-Boot?

What would be easiest as a starting point? How about struct
efi_system_partition?

Or perhaps could we drop efi_disk_register() and have it scan the disk
'on the fly' when needed?

There must be something that can be done here. I am sure there is a
mismatch of concepts/API, but some of these mismatches are features
that U-Boot could add and some can be 'thinned down' so that UEFI is
not such a lot of code.

Regards,
Simon

Re: [PATCH] RFC: Support an EFI-loader bootflow

[Simon Glass]

Hi Takahiro,

On Sun, 5 Sept 2021 at 19:47, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> Hi Simon,
>
> On Fri, Sep 03, 2021 at 02:53:52AM -0600, Simon Glass wrote:
> > Hi Takahiro,
> >
> > On Thu, 2 Sept 2021 at 20:27, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> > >
> > > Simon,
> > >
> > > On Thu, Sep 02, 2021 at 10:40:57AM -0600, Simon Glass wrote:
> > > > Hi Takahiro,
> > > >
> > > > On Tue, 31 Aug 2021 at 00:14, AKASHI Takahiro
> > > > <takahiro.akashi@linaro.org> wrote:
> > > > >
> > > > > Simon,
> > > > >
> > > > > On Sat, Aug 28, 2021 at 02:35:21PM -0600, Simon Glass wrote:
> > > > > > This is just a demonstration of how to support EFI loader using bootflow.
> > > > > > Various things need cleaning up, not least that the naming needs to be
> > > > > > finalised. I will deal with that in the v2 series.
> > > > > >
> > > > > > In order to support multiple methods of booting from the same device, we
> > > > > > should probably separate out the different implementations (syslinux,
> > > > > > EFI loader
> > > > >
> > > > > I still believe that we'd better add "removable media" support
> > > > > to UEFI boot manager first (and then probably call this functionality
> > >     ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > >
> > > > > from bootflow?).
> > > > >
> > > > > I admit that, in this case, we will have an issue that we will not
> > > > > recognize any device which is plugged in dynamically after UEFI
> > > > > subsystem is initialized. But this issue will potentially exist
> > > > > even with your approach.
> > > >
> > > > That can be fixed by dropping the UEFI tables and using driver model
> > > > instead. I may have mentioned that :-)
> > >
> > > I'm afraid that you don't get my point above.
> > >
> > > > >
> > > > > > and soon bootmgr,
> > > > >
> > > > > What will you expect in UEFI boot manager case?
> > > > > Boot parameters (options) as well as the boot order are well defined
> > > > > by BootXXXX and BootOrder variables. How are they fit into your scheme?
> > > >
> > > > I haven't looked at boot manager yet, but I can't imagine it
> > > > presenting an insurmountable challenge.
> > >
> > > I don't say it's challenging.
> > > Since you have not yet explained your idea about how to specify
> > > the *boot order* in your scheme, I wonder how "BootXXXX"/"BootOrder"
> > > be treated and honored.
> > > There might be a parallel world again.
> >
> > Well as I mentioned, I haven't looked at it yet. The original question
> > was how to do EFI LOADER and I did a patch  to show that.
> >
> > Are we likely to see mixed-boot environments, that use distro boot for
> > some OSes and EFI for others? I hope not as it would be confusing. EFI
> > is the parallel world, as I see it.
>
> Yeah, I absolutely agree here.
> That is why I constantly insist "removable media" support should be
> implemented in UEFI boot manager in the first place. Please remember
> that "removable media" support is part of UEFI specification,
> UEFI boot manager and hence "EFI LOADER".
>
> # By "removable media" support, I mean that the image to be loaded
> # is determined by searching for the default file path, or /EFI/BOOT/xxx.efi,
> # in the system partition. The media can be, say, a fixed (non-removable)
> # HD on the system.
> # This is definitely part of UEFI environment.
>

OK.

> > It should be easy enough for the 'bootmgr' bootflow to read the EFI
> > variables and select the correct ordering. As I understand it, EFI
> > does not support lazy boot, so it is acceptable to probe all the
> > devices before selecting one?
> > >
> > > > >
> > > > > But anyway, we can use the following commands to run a specific
> > > > > boot flow in UEFI world:
> > > > > => efidebug boot next 1(or whatever else); bootefi bootmgr
> > > >
> > > > OK.
> > > >
> > > > As you probably noticed I was trying to have bootflow connect directly
> > > > to the code that does the booting so that 'CONFIG_CMDLINE' can be
> > > > disabled (e.g. for security reasons) and the boot will still work.
> > >
> > > # Maybe, it sounds kinda chicken and egg.
> > >
> > > Even now, you can code this way :)
> > >
> > >    efi_set_variable(u"BootNext", ..., u"Boot0001");
> > >    do_efibootmgr();
> > >
> > > That's it. My concern is what I mentioned above.
> >
> > OK. But then you would need to export those functions. I think it
> > would be better to split up the logic a bit and move things out of the
> > cmd/ directory (at some point).
>
> If we don't take "mixed-boot environments", adding efibootmger to
> bootflow mechanism, or implementing efibootmger using bootflow,
> makes little sense.

I'm not convinced of that, actually. Let's leave the discussion for
when bootflow is a little closer. I think it should be possible to
boot without needing CONFIG_CMDLINE, for example. But even without
that, a unified approach to what is available to boot and what to
select could be quite helpful I think. It can support custom flows
too, such as Chrome OS.

>
> > >
> > > Just a note:
> > > In the current distro_bootcmd, UEFI boot manager is also called
> > > *every time* one of boot media in "boot_targets" is scanned/enumerated.
> > > But it will make little sense because the current boot manager only
> > > allows/requires users to specify both the boot device and the image file
> > > path explicitly in a boot option, i.e. "BootXXXX" variable, and tries
> > > all the boot options in "BootOrder" until it successfully launches
> > > one of those images.
> >
> > Yes, is the idea of lazy boot entirely impossible? Or is it still
> > possible to do that to some extent, e.g. by scanning until you find
> > the first thing in the boot order?
>
> If I correctly understand what 'lazy boot' means here, you can do
> all the enumeration/scanning first, then call UEFI boot manager:
> => scsi rescan; usb start; mmc rescan; (and whatever else); bootefi bootmger

I mean scan the first device, try to boot it. If that doesn't work,
scan the next one. It can make the boot a lot faster.

>
> The order of scanning commands doesn't matter as "BootOrder" determine
> the priorities among BootXXXX (boot options).
> What the current UEFI cannot provides is to detect a new device that
> is attached to the board once UEFI subsystem is initialized.
>
> That issue, which I have mentioned in the ML from time to time, should be
> separately resolved as part of "integration" efforts between UEFI objects
> and driver model.

Yes, this limitation is a side effect of the implementation method. I
will try to watch out for additional 'parallel' tables being added,
but I would like to see some effort to reduce what is there.

>
> > >
> > > > >
> > > > > > Chromium OS, Android, VBE) into pluggable
> > > > > > drivers and number them as we do with partitions. For now the sequence
> > > > > > number is used to determine both the partition number and the
> > > > > > implementation to use.
> > > > > >
> > > > > > The same boot command is used as before ('bootflow scan -lb') so there is
> > > > > > no change to that. It can boot both Fedora 31 and 34, for example.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > ---
> > > > > > See u-boot-dm/bmea for the tree containing this patch and the series
> > > > > > that it relies on:
> > > > > >
> > > > > >   https://patchwork.ozlabs.org/project/uboot/list/?series=258654&state=*
> > > > > >
> > > >
> > > > [..]
> > > >
> > > > > > +static int efiload_read_file(struct blk_desc *desc, int partnum,
> > > > > > +                          struct bootflow *bflow)
> > > > > > +{
> > > > > > +     const struct udevice *media_dev;
> > > > > > +     int size = bflow->size;
> > > > > > +     char devnum_str[9];
> > > > > > +     char dirname[200];
> > > > > > +     loff_t bytes_read;
> > > > > > +     char *last_slash;
> > > > > > +     ulong addr;
> > > > > > +     char *buf;
> > > > > > +     int ret;
> > > > > > +
> > > > > > +     /* Sadly FS closes the file after fs_size() so we must redo this */
> > > > > > +     ret = fs_set_blk_dev_with_part(desc, partnum);
> > > > > > +     if (ret)
> > > > > > +             return log_msg_ret("set", ret);
> > > > > > +
> > > > > > +     buf = malloc(size + 1);
> > > > > > +     if (!buf)
> > > > > > +             return log_msg_ret("buf", -ENOMEM);
> > > > > > +     addr = map_to_sysmem(buf);
> > > > > > +
> > > > > > +     ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
> > > > > > +     if (ret) {
> > > > > > +             free(buf);
> > > > > > +             return log_msg_ret("read", ret);
> > > > > > +     }
> > > > > > +     if (size != bytes_read)
> > > > > > +             return log_msg_ret("bread", -EINVAL);
> > > > > > +     buf[size] = '\0';
> > > > > > +     bflow->state = BOOTFLOWST_LOADED;
> > > > > > +     bflow->buf = buf;
> > > > > > +
> > > > > > +     /*
> > > > > > +      * This is a horrible hack to tell EFI about this boot device. Once we
> > > > > > +      * unify EFI with the rest of U-Boot we can clean this up. The same hack
> > > > > > +      * exists in multiple places, e.g. in the fs, tftp and load commands.
> > > > >
> > > > > Which part do you call a "horrible hack"? efi_set_bootdev()?
> > > > > In fact, there are a couple of reason why we need to call this function:
> > > > > 1. to remember a device to create a dummy device path for the loaded
> > > > >    image later,
> > > > > 2. to remember a size of loaded image which is used for sanity check and
> > > > >    image authentication later,
> > > > > 3. to avoid those parameters being remembered accidentally by "loading" dtb
> > > > >    and/or other binaries than the image itself,
> > > > >
> > > > > I hope that (1) and (2) will be avoidable if we modify the current
> > > > > implementation (and bootefi syntax), and then we won't need (3).
> > > >
> > > > Yes thank you...I do understand why it is needed now, but it is
> > > > basically due to the the  fat that EFI has its own driver structures.
> > > > Once we stop those, it will go away.
> > >
> > > Here, my point is, even under the current implementation, we will
> > > be able to eliminate efi_set_bootdev() with some tweaks.
> > >
> > > In other words, even you could integrate UEFI into the device model,
> > > the issue (2), for example, would still remain unsolved.
> > > In case of (2), we use the *size* information for sanity check against
> > > image's header information as well as calculating a hash value for
> > > UEFI secure boot when efi_load_image() is called. Even if the integration
> > > is done, we need to pass on the size information to "bootefi <addr>"
> > > command implicitly or explicitly.
> >
> > If you look at 'struct bootflow' you can see that it stores the size.
> > It can easily store other info as needed by EFI. So I don't think we
> > need that hack in the end, when things are using bootflow.
>
> I'm afraid that you miss the context of my discussion.
> We support a command sequence like the following:
> => load scsi 0:1 50000000 /hellowworld.efi
> => bootefi 50000000
>
> distro_bootcmd does use this method to add "removable media" support,
> but even if the distro script is re-implemented using bootflow machanism,
> we cannot remove "horrible hack" in load command as long as we continue
> to use load command.

Actually 'bootflow' does not use the load command so I think it is a
path forward here.

>
> > >
> > > > >
> > > > > > +      * Once we can clean up the EFI code to make proper use of driver model,
> > > > > > +      * this can go away.
> > > > >
> > > > > My point is, however, that this kind of cleanup is irrelevant to
> > > > > whether we use driver model or not.
> > > >
> > > > Are you sure? Without driver model how are you going to reference a
> > > > udevice? If not that, how are you going to reference a device? The
> > > > tables in the UEFI implementation were specifically added to avoid
> > > > relying on driver model. It is a crying shame that I did not push back
> > > > harder on this at the time.
> > >
> > > I hope you will get my point in the previous comment now.
> >
> > Well yes I do, but I don't understand why there is such resistance to
> > sorting out the EFI implementation? It is quite a mess at the moment.
>
> As you know, I have a positive opinion to the integration.
> What I object to here is to integrate UEFI boot manager into bootflow.

Well I have not looked at this yet and it is not my immediate focus.
For now I can just make bootflow use the 'bootmgr' command in that
case.

>
> > I think the first step is to drop the non-DM code, but the second is
> > to drop the parallel tables that EFI keeps about each device.
>
> As Heinrich suggested in his reply, there are many concepts that might
> not be easily mapped to U-Boot counterparts. If our only concern is on
> the boot flow, it would be easier, but we would also like to support
> a lot of API, either boot/runtime services or protocols. Filling
> semantic gaps would make it harder to completely drop some sort of
> "parallel tables".

But I'm not asking for that. In fact I sent Heinrich an email recently
asking for just one thing to be removed, as a starting point.

>
> I think that the first thing that we should tackle is to eliminate
> the issue, as I mentioned above, regarding "block devices".
> It would shed light on how we could go further on this issue.

Do you mean drop the parallel tables for block devices? If so, yes
that sounds good.

Regards,
Simon

Re: [PATCH] RFC: Support an EFI-loader bootflow

[AKASHI Takahiro]

On Mon, Sep 06, 2021 at 04:41:55AM -0600, Simon Glass wrote:
> Hi Heinrich,
> 
> On Fri, 3 Sept 2021 at 10:30, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> >
> >
> > On 9/3/21 10:53 AM, Simon Glass wrote:
> > > Hi Takahiro,
> > >
> > > On Thu, 2 Sept 2021 at 20:27, AKASHI Takahiro
> > > <takahiro.akashi@linaro.org> wrote:
> > >>
> > >> Simon,
> > >>
> > >> On Thu, Sep 02, 2021 at 10:40:57AM -0600, Simon Glass wrote:
> > >>> Hi Takahiro,
> > >>>
> > >>> On Tue, 31 Aug 2021 at 00:14, AKASHI Takahiro
> > >>> <takahiro.akashi@linaro.org> wrote:
> > >>>>
> > >>>> Simon,
> > >>>>
> > >>>> On Sat, Aug 28, 2021 at 02:35:21PM -0600, Simon Glass wrote:
> > >>>>> This is just a demonstration of how to support EFI loader using bootflow.
> > >>>>> Various things need cleaning up, not least that the naming needs to be
> > >>>>> finalised. I will deal with that in the v2 series.
> > >>>>>
> > >>>>> In order to support multiple methods of booting from the same device, we
> > >>>>> should probably separate out the different implementations (syslinux,
> > >>>>> EFI loader
> > >>>>
> > >>>> I still believe that we'd better add "removable media" support
> > >>>> to UEFI boot manager first (and then probably call this functionality
> > >>      ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > >>
> > >>>> from bootflow?).
> > >>>>
> > >>>> I admit that, in this case, we will have an issue that we will not
> > >>>> recognize any device which is plugged in dynamically after UEFI
> > >>>> subsystem is initialized. But this issue will potentially exist
> > >>>> even with your approach.
> > >>>
> > >>> That can be fixed by dropping the UEFI tables and using driver model
> > >>> instead. I may have mentioned that :-)
> > >>
> > >> I'm afraid that you don't get my point above.
> > >>
> > >>>>
> > >>>>> and soon bootmgr,
> > >>>>
> > >>>> What will you expect in UEFI boot manager case?
> > >>>> Boot parameters (options) as well as the boot order are well defined
> > >>>> by BootXXXX and BootOrder variables. How are they fit into your scheme?
> > >>>
> > >>> I haven't looked at boot manager yet, but I can't imagine it
> > >>> presenting an insurmountable challenge.
> > >>
> > >> I don't say it's challenging.
> > >> Since you have not yet explained your idea about how to specify
> > >> the *boot order* in your scheme, I wonder how "BootXXXX"/"BootOrder"
> > >> be treated and honored.
> > >> There might be a parallel world again.
> > >
> > > Well as I mentioned, I haven't looked at it yet. The original question
> > > was how to do EFI LOADER and I did a patch  to show that.
> > >
> > > Are we likely to see mixed-boot environments, that use distro boot for
> > > some OSes and EFI for others? I hope not as it would be confusing. EFI
> > > is the parallel world, as I see it.
> > >
> > > It should be easy enough for the 'bootmgr' bootflow to read the EFI
> > > variables and select the correct ordering. As I understand it, EFI
> > > does not support lazy boot, so it is acceptable to probe all the
> > > devices before selecting one?
> > >
> > >>
> > >>>>
> > >>>> But anyway, we can use the following commands to run a specific
> > >>>> boot flow in UEFI world:
> > >>>> => efidebug boot next 1(or whatever else); bootefi bootmgr
> > >>>
> > >>> OK.
> > >>>
> > >>> As you probably noticed I was trying to have bootflow connect directly
> > >>> to the code that does the booting so that 'CONFIG_CMDLINE' can be
> > >>> disabled (e.g. for security reasons) and the boot will still work.
> > >>
> > >> # Maybe, it sounds kinda chicken and egg.
> > >>
> > >> Even now, you can code this way :)
> > >>
> > >>     efi_set_variable(u"BootNext", ..., u"Boot0001");
> > >>     do_efibootmgr();
> > >>
> > >> That's it. My concern is what I mentioned above.
> > >
> > > OK. But then you would need to export those functions. I think it
> > > would be better to split up the logic a bit and move things out of the
> > > cmd/ directory (at some point).
> > >
> > >>
> > >> Just a note:
> > >> In the current distro_bootcmd, UEFI boot manager is also called
> > >> *every time* one of boot media in "boot_targets" is scanned/enumerated.
> > >> But it will make little sense because the current boot manager only
> > >> allows/requires users to specify both the boot device and the image file
> > >> path explicitly in a boot option, i.e. "BootXXXX" variable, and tries
> > >> all the boot options in "BootOrder" until it successfully launches
> > >> one of those images.
> > >
> > > Yes, is the idea of lazy boot entirely impossible? Or is it still
> > > possible to do that to some extent, e.g. by scanning until you find
> > > the first thing in the boot order?
> > >
> > >>
> > >>>>
> > >>>>> Chromium OS, Android, VBE) into pluggable
> > >>>>> drivers and number them as we do with partitions. For now the sequence
> > >>>>> number is used to determine both the partition number and the
> > >>>>> implementation to use.
> > >>>>>
> > >>>>> The same boot command is used as before ('bootflow scan -lb') so there is
> > >>>>> no change to that. It can boot both Fedora 31 and 34, for example.
> > >>>>>
> > >>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
> > >>>>> ---
> > >>>>> See u-boot-dm/bmea for the tree containing this patch and the series
> > >>>>> that it relies on:
> > >>>>>
> > >>>>>    https://patchwork.ozlabs.org/project/uboot/list/?series=258654&state=*
> > >>>>>
> > >>>
> > >>> [..]
> > >>>
> > >>>>> +static int efiload_read_file(struct blk_desc *desc, int partnum,
> > >>>>> +                          struct bootflow *bflow)
> > >>>>> +{
> > >>>>> +     const struct udevice *media_dev;
> > >>>>> +     int size = bflow->size;
> > >>>>> +     char devnum_str[9];
> > >>>>> +     char dirname[200];
> > >>>>> +     loff_t bytes_read;
> > >>>>> +     char *last_slash;
> > >>>>> +     ulong addr;
> > >>>>> +     char *buf;
> > >>>>> +     int ret;
> > >>>>> +
> > >>>>> +     /* Sadly FS closes the file after fs_size() so we must redo this */
> > >>>>> +     ret = fs_set_blk_dev_with_part(desc, partnum);
> > >>>>> +     if (ret)
> > >>>>> +             return log_msg_ret("set", ret);
> > >>>>> +
> > >>>>> +     buf = malloc(size + 1);
> > >>>>> +     if (!buf)
> > >>>>> +             return log_msg_ret("buf", -ENOMEM);
> > >>>>> +     addr = map_to_sysmem(buf);
> > >>>>> +
> > >>>>> +     ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
> > >>>>> +     if (ret) {
> > >>>>> +             free(buf);
> > >>>>> +             return log_msg_ret("read", ret);
> > >>>>> +     }
> > >>>>> +     if (size != bytes_read)
> > >>>>> +             return log_msg_ret("bread", -EINVAL);
> > >>>>> +     buf[size] = '\0';
> > >>>>> +     bflow->state = BOOTFLOWST_LOADED;
> > >>>>> +     bflow->buf = buf;
> > >>>>> +
> > >>>>> +     /*
> > >>>>> +      * This is a horrible hack to tell EFI about this boot device. Once we
> > >>>>> +      * unify EFI with the rest of U-Boot we can clean this up. The same hack
> > >>>>> +      * exists in multiple places, e.g. in the fs, tftp and load commands.
> > >>>>
> > >>>> Which part do you call a "horrible hack"? efi_set_bootdev()?
> > >>>> In fact, there are a couple of reason why we need to call this function:
> > >>>> 1. to remember a device to create a dummy device path for the loaded
> > >>>>     image later,
> > >>>> 2. to remember a size of loaded image which is used for sanity check and
> > >>>>     image authentication later,
> > >>>> 3. to avoid those parameters being remembered accidentally by "loading" dtb
> > >>>>     and/or other binaries than the image itself,
> > >>>>
> > >>>> I hope that (1) and (2) will be avoidable if we modify the current
> > >>>> implementation (and bootefi syntax), and then we won't need (3).
> > >>>
> > >>> Yes thank you...I do understand why it is needed now, but it is
> > >>> basically due to the the  fat that EFI has its own driver structures.
> > >>> Once we stop those, it will go away.
> > >>
> > >> Here, my point is, even under the current implementation, we will
> > >> be able to eliminate efi_set_bootdev() with some tweaks.
> > >>
> > >> In other words, even you could integrate UEFI into the device model,
> > >> the issue (2), for example, would still remain unsolved.
> > >> In case of (2), we use the *size* information for sanity check against
> > >> image's header information as well as calculating a hash value for
> > >> UEFI secure boot when efi_load_image() is called. Even if the integration
> > >> is done, we need to pass on the size information to "bootefi <addr>"
> > >> command implicitly or explicitly.
> > >
> > > If you look at 'struct bootflow' you can see that it stores the size.
> > > It can easily store other info as needed by EFI. So I don't think we
> > > need that hack in the end, when things are using bootflow.
> > >
> > >>
> > >>>>
> > >>>>> +      * Once we can clean up the EFI code to make proper use of driver model,
> > >>>>> +      * this can go away.
> > >>>>
> > >>>> My point is, however, that this kind of cleanup is irrelevant to
> > >>>> whether we use driver model or not.
> > >>>
> > >>> Are you sure? Without driver model how are you going to reference a
> > >>> udevice? If not that, how are you going to reference a device? The
> > >>> tables in the UEFI implementation were specifically added to avoid
> > >>> relying on driver model. It is a crying shame that I did not push back
> > >>> harder on this at the time.
> > >>
> > >> I hope you will get my point in the previous comment now.
> > >
> > > Well yes I do, but I don't understand why there is such resistance to
> > > sorting out the EFI implementation? It is quite a mess at the moment.
> > > I think the first step is to drop the non-DM code, but the second is
> > > to drop the parallel tables that EFI keeps about each device.
> >
> >
> > Concerning these "parallel" tables. Please, have a look at the UEFI spec
> > to understand what a handle, a protocol interface, and an event are.
> >
> > I also gave as short overview in
> > https://archive.fosdem.org/2020/schedule/event/firmware_duwu/ starting
> > at slide 10 or 08:00 of the video.
> >
> > Handles may refer to devices, to drivers or anything else.
> > Handles are both to be created by U-Boot and by EFI binaries that we
> > execute.
> >
> > On these handles protocol interfaces can be installed. These may be
> > those defined in the UEFI spec or something completely independent. The
> > protocol interfaces contain pointers to functions and additional data.
> >
> > Further objects are events. These may contain references to an event
> > handler function that is called when the event is triggered.
> 
> Thank you for the info. I did read the entire spec some years ago
> (about 6 I think) and have dug into bits of it since. I do find the
> naming confusing so I cannot claim to understand it. The use of void *
> everywhere is a pain.. I will doubtless take another look.
> 
> U-Boot does have devices, which have a (single) uclass and API. It
> also has drivers. After all, the EFI tables are created from U-Boot
> data structures, so presumably there is some correspondence.
> 
> >
> > None of the above objects matches one to one to objects in the driver
> > model. But we can use some integration:
> >
> > When a U-Boot device is probed we must create the UEFI handle and
> > install the expected protocols on it.
> >
> > When a U-Boot device is removed we must destroy the handle. There are
> > certain conditions under which a protocol must not be removed from a
> > handle and the handle cannot be destroyed. In this case removing a
> > device should not be possible.
> 
> I wish this had been done from the beginning, but we may as well start
> now. It is quite a complicated task at this point.
> 
> Can we pick one data structure that we can make ephemeral, using only
> the DM structs and features? Can we look at what features need to be
> added (such as partition devices?) and add those to U-Boot?
> 
> What would be easiest as a starting point? How about struct
> efi_system_partition?

I have already proposed it as part of my more ambitious approach[1],
in particular, patch#11,#12 and #13.
If you want, I'm willing to re-post those (11-13 only).
But it is not so much beneficial on its own because there are a lot
of U-Boot interfaces that still take parameters, blk_desc + part_num.

> Or perhaps could we drop efi_disk_register() and have it scan the disk
> 'on the fly' when needed?

Patch#13 does this. Heinrich, instead, suggested another way[2].
(Both have a lack for "removing-device" support.)

Either way, we need to have some sort of mechanism (callback or binding)
so that we can bridge UEFI world and U-Boot environment.

-Takahiro Akashi

[1] https://lists.denx.de/pipermail/u-boot/2019-February/357923.html
[2] https://lists.denx.de/pipermail/u-boot/2021-June/452297.html

> There must be something that can be done here. I am sure there is a
> mismatch of concepts/API, but some of these mismatches are features
> that U-Boot could add and some can be 'thinned down' so that UEFI is
> not such a lot of code.
> 
> Regards,
> Simon

Re: [PATCH] RFC: Support an EFI-loader bootflow

[Simon Glass]

Hi Takahiro,

On Mon, 6 Sept 2021 at 21:58, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> On Mon, Sep 06, 2021 at 04:41:55AM -0600, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Fri, 3 Sept 2021 at 10:30, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > >
> > >
> > >
> > > On 9/3/21 10:53 AM, Simon Glass wrote:
> > > > Hi Takahiro,
> > > >
> > > > On Thu, 2 Sept 2021 at 20:27, AKASHI Takahiro
> > > > <takahiro.akashi@linaro.org> wrote:
> > > >>
> > > >> Simon,
> > > >>
> > > >> On Thu, Sep 02, 2021 at 10:40:57AM -0600, Simon Glass wrote:
> > > >>> Hi Takahiro,
> > > >>>
> > > >>> On Tue, 31 Aug 2021 at 00:14, AKASHI Takahiro
> > > >>> <takahiro.akashi@linaro.org> wrote:
> > > >>>>
> > > >>>> Simon,
> > > >>>>
> > > >>>> On Sat, Aug 28, 2021 at 02:35:21PM -0600, Simon Glass wrote:
> > > >>>>> This is just a demonstration of how to support EFI loader using bootflow.
> > > >>>>> Various things need cleaning up, not least that the naming needs to be
> > > >>>>> finalised. I will deal with that in the v2 series.
> > > >>>>>
> > > >>>>> In order to support multiple methods of booting from the same device, we
> > > >>>>> should probably separate out the different implementations (syslinux,
> > > >>>>> EFI loader
> > > >>>>
> > > >>>> I still believe that we'd better add "removable media" support
> > > >>>> to UEFI boot manager first (and then probably call this functionality
> > > >>      ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > >>
> > > >>>> from bootflow?).
> > > >>>>
> > > >>>> I admit that, in this case, we will have an issue that we will not
> > > >>>> recognize any device which is plugged in dynamically after UEFI
> > > >>>> subsystem is initialized. But this issue will potentially exist
> > > >>>> even with your approach.
> > > >>>
> > > >>> That can be fixed by dropping the UEFI tables and using driver model
> > > >>> instead. I may have mentioned that :-)
> > > >>
> > > >> I'm afraid that you don't get my point above.
> > > >>
> > > >>>>
> > > >>>>> and soon bootmgr,
> > > >>>>
> > > >>>> What will you expect in UEFI boot manager case?
> > > >>>> Boot parameters (options) as well as the boot order are well defined
> > > >>>> by BootXXXX and BootOrder variables. How are they fit into your scheme?
> > > >>>
> > > >>> I haven't looked at boot manager yet, but I can't imagine it
> > > >>> presenting an insurmountable challenge.
> > > >>
> > > >> I don't say it's challenging.
> > > >> Since you have not yet explained your idea about how to specify
> > > >> the *boot order* in your scheme, I wonder how "BootXXXX"/"BootOrder"
> > > >> be treated and honored.
> > > >> There might be a parallel world again.
> > > >
> > > > Well as I mentioned, I haven't looked at it yet. The original question
> > > > was how to do EFI LOADER and I did a patch  to show that.
> > > >
> > > > Are we likely to see mixed-boot environments, that use distro boot for
> > > > some OSes and EFI for others? I hope not as it would be confusing. EFI
> > > > is the parallel world, as I see it.
> > > >
> > > > It should be easy enough for the 'bootmgr' bootflow to read the EFI
> > > > variables and select the correct ordering. As I understand it, EFI
> > > > does not support lazy boot, so it is acceptable to probe all the
> > > > devices before selecting one?
> > > >
> > > >>
> > > >>>>
> > > >>>> But anyway, we can use the following commands to run a specific
> > > >>>> boot flow in UEFI world:
> > > >>>> => efidebug boot next 1(or whatever else); bootefi bootmgr
> > > >>>
> > > >>> OK.
> > > >>>
> > > >>> As you probably noticed I was trying to have bootflow connect directly
> > > >>> to the code that does the booting so that 'CONFIG_CMDLINE' can be
> > > >>> disabled (e.g. for security reasons) and the boot will still work.
> > > >>
> > > >> # Maybe, it sounds kinda chicken and egg.
> > > >>
> > > >> Even now, you can code this way :)
> > > >>
> > > >>     efi_set_variable(u"BootNext", ..., u"Boot0001");
> > > >>     do_efibootmgr();
> > > >>
> > > >> That's it. My concern is what I mentioned above.
> > > >
> > > > OK. But then you would need to export those functions. I think it
> > > > would be better to split up the logic a bit and move things out of the
> > > > cmd/ directory (at some point).
> > > >
> > > >>
> > > >> Just a note:
> > > >> In the current distro_bootcmd, UEFI boot manager is also called
> > > >> *every time* one of boot media in "boot_targets" is scanned/enumerated.
> > > >> But it will make little sense because the current boot manager only
> > > >> allows/requires users to specify both the boot device and the image file
> > > >> path explicitly in a boot option, i.e. "BootXXXX" variable, and tries
> > > >> all the boot options in "BootOrder" until it successfully launches
> > > >> one of those images.
> > > >
> > > > Yes, is the idea of lazy boot entirely impossible? Or is it still
> > > > possible to do that to some extent, e.g. by scanning until you find
> > > > the first thing in the boot order?
> > > >
> > > >>
> > > >>>>
> > > >>>>> Chromium OS, Android, VBE) into pluggable
> > > >>>>> drivers and number them as we do with partitions. For now the sequence
> > > >>>>> number is used to determine both the partition number and the
> > > >>>>> implementation to use.
> > > >>>>>
> > > >>>>> The same boot command is used as before ('bootflow scan -lb') so there is
> > > >>>>> no change to that. It can boot both Fedora 31 and 34, for example.
> > > >>>>>
> > > >>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
> > > >>>>> ---
> > > >>>>> See u-boot-dm/bmea for the tree containing this patch and the series
> > > >>>>> that it relies on:
> > > >>>>>
> > > >>>>>    https://patchwork.ozlabs.org/project/uboot/list/?series=258654&state=*
> > > >>>>>
> > > >>>
> > > >>> [..]
> > > >>>
> > > >>>>> +static int efiload_read_file(struct blk_desc *desc, int partnum,
> > > >>>>> +                          struct bootflow *bflow)
> > > >>>>> +{
> > > >>>>> +     const struct udevice *media_dev;
> > > >>>>> +     int size = bflow->size;
> > > >>>>> +     char devnum_str[9];
> > > >>>>> +     char dirname[200];
> > > >>>>> +     loff_t bytes_read;
> > > >>>>> +     char *last_slash;
> > > >>>>> +     ulong addr;
> > > >>>>> +     char *buf;
> > > >>>>> +     int ret;
> > > >>>>> +
> > > >>>>> +     /* Sadly FS closes the file after fs_size() so we must redo this */
> > > >>>>> +     ret = fs_set_blk_dev_with_part(desc, partnum);
> > > >>>>> +     if (ret)
> > > >>>>> +             return log_msg_ret("set", ret);
> > > >>>>> +
> > > >>>>> +     buf = malloc(size + 1);
> > > >>>>> +     if (!buf)
> > > >>>>> +             return log_msg_ret("buf", -ENOMEM);
> > > >>>>> +     addr = map_to_sysmem(buf);
> > > >>>>> +
> > > >>>>> +     ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
> > > >>>>> +     if (ret) {
> > > >>>>> +             free(buf);
> > > >>>>> +             return log_msg_ret("read", ret);
> > > >>>>> +     }
> > > >>>>> +     if (size != bytes_read)
> > > >>>>> +             return log_msg_ret("bread", -EINVAL);
> > > >>>>> +     buf[size] = '\0';
> > > >>>>> +     bflow->state = BOOTFLOWST_LOADED;
> > > >>>>> +     bflow->buf = buf;
> > > >>>>> +
> > > >>>>> +     /*
> > > >>>>> +      * This is a horrible hack to tell EFI about this boot device. Once we
> > > >>>>> +      * unify EFI with the rest of U-Boot we can clean this up. The same hack
> > > >>>>> +      * exists in multiple places, e.g. in the fs, tftp and load commands.
> > > >>>>
> > > >>>> Which part do you call a "horrible hack"? efi_set_bootdev()?
> > > >>>> In fact, there are a couple of reason why we need to call this function:
> > > >>>> 1. to remember a device to create a dummy device path for the loaded
> > > >>>>     image later,
> > > >>>> 2. to remember a size of loaded image which is used for sanity check and
> > > >>>>     image authentication later,
> > > >>>> 3. to avoid those parameters being remembered accidentally by "loading" dtb
> > > >>>>     and/or other binaries than the image itself,
> > > >>>>
> > > >>>> I hope that (1) and (2) will be avoidable if we modify the current
> > > >>>> implementation (and bootefi syntax), and then we won't need (3).
> > > >>>
> > > >>> Yes thank you...I do understand why it is needed now, but it is
> > > >>> basically due to the the  fat that EFI has its own driver structures.
> > > >>> Once we stop those, it will go away.
> > > >>
> > > >> Here, my point is, even under the current implementation, we will
> > > >> be able to eliminate efi_set_bootdev() with some tweaks.
> > > >>
> > > >> In other words, even you could integrate UEFI into the device model,
> > > >> the issue (2), for example, would still remain unsolved.
> > > >> In case of (2), we use the *size* information for sanity check against
> > > >> image's header information as well as calculating a hash value for
> > > >> UEFI secure boot when efi_load_image() is called. Even if the integration
> > > >> is done, we need to pass on the size information to "bootefi <addr>"
> > > >> command implicitly or explicitly.
> > > >
> > > > If you look at 'struct bootflow' you can see that it stores the size.
> > > > It can easily store other info as needed by EFI. So I don't think we
> > > > need that hack in the end, when things are using bootflow.
> > > >
> > > >>
> > > >>>>
> > > >>>>> +      * Once we can clean up the EFI code to make proper use of driver model,
> > > >>>>> +      * this can go away.
> > > >>>>
> > > >>>> My point is, however, that this kind of cleanup is irrelevant to
> > > >>>> whether we use driver model or not.
> > > >>>
> > > >>> Are you sure? Without driver model how are you going to reference a
> > > >>> udevice? If not that, how are you going to reference a device? The
> > > >>> tables in the UEFI implementation were specifically added to avoid
> > > >>> relying on driver model. It is a crying shame that I did not push back
> > > >>> harder on this at the time.
> > > >>
> > > >> I hope you will get my point in the previous comment now.
> > > >
> > > > Well yes I do, but I don't understand why there is such resistance to
> > > > sorting out the EFI implementation? It is quite a mess at the moment.
> > > > I think the first step is to drop the non-DM code, but the second is
> > > > to drop the parallel tables that EFI keeps about each device.
> > >
> > >
> > > Concerning these "parallel" tables. Please, have a look at the UEFI spec
> > > to understand what a handle, a protocol interface, and an event are.
> > >
> > > I also gave as short overview in
> > > https://archive.fosdem.org/2020/schedule/event/firmware_duwu/ starting
> > > at slide 10 or 08:00 of the video.
> > >
> > > Handles may refer to devices, to drivers or anything else.
> > > Handles are both to be created by U-Boot and by EFI binaries that we
> > > execute.
> > >
> > > On these handles protocol interfaces can be installed. These may be
> > > those defined in the UEFI spec or something completely independent. The
> > > protocol interfaces contain pointers to functions and additional data.
> > >
> > > Further objects are events. These may contain references to an event
> > > handler function that is called when the event is triggered.
> >
> > Thank you for the info. I did read the entire spec some years ago
> > (about 6 I think) and have dug into bits of it since. I do find the
> > naming confusing so I cannot claim to understand it. The use of void *
> > everywhere is a pain.. I will doubtless take another look.
> >
> > U-Boot does have devices, which have a (single) uclass and API. It
> > also has drivers. After all, the EFI tables are created from U-Boot
> > data structures, so presumably there is some correspondence.
> >
> > >
> > > None of the above objects matches one to one to objects in the driver
> > > model. But we can use some integration:
> > >
> > > When a U-Boot device is probed we must create the UEFI handle and
> > > install the expected protocols on it.
> > >
> > > When a U-Boot device is removed we must destroy the handle. There are
> > > certain conditions under which a protocol must not be removed from a
> > > handle and the handle cannot be destroyed. In this case removing a
> > > device should not be possible.
> >
> > I wish this had been done from the beginning, but we may as well start
> > now. It is quite a complicated task at this point.
> >
> > Can we pick one data structure that we can make ephemeral, using only
> > the DM structs and features? Can we look at what features need to be
> > added (such as partition devices?) and add those to U-Boot?
> >
> > What would be easiest as a starting point? How about struct
> > efi_system_partition?
>
> I have already proposed it as part of my more ambitious approach[1],
> in particular, patch#11,#12 and #13.
> If you want, I'm willing to re-post those (11-13 only).
> But it is not so much beneficial on its own because there are a lot
> of U-Boot interfaces that still take parameters, blk_desc + part_num.
>
> > Or perhaps could we drop efi_disk_register() and have it scan the disk
> > 'on the fly' when needed?
>
> Patch#13 does this. Heinrich, instead, suggested another way[2].
> (Both have a lack for "removing-device" support.)
>
> Either way, we need to have some sort of mechanism (callback or binding)
> so that we can bridge UEFI world and U-Boot environment.

Ilias pointed me to your patches from a few years ago that seemed to
get stuck in review. To me they point the way forward. Do you think
you could report the whole series?


- Simon

>
> -Takahiro Akashi
>
> [1] https://lists.denx.de/pipermail/u-boot/2019-February/357923.html
> [2] https://lists.denx.de/pipermail/u-boot/2021-June/452297.html
>
> > There must be something that can be done here. I am sure there is a
> > mismatch of concepts/API, but some of these mismatches are features
> > that U-Boot could add and some can be 'thinned down' so that UEFI is
> > not such a lot of code.
> >
> > Regards,
> > Simon

Re: [PATCH] RFC: Support an EFI-loader bootflow

[AKASHI Takahiro]

On Wed, Sep 29, 2021 at 10:08:48PM -0600, Simon Glass wrote:
> Hi Takahiro,
> 
> On Mon, 6 Sept 2021 at 21:58, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > On Mon, Sep 06, 2021 at 04:41:55AM -0600, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Fri, 3 Sept 2021 at 10:30, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > >
> > > >
> > > >
> > > > On 9/3/21 10:53 AM, Simon Glass wrote:
> > > > > Hi Takahiro,
> > > > >
> > > > > On Thu, 2 Sept 2021 at 20:27, AKASHI Takahiro
> > > > > <takahiro.akashi@linaro.org> wrote:
> > > > >>
> > > > >> Simon,
> > > > >>
> > > > >> On Thu, Sep 02, 2021 at 10:40:57AM -0600, Simon Glass wrote:
> > > > >>> Hi Takahiro,
> > > > >>>
> > > > >>> On Tue, 31 Aug 2021 at 00:14, AKASHI Takahiro
> > > > >>> <takahiro.akashi@linaro.org> wrote:
> > > > >>>>
> > > > >>>> Simon,
> > > > >>>>
> > > > >>>> On Sat, Aug 28, 2021 at 02:35:21PM -0600, Simon Glass wrote:
> > > > >>>>> This is just a demonstration of how to support EFI loader using bootflow.
> > > > >>>>> Various things need cleaning up, not least that the naming needs to be
> > > > >>>>> finalised. I will deal with that in the v2 series.
> > > > >>>>>
> > > > >>>>> In order to support multiple methods of booting from the same device, we
> > > > >>>>> should probably separate out the different implementations (syslinux,
> > > > >>>>> EFI loader
> > > > >>>>
> > > > >>>> I still believe that we'd better add "removable media" support
> > > > >>>> to UEFI boot manager first (and then probably call this functionality
> > > > >>      ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > > >>
> > > > >>>> from bootflow?).
> > > > >>>>
> > > > >>>> I admit that, in this case, we will have an issue that we will not
> > > > >>>> recognize any device which is plugged in dynamically after UEFI
> > > > >>>> subsystem is initialized. But this issue will potentially exist
> > > > >>>> even with your approach.
> > > > >>>
> > > > >>> That can be fixed by dropping the UEFI tables and using driver model
> > > > >>> instead. I may have mentioned that :-)
> > > > >>
> > > > >> I'm afraid that you don't get my point above.
> > > > >>
> > > > >>>>
> > > > >>>>> and soon bootmgr,
> > > > >>>>
> > > > >>>> What will you expect in UEFI boot manager case?
> > > > >>>> Boot parameters (options) as well as the boot order are well defined
> > > > >>>> by BootXXXX and BootOrder variables. How are they fit into your scheme?
> > > > >>>
> > > > >>> I haven't looked at boot manager yet, but I can't imagine it
> > > > >>> presenting an insurmountable challenge.
> > > > >>
> > > > >> I don't say it's challenging.
> > > > >> Since you have not yet explained your idea about how to specify
> > > > >> the *boot order* in your scheme, I wonder how "BootXXXX"/"BootOrder"
> > > > >> be treated and honored.
> > > > >> There might be a parallel world again.
> > > > >
> > > > > Well as I mentioned, I haven't looked at it yet. The original question
> > > > > was how to do EFI LOADER and I did a patch  to show that.
> > > > >
> > > > > Are we likely to see mixed-boot environments, that use distro boot for
> > > > > some OSes and EFI for others? I hope not as it would be confusing. EFI
> > > > > is the parallel world, as I see it.
> > > > >
> > > > > It should be easy enough for the 'bootmgr' bootflow to read the EFI
> > > > > variables and select the correct ordering. As I understand it, EFI
> > > > > does not support lazy boot, so it is acceptable to probe all the
> > > > > devices before selecting one?
> > > > >
> > > > >>
> > > > >>>>
> > > > >>>> But anyway, we can use the following commands to run a specific
> > > > >>>> boot flow in UEFI world:
> > > > >>>> => efidebug boot next 1(or whatever else); bootefi bootmgr
> > > > >>>
> > > > >>> OK.
> > > > >>>
> > > > >>> As you probably noticed I was trying to have bootflow connect directly
> > > > >>> to the code that does the booting so that 'CONFIG_CMDLINE' can be
> > > > >>> disabled (e.g. for security reasons) and the boot will still work.
> > > > >>
> > > > >> # Maybe, it sounds kinda chicken and egg.
> > > > >>
> > > > >> Even now, you can code this way :)
> > > > >>
> > > > >>     efi_set_variable(u"BootNext", ..., u"Boot0001");
> > > > >>     do_efibootmgr();
> > > > >>
> > > > >> That's it. My concern is what I mentioned above.
> > > > >
> > > > > OK. But then you would need to export those functions. I think it
> > > > > would be better to split up the logic a bit and move things out of the
> > > > > cmd/ directory (at some point).
> > > > >
> > > > >>
> > > > >> Just a note:
> > > > >> In the current distro_bootcmd, UEFI boot manager is also called
> > > > >> *every time* one of boot media in "boot_targets" is scanned/enumerated.
> > > > >> But it will make little sense because the current boot manager only
> > > > >> allows/requires users to specify both the boot device and the image file
> > > > >> path explicitly in a boot option, i.e. "BootXXXX" variable, and tries
> > > > >> all the boot options in "BootOrder" until it successfully launches
> > > > >> one of those images.
> > > > >
> > > > > Yes, is the idea of lazy boot entirely impossible? Or is it still
> > > > > possible to do that to some extent, e.g. by scanning until you find
> > > > > the first thing in the boot order?
> > > > >
> > > > >>
> > > > >>>>
> > > > >>>>> Chromium OS, Android, VBE) into pluggable
> > > > >>>>> drivers and number them as we do with partitions. For now the sequence
> > > > >>>>> number is used to determine both the partition number and the
> > > > >>>>> implementation to use.
> > > > >>>>>
> > > > >>>>> The same boot command is used as before ('bootflow scan -lb') so there is
> > > > >>>>> no change to that. It can boot both Fedora 31 and 34, for example.
> > > > >>>>>
> > > > >>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > >>>>> ---
> > > > >>>>> See u-boot-dm/bmea for the tree containing this patch and the series
> > > > >>>>> that it relies on:
> > > > >>>>>
> > > > >>>>>    https://patchwork.ozlabs.org/project/uboot/list/?series=258654&state=*
> > > > >>>>>
> > > > >>>
> > > > >>> [..]
> > > > >>>
> > > > >>>>> +static int efiload_read_file(struct blk_desc *desc, int partnum,
> > > > >>>>> +                          struct bootflow *bflow)
> > > > >>>>> +{
> > > > >>>>> +     const struct udevice *media_dev;
> > > > >>>>> +     int size = bflow->size;
> > > > >>>>> +     char devnum_str[9];
> > > > >>>>> +     char dirname[200];
> > > > >>>>> +     loff_t bytes_read;
> > > > >>>>> +     char *last_slash;
> > > > >>>>> +     ulong addr;
> > > > >>>>> +     char *buf;
> > > > >>>>> +     int ret;
> > > > >>>>> +
> > > > >>>>> +     /* Sadly FS closes the file after fs_size() so we must redo this */
> > > > >>>>> +     ret = fs_set_blk_dev_with_part(desc, partnum);
> > > > >>>>> +     if (ret)
> > > > >>>>> +             return log_msg_ret("set", ret);
> > > > >>>>> +
> > > > >>>>> +     buf = malloc(size + 1);
> > > > >>>>> +     if (!buf)
> > > > >>>>> +             return log_msg_ret("buf", -ENOMEM);
> > > > >>>>> +     addr = map_to_sysmem(buf);
> > > > >>>>> +
> > > > >>>>> +     ret = fs_read(bflow->fname, addr, 0, 0, &bytes_read);
> > > > >>>>> +     if (ret) {
> > > > >>>>> +             free(buf);
> > > > >>>>> +             return log_msg_ret("read", ret);
> > > > >>>>> +     }
> > > > >>>>> +     if (size != bytes_read)
> > > > >>>>> +             return log_msg_ret("bread", -EINVAL);
> > > > >>>>> +     buf[size] = '\0';
> > > > >>>>> +     bflow->state = BOOTFLOWST_LOADED;
> > > > >>>>> +     bflow->buf = buf;
> > > > >>>>> +
> > > > >>>>> +     /*
> > > > >>>>> +      * This is a horrible hack to tell EFI about this boot device. Once we
> > > > >>>>> +      * unify EFI with the rest of U-Boot we can clean this up. The same hack
> > > > >>>>> +      * exists in multiple places, e.g. in the fs, tftp and load commands.
> > > > >>>>
> > > > >>>> Which part do you call a "horrible hack"? efi_set_bootdev()?
> > > > >>>> In fact, there are a couple of reason why we need to call this function:
> > > > >>>> 1. to remember a device to create a dummy device path for the loaded
> > > > >>>>     image later,
> > > > >>>> 2. to remember a size of loaded image which is used for sanity check and
> > > > >>>>     image authentication later,
> > > > >>>> 3. to avoid those parameters being remembered accidentally by "loading" dtb
> > > > >>>>     and/or other binaries than the image itself,
> > > > >>>>
> > > > >>>> I hope that (1) and (2) will be avoidable if we modify the current
> > > > >>>> implementation (and bootefi syntax), and then we won't need (3).
> > > > >>>
> > > > >>> Yes thank you...I do understand why it is needed now, but it is
> > > > >>> basically due to the the  fat that EFI has its own driver structures.
> > > > >>> Once we stop those, it will go away.
> > > > >>
> > > > >> Here, my point is, even under the current implementation, we will
> > > > >> be able to eliminate efi_set_bootdev() with some tweaks.
> > > > >>
> > > > >> In other words, even you could integrate UEFI into the device model,
> > > > >> the issue (2), for example, would still remain unsolved.
> > > > >> In case of (2), we use the *size* information for sanity check against
> > > > >> image's header information as well as calculating a hash value for
> > > > >> UEFI secure boot when efi_load_image() is called. Even if the integration
> > > > >> is done, we need to pass on the size information to "bootefi <addr>"
> > > > >> command implicitly or explicitly.
> > > > >
> > > > > If you look at 'struct bootflow' you can see that it stores the size.
> > > > > It can easily store other info as needed by EFI. So I don't think we
> > > > > need that hack in the end, when things are using bootflow.
> > > > >
> > > > >>
> > > > >>>>
> > > > >>>>> +      * Once we can clean up the EFI code to make proper use of driver model,
> > > > >>>>> +      * this can go away.
> > > > >>>>
> > > > >>>> My point is, however, that this kind of cleanup is irrelevant to
> > > > >>>> whether we use driver model or not.
> > > > >>>
> > > > >>> Are you sure? Without driver model how are you going to reference a
> > > > >>> udevice? If not that, how are you going to reference a device? The
> > > > >>> tables in the UEFI implementation were specifically added to avoid
> > > > >>> relying on driver model. It is a crying shame that I did not push back
> > > > >>> harder on this at the time.
> > > > >>
> > > > >> I hope you will get my point in the previous comment now.
> > > > >
> > > > > Well yes I do, but I don't understand why there is such resistance to
> > > > > sorting out the EFI implementation? It is quite a mess at the moment.
> > > > > I think the first step is to drop the non-DM code, but the second is
> > > > > to drop the parallel tables that EFI keeps about each device.
> > > >
> > > >
> > > > Concerning these "parallel" tables. Please, have a look at the UEFI spec
> > > > to understand what a handle, a protocol interface, and an event are.
> > > >
> > > > I also gave as short overview in
> > > > https://archive.fosdem.org/2020/schedule/event/firmware_duwu/ starting
> > > > at slide 10 or 08:00 of the video.
> > > >
> > > > Handles may refer to devices, to drivers or anything else.
> > > > Handles are both to be created by U-Boot and by EFI binaries that we
> > > > execute.
> > > >
> > > > On these handles protocol interfaces can be installed. These may be
> > > > those defined in the UEFI spec or something completely independent. The
> > > > protocol interfaces contain pointers to functions and additional data.
> > > >
> > > > Further objects are events. These may contain references to an event
> > > > handler function that is called when the event is triggered.
> > >
> > > Thank you for the info. I did read the entire spec some years ago
> > > (about 6 I think) and have dug into bits of it since. I do find the
> > > naming confusing so I cannot claim to understand it. The use of void *
> > > everywhere is a pain.. I will doubtless take another look.
> > >
> > > U-Boot does have devices, which have a (single) uclass and API. It
> > > also has drivers. After all, the EFI tables are created from U-Boot
> > > data structures, so presumably there is some correspondence.
> > >
> > > >
> > > > None of the above objects matches one to one to objects in the driver
> > > > model. But we can use some integration:
> > > >
> > > > When a U-Boot device is probed we must create the UEFI handle and
> > > > install the expected protocols on it.
> > > >
> > > > When a U-Boot device is removed we must destroy the handle. There are
> > > > certain conditions under which a protocol must not be removed from a
> > > > handle and the handle cannot be destroyed. In this case removing a
> > > > device should not be possible.
> > >
> > > I wish this had been done from the beginning, but we may as well start
> > > now. It is quite a complicated task at this point.
> > >
> > > Can we pick one data structure that we can make ephemeral, using only
> > > the DM structs and features? Can we look at what features need to be
> > > added (such as partition devices?) and add those to U-Boot?
> > >
> > > What would be easiest as a starting point? How about struct
> > > efi_system_partition?
> >
> > I have already proposed it as part of my more ambitious approach[1],
> > in particular, patch#11,#12 and #13.
> > If you want, I'm willing to re-post those (11-13 only).
> > But it is not so much beneficial on its own because there are a lot
> > of U-Boot interfaces that still take parameters, blk_desc + part_num.
> >
> > > Or perhaps could we drop efi_disk_register() and have it scan the disk
> > > 'on the fly' when needed?
> >
> > Patch#13 does this. Heinrich, instead, suggested another way[2].
> > (Both have a lack for "removing-device" support.)
> >
> > Either way, we need to have some sort of mechanism (callback or binding)
> > so that we can bridge UEFI world and U-Boot environment.
> 
> Ilias pointed me to your patches from a few years ago that seemed to
> get stuck in review. To me they point the way forward. Do you think
> you could report the whole series?

I have now resumed my previous work, with focus on better integration of
U-Boot block devices and UEFI disks.
You will see my RFC soon (later today or tomorrow?).

-Takahiro Akashi

> 
> - Simon
> 
> >
> > -Takahiro Akashi
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2019-February/357923.html
> > [2] https://lists.denx.de/pipermail/u-boot/2021-June/452297.html
> >
> > > There must be something that can be done here. I am sure there is a
> > > mismatch of concepts/API, but some of these mismatches are features
> > > that U-Boot could add and some can be 'thinned down' so that UEFI is
> > > not such a lot of code.
> > >
> > > Regards,
> > > Simon