* [RFC 00/13] cmd: bootefi: refactor the code for bootmgr
@ 2023-10-26 5:30 AKASHI Takahiro
2023-10-26 5:30 ` [RFC 01/13] cmd: bootefi: unfold do_bootefi_image() AKASHI Takahiro
` (13 more replies)
0 siblings, 14 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
This patch set is motivated by the discussion[1] regarding
CONFIG_BOOTEFI_BOOTMGR option.
At the end, bootefi.c will be decomposed into two parts, one for
providing the command itself and one for implementing helper functions.
EFI_LOADER will now be available without CONFIG_CMDLINE or specifically
CONFIG_CMD_BOOTEFI if invoked via bootmeth/bootstd.
Then, EFI_LOADER library side will be further split into two options
for fine-grain control:
CONFIG_EFI_BINARY_EXEC: execute UEFI binaries which are to be explicitly
loaded by U-Boot's load commands/functions or other methods
(like a jtag debugger?)
It supports bootmeth_efi as well as "bootefi <addr>|hello"(/"bootm"?).
CONFIG_EFI_BOOTMGR: provide EFI boot manger functionality
It supports bootmeth_efi_mgr as well as "bootefi bootmgr".
As such, We will no longer need CONFIG_BINARY_EXEC if we want to only
make use of the boot manger for booting a next stage OS.
Prerequisite
============
This patch set is based on top of Simon/Tom's [2].
Patches
=======
Patch#1-#11: I hope that those commits show step-by-step refactoring
without introducing degradation.
Patch#12-#13: Those are not directly related to the patch's aim, but
they are necessary to compile U-Boot on sandbox (sandbox_defconfig)
without CONFIG_CMDLINE.
[1] https://lists.denx.de/pipermail/u-boot/2023-October/534598.html
[2] origin/TEST/v4.1-tidy-up-use-of-CONFIG_CMDLINE
AKASHI Takahiro (13):
cmd: bootefi: unfold do_bootefi_image()
cmd: bootefi: re-organize do_bootefi_image()
cmd: bootefi: carve out EFI boot manager interface
cmd: bootefi: carve out binary execution interface
cmd: bootefi: move library interfaces under lib/efi_loader
cmd: efidebug: ease efi configuration dependency
bootmeth: use efi_loader interfaces instead of bootefi command
efi_loader: split unrelated code from efi_bootmgr.c
efi_loader: rename BOOTEFI_BOOTMGR to EFI_BOOTMGR
net: tftp: remove explicit efi configuration dependency
fs: remove explicit efi configuration dependency
lib: uuid: move CONFIG_RANDOM_UUID
block: rkmtd: select CONFIG_RANDOM_UUID explicitly
boot/Kconfig | 4 +-
boot/Makefile | 2 +-
boot/bootm_os.c | 31 +-
boot/bootmeth_efi.c | 8 +-
boot/bootmeth_efi_mgr.c | 3 +-
cmd/Kconfig | 28 +-
cmd/bootefi.c | 658 +++++--------------------------
cmd/efidebug.c | 4 +-
drivers/block/Kconfig | 1 +
fs/fs.c | 7 +-
include/efi_loader.h | 34 +-
lib/Kconfig | 7 +
lib/efi_loader/Kconfig | 11 +-
lib/efi_loader/Makefile | 2 +-
lib/efi_loader/efi_bootmgr.c | 37 ++
lib/efi_loader/efi_device_path.c | 3 +-
lib/efi_loader/efi_helper.c | 486 ++++++++++++++++++++++-
net/tftp.c | 10 +-
test/boot/bootflow.c | 2 +-
19 files changed, 697 insertions(+), 641 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [RFC 01/13] cmd: bootefi: unfold do_bootefi_image()
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
@ 2023-10-26 5:30 ` AKASHI Takahiro
2023-10-26 11:01 ` Heinrich Schuchardt
2023-10-26 5:30 ` [RFC 02/13] cmd: bootefi: re-organize do_bootefi_image() AKASHI Takahiro
` (12 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
Unfold do_bootefi_image() into do_bootefi() for the sake of the succeeding
refactor work.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/bootefi.c | 101 ++++++++++++++++++--------------------------------
1 file changed, 37 insertions(+), 64 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 20e5c94a33a4..1b28bf5a318d 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -425,58 +425,6 @@ static int do_efibootmgr(void)
return CMD_RET_SUCCESS;
}
-/**
- * do_bootefi_image() - execute EFI binary
- *
- * Set up memory image for the binary to be loaded, prepare device path, and
- * then call do_bootefi_exec() to execute it.
- *
- * @image_opt: string with image start address
- * @size_opt: string with image size or NULL
- * Return: status code
- */
-static int do_bootefi_image(const char *image_opt, const char *size_opt)
-{
- void *image_buf;
- unsigned long addr, size;
- efi_status_t ret;
-
-#ifdef CONFIG_CMD_BOOTEFI_HELLO
- if (!strcmp(image_opt, "hello")) {
- image_buf = __efi_helloworld_begin;
- size = __efi_helloworld_end - __efi_helloworld_begin;
- efi_clear_bootdev();
- } else
-#endif
- {
- addr = strtoul(image_opt, NULL, 16);
- /* Check that a numeric value was passed */
- if (!addr)
- return CMD_RET_USAGE;
- image_buf = map_sysmem(addr, 0);
-
- if (size_opt) {
- size = strtoul(size_opt, NULL, 16);
- if (!size)
- return CMD_RET_USAGE;
- efi_clear_bootdev();
- } else {
- if (image_buf != image_addr) {
- log_err("No UEFI binary known at %s\n",
- image_opt);
- return CMD_RET_FAILURE;
- }
- size = image_size;
- }
- }
- ret = efi_run_image(image_buf, size);
-
- if (ret != EFI_SUCCESS)
- return CMD_RET_FAILURE;
-
- return CMD_RET_SUCCESS;
-}
-
/**
* efi_run_image() - run loaded UEFI image
*
@@ -648,8 +596,9 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
{
efi_status_t ret;
- char *img_addr, *img_size, *str_copy, *pos;
- void *fdt;
+ char *p;
+ void *fdt, *image_buf;
+ unsigned long addr, size;
if (argc < 2)
return CMD_RET_USAGE;
@@ -684,18 +633,42 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
if (!strcmp(argv[1], "selftest"))
return do_efi_selftest();
#endif
- str_copy = strdup(argv[1]);
- if (!str_copy) {
- log_err("Out of memory\n");
- return CMD_RET_FAILURE;
+
+#ifdef CONFIG_CMD_BOOTEFI_HELLO
+ if (!strcmp(argv[1], "hello")) {
+ image_buf = __efi_helloworld_begin;
+ size = __efi_helloworld_end - __efi_helloworld_begin;
+ efi_clear_bootdev();
+ } else
+#endif
+ {
+ addr = strtoul(argv[1], NULL, 16);
+ /* Check that a numeric value was passed */
+ if (!addr)
+ return CMD_RET_USAGE;
+ image_buf = map_sysmem(addr, 0);
+
+ p = strchr(argv[1], ':');
+ if (p) {
+ size = strtoul(++p, NULL, 16);
+ if (!size)
+ return CMD_RET_USAGE;
+ efi_clear_bootdev();
+ } else {
+ if (image_buf != image_addr) {
+ log_err("No UEFI binary known at %s\n",
+ argv[1]);
+ return CMD_RET_FAILURE;
+ }
+ size = image_size;
+ }
}
- pos = str_copy;
- img_addr = strsep(&pos, ":");
- img_size = strsep(&pos, ":");
- ret = do_bootefi_image(img_addr, img_size);
- free(str_copy);
+ ret = efi_run_image(image_buf, size);
- return ret;
+ if (ret != EFI_SUCCESS)
+ return CMD_RET_FAILURE;
+
+ return CMD_RET_SUCCESS;
}
U_BOOT_LONGHELP(bootefi,
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [RFC 02/13] cmd: bootefi: re-organize do_bootefi_image()
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
2023-10-26 5:30 ` [RFC 01/13] cmd: bootefi: unfold do_bootefi_image() AKASHI Takahiro
@ 2023-10-26 5:30 ` AKASHI Takahiro
2023-10-26 10:44 ` Heinrich Schuchardt
2023-10-26 5:30 ` [RFC 03/13] cmd: bootefi: carve out EFI boot manager interface AKASHI Takahiro
` (11 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
Decompose and re-organize do_bootefi_image() into three parts for
the succeeding refactor work.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/Kconfig | 15 ++++++--
cmd/bootefi.c | 82 ++++++++++++++++++++++++++++++--------------
include/efi_loader.h | 2 --
3 files changed, 69 insertions(+), 30 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 0eb739203ade..825a41d68aad 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -363,9 +363,19 @@ config CMD_BOOTEFI
help
Boot an EFI image from memory.
+if CMD_BOOTEFI
+config CMD_BOOTEFI_BINARY
+ bool "Allow booting an EFI binary directly"
+ depends on BOOTEFI_BOOTMGR
+ default y
+ help
+ Select this option to enable direct execution of binary at 'bootefi'.
+ This subcommand will allow you to load the UEFI binary using
+ other U-Boot commands or external methods and then run it.
+
config CMD_BOOTEFI_BOOTMGR
bool "UEFI Boot Manager command"
- depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
+ depends on BOOTEFI_BOOTMGR
default y
help
Select this option to enable the 'bootmgr' subcommand of 'bootefi'.
@@ -374,7 +384,7 @@ config CMD_BOOTEFI_BOOTMGR
config CMD_BOOTEFI_HELLO_COMPILE
bool "Compile a standard EFI hello world binary for testing"
- depends on CMD_BOOTEFI && !CPU_V7M
+ depends on !CPU_V7M
default y
help
This compiles a standard EFI hello world application with U-Boot so
@@ -396,6 +406,7 @@ config CMD_BOOTEFI_HELLO
up EFI support on a new architecture.
source lib/efi_selftest/Kconfig
+endif
config CMD_BOOTMENU
bool "bootmenu"
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 1b28bf5a318d..ae00bba3b4f0 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -491,7 +491,6 @@ out:
return (ret != EFI_SUCCESS) ? ret : ret2;
}
-#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
static efi_status_t bootefi_run_prepare(const char *load_options_path,
struct efi_device_path *device_path,
struct efi_device_path *image_path,
@@ -581,7 +580,6 @@ static int do_efi_selftest(void)
return ret != EFI_SUCCESS;
}
-#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
/**
* do_bootefi() - execute `bootefi` command
@@ -603,14 +601,6 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
if (argc < 2)
return CMD_RET_USAGE;
- /* Initialize EFI drivers */
- ret = efi_init_obj_list();
- if (ret != EFI_SUCCESS) {
- log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
- ret & ~EFI_ERROR_MASK);
- return CMD_RET_FAILURE;
- }
-
if (argc > 2) {
uintptr_t fdt_addr;
@@ -619,29 +609,54 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
} else {
fdt = EFI_FDT_USE_INTERNAL;
}
- ret = efi_install_fdt(fdt);
- if (ret == EFI_INVALID_PARAMETER)
- return CMD_RET_USAGE;
- else if (ret != EFI_SUCCESS)
- return CMD_RET_FAILURE;
- if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
- if (!strcmp(argv[1], "bootmgr"))
- return do_efibootmgr();
+ if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
+ !strcmp(argv[1], "bootmgr")) {
+ /* Initialize EFI drivers */
+ ret = efi_init_obj_list();
+ if (ret != EFI_SUCCESS) {
+ log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+ ret & ~EFI_ERROR_MASK);
+ return CMD_RET_FAILURE;
+ }
+
+ ret = efi_install_fdt(fdt);
+ if (ret == EFI_INVALID_PARAMETER)
+ return CMD_RET_USAGE;
+ else if (ret != EFI_SUCCESS)
+ return CMD_RET_FAILURE;
+
+ return do_efibootmgr();
}
-#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
- if (!strcmp(argv[1], "selftest"))
+
+ if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
+ !strcmp(argv[1], "selftest")) {
+ /* Initialize EFI drivers */
+ ret = efi_init_obj_list();
+ if (ret != EFI_SUCCESS) {
+ log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+ ret & ~EFI_ERROR_MASK);
+ return CMD_RET_FAILURE;
+ }
+
+ ret = efi_install_fdt(fdt);
+ if (ret == EFI_INVALID_PARAMETER)
+ return CMD_RET_USAGE;
+ else if (ret != EFI_SUCCESS)
+ return CMD_RET_FAILURE;
+
return do_efi_selftest();
-#endif
+ }
-#ifdef CONFIG_CMD_BOOTEFI_HELLO
- if (!strcmp(argv[1], "hello")) {
+ if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BINARY))
+ return CMD_RET_SUCCESS;
+
+ if (IS_ENABLED(CONFIG_CMD_BOOTEFI_HELLO) &&
+ !strcmp(argv[1], "hello")) {
image_buf = __efi_helloworld_begin;
size = __efi_helloworld_end - __efi_helloworld_begin;
efi_clear_bootdev();
- } else
-#endif
- {
+ } else {
addr = strtoul(argv[1], NULL, 16);
/* Check that a numeric value was passed */
if (!addr)
@@ -663,6 +678,21 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
size = image_size;
}
}
+
+ /* Initialize EFI drivers */
+ ret = efi_init_obj_list();
+ if (ret != EFI_SUCCESS) {
+ log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+ ret & ~EFI_ERROR_MASK);
+ return CMD_RET_FAILURE;
+ }
+
+ ret = efi_install_fdt(fdt);
+ if (ret == EFI_INVALID_PARAMETER)
+ return CMD_RET_USAGE;
+ else if (ret != EFI_SUCCESS)
+ return CMD_RET_FAILURE;
+
ret = efi_run_image(image_buf, size);
if (ret != EFI_SUCCESS)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index e24410505f40..48d4999e56a9 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -878,14 +878,12 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
efi_status_t __efi_runtime EFIAPI efi_set_time(struct efi_time *time);
-#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
/*
* Entry point for the tests of the EFI API.
* It is called by 'bootefi selftest'
*/
efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
struct efi_system_table *systab);
-#endif
efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
const efi_guid_t *vendor, u32 *attributes,
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [RFC 03/13] cmd: bootefi: carve out EFI boot manager interface
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
2023-10-26 5:30 ` [RFC 01/13] cmd: bootefi: unfold do_bootefi_image() AKASHI Takahiro
2023-10-26 5:30 ` [RFC 02/13] cmd: bootefi: re-organize do_bootefi_image() AKASHI Takahiro
@ 2023-10-26 5:30 ` AKASHI Takahiro
2023-10-26 5:30 ` [RFC 04/13] cmd: bootefi: carve out binary execution interface AKASHI Takahiro
` (10 subsequent siblings)
13 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
Carve EFI boot manager related code out of do_bootefi_image().
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/bootefi.c | 43 ++++++++++++++++++++++++-------------------
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index ae00bba3b4f0..899ed90f6817 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -401,28 +401,40 @@ out:
}
/**
- * do_efibootmgr() - execute EFI boot manager
+ * efi_bootmgr_run() - execute EFI boot manager
+ * fdt: Flat device tree
+ *
+ * Invoke EFI boot manager and execute a binary depending on
+ * boot options. If @fdt is not NULL, it will be passed to
+ * the executed binary.
*
* Return: status code
*/
-static int do_efibootmgr(void)
+static efi_status_t efi_bootmgr_run(void *fdt)
{
efi_handle_t handle;
- efi_status_t ret;
void *load_options;
+ efi_status_t ret;
- ret = efi_bootmgr_load(&handle, &load_options);
+ /* Initialize EFI drivers */
+ ret = efi_init_obj_list();
if (ret != EFI_SUCCESS) {
- log_notice("EFI boot manager: Cannot load any image\n");
+ log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+ ret & ~EFI_ERROR_MASK);
return CMD_RET_FAILURE;
}
- ret = do_bootefi_exec(handle, load_options);
-
+ ret = efi_install_fdt(fdt);
if (ret != EFI_SUCCESS)
- return CMD_RET_FAILURE;
+ return ret;
- return CMD_RET_SUCCESS;
+ ret = efi_bootmgr_load(&handle, &load_options);
+ if (ret != EFI_SUCCESS) {
+ log_notice("EFI boot manager: Cannot load any image\n");
+ return ret;
+ }
+
+ return do_bootefi_exec(handle, load_options);
}
/**
@@ -612,21 +624,14 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
!strcmp(argv[1], "bootmgr")) {
- /* Initialize EFI drivers */
- ret = efi_init_obj_list();
- if (ret != EFI_SUCCESS) {
- log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
- ret & ~EFI_ERROR_MASK);
- return CMD_RET_FAILURE;
- }
+ ret = efi_bootmgr_run(fdt);
- ret = efi_install_fdt(fdt);
if (ret == EFI_INVALID_PARAMETER)
return CMD_RET_USAGE;
- else if (ret != EFI_SUCCESS)
+ else if (ret)
return CMD_RET_FAILURE;
- return do_efibootmgr();
+ return CMD_RET_SUCCESS;
}
if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [RFC 04/13] cmd: bootefi: carve out binary execution interface
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
` (2 preceding siblings ...)
2023-10-26 5:30 ` [RFC 03/13] cmd: bootefi: carve out EFI boot manager interface AKASHI Takahiro
@ 2023-10-26 5:30 ` AKASHI Takahiro
2023-10-26 5:30 ` [RFC 05/13] cmd: bootefi: move library interfaces under lib/efi_loader AKASHI Takahiro
` (9 subsequent siblings)
13 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
Carve binary execution code out of do_bootefi_image().
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/bootefi.c | 46 ++++++++++++++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 899ed90f6817..8b0bd07f1ff8 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -503,6 +503,36 @@ out:
return (ret != EFI_SUCCESS) ? ret : ret2;
}
+/**
+ * efi_binary_run() - run loaded UEFI image
+ *
+ * @image: memory address of the UEFI image
+ * @size: size of the UEFI image
+ *
+ * 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
+ */
+static efi_status_t efi_binary_run(void *image, size_t size, void *fdt)
+{
+ efi_status_t ret;
+
+ /* Initialize EFI drivers */
+ ret = efi_init_obj_list();
+ if (ret != EFI_SUCCESS) {
+ log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+ ret & ~EFI_ERROR_MASK);
+ return ret;
+ }
+
+ ret = efi_install_fdt(fdt);
+ if (ret != EFI_SUCCESS)
+ return ret;
+
+ return efi_run_image(image, size);
+}
+
static efi_status_t bootefi_run_prepare(const char *load_options_path,
struct efi_device_path *device_path,
struct efi_device_path *image_path,
@@ -684,23 +714,11 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
}
}
- /* Initialize EFI drivers */
- ret = efi_init_obj_list();
- if (ret != EFI_SUCCESS) {
- log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
- ret & ~EFI_ERROR_MASK);
- return CMD_RET_FAILURE;
- }
+ ret = efi_binary_run(image_buf, size, fdt);
- ret = efi_install_fdt(fdt);
if (ret == EFI_INVALID_PARAMETER)
return CMD_RET_USAGE;
- else if (ret != EFI_SUCCESS)
- return CMD_RET_FAILURE;
-
- ret = efi_run_image(image_buf, size);
-
- if (ret != EFI_SUCCESS)
+ else if (ret)
return CMD_RET_FAILURE;
return CMD_RET_SUCCESS;
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [RFC 05/13] cmd: bootefi: move library interfaces under lib/efi_loader
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
` (3 preceding siblings ...)
2023-10-26 5:30 ` [RFC 04/13] cmd: bootefi: carve out binary execution interface AKASHI Takahiro
@ 2023-10-26 5:30 ` AKASHI Takahiro
2023-10-26 5:30 ` [RFC 06/13] cmd: efidebug: ease efi configuration dependency AKASHI Takahiro
` (8 subsequent siblings)
13 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
In the prior commits, interfaces for executing EFI binary or boot manager
were carved out. Move them under efi_loader directory so that they can
be called from other places without depending on bootefi command.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/bootefi.c | 546 ++---------------------------------
include/efi_loader.h | 10 +
lib/efi_loader/efi_bootmgr.c | 517 +++++++++++++++++++++++++++++++++
3 files changed, 551 insertions(+), 522 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 8b0bd07f1ff8..9cf9027bf409 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -7,531 +7,22 @@
#define LOG_CATEGORY LOGC_EFI
-#include <common.h>
-#include <bootm.h>
-#include <charset.h>
#include <command.h>
-#include <dm.h>
+#include <efi.h>
#include <efi_loader.h>
-#include <efi_selftest.h>
-#include <env.h>
-#include <errno.h>
-#include <image.h>
+#include <exports.h>
#include <log.h>
#include <malloc.h>
-#include <asm/global_data.h>
-#include <linux/libfdt.h>
-#include <linux/libfdt_env.h>
#include <mapmem.h>
-#include <memalign.h>
+#include <vsprintf.h>
#include <asm-generic/sections.h>
-#include <linux/linkage.h>
+#include <asm/global_data.h>
+#include <linux/string.h>
DECLARE_GLOBAL_DATA_PTR;
-static struct efi_device_path *bootefi_image_path;
-static struct efi_device_path *bootefi_device_path;
-static void *image_addr;
-static size_t image_size;
-
-/**
- * efi_get_image_parameters() - return image parameters
- *
- * @img_addr: address of loaded image in memory
- * @img_size: size of loaded image
- */
-void efi_get_image_parameters(void **img_addr, size_t *img_size)
-{
- *img_addr = image_addr;
- *img_size = image_size;
-}
-
-/**
- * efi_clear_bootdev() - clear boot device
- */
-static void efi_clear_bootdev(void)
-{
- efi_free_pool(bootefi_device_path);
- efi_free_pool(bootefi_image_path);
- bootefi_device_path = NULL;
- bootefi_image_path = NULL;
- image_addr = NULL;
- image_size = 0;
-}
-
-/**
- * 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.
- *
- * @dev: device, e.g. "MMC"
- * @devnr: number of the device, e.g. "1:2"
- * @path: path to file loaded
- * @buffer: buffer with file loaded
- * @buffer_size: size of file loaded
- */
-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,
- devnr, path, buffer, buffer_size);
-
- /* Forget overwritten image */
- if (buffer + buffer_size >= image_addr &&
- image_addr + image_size >= buffer)
- efi_clear_bootdev();
-
- /* Remember only PE-COFF and FIT images */
- if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) {
- if (IS_ENABLED(CONFIG_FIT) &&
- !fit_check_format(buffer, IMAGE_SIZE_INVAL)) {
- /*
- * FIT images of type EFI_OS are started via command
- * bootm. We should not use their boot device with the
- * bootefi command.
- */
- buffer = 0;
- buffer_size = 0;
- } else {
- log_debug("- not remembering image\n");
- return;
- }
- }
-
- /* efi_set_bootdev() is typically called repeatedly, recover memory */
- efi_clear_bootdev();
-
- 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 {
- log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
- efi_clear_bootdev();
- }
-}
-
-/**
- * efi_env_set_load_options() - set load options from environment variable
- *
- * @handle: the image handle
- * @env_var: name of the environment variable
- * @load_options: pointer to load options (output)
- * Return: status code
- */
-static efi_status_t efi_env_set_load_options(efi_handle_t handle,
- const char *env_var,
- u16 **load_options)
-{
- const char *env = env_get(env_var);
- size_t size;
- u16 *pos;
- efi_status_t ret;
-
- *load_options = NULL;
- if (!env)
- return EFI_SUCCESS;
- size = sizeof(u16) * (utf8_utf16_strlen(env) + 1);
- pos = calloc(size, 1);
- if (!pos)
- return EFI_OUT_OF_RESOURCES;
- *load_options = pos;
- utf8_utf16_strcpy(&pos, env);
- ret = efi_set_load_options(handle, size, *load_options);
- if (ret != EFI_SUCCESS) {
- free(*load_options);
- *load_options = NULL;
- }
- return ret;
-}
-
-#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
-
-/**
- * copy_fdt() - Copy the device tree to a new location available to EFI
- *
- * The FDT is copied to a suitable location within the EFI memory map.
- * Additional 12 KiB are added to the space in case the device tree needs to be
- * expanded later with fdt_open_into().
- *
- * @fdtp: On entry a pointer to the flattened device tree.
- * On exit a pointer to the copy of the flattened device tree.
- * FDT start
- * Return: status code
- */
-static efi_status_t copy_fdt(void **fdtp)
-{
- unsigned long fdt_ram_start = -1L, fdt_pages;
- efi_status_t ret = 0;
- void *fdt, *new_fdt;
- u64 new_fdt_addr;
- uint fdt_size;
- int i;
-
- for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
- u64 ram_start = gd->bd->bi_dram[i].start;
- u64 ram_size = gd->bd->bi_dram[i].size;
-
- if (!ram_size)
- continue;
-
- if (ram_start < fdt_ram_start)
- fdt_ram_start = ram_start;
- }
-
- /*
- * Give us at least 12 KiB of breathing room in case the device tree
- * needs to be expanded later.
- */
- fdt = *fdtp;
- fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
- fdt_size = fdt_pages << EFI_PAGE_SHIFT;
-
- ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
- EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
- &new_fdt_addr);
- if (ret != EFI_SUCCESS) {
- log_err("ERROR: Failed to reserve space for FDT\n");
- goto done;
- }
- new_fdt = (void *)(uintptr_t)new_fdt_addr;
- memcpy(new_fdt, fdt, fdt_totalsize(fdt));
- fdt_set_totalsize(new_fdt, fdt_size);
-
- *fdtp = (void *)(uintptr_t)new_fdt_addr;
-done:
- return ret;
-}
-
-/**
- * get_config_table() - get configuration table
- *
- * @guid: GUID of the configuration table
- * Return: pointer to configuration table or NULL
- */
-static void *get_config_table(const efi_guid_t *guid)
-{
- size_t i;
-
- for (i = 0; i < systab.nr_tables; i++) {
- if (!guidcmp(guid, &systab.tables[i].guid))
- return systab.tables[i].table;
- }
- return NULL;
-}
-
-#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
-
-/**
- * efi_install_fdt() - install device tree
- *
- * If fdt is not EFI_FDT_USE_INTERNAL, the device tree located at that memory
- * address will will be installed as configuration table, otherwise the device
- * tree located at the address indicated by environment variable fdt_addr or as
- * fallback fdtcontroladdr will be used.
- *
- * On architectures using ACPI tables device trees shall not be installed as
- * configuration table.
- *
- * @fdt: address of device tree or EFI_FDT_USE_INTERNAL to use the
- * the hardware device tree as indicated by environment variable
- * fdt_addr or as fallback the internal device tree as indicated by
- * the environment variable fdtcontroladdr
- * Return: status code
- */
-efi_status_t efi_install_fdt(void *fdt)
-{
- /*
- * The EBBR spec requires that we have either an FDT or an ACPI table
- * but not both.
- */
-#if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
- if (fdt) {
- log_warning("WARNING: Can't have ACPI table and device tree - ignoring DT.\n");
- return EFI_SUCCESS;
- }
-#else
- struct bootm_headers img = { 0 };
- efi_status_t ret;
-
- if (fdt == EFI_FDT_USE_INTERNAL) {
- const char *fdt_opt;
- uintptr_t fdt_addr;
-
- /* Look for device tree that is already installed */
- if (get_config_table(&efi_guid_fdt))
- return EFI_SUCCESS;
- /* Check if there is a hardware device tree */
- fdt_opt = env_get("fdt_addr");
- /* Use our own device tree as fallback */
- if (!fdt_opt) {
- fdt_opt = env_get("fdtcontroladdr");
- if (!fdt_opt) {
- log_err("ERROR: need device tree\n");
- return EFI_NOT_FOUND;
- }
- }
- fdt_addr = hextoul(fdt_opt, NULL);
- if (!fdt_addr) {
- log_err("ERROR: invalid $fdt_addr or $fdtcontroladdr\n");
- return EFI_LOAD_ERROR;
- }
- fdt = map_sysmem(fdt_addr, 0);
- }
-
- /* Install device tree */
- if (fdt_check_header(fdt)) {
- log_err("ERROR: invalid device tree\n");
- return EFI_LOAD_ERROR;
- }
-
- /* Prepare device tree for payload */
- ret = copy_fdt(&fdt);
- if (ret) {
- log_err("ERROR: out of memory\n");
- return EFI_OUT_OF_RESOURCES;
- }
-
- if (image_setup_libfdt(&img, fdt, 0, NULL)) {
- log_err("ERROR: failed to process device tree\n");
- return EFI_LOAD_ERROR;
- }
-
- /* Create memory reservations as indicated by the device tree */
- efi_carve_out_dt_rsv(fdt);
-
- efi_try_purge_kaslr_seed(fdt);
-
- if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
- ret = efi_tcg2_measure_dtb(fdt);
- if (ret == EFI_SECURITY_VIOLATION) {
- log_err("ERROR: failed to measure DTB\n");
- return ret;
- }
- }
-
- /* Install device tree as UEFI table */
- ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
- if (ret != EFI_SUCCESS) {
- log_err("ERROR: failed to install device tree\n");
- return ret;
- }
-#endif /* GENERATE_ACPI_TABLE */
-
- return EFI_SUCCESS;
-}
-
-/**
- * do_bootefi_exec() - execute EFI binary
- *
- * The image indicated by @handle is started. When it returns the allocated
- * memory for the @load_options is freed.
- *
- * @handle: handle of loaded image
- * @load_options: load options
- * Return: status code
- *
- * Load the EFI binary into a newly assigned memory unwinding the relocation
- * information, install the loaded image protocol, and call the binary.
- */
-static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
-{
- efi_status_t ret;
- efi_uintn_t exit_data_size = 0;
- u16 *exit_data = NULL;
-
- /* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */
- switch_to_non_secure_mode();
-
- /*
- * The UEFI standard requires that the watchdog timer is set to five
- * minutes when invoking an EFI boot option.
- *
- * Unified Extensible Firmware Interface (UEFI), version 2.7 Errata A
- * 7.5. Miscellaneous Boot Services - EFI_BOOT_SERVICES.SetWatchdogTimer
- */
- ret = efi_set_watchdog(300);
- if (ret != EFI_SUCCESS) {
- log_err("ERROR: Failed to set watchdog timer\n");
- goto out;
- }
-
- /* Call our payload! */
- ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data));
- if (ret != EFI_SUCCESS) {
- log_err("## Application failed, r = %lu\n",
- ret & ~EFI_ERROR_MASK);
- if (exit_data) {
- log_err("## %ls\n", exit_data);
- efi_free_pool(exit_data);
- }
- }
-
- efi_restore_gd();
-
-out:
- free(load_options);
-
- if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
- if (efi_initrd_deregister() != EFI_SUCCESS)
- log_err("Failed to remove loadfile2 for initrd\n");
- }
-
- /* Control is returned to U-Boot, disable EFI watchdog */
- efi_set_watchdog(0);
-
- return ret;
-}
-
-/**
- * efi_bootmgr_run() - execute EFI boot manager
- * fdt: Flat device tree
- *
- * Invoke EFI boot manager and execute a binary depending on
- * boot options. If @fdt is not NULL, it will be passed to
- * the executed binary.
- *
- * Return: status code
- */
-static efi_status_t efi_bootmgr_run(void *fdt)
-{
- efi_handle_t handle;
- void *load_options;
- efi_status_t ret;
-
- /* Initialize EFI drivers */
- ret = efi_init_obj_list();
- if (ret != EFI_SUCCESS) {
- log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
- ret & ~EFI_ERROR_MASK);
- return CMD_RET_FAILURE;
- }
-
- ret = efi_install_fdt(fdt);
- if (ret != EFI_SUCCESS)
- return ret;
-
- ret = efi_bootmgr_load(&handle, &load_options);
- if (ret != EFI_SUCCESS) {
- log_notice("EFI boot manager: Cannot load any image\n");
- return ret;
- }
-
- return do_bootefi_exec(handle, load_options);
-}
-
-/**
- * efi_run_image() - run loaded UEFI image
- *
- * @source_buffer: memory address of the UEFI image
- * @source_size: size of the UEFI image
- * Return: status code
- */
-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;
- struct efi_device_path *msg_path;
- efi_status_t ret, ret2;
- u16 *load_options;
-
- 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)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_append(bootefi_device_path,
- bootefi_image_path);
- msg_path = bootefi_image_path;
- log_debug("Loaded from disk\n");
- }
-
- log_info("Booting %pD\n", msg_path);
-
- ret = EFI_CALL(efi_load_image(false, efi_root, file_path, source_buffer,
- source_size, &handle));
- if (ret != EFI_SUCCESS) {
- log_err("Loading image failed\n");
- goto out;
- }
-
- /* Transfer environment variable as load options */
- ret = efi_env_set_load_options(handle, "bootargs", &load_options);
- if (ret != EFI_SUCCESS)
- goto out;
-
- ret = do_bootefi_exec(handle, load_options);
-
-out:
- ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle,
- &efi_guid_device_path,
- file_path, NULL);
- efi_free_pool(file_path);
- return (ret != EFI_SUCCESS) ? ret : ret2;
-}
-
-/**
- * efi_binary_run() - run loaded UEFI image
- *
- * @image: memory address of the UEFI image
- * @size: size of the UEFI image
- *
- * 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
- */
-static efi_status_t efi_binary_run(void *image, size_t size, void *fdt)
-{
- efi_status_t ret;
-
- /* Initialize EFI drivers */
- ret = efi_init_obj_list();
- if (ret != EFI_SUCCESS) {
- log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
- ret & ~EFI_ERROR_MASK);
- return ret;
- }
-
- ret = efi_install_fdt(fdt);
- if (ret != EFI_SUCCESS)
- return ret;
-
- return efi_run_image(image, size);
-}
+static struct efi_device_path *test_image_path;
+static struct efi_device_path *test_device_path;
static efi_status_t bootefi_run_prepare(const char *load_options_path,
struct efi_device_path *device_path,
@@ -574,23 +65,26 @@ static efi_status_t bootefi_test_prepare
efi_status_t ret;
/* Construct a dummy device path */
- bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0);
- if (!bootefi_device_path)
+ test_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0);
+ if (!test_device_path)
return EFI_OUT_OF_RESOURCES;
- bootefi_image_path = efi_dp_from_file(NULL, path);
- if (!bootefi_image_path) {
+ test_image_path = efi_dp_from_file(NULL, path);
+ if (!test_image_path) {
ret = EFI_OUT_OF_RESOURCES;
goto failure;
}
- ret = bootefi_run_prepare(load_options_path, bootefi_device_path,
- bootefi_image_path, image_objp,
+ ret = bootefi_run_prepare(load_options_path, test_device_path,
+ test_image_path, image_objp,
loaded_image_infop);
if (ret == EFI_SUCCESS)
return ret;
failure:
+ efi_free_pool(test_device_path);
+ efi_free_pool(test_image_path);
+ /* TODO: not sure calling clear function is necessary */
efi_clear_bootdev();
return ret;
}
@@ -615,6 +109,8 @@ static int do_efi_selftest(void)
ret = EFI_CALL(efi_selftest(&image_obj->header, &systab));
efi_restore_gd();
free(loaded_image_info->load_options);
+ efi_free_pool(test_device_path);
+ efi_free_pool(test_image_path);
if (ret != EFI_SUCCESS)
efi_delete_handle(&image_obj->header);
else
@@ -639,6 +135,8 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
char *p;
void *fdt, *image_buf;
unsigned long addr, size;
+ void *image_addr;
+ size_t image_size;
if (argc < 2)
return CMD_RET_USAGE;
@@ -690,6 +188,7 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
!strcmp(argv[1], "hello")) {
image_buf = __efi_helloworld_begin;
size = __efi_helloworld_end - __efi_helloworld_begin;
+ /* TODO: not sure calling clear function is necessary */
efi_clear_bootdev();
} else {
addr = strtoul(argv[1], NULL, 16);
@@ -705,6 +204,9 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
return CMD_RET_USAGE;
efi_clear_bootdev();
} else {
+ /* Image should be already loaded */
+ efi_get_image_parameters(&image_addr, &image_size);
+
if (image_buf != image_addr) {
log_err("No UEFI binary known at %s\n",
argv[1]);
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 48d4999e56a9..031a8f76846a 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -91,6 +91,8 @@ efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len);
* back to u-boot world
*/
void efi_restore_gd(void);
+/* Call this to unset the current device name */
+void efi_clear_bootdev(void);
/* Call this to set the current device name */
void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
void *buffer, size_t buffer_size);
@@ -115,6 +117,7 @@ static inline efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len)
/* No loader configured, stub out EFI_ENTRY */
static inline void efi_restore_gd(void) { }
+static inline void efi_clear_bootdev(void) { }
static inline void efi_set_bootdev(const char *dev, const char *devnr,
const char *path, void *buffer,
size_t buffer_size) { }
@@ -526,14 +529,21 @@ efi_status_t efi_bootmgr_get_unused_bootoption(u16 *buf,
efi_status_t efi_bootmgr_update_media_device_boot_option(void);
/* Delete selected boot option */
efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index);
+/* Invoke EFI boot manager */
+efi_status_t efi_bootmgr_run(void *fdt);
/* search the boot option index in BootOrder */
bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index);
/* Set up console modes */
void efi_setup_console_size(void);
+/* Set up load options from environment variable */
+efi_status_t efi_env_set_load_options(efi_handle_t handle, const char *env_var,
+ u16 **load_options);
/* Install device tree */
efi_status_t efi_install_fdt(void *fdt);
/* Run loaded UEFI image */
efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size);
+/* Run loaded UEFI image with given fdt */
+efi_status_t efi_binary_run(void *image, size_t size, void *fdt);
/* Initialize variable services */
efi_status_t efi_init_variables(void);
/* Notify ExitBootServices() is called */
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index a40762c74c83..cc8d88b4b025 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -3,6 +3,8 @@
* EFI boot manager
*
* Copyright (c) 2017 Rob Clark
+ * For the code moved from cmd/bootefi.c
+ * Copyright (c) 2016 Alexander Graf
*/
#define LOG_CATEGORY LOGC_EFI
@@ -16,6 +18,17 @@
#include <efi_variable.h>
#include <asm/unaligned.h>
+/* TODO: temporarily added here; clean up later */
+#include <bootm.h>
+#include <efi_selftest.h>
+#include <env.h>
+#include <mapmem.h>
+#include <asm/global_data.h>
+#include <linux/libfdt.h>
+#include <linux/libfdt_env.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
static const struct efi_boot_services *bs;
static const struct efi_runtime_services *rs;
@@ -729,3 +742,507 @@ out:
return EFI_SUCCESS;
return ret;
}
+
+static struct efi_device_path *bootefi_image_path;
+static struct efi_device_path *bootefi_device_path;
+static void *image_addr;
+static size_t image_size;
+
+/**
+ * efi_get_image_parameters() - return image parameters
+ *
+ * @img_addr: address of loaded image in memory
+ * @img_size: size of loaded image
+ */
+void efi_get_image_parameters(void **img_addr, size_t *img_size)
+{
+ *img_addr = image_addr;
+ *img_size = image_size;
+}
+
+/**
+ * efi_clear_bootdev() - clear boot device
+ */
+void efi_clear_bootdev(void)
+{
+ efi_free_pool(bootefi_device_path);
+ efi_free_pool(bootefi_image_path);
+ bootefi_device_path = NULL;
+ bootefi_image_path = NULL;
+ image_addr = NULL;
+ image_size = 0;
+}
+
+/**
+ * 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.
+ *
+ * @dev: device, e.g. "MMC"
+ * @devnr: number of the device, e.g. "1:2"
+ * @path: path to file loaded
+ * @buffer: buffer with file loaded
+ * @buffer_size: size of file loaded
+ */
+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,
+ devnr, path, buffer, buffer_size);
+
+ /* Forget overwritten image */
+ if (buffer + buffer_size >= image_addr &&
+ image_addr + image_size >= buffer)
+ efi_clear_bootdev();
+
+ /* Remember only PE-COFF and FIT images */
+ if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) {
+ if (IS_ENABLED(CONFIG_FIT) &&
+ !fit_check_format(buffer, IMAGE_SIZE_INVAL)) {
+ /*
+ * FIT images of type EFI_OS are started via command
+ * bootm. We should not use their boot device with the
+ * bootefi command.
+ */
+ buffer = 0;
+ buffer_size = 0;
+ } else {
+ log_debug("- not remembering image\n");
+ return;
+ }
+ }
+
+ /* efi_set_bootdev() is typically called repeatedly, recover memory */
+ efi_clear_bootdev();
+
+ 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 {
+ log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
+ efi_clear_bootdev();
+ }
+}
+
+/**
+ * efi_env_set_load_options() - set load options from environment variable
+ *
+ * @handle: the image handle
+ * @env_var: name of the environment variable
+ * @load_options: pointer to load options (output)
+ * Return: status code
+ */
+efi_status_t efi_env_set_load_options(efi_handle_t handle,
+ const char *env_var,
+ u16 **load_options)
+{
+ const char *env = env_get(env_var);
+ size_t size;
+ u16 *pos;
+ efi_status_t ret;
+
+ *load_options = NULL;
+ if (!env)
+ return EFI_SUCCESS;
+ size = sizeof(u16) * (utf8_utf16_strlen(env) + 1);
+ pos = calloc(size, 1);
+ if (!pos)
+ return EFI_OUT_OF_RESOURCES;
+ *load_options = pos;
+ utf8_utf16_strcpy(&pos, env);
+ ret = efi_set_load_options(handle, size, *load_options);
+ if (ret != EFI_SUCCESS) {
+ free(*load_options);
+ *load_options = NULL;
+ }
+ return ret;
+}
+
+#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
+
+/**
+ * copy_fdt() - Copy the device tree to a new location available to EFI
+ *
+ * The FDT is copied to a suitable location within the EFI memory map.
+ * Additional 12 KiB are added to the space in case the device tree needs to be
+ * expanded later with fdt_open_into().
+ *
+ * @fdtp: On entry a pointer to the flattened device tree.
+ * On exit a pointer to the copy of the flattened device tree.
+ * FDT start
+ * Return: status code
+ */
+static efi_status_t copy_fdt(void **fdtp)
+{
+ unsigned long fdt_ram_start = -1L, fdt_pages;
+ efi_status_t ret = 0;
+ void *fdt, *new_fdt;
+ u64 new_fdt_addr;
+ uint fdt_size;
+ int i;
+
+ for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
+ u64 ram_start = gd->bd->bi_dram[i].start;
+ u64 ram_size = gd->bd->bi_dram[i].size;
+
+ if (!ram_size)
+ continue;
+
+ if (ram_start < fdt_ram_start)
+ fdt_ram_start = ram_start;
+ }
+
+ /*
+ * Give us at least 12 KiB of breathing room in case the device tree
+ * needs to be expanded later.
+ */
+ fdt = *fdtp;
+ fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
+ fdt_size = fdt_pages << EFI_PAGE_SHIFT;
+
+ ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
+ EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
+ &new_fdt_addr);
+ if (ret != EFI_SUCCESS) {
+ log_err("ERROR: Failed to reserve space for FDT\n");
+ goto done;
+ }
+ new_fdt = (void *)(uintptr_t)new_fdt_addr;
+ memcpy(new_fdt, fdt, fdt_totalsize(fdt));
+ fdt_set_totalsize(new_fdt, fdt_size);
+
+ *fdtp = (void *)(uintptr_t)new_fdt_addr;
+done:
+ return ret;
+}
+
+/**
+ * get_config_table() - get configuration table
+ *
+ * @guid: GUID of the configuration table
+ * Return: pointer to configuration table or NULL
+ */
+static void *get_config_table(const efi_guid_t *guid)
+{
+ size_t i;
+
+ for (i = 0; i < systab.nr_tables; i++) {
+ if (!guidcmp(guid, &systab.tables[i].guid))
+ return systab.tables[i].table;
+ }
+ return NULL;
+}
+
+#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
+
+/**
+ * efi_install_fdt() - install device tree
+ *
+ * If fdt is not EFI_FDT_USE_INTERNAL, the device tree located at that memory
+ * address will be installed as configuration table, otherwise the device
+ * tree located at the address indicated by environment variable fdt_addr or as
+ * fallback fdtcontroladdr will be used.
+ *
+ * On architectures using ACPI tables device trees shall not be installed as
+ * configuration table.
+ *
+ * @fdt: address of device tree or EFI_FDT_USE_INTERNAL to use
+ * the hardware device tree as indicated by environment variable
+ * fdt_addr or as fallback the internal device tree as indicated by
+ * the environment variable fdtcontroladdr
+ * Return: status code
+ */
+efi_status_t efi_install_fdt(void *fdt)
+{
+ /*
+ * The EBBR spec requires that we have either an FDT or an ACPI table
+ * but not both.
+ */
+#if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
+ if (fdt) {
+ log_warning("WARNING: Can't have ACPI table and device tree - ignoring DT.\n");
+ return EFI_SUCCESS;
+ }
+#else
+ struct bootm_headers img = { 0 };
+ efi_status_t ret;
+
+ if (fdt == EFI_FDT_USE_INTERNAL) {
+ const char *fdt_opt;
+ uintptr_t fdt_addr;
+
+ /* Look for device tree that is already installed */
+ if (get_config_table(&efi_guid_fdt))
+ return EFI_SUCCESS;
+ /* Check if there is a hardware device tree */
+ fdt_opt = env_get("fdt_addr");
+ /* Use our own device tree as fallback */
+ if (!fdt_opt) {
+ fdt_opt = env_get("fdtcontroladdr");
+ if (!fdt_opt) {
+ log_err("ERROR: need device tree\n");
+ return EFI_NOT_FOUND;
+ }
+ }
+ fdt_addr = hextoul(fdt_opt, NULL);
+ if (!fdt_addr) {
+ log_err("ERROR: invalid $fdt_addr or $fdtcontroladdr\n");
+ return EFI_LOAD_ERROR;
+ }
+ fdt = map_sysmem(fdt_addr, 0);
+ }
+
+ /* Install device tree */
+ if (fdt_check_header(fdt)) {
+ log_err("ERROR: invalid device tree\n");
+ return EFI_LOAD_ERROR;
+ }
+
+ /* Prepare device tree for payload */
+ ret = copy_fdt(&fdt);
+ if (ret) {
+ log_err("ERROR: out of memory\n");
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ if (image_setup_libfdt(&img, fdt, 0, NULL)) {
+ log_err("ERROR: failed to process device tree\n");
+ return EFI_LOAD_ERROR;
+ }
+
+ /* Create memory reservations as indicated by the device tree */
+ efi_carve_out_dt_rsv(fdt);
+
+ efi_try_purge_kaslr_seed(fdt);
+
+ if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
+ ret = efi_tcg2_measure_dtb(fdt);
+ if (ret == EFI_SECURITY_VIOLATION) {
+ log_err("ERROR: failed to measure DTB\n");
+ return ret;
+ }
+ }
+
+ /* Install device tree as UEFI table */
+ ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
+ if (ret != EFI_SUCCESS) {
+ log_err("ERROR: failed to install device tree\n");
+ return ret;
+ }
+#endif /* GENERATE_ACPI_TABLE */
+
+ return EFI_SUCCESS;
+}
+
+/**
+ * do_bootefi_exec() - execute EFI binary
+ *
+ * The image indicated by @handle is started. When it returns the allocated
+ * memory for the @load_options is freed.
+ *
+ * @handle: handle of loaded image
+ * @load_options: load options
+ * Return: status code
+ *
+ * Load the EFI binary into a newly assigned memory unwinding the relocation
+ * information, install the loaded image protocol, and call the binary.
+ */
+static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
+{
+ efi_status_t ret;
+ efi_uintn_t exit_data_size = 0;
+ u16 *exit_data = NULL;
+
+ /* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */
+ switch_to_non_secure_mode();
+
+ /*
+ * The UEFI standard requires that the watchdog timer is set to five
+ * minutes when invoking an EFI boot option.
+ *
+ * Unified Extensible Firmware Interface (UEFI), version 2.7 Errata A
+ * 7.5. Miscellaneous Boot Services - EFI_BOOT_SERVICES.SetWatchdogTimer
+ */
+ ret = efi_set_watchdog(300);
+ if (ret != EFI_SUCCESS) {
+ log_err("ERROR: Failed to set watchdog timer\n");
+ goto out;
+ }
+
+ /* Call our payload! */
+ ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data));
+ if (ret != EFI_SUCCESS) {
+ log_err("## Application failed, r = %lu\n",
+ ret & ~EFI_ERROR_MASK);
+ if (exit_data) {
+ log_err("## %ls\n", exit_data);
+ efi_free_pool(exit_data);
+ }
+ }
+
+ efi_restore_gd();
+
+out:
+ free(load_options);
+
+ if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
+ if (efi_initrd_deregister() != EFI_SUCCESS)
+ log_err("Failed to remove loadfile2 for initrd\n");
+ }
+
+ /* Control is returned to U-Boot, disable EFI watchdog */
+ efi_set_watchdog(0);
+
+ return ret;
+}
+
+/**
+ * efi_bootmgr_run() - execute EFI boot manager
+ * fdt: Flat device tree
+ *
+ * Invoke EFI boot manager and execute a binary depending on
+ * boot options. If @fdt is not NULL, it will be passed to
+ * the executed binary.
+ *
+ * Return: status code
+ */
+efi_status_t efi_bootmgr_run(void *fdt)
+{
+ efi_handle_t handle;
+ void *load_options;
+ efi_status_t ret;
+
+ /* Initialize EFI drivers */
+ ret = efi_init_obj_list();
+ if (ret != EFI_SUCCESS) {
+ log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+ ret & ~EFI_ERROR_MASK);
+ return CMD_RET_FAILURE;
+ }
+
+ ret = efi_install_fdt(fdt);
+ if (ret != EFI_SUCCESS)
+ return ret;
+
+ ret = efi_bootmgr_load(&handle, &load_options);
+ if (ret != EFI_SUCCESS) {
+ log_notice("EFI boot manager: Cannot load any image\n");
+ return ret;
+ }
+
+ return do_bootefi_exec(handle, load_options);
+}
+
+/**
+ * efi_run_image() - run loaded UEFI image
+ *
+ * @source_buffer: memory address of the UEFI image
+ * @source_size: size of the UEFI image
+ * Return: status code
+ */
+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;
+ struct efi_device_path *msg_path;
+ efi_status_t ret, ret2;
+ u16 *load_options;
+
+ 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)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_append(bootefi_device_path,
+ bootefi_image_path);
+ msg_path = bootefi_image_path;
+ log_debug("Loaded from disk\n");
+ }
+
+ log_info("Booting %pD\n", msg_path);
+
+ ret = EFI_CALL(efi_load_image(false, efi_root, file_path, source_buffer,
+ source_size, &handle));
+ if (ret != EFI_SUCCESS) {
+ log_err("Loading image failed\n");
+ goto out;
+ }
+
+ /* Transfer environment variable as load options */
+ ret = efi_env_set_load_options(handle, "bootargs", &load_options);
+ if (ret != EFI_SUCCESS)
+ goto out;
+
+ ret = do_bootefi_exec(handle, load_options);
+
+out:
+ ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle,
+ &efi_guid_device_path,
+ file_path, NULL);
+ efi_free_pool(file_path);
+ return (ret != EFI_SUCCESS) ? ret : ret2;
+}
+
+/**
+ * efi_binary_run() - run loaded UEFI image
+ *
+ * @image: memory address of the UEFI image
+ * @size: size of the UEFI image
+ *
+ * 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)
+{
+ efi_status_t ret;
+
+ /* Initialize EFI drivers */
+ ret = efi_init_obj_list();
+ if (ret != EFI_SUCCESS) {
+ log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+ ret & ~EFI_ERROR_MASK);
+ return -1;
+ }
+
+ ret = efi_install_fdt(fdt);
+ if (ret != EFI_SUCCESS)
+ return ret;
+
+ return efi_run_image(image, size);
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [RFC 06/13] cmd: efidebug: ease efi configuration dependency
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
` (4 preceding siblings ...)
2023-10-26 5:30 ` [RFC 05/13] cmd: bootefi: move library interfaces under lib/efi_loader AKASHI Takahiro
@ 2023-10-26 5:30 ` AKASHI Takahiro
2023-10-26 5:30 ` [RFC 07/13] bootmeth: use efi_loader interfaces instead of bootefi command AKASHI Takahiro
` (7 subsequent siblings)
13 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
Now it is clear that the command actually depends on interfaces,
not "bootefi bootmgr" command.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/efidebug.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 201531ac19fc..c2b2b074e094 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -1335,7 +1335,7 @@ static __maybe_unused int do_efi_test_bootmgr(struct cmd_tbl *cmdtp, int flag,
}
static struct cmd_tbl cmd_efidebug_test_sub[] = {
-#ifdef CONFIG_CMD_BOOTEFI_BOOTMGR
+#ifdef CONFIG_BOOTEFI_BOOTMGR
U_BOOT_CMD_MKENT(bootmgr, CONFIG_SYS_MAXARGS, 1, do_efi_test_bootmgr,
"", ""),
#endif
@@ -1526,7 +1526,7 @@ U_BOOT_LONGHELP(efidebug,
" - show UEFI memory map\n"
"efidebug tables\n"
" - show UEFI configuration tables\n"
-#ifdef CONFIG_CMD_BOOTEFI_BOOTMGR
+#ifdef CONFIG_BOOTEFI_BOOTMGR
"efidebug test bootmgr\n"
" - run simple bootmgr for test\n"
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [RFC 07/13] bootmeth: use efi_loader interfaces instead of bootefi command
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
` (5 preceding siblings ...)
2023-10-26 5:30 ` [RFC 06/13] cmd: efidebug: ease efi configuration dependency AKASHI Takahiro
@ 2023-10-26 5:30 ` AKASHI Takahiro
2023-10-26 5:30 ` [RFC 08/13] efi_loader: split unrelated code from efi_bootmgr.c AKASHI Takahiro
` (6 subsequent siblings)
13 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
Now that efi_loader subsystem provides interfaces that are equivalent
with bootefi command, we can replace command invocations with APIs.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
boot/Kconfig | 4 ++--
boot/Makefile | 2 +-
boot/bootm_os.c | 31 +++++++------------------------
boot/bootmeth_efi.c | 8 +-------
boot/bootmeth_efi_mgr.c | 3 ++-
cmd/Kconfig | 2 +-
test/boot/bootflow.c | 2 +-
7 files changed, 15 insertions(+), 37 deletions(-)
diff --git a/boot/Kconfig b/boot/Kconfig
index fabf6fec2195..a441fac8ade1 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -511,7 +511,7 @@ config BOOTMETH_EXTLINUX_PXE
config BOOTMETH_EFILOADER
bool "Bootdev support for EFI boot"
- depends on CMD_BOOTEFI
+ depends on BOOTEFI_BOOTMGR
default y
help
Enables support for EFI boot using bootdevs. This makes the
@@ -546,7 +546,7 @@ config BOOTMETH_DISTRO
select BOOTMETH_SCRIPT if CMDLINE # E.g. Armbian uses scripts
select BOOTMETH_EXTLINUX # E.g. Debian uses these
select BOOTMETH_EXTLINUX_PXE if CMD_PXE && CMD_NET && DM_ETH
- select BOOTMETH_EFILOADER if CMD_BOOTEFI # E.g. Ubuntu uses this
+ select BOOTMETH_EFILOADER if BOOTEFI_BOOTMGR # E.g. Ubuntu uses this
config SPL_BOOTMETH_VBE
bool "Bootdev support for Verified Boot for Embedded (SPL)"
diff --git a/boot/Makefile b/boot/Makefile
index 3fd048bb41ab..4eaa5eab4b77 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -32,7 +32,7 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_CROS) += bootm.o bootm_os.o bootmeth_cros.o
obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SANDBOX) += bootmeth_sandbox.o
obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SCRIPT) += bootmeth_script.o
ifdef CONFIG_$(SPL_TPL_)BOOTSTD_FULL
-obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += bootmeth_efi_mgr.o
+obj-$(CONFIG_BOOTEFI_BOOTMGR) += bootmeth_efi_mgr.o
obj-$(CONFIG_$(SPL_TPL_)EXPO) += bootflow_menu.o
obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootflow_menu.o
obj-$(CONFIG_$(SPL_TPL_)CEDIT) += cedit.o
diff --git a/boot/bootm_os.c b/boot/bootm_os.c
index 9c035b5be886..029baac478ae 100644
--- a/boot/bootm_os.c
+++ b/boot/bootm_os.c
@@ -494,7 +494,6 @@ static int do_bootm_efi(int flag, int argc, char *const argv[],
struct bootm_headers *images)
{
int ret;
- efi_status_t efi_ret;
void *image_buf;
if (flag != BOOTM_STATE_OS_GO)
@@ -505,37 +504,21 @@ static int do_bootm_efi(int flag, int argc, char *const argv[],
if (ret)
return ret;
- /* Initialize EFI drivers */
- efi_ret = efi_init_obj_list();
- if (efi_ret != EFI_SUCCESS) {
- printf("## Failed to initialize UEFI sub-system: r = %lu\n",
- efi_ret & ~EFI_ERROR_MASK);
- return 1;
- }
+ /* We expect to return */
+ images->os.type = IH_TYPE_STANDALONE;
- /* Install device tree */
- efi_ret = efi_install_fdt(images->ft_len
- ? images->ft_addr : EFI_FDT_USE_INTERNAL);
- if (efi_ret != EFI_SUCCESS) {
- printf("## Failed to install device tree: r = %lu\n",
- efi_ret & ~EFI_ERROR_MASK);
- return 1;
- }
+ image_buf = map_sysmem(images->ep, images->os.image_len);
/* Run EFI image */
printf("## Transferring control to EFI (at address %08lx) ...\n",
images->ep);
bootstage_mark(BOOTSTAGE_ID_RUN_OS);
- /* We expect to return */
- images->os.type = IH_TYPE_STANDALONE;
-
- image_buf = map_sysmem(images->ep, images->os.image_len);
+ ret = efi_binary_run(image_buf, images->os.image_len,
+ images->ft_len
+ ? images->ft_addr : EFI_FDT_USE_INTERNAL);
- efi_ret = efi_run_image(image_buf, images->os.image_len);
- if (efi_ret != EFI_SUCCESS)
- return 1;
- return 0;
+ return ret;
}
#endif
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index ae936c8daa18..2a9f29f9db5a 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -412,7 +412,6 @@ static int distro_efi_read_bootflow(struct udevice *dev, struct bootflow *bflow)
static int distro_efi_boot(struct udevice *dev, struct bootflow *bflow)
{
ulong kernel, fdt;
- char cmd[50];
int ret;
kernel = env_get_hex("kernel_addr_r", 0);
@@ -441,12 +440,7 @@ static int distro_efi_boot(struct udevice *dev, struct bootflow *bflow)
fdt = env_get_hex("fdt_addr_r", 0);
}
- /*
- * At some point we can add a real interface to bootefi so we can call
- * this directly. For now, go through the CLI, like distro boot.
- */
- snprintf(cmd, sizeof(cmd), "bootefi %lx %lx", kernel, fdt);
- if (run_command(cmd, 0))
+ if (efi_binary_run(map_sysmem(kernel, 0), 0, map_sysmem(fdt, 0)))
return log_msg_ret("run", -EINVAL);
return 0;
diff --git a/boot/bootmeth_efi_mgr.c b/boot/bootmeth_efi_mgr.c
index e9d973429f75..f2b47fac190f 100644
--- a/boot/bootmeth_efi_mgr.c
+++ b/boot/bootmeth_efi_mgr.c
@@ -14,6 +14,7 @@
#include <bootmeth.h>
#include <command.h>
#include <dm.h>
+#include <efi_loader.h>
/**
* struct efi_mgr_priv - private info for the efi-mgr driver
@@ -70,7 +71,7 @@ static int efi_mgr_boot(struct udevice *dev, struct bootflow *bflow)
int ret;
/* Booting is handled by the 'bootefi bootmgr' command */
- ret = run_command("bootefi bootmgr", 0);
+ ret = efi_bootmgr_run(EFI_FDT_USE_INTERNAL);
return 0;
}
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 825a41d68aad..81e959c96207 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -274,7 +274,7 @@ config CMD_BOOTMETH
config BOOTM_EFI
bool "Support booting UEFI FIT images"
- depends on CMD_BOOTEFI && CMD_BOOTM && FIT
+ depends on BOOTEFI_BOOTMGR && CMD_BOOTM && FIT
default y
help
Support booting UEFI FIT images via the bootm command.
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index f5b2059140ac..47c552a94165 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -374,7 +374,7 @@ static int bootflow_system(struct unit_test_state *uts)
{
struct udevice *bootstd, *dev;
- if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR))
+ if (!IS_ENABLED(CONFIG_BOOTEFI_BOOTMGR))
return -EAGAIN;
ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd));
ut_assertok(device_bind(bootstd, DM_DRIVER_GET(bootmeth_efi_mgr),
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [RFC 08/13] efi_loader: split unrelated code from efi_bootmgr.c
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
` (6 preceding siblings ...)
2023-10-26 5:30 ` [RFC 07/13] bootmeth: use efi_loader interfaces instead of bootefi command AKASHI Takahiro
@ 2023-10-26 5:30 ` AKASHI Takahiro
2023-10-26 5:30 ` [RFC 09/13] efi_loader: rename BOOTEFI_BOOTMGR to EFI_BOOTMGR AKASHI Takahiro
` (5 subsequent siblings)
13 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
Some code moved from cmd/bootefi.c is actually necessary only for "bootefi
<addr>" command (starting an image manually loaded by a user using U-Boot
load commands or other methods (like JTAG debugger).
The code will never been opted out as unused code by a compiler which
doesn't know how EFI boot manager is implemented. So introduce a new
configuration, CONFIG_EFI_BINARY_EXEC, to enforce theem opted out
explicitly.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
boot/Kconfig | 4 +-
cmd/Kconfig | 6 +-
include/efi_loader.h | 28 +-
lib/efi_loader/Kconfig | 9 +
lib/efi_loader/efi_bootmgr.c | 480 ------------------------------
lib/efi_loader/efi_device_path.c | 3 +-
lib/efi_loader/efi_helper.c | 486 ++++++++++++++++++++++++++++++-
7 files changed, 516 insertions(+), 500 deletions(-)
diff --git a/boot/Kconfig b/boot/Kconfig
index a441fac8ade1..d371d0a25962 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -511,7 +511,7 @@ config BOOTMETH_EXTLINUX_PXE
config BOOTMETH_EFILOADER
bool "Bootdev support for EFI boot"
- depends on BOOTEFI_BOOTMGR
+ depends on EFI_BINARY_EXEC
default y
help
Enables support for EFI boot using bootdevs. This makes the
@@ -546,7 +546,7 @@ config BOOTMETH_DISTRO
select BOOTMETH_SCRIPT if CMDLINE # E.g. Armbian uses scripts
select BOOTMETH_EXTLINUX # E.g. Debian uses these
select BOOTMETH_EXTLINUX_PXE if CMD_PXE && CMD_NET && DM_ETH
- select BOOTMETH_EFILOADER if BOOTEFI_BOOTMGR # E.g. Ubuntu uses this
+ select BOOTMETH_EFILOADER if EFI_BINARY_EXEC # E.g. Ubuntu uses this
config SPL_BOOTMETH_VBE
bool "Bootdev support for Verified Boot for Embedded (SPL)"
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 81e959c96207..bf16fad04d37 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -274,7 +274,7 @@ config CMD_BOOTMETH
config BOOTM_EFI
bool "Support booting UEFI FIT images"
- depends on BOOTEFI_BOOTMGR && CMD_BOOTM && FIT
+ depends on EFI_BINARY_EXEC && CMD_BOOTM && FIT
default y
help
Support booting UEFI FIT images via the bootm command.
@@ -366,7 +366,7 @@ config CMD_BOOTEFI
if CMD_BOOTEFI
config CMD_BOOTEFI_BINARY
bool "Allow booting an EFI binary directly"
- depends on BOOTEFI_BOOTMGR
+ depends on EFI_BINARY_EXEC
default y
help
Select this option to enable direct execution of binary at 'bootefi'.
@@ -397,7 +397,7 @@ config CMD_BOOTEFI_HELLO_COMPILE
config CMD_BOOTEFI_HELLO
bool "Allow booting a standard EFI hello world for testing"
- depends on CMD_BOOTEFI_HELLO_COMPILE
+ depends on CMD_BOOTEFI_BINARY && CMD_BOOTEFI_HELLO_COMPILE
default y if CMD_BOOTEFI_SELFTEST
help
This adds a standard EFI hello world application to U-Boot so that
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 031a8f76846a..e9bc2f540cc8 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -91,11 +91,7 @@ efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len);
* back to u-boot world
*/
void efi_restore_gd(void);
-/* Call this to unset the current device name */
-void efi_clear_bootdev(void);
-/* Call this to set the current device name */
-void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
- void *buffer, size_t buffer_size);
+
/* Called by networking code to memorize the dhcp ack package */
void efi_net_set_dhcp_ack(void *pkt, int len);
/* Print information about all loaded images */
@@ -117,10 +113,6 @@ static inline efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len)
/* No loader configured, stub out EFI_ENTRY */
static inline void efi_restore_gd(void) { }
-static inline void efi_clear_bootdev(void) { }
-static inline void efi_set_bootdev(const char *dev, const char *devnr,
- const char *path, void *buffer,
- size_t buffer_size) { }
static inline void efi_net_set_dhcp_ack(void *pkt, int len) { }
static inline void efi_print_image_infos(void *pc) { }
static inline efi_status_t efi_launch_capsules(void)
@@ -130,6 +122,20 @@ static inline efi_status_t efi_launch_capsules(void)
#endif /* CONFIG_IS_ENABLED(EFI_LOADER) */
+#if CONFIG_IS_ENABLED(EFI_BINARY_EXEC)
+/* Call this to unset the current device name */
+void efi_clear_bootdev(void);
+/* Call this to set the current device name */
+void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
+ void *buffer, size_t buffer_size);
+#else
+static inline void efi_clear_bootdev(void) { }
+
+static inline void efi_set_bootdev(const char *dev, const char *devnr,
+ const char *path, void *buffer,
+ size_t buffer_size) { }
+#endif
+
/* Maximum number of configuration tables */
#define EFI_MAX_CONFIGURATION_TABLES 16
@@ -540,8 +546,8 @@ efi_status_t efi_env_set_load_options(efi_handle_t handle, const char *env_var,
u16 **load_options);
/* Install device tree */
efi_status_t efi_install_fdt(void *fdt);
-/* Run loaded UEFI image */
-efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size);
+/* Execute loaded UEFI image */
+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);
/* Initialize variable services */
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 13cad6342c36..987a990509f3 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -32,6 +32,15 @@ config EFI_LOADER
if EFI_LOADER
+config EFI_BINARY_EXEC
+ bool "Execute UEFI binary"
+ default y
+ help
+ Select this option if you want to execute the UEFI binary after
+ loading it with U-Boot load commands or other methods.
+ You may enable CMD_BOOTEFI_BINARY so that you can use bootefi
+ command to do that.
+
config BOOTEFI_BOOTMGR
bool "UEFI Boot Manager"
default y
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index cc8d88b4b025..06799277f5b5 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -3,8 +3,6 @@
* EFI boot manager
*
* Copyright (c) 2017 Rob Clark
- * For the code moved from cmd/bootefi.c
- * Copyright (c) 2016 Alexander Graf
*/
#define LOG_CATEGORY LOGC_EFI
@@ -18,17 +16,6 @@
#include <efi_variable.h>
#include <asm/unaligned.h>
-/* TODO: temporarily added here; clean up later */
-#include <bootm.h>
-#include <efi_selftest.h>
-#include <env.h>
-#include <mapmem.h>
-#include <asm/global_data.h>
-#include <linux/libfdt.h>
-#include <linux/libfdt_env.h>
-
-DECLARE_GLOBAL_DATA_PTR;
-
static const struct efi_boot_services *bs;
static const struct efi_runtime_services *rs;
@@ -743,377 +730,6 @@ out:
return ret;
}
-static struct efi_device_path *bootefi_image_path;
-static struct efi_device_path *bootefi_device_path;
-static void *image_addr;
-static size_t image_size;
-
-/**
- * efi_get_image_parameters() - return image parameters
- *
- * @img_addr: address of loaded image in memory
- * @img_size: size of loaded image
- */
-void efi_get_image_parameters(void **img_addr, size_t *img_size)
-{
- *img_addr = image_addr;
- *img_size = image_size;
-}
-
-/**
- * efi_clear_bootdev() - clear boot device
- */
-void efi_clear_bootdev(void)
-{
- efi_free_pool(bootefi_device_path);
- efi_free_pool(bootefi_image_path);
- bootefi_device_path = NULL;
- bootefi_image_path = NULL;
- image_addr = NULL;
- image_size = 0;
-}
-
-/**
- * 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.
- *
- * @dev: device, e.g. "MMC"
- * @devnr: number of the device, e.g. "1:2"
- * @path: path to file loaded
- * @buffer: buffer with file loaded
- * @buffer_size: size of file loaded
- */
-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,
- devnr, path, buffer, buffer_size);
-
- /* Forget overwritten image */
- if (buffer + buffer_size >= image_addr &&
- image_addr + image_size >= buffer)
- efi_clear_bootdev();
-
- /* Remember only PE-COFF and FIT images */
- if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) {
- if (IS_ENABLED(CONFIG_FIT) &&
- !fit_check_format(buffer, IMAGE_SIZE_INVAL)) {
- /*
- * FIT images of type EFI_OS are started via command
- * bootm. We should not use their boot device with the
- * bootefi command.
- */
- buffer = 0;
- buffer_size = 0;
- } else {
- log_debug("- not remembering image\n");
- return;
- }
- }
-
- /* efi_set_bootdev() is typically called repeatedly, recover memory */
- efi_clear_bootdev();
-
- 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 {
- log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
- efi_clear_bootdev();
- }
-}
-
-/**
- * efi_env_set_load_options() - set load options from environment variable
- *
- * @handle: the image handle
- * @env_var: name of the environment variable
- * @load_options: pointer to load options (output)
- * Return: status code
- */
-efi_status_t efi_env_set_load_options(efi_handle_t handle,
- const char *env_var,
- u16 **load_options)
-{
- const char *env = env_get(env_var);
- size_t size;
- u16 *pos;
- efi_status_t ret;
-
- *load_options = NULL;
- if (!env)
- return EFI_SUCCESS;
- size = sizeof(u16) * (utf8_utf16_strlen(env) + 1);
- pos = calloc(size, 1);
- if (!pos)
- return EFI_OUT_OF_RESOURCES;
- *load_options = pos;
- utf8_utf16_strcpy(&pos, env);
- ret = efi_set_load_options(handle, size, *load_options);
- if (ret != EFI_SUCCESS) {
- free(*load_options);
- *load_options = NULL;
- }
- return ret;
-}
-
-#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
-
-/**
- * copy_fdt() - Copy the device tree to a new location available to EFI
- *
- * The FDT is copied to a suitable location within the EFI memory map.
- * Additional 12 KiB are added to the space in case the device tree needs to be
- * expanded later with fdt_open_into().
- *
- * @fdtp: On entry a pointer to the flattened device tree.
- * On exit a pointer to the copy of the flattened device tree.
- * FDT start
- * Return: status code
- */
-static efi_status_t copy_fdt(void **fdtp)
-{
- unsigned long fdt_ram_start = -1L, fdt_pages;
- efi_status_t ret = 0;
- void *fdt, *new_fdt;
- u64 new_fdt_addr;
- uint fdt_size;
- int i;
-
- for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
- u64 ram_start = gd->bd->bi_dram[i].start;
- u64 ram_size = gd->bd->bi_dram[i].size;
-
- if (!ram_size)
- continue;
-
- if (ram_start < fdt_ram_start)
- fdt_ram_start = ram_start;
- }
-
- /*
- * Give us at least 12 KiB of breathing room in case the device tree
- * needs to be expanded later.
- */
- fdt = *fdtp;
- fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
- fdt_size = fdt_pages << EFI_PAGE_SHIFT;
-
- ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
- EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
- &new_fdt_addr);
- if (ret != EFI_SUCCESS) {
- log_err("ERROR: Failed to reserve space for FDT\n");
- goto done;
- }
- new_fdt = (void *)(uintptr_t)new_fdt_addr;
- memcpy(new_fdt, fdt, fdt_totalsize(fdt));
- fdt_set_totalsize(new_fdt, fdt_size);
-
- *fdtp = (void *)(uintptr_t)new_fdt_addr;
-done:
- return ret;
-}
-
-/**
- * get_config_table() - get configuration table
- *
- * @guid: GUID of the configuration table
- * Return: pointer to configuration table or NULL
- */
-static void *get_config_table(const efi_guid_t *guid)
-{
- size_t i;
-
- for (i = 0; i < systab.nr_tables; i++) {
- if (!guidcmp(guid, &systab.tables[i].guid))
- return systab.tables[i].table;
- }
- return NULL;
-}
-
-#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
-
-/**
- * efi_install_fdt() - install device tree
- *
- * If fdt is not EFI_FDT_USE_INTERNAL, the device tree located at that memory
- * address will be installed as configuration table, otherwise the device
- * tree located at the address indicated by environment variable fdt_addr or as
- * fallback fdtcontroladdr will be used.
- *
- * On architectures using ACPI tables device trees shall not be installed as
- * configuration table.
- *
- * @fdt: address of device tree or EFI_FDT_USE_INTERNAL to use
- * the hardware device tree as indicated by environment variable
- * fdt_addr or as fallback the internal device tree as indicated by
- * the environment variable fdtcontroladdr
- * Return: status code
- */
-efi_status_t efi_install_fdt(void *fdt)
-{
- /*
- * The EBBR spec requires that we have either an FDT or an ACPI table
- * but not both.
- */
-#if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
- if (fdt) {
- log_warning("WARNING: Can't have ACPI table and device tree - ignoring DT.\n");
- return EFI_SUCCESS;
- }
-#else
- struct bootm_headers img = { 0 };
- efi_status_t ret;
-
- if (fdt == EFI_FDT_USE_INTERNAL) {
- const char *fdt_opt;
- uintptr_t fdt_addr;
-
- /* Look for device tree that is already installed */
- if (get_config_table(&efi_guid_fdt))
- return EFI_SUCCESS;
- /* Check if there is a hardware device tree */
- fdt_opt = env_get("fdt_addr");
- /* Use our own device tree as fallback */
- if (!fdt_opt) {
- fdt_opt = env_get("fdtcontroladdr");
- if (!fdt_opt) {
- log_err("ERROR: need device tree\n");
- return EFI_NOT_FOUND;
- }
- }
- fdt_addr = hextoul(fdt_opt, NULL);
- if (!fdt_addr) {
- log_err("ERROR: invalid $fdt_addr or $fdtcontroladdr\n");
- return EFI_LOAD_ERROR;
- }
- fdt = map_sysmem(fdt_addr, 0);
- }
-
- /* Install device tree */
- if (fdt_check_header(fdt)) {
- log_err("ERROR: invalid device tree\n");
- return EFI_LOAD_ERROR;
- }
-
- /* Prepare device tree for payload */
- ret = copy_fdt(&fdt);
- if (ret) {
- log_err("ERROR: out of memory\n");
- return EFI_OUT_OF_RESOURCES;
- }
-
- if (image_setup_libfdt(&img, fdt, 0, NULL)) {
- log_err("ERROR: failed to process device tree\n");
- return EFI_LOAD_ERROR;
- }
-
- /* Create memory reservations as indicated by the device tree */
- efi_carve_out_dt_rsv(fdt);
-
- efi_try_purge_kaslr_seed(fdt);
-
- if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
- ret = efi_tcg2_measure_dtb(fdt);
- if (ret == EFI_SECURITY_VIOLATION) {
- log_err("ERROR: failed to measure DTB\n");
- return ret;
- }
- }
-
- /* Install device tree as UEFI table */
- ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
- if (ret != EFI_SUCCESS) {
- log_err("ERROR: failed to install device tree\n");
- return ret;
- }
-#endif /* GENERATE_ACPI_TABLE */
-
- return EFI_SUCCESS;
-}
-
-/**
- * do_bootefi_exec() - execute EFI binary
- *
- * The image indicated by @handle is started. When it returns the allocated
- * memory for the @load_options is freed.
- *
- * @handle: handle of loaded image
- * @load_options: load options
- * Return: status code
- *
- * Load the EFI binary into a newly assigned memory unwinding the relocation
- * information, install the loaded image protocol, and call the binary.
- */
-static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
-{
- efi_status_t ret;
- efi_uintn_t exit_data_size = 0;
- u16 *exit_data = NULL;
-
- /* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */
- switch_to_non_secure_mode();
-
- /*
- * The UEFI standard requires that the watchdog timer is set to five
- * minutes when invoking an EFI boot option.
- *
- * Unified Extensible Firmware Interface (UEFI), version 2.7 Errata A
- * 7.5. Miscellaneous Boot Services - EFI_BOOT_SERVICES.SetWatchdogTimer
- */
- ret = efi_set_watchdog(300);
- if (ret != EFI_SUCCESS) {
- log_err("ERROR: Failed to set watchdog timer\n");
- goto out;
- }
-
- /* Call our payload! */
- ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data));
- if (ret != EFI_SUCCESS) {
- log_err("## Application failed, r = %lu\n",
- ret & ~EFI_ERROR_MASK);
- if (exit_data) {
- log_err("## %ls\n", exit_data);
- efi_free_pool(exit_data);
- }
- }
-
- efi_restore_gd();
-
-out:
- free(load_options);
-
- if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
- if (efi_initrd_deregister() != EFI_SUCCESS)
- log_err("Failed to remove loadfile2 for initrd\n");
- }
-
- /* Control is returned to U-Boot, disable EFI watchdog */
- efi_set_watchdog(0);
-
- return ret;
-}
-
/**
* efi_bootmgr_run() - execute EFI boot manager
* fdt: Flat device tree
@@ -1150,99 +766,3 @@ efi_status_t efi_bootmgr_run(void *fdt)
return do_bootefi_exec(handle, load_options);
}
-
-/**
- * efi_run_image() - run loaded UEFI image
- *
- * @source_buffer: memory address of the UEFI image
- * @source_size: size of the UEFI image
- * Return: status code
- */
-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;
- struct efi_device_path *msg_path;
- efi_status_t ret, ret2;
- u16 *load_options;
-
- 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)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_append(bootefi_device_path,
- bootefi_image_path);
- msg_path = bootefi_image_path;
- log_debug("Loaded from disk\n");
- }
-
- log_info("Booting %pD\n", msg_path);
-
- ret = EFI_CALL(efi_load_image(false, efi_root, file_path, source_buffer,
- source_size, &handle));
- if (ret != EFI_SUCCESS) {
- log_err("Loading image failed\n");
- goto out;
- }
-
- /* Transfer environment variable as load options */
- ret = efi_env_set_load_options(handle, "bootargs", &load_options);
- if (ret != EFI_SUCCESS)
- goto out;
-
- ret = do_bootefi_exec(handle, load_options);
-
-out:
- ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle,
- &efi_guid_device_path,
- file_path, NULL);
- efi_free_pool(file_path);
- return (ret != EFI_SUCCESS) ? ret : ret2;
-}
-
-/**
- * efi_binary_run() - run loaded UEFI image
- *
- * @image: memory address of the UEFI image
- * @size: size of the UEFI image
- *
- * 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)
-{
- efi_status_t ret;
-
- /* Initialize EFI drivers */
- ret = efi_init_obj_list();
- if (ret != EFI_SUCCESS) {
- log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
- ret & ~EFI_ERROR_MASK);
- return -1;
- }
-
- ret = efi_install_fdt(fdt);
- if (ret != EFI_SUCCESS)
- return ret;
-
- return efi_run_image(image, size);
-}
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index ed7214f3a347..786d8a70e2ad 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -1090,7 +1090,8 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
if (path && !file)
return EFI_INVALID_PARAMETER;
- if (!strcmp(dev, "Mem") || !strcmp(dev, "hostfs")) {
+ if (IS_ENABLED(CONFIG_EFI_BINARY_EXEC) &&
+ (!strcmp(dev, "Mem") || !strcmp(dev, "hostfs"))) {
/* loadm command and semihosting */
efi_get_image_parameters(&image_addr, &image_size);
diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
index cdfd16ea7742..dd8c38f0cb5c 100644
--- a/lib/efi_loader/efi_helper.c
+++ b/lib/efi_loader/efi_helper.c
@@ -1,17 +1,28 @@
// SPDX-License-Identifier: GPL-2.0+
/*
* Copyright (c) 2020, Linaro Limited
+ * For the code moved from cmd/bootefi.c
+ * Copyright (c) 2016 Alexander Graf
*/
#define LOG_CATEGORY LOGC_EFI
+#include <bootm.h>
#include <common.h>
-#include <env.h>
-#include <malloc.h>
#include <dm.h>
-#include <fs.h>
#include <efi_load_initrd.h>
#include <efi_loader.h>
#include <efi_variable.h>
+#include <env.h>
+#include <fs.h>
+#include <log.h>
+#include <malloc.h>
+#include <mapmem.h>
+#include <vsprintf.h>
+#include <asm/global_data.h>
+#include <linux/libfdt.h>
+#include <linux/libfdt_env.h>
+
+DECLARE_GLOBAL_DATA_PTR;
#if defined(CONFIG_CMD_EFIDEBUG) || defined(CONFIG_EFI_LOAD_FILE2_INITRD)
/* GUID used by Linux to identify the LoadFile2 protocol with the initrd */
@@ -282,3 +293,472 @@ bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *inde
return false;
}
+
+#if !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
+
+/**
+ * copy_fdt() - Copy the device tree to a new location available to EFI
+ *
+ * The FDT is copied to a suitable location within the EFI memory map.
+ * Additional 12 KiB are added to the space in case the device tree needs to be
+ * expanded later with fdt_open_into().
+ *
+ * @fdtp: On entry a pointer to the flattened device tree.
+ * On exit a pointer to the copy of the flattened device tree.
+ * FDT start
+ * Return: status code
+ */
+static efi_status_t copy_fdt(void **fdtp)
+{
+ unsigned long fdt_ram_start = -1L, fdt_pages;
+ efi_status_t ret = 0;
+ void *fdt, *new_fdt;
+ u64 new_fdt_addr;
+ uint fdt_size;
+ int i;
+
+ for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
+ u64 ram_start = gd->bd->bi_dram[i].start;
+ u64 ram_size = gd->bd->bi_dram[i].size;
+
+ if (!ram_size)
+ continue;
+
+ if (ram_start < fdt_ram_start)
+ fdt_ram_start = ram_start;
+ }
+
+ /*
+ * Give us at least 12 KiB of breathing room in case the device tree
+ * needs to be expanded later.
+ */
+ fdt = *fdtp;
+ fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
+ fdt_size = fdt_pages << EFI_PAGE_SHIFT;
+
+ ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
+ EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
+ &new_fdt_addr);
+ if (ret != EFI_SUCCESS) {
+ log_err("ERROR: Failed to reserve space for FDT\n");
+ goto done;
+ }
+ new_fdt = (void *)(uintptr_t)new_fdt_addr;
+ memcpy(new_fdt, fdt, fdt_totalsize(fdt));
+ fdt_set_totalsize(new_fdt, fdt_size);
+
+ *fdtp = (void *)(uintptr_t)new_fdt_addr;
+done:
+ return ret;
+}
+
+/**
+ * get_config_table() - get configuration table
+ *
+ * @guid: GUID of the configuration table
+ * Return: pointer to configuration table or NULL
+ */
+static void *get_config_table(const efi_guid_t *guid)
+{
+ size_t i;
+
+ for (i = 0; i < systab.nr_tables; i++) {
+ if (!guidcmp(guid, &systab.tables[i].guid))
+ return systab.tables[i].table;
+ }
+ return NULL;
+}
+
+#endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
+
+/**
+ * efi_install_fdt() - install device tree
+ *
+ * If fdt is not EFI_FDT_USE_INTERNAL, the device tree located at that memory
+ * address will be installed as configuration table, otherwise the device
+ * tree located at the address indicated by environment variable fdt_addr or as
+ * fallback fdtcontroladdr will be used.
+ *
+ * On architectures using ACPI tables device trees shall not be installed as
+ * configuration table.
+ *
+ * @fdt: address of device tree or EFI_FDT_USE_INTERNAL to use
+ * the hardware device tree as indicated by environment variable
+ * fdt_addr or as fallback the internal device tree as indicated by
+ * the environment variable fdtcontroladdr
+ * Return: status code
+ */
+efi_status_t efi_install_fdt(void *fdt)
+{
+ /*
+ * The EBBR spec requires that we have either an FDT or an ACPI table
+ * but not both.
+ */
+#if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
+ if (fdt) {
+ log_warning("WARNING: Can't have ACPI table and device tree - ignoring DT.\n");
+ return EFI_SUCCESS;
+ }
+#else
+ struct bootm_headers img = { 0 };
+ efi_status_t ret;
+
+ if (fdt == EFI_FDT_USE_INTERNAL) {
+ const char *fdt_opt;
+ uintptr_t fdt_addr;
+
+ /* Look for device tree that is already installed */
+ if (get_config_table(&efi_guid_fdt))
+ return EFI_SUCCESS;
+ /* Check if there is a hardware device tree */
+ fdt_opt = env_get("fdt_addr");
+ /* Use our own device tree as fallback */
+ if (!fdt_opt) {
+ fdt_opt = env_get("fdtcontroladdr");
+ if (!fdt_opt) {
+ log_err("ERROR: need device tree\n");
+ return EFI_NOT_FOUND;
+ }
+ }
+ fdt_addr = hextoul(fdt_opt, NULL);
+ if (!fdt_addr) {
+ log_err("ERROR: invalid $fdt_addr or $fdtcontroladdr\n");
+ return EFI_LOAD_ERROR;
+ }
+ fdt = map_sysmem(fdt_addr, 0);
+ }
+
+ /* Install device tree */
+ if (fdt_check_header(fdt)) {
+ log_err("ERROR: invalid device tree\n");
+ return EFI_LOAD_ERROR;
+ }
+
+ /* Prepare device tree for payload */
+ ret = copy_fdt(&fdt);
+ if (ret) {
+ log_err("ERROR: out of memory\n");
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ if (image_setup_libfdt(&img, fdt, 0, NULL)) {
+ log_err("ERROR: failed to process device tree\n");
+ return EFI_LOAD_ERROR;
+ }
+
+ /* Create memory reservations as indicated by the device tree */
+ efi_carve_out_dt_rsv(fdt);
+
+ efi_try_purge_kaslr_seed(fdt);
+
+ if (CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL_MEASURE_DTB)) {
+ ret = efi_tcg2_measure_dtb(fdt);
+ if (ret == EFI_SECURITY_VIOLATION) {
+ log_err("ERROR: failed to measure DTB\n");
+ return ret;
+ }
+ }
+
+ /* Install device tree as UEFI table */
+ ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
+ if (ret != EFI_SUCCESS) {
+ log_err("ERROR: failed to install device tree\n");
+ return ret;
+ }
+#endif /* GENERATE_ACPI_TABLE */
+
+ return EFI_SUCCESS;
+}
+
+/**
+ * do_bootefi_exec() - execute EFI binary
+ *
+ * The image indicated by @handle is started. When it returns the allocated
+ * memory for the @load_options is freed.
+ *
+ * @handle: handle of loaded image
+ * @load_options: load options
+ * Return: status code
+ *
+ * Load the EFI binary into a newly assigned memory unwinding the relocation
+ * information, install the loaded image protocol, and call the binary.
+ */
+efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
+{
+ efi_status_t ret;
+ efi_uintn_t exit_data_size = 0;
+ u16 *exit_data = NULL;
+
+ /* On ARM switch from EL3 or secure mode to EL2 or non-secure mode */
+ switch_to_non_secure_mode();
+
+ /*
+ * The UEFI standard requires that the watchdog timer is set to five
+ * minutes when invoking an EFI boot option.
+ *
+ * Unified Extensible Firmware Interface (UEFI), version 2.7 Errata A
+ * 7.5. Miscellaneous Boot Services - EFI_BOOT_SERVICES.SetWatchdogTimer
+ */
+ ret = efi_set_watchdog(300);
+ if (ret != EFI_SUCCESS) {
+ log_err("ERROR: Failed to set watchdog timer\n");
+ goto out;
+ }
+
+ /* Call our payload! */
+ ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data));
+ if (ret != EFI_SUCCESS) {
+ log_err("## Application failed, r = %lu\n",
+ ret & ~EFI_ERROR_MASK);
+ if (exit_data) {
+ log_err("## %ls\n", exit_data);
+ efi_free_pool(exit_data);
+ }
+ }
+
+ efi_restore_gd();
+
+out:
+ free(load_options);
+
+ if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
+ if (efi_initrd_deregister() != EFI_SUCCESS)
+ log_err("Failed to remove loadfile2 for initrd\n");
+ }
+
+ /* Control is returned to U-Boot, disable EFI watchdog */
+ efi_set_watchdog(0);
+
+ return ret;
+}
+
+#if CONFIG_IS_ENABLED(EFI_BINARY_EXEC)
+static struct efi_device_path *bootefi_image_path;
+static struct efi_device_path *bootefi_device_path;
+static void *image_addr;
+static size_t image_size;
+
+/**
+ * efi_get_image_parameters() - return image parameters
+ *
+ * @img_addr: address of loaded image in memory
+ * @img_size: size of loaded image
+ */
+void efi_get_image_parameters(void **img_addr, size_t *img_size)
+{
+ *img_addr = image_addr;
+ *img_size = image_size;
+}
+
+/**
+ * efi_clear_bootdev() - clear boot device
+ */
+void efi_clear_bootdev(void)
+{
+ efi_free_pool(bootefi_device_path);
+ efi_free_pool(bootefi_image_path);
+ bootefi_device_path = NULL;
+ bootefi_image_path = NULL;
+ image_addr = NULL;
+ image_size = 0;
+}
+
+/**
+ * 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.
+ *
+ * @dev: device, e.g. "MMC"
+ * @devnr: number of the device, e.g. "1:2"
+ * @path: path to file loaded
+ * @buffer: buffer with file loaded
+ * @buffer_size: size of file loaded
+ */
+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,
+ devnr, path, buffer, buffer_size);
+
+ /* Forget overwritten image */
+ if (buffer + buffer_size >= image_addr &&
+ image_addr + image_size >= buffer)
+ efi_clear_bootdev();
+
+ /* Remember only PE-COFF and FIT images */
+ if (efi_check_pe(buffer, buffer_size, NULL) != EFI_SUCCESS) {
+ if (IS_ENABLED(CONFIG_FIT) &&
+ !fit_check_format(buffer, IMAGE_SIZE_INVAL)) {
+ /*
+ * FIT images of type EFI_OS are started via command
+ * bootm. We should not use their boot device with the
+ * bootefi command.
+ */
+ buffer = 0;
+ buffer_size = 0;
+ } else {
+ log_debug("- not remembering image\n");
+ return;
+ }
+ }
+
+ /* efi_set_bootdev() is typically called repeatedly, recover memory */
+ efi_clear_bootdev();
+
+ 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 {
+ log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
+ efi_clear_bootdev();
+ }
+}
+
+/**
+ * efi_env_set_load_options() - set load options from environment variable
+ *
+ * @handle: the image handle
+ * @env_var: name of the environment variable
+ * @load_options: pointer to load options (output)
+ * Return: status code
+ */
+efi_status_t efi_env_set_load_options(efi_handle_t handle,
+ const char *env_var,
+ u16 **load_options)
+{
+ const char *env = env_get(env_var);
+ size_t size;
+ u16 *pos;
+ efi_status_t ret;
+
+ *load_options = NULL;
+ if (!env)
+ return EFI_SUCCESS;
+ size = sizeof(u16) * (utf8_utf16_strlen(env) + 1);
+ pos = calloc(size, 1);
+ if (!pos)
+ return EFI_OUT_OF_RESOURCES;
+ *load_options = pos;
+ utf8_utf16_strcpy(&pos, env);
+ ret = efi_set_load_options(handle, size, *load_options);
+ if (ret != EFI_SUCCESS) {
+ free(*load_options);
+ *load_options = NULL;
+ }
+ return ret;
+}
+
+/**
+ * efi_run_image() - run loaded UEFI image
+ *
+ * @source_buffer: memory address of the UEFI image
+ * @source_size: size of the UEFI image
+ * Return: status code
+ */
+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;
+ struct efi_device_path *msg_path;
+ efi_status_t ret, ret2;
+ u16 *load_options;
+
+ 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)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_append(bootefi_device_path,
+ bootefi_image_path);
+ msg_path = bootefi_image_path;
+ log_debug("Loaded from disk\n");
+ }
+
+ log_info("Booting %pD\n", msg_path);
+
+ ret = EFI_CALL(efi_load_image(false, efi_root, file_path, source_buffer,
+ source_size, &handle));
+ if (ret != EFI_SUCCESS) {
+ log_err("Loading image failed\n");
+ goto out;
+ }
+
+ /* Transfer environment variable as load options */
+ ret = efi_env_set_load_options(handle, "bootargs", &load_options);
+ if (ret != EFI_SUCCESS)
+ goto out;
+
+ ret = do_bootefi_exec(handle, load_options);
+
+out:
+ ret2 = efi_uninstall_multiple_protocol_interfaces(mem_handle,
+ &efi_guid_device_path,
+ file_path, NULL);
+ efi_free_pool(file_path);
+ return (ret != EFI_SUCCESS) ? ret : ret2;
+}
+
+/**
+ * efi_binary_run() - run loaded UEFI image
+ *
+ * @image: memory address of the UEFI image
+ * @size: size of the UEFI image
+ *
+ * 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)
+{
+ efi_status_t ret;
+
+ /* Initialize EFI drivers */
+ ret = efi_init_obj_list();
+ if (ret != EFI_SUCCESS) {
+ log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+ ret & ~EFI_ERROR_MASK);
+ return ret;
+ }
+
+ ret = efi_install_fdt(fdt);
+ if (ret != EFI_SUCCESS)
+ return ret;
+
+ return efi_run_image(image, size);
+}
+#endif /* CONFIG_EFI_BINARY_EXEC */
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [RFC 09/13] efi_loader: rename BOOTEFI_BOOTMGR to EFI_BOOTMGR
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
` (7 preceding siblings ...)
2023-10-26 5:30 ` [RFC 08/13] efi_loader: split unrelated code from efi_bootmgr.c AKASHI Takahiro
@ 2023-10-26 5:30 ` AKASHI Takahiro
2023-10-26 5:30 ` [RFC 10/13] net: tftp: remove explicit efi configuration dependency AKASHI Takahiro
` (4 subsequent siblings)
13 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
At this point, EFI boot manager interfaces is fully independent from
bootefi command. So just rename the configuration parameter.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
boot/Makefile | 2 +-
cmd/Kconfig | 4 ++--
cmd/efidebug.c | 4 ++--
lib/efi_loader/Kconfig | 2 +-
lib/efi_loader/Makefile | 2 +-
test/boot/bootflow.c | 2 +-
6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/boot/Makefile b/boot/Makefile
index 4eaa5eab4b77..48d74c67d680 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -32,7 +32,7 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_CROS) += bootm.o bootm_os.o bootmeth_cros.o
obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SANDBOX) += bootmeth_sandbox.o
obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SCRIPT) += bootmeth_script.o
ifdef CONFIG_$(SPL_TPL_)BOOTSTD_FULL
-obj-$(CONFIG_BOOTEFI_BOOTMGR) += bootmeth_efi_mgr.o
+obj-$(CONFIG_EFI_BOOTMGR) += bootmeth_efi_mgr.o
obj-$(CONFIG_$(SPL_TPL_)EXPO) += bootflow_menu.o
obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootflow_menu.o
obj-$(CONFIG_$(SPL_TPL_)CEDIT) += cedit.o
diff --git a/cmd/Kconfig b/cmd/Kconfig
index bf16fad04d37..3a8483f7d042 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -375,7 +375,7 @@ config CMD_BOOTEFI_BINARY
config CMD_BOOTEFI_BOOTMGR
bool "UEFI Boot Manager command"
- depends on BOOTEFI_BOOTMGR
+ depends on EFI_BOOTMGR
default y
help
Select this option to enable the 'bootmgr' subcommand of 'bootefi'.
@@ -2122,7 +2122,7 @@ config CMD_EFIDEBUG
config CMD_EFICONFIG
bool "eficonfig - provide menu-driven uefi variables maintenance interface"
default y if !HAS_BOARD_SIZE_LIMIT
- depends on BOOTEFI_BOOTMGR
+ depends on EFI_BOOTMGR
select MENU
help
Enable the 'eficonfig' command which provides the menu-driven UEFI
diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index c2b2b074e094..7ba2331ad19f 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -1335,7 +1335,7 @@ static __maybe_unused int do_efi_test_bootmgr(struct cmd_tbl *cmdtp, int flag,
}
static struct cmd_tbl cmd_efidebug_test_sub[] = {
-#ifdef CONFIG_BOOTEFI_BOOTMGR
+#ifdef CONFIG_EFI_BOOTMGR
U_BOOT_CMD_MKENT(bootmgr, CONFIG_SYS_MAXARGS, 1, do_efi_test_bootmgr,
"", ""),
#endif
@@ -1526,7 +1526,7 @@ U_BOOT_LONGHELP(efidebug,
" - show UEFI memory map\n"
"efidebug tables\n"
" - show UEFI configuration tables\n"
-#ifdef CONFIG_BOOTEFI_BOOTMGR
+#ifdef CONFIG_EFI_BOOTMGR
"efidebug test bootmgr\n"
" - run simple bootmgr for test\n"
#endif
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 987a990509f3..27d3f52897a9 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -41,7 +41,7 @@ config EFI_BINARY_EXEC
You may enable CMD_BOOTEFI_BINARY so that you can use bootefi
command to do that.
-config BOOTEFI_BOOTMGR
+config EFI_BOOTMGR
bool "UEFI Boot Manager"
default y
select BOOTMETH_GLOBAL if BOOTSTD
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 0a2cb6e3c476..f882474bba6f 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -42,7 +42,7 @@ targets += initrddump.o
endif
obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
-obj-$(CONFIG_BOOTEFI_BOOTMGR) += efi_bootmgr.o
+obj-$(CONFIG_EFI_BOOTMGR) += efi_bootmgr.o
obj-y += efi_boottime.o
obj-y += efi_helper.o
obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 47c552a94165..7eafdc79de3b 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -374,7 +374,7 @@ static int bootflow_system(struct unit_test_state *uts)
{
struct udevice *bootstd, *dev;
- if (!IS_ENABLED(CONFIG_BOOTEFI_BOOTMGR))
+ if (!IS_ENABLED(CONFIG_EFI_BOOTMGR))
return -EAGAIN;
ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd));
ut_assertok(device_bind(bootstd, DM_DRIVER_GET(bootmeth_efi_mgr),
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [RFC 10/13] net: tftp: remove explicit efi configuration dependency
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
` (8 preceding siblings ...)
2023-10-26 5:30 ` [RFC 09/13] efi_loader: rename BOOTEFI_BOOTMGR to EFI_BOOTMGR AKASHI Takahiro
@ 2023-10-26 5:30 ` AKASHI Takahiro
2023-10-26 5:30 ` [RFC 11/13] fs: " AKASHI Takahiro
` (3 subsequent siblings)
13 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
Now it is clear that the feature actually depends on efi interfaces,
not "bootefi" command. efi_set_bootdev() will automatically be nullified
if necessary efi component is disabled.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
net/tftp.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/net/tftp.c b/net/tftp.c
index 88e71e67de35..2e335413492b 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -302,12 +302,10 @@ static void tftp_complete(void)
time_start * 1000, "/s");
}
puts("\ndone\n");
- if (IS_ENABLED(CONFIG_CMD_BOOTEFI)) {
- if (!tftp_put_active)
- efi_set_bootdev("Net", "", tftp_filename,
- map_sysmem(tftp_load_addr, 0),
- net_boot_file_size);
- }
+ if (!tftp_put_active)
+ efi_set_bootdev("Net", "", tftp_filename,
+ map_sysmem(tftp_load_addr, 0),
+ net_boot_file_size);
net_set_state(NETLOOP_SUCCESS);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [RFC 11/13] fs: remove explicit efi configuration dependency
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
` (9 preceding siblings ...)
2023-10-26 5:30 ` [RFC 10/13] net: tftp: remove explicit efi configuration dependency AKASHI Takahiro
@ 2023-10-26 5:30 ` AKASHI Takahiro
2023-10-26 7:58 ` Heinrich Schuchardt
2023-10-26 5:30 ` [RFC 12/13] lib: uuid: move CONFIG_RANDOM_UUID AKASHI Takahiro
` (2 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
Now it is clear that the feature actually depends on efi interfaces,
not "bootefi" command. efi_set_bootdev() will automatically be nullified
if necessary efi component is disabled.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
fs/fs.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/fs.c b/fs/fs.c
index 4cb4310c9cc2..70cdb594c4c8 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -791,10 +791,9 @@ int do_load(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[],
return 1;
}
- if (IS_ENABLED(CONFIG_CMD_BOOTEFI))
- efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
- (argc > 4) ? argv[4] : "", map_sysmem(addr, 0),
- len_read);
+ efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
+ (argc > 4) ? argv[4] : "", map_sysmem(addr, 0),
+ len_read);
printf("%llu bytes read in %lu ms", len_read, time);
if (time > 0) {
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [RFC 12/13] lib: uuid: move CONFIG_RANDOM_UUID
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
` (10 preceding siblings ...)
2023-10-26 5:30 ` [RFC 11/13] fs: " AKASHI Takahiro
@ 2023-10-26 5:30 ` AKASHI Takahiro
2023-10-26 12:47 ` Tom Rini
2023-10-27 12:24 ` Ilias Apalodimas
2023-10-26 5:30 ` [RFC 13/13] block: rkmtd: select CONFIG_RANDOM_UUID explicitly AKASHI Takahiro
2023-10-26 12:47 ` [RFC 00/13] cmd: bootefi: refactor the code for bootmgr Tom Rini
13 siblings, 2 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
This option is independent from any commands and should be managed
under lib. For instance, drivers/block/rkmtd.c is a user.
It would be better to remove this configuration.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
cmd/Kconfig | 7 -------
lib/Kconfig | 7 +++++++
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 3a8483f7d042..c42745d06b39 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1143,13 +1143,6 @@ config CMD_GPT
Enable the 'gpt' command to ready and write GPT style partition
tables.
-config RANDOM_UUID
- bool "GPT Random UUID generation"
- select LIB_UUID
- help
- Enable the generation of partitions with random UUIDs if none
- are provided.
-
config CMD_GPT_RENAME
bool "GPT partition renaming commands"
depends on CMD_GPT
diff --git a/lib/Kconfig b/lib/Kconfig
index f6ca559897e7..b6d580c30615 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -74,6 +74,13 @@ config HAVE_PRIVATE_LIBGCC
config LIB_UUID
bool
+config RANDOM_UUID
+ bool "GPT Random UUID generation"
+ select LIB_UUID
+ help
+ Enable the generation of partitions with random UUIDs if none
+ are provided.
+
config SPL_LIB_UUID
depends on SPL
bool
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [RFC 13/13] block: rkmtd: select CONFIG_RANDOM_UUID explicitly
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
` (11 preceding siblings ...)
2023-10-26 5:30 ` [RFC 12/13] lib: uuid: move CONFIG_RANDOM_UUID AKASHI Takahiro
@ 2023-10-26 5:30 ` AKASHI Takahiro
2023-10-26 12:47 ` Tom Rini
2023-10-26 12:47 ` [RFC 00/13] cmd: bootefi: refactor the code for bootmgr Tom Rini
13 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 5:30 UTC (permalink / raw)
To: trini, sjg, xypron.glpk, ilias.apalodimas
Cc: jbx6244, u-boot, AKASHI Takahiro
This option is necessary to compile any way.
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
drivers/block/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 048a6caef00f..5cda21551043 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -265,6 +265,7 @@ config SYS_64BIT_LBA
config RKMTD
bool "Rockchip rkmtd virtual block device"
+ imply RANDOM_UUID
help
Enable "rkmtd" class and driver to create a virtual block device
to transfer Rockchip boot block data to and from NAND with block
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [RFC 11/13] fs: remove explicit efi configuration dependency
2023-10-26 5:30 ` [RFC 11/13] fs: " AKASHI Takahiro
@ 2023-10-26 7:58 ` Heinrich Schuchardt
2023-10-26 8:48 ` AKASHI Takahiro
0 siblings, 1 reply; 31+ messages in thread
From: Heinrich Schuchardt @ 2023-10-26 7:58 UTC (permalink / raw)
To: AKASHI Takahiro, trini, sjg, ilias.apalodimas; +Cc: jbx6244, u-boot
Am 26. Oktober 2023 07:30:50 MESZ schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>:
>Now it is clear that the feature actually depends on efi interfaces,
>not "bootefi" command. efi_set_bootdev() will automatically be nullified
>if necessary efi component is disabled.
>
>Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>---
> fs/fs.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
>diff --git a/fs/fs.c b/fs/fs.c
>index 4cb4310c9cc2..70cdb594c4c8 100644
>--- a/fs/fs.c
>+++ b/fs/fs.c
>@@ -791,10 +791,9 @@ int do_load(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[],
> return 1;
> }
>
>- if (IS_ENABLED(CONFIG_CMD_BOOTEFI))
>- efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
>- (argc > 4) ? argv[4] : "", map_sysmem(addr, 0),
>- len_read);
>+ efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
This function should not exist for CONFIG_EFI_LOADER=n. There are other places where the function is invoked. Please, review all of them.
Best regards
Heinrich
>+ (argc > 4) ? argv[4] : "", map_sysmem(addr, 0),
>+ len_read);
>
> printf("%llu bytes read in %lu ms", len_read, time);
> if (time > 0) {
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 11/13] fs: remove explicit efi configuration dependency
2023-10-26 7:58 ` Heinrich Schuchardt
@ 2023-10-26 8:48 ` AKASHI Takahiro
2023-10-26 12:47 ` Tom Rini
0 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-26 8:48 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: trini, sjg, ilias.apalodimas, jbx6244, u-boot
On Thu, Oct 26, 2023 at 09:58:53AM +0200, Heinrich Schuchardt wrote:
>
>
> Am 26. Oktober 2023 07:30:50 MESZ schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>:
> >Now it is clear that the feature actually depends on efi interfaces,
> >not "bootefi" command. efi_set_bootdev() will automatically be nullified
> >if necessary efi component is disabled.
> >
> >Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >---
> > fs/fs.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> >diff --git a/fs/fs.c b/fs/fs.c
> >index 4cb4310c9cc2..70cdb594c4c8 100644
> >--- a/fs/fs.c
> >+++ b/fs/fs.c
> >@@ -791,10 +791,9 @@ int do_load(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[],
> > return 1;
> > }
> >
> >- if (IS_ENABLED(CONFIG_CMD_BOOTEFI))
> >- efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
> >- (argc > 4) ? argv[4] : "", map_sysmem(addr, 0),
> >- len_read);
> >+ efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
>
> This function should not exist for CONFIG_EFI_LOADER=n. There are other places where the function is invoked. Please, review all of them.
Please go through the whole patch set, especially patch#8
"efi_loader: split unrelated code from efi_bootmgr.c".
efi_[set|clear]_bootdev() will be nullified if not necessary.
-Takahiro Akashi
> Best regards
>
> Heinrich
>
>
> >+ (argc > 4) ? argv[4] : "", map_sysmem(addr, 0),
> >+ len_read);
> >
> > printf("%llu bytes read in %lu ms", len_read, time);
> > if (time > 0) {
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 02/13] cmd: bootefi: re-organize do_bootefi_image()
2023-10-26 5:30 ` [RFC 02/13] cmd: bootefi: re-organize do_bootefi_image() AKASHI Takahiro
@ 2023-10-26 10:44 ` Heinrich Schuchardt
2023-10-26 12:47 ` Tom Rini
2023-10-27 0:35 ` AKASHI Takahiro
0 siblings, 2 replies; 31+ messages in thread
From: Heinrich Schuchardt @ 2023-10-26 10:44 UTC (permalink / raw)
To: AKASHI Takahiro; +Cc: jbx6244, u-boot, trini, sjg, ilias.apalodimas
On 10/26/23 07:30, AKASHI Takahiro wrote:
> Decompose and re-organize do_bootefi_image() into three parts for
> the succeeding refactor work.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> cmd/Kconfig | 15 ++++++--
> cmd/bootefi.c | 82 ++++++++++++++++++++++++++++++--------------
> include/efi_loader.h | 2 --
> 3 files changed, 69 insertions(+), 30 deletions(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 0eb739203ade..825a41d68aad 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -363,9 +363,19 @@ config CMD_BOOTEFI
> help
> Boot an EFI image from memory.
>
> +if CMD_BOOTEFI
> +config CMD_BOOTEFI_BINARY
> + bool "Allow booting an EFI binary directly"
> + depends on BOOTEFI_BOOTMGR
> + default y
> + help
> + Select this option to enable direct execution of binary at 'bootefi'.
> + This subcommand will allow you to load the UEFI binary using
> + other U-Boot commands or external methods and then run isince 2021. t.
> +
> config CMD_BOOTEFI_BOOTMGR
This symbol is in lib/efi_loader/Kconfig:
lib/efi_loader/Kconfig:35:config CMD_BOOTEFI_BOOTMGR
Please, rebase your series on origin/master.
Best regards
Heinrich
> bool "UEFI Boot Manager command"
> - depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
> + depends on BOOTEFI_BOOTMGR
> default y
> help
> Select this option to enable the 'bootmgr' subcommand of 'bootefi'.
> @@ -374,7 +384,7 @@ config CMD_BOOTEFI_BOOTMGR
>
> config CMD_BOOTEFI_HELLO_COMPILE
> bool "Compile a standard EFI hello world binary for testing"
> - depends on CMD_BOOTEFI && !CPU_V7M
> + depends on !CPU_V7M
> default y
> help
> This compiles a standard EFI hello world application with U-Boot so
> @@ -396,6 +406,7 @@ config CMD_BOOTEFI_HELLO
> up EFI support on a new architecture.
>
> source lib/efi_selftest/Kconfig
> +endif
>
> config CMD_BOOTMENU
> bool "bootmenu"
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 1b28bf5a318d..ae00bba3b4f0 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -491,7 +491,6 @@ out:
> return (ret != EFI_SUCCESS) ? ret : ret2;
> }
>
> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> static efi_status_t bootefi_run_prepare(const char *load_options_path,
> struct efi_device_path *device_path,
> struct efi_device_path *image_path,
> @@ -581,7 +580,6 @@ static int do_efi_selftest(void)
>
> return ret != EFI_SUCCESS;
> }
> -#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
>
> /**
> * do_bootefi() - execute `bootefi` command
> @@ -603,14 +601,6 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> if (argc < 2)
> return CMD_RET_USAGE;
>
> - /* Initialize EFI drivers */
> - ret = efi_init_obj_list();
> - if (ret != EFI_SUCCESS) {
> - log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> - ret & ~EFI_ERROR_MASK);
> - return CMD_RET_FAILURE;
> - }
> -
> if (argc > 2) {
> uintptr_t fdt_addr;
>
> @@ -619,29 +609,54 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> } else {
> fdt = EFI_FDT_USE_INTERNAL;
> }
> - ret = efi_install_fdt(fdt);
> - if (ret == EFI_INVALID_PARAMETER)
> - return CMD_RET_USAGE;
> - else if (ret != EFI_SUCCESS)
> - return CMD_RET_FAILURE;
>
> - if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> - if (!strcmp(argv[1], "bootmgr"))
> - return do_efibootmgr();
> + if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
> + !strcmp(argv[1], "bootmgr")) {
> + /* Initialize EFI drivers */
> + ret = efi_init_obj_list();
> + if (ret != EFI_SUCCESS) {
> + log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> + ret & ~EFI_ERROR_MASK);
> + return CMD_RET_FAILURE;
> + }
> +
> + ret = efi_install_fdt(fdt);
> + if (ret == EFI_INVALID_PARAMETER)
> + return CMD_RET_USAGE;
> + else if (ret != EFI_SUCCESS)
> + return CMD_RET_FAILURE;
> +
> + return do_efibootmgr();
> }
> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> - if (!strcmp(argv[1], "selftest"))
> +
> + if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
> + !strcmp(argv[1], "selftest")) {
> + /* Initialize EFI drivers */
> + ret = efi_init_obj_list();
> + if (ret != EFI_SUCCESS) {
> + log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> + ret & ~EFI_ERROR_MASK);
> + return CMD_RET_FAILURE;
> + }
> +
> + ret = efi_install_fdt(fdt);
> + if (ret == EFI_INVALID_PARAMETER)
> + return CMD_RET_USAGE;
> + else if (ret != EFI_SUCCESS)
> + return CMD_RET_FAILURE;
> +
> return do_efi_selftest();
> -#endif
> + }
>
> -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> - if (!strcmp(argv[1], "hello")) {
> + if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BINARY))
> + return CMD_RET_SUCCESS;
> +
> + if (IS_ENABLED(CONFIG_CMD_BOOTEFI_HELLO) &&
> + !strcmp(argv[1], "hello")) {
> image_buf = __efi_helloworld_begin;
> size = __efi_helloworld_end - __efi_helloworld_begin;
> efi_clear_bootdev();
> - } else
> -#endif
> - {
> + } else {
> addr = strtoul(argv[1], NULL, 16);
> /* Check that a numeric value was passed */
> if (!addr)
> @@ -663,6 +678,21 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> size = image_size;
> }
> }
> +
> + /* Initialize EFI drivers */
> + ret = efi_init_obj_list();
> + if (ret != EFI_SUCCESS) {
> + log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> + ret & ~EFI_ERROR_MASK);
> + return CMD_RET_FAILURE;
> + }
> +
> + ret = efi_install_fdt(fdt);
> + if (ret == EFI_INVALID_PARAMETER)
> + return CMD_RET_USAGE;
> + else if (ret != EFI_SUCCESS)
> + return CMD_RET_FAILURE;
> +
> ret = efi_run_image(image_buf, size);
>
> if (ret != EFI_SUCCESS)
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index e24410505f40..48d4999e56a9 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -878,14 +878,12 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
>
> efi_status_t __efi_runtime EFIAPI efi_set_time(struct efi_time *time);
>
> -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> /*
> * Entry point for the tests of the EFI API.
> * It is called by 'bootefi selftest'
> */
> efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
> struct efi_system_table *systab);
> -#endif
>
> efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> const efi_guid_t *vendor, u32 *attributes,
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 01/13] cmd: bootefi: unfold do_bootefi_image()
2023-10-26 5:30 ` [RFC 01/13] cmd: bootefi: unfold do_bootefi_image() AKASHI Takahiro
@ 2023-10-26 11:01 ` Heinrich Schuchardt
2023-10-27 0:25 ` AKASHI Takahiro
0 siblings, 1 reply; 31+ messages in thread
From: Heinrich Schuchardt @ 2023-10-26 11:01 UTC (permalink / raw)
To: AKASHI Takahiro; +Cc: jbx6244, u-boot, trini, sjg, ilias.apalodimas
On 10/26/23 07:30, AKASHI Takahiro wrote:
> Unfold do_bootefi_image() into do_bootefi() for the sake of the succeeding
> refactor work.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> cmd/bootefi.c | 101 ++++++++++++++++++--------------------------------
> 1 file changed, 37 insertions(+), 64 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 20e5c94a33a4..1b28bf5a318d 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -425,58 +425,6 @@ static int do_efibootmgr(void)
> return CMD_RET_SUCCESS;
> }
>
> -/**
> - * do_bootefi_image() - execute EFI binary
> - *
> - * Set up memory image for the binary to be loaded, prepare device path, and
> - * then call do_bootefi_exec() to execute it.
> - *
> - * @image_opt: string with image start address
> - * @size_opt: string with image size or NULL
> - * Return: status code
> - */
> -static int do_bootefi_image(const char *image_opt, const char *size_opt)
> -{
> - void *image_buf;
> - unsigned long addr, size;
> - efi_status_t ret;
> -
> -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> - if (!strcmp(image_opt, "hello")) {
> - image_buf = __efi_helloworld_begin;
> - size = __efi_helloworld_end - __efi_helloworld_begin;
> - efi_clear_bootdev();
> - } else
> -#endif
> - {
> - addr = strtoul(image_opt, NULL, 16);
> - /* Check that a numeric value was passed */
> - if (!addr)
> - return CMD_RET_USAGE;
> - image_buf = map_sysmem(addr, 0);
> -
> - if (size_opt) {
> - size = strtoul(size_opt, NULL, 16);
> - if (!size)
> - return CMD_RET_USAGE;
> - efi_clear_bootdev();
> - } else {
> - if (image_buf != image_addr) {
> - log_err("No UEFI binary known at %s\n",
> - image_opt);
> - return CMD_RET_FAILURE;
> - }
> - size = image_size;
> - }
> - }
> - ret = efi_run_image(image_buf, size);
> -
> - if (ret != EFI_SUCCESS)
> - return CMD_RET_FAILURE;
> -
> - return CMD_RET_SUCCESS;
> -}
> -
> /**
> * efi_run_image() - run loaded UEFI image
> *
> @@ -648,8 +596,9 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> char *const argv[])
> {
> efi_status_t ret;
> - char *img_addr, *img_size, *str_copy, *pos;
> - void *fdt;
> + char *p;
> + void *fdt, *image_buf;
> + unsigned long addr, size;
>
> if (argc < 2)
> return CMD_RET_USAGE;
> @@ -684,18 +633,42 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> if (!strcmp(argv[1], "selftest"))
> return do_efi_selftest();
> #endif
> - str_copy = strdup(argv[1]);
> - if (!str_copy) {
> - log_err("Out of memory\n");
> - return CMD_RET_FAILURE;
> +
> +#ifdef CONFIG_CMD_BOOTEFI_HELLO
I would prefer a normal if and let the linker sort it out.
Otherwise looks good to me.
Best regards
Heinrich
> + if (!strcmp(argv[1], "hello")) {
> + image_buf = __efi_helloworld_begin;
> + size = __efi_helloworld_end - __efi_helloworld_begin;
> + efi_clear_bootdev();
> + } else
> +#endif
> + {
> + addr = strtoul(argv[1], NULL, 16);
> + /* Check that a numeric value was passed */
> + if (!addr)
> + return CMD_RET_USAGE;
> + image_buf = map_sysmem(addr, 0);
> +
> + p = strchr(argv[1], ':');
> + if (p) {
> + size = strtoul(++p, NULL, 16);
> + if (!size)
> + return CMD_RET_USAGE;
> + efi_clear_bootdev();
> + } else {
> + if (image_buf != image_addr) {
> + log_err("No UEFI binary known at %s\n",
> + argv[1]);
> + return CMD_RET_FAILURE;
> + }
> + size = image_size;
> + }
> }
> - pos = str_copy;
> - img_addr = strsep(&pos, ":");
> - img_size = strsep(&pos, ":");
> - ret = do_bootefi_image(img_addr, img_size);
> - free(str_copy);
> + ret = efi_run_image(image_buf, size);
>
> - return ret;
> + if (ret != EFI_SUCCESS)
> + return CMD_RET_FAILURE;
> +
> + return CMD_RET_SUCCESS;
> }
>
> U_BOOT_LONGHELP(bootefi,
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 12/13] lib: uuid: move CONFIG_RANDOM_UUID
2023-10-26 5:30 ` [RFC 12/13] lib: uuid: move CONFIG_RANDOM_UUID AKASHI Takahiro
@ 2023-10-26 12:47 ` Tom Rini
2023-10-27 12:24 ` Ilias Apalodimas
1 sibling, 0 replies; 31+ messages in thread
From: Tom Rini @ 2023-10-26 12:47 UTC (permalink / raw)
To: AKASHI Takahiro; +Cc: sjg, xypron.glpk, ilias.apalodimas, jbx6244, u-boot
[-- Attachment #1: Type: text/plain, Size: 380 bytes --]
On Thu, Oct 26, 2023 at 02:30:51PM +0900, AKASHI Takahiro wrote:
> This option is independent from any commands and should be managed
> under lib. For instance, drivers/block/rkmtd.c is a user.
>
> It would be better to remove this configuration.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Tom Rini <trini@konsulko.com>
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 13/13] block: rkmtd: select CONFIG_RANDOM_UUID explicitly
2023-10-26 5:30 ` [RFC 13/13] block: rkmtd: select CONFIG_RANDOM_UUID explicitly AKASHI Takahiro
@ 2023-10-26 12:47 ` Tom Rini
0 siblings, 0 replies; 31+ messages in thread
From: Tom Rini @ 2023-10-26 12:47 UTC (permalink / raw)
To: AKASHI Takahiro; +Cc: sjg, xypron.glpk, ilias.apalodimas, jbx6244, u-boot
[-- Attachment #1: Type: text/plain, Size: 811 bytes --]
On Thu, Oct 26, 2023 at 02:30:52PM +0900, AKASHI Takahiro wrote:
> This option is necessary to compile any way.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> drivers/block/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> index 048a6caef00f..5cda21551043 100644
> --- a/drivers/block/Kconfig
> +++ b/drivers/block/Kconfig
> @@ -265,6 +265,7 @@ config SYS_64BIT_LBA
>
> config RKMTD
> bool "Rockchip rkmtd virtual block device"
> + imply RANDOM_UUID
> help
> Enable "rkmtd" class and driver to create a virtual block device
> to transfer Rockchip boot block data to and from NAND with block
Since it's necessary to compile it should be select not imply here,
thanks.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 00/13] cmd: bootefi: refactor the code for bootmgr
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
` (12 preceding siblings ...)
2023-10-26 5:30 ` [RFC 13/13] block: rkmtd: select CONFIG_RANDOM_UUID explicitly AKASHI Takahiro
@ 2023-10-26 12:47 ` Tom Rini
13 siblings, 0 replies; 31+ messages in thread
From: Tom Rini @ 2023-10-26 12:47 UTC (permalink / raw)
To: AKASHI Takahiro; +Cc: sjg, xypron.glpk, ilias.apalodimas, jbx6244, u-boot
[-- Attachment #1: Type: text/plain, Size: 1748 bytes --]
On Thu, Oct 26, 2023 at 02:30:39PM +0900, AKASHI Takahiro wrote:
> This patch set is motivated by the discussion[1] regarding
> CONFIG_BOOTEFI_BOOTMGR option.
>
> At the end, bootefi.c will be decomposed into two parts, one for
> providing the command itself and one for implementing helper functions.
> EFI_LOADER will now be available without CONFIG_CMDLINE or specifically
> CONFIG_CMD_BOOTEFI if invoked via bootmeth/bootstd.
>
> Then, EFI_LOADER library side will be further split into two options
> for fine-grain control:
> CONFIG_EFI_BINARY_EXEC: execute UEFI binaries which are to be explicitly
> loaded by U-Boot's load commands/functions or other methods
> (like a jtag debugger?)
> It supports bootmeth_efi as well as "bootefi <addr>|hello"(/"bootm"?).
>
> CONFIG_EFI_BOOTMGR: provide EFI boot manger functionality
> It supports bootmeth_efi_mgr as well as "bootefi bootmgr".
>
> As such, We will no longer need CONFIG_BINARY_EXEC if we want to only
> make use of the boot manger for booting a next stage OS.
>
> Prerequisite
> ============
> This patch set is based on top of Simon/Tom's [2].
>
> Patches
> =======
> Patch#1-#11: I hope that those commits show step-by-step refactoring
> without introducing degradation.
> Patch#12-#13: Those are not directly related to the patch's aim, but
> they are necessary to compile U-Boot on sandbox (sandbox_defconfig)
> without CONFIG_CMDLINE.
I'm very happy to see this. In general, I need to make a v5 of my series
as that will also include Heinrich's patch (v3 of it) to have sandbox
enable --gc-sections and that in turn means that some of the work
around handling unused functions should be easier.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 11/13] fs: remove explicit efi configuration dependency
2023-10-26 8:48 ` AKASHI Takahiro
@ 2023-10-26 12:47 ` Tom Rini
2023-10-27 0:59 ` AKASHI Takahiro
0 siblings, 1 reply; 31+ messages in thread
From: Tom Rini @ 2023-10-26 12:47 UTC (permalink / raw)
To: AKASHI Takahiro, Heinrich Schuchardt, sjg, ilias.apalodimas,
jbx6244, u-boot
[-- Attachment #1: Type: text/plain, Size: 1917 bytes --]
On Thu, Oct 26, 2023 at 05:48:30PM +0900, AKASHI Takahiro wrote:
> On Thu, Oct 26, 2023 at 09:58:53AM +0200, Heinrich Schuchardt wrote:
> >
> >
> > Am 26. Oktober 2023 07:30:50 MESZ schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>:
> > >Now it is clear that the feature actually depends on efi interfaces,
> > >not "bootefi" command. efi_set_bootdev() will automatically be nullified
> > >if necessary efi component is disabled.
> > >
> > >Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > >---
> > > fs/fs.c | 7 +++----
> > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > >diff --git a/fs/fs.c b/fs/fs.c
> > >index 4cb4310c9cc2..70cdb594c4c8 100644
> > >--- a/fs/fs.c
> > >+++ b/fs/fs.c
> > >@@ -791,10 +791,9 @@ int do_load(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[],
> > > return 1;
> > > }
> > >
> > >- if (IS_ENABLED(CONFIG_CMD_BOOTEFI))
> > >- efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
> > >- (argc > 4) ? argv[4] : "", map_sysmem(addr, 0),
> > >- len_read);
> > >+ efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
> >
> > This function should not exist for CONFIG_EFI_LOADER=n. There are other places where the function is invoked. Please, review all of them.
>
> Please go through the whole patch set, especially patch#8
> "efi_loader: split unrelated code from efi_bootmgr.c".
>
> efi_[set|clear]_bootdev() will be nullified if not necessary.
In this case I think what we have here today is more readable / clearer.
We don't need empty functions as we can either do like this section of
the code does today or the linker will discard things correctly as it's
a case of funcB() calls funcA() and nothing calls funcB() so it will be
discarded. I don't know without digging through the series more what the
correct IS_ENABLED() guard should be here (and then also in patch #10).
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 02/13] cmd: bootefi: re-organize do_bootefi_image()
2023-10-26 10:44 ` Heinrich Schuchardt
@ 2023-10-26 12:47 ` Tom Rini
2023-10-27 0:35 ` AKASHI Takahiro
1 sibling, 0 replies; 31+ messages in thread
From: Tom Rini @ 2023-10-26 12:47 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: AKASHI Takahiro, jbx6244, u-boot, sjg, ilias.apalodimas
[-- Attachment #1: Type: text/plain, Size: 1555 bytes --]
On Thu, Oct 26, 2023 at 12:44:00PM +0200, Heinrich Schuchardt wrote:
> On 10/26/23 07:30, AKASHI Takahiro wrote:
> > Decompose and re-organize do_bootefi_image() into three parts for
> > the succeeding refactor work.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > cmd/Kconfig | 15 ++++++--
> > cmd/bootefi.c | 82 ++++++++++++++++++++++++++++++--------------
> > include/efi_loader.h | 2 --
> > 3 files changed, 69 insertions(+), 30 deletions(-)
> >
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index 0eb739203ade..825a41d68aad 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -363,9 +363,19 @@ config CMD_BOOTEFI
> > help
> > Boot an EFI image from memory.
> >
> > +if CMD_BOOTEFI
> > +config CMD_BOOTEFI_BINARY
> > + bool "Allow booting an EFI binary directly"
> > + depends on BOOTEFI_BOOTMGR
> > + default y
> > + help
> > + Select this option to enable direct execution of binary at 'bootefi'.
> > + This subcommand will allow you to load the UEFI binary using
> > + other U-Boot commands or external methods and then run isince 2021. t.
> > +
> > config CMD_BOOTEFI_BOOTMGR
>
> This symbol is in lib/efi_loader/Kconfig:
> lib/efi_loader/Kconfig:35:config CMD_BOOTEFI_BOOTMGR
>
> Please, rebase your series on origin/master.
To properly test this series, it needs concepts in the series to disable
CMDLINE. I'll get a v5 going soon, and I am aiming to have that be in
-next once that opens (after reviews).
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 01/13] cmd: bootefi: unfold do_bootefi_image()
2023-10-26 11:01 ` Heinrich Schuchardt
@ 2023-10-27 0:25 ` AKASHI Takahiro
2023-10-27 1:00 ` Tom Rini
0 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-27 0:25 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: jbx6244, u-boot, trini, sjg, ilias.apalodimas
On Thu, Oct 26, 2023 at 01:01:52PM +0200, Heinrich Schuchardt wrote:
> On 10/26/23 07:30, AKASHI Takahiro wrote:
> > Unfold do_bootefi_image() into do_bootefi() for the sake of the succeeding
> > refactor work.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > cmd/bootefi.c | 101 ++++++++++++++++++--------------------------------
> > 1 file changed, 37 insertions(+), 64 deletions(-)
> >
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index 20e5c94a33a4..1b28bf5a318d 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -425,58 +425,6 @@ static int do_efibootmgr(void)
> > return CMD_RET_SUCCESS;
> > }
> >
> > -/**
> > - * do_bootefi_image() - execute EFI binary
> > - *
> > - * Set up memory image for the binary to be loaded, prepare device path, and
> > - * then call do_bootefi_exec() to execute it.
> > - *
> > - * @image_opt: string with image start address
> > - * @size_opt: string with image size or NULL
> > - * Return: status code
> > - */
> > -static int do_bootefi_image(const char *image_opt, const char *size_opt)
> > -{
> > - void *image_buf;
> > - unsigned long addr, size;
> > - efi_status_t ret;
> > -
> > -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> > - if (!strcmp(image_opt, "hello")) {
> > - image_buf = __efi_helloworld_begin;
> > - size = __efi_helloworld_end - __efi_helloworld_begin;
> > - efi_clear_bootdev();
> > - } else
> > -#endif
> > - {
> > - addr = strtoul(image_opt, NULL, 16);
> > - /* Check that a numeric value was passed */
> > - if (!addr)
> > - return CMD_RET_USAGE;
> > - image_buf = map_sysmem(addr, 0);
> > -
> > - if (size_opt) {
> > - size = strtoul(size_opt, NULL, 16);
> > - if (!size)
> > - return CMD_RET_USAGE;
> > - efi_clear_bootdev();
> > - } else {
> > - if (image_buf != image_addr) {
> > - log_err("No UEFI binary known at %s\n",
> > - image_opt);
> > - return CMD_RET_FAILURE;
> > - }
> > - size = image_size;
> > - }
> > - }
> > - ret = efi_run_image(image_buf, size);
> > -
> > - if (ret != EFI_SUCCESS)
> > - return CMD_RET_FAILURE;
> > -
> > - return CMD_RET_SUCCESS;
> > -}
> > -
> > /**
> > * efi_run_image() - run loaded UEFI image
> > *
> > @@ -648,8 +596,9 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > char *const argv[])
> > {
> > efi_status_t ret;
> > - char *img_addr, *img_size, *str_copy, *pos;
> > - void *fdt;
> > + char *p;
> > + void *fdt, *image_buf;
> > + unsigned long addr, size;
> >
> > if (argc < 2)
> > return CMD_RET_USAGE;
> > @@ -684,18 +633,42 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > if (!strcmp(argv[1], "selftest"))
> > return do_efi_selftest();
> > #endif
> > - str_copy = strdup(argv[1]);
> > - if (!str_copy) {
> > - log_err("Out of memory\n");
> > - return CMD_RET_FAILURE;
> > +
> > +#ifdef CONFIG_CMD_BOOTEFI_HELLO
>
> I would prefer a normal if and let the linker sort it out.
Here I focused on simply unfolding("moving") the function, keeping the code
as same as possible. Any how the code is in a tentative form
at this point and will be reshaped in later patches, using IS_ENABLED().
Please look at the final code after applying the whole series.
-Takahiro Akashi
> Otherwise looks good to me.
>
> Best regards
>
> Heinrich
>
> > + if (!strcmp(argv[1], "hello")) {
> > + image_buf = __efi_helloworld_begin;
> > + size = __efi_helloworld_end - __efi_helloworld_begin;
> > + efi_clear_bootdev();
> > + } else
> > +#endif
> > + {
> > + addr = strtoul(argv[1], NULL, 16);
> > + /* Check that a numeric value was passed */
> > + if (!addr)
> > + return CMD_RET_USAGE;
> > + image_buf = map_sysmem(addr, 0);
> > +
> > + p = strchr(argv[1], ':');
> > + if (p) {
> > + size = strtoul(++p, NULL, 16);
> > + if (!size)
> > + return CMD_RET_USAGE;
> > + efi_clear_bootdev();
> > + } else {
> > + if (image_buf != image_addr) {
> > + log_err("No UEFI binary known at %s\n",
> > + argv[1]);
> > + return CMD_RET_FAILURE;
> > + }
> > + size = image_size;
> > + }
> > }
> > - pos = str_copy;
> > - img_addr = strsep(&pos, ":");
> > - img_size = strsep(&pos, ":");
> > - ret = do_bootefi_image(img_addr, img_size);
> > - free(str_copy);
> > + ret = efi_run_image(image_buf, size);
> >
> > - return ret;
> > + if (ret != EFI_SUCCESS)
> > + return CMD_RET_FAILURE;
> > +
> > + return CMD_RET_SUCCESS;
> > }
> >
> > U_BOOT_LONGHELP(bootefi,
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 02/13] cmd: bootefi: re-organize do_bootefi_image()
2023-10-26 10:44 ` Heinrich Schuchardt
2023-10-26 12:47 ` Tom Rini
@ 2023-10-27 0:35 ` AKASHI Takahiro
1 sibling, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-27 0:35 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: jbx6244, u-boot, trini, sjg, ilias.apalodimas
On Thu, Oct 26, 2023 at 12:44:00PM +0200, Heinrich Schuchardt wrote:
> On 10/26/23 07:30, AKASHI Takahiro wrote:
> > Decompose and re-organize do_bootefi_image() into three parts for
> > the succeeding refactor work.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > cmd/Kconfig | 15 ++++++--
> > cmd/bootefi.c | 82 ++++++++++++++++++++++++++++++--------------
> > include/efi_loader.h | 2 --
> > 3 files changed, 69 insertions(+), 30 deletions(-)
> >
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index 0eb739203ade..825a41d68aad 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -363,9 +363,19 @@ config CMD_BOOTEFI
> > help
> > Boot an EFI image from memory.
> >
> > +if CMD_BOOTEFI
> > +config CMD_BOOTEFI_BINARY
> > + bool "Allow booting an EFI binary directly"
> > + depends on BOOTEFI_BOOTMGR
> > + default y
> > + help
> > + Select this option to enable direct execution of binary at 'bootefi'.
> > + This subcommand will allow you to load the UEFI binary using
> > + other U-Boot commands or external methods and then run isince 2021. t.
> > +
> > config CMD_BOOTEFI_BOOTMGR
>
> This symbol is in lib/efi_loader/Kconfig:
> lib/efi_loader/Kconfig:35:config CMD_BOOTEFI_BOOTMGR
In the cover letter, I mentioned that the RFC was based on Tom's branch.
>
> Please, rebase your series on origin/master.
If you agree to my idea in this whole series, I will re-post a new version,
rebasing it on Tom's "-next" branch with CONFIG_CMDLINE tweaks.
So please review other commits as well.
Thanks,
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> > bool "UEFI Boot Manager command"
> > - depends on BOOTEFI_BOOTMGR && CMD_BOOTEFI
> > + depends on BOOTEFI_BOOTMGR
> > default y
> > help
> > Select this option to enable the 'bootmgr' subcommand of 'bootefi'.
> > @@ -374,7 +384,7 @@ config CMD_BOOTEFI_BOOTMGR
> >
> > config CMD_BOOTEFI_HELLO_COMPILE
> > bool "Compile a standard EFI hello world binary for testing"
> > - depends on CMD_BOOTEFI && !CPU_V7M
> > + depends on !CPU_V7M
> > default y
> > help
> > This compiles a standard EFI hello world application with U-Boot so
> > @@ -396,6 +406,7 @@ config CMD_BOOTEFI_HELLO
> > up EFI support on a new architecture.
> >
> > source lib/efi_selftest/Kconfig
> > +endif
> >
> > config CMD_BOOTMENU
> > bool "bootmenu"
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index 1b28bf5a318d..ae00bba3b4f0 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -491,7 +491,6 @@ out:
> > return (ret != EFI_SUCCESS) ? ret : ret2;
> > }
> >
> > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > static efi_status_t bootefi_run_prepare(const char *load_options_path,
> > struct efi_device_path *device_path,
> > struct efi_device_path *image_path,
> > @@ -581,7 +580,6 @@ static int do_efi_selftest(void)
> >
> > return ret != EFI_SUCCESS;
> > }
> > -#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
> >
> > /**
> > * do_bootefi() - execute `bootefi` command
> > @@ -603,14 +601,6 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > if (argc < 2)
> > return CMD_RET_USAGE;
> >
> > - /* Initialize EFI drivers */
> > - ret = efi_init_obj_list();
> > - if (ret != EFI_SUCCESS) {
> > - log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > - ret & ~EFI_ERROR_MASK);
> > - return CMD_RET_FAILURE;
> > - }
> > -
> > if (argc > 2) {
> > uintptr_t fdt_addr;
> >
> > @@ -619,29 +609,54 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > } else {
> > fdt = EFI_FDT_USE_INTERNAL;
> > }
> > - ret = efi_install_fdt(fdt);
> > - if (ret == EFI_INVALID_PARAMETER)
> > - return CMD_RET_USAGE;
> > - else if (ret != EFI_SUCCESS)
> > - return CMD_RET_FAILURE;
> >
> > - if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) {
> > - if (!strcmp(argv[1], "bootmgr"))
> > - return do_efibootmgr();
> > + if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) &&
> > + !strcmp(argv[1], "bootmgr")) {
> > + /* Initialize EFI drivers */
> > + ret = efi_init_obj_list();
> > + if (ret != EFI_SUCCESS) {
> > + log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > + ret & ~EFI_ERROR_MASK);
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + ret = efi_install_fdt(fdt);
> > + if (ret == EFI_INVALID_PARAMETER)
> > + return CMD_RET_USAGE;
> > + else if (ret != EFI_SUCCESS)
> > + return CMD_RET_FAILURE;
> > +
> > + return do_efibootmgr();
> > }
> > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > - if (!strcmp(argv[1], "selftest"))
> > +
> > + if (IS_ENABLED(CONFIG_CMD_BOOTEFI_SELFTEST) &&
> > + !strcmp(argv[1], "selftest")) {
> > + /* Initialize EFI drivers */
> > + ret = efi_init_obj_list();
> > + if (ret != EFI_SUCCESS) {
> > + log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > + ret & ~EFI_ERROR_MASK);
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + ret = efi_install_fdt(fdt);
> > + if (ret == EFI_INVALID_PARAMETER)
> > + return CMD_RET_USAGE;
> > + else if (ret != EFI_SUCCESS)
> > + return CMD_RET_FAILURE;
> > +
> > return do_efi_selftest();
> > -#endif
> > + }
> >
> > -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> > - if (!strcmp(argv[1], "hello")) {
> > + if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BINARY))
> > + return CMD_RET_SUCCESS;
> > +
> > + if (IS_ENABLED(CONFIG_CMD_BOOTEFI_HELLO) &&
> > + !strcmp(argv[1], "hello")) {
> > image_buf = __efi_helloworld_begin;
> > size = __efi_helloworld_end - __efi_helloworld_begin;
> > efi_clear_bootdev();
> > - } else
> > -#endif
> > - {
> > + } else {
> > addr = strtoul(argv[1], NULL, 16);
> > /* Check that a numeric value was passed */
> > if (!addr)
> > @@ -663,6 +678,21 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > size = image_size;
> > }
> > }
> > +
> > + /* Initialize EFI drivers */
> > + ret = efi_init_obj_list();
> > + if (ret != EFI_SUCCESS) {
> > + log_err("Error: Cannot initialize UEFI sub-system, r = %lu\n",
> > + ret & ~EFI_ERROR_MASK);
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + ret = efi_install_fdt(fdt);
> > + if (ret == EFI_INVALID_PARAMETER)
> > + return CMD_RET_USAGE;
> > + else if (ret != EFI_SUCCESS)
> > + return CMD_RET_FAILURE;
> > +
> > ret = efi_run_image(image_buf, size);
> >
> > if (ret != EFI_SUCCESS)
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index e24410505f40..48d4999e56a9 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -878,14 +878,12 @@ efi_status_t __efi_runtime EFIAPI efi_get_time(
> >
> > efi_status_t __efi_runtime EFIAPI efi_set_time(struct efi_time *time);
> >
> > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> > /*
> > * Entry point for the tests of the EFI API.
> > * It is called by 'bootefi selftest'
> > */
> > efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
> > struct efi_system_table *systab);
> > -#endif
> >
> > efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> > const efi_guid_t *vendor, u32 *attributes,
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 11/13] fs: remove explicit efi configuration dependency
2023-10-26 12:47 ` Tom Rini
@ 2023-10-27 0:59 ` AKASHI Takahiro
2023-10-27 1:04 ` Tom Rini
0 siblings, 1 reply; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-27 0:59 UTC (permalink / raw)
To: Tom Rini; +Cc: Heinrich Schuchardt, sjg, ilias.apalodimas, jbx6244, u-boot
[-- Attachment #1: Type: text/plain, Size: 2833 bytes --]
On Thu, Oct 26, 2023 at 08:47:52AM -0400, Tom Rini wrote:
> On Thu, Oct 26, 2023 at 05:48:30PM +0900, AKASHI Takahiro wrote:
> > On Thu, Oct 26, 2023 at 09:58:53AM +0200, Heinrich Schuchardt wrote:
> > >
> > >
> > > Am 26. Oktober 2023 07:30:50 MESZ schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>:
> > > >Now it is clear that the feature actually depends on efi interfaces,
> > > >not "bootefi" command. efi_set_bootdev() will automatically be nullified
> > > >if necessary efi component is disabled.
> > > >
> > > >Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > >---
> > > > fs/fs.c | 7 +++----
> > > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > > >
> > > >diff --git a/fs/fs.c b/fs/fs.c
> > > >index 4cb4310c9cc2..70cdb594c4c8 100644
> > > >--- a/fs/fs.c
> > > >+++ b/fs/fs.c
> > > >@@ -791,10 +791,9 @@ int do_load(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[],
> > > > return 1;
> > > > }
> > > >
> > > >- if (IS_ENABLED(CONFIG_CMD_BOOTEFI))
> > > >- efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
> > > >- (argc > 4) ? argv[4] : "", map_sysmem(addr, 0),
> > > >- len_read);
> > > >+ efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
> > >
> > > This function should not exist for CONFIG_EFI_LOADER=n. There are other places where the function is invoked. Please, review all of them.
> >
> > Please go through the whole patch set, especially patch#8
> > "efi_loader: split unrelated code from efi_bootmgr.c".
> >
> > efi_[set|clear]_bootdev() will be nullified if not necessary.
>
> In this case I think what we have here today is more readable / clearer.
> We don't need empty functions as we can either do like this section of
> the code does today or the linker will discard things correctly as it's
> a case of funcB() calls funcA() and nothing calls funcB() so it will be
> discarded. I don't know without digging through the series more what the
> correct IS_ENABLED() guard should be here (and then also in patch #10).
If I correctly understand your question, it is my point in this commit.
I believe that a caller should not be bothered by thinking of what efi CONFIG
be used for the guard. All that should be done is to just put a right hook
(efi_set_bootdev() in this case) at a right place as a caller (do_load()
in this case) is apparently irrelevant to UEFI subsystem.
Hereafter, whatever rework may be done on UEFI side (like my refactoring
here), other code won't need to be modified.
If you want to rely on an intelligent linker behavior, I would suggest
another approach, modifying efi_set_bootdev() as follows;
efi_set_bootdev(...)
{
if (!IS_ENABLED(EFI_BINARY_EXEC))
return;
...
}
I hope it would also work.
-Takahiro Akashi
>
> --
> Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 01/13] cmd: bootefi: unfold do_bootefi_image()
2023-10-27 0:25 ` AKASHI Takahiro
@ 2023-10-27 1:00 ` Tom Rini
2023-10-27 12:23 ` Ilias Apalodimas
0 siblings, 1 reply; 31+ messages in thread
From: Tom Rini @ 2023-10-27 1:00 UTC (permalink / raw)
To: AKASHI Takahiro, Heinrich Schuchardt, jbx6244, u-boot, sjg,
ilias.apalodimas
[-- Attachment #1: Type: text/plain, Size: 3658 bytes --]
On Fri, Oct 27, 2023 at 09:25:44AM +0900, AKASHI Takahiro wrote:
> On Thu, Oct 26, 2023 at 01:01:52PM +0200, Heinrich Schuchardt wrote:
> > On 10/26/23 07:30, AKASHI Takahiro wrote:
> > > Unfold do_bootefi_image() into do_bootefi() for the sake of the succeeding
> > > refactor work.
> > >
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > ---
> > > cmd/bootefi.c | 101 ++++++++++++++++++--------------------------------
> > > 1 file changed, 37 insertions(+), 64 deletions(-)
> > >
> > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > > index 20e5c94a33a4..1b28bf5a318d 100644
> > > --- a/cmd/bootefi.c
> > > +++ b/cmd/bootefi.c
> > > @@ -425,58 +425,6 @@ static int do_efibootmgr(void)
> > > return CMD_RET_SUCCESS;
> > > }
> > >
> > > -/**
> > > - * do_bootefi_image() - execute EFI binary
> > > - *
> > > - * Set up memory image for the binary to be loaded, prepare device path, and
> > > - * then call do_bootefi_exec() to execute it.
> > > - *
> > > - * @image_opt: string with image start address
> > > - * @size_opt: string with image size or NULL
> > > - * Return: status code
> > > - */
> > > -static int do_bootefi_image(const char *image_opt, const char *size_opt)
> > > -{
> > > - void *image_buf;
> > > - unsigned long addr, size;
> > > - efi_status_t ret;
> > > -
> > > -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> > > - if (!strcmp(image_opt, "hello")) {
> > > - image_buf = __efi_helloworld_begin;
> > > - size = __efi_helloworld_end - __efi_helloworld_begin;
> > > - efi_clear_bootdev();
> > > - } else
> > > -#endif
> > > - {
> > > - addr = strtoul(image_opt, NULL, 16);
> > > - /* Check that a numeric value was passed */
> > > - if (!addr)
> > > - return CMD_RET_USAGE;
> > > - image_buf = map_sysmem(addr, 0);
> > > -
> > > - if (size_opt) {
> > > - size = strtoul(size_opt, NULL, 16);
> > > - if (!size)
> > > - return CMD_RET_USAGE;
> > > - efi_clear_bootdev();
> > > - } else {
> > > - if (image_buf != image_addr) {
> > > - log_err("No UEFI binary known at %s\n",
> > > - image_opt);
> > > - return CMD_RET_FAILURE;
> > > - }
> > > - size = image_size;
> > > - }
> > > - }
> > > - ret = efi_run_image(image_buf, size);
> > > -
> > > - if (ret != EFI_SUCCESS)
> > > - return CMD_RET_FAILURE;
> > > -
> > > - return CMD_RET_SUCCESS;
> > > -}
> > > -
> > > /**
> > > * efi_run_image() - run loaded UEFI image
> > > *
> > > @@ -648,8 +596,9 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > char *const argv[])
> > > {
> > > efi_status_t ret;
> > > - char *img_addr, *img_size, *str_copy, *pos;
> > > - void *fdt;
> > > + char *p;
> > > + void *fdt, *image_buf;
> > > + unsigned long addr, size;
> > >
> > > if (argc < 2)
> > > return CMD_RET_USAGE;
> > > @@ -684,18 +633,42 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > if (!strcmp(argv[1], "selftest"))
> > > return do_efi_selftest();
> > > #endif
> > > - str_copy = strdup(argv[1]);
> > > - if (!str_copy) {
> > > - log_err("Out of memory\n");
> > > - return CMD_RET_FAILURE;
> > > +
> > > +#ifdef CONFIG_CMD_BOOTEFI_HELLO
> >
> > I would prefer a normal if and let the linker sort it out.
>
> Here I focused on simply unfolding("moving") the function, keeping the code
> as same as possible. Any how the code is in a tentative form
> at this point and will be reshaped in later patches, using IS_ENABLED().
> Please look at the final code after applying the whole series.
I also agree with this approach.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 11/13] fs: remove explicit efi configuration dependency
2023-10-27 0:59 ` AKASHI Takahiro
@ 2023-10-27 1:04 ` Tom Rini
0 siblings, 0 replies; 31+ messages in thread
From: Tom Rini @ 2023-10-27 1:04 UTC (permalink / raw)
To: AKASHI Takahiro, Heinrich Schuchardt, sjg, ilias.apalodimas,
jbx6244, u-boot
[-- Attachment #1: Type: text/plain, Size: 3147 bytes --]
On Fri, Oct 27, 2023 at 09:59:02AM +0900, AKASHI Takahiro wrote:
> On Thu, Oct 26, 2023 at 08:47:52AM -0400, Tom Rini wrote:
> > On Thu, Oct 26, 2023 at 05:48:30PM +0900, AKASHI Takahiro wrote:
> > > On Thu, Oct 26, 2023 at 09:58:53AM +0200, Heinrich Schuchardt wrote:
> > > >
> > > >
> > > > Am 26. Oktober 2023 07:30:50 MESZ schrieb AKASHI Takahiro <takahiro.akashi@linaro.org>:
> > > > >Now it is clear that the feature actually depends on efi interfaces,
> > > > >not "bootefi" command. efi_set_bootdev() will automatically be nullified
> > > > >if necessary efi component is disabled.
> > > > >
> > > > >Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > >---
> > > > > fs/fs.c | 7 +++----
> > > > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > > > >
> > > > >diff --git a/fs/fs.c b/fs/fs.c
> > > > >index 4cb4310c9cc2..70cdb594c4c8 100644
> > > > >--- a/fs/fs.c
> > > > >+++ b/fs/fs.c
> > > > >@@ -791,10 +791,9 @@ int do_load(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[],
> > > > > return 1;
> > > > > }
> > > > >
> > > > >- if (IS_ENABLED(CONFIG_CMD_BOOTEFI))
> > > > >- efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
> > > > >- (argc > 4) ? argv[4] : "", map_sysmem(addr, 0),
> > > > >- len_read);
> > > > >+ efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
> > > >
> > > > This function should not exist for CONFIG_EFI_LOADER=n. There are other places where the function is invoked. Please, review all of them.
> > >
> > > Please go through the whole patch set, especially patch#8
> > > "efi_loader: split unrelated code from efi_bootmgr.c".
> > >
> > > efi_[set|clear]_bootdev() will be nullified if not necessary.
> >
> > In this case I think what we have here today is more readable / clearer.
> > We don't need empty functions as we can either do like this section of
> > the code does today or the linker will discard things correctly as it's
> > a case of funcB() calls funcA() and nothing calls funcB() so it will be
> > discarded. I don't know without digging through the series more what the
> > correct IS_ENABLED() guard should be here (and then also in patch #10).
>
> If I correctly understand your question, it is my point in this commit.
>
> I believe that a caller should not be bothered by thinking of what efi CONFIG
> be used for the guard. All that should be done is to just put a right hook
> (efi_set_bootdev() in this case) at a right place as a caller (do_load()
> in this case) is apparently irrelevant to UEFI subsystem.
> Hereafter, whatever rework may be done on UEFI side (like my refactoring
> here), other code won't need to be modified.
>
> If you want to rely on an intelligent linker behavior, I would suggest
> another approach, modifying efi_set_bootdev() as follows;
>
> efi_set_bootdev(...)
> {
> if (!IS_ENABLED(EFI_BINARY_EXEC))
> return;
> ...
> }
>
> I hope it would also work.
Looking at all of the ways to solve this particular case again, OK, I
think we can go with #8, #10 and #11 as they are in this series, thanks
again.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 01/13] cmd: bootefi: unfold do_bootefi_image()
2023-10-27 1:00 ` Tom Rini
@ 2023-10-27 12:23 ` Ilias Apalodimas
2023-10-30 0:34 ` AKASHI Takahiro
0 siblings, 1 reply; 31+ messages in thread
From: Ilias Apalodimas @ 2023-10-27 12:23 UTC (permalink / raw)
To: Tom Rini; +Cc: AKASHI Takahiro, Heinrich Schuchardt, jbx6244, u-boot, sjg
Akashi-san
On Fri, 27 Oct 2023 at 04:00, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Oct 27, 2023 at 09:25:44AM +0900, AKASHI Takahiro wrote:
> > On Thu, Oct 26, 2023 at 01:01:52PM +0200, Heinrich Schuchardt wrote:
> > > On 10/26/23 07:30, AKASHI Takahiro wrote:
> > > > Unfold do_bootefi_image() into do_bootefi() for the sake of the succeeding
> > > > refactor work.
> > > >
I don't disagree with the patchset, but what the patch does is pretty
obvious. It would help a lot to briefly describe how the unfolding
process helps the refactoring.
Thanks
/Ilias
> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > ---
> > > > cmd/bootefi.c | 101 ++++++++++++++++++--------------------------------
> > > > 1 file changed, 37 insertions(+), 64 deletions(-)
> > > >
> > > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > > > index 20e5c94a33a4..1b28bf5a318d 100644
> > > > --- a/cmd/bootefi.c
> > > > +++ b/cmd/bootefi.c
> > > > @@ -425,58 +425,6 @@ static int do_efibootmgr(void)
> > > > return CMD_RET_SUCCESS;
> > > > }
> > > >
> > > > -/**
> > > > - * do_bootefi_image() - execute EFI binary
> > > > - *
> > > > - * Set up memory image for the binary to be loaded, prepare device path, and
> > > > - * then call do_bootefi_exec() to execute it.
> > > > - *
> > > > - * @image_opt: string with image start address
> > > > - * @size_opt: string with image size or NULL
> > > > - * Return: status code
> > > > - */
> > > > -static int do_bootefi_image(const char *image_opt, const char *size_opt)
> > > > -{
> > > > - void *image_buf;
> > > > - unsigned long addr, size;
> > > > - efi_status_t ret;
> > > > -
> > > > -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> > > > - if (!strcmp(image_opt, "hello")) {
> > > > - image_buf = __efi_helloworld_begin;
> > > > - size = __efi_helloworld_end - __efi_helloworld_begin;
> > > > - efi_clear_bootdev();
> > > > - } else
> > > > -#endif
> > > > - {
> > > > - addr = strtoul(image_opt, NULL, 16);
> > > > - /* Check that a numeric value was passed */
> > > > - if (!addr)
> > > > - return CMD_RET_USAGE;
> > > > - image_buf = map_sysmem(addr, 0);
> > > > -
> > > > - if (size_opt) {
> > > > - size = strtoul(size_opt, NULL, 16);
> > > > - if (!size)
> > > > - return CMD_RET_USAGE;
> > > > - efi_clear_bootdev();
> > > > - } else {
> > > > - if (image_buf != image_addr) {
> > > > - log_err("No UEFI binary known at %s\n",
> > > > - image_opt);
> > > > - return CMD_RET_FAILURE;
> > > > - }
> > > > - size = image_size;
> > > > - }
> > > > - }
> > > > - ret = efi_run_image(image_buf, size);
> > > > -
> > > > - if (ret != EFI_SUCCESS)
> > > > - return CMD_RET_FAILURE;
> > > > -
> > > > - return CMD_RET_SUCCESS;
> > > > -}
> > > > -
> > > > /**
> > > > * efi_run_image() - run loaded UEFI image
> > > > *
> > > > @@ -648,8 +596,9 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > char *const argv[])
> > > > {
> > > > efi_status_t ret;
> > > > - char *img_addr, *img_size, *str_copy, *pos;
> > > > - void *fdt;
> > > > + char *p;
> > > > + void *fdt, *image_buf;
> > > > + unsigned long addr, size;
> > > >
> > > > if (argc < 2)
> > > > return CMD_RET_USAGE;
> > > > @@ -684,18 +633,42 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > if (!strcmp(argv[1], "selftest"))
> > > > return do_efi_selftest();
> > > > #endif
> > > > - str_copy = strdup(argv[1]);
> > > > - if (!str_copy) {
> > > > - log_err("Out of memory\n");
> > > > - return CMD_RET_FAILURE;
> > > > +
> > > > +#ifdef CONFIG_CMD_BOOTEFI_HELLO
> > >
> > > I would prefer a normal if and let the linker sort it out.
> >
> > Here I focused on simply unfolding("moving") the function, keeping the code
> > as same as possible. Any how the code is in a tentative form
> > at this point and will be reshaped in later patches, using IS_ENABLED().
> > Please look at the final code after applying the whole series.
>
> I also agree with this approach.
>
> --
> Tom
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 12/13] lib: uuid: move CONFIG_RANDOM_UUID
2023-10-26 5:30 ` [RFC 12/13] lib: uuid: move CONFIG_RANDOM_UUID AKASHI Takahiro
2023-10-26 12:47 ` Tom Rini
@ 2023-10-27 12:24 ` Ilias Apalodimas
1 sibling, 0 replies; 31+ messages in thread
From: Ilias Apalodimas @ 2023-10-27 12:24 UTC (permalink / raw)
To: AKASHI Takahiro; +Cc: trini, sjg, xypron.glpk, jbx6244, u-boot
On Thu, 26 Oct 2023 at 08:31, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> This option is independent from any commands and should be managed
> under lib. For instance, drivers/block/rkmtd.c is a user.
>
> It would be better to remove this configuration.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> cmd/Kconfig | 7 -------
> lib/Kconfig | 7 +++++++
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 3a8483f7d042..c42745d06b39 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1143,13 +1143,6 @@ config CMD_GPT
> Enable the 'gpt' command to ready and write GPT style partition
> tables.
>
> -config RANDOM_UUID
> - bool "GPT Random UUID generation"
> - select LIB_UUID
> - help
> - Enable the generation of partitions with random UUIDs if none
> - are provided.
> -
> config CMD_GPT_RENAME
> bool "GPT partition renaming commands"
> depends on CMD_GPT
> diff --git a/lib/Kconfig b/lib/Kconfig
> index f6ca559897e7..b6d580c30615 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -74,6 +74,13 @@ config HAVE_PRIVATE_LIBGCC
> config LIB_UUID
> bool
>
> +config RANDOM_UUID
> + bool "GPT Random UUID generation"
> + select LIB_UUID
> + help
> + Enable the generation of partitions with random UUIDs if none
> + are provided.
> +
> config SPL_LIB_UUID
> depends on SPL
> bool
> --
> 2.34.1
>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC 01/13] cmd: bootefi: unfold do_bootefi_image()
2023-10-27 12:23 ` Ilias Apalodimas
@ 2023-10-30 0:34 ` AKASHI Takahiro
0 siblings, 0 replies; 31+ messages in thread
From: AKASHI Takahiro @ 2023-10-30 0:34 UTC (permalink / raw)
To: Ilias Apalodimas; +Cc: Tom Rini, Heinrich Schuchardt, jbx6244, u-boot, sjg
Hi Ilias,
On Fri, Oct 27, 2023 at 03:23:07PM +0300, Ilias Apalodimas wrote:
> Akashi-san
>
> On Fri, 27 Oct 2023 at 04:00, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Oct 27, 2023 at 09:25:44AM +0900, AKASHI Takahiro wrote:
> > > On Thu, Oct 26, 2023 at 01:01:52PM +0200, Heinrich Schuchardt wrote:
> > > > On 10/26/23 07:30, AKASHI Takahiro wrote:
> > > > > Unfold do_bootefi_image() into do_bootefi() for the sake of the succeeding
> > > > > refactor work.
> > > > >
>
> I don't disagree with the patchset, but what the patch does is pretty
> obvious.
Yeah, that is what I tried to do in this patch series, i.e. each step
be as trivial as possible to ensure that the semantics is unchanged
while the code is being morphed.
> It would help a lot to briefly describe how the unfolding
> process helps the refactoring.
Not sure, but will add a few more words.
Thanks,
-Takahiro Akashi
> Thanks
> /Ilias
> > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > > ---
> > > > > cmd/bootefi.c | 101 ++++++++++++++++++--------------------------------
> > > > > 1 file changed, 37 insertions(+), 64 deletions(-)
> > > > >
> > > > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > > > > index 20e5c94a33a4..1b28bf5a318d 100644
> > > > > --- a/cmd/bootefi.c
> > > > > +++ b/cmd/bootefi.c
> > > > > @@ -425,58 +425,6 @@ static int do_efibootmgr(void)
> > > > > return CMD_RET_SUCCESS;
> > > > > }
> > > > >
> > > > > -/**
> > > > > - * do_bootefi_image() - execute EFI binary
> > > > > - *
> > > > > - * Set up memory image for the binary to be loaded, prepare device path, and
> > > > > - * then call do_bootefi_exec() to execute it.
> > > > > - *
> > > > > - * @image_opt: string with image start address
> > > > > - * @size_opt: string with image size or NULL
> > > > > - * Return: status code
> > > > > - */
> > > > > -static int do_bootefi_image(const char *image_opt, const char *size_opt)
> > > > > -{
> > > > > - void *image_buf;
> > > > > - unsigned long addr, size;
> > > > > - efi_status_t ret;
> > > > > -
> > > > > -#ifdef CONFIG_CMD_BOOTEFI_HELLO
> > > > > - if (!strcmp(image_opt, "hello")) {
> > > > > - image_buf = __efi_helloworld_begin;
> > > > > - size = __efi_helloworld_end - __efi_helloworld_begin;
> > > > > - efi_clear_bootdev();
> > > > > - } else
> > > > > -#endif
> > > > > - {
> > > > > - addr = strtoul(image_opt, NULL, 16);
> > > > > - /* Check that a numeric value was passed */
> > > > > - if (!addr)
> > > > > - return CMD_RET_USAGE;
> > > > > - image_buf = map_sysmem(addr, 0);
> > > > > -
> > > > > - if (size_opt) {
> > > > > - size = strtoul(size_opt, NULL, 16);
> > > > > - if (!size)
> > > > > - return CMD_RET_USAGE;
> > > > > - efi_clear_bootdev();
> > > > > - } else {
> > > > > - if (image_buf != image_addr) {
> > > > > - log_err("No UEFI binary known at %s\n",
> > > > > - image_opt);
> > > > > - return CMD_RET_FAILURE;
> > > > > - }
> > > > > - size = image_size;
> > > > > - }
> > > > > - }
> > > > > - ret = efi_run_image(image_buf, size);
> > > > > -
> > > > > - if (ret != EFI_SUCCESS)
> > > > > - return CMD_RET_FAILURE;
> > > > > -
> > > > > - return CMD_RET_SUCCESS;
> > > > > -}
> > > > > -
> > > > > /**
> > > > > * efi_run_image() - run loaded UEFI image
> > > > > *
> > > > > @@ -648,8 +596,9 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > > char *const argv[])
> > > > > {
> > > > > efi_status_t ret;
> > > > > - char *img_addr, *img_size, *str_copy, *pos;
> > > > > - void *fdt;
> > > > > + char *p;
> > > > > + void *fdt, *image_buf;
> > > > > + unsigned long addr, size;
> > > > >
> > > > > if (argc < 2)
> > > > > return CMD_RET_USAGE;
> > > > > @@ -684,18 +633,42 @@ static int do_bootefi(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > > if (!strcmp(argv[1], "selftest"))
> > > > > return do_efi_selftest();
> > > > > #endif
> > > > > - str_copy = strdup(argv[1]);
> > > > > - if (!str_copy) {
> > > > > - log_err("Out of memory\n");
> > > > > - return CMD_RET_FAILURE;
> > > > > +
> > > > > +#ifdef CONFIG_CMD_BOOTEFI_HELLO
> > > >
> > > > I would prefer a normal if and let the linker sort it out.
> > >
> > > Here I focused on simply unfolding("moving") the function, keeping the code
> > > as same as possible. Any how the code is in a tentative form
> > > at this point and will be reshaped in later patches, using IS_ENABLED().
> > > Please look at the final code after applying the whole series.
> >
> > I also agree with this approach.
> >
> > --
> > Tom
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2023-10-30 0:34 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-26 5:30 [RFC 00/13] cmd: bootefi: refactor the code for bootmgr AKASHI Takahiro
2023-10-26 5:30 ` [RFC 01/13] cmd: bootefi: unfold do_bootefi_image() AKASHI Takahiro
2023-10-26 11:01 ` Heinrich Schuchardt
2023-10-27 0:25 ` AKASHI Takahiro
2023-10-27 1:00 ` Tom Rini
2023-10-27 12:23 ` Ilias Apalodimas
2023-10-30 0:34 ` AKASHI Takahiro
2023-10-26 5:30 ` [RFC 02/13] cmd: bootefi: re-organize do_bootefi_image() AKASHI Takahiro
2023-10-26 10:44 ` Heinrich Schuchardt
2023-10-26 12:47 ` Tom Rini
2023-10-27 0:35 ` AKASHI Takahiro
2023-10-26 5:30 ` [RFC 03/13] cmd: bootefi: carve out EFI boot manager interface AKASHI Takahiro
2023-10-26 5:30 ` [RFC 04/13] cmd: bootefi: carve out binary execution interface AKASHI Takahiro
2023-10-26 5:30 ` [RFC 05/13] cmd: bootefi: move library interfaces under lib/efi_loader AKASHI Takahiro
2023-10-26 5:30 ` [RFC 06/13] cmd: efidebug: ease efi configuration dependency AKASHI Takahiro
2023-10-26 5:30 ` [RFC 07/13] bootmeth: use efi_loader interfaces instead of bootefi command AKASHI Takahiro
2023-10-26 5:30 ` [RFC 08/13] efi_loader: split unrelated code from efi_bootmgr.c AKASHI Takahiro
2023-10-26 5:30 ` [RFC 09/13] efi_loader: rename BOOTEFI_BOOTMGR to EFI_BOOTMGR AKASHI Takahiro
2023-10-26 5:30 ` [RFC 10/13] net: tftp: remove explicit efi configuration dependency AKASHI Takahiro
2023-10-26 5:30 ` [RFC 11/13] fs: " AKASHI Takahiro
2023-10-26 7:58 ` Heinrich Schuchardt
2023-10-26 8:48 ` AKASHI Takahiro
2023-10-26 12:47 ` Tom Rini
2023-10-27 0:59 ` AKASHI Takahiro
2023-10-27 1:04 ` Tom Rini
2023-10-26 5:30 ` [RFC 12/13] lib: uuid: move CONFIG_RANDOM_UUID AKASHI Takahiro
2023-10-26 12:47 ` Tom Rini
2023-10-27 12:24 ` Ilias Apalodimas
2023-10-26 5:30 ` [RFC 13/13] block: rkmtd: select CONFIG_RANDOM_UUID explicitly AKASHI Takahiro
2023-10-26 12:47 ` Tom Rini
2023-10-26 12:47 ` [RFC 00/13] cmd: bootefi: refactor the code for bootmgr Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.