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

Heinrich Schuchardt xypron.glpk at gmx.de
Thu Mar 11 13:50:34 CET 2021


On 11.03.21 13:30, Ilias Apalodimas wrote:
> On Thu, Mar 11, 2021 at 01:23:05PM +0100, Heinrich Schuchardt wrote:
>> On 05.03.21 23:23, 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"
>>>
>>> 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     |   3 +
>>>  lib/efi_loader/efi_load_initrd.c | 209 +++++++++++++++++++++----------
>>>  5 files changed, 152 insertions(+), 79 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 eb11a8c7d4b1..0d4f5d9acc9f 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..d1baa8c71a4d 100644
>>> --- a/lib/efi_loader/efi_bootmgr.c
>>> +++ b/lib/efi_loader/efi_bootmgr.c
>>> @@ -118,6 +118,9 @@ 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);
>>
>> Why would you continue if BootCurrent is not set?
>
> Because the bitops below is just trying to simplify the code
> reading, since both must call efi_unload_image(). Calling
> efi_initrd_register() without a BootCurrent is guaranteed to fail as well.
>
>>
>>> +		/* try to register load file2 for initrd's */
>>> +		if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD))
>>> +			ret |= efi_initrd_register();
>>
>> ret is not a boolean. So |= does not make sense to me here.
>
> Uhm? It's an unsigned long though and you can hav bitops properly?

EFI_DEVICE_ERROR | EFI_INVALID_PARAMETER => EFI_ACCESS_DENIED

That does not make much sense.

Best regards

Heinrich

>
>>
>>>  		if (ret != EFI_SUCCESS) {
>>>  			if (EFI_CALL(efi_unload_image(*handle))
>>>  			    != EFI_SUCCESS)
>>> diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
>>> index b9ee8839054f..b11c5ee293fe 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,71 @@ 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
>>>   */
>>> -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;
>>> +	const efi_guid_t lf2_initrd_guid = EFI_INITRD_MEDIA_GUID;
>>> +	struct efi_device_path *cur = NULL;
>>> +	struct efi_device_path *dp = NULL;
>>> +	struct efi_device_path *tmp_dp;
>>> +	efi_uintn_t ret;
>>> +	efi_uintn_t size;
>>>
>>> -	ret = fs_set_blk_dev(dev, part, FS_TYPE_ANY);
>>> -	if (ret) {
>>> -		*status = EFI_NO_MEDIA;
>> gg/> +	/*
>>> +	 * if bootmgr is setup with and initrd, the device path will be
>>> +	 * in the FilePathList[] of our load options in Boot####.
>>> +	 * The first device path of the multi instance device path will
>>> +	 * start with a VenMedia and the initrds will follow.
>>> +	 *
>>> +	 * If the device path is not found return EFI_INVALID_PARAMETER.
>>> +	 * We can then use this specific return value and not install the
>>> +	 * protocol, while allowing the boot to continue
>>> +	 */
>>> +	dp = efi_get_dp_from_boot(lf2_initrd_guid);
>>> +	if (!dp) {
>>> +		ret = EFI_INVALID_PARAMETER;
>>>  		goto out;
>>>  	}
>>>
>>> -	ret = fs_size(file, &sz);
>>> -	if (ret) {
>>> -		sz = 0;
>>> -		*status = EFI_NOT_FOUND;
>>> +	tmp_dp = dp;
>>> +	cur = efi_dp_get_next_instance(&tmp_dp, &size);
>>> +	if (!cur) {
>>> +		ret = EFI_OUT_OF_RESOURCES;
>>>  		goto out;
>>>  	}
>>>
>>> +	/*
>>> +	 * We don't care if the file path is eventually NULL here. The
>>> +	 * callers will try to load a file from it and eventually fail
>>> +	 * but let's be explicit with our return values
>>> +	 */
>>> +	if (!tmp_dp) {
>>> +		*initrd_fp = NULL;
>>> +		ret = EFI_NOT_FOUND;
>>> +	} else {
>>> +		*initrd_fp = efi_dp_dup(tmp_dp);
>>> +		if (*initrd_fp)
>>> +			ret = EFI_SUCCESS;
>>> +		else
>>> +			ret = EFI_OUT_OF_RESOURCES;
>>> +	}
>>> +
>>>  out:
>>> -	return sz;
>>> +	efi_free_pool(cur);
>>> +	efi_free_pool(dp);
>>> +	return ret;
>>>  }
>>>
>>>  /**
>>> - * 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.
>>> @@ -93,98 +125,131 @@ 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 efi_device_path *initrd_fp = NULL;
>>> +	struct efi_file_info *info = NULL;
>>> +	efi_status_t ret = EFI_NOT_FOUND;
>>> +	struct efi_file_handle *f;
>>> +	efi_uintn_t bs;
>>>
>>>  	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;
>>> +		ret = EFI_INVALID_PARAMETER;
>>>  		goto out;
>>>  	}
>>>
>>>  	if (file_path->type != dp.end.type ||
>>>  	    file_path->sub_type != dp.end.sub_type) {
>>> -		status = EFI_INVALID_PARAMETER;
>>> +		ret = EFI_INVALID_PARAMETER;
>>>  		goto out;
>>>  	}
>>>
>>>  	if (boot_policy) {
>>> -		status = EFI_UNSUPPORTED;
>>> +		ret = EFI_UNSUPPORTED;
>>>  		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)
>>> +	ret = get_initrd_fp(&initrd_fp);
>>> +	if (ret != EFI_SUCCESS)
>>>  		goto out;
>>> -	part = strsep(&pos, " ");
>>> -	if (!part)
>>> +
>>> +	/* Open file */
>>> +	f = efi_file_from_path(initrd_fp);
>>> +	if (!f) {
>>> +		ret = EFI_NOT_FOUND;
>>>  		goto out;
>>> -	file = strsep(&pos, " ");
>>> -	if (!file)
>>> +	}
>>> +
>>> +	/* Get file size */
>>> +	bs = 0;
>>> +	EFI_CALL(ret = f->getinfo(f, (efi_guid_t *)&efi_file_info_guid,
>>> +				  &bs, info));
>>
>> Please, use
>>
>> 	ret = EFI_CALL(f-> ...);
>>
>> througout the code.
>>
>> In efi_load_image_from_file() and efi_capsule_read_file() we also
>> retrieve the file size. We should factor out a function efi_get_file_size().
>>
>>> +	if (ret != EFI_BUFFER_TOO_SMALL) {
>>> +		ret = EFI_DEVICE_ERROR;
>>>  		goto out;
>>> +	}
>>>
>>> -	file_sz = get_file_size(dev, part, file, &status);
>>> -	if (!file_sz)
>>> +	info = malloc(bs);
>>> +	EFI_CALL(ret = f->getinfo(f, (efi_guid_t *)&efi_file_info_guid, &bs,
>>> +				  info));
>>> +	if (ret != EFI_SUCCESS)
>>>  		goto out;
>>>
>>> -	if (!buffer || *buffer_size < file_sz) {
>>> -		status = EFI_BUFFER_TOO_SMALL;
>>> -		*buffer_size = file_sz;
>>> +	bs = info->file_size;
>>> +	if (!buffer || *buffer_size < bs) {
>>> +		ret = EFI_BUFFER_TOO_SMALL;
>>> +		*buffer_size = bs;
>>>  	} else {
>>> -		ret = fs_set_blk_dev(dev, 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)
>>> -			goto out;
>>> -		*buffer_size = read_sz;
>>> -
>>> -		status = EFI_SUCCESS;
>>> +		EFI_CALL(ret = f->read(f, &bs, (void *)(uintptr_t)buffer));
>>> +		*buffer_size = bs;
>>>  	}
>>>
>>>  out:
>>> -	free(filespec);
>>> -	return EFI_EXIT(status);
>>> +	free(info);
>>> +	efi_free_pool(initrd_fp);
>>> +	EFI_CALL(f->close(f));
>>> +	return EFI_EXIT(ret);
>>> +}
>>> +
>>> +/**
>>> + * check_initrd() - Determine if the file defined as an initrd in Boot####
>>> + *		    load_options device path is present
>>> + *
>>> + * Return:	status code
>>> + */
>>> +static efi_status_t check_initrd(void)
>>> +{
>>> +	struct efi_device_path *initrd_fp = NULL;
>>> +	struct efi_file_handle *f;
>>> +	efi_status_t ret;
>>> +
>>> +	ret = get_initrd_fp(&initrd_fp);
>>> +	if (ret != EFI_SUCCESS)
>>> +		goto out;
>>> +
>>> +	/*
>>> +	 * If the file is not found, but the file path is set, return an error
>>> +	 * and trigger the bootmgr fallback
>>> +	 */
>>> +	f = efi_file_from_path(initrd_fp);
>>> +	if (!f) {
>>
>> This deserves a log_warning().
>>
>>> +		ret = EFI_NOT_FOUND;
>>> +		goto out;
>>> +	}
>>> +
>>> +	EFI_CALL(f->close(f));
>>> +
>>> +out:
>>> +	efi_free_pool(initrd_fp);
>>> +	return ret;
>>>  }
>>>
>>>  /**
>>>   * efi_initrd_register() - create handle for loading initial RAM disk
>>>   *
>>>   * This function creates a new handle and installs a Linux specific vendor
>>> - * device path and an EFI_LOAD_FILE_2_PROTOCOL. Linux uses the device path
>>> + * device path and an EFI_LOAD_FILE2_PROTOCOL. Linux uses the device path
>>>   * to identify the handle and then calls the LoadFile service of the
>>> - * EFI_LOAD_FILE_2_PROTOCOL to read the initial RAM disk.
>>> + * EFI_LOAD_FILE2_PROTOCOL to read the initial RAM disk.
>>>   *
>>>   * Return:	status code
>>>   */
>>>  efi_status_t efi_initrd_register(void)
>>>  {
>>> -	efi_handle_t efi_initrd_handle = NULL;
>>>  	efi_status_t ret;
>>>
>>> +	/*
>>> +	 * Allow the user to continue if Boot#### file path is not set for
>>> +	 * an initrd
>>> +	 */
>>> +	ret = check_initrd();
>>> +	if (ret == EFI_INVALID_PARAMETER)
>>> +		return EFI_SUCCESS;
>>> +	if (ret != EFI_SUCCESS)
>>> +		return ret;
>>> +
>>>  	ret = EFI_CALL(efi_install_multiple_protocol_interfaces
>>>  		       (&efi_initrd_handle,
>>>  			/* initramfs */
>>> @@ -196,3 +261,9 @@ efi_status_t efi_initrd_register(void)
>>>
>>>  	return ret;
>>>  }
>>> +
>>> +void efi_initrd_deregister(void)
>>
>> Missing description of an exported function.
>>
>> Best regards
>>
>> Heinrich
>>
>>> +{
>>> +	efi_delete_handle(efi_initrd_handle);
>>> +	efi_initrd_handle = NULL;
>>> +}
>>>
>>



More information about the U-Boot mailing list