From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ilias Apalodimas Date: Thu, 11 Mar 2021 14:30:21 +0200 Subject: [PATCH 4/6] efi_loader: Replace config option for initrd loading In-Reply-To: <0934e763-e275-3ee5-e481-370463dbd258@gmx.de> References: <20210305222303.2866284-1-ilias.apalodimas@linaro.org> <20210305222303.2866284-4-ilias.apalodimas@linaro.org> <0934e763-e275-3ee5-e481-370463dbd258@gmx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > --- > > 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= 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= 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 > > #include > > +#include > > #include > > +#include > > #include > > #include > > #include > > @@ -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; > > +} > > >