[PATCH 6/8 v2] efi_loader: Replace config option with EFI variable for initrd loading

[AKASHI Takahiro]

On Wed, Dec 30, 2020 at 05:07:18PM +0200, Ilias Apalodimas wrote:
> Up to now we install EFI_LOAD_FILE2_PROTOCOL to load an initrd
> unconditionally. Although we correctly return various EFI exit codes
> depending on the file status (i.e EFI_NO_MEDIA, EFI_NOT_FOUND etc), the
> kernel loader, only falls back to the cmdline interpreted initrd if the
> protocol is not installed.
> 
> This creates a problem for EFI installers, since they won't be able to
> load their own initrd and continue the installation. It also makes the 
> feature hard to use, since we can either have a single initrd or we have
> to recompile u-boot if the filename changes.
> 
> So let's introduce a different logic that will decouple the initrd
> path from the config option we currently have.
> When the EFI application is launched through the bootmgr, we'll try to
> match the BootCurrent value to an Initrd#### EFI variable.
> e.g Boot0000 -> Initrd0000, Boot0010 -> Initrd0010 etc.

Do you think this semantics be kept U-boot specific?
It seems that it can work for any other EFI firmware (implementation).

> The Initrd#### EFI variable is expected to include the full file path,
> i.e 'mmc 0:1 initrd'.  If the file is found, we'll install the

If so, the content of "Initrd####" should contain a generic EFI
representation of file path.
Please note that "Boot####" internally contains a flattened string of
"EFI device path" to the image while efidebug command accepts a style of
"mmc 0:1 image" as arguments to "efidebug boot add" sub-command.

-Takahiro Akashi

> appropriate protocol so the kernel's efi-stub can use it and load our 
> initrd. If the file is not found the kernel will still try to load an 
> initrd parsing the kernel cmdline, since the protocol won't be installed.
> 
> This opens up another path using U-Boot and defines a new boot flow.
> A user will be able to control the kernel/initrd pairs without explicit
> cmdline args or GRUB. So we can base the whole boot flow on the Boot####
> and Initrd#### paired values.
> 
> Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
>  cmd/bootefi.c                    | 13 +++++
>  include/efi_loader.h             |  2 +-
>  lib/efi_loader/Kconfig           | 13 ++---
>  lib/efi_loader/efi_load_initrd.c | 93 ++++++++++----------------------
>  4 files changed, 46 insertions(+), 75 deletions(-)
> 
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index fdf909f8da2c..0234b0df2258 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -377,6 +377,19 @@ static int do_efibootmgr(void)
>  		return CMD_RET_FAILURE;
>  	}
>  
> +	/* efi_exit() will delete all protocols and the handle itself on exit */
> +	if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> +		ret = efi_initrd_register(&handle);
> +		/* Just warn the user. If the installation fails, the kernel
> +		 * either load an initrd specificied in the cmdline or fail
> +		 * to boot.
> +		 * If we decide to fail the command here we need to uninstall
> +		 * the protocols efi_bootmgr_load() installed
> +		 */
> +		if (ret != EFI_SUCCESS)
> +			log_warning("EFI boot manager: Failed to install Loadfile2 for initrd\n");
> +	}
> +
>  	ret = do_bootefi_exec(handle, load_options);
>  
>  	if (ret != EFI_SUCCESS)
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index af30dbafab77..000cc942e31c 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -422,7 +422,7 @@ efi_status_t efi_gop_register(void);
>  efi_status_t efi_net_register(void);
>  /* Called by bootefi to make the watchdog available */
>  efi_status_t efi_watchdog_register(void);
> -efi_status_t efi_initrd_register(void);
> +efi_status_t efi_initrd_register(efi_handle_t *initrd_handle);
>  /* Called by bootefi to make SMBIOS tables available */
>  /**
>   * efi_acpi_register() - write out ACPI tables
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index dd8b93bd3c5a..96b5934a221d 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -212,14 +212,11 @@ config EFI_LOAD_FILE2_INITRD
>  	help
>  	  Expose a EFI_FILE_LOAD2_PROTOCOL that the Linux UEFI stub can
>  	  use to load the initial ramdisk. Once this is enabled using
> -	  initrd=<ramdisk> will stop working.
> -
> -config EFI_INITRD_FILESPEC
> -	string "initramfs path"
> -	default "host 0:1 initrd"
> -	depends on EFI_LOAD_FILE2_INITRD
> -	help
> -	  Full path of the initramfs file, e.g. mmc 0:2 initramfs.cpio.gz.
> +	  initrd=<ramdisk> will stop working. The protocol will only be
> +	  installed if bootmgr is used and the file is found on the defined
> +	  path. A boot entry of Boot0001 will try to match Initrd0001 and use
> +	  it. Initrd EFI variable format should be '<dev> <part> <filename>'
> +	  i.e 'mmc 0:1 boot/initrd'
>  
>  config EFI_SECURE_BOOT
>  	bool "Enable EFI secure boot support"
> diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> index 4ec55d70979d..6289957c97bc 100644
> --- a/lib/efi_loader/efi_load_initrd.c
> +++ b/lib/efi_loader/efi_load_initrd.c
> @@ -5,7 +5,9 @@
>  
>  #include <common.h>
>  #include <efi_loader.h>
> +#include <efi_helper.h>
>  #include <efi_load_initrd.h>
> +#include <efi_variable.h>
>  #include <fs.h>
>  #include <malloc.h>
>  #include <mapmem.h>
> @@ -43,40 +45,7 @@ static const struct efi_initrd_dp dp = {
>  };
>  
>  /**
> - * get_file_size() - retrieve the size of initramfs, set efi status on error
> - *
> - * @dev:			device to read from, e.g. "mmc"
> - * @part:			device partition, e.g. "0:1"
> - * @file:			name of file
> - * @status:			EFI exit code in case of failure
> - *
> - * Return:			size of file
> - */
> -static loff_t get_file_size(const char *dev, const char *part, const char *file,
> -			    efi_status_t *status)
> -{
> -	loff_t sz = 0;
> -	int ret;
> -
> -	ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
> -	if (ret) {
> -		*status = EFI_NO_MEDIA;
> -		goto out;
> -	}
> -
> -	ret = fs_size(file, &sz);
> -	if (ret) {
> -		sz = 0;
> -		*status = EFI_NOT_FOUND;
> -		goto out;
> -	}
> -
> -out:
> -	return sz;
> -}
> -
> -/**
> - * efi_load_file2initrd() - load initial RAM disk
> + * efi_load_file2_initrd() - load initial RAM disk
>   *
>   * This function implements the LoadFile service of the EFI_LOAD_FILE2_PROTOCOL
>   * in order to load an initial RAM disk requested by the Linux kernel stub.
> @@ -96,21 +65,15 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
>  		      struct efi_device_path *file_path, bool boot_policy,
>  		      efi_uintn_t *buffer_size, void *buffer)
>  {
> -	char *filespec;
>  	efi_status_t status = EFI_NOT_FOUND;
>  	loff_t file_sz = 0, read_sz = 0;
> -	char *dev, *part, *file;
> -	char *pos;
>  	int ret;
> +	struct load_file_info info;
> +	char initrd[] = "Initrd";
>  
>  	EFI_ENTRY("%p, %p, %d, %p, %p", this, file_path, boot_policy,
>  		  buffer_size, buffer);
>  
> -	filespec = strdup(CONFIG_EFI_INITRD_FILESPEC);
> -	if (!filespec)
> -		goto out;
> -	pos = filespec;
> -
>  	if (!this || this != &efi_lf2_protocol ||
>  	    !buffer_size) {
>  		status = EFI_INVALID_PARAMETER;
> @@ -128,24 +91,11 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
>  		goto out;
>  	}
>  
> -	/*
> -	 * expect a string with three space separated parts:
> -	 *
> -	 * * a block device type, e.g. "mmc"
> -	 * * a device and partition identifier, e.g. "0:1"
> -	 * * a file path on the block device, e.g. "/boot/initrd.cpio.gz"
> -	 */
> -	dev = strsep(&pos, " ");
> -	if (!dev)
> -		goto out;
> -	part = strsep(&pos, " ");
> -	if (!part)
> -		goto out;
> -	file = strsep(&pos, " ");
> -	if (!file)
> +	status = efi_get_fp_from_var(initrd, &info);
> +	if (status != EFI_SUCCESS)
>  		goto out;
>  
> -	file_sz = get_file_size(dev, part, file, &status);
> +	file_sz = get_file_size(&info, &status);
>  	if (!file_sz)
>  		goto out;
>  
> @@ -153,23 +103,25 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
>  		status = EFI_BUFFER_TOO_SMALL;
>  		*buffer_size = file_sz;
>  	} else {
> -		ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
> +		ret = fs_set_blk_dev(info.dev, info.part,
> +				     FS_TYPE_ANY);
>  		if (ret) {
>  			status = EFI_NO_MEDIA;
>  			goto out;
>  		}
>  
> -		ret = fs_read(file, map_to_sysmem(buffer), 0, *buffer_size,
> -			      &read_sz);
> -		if (ret || read_sz != file_sz)
> +		ret = fs_read(info.filename, map_to_sysmem(buffer), 0,
> +			      *buffer_size, &read_sz);
> +		if (ret || read_sz != file_sz) {
> +			status = EFI_DEVICE_ERROR;
>  			goto out;
> +		}
>  		*buffer_size = read_sz;
>  
>  		status = EFI_SUCCESS;
>  	}
>  
>  out:
> -	free(filespec);
>  	return EFI_EXIT(status);
>  }
>  
> @@ -183,13 +135,22 @@ out:
>   *
>   * Return:	status code
>   */
> -efi_status_t efi_initrd_register(void)
> +efi_status_t efi_initrd_register(efi_handle_t *efi_initrd_handle)
>  {
> -	efi_handle_t efi_initrd_handle = NULL;
>  	efi_status_t ret;
> +	struct load_file_info info;
> +	char initrd[] = "Initrd";
> +
> +	ret = efi_get_fp_from_var(initrd, &info);
> +	/*
> +	 * Don't fail here. If we don't install the protocol the efi-stub will
> +	 * try to load and initrd parsing the kernel cmdline
> +	 */
> +	if (ret != EFI_SUCCESS)
> +		return EFI_SUCCESS;
>  
>  	ret = EFI_CALL(efi_install_multiple_protocol_interfaces
> -		       (&efi_initrd_handle,
> +		       (efi_initrd_handle,
>  			/* initramfs */
>  			&efi_guid_device_path, &dp,
>  			/* LOAD_FILE2 */
> -- 
> 2.30.0
>