[PATCH 4/6] efi_loader: Replace config option for initrd loading
Ilias Apalodimas
ilias.apalodimas at linaro.org
Thu Mar 11 13:30:21 CET 2021
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?
>
> > 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