[PATCH 0/6] efi: Start to chip away at the EFI workaround
At present there is a function, efi_set_bootdev(), which is used in various places to tell the EFI loader which device a file came from.
With bootstd, this information is available in the bootflow.
This little series provides a way for bootstd to provide the bootflow to the EFI loader, so that it is able to select the correct paths.
For now only the EFI bootmeth is updated.
Simon Glass (6): efi: Refactor device and image paths into a function efi: Make efi_run_image() static efi: Update efi_run_image() to accept image and device path efi: Add a version of efi_binary_run() with more parameters efi: Move the fallback code from efi_run_image() efi: Pass in the require parameters from EFI bootmeth
boot/bootmeth_efi.c | 49 +------- include/efi_loader.h | 10 ++ lib/efi_loader/efi_bootbin.c | 210 ++++++++++++++++++++++++----------- 3 files changed, 159 insertions(+), 110 deletions(-)
Move this code into a function so it can be called from elsewhere.
Signed-off-by: Simon Glass sjg@chromium.org ---
lib/efi_loader/efi_bootbin.c | 53 +++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 16 deletions(-)
diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index a87006b3c0e..a0ab677ee1a 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -44,12 +44,46 @@ void efi_clear_bootdev(void) image_size = 0; }
+efi_status_t calculate_paths(const char *dev, const char *devnr, const char *path, + struct efi_device_path **device_pathp, + struct efi_device_path **image_pathp) + +{ + struct efi_device_path *image, *device; + efi_status_t ret; + + ret = efi_dp_from_name(dev, devnr, path, &device, &image); + if (ret != EFI_SUCCESS) + return ret; + + *device_pathp = device; + if (image) { + /* FIXME: image should not contain device */ + struct efi_device_path *image_tmp = image; + + efi_dp_split_file_path(image, &device, &image); + efi_free_pool(image_tmp); + } + *image_pathp = image; + log_debug("- boot device %pD\n", device); + if (image) + log_debug("- image %pD\n", image); + + return EFI_SUCCESS; +} + /** * efi_set_bootdev() - set boot device * * This function is called when a file is loaded, e.g. via the 'load' command. * We use the path to this file to inform the UEFI binary about the boot device. * + * For a valid image, it sets: + * - image_addr to the provided buffer + * - image_size to the provided buffer_size + * - bootefi_device_path to the EFI device-path + * - bootefi_image_path to the EFI image-path + * * @dev: device, e.g. "MMC" * @devnr: number of the device, e.g. "1:2" * @path: path to file loaded @@ -59,7 +93,6 @@ void efi_clear_bootdev(void) void efi_set_bootdev(const char *dev, const char *devnr, const char *path, void *buffer, size_t buffer_size) { - struct efi_device_path *device, *image; efi_status_t ret;
log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev, @@ -93,21 +126,9 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path, image_addr = buffer; image_size = buffer_size;
- ret = efi_dp_from_name(dev, devnr, path, &device, &image); - if (ret == EFI_SUCCESS) { - bootefi_device_path = device; - if (image) { - /* FIXME: image should not contain device */ - struct efi_device_path *image_tmp = image; - - efi_dp_split_file_path(image, &device, &image); - efi_free_pool(image_tmp); - } - bootefi_image_path = image; - log_debug("- boot device %pD\n", device); - if (image) - log_debug("- image %pD\n", image); - } else { + ret = calculate_paths(dev, devnr, path, &bootefi_device_path, + &bootefi_image_path); + if (ret) { log_debug("- efi_dp_from_name() failed, err=%lx\n", ret); efi_clear_bootdev(); }
This function is not called from outside this file and has no entry in the header file, so mark it static.
Signed-off-by: Simon Glass sjg@chromium.org ---
lib/efi_loader/efi_bootbin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index a0ab677ee1a..eb9b9c4afe0 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -141,7 +141,7 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path, * @source_size: size of the UEFI image * Return: status code */ -efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) +static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) { efi_handle_t mem_handle = NULL, handle; struct efi_device_path *file_path = NULL;
Hi Simon,
Please mark the patches as efi_loader. I didn't notice this one up to now
On Fri, 18 Oct 2024 at 17:52, Simon Glass sjg@chromium.org wrote:
This function is not called from outside this file and has no entry in the header file, so mark it static.
Signed-off-by: Simon Glass sjg@chromium.org
lib/efi_loader/efi_bootbin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index a0ab677ee1a..eb9b9c4afe0 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -141,7 +141,7 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
- @source_size: size of the UEFI image
- Return: status code
*/ -efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) +static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) { efi_handle_t mem_handle = NULL, handle; struct efi_device_path *file_path = NULL; -- 2.34.1
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Provide these globals as parameters to this function, on the way to making it possible to start an image without relying on the globals.
Signed-off-by: Simon Glass sjg@chromium.org ---
lib/efi_loader/efi_bootbin.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index eb9b9c4afe0..8881d648ed6 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -139,9 +139,13 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path, * * @source_buffer: memory address of the UEFI image * @source_size: size of the UEFI image + * @device: EFI device-path + * @image: EFI image-path * Return: status code */ -static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) +static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size, + struct efi_device_path *device, + struct efi_device_path *image) { efi_handle_t mem_handle = NULL, handle; struct efi_device_path *file_path = NULL; @@ -149,7 +153,7 @@ static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) efi_status_t ret; u16 *load_options;
- if (!bootefi_device_path || !bootefi_image_path) { + if (!device || !image) { log_debug("Not loaded from disk\n"); /* * Special case for efi payload not loaded from disk, @@ -170,9 +174,8 @@ static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) goto out; msg_path = file_path; } else { - file_path = efi_dp_concat(bootefi_device_path, - bootefi_image_path, 0); - msg_path = bootefi_image_path; + file_path = efi_dp_concat(device, image, 0); + msg_path = image; log_debug("Loaded from disk\n"); }
@@ -209,7 +212,7 @@ out: /** * efi_binary_run() - run loaded UEFI image * - * @image: memory address of the UEFI image + * @image_ptr: memory address of the UEFI image * @size: size of the UEFI image * @fdt: device-tree * @@ -218,7 +221,7 @@ out: * * Return: status code */ -efi_status_t efi_binary_run(void *image, size_t size, void *fdt) +efi_status_t efi_binary_run(void *image_ptr, size_t size, void *fdt) { efi_status_t ret;
@@ -234,5 +237,6 @@ efi_status_t efi_binary_run(void *image, size_t size, void *fdt) if (ret != EFI_SUCCESS) return ret;
- return efi_run_image(image, size); + return efi_run_image(image_ptr, size, bootefi_image_path, + bootefi_device_path); }
This uses a few global variables at present. With the bootflow we have the required parameters, so add a function which accepts these. Update the existing function to call the new one with the globals.
Signed-off-by: Simon Glass sjg@chromium.org ---
lib/efi_loader/efi_bootbin.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index 8881d648ed6..920a4dfc74a 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -210,18 +210,22 @@ out: }
/** - * efi_binary_run() - run loaded UEFI image + * efi_binary_run_() - run loaded UEFI image * * @image_ptr: memory address of the UEFI image * @size: size of the UEFI image * @fdt: device-tree + * @device: EFI device-path + * @image: EFI image-path * * Execute an EFI binary image loaded at @image. * @size may be zero if the binary is loaded with U-Boot load command. * * Return: status code */ -efi_status_t efi_binary_run(void *image_ptr, size_t size, void *fdt) +efi_status_t efi_binary_run_(void *image_ptr, size_t size, void *fdt, + struct efi_device_path *device, + struct efi_device_path *image) { efi_status_t ret;
@@ -237,6 +241,23 @@ efi_status_t efi_binary_run(void *image_ptr, size_t size, void *fdt) if (ret != EFI_SUCCESS) return ret;
- return efi_run_image(image_ptr, size, bootefi_image_path, - bootefi_device_path); + return efi_run_image(image_ptr, size, device, image); +} + +/** + * efi_binary_run() - run loaded UEFI image + * + * @image: memory address of the UEFI image + * @size: size of the UEFI image + * @fdt: device-tree + * + * Execute an EFI binary image loaded at @image. + * @size may be zero if the binary is loaded with U-Boot load command. + * + * Return: status code + */ +efi_status_t efi_binary_run(void *image, size_t size, void *fdt) +{ + return efi_binary_run_(image, size, fdt, bootefi_device_path, + bootefi_image_path); }
This code is only needed if an invalid image/device path is passed in. Move the code out to a caller where this can be dealt with. The normal flow will provide these parameters.
Signed-off-by: Simon Glass sjg@chromium.org ---
lib/efi_loader/efi_bootbin.c | 104 ++++++++++++++++------------------- 1 file changed, 48 insertions(+), 56 deletions(-)
diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index 920a4dfc74a..3646c706dd3 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -147,37 +147,13 @@ static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size, struct efi_device_path *device, struct efi_device_path *image) { - efi_handle_t mem_handle = NULL, handle; - struct efi_device_path *file_path = NULL; - struct efi_device_path *msg_path; + efi_handle_t handle; + struct efi_device_path *msg_path, *file_path; efi_status_t ret; u16 *load_options;
- if (!device || !image) { - log_debug("Not loaded from disk\n"); - /* - * Special case for efi payload not loaded from disk, - * such as 'bootefi hello' or for example payload - * loaded directly into memory via JTAG, etc: - */ - file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, - (uintptr_t)source_buffer, - source_size); - /* - * Make sure that device for device_path exist - * in load_image(). Otherwise, shell and grub will fail. - */ - ret = efi_install_multiple_protocol_interfaces(&mem_handle, - &efi_guid_device_path, - file_path, NULL); - if (ret != EFI_SUCCESS) - goto out; - msg_path = file_path; - } else { - file_path = efi_dp_concat(device, image, 0); - msg_path = image; - log_debug("Loaded from disk\n"); - } + file_path = efi_dp_concat(device, image, 0); + msg_path = image;
log_info("Booting %pD\n", msg_path);
@@ -196,36 +172,13 @@ static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size, ret = do_bootefi_exec(handle, load_options);
out: - if (mem_handle) { - efi_status_t r; - - r = efi_uninstall_multiple_protocol_interfaces( - mem_handle, &efi_guid_device_path, file_path, NULL); - if (r != EFI_SUCCESS) - log_err("Uninstalling protocol interfaces failed\n"); - } - efi_free_pool(file_path);
return ret; }
-/** - * efi_binary_run_() - run loaded UEFI image - * - * @image_ptr: memory address of the UEFI image - * @size: size of the UEFI image - * @fdt: device-tree - * @device: EFI device-path - * @image: EFI image-path - * - * Execute an EFI binary image loaded at @image. - * @size may be zero if the binary is loaded with U-Boot load command. - * - * Return: status code - */ -efi_status_t efi_binary_run_(void *image_ptr, size_t size, void *fdt, - struct efi_device_path *device, - struct efi_device_path *image) +static efi_status_t efi_binary_run_(void *image_ptr, size_t size, void *fdt, + struct efi_device_path *device, + struct efi_device_path *image) { efi_status_t ret;
@@ -258,6 +211,45 @@ efi_status_t efi_binary_run_(void *image_ptr, size_t size, void *fdt, */ efi_status_t efi_binary_run(void *image, size_t size, void *fdt) { - return efi_binary_run_(image, size, fdt, bootefi_device_path, - bootefi_image_path); + efi_handle_t mem_handle = NULL; + struct efi_device_path *file_path = NULL; + efi_status_t ret; + + if (!bootefi_device_path || !bootefi_image_path) { + log_debug("Not loaded from disk\n"); + /* + * Special case for efi payload not loaded from disk, + * such as 'bootefi hello' or for example payload + * loaded directly into memory via JTAG, etc: + */ + file_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, + (uintptr_t)image, size); + /* + * Make sure that device for device_path exist + * in load_image(). Otherwise, shell and grub will fail. + */ + ret = efi_install_multiple_protocol_interfaces(&mem_handle, + &efi_guid_device_path, + file_path, NULL); + if (ret != EFI_SUCCESS) + goto out; + } else { + log_debug("Loaded from disk\n"); + } + + ret = efi_binary_run_(image, size, fdt, bootefi_device_path, + bootefi_image_path); +out: + if (mem_handle) { + efi_status_t r; + + r = efi_uninstall_multiple_protocol_interfaces(mem_handle, + &efi_guid_device_path, file_path, NULL); + if (r != EFI_SUCCESS) + log_err("Uninstalling protocol interfaces failed\n"); + } + efi_free_pool(file_path); + + return ret; } +
Rather than setting up the global variables and then making the call, pass them into function directly. This cleans up the code and makes it all a bit easier to understand.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootmeth_efi.c | 49 ++---------------------------------- include/efi_loader.h | 10 ++++++++ lib/efi_loader/efi_bootbin.c | 46 +++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 47 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 2ad6d3b4ace..f5b73c09441 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -73,40 +73,6 @@ static bool bootmeth_uses_network(struct bootflow *bflow) device_get_uclass_id(media) == UCLASS_ETH; }
-static void set_efi_bootdev(struct blk_desc *desc, struct bootflow *bflow) -{ - const struct udevice *media_dev; - int size = bflow->size; - const char *dev_name; - char devnum_str[9]; - char dirname[200]; - char *last_slash; - - /* - * This is a horrible hack to tell EFI about this boot device. Once we - * unify EFI with the rest of U-Boot we can clean this up. The same hack - * exists in multiple places, e.g. in the fs, tftp and load commands. - * - * Once we can clean up the EFI code to make proper use of driver model, - * this can go away. - */ - media_dev = dev_get_parent(bflow->dev); - snprintf(devnum_str, sizeof(devnum_str), "%x:%x", - desc ? desc->devnum : dev_seq(media_dev), - bflow->part); - - strlcpy(dirname, bflow->fname, sizeof(dirname)); - last_slash = strrchr(dirname, '/'); - if (last_slash) - *last_slash = '\0'; - - dev_name = device_get_uclass_id(media_dev) == UCLASS_MASS_STORAGE ? - "usb" : blk_get_uclass_name(device_get_uclass_id(media_dev)); - log_debug("setting bootdev %s, %s, %s, %p, %x\n", - dev_name, devnum_str, bflow->fname, bflow->buf, size); - efi_set_bootdev(dev_name, devnum_str, bflow->fname, bflow->buf, size); -} - static int efiload_read_file(struct bootflow *bflow, ulong addr) { struct blk_desc *desc = NULL; @@ -124,8 +90,6 @@ static int efiload_read_file(struct bootflow *bflow, ulong addr) return log_msg_ret("read", ret); bflow->buf = map_sysmem(addr, bflow->size);
- set_efi_bootdev(desc, bflow); - return 0; }
@@ -363,17 +327,8 @@ static int distro_efi_boot(struct udevice *dev, struct bootflow *bflow) fdt = env_get_hex("fdt_addr_r", 0); }
- if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) { - log_debug("Booting with built-in fdt\n"); - if (efi_binary_run(map_sysmem(kernel, 0), bflow->size, - EFI_FDT_USE_INTERNAL)) - return log_msg_ret("run", -EINVAL); - } else { - log_debug("Booting with external fdt\n"); - if (efi_binary_run(map_sysmem(kernel, 0), bflow->size, - map_sysmem(fdt, 0))) - return log_msg_ret("run", -EINVAL); - } + if (efi_bootflow_run(bflow)) + return log_msg_ret("run", -EINVAL);
return 0; } diff --git a/include/efi_loader.h b/include/efi_loader.h index 291eca5c077..3a4b948f60f 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -19,6 +19,7 @@ #include <linux/oid_registry.h>
struct blk_desc; +struct bootflow; struct jmp_buf_data;
#if CONFIG_IS_ENABLED(EFI_LOADER) @@ -544,6 +545,15 @@ efi_status_t efi_install_fdt(void *fdt); efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options); /* Run loaded UEFI image with given fdt */ efi_status_t efi_binary_run(void *image, size_t size, void *fdt); + +/** + * efi_bootflow_run() - Run a bootflow containing an EFI application + * + * @bootflow: Bootflow to run + * Return: Status code, something went wrong + */ +efi_status_t efi_bootflow_run(struct bootflow *bootflow); + /* Initialize variable services */ efi_status_t efi_init_variables(void); /* Notify ExitBootServices() is called */ diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index 3646c706dd3..e4a130da2a4 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -6,13 +6,16 @@
#define LOG_CATEGORY LOGC_EFI
+#include <bootflow.h> #include <charset.h> +#include <dm.h> #include <efi.h> #include <efi_loader.h> #include <env.h> #include <image.h> #include <log.h> #include <malloc.h> +#include <mapmem.h>
static struct efi_device_path *bootefi_image_path; static struct efi_device_path *bootefi_device_path; @@ -253,3 +256,46 @@ out: return ret; }
+efi_status_t efi_bootflow_run(struct bootflow *bflow) +{ + struct efi_device_path *device, *image; + const struct udevice *media_dev; + struct blk_desc *desc = NULL; + const char *dev_name; + char devnum_str[9]; + char dirname[200]; + char *last_slash; + efi_status_t ret; + void *fdt; + + if (bflow->blk) + desc = dev_get_uclass_plat(bflow->blk); + + media_dev = dev_get_parent(bflow->dev); + snprintf(devnum_str, sizeof(devnum_str), "%x:%x", + desc ? desc->devnum : dev_seq(media_dev), + bflow->part); + + strlcpy(dirname, bflow->fname, sizeof(dirname)); + last_slash = strrchr(dirname, '/'); + if (last_slash) + *last_slash = '\0'; + dev_name = device_get_uclass_id(media_dev) == UCLASS_MASS_STORAGE ? + "usb" : blk_get_uclass_name(device_get_uclass_id(media_dev)); + + ret = calculate_paths(dev_name, devnum_str, dirname, &device, &image); + if (ret) + return ret; + + if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) { + log_debug("Booting with built-in fdt\n"); + fdt = EFI_FDT_USE_INTERNAL; + } else { + log_debug("Booting with external fdt\n"); + fdt = map_sysmem(bflow->fdt_addr, 0); + } + ret = efi_binary_run_(bflow->buf, bflow->size, fdt, device, image); + + return ret; +} +
On Fri, Oct 18, 2024 at 08:51:51AM -0600, Simon Glass wrote:
At present there is a function, efi_set_bootdev(), which is used in various places to tell the EFI loader which device a file came from.
With bootstd, this information is available in the bootflow.
This little series provides a way for bootstd to provide the bootflow to the EFI loader, so that it is able to select the correct paths.
For now only the EFI bootmeth is updated.
I think this takes things in the wrong direction. I'm going to link to my post in another series now: https://lore.kernel.org/u-boot/20241017232413.724808-1-sjg@chromium.org/T/#m... as I think we need to deal with this problem at a more common point which will allow us to handle other enhancements as well, without requiring changes in every possible path we could load something in, to capture this information.
participants (3)
-
Ilias Apalodimas -
Simon Glass -
Tom Rini