[PATCH] RFC: Support an EFI-loader bootflow

AKASHI Takahiro takahiro.akashi at linaro.org
Tue Aug 31 08:14:36 CEST 2021


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 at 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 at 7e202000: 0, sdhci at 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 at 7e980000: USB DWC2
> scanning bus usb at 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 at 7e202000.bootmethod':
>   0  efi-loader   loaded  mmc          1  mmc at 7e202000.bootmethod.p efi/boot/bootarm.efi
> ** Booting bootflow 'mmc at 7e202000.bootmethod.part_1'
> Scanning disk mmc at 7e202000.blk...
> ** Unrecognized filesystem type **
> Card did not respond to voltage select! : -110
> Scanning disk sdhci at 7e300000.blk...
> Disk sdhci at 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 at 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 at 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 at 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
> 


More information about the U-Boot mailing list