[PATCH 4/6 v2] efi_loader: Replace config option for initrd loading

[Heinrich Schuchardt]

On 3/13/21 10:47 PM, 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 defining a UEFI BootXXXX we can use the filepathlist and store
> a file path pointing to our initrd. Specifically the EFI spec describes:
>
> "The first element of the array is a device path that describes the device
> and location of the Image for this load option. Other device paths may
> optionally exist in the FilePathList, but their usage is OSV specific".
> That means we can install a device path to our initrd(s) after the loaded
> image looking like this:
> VenMedia - initrd1 - end node (0x01) initrd2 - end node (0xff)
>
> When the EFI application is launched through the bootmgr, we'll try to
> interpret the extra device path. If that points to a file that exists on
> our disk, we'll now install the load_file2 and the efi-stub will be able
> to use it.
>
> 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.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
>   cmd/bootefi.c                    |   3 +
>   include/efi_loader.h             |   1 +
>   lib/efi_loader/Kconfig           |  15 +--
>   lib/efi_loader/efi_bootmgr.c     |  19 +++-
>   lib/efi_loader/efi_load_initrd.c | 189 ++++++++++++++++++-------------
>   5 files changed, 136 insertions(+), 91 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 271b385edea6..cba81ffe75e4 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -358,6 +358,9 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
>
>   	free(load_options);
>
> +	if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD))
> +		efi_initrd_deregister();
> +
>   	return ret;
>   }
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index aa812a9a3052..9e57eb37ff28 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -431,6 +431,7 @@ 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);
> +void efi_initrd_deregister(void);
>   /* 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 e729f727df11..029f0e515f57 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -317,16 +317,11 @@ config EFI_LOAD_FILE2_INITRD
>   	bool "EFI_FILE_LOAD2_PROTOCOL for Linux initial ramdisk"
>   	default n
>   	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.
> +		Linux v5.7 and later can make use of this option. If the boot option
> +		selected by the UEFI boot manager specifies an existing file to be used
> +		as initial RAM disk, a Linux specific Load File2 protocol will be
> +		installed and Linux 5.7+ will ignore any initrd=<ramdisk> command line
> +		argument.
>
>   config EFI_SECURE_BOOT
>   	bool "Enable EFI secure boot support"
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 25f5cebfdb67..46c8011344b9 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -118,11 +118,13 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
>   		ret = efi_set_variable_int(L"BootCurrent",
>   					   &efi_global_variable_guid,
>   					   attributes, sizeof(n), &n, false);
> -		if (ret != EFI_SUCCESS) {
> -			if (EFI_CALL(efi_unload_image(*handle))
> -			    != EFI_SUCCESS)
> -				log_err("Unloading image failed\n");
> -			goto error;
> +		if (ret != EFI_SUCCESS)
> +			goto unload;
> +		/* try to register load file2 for initrd's */
> +		if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> +			ret = efi_initrd_register();
> +			if (ret != EFI_SUCCESS)
> +				goto unload;
>   		}
>
>   		log_info("Booting: %ls\n", lo.label);
> @@ -146,6 +148,13 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,
>   error:
>   	free(load_option);
>
> +	return ret;
> +
> +unload:
> +	if (EFI_CALL(efi_unload_image(*handle)) != EFI_SUCCESS)
> +		log_err("Unloading image failed\n");
> +	free(load_option);
> +
>   	return ret;
>   }
>
> diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> index b9ee8839054f..e76de30808e3 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>
> @@ -39,41 +41,40 @@ static const struct efi_initrd_dp dp = {
>   	}
>   };
>
> +static efi_handle_t efi_initrd_handle;
> +
>   /**
> - * get_file_size() - retrieve the size of initramfs, set efi status on error
> + * get_initrd_fp() - Get initrd device path from a FilePathList device path
>    *
> - * @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
> + * @initrd_fp:			the final initrd filepath
>    *
> - * Return:			size of file
> + * Return:			status code. Caller must free initrd_fp
>    */
> -static loff_t get_file_size(const char *dev, const char *part, const char *file,
> -			    efi_status_t *status)
> +static efi_status_t get_initrd_fp(struct efi_device_path **initrd_fp)
>   {
> -	loff_t sz = 0;
> -	int ret;
> -
> -	ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
> -	if (ret) {
> -		*status = EFI_NO_MEDIA;
> -		goto out;
> -	}
> +	const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID;
> +	struct efi_device_path *dp = NULL;

This variable name shadows the variable defined in line 28. I think we
should give the global variable a better name then simply dp. How about
dp_lf2_handle?

Best regards

Heinrich