* [PATCH 01/15] log: Add a new category for tests
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
@ 2024-10-28 12:47 ` Simon Glass
2024-10-28 12:47 ` [PATCH 02/15] test: Allow saving and restoring the bloblist Simon Glass
` (14 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:47 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass,
Quentin Schulz, Thomas Weißschuh
In some core test code, no existing categories make sense. Add a new one
for testing.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
common/log.c | 1 +
include/log.h | 2 ++
2 files changed, 3 insertions(+)
diff --git a/common/log.c b/common/log.c
index b83a6618900..c9fe35230d6 100644
--- a/common/log.c
+++ b/common/log.c
@@ -32,6 +32,7 @@ static const char *const log_cat_name[] = {
"fs",
"expo",
"console",
+ "test",
};
_Static_assert(ARRAY_SIZE(log_cat_name) == LOGC_COUNT - LOGC_NONE,
diff --git a/include/log.h b/include/log.h
index bf81a27011f..4f6d6a2c2cf 100644
--- a/include/log.h
+++ b/include/log.h
@@ -106,6 +106,8 @@ enum log_category_t {
LOGC_EXPO,
/** @LOGC_CONSOLE: Related to the console and stdio */
LOGC_CONSOLE,
+ /** @LOGC_TEST: Related to testing */
+ LOGC_TEST,
/** @LOGC_COUNT: Number of log categories */
LOGC_COUNT,
/** @LOGC_END: Sentinel value for lists of log categories */
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 02/15] test: Allow saving and restoring the bloblist
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
2024-10-28 12:47 ` [PATCH 01/15] log: Add a new category for tests Simon Glass
@ 2024-10-28 12:47 ` Simon Glass
2024-10-28 12:47 ` [PATCH 03/15] bloblist: test: Mark tests with UTF_BLOBLIST Simon Glass
` (13 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:47 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass,
Mattijs Korpershoek
Tests which create a new bloblist overwrite the existing one in sandbox.
Provide a flag for tests to declare this behaviour. Save and restore the
bloblist pointer so that other tests remain unaffected.
Note that when sandbox is running normally, the bloblist has been
relocated to high in memory. The existing bloblist tests create a new
bloblist low in memory, so they do not conflict.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
include/test/test.h | 3 +++
test/test-main.c | 13 +++++++++++++
2 files changed, 16 insertions(+)
diff --git a/include/test/test.h b/include/test/test.h
index 92eec2eb6f9..21c0478befe 100644
--- a/include/test/test.h
+++ b/include/test/test.h
@@ -29,6 +29,7 @@
* @of_other: Live tree for the other FDT
* @runs_per_test: Number of times to run each test (typically 1)
* @force_run: true to run tests marked with the UTF_MANUAL flag
+ * @old_bloblist: stores the old gd->bloblist pointer
* @expect_str: Temporary string used to hold expected string value
* @actual_str: Temporary string used to hold actual string value
*/
@@ -50,6 +51,7 @@ struct unit_test_state {
struct device_node *of_other;
int runs_per_test;
bool force_run;
+ void *old_bloblist;
char expect_str[512];
char actual_str[512];
};
@@ -73,6 +75,7 @@ enum ut_flags {
UTF_MANUAL = BIT(8),
UTF_ETH_BOOTDEV = BIT(9), /* enable Ethernet bootdevs */
UTF_SF_BOOTDEV = BIT(10), /* enable SPI flash bootdevs */
+ UFT_BLOBLIST = BIT(11), /* test changes gd->bloblist */
};
/**
diff --git a/test/test-main.c b/test/test-main.c
index c825f9c7797..e1e8c6cde4b 100644
--- a/test/test-main.c
+++ b/test/test-main.c
@@ -4,6 +4,8 @@
* Written by Simon Glass <sjg@chromium.org>
*/
+#define LOG_CATEGORY LOGC_TEST
+
#include <blk.h>
#include <console.h>
#include <cyclic.h>
@@ -379,6 +381,12 @@ static int test_pre_run(struct unit_test_state *uts, struct unit_test *test)
return -EAGAIN;
}
}
+ if (test->flags & UFT_BLOBLIST) {
+ log_debug("save bloblist %p\n", gd->bloblist);
+ uts->old_bloblist = gd->bloblist;
+ gd->bloblist = NULL;
+ }
+
ut_silence_console(uts);
return 0;
@@ -402,6 +410,11 @@ static int test_post_run(struct unit_test_state *uts, struct unit_test *test)
free(uts->of_other);
uts->of_other = NULL;
+ if (test->flags & UFT_BLOBLIST) {
+ gd->bloblist = uts->old_bloblist;
+ log_debug("restore bloblist %p\n", gd->bloblist);
+ }
+
blkcache_free();
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 03/15] bloblist: test: Mark tests with UTF_BLOBLIST
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
2024-10-28 12:47 ` [PATCH 01/15] log: Add a new category for tests Simon Glass
2024-10-28 12:47 ` [PATCH 02/15] test: Allow saving and restoring the bloblist Simon Glass
@ 2024-10-28 12:47 ` Simon Glass
2024-10-28 12:47 ` [PATCH 04/15] sandbox: Convert sb command to use new macro Simon Glass
` (12 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:47 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass,
Raymond Mao
Mark bloblist tests with this flag so that other tests which use
bloblist remain unaffected.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
test/common/bloblist.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/test/common/bloblist.c b/test/common/bloblist.c
index dbd174299fd..4ff0234ca00 100644
--- a/test/common/bloblist.c
+++ b/test/common/bloblist.c
@@ -87,7 +87,7 @@ static int bloblist_test_init(struct unit_test_state *uts)
return 1;
}
-COMMON_TEST(bloblist_test_init, 0);
+COMMON_TEST(bloblist_test_init, UFT_BLOBLIST);
static int bloblist_test_blob(struct unit_test_state *uts)
{
@@ -127,7 +127,7 @@ static int bloblist_test_blob(struct unit_test_state *uts)
return 0;
}
-COMMON_TEST(bloblist_test_blob, 0);
+COMMON_TEST(bloblist_test_blob, UFT_BLOBLIST);
/* Check bloblist_ensure_size_ret() */
static int bloblist_test_blob_ensure(struct unit_test_state *uts)
@@ -161,7 +161,7 @@ static int bloblist_test_blob_ensure(struct unit_test_state *uts)
return 0;
}
-COMMON_TEST(bloblist_test_blob_ensure, 0);
+COMMON_TEST(bloblist_test_blob_ensure, UFT_BLOBLIST);
static int bloblist_test_bad_blob(struct unit_test_state *uts)
{
@@ -177,7 +177,7 @@ static int bloblist_test_bad_blob(struct unit_test_state *uts)
return 0;
}
-COMMON_TEST(bloblist_test_bad_blob, 0);
+COMMON_TEST(bloblist_test_bad_blob, UFT_BLOBLIST);
static int bloblist_test_checksum(struct unit_test_state *uts)
{
@@ -250,7 +250,7 @@ static int bloblist_test_checksum(struct unit_test_state *uts)
return 0;
}
-COMMON_TEST(bloblist_test_checksum, 0);
+COMMON_TEST(bloblist_test_checksum, UFT_BLOBLIST);
/* Test the 'bloblist info' command */
static int bloblist_test_cmd_info(struct unit_test_state *uts)
@@ -271,7 +271,7 @@ static int bloblist_test_cmd_info(struct unit_test_state *uts)
return 0;
}
-COMMON_TEST(bloblist_test_cmd_info, UTF_CONSOLE);
+COMMON_TEST(bloblist_test_cmd_info, UFT_BLOBLIST | UTF_CONSOLE);
/* Test the 'bloblist list' command */
static int bloblist_test_cmd_list(struct unit_test_state *uts)
@@ -293,7 +293,7 @@ static int bloblist_test_cmd_list(struct unit_test_state *uts)
return 0;
}
-COMMON_TEST(bloblist_test_cmd_list, UTF_CONSOLE);
+COMMON_TEST(bloblist_test_cmd_list, UFT_BLOBLIST | UTF_CONSOLE);
/* Test alignment of bloblist blobs */
static int bloblist_test_align(struct unit_test_state *uts)
@@ -351,7 +351,7 @@ static int bloblist_test_align(struct unit_test_state *uts)
return 0;
}
-COMMON_TEST(bloblist_test_align, 0);
+COMMON_TEST(bloblist_test_align, UFT_BLOBLIST);
/* Test relocation of a bloblist */
static int bloblist_test_reloc(struct unit_test_state *uts)
@@ -385,7 +385,7 @@ static int bloblist_test_reloc(struct unit_test_state *uts)
return 0;
}
-COMMON_TEST(bloblist_test_reloc, 0);
+COMMON_TEST(bloblist_test_reloc, UFT_BLOBLIST);
/* Test expansion of a blob */
static int bloblist_test_grow(struct unit_test_state *uts)
@@ -438,7 +438,7 @@ static int bloblist_test_grow(struct unit_test_state *uts)
return 0;
}
-COMMON_TEST(bloblist_test_grow, 0);
+COMMON_TEST(bloblist_test_grow, UFT_BLOBLIST);
/* Test shrinking of a blob */
static int bloblist_test_shrink(struct unit_test_state *uts)
@@ -488,7 +488,7 @@ static int bloblist_test_shrink(struct unit_test_state *uts)
return 0;
}
-COMMON_TEST(bloblist_test_shrink, 0);
+COMMON_TEST(bloblist_test_shrink, UFT_BLOBLIST);
/* Test failing to adjust a blob size */
static int bloblist_test_resize_fail(struct unit_test_state *uts)
@@ -523,7 +523,7 @@ static int bloblist_test_resize_fail(struct unit_test_state *uts)
return 0;
}
-COMMON_TEST(bloblist_test_resize_fail, 0);
+COMMON_TEST(bloblist_test_resize_fail, UFT_BLOBLIST);
/* Test expanding the last blob in a bloblist */
static int bloblist_test_resize_last(struct unit_test_state *uts)
@@ -574,7 +574,7 @@ static int bloblist_test_resize_last(struct unit_test_state *uts)
return 0;
}
-COMMON_TEST(bloblist_test_resize_last, 0);
+COMMON_TEST(bloblist_test_resize_last, UFT_BLOBLIST);
/* Check a completely full bloblist */
static int bloblist_test_blob_maxsize(struct unit_test_state *uts)
@@ -597,4 +597,4 @@ static int bloblist_test_blob_maxsize(struct unit_test_state *uts)
return 0;
}
-COMMON_TEST(bloblist_test_blob_maxsize, 0);
+COMMON_TEST(bloblist_test_blob_maxsize, UFT_BLOBLIST);
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 04/15] sandbox: Convert sb command to use new macro
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
` (2 preceding siblings ...)
2024-10-28 12:47 ` [PATCH 03/15] bloblist: test: Mark tests with UTF_BLOBLIST Simon Glass
@ 2024-10-28 12:47 ` Simon Glass
2024-10-28 12:47 ` [PATCH 05/15] doc: sandbox: Add docs for the sb command Simon Glass
` (11 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:47 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass
Ise the new U_BOOT_CMD_WITH_SUBCMDS() macro instead of writing the code
out manually.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
cmd/sb.c | 31 ++++++-------------------------
1 file changed, 6 insertions(+), 25 deletions(-)
diff --git a/cmd/sb.c b/cmd/sb.c
index db485fddfca..9dbb53275b3 100644
--- a/cmd/sb.c
+++ b/cmd/sb.c
@@ -40,29 +40,10 @@ static int do_sb_state(struct cmd_tbl *cmdtp, int flag, int argc,
return 0;
}
-static struct cmd_tbl cmd_sb_sub[] = {
- U_BOOT_CMD_MKENT(handoff, 1, 0, do_sb_handoff, "", ""),
- U_BOOT_CMD_MKENT(state, 1, 0, do_sb_state, "", ""),
-};
-
-static int do_sb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
-{
- struct cmd_tbl *c;
-
- /* Skip past 'sb' */
- argc--;
- argv++;
-
- c = find_cmd_tbl(argv[0], cmd_sb_sub, ARRAY_SIZE(cmd_sb_sub));
- if (c)
- return c->cmd(cmdtp, flag, argc, argv);
- else
- return CMD_RET_USAGE;
-}
-
-U_BOOT_CMD(
- sb, 8, 1, do_sb,
- "Sandbox status commands",
+U_BOOT_LONGHELP(sb,
"handoff - Show handoff data received from SPL\n"
- "sb state - Show sandbox state"
-);
+ "sb state - Show sandbox state");
+
+U_BOOT_CMD_WITH_SUBCMDS(sb, "Sandbox status commands", sb_help_text,
+ U_BOOT_SUBCMD_MKENT(handoff, 1, 1, do_sb_handoff),
+ U_BOOT_SUBCMD_MKENT(state, 1, 1, do_sb_state));
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 05/15] doc: sandbox: Add docs for the sb command
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
` (3 preceding siblings ...)
2024-10-28 12:47 ` [PATCH 04/15] sandbox: Convert sb command to use new macro Simon Glass
@ 2024-10-28 12:47 ` Simon Glass
2024-11-03 23:24 ` Heinrich Schuchardt
2024-10-28 12:47 ` [PATCH 06/15] sandbox: Add a way to show the sandbox memory-mapping Simon Glass
` (10 subsequent siblings)
15 siblings, 1 reply; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:47 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass,
AKASHI Takahiro, Emil Kronborg, Francis Laniel, Hou Zhiqiang
This command has a few small features, so document it.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
doc/usage/cmd/sb.rst | 54 ++++++++++++++++++++++++++++++++++++++++++++
doc/usage/index.rst | 1 +
2 files changed, 55 insertions(+)
create mode 100644 doc/usage/cmd/sb.rst
diff --git a/doc/usage/cmd/sb.rst b/doc/usage/cmd/sb.rst
new file mode 100644
index 00000000000..6f54f9d9eb7
--- /dev/null
+++ b/doc/usage/cmd/sb.rst
@@ -0,0 +1,54 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+.. index::
+ single: sbi (command)
+
+sbi command
+===========
+
+Synopsis
+--------
+
+::
+
+ sb handoff
+ sb state
+
+Description
+-----------
+
+The *sb* command is used to display information about sandbox's internal
+operation. See :doc:`/arch/sandbox/index` for more information.
+
+sb handoff
+~~~~~~~~~~
+
+This shows information about any handoff information received from SPL. If
+U-Boot is started from an SPL build, it shows a valid magic number.
+
+sb state
+~~~~~~~~
+
+This shows basic information about the sandbox state, currently just the
+command-line with which sandbox was started.
+
+Example
+-------
+
+This shows checking for the presence of SPL-handoff information. For this to
+work, ``u-boot-spl`` must be run, with build that enables ``CONFIG_SPL``, such
+as ``sandbox_spl``::
+
+ => sb handoff
+ SPL handoff magic 14f93c7b
+
+This shows output from the *sb state* subcommand::
+
+ => sb state
+ Arguments:
+ /tmp/b/sandbox/u-boot -D
+
+Configuration
+-------------
+
+The *sb handoff* command is only supported if CONFIG_HANDOFF is enabled.
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index db71711c393..24b2d2637b1 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -103,6 +103,7 @@ Shell commands
cmd/reset
cmd/rng
cmd/saves
+ cmd/sb
cmd/sbi
cmd/scmi
cmd/scp03
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 05/15] doc: sandbox: Add docs for the sb command
2024-10-28 12:47 ` [PATCH 05/15] doc: sandbox: Add docs for the sb command Simon Glass
@ 2024-11-03 23:24 ` Heinrich Schuchardt
0 siblings, 0 replies; 40+ messages in thread
From: Heinrich Schuchardt @ 2024-11-03 23:24 UTC (permalink / raw)
To: Simon Glass
Cc: Ilias Apalodimas, Tom Rini, AKASHI Takahiro, Emil Kronborg,
Francis Laniel, Hou Zhiqiang, U-Boot Mailing List
On 10/28/24 13:47, Simon Glass wrote:
> This command has a few small features, so document it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> doc/usage/cmd/sb.rst | 54 ++++++++++++++++++++++++++++++++++++++++++++
> doc/usage/index.rst | 1 +
> 2 files changed, 55 insertions(+)
> create mode 100644 doc/usage/cmd/sb.rst
>
> diff --git a/doc/usage/cmd/sb.rst b/doc/usage/cmd/sb.rst
> new file mode 100644
> index 00000000000..6f54f9d9eb7
> --- /dev/null
> +++ b/doc/usage/cmd/sb.rst
> @@ -0,0 +1,54 @@
> +.. SPDX-License-Identifier: GPL-2.0+
Please, use a valid SPDX identifier. See here
https://spdx.org/licenses/GPL-2.0-or-later.html
> +
> +.. index::
> + single: sbi (command)
The sbi command is used to show the status of the RISC-V SBI firmware.
You wanted to describe the sb command?
> +
> +sbi command
ditto
> +===========
> +
> +Synopsis
> +--------
> +
> +::
> +
> + sb handoff
> + sb state
> +
> +Description
> +-----------
> +
> +The *sb* command is used to display information about sandbox's internal
> +operation. See :doc:`/arch/sandbox/index` for more information.
The command can only show states and not any operation.
%s/operation/operational state/ ?
> +
> +sb handoff
> +~~~~~~~~~~
> +
> +This shows information about any handoff information received from SPL. If
> +U-Boot is started from an SPL build, it shows a valid magic number.
> +
> +sb state
> +~~~~~~~~
> +
> +This shows basic information about the sandbox state, currently just the
> +command-line with which sandbox was started.
> +
> +Example
> +-------
> +
> +This shows checking for the presence of SPL-handoff information. For this to
> +work, ``u-boot-spl`` must be run, with build that enables ``CONFIG_SPL``, such
> +as ``sandbox_spl``::
> +
> + => sb handoff
> + SPL handoff magic 14f93c7b
As a user I would have no clue what this magic is used for. This needs
some explanation. Why should I care for some random number?
Is this really sandbox only information?
I would be much more interested what information is passed in general
from SPL.
> +
> +This shows output from the *sb state* subcommand::
> +
> + => sb state
> + Arguments:
> + /tmp/b/sandbox/u-boot -D
Please, remove /tmp/b/sandbox. I would expect the average user to invoke
the sandbox from the build directory:
./u-boot -D
> +
> +Configuration
> +-------------
> +
> +The *sb handoff* command is only supported if CONFIG_HANDOFF is enabled.
What enables the sb command?
Why do we need two sub-commands to show so little information?
It would be much easier to simply type 'sb' and get all the information.
If you there is so little information of interest, why don't you simply
implement arch_print_bdinfo() like you did on x86?
Best regards
Heinrich
> diff --git a/doc/usage/index.rst b/doc/usage/index.rst
> index db71711c393..24b2d2637b1 100644
> --- a/doc/usage/index.rst
> +++ b/doc/usage/index.rst
> @@ -103,6 +103,7 @@ Shell commands
> cmd/reset
> cmd/rng
> cmd/saves
> + cmd/sb
> cmd/sbi
> cmd/scmi
> cmd/scp03
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 06/15] sandbox: Add a way to show the sandbox memory-mapping
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
` (4 preceding siblings ...)
2024-10-28 12:47 ` [PATCH 05/15] doc: sandbox: Add docs for the sb command Simon Glass
@ 2024-10-28 12:47 ` Simon Glass
2024-10-29 9:59 ` Ilias Apalodimas
2024-10-28 12:47 ` [PATCH 07/15] sandbox: Fix comment for nomap_sysmem() function Simon Glass
` (9 subsequent siblings)
15 siblings, 1 reply; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:47 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass
This is mostly hidden in the background, but it is sometimes useful to
look at it. Add a function to allow this.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
arch/sandbox/cpu/cpu.c | 13 +++++++++++++
arch/sandbox/include/asm/cpu.h | 3 +++
cmd/sb.c | 11 +++++++++++
doc/usage/cmd/sb.rst | 25 +++++++++++++++++++++++++
4 files changed, 52 insertions(+)
diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
index 06f8c13fab9..d1c4dcf0764 100644
--- a/arch/sandbox/cpu/cpu.c
+++ b/arch/sandbox/cpu/cpu.c
@@ -253,6 +253,19 @@ phys_addr_t map_to_sysmem(const void *ptr)
return mentry->tag;
}
+void sandbox_map_list(void)
+{
+ struct sandbox_mapmem_entry *mentry;
+ struct sandbox_state *state = state_get_current();
+
+ printf("Sandbox memory-mapping\n");
+ printf("%8s %16s %6s\n", "Addr", "Mapping", "Refcnt");
+ list_for_each_entry(mentry, &state->mapmem_head, sibling_node) {
+ printf("%8lx %p %6d\n", mentry->tag, mentry->ptr,
+ mentry->refcnt);
+ }
+}
+
unsigned long sandbox_read(const void *addr, enum sandboxio_size_t size)
{
struct sandbox_state *state = state_get_current();
diff --git a/arch/sandbox/include/asm/cpu.h b/arch/sandbox/include/asm/cpu.h
index c97ac7ba95b..682bb3376d1 100644
--- a/arch/sandbox/include/asm/cpu.h
+++ b/arch/sandbox/include/asm/cpu.h
@@ -8,4 +8,7 @@
void cpu_sandbox_set_current(const char *name);
+/* show the mapping of sandbox addresses to pointers */
+void sandbox_map_list(void);
+
#endif /* __SANDBOX_CPU_H */
diff --git a/cmd/sb.c b/cmd/sb.c
index 9dbb53275b3..9245052492e 100644
--- a/cmd/sb.c
+++ b/cmd/sb.c
@@ -7,6 +7,7 @@
#include <command.h>
#include <dm.h>
#include <spl.h>
+#include <asm/cpu.h>
#include <asm/global_data.h>
#include <asm/state.h>
@@ -29,6 +30,14 @@ static int do_sb_handoff(struct cmd_tbl *cmdtp, int flag, int argc,
#endif
}
+static int do_sb_map(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ sandbox_map_list();
+
+ return 0;
+}
+
static int do_sb_state(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
{
@@ -42,8 +51,10 @@ static int do_sb_state(struct cmd_tbl *cmdtp, int flag, int argc,
U_BOOT_LONGHELP(sb,
"handoff - Show handoff data received from SPL\n"
+ "sb map - Show mapped memory\n"
"sb state - Show sandbox state");
U_BOOT_CMD_WITH_SUBCMDS(sb, "Sandbox status commands", sb_help_text,
U_BOOT_SUBCMD_MKENT(handoff, 1, 1, do_sb_handoff),
+ U_BOOT_SUBCMD_MKENT(map, 1, 1, do_sb_map),
U_BOOT_SUBCMD_MKENT(state, 1, 1, do_sb_state));
diff --git a/doc/usage/cmd/sb.rst b/doc/usage/cmd/sb.rst
index 6f54f9d9eb7..37431aff7c8 100644
--- a/doc/usage/cmd/sb.rst
+++ b/doc/usage/cmd/sb.rst
@@ -12,6 +12,7 @@ Synopsis
::
sb handoff
+ sb map
sb state
Description
@@ -26,6 +27,24 @@ sb handoff
This shows information about any handoff information received from SPL. If
U-Boot is started from an SPL build, it shows a valid magic number.
+sb map
+~~~~~~
+
+This shows any mappings between sandbox's emulated RAM and the underlying host
+address-space.
+
+Fields shown are:
+
+Addr
+ Address in emulated RAM
+
+Mapping
+ Equivalent address in the host address-space. While sandbox requests address
+ ``0x10000000`` from the OS, this is not always available.
+
+Refcnt
+ Shows the number of references to this mapping.
+
sb state
~~~~~~~~
@@ -42,6 +61,12 @@ as ``sandbox_spl``::
=> sb handoff
SPL handoff magic 14f93c7b
+This shows output from the *sb map* subcommand, with a single mapping::
+
+ Sandbox memory-mapping
+ Addr Mapping Refcnt
+ ff000000 000056185b46d6d0 2
+
This shows output from the *sb state* subcommand::
=> sb state
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 06/15] sandbox: Add a way to show the sandbox memory-mapping
2024-10-28 12:47 ` [PATCH 06/15] sandbox: Add a way to show the sandbox memory-mapping Simon Glass
@ 2024-10-29 9:59 ` Ilias Apalodimas
2024-10-29 15:46 ` Simon Glass
0 siblings, 1 reply; 40+ messages in thread
From: Ilias Apalodimas @ 2024-10-29 9:59 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List, Tom Rini, Heinrich Schuchardt
Hi Simon
This seems completely unrelated to the series.
Please send it as a separate patch
Thanks
/Ilias
On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
>
> This is mostly hidden in the background, but it is sometimes useful to
> look at it. Add a function to allow this.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> arch/sandbox/cpu/cpu.c | 13 +++++++++++++
> arch/sandbox/include/asm/cpu.h | 3 +++
> cmd/sb.c | 11 +++++++++++
> doc/usage/cmd/sb.rst | 25 +++++++++++++++++++++++++
> 4 files changed, 52 insertions(+)
>
> diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
> index 06f8c13fab9..d1c4dcf0764 100644
> --- a/arch/sandbox/cpu/cpu.c
> +++ b/arch/sandbox/cpu/cpu.c
> @@ -253,6 +253,19 @@ phys_addr_t map_to_sysmem(const void *ptr)
> return mentry->tag;
> }
>
> +void sandbox_map_list(void)
> +{
> + struct sandbox_mapmem_entry *mentry;
> + struct sandbox_state *state = state_get_current();
> +
> + printf("Sandbox memory-mapping\n");
> + printf("%8s %16s %6s\n", "Addr", "Mapping", "Refcnt");
> + list_for_each_entry(mentry, &state->mapmem_head, sibling_node) {
> + printf("%8lx %p %6d\n", mentry->tag, mentry->ptr,
> + mentry->refcnt);
> + }
> +}
> +
> unsigned long sandbox_read(const void *addr, enum sandboxio_size_t size)
> {
> struct sandbox_state *state = state_get_current();
> diff --git a/arch/sandbox/include/asm/cpu.h b/arch/sandbox/include/asm/cpu.h
> index c97ac7ba95b..682bb3376d1 100644
> --- a/arch/sandbox/include/asm/cpu.h
> +++ b/arch/sandbox/include/asm/cpu.h
> @@ -8,4 +8,7 @@
>
> void cpu_sandbox_set_current(const char *name);
>
> +/* show the mapping of sandbox addresses to pointers */
> +void sandbox_map_list(void);
> +
> #endif /* __SANDBOX_CPU_H */
> diff --git a/cmd/sb.c b/cmd/sb.c
> index 9dbb53275b3..9245052492e 100644
> --- a/cmd/sb.c
> +++ b/cmd/sb.c
> @@ -7,6 +7,7 @@
> #include <command.h>
> #include <dm.h>
> #include <spl.h>
> +#include <asm/cpu.h>
> #include <asm/global_data.h>
> #include <asm/state.h>
>
> @@ -29,6 +30,14 @@ static int do_sb_handoff(struct cmd_tbl *cmdtp, int flag, int argc,
> #endif
> }
>
> +static int do_sb_map(struct cmd_tbl *cmdtp, int flag, int argc,
> + char *const argv[])
> +{
> + sandbox_map_list();
> +
> + return 0;
> +}
> +
> static int do_sb_state(struct cmd_tbl *cmdtp, int flag, int argc,
> char *const argv[])
> {
> @@ -42,8 +51,10 @@ static int do_sb_state(struct cmd_tbl *cmdtp, int flag, int argc,
>
> U_BOOT_LONGHELP(sb,
> "handoff - Show handoff data received from SPL\n"
> + "sb map - Show mapped memory\n"
> "sb state - Show sandbox state");
>
> U_BOOT_CMD_WITH_SUBCMDS(sb, "Sandbox status commands", sb_help_text,
> U_BOOT_SUBCMD_MKENT(handoff, 1, 1, do_sb_handoff),
> + U_BOOT_SUBCMD_MKENT(map, 1, 1, do_sb_map),
> U_BOOT_SUBCMD_MKENT(state, 1, 1, do_sb_state));
> diff --git a/doc/usage/cmd/sb.rst b/doc/usage/cmd/sb.rst
> index 6f54f9d9eb7..37431aff7c8 100644
> --- a/doc/usage/cmd/sb.rst
> +++ b/doc/usage/cmd/sb.rst
> @@ -12,6 +12,7 @@ Synopsis
> ::
>
> sb handoff
> + sb map
> sb state
>
> Description
> @@ -26,6 +27,24 @@ sb handoff
> This shows information about any handoff information received from SPL. If
> U-Boot is started from an SPL build, it shows a valid magic number.
>
> +sb map
> +~~~~~~
> +
> +This shows any mappings between sandbox's emulated RAM and the underlying host
> +address-space.
> +
> +Fields shown are:
> +
> +Addr
> + Address in emulated RAM
> +
> +Mapping
> + Equivalent address in the host address-space. While sandbox requests address
> + ``0x10000000`` from the OS, this is not always available.
> +
> +Refcnt
> + Shows the number of references to this mapping.
> +
> sb state
> ~~~~~~~~
>
> @@ -42,6 +61,12 @@ as ``sandbox_spl``::
> => sb handoff
> SPL handoff magic 14f93c7b
>
> +This shows output from the *sb map* subcommand, with a single mapping::
> +
> + Sandbox memory-mapping
> + Addr Mapping Refcnt
> + ff000000 000056185b46d6d0 2
> +
> This shows output from the *sb state* subcommand::
>
> => sb state
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 06/15] sandbox: Add a way to show the sandbox memory-mapping
2024-10-29 9:59 ` Ilias Apalodimas
@ 2024-10-29 15:46 ` Simon Glass
0 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-29 15:46 UTC (permalink / raw)
To: Ilias Apalodimas; +Cc: U-Boot Mailing List, Tom Rini, Heinrich Schuchardt
Hi Ilias,
On Tue, 29 Oct 2024 at 10:59, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Simon
>
> This seems completely unrelated to the series.
>
> Please send it as a separate patch
I've moved the initial patches in this series to my queue in patchwork, for now.
Regards,
Simon
>
> Thanks
> /Ilias
>
> On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
> >
> > This is mostly hidden in the background, but it is sometimes useful to
> > look at it. Add a function to allow this.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > arch/sandbox/cpu/cpu.c | 13 +++++++++++++
> > arch/sandbox/include/asm/cpu.h | 3 +++
> > cmd/sb.c | 11 +++++++++++
> > doc/usage/cmd/sb.rst | 25 +++++++++++++++++++++++++
> > 4 files changed, 52 insertions(+)
> >
> > diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
> > index 06f8c13fab9..d1c4dcf0764 100644
> > --- a/arch/sandbox/cpu/cpu.c
> > +++ b/arch/sandbox/cpu/cpu.c
> > @@ -253,6 +253,19 @@ phys_addr_t map_to_sysmem(const void *ptr)
> > return mentry->tag;
> > }
> >
> > +void sandbox_map_list(void)
> > +{
> > + struct sandbox_mapmem_entry *mentry;
> > + struct sandbox_state *state = state_get_current();
> > +
> > + printf("Sandbox memory-mapping\n");
> > + printf("%8s %16s %6s\n", "Addr", "Mapping", "Refcnt");
> > + list_for_each_entry(mentry, &state->mapmem_head, sibling_node) {
> > + printf("%8lx %p %6d\n", mentry->tag, mentry->ptr,
> > + mentry->refcnt);
> > + }
> > +}
> > +
> > unsigned long sandbox_read(const void *addr, enum sandboxio_size_t size)
> > {
> > struct sandbox_state *state = state_get_current();
> > diff --git a/arch/sandbox/include/asm/cpu.h b/arch/sandbox/include/asm/cpu.h
> > index c97ac7ba95b..682bb3376d1 100644
> > --- a/arch/sandbox/include/asm/cpu.h
> > +++ b/arch/sandbox/include/asm/cpu.h
> > @@ -8,4 +8,7 @@
> >
> > void cpu_sandbox_set_current(const char *name);
> >
> > +/* show the mapping of sandbox addresses to pointers */
> > +void sandbox_map_list(void);
> > +
> > #endif /* __SANDBOX_CPU_H */
> > diff --git a/cmd/sb.c b/cmd/sb.c
> > index 9dbb53275b3..9245052492e 100644
> > --- a/cmd/sb.c
> > +++ b/cmd/sb.c
> > @@ -7,6 +7,7 @@
> > #include <command.h>
> > #include <dm.h>
> > #include <spl.h>
> > +#include <asm/cpu.h>
> > #include <asm/global_data.h>
> > #include <asm/state.h>
> >
> > @@ -29,6 +30,14 @@ static int do_sb_handoff(struct cmd_tbl *cmdtp, int flag, int argc,
> > #endif
> > }
> >
> > +static int do_sb_map(struct cmd_tbl *cmdtp, int flag, int argc,
> > + char *const argv[])
> > +{
> > + sandbox_map_list();
> > +
> > + return 0;
> > +}
> > +
> > static int do_sb_state(struct cmd_tbl *cmdtp, int flag, int argc,
> > char *const argv[])
> > {
> > @@ -42,8 +51,10 @@ static int do_sb_state(struct cmd_tbl *cmdtp, int flag, int argc,
> >
> > U_BOOT_LONGHELP(sb,
> > "handoff - Show handoff data received from SPL\n"
> > + "sb map - Show mapped memory\n"
> > "sb state - Show sandbox state");
> >
> > U_BOOT_CMD_WITH_SUBCMDS(sb, "Sandbox status commands", sb_help_text,
> > U_BOOT_SUBCMD_MKENT(handoff, 1, 1, do_sb_handoff),
> > + U_BOOT_SUBCMD_MKENT(map, 1, 1, do_sb_map),
> > U_BOOT_SUBCMD_MKENT(state, 1, 1, do_sb_state));
> > diff --git a/doc/usage/cmd/sb.rst b/doc/usage/cmd/sb.rst
> > index 6f54f9d9eb7..37431aff7c8 100644
> > --- a/doc/usage/cmd/sb.rst
> > +++ b/doc/usage/cmd/sb.rst
> > @@ -12,6 +12,7 @@ Synopsis
> > ::
> >
> > sb handoff
> > + sb map
> > sb state
> >
> > Description
> > @@ -26,6 +27,24 @@ sb handoff
> > This shows information about any handoff information received from SPL. If
> > U-Boot is started from an SPL build, it shows a valid magic number.
> >
> > +sb map
> > +~~~~~~
> > +
> > +This shows any mappings between sandbox's emulated RAM and the underlying host
> > +address-space.
> > +
> > +Fields shown are:
> > +
> > +Addr
> > + Address in emulated RAM
> > +
> > +Mapping
> > + Equivalent address in the host address-space. While sandbox requests address
> > + ``0x10000000`` from the OS, this is not always available.
> > +
> > +Refcnt
> > + Shows the number of references to this mapping.
> > +
> > sb state
> > ~~~~~~~~
> >
> > @@ -42,6 +61,12 @@ as ``sandbox_spl``::
> > => sb handoff
> > SPL handoff magic 14f93c7b
> >
> > +This shows output from the *sb map* subcommand, with a single mapping::
> > +
> > + Sandbox memory-mapping
> > + Addr Mapping Refcnt
> > + ff000000 000056185b46d6d0 2
> > +
> > This shows output from the *sb state* subcommand::
> >
> > => sb state
> > --
> > 2.43.0
> >
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 07/15] sandbox: Fix comment for nomap_sysmem() function
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
` (5 preceding siblings ...)
2024-10-28 12:47 ` [PATCH 06/15] sandbox: Add a way to show the sandbox memory-mapping Simon Glass
@ 2024-10-28 12:47 ` Simon Glass
2024-10-28 12:47 ` [PATCH 08/15] lmb: Drop extra 16KB of stack space Simon Glass
` (8 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:47 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass,
Igor Prusov
This should say 'cast' rather than 'case', so fix it.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
arch/sandbox/include/asm/io.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sandbox/include/asm/io.h b/arch/sandbox/include/asm/io.h
index a23bd64994a..3d09170063f 100644
--- a/arch/sandbox/include/asm/io.h
+++ b/arch/sandbox/include/asm/io.h
@@ -235,7 +235,7 @@ static inline void unmap_sysmem(const void *vaddr)
* nomap_sysmem() - pass through an address unchanged
*
* This is used to indicate an address which should NOT be mapped, e.g. in
- * SMBIOS tables. Using this function instead of a case shows that the sandbox
+ * SMBIOS tables. Using this function instead of a cast shows that the sandbox
* conversion has been done
*/
static inline void *nomap_sysmem(phys_addr_t paddr, unsigned long len)
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 08/15] lmb: Drop extra 16KB of stack space
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
` (6 preceding siblings ...)
2024-10-28 12:47 ` [PATCH 07/15] sandbox: Fix comment for nomap_sysmem() function Simon Glass
@ 2024-10-28 12:47 ` Simon Glass
2024-10-28 12:48 ` [PATCH 09/15] efi_loader: Fix free in ..._media_device_boot_option() Simon Glass
` (7 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:47 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass,
Sughosh Ganu
There is already a defined stack-size which is used to reserve space for
the stack. It is confusing to add more in the lmb module, since then the
memory map (with meminfo command) seems to have a hole in it.
Drop this unnecessary feature.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
lib/lmb.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/lib/lmb.c b/lib/lmb.c
index eec99c185ee..9d60865122f 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -219,8 +219,6 @@ static void lmb_reserve_uboot_region(void)
*/
debug("## Current stack ends at 0x%08lx ", (ulong)rsv_start);
- /* adjust sp by 16K to be safe */
- rsv_start -= SZ_16K;
for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
if (!gd->bd->bi_dram[bank].size ||
rsv_start < gd->bd->bi_dram[bank].start)
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 09/15] efi_loader: Fix free in ..._media_device_boot_option()
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
` (7 preceding siblings ...)
2024-10-28 12:47 ` [PATCH 08/15] lmb: Drop extra 16KB of stack space Simon Glass
@ 2024-10-28 12:48 ` Simon Glass
2024-10-29 10:01 ` Ilias Apalodimas
2024-10-28 12:48 ` [PATCH 10/15] efi_loader: Add support for logging EFI calls Simon Glass
` (6 subsequent siblings)
15 siblings, 1 reply; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:48 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass,
AKASHI Takahiro, Masahisa Kojima
Freeing a NULL pointer is an error in EFI, so check the pointer first,
before freeing it.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
lib/efi_loader/efi_bootmgr.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index a3aa2b8d1b9..431a38704e9 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -1180,7 +1180,8 @@ out:
free(opt[i].lo);
}
free(opt);
- efi_free_pool(handles);
+ if (handles)
+ efi_free_pool(handles);
if (ret == EFI_NOT_FOUND)
return EFI_SUCCESS;
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 09/15] efi_loader: Fix free in ..._media_device_boot_option()
2024-10-28 12:48 ` [PATCH 09/15] efi_loader: Fix free in ..._media_device_boot_option() Simon Glass
@ 2024-10-29 10:01 ` Ilias Apalodimas
2024-10-29 15:45 ` Simon Glass
0 siblings, 1 reply; 40+ messages in thread
From: Ilias Apalodimas @ 2024-10-29 10:01 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Tom Rini, Heinrich Schuchardt,
AKASHI Takahiro, Masahisa Kojima
Hi Simon,
On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
>
> Freeing a NULL pointer is an error in EFI, so check the pointer first,
> before freeing it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> lib/efi_loader/efi_bootmgr.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index a3aa2b8d1b9..431a38704e9 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -1180,7 +1180,8 @@ out:
> free(opt[i].lo);
> }
> free(opt);
> - efi_free_pool(handles);
> + if (handles)
> + efi_free_pool(handles);
We don't need this, efi_free_pool() checks the pointer already.
Thanks
/Ilias
>
> if (ret == EFI_NOT_FOUND)
> return EFI_SUCCESS;
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 09/15] efi_loader: Fix free in ..._media_device_boot_option()
2024-10-29 10:01 ` Ilias Apalodimas
@ 2024-10-29 15:45 ` Simon Glass
2024-10-29 22:13 ` Heinrich Schuchardt
0 siblings, 1 reply; 40+ messages in thread
From: Simon Glass @ 2024-10-29 15:45 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: U-Boot Mailing List, Tom Rini, Heinrich Schuchardt,
AKASHI Takahiro, Masahisa Kojima
Hi Ilias,
On Tue, 29 Oct 2024 at 11:02, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Simon,
>
> On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
> >
> > Freeing a NULL pointer is an error in EFI, so check the pointer first,
> > before freeing it.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > lib/efi_loader/efi_bootmgr.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> > index a3aa2b8d1b9..431a38704e9 100644
> > --- a/lib/efi_loader/efi_bootmgr.c
> > +++ b/lib/efi_loader/efi_bootmgr.c
> > @@ -1180,7 +1180,8 @@ out:
> > free(opt[i].lo);
> > }
> > free(opt);
> > - efi_free_pool(handles);
> > + if (handles)
> > + efi_free_pool(handles);
>
> We don't need this, efi_free_pool() checks the pointer already.
Yes, but it then returns an error (EFI_INVALID_PARAMETER), which gets
logged, with this series.
Regards,
Simon
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 09/15] efi_loader: Fix free in ..._media_device_boot_option()
2024-10-29 15:45 ` Simon Glass
@ 2024-10-29 22:13 ` Heinrich Schuchardt
2024-10-31 17:51 ` Simon Glass
0 siblings, 1 reply; 40+ messages in thread
From: Heinrich Schuchardt @ 2024-10-29 22:13 UTC (permalink / raw)
To: Simon Glass, Ilias Apalodimas
Cc: U-Boot Mailing List, Tom Rini, AKASHI Takahiro, Masahisa Kojima
Am 29. Oktober 2024 16:45:29 MEZ schrieb Simon Glass <sjg@chromium.org>:
>Hi Ilias,
>
>On Tue, 29 Oct 2024 at 11:02, Ilias Apalodimas
><ilias.apalodimas@linaro.org> wrote:
>>
>> Hi Simon,
>>
>> On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
>> >
>> > Freeing a NULL pointer is an error in EFI, so check the pointer first,
>> > before freeing it.
>> >
>> > Signed-off-by: Simon Glass <sjg@chromium.org>
>> > ---
>> >
>> > lib/efi_loader/efi_bootmgr.c | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>> > index a3aa2b8d1b9..431a38704e9 100644
>> > --- a/lib/efi_loader/efi_bootmgr.c
>> > +++ b/lib/efi_loader/efi_bootmgr.c
>> > @@ -1180,7 +1180,8 @@ out:
>> > free(opt[i].lo);
>> > }
>> > free(opt);
>> > - efi_free_pool(handles);
>> > + if (handles)
>> > + efi_free_pool(handles);
>>
>> We don't need this, efi_free_pool() checks the pointer already.
>
>Yes, but it then returns an error (EFI_INVALID_PARAMETER), which gets
>logged, with this series.
So this is not a problem of the existing code but of your patch series which creates a superfluous log message.
Best regards
Heinrich
>
>Regards,
>Simon
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 09/15] efi_loader: Fix free in ..._media_device_boot_option()
2024-10-29 22:13 ` Heinrich Schuchardt
@ 2024-10-31 17:51 ` Simon Glass
0 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-31 17:51 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Ilias Apalodimas, U-Boot Mailing List, Tom Rini, AKASHI Takahiro,
Masahisa Kojima
Hi Heinrich,
On Tue, 29 Oct 2024 at 23:13, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> Am 29. Oktober 2024 16:45:29 MEZ schrieb Simon Glass <sjg@chromium.org>:
> >Hi Ilias,
> >
> >On Tue, 29 Oct 2024 at 11:02, Ilias Apalodimas
> ><ilias.apalodimas@linaro.org> wrote:
> >>
> >> Hi Simon,
> >>
> >> On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
> >> >
> >> > Freeing a NULL pointer is an error in EFI, so check the pointer first,
> >> > before freeing it.
> >> >
> >> > Signed-off-by: Simon Glass <sjg@chromium.org>
> >> > ---
> >> >
> >> > lib/efi_loader/efi_bootmgr.c | 3 ++-
> >> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> >> > index a3aa2b8d1b9..431a38704e9 100644
> >> > --- a/lib/efi_loader/efi_bootmgr.c
> >> > +++ b/lib/efi_loader/efi_bootmgr.c
> >> > @@ -1180,7 +1180,8 @@ out:
> >> > free(opt[i].lo);
> >> > }
> >> > free(opt);
> >> > - efi_free_pool(handles);
> >> > + if (handles)
> >> > + efi_free_pool(handles);
> >>
> >> We don't need this, efi_free_pool() checks the pointer already.
> >
> >Yes, but it then returns an error (EFI_INVALID_PARAMETER), which gets
> >logged, with this series.
>
> So this is not a problem of the existing code but of your patch series which creates a superfluous log message.
Is it an error to free a zero pointer in EFI?
Regards,
Simon
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 10/15] efi_loader: Add support for logging EFI calls
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
` (8 preceding siblings ...)
2024-10-28 12:48 ` [PATCH 09/15] efi_loader: Fix free in ..._media_device_boot_option() Simon Glass
@ 2024-10-28 12:48 ` Simon Glass
2024-10-29 11:44 ` Heinrich Schuchardt
2024-10-29 22:19 ` Heinrich Schuchardt
2024-10-28 12:48 ` [PATCH 11/15] efi_loader: Create the log on startup Simon Glass
` (5 subsequent siblings)
15 siblings, 2 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:48 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass,
AKASHI Takahiro, Caleb Connolly, Levi Yun, Marek Vasut,
Mattijs Korpershoek, Michal Simek, Oliver Gaskell,
Patrick Rudolph, Raymond Mao, Robert Marko, Sam Protsenko,
Sumit Garg, Vincent Stehlé
The current logging system suffers from some disadvantages, mainly that
it writes its output to the console and cannot be easily reviewed.
Add a dedicated log, storing records in a binary format and including
the result codes and any return values from each call. The log is built
sequentially in memory and can be reviewed after any EFI operation. It
could potentially be written to media for later review, but that is not
implemented so far.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
MAINTAINERS | 6 +
include/bloblist.h | 1 +
include/efi.h | 1 +
include/efi_log.h | 169 ++++++++++++++++++++++++++
lib/efi_loader/Kconfig | 19 +++
lib/efi_loader/Makefile | 1 +
lib/efi_loader/efi_log.c | 256 +++++++++++++++++++++++++++++++++++++++
7 files changed, 453 insertions(+)
create mode 100644 include/efi_log.h
create mode 100644 lib/efi_loader/efi_log.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 38c714cf46a..9ade0ca4bc3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1067,6 +1067,12 @@ F: lib/efi/efi_app.c
F: scripts/build-efi.sh
F: test/dm/efi_media.c
+EFI LOGGING
+M: Simon Glass <sjg@chromium.org>
+S: Maintained
+F: include/efi_log.h
+F: lib/efi_loader/efi_log.c
+
EFI PAYLOAD
M: Heinrich Schuchardt <xypron.glpk@gmx.de>
M: Ilias Apalodimas <ilias.apalodimas@linaro.org>
diff --git a/include/bloblist.h b/include/bloblist.h
index ff32d3fecfd..1e1ca34aa92 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -153,6 +153,7 @@ enum bloblist_tag_t {
BLOBLISTT_U_BOOT_SPL_HANDOFF = 0xfff000, /* Hand-off info from SPL */
BLOBLISTT_VBE = 0xfff001, /* VBE per-phase state */
BLOBLISTT_U_BOOT_VIDEO = 0xfff002, /* Video info from SPL */
+ BLOBLISTT_EFI_LOG = 0xfff003, /* Log of EFI calls */
};
/**
diff --git a/include/efi.h b/include/efi.h
index 84640cf7b25..f7419b80d4d 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -127,6 +127,7 @@ static inline void *guidcpy(void *dst, const void *src)
#define EFI_COMPROMISED_DATA (EFI_ERROR_MASK | 33)
#define EFI_IP_ADDRESS_CONFLICT (EFI_ERROR_MASK | 34)
#define EFI_HTTP_ERROR (EFI_ERROR_MASK | 35)
+#define EFI_ERROR_COUNT 36
#define EFI_WARN_UNKNOWN_GLYPH 1
#define EFI_WARN_DELETE_FAILURE 2
diff --git a/include/efi_log.h b/include/efi_log.h
new file mode 100644
index 00000000000..1988e5f9df0
--- /dev/null
+++ b/include/efi_log.h
@@ -0,0 +1,169 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Logging (to memory) of calls from an EFI app
+ *
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#ifndef __EFI_LOG_H
+#define __EFI_LOG_H
+
+#include <linux/types.h>
+#include <efi.h>
+
+/**
+ * enum efil_tag - Types of logging records which can be created
+ */
+enum efil_tag {
+ EFILT_TESTING,
+
+ EFILT_COUNT,
+};
+
+/**
+ * struct efil_rec_hdr - Header for each logging record
+ *
+ * @tag: Tag which indicates the type of the record
+ * @size: Size of the record in bytes
+ * @ended: true if record has been completed (i.e. the function returned), false
+ * if it is still pending
+ * @e_ret: Records the return function from the logged function
+ */
+struct efil_rec_hdr {
+ enum efil_tag tag;
+ int size;
+ bool ended;
+ efi_status_t e_ret;
+};
+
+/**
+ * struct efil_hdr - Holds the header for the log
+ *
+ * @upto: Offset at which to store the next log record
+ * @size: Total size of the log in bytes
+ */
+struct efil_hdr {
+ int upto;
+ int size;
+};
+
+enum efil_test_t {
+ EFI_LOG_TEST0,
+ EFI_LOG_TEST1,
+
+ EFI_LOG_TEST_COUNT,
+};
+
+/**
+ * struct efil_testing - used for testing the log
+ */
+struct efil_testing {
+ enum efil_test_t enum_val;
+ efi_uintn_t int_val;
+ u64 *memory;
+ void **buffer;
+ u64 e_memory;
+ void *e_buffer;
+};
+
+/**
+ * struct efil_allocate_pages - holds info from efi_allocate_pages() call
+ *
+ * @e_memory: Contains the value of *@memory on return from the EFI function
+ */
+struct efil_allocate_pages {
+ enum efi_allocate_type type;
+ enum efi_memory_type memory_type;
+ efi_uintn_t pages;
+ u64 *memory;
+ u64 e_memory;
+};
+
+/*
+ * The functions below are in pairs, with a 'start' and 'end' call for each EFI
+ * function. The 'start' function (efi_logs_...) is called when the function is
+ * started. It records all the arguments. The 'end' function (efi_loge_...) is
+ * called when the function is ready to return. It records any output arguments
+ * as well as the return value.
+ *
+ * The start function returns the offset of the log record. This must be passed
+ * to the end function, so it can add the status code and any other useful
+ * information. It is not possible for the end functions to remember the offset
+ * from the associated start function, since EFI functions may be called in a
+ * nested way and there is no obvious way to determine the log record to which
+ * the end function refers.
+ *
+ * If the start function returns an error code (i.e. an offset < 0) then it is
+ * safe to pass that to the end function. It will simply ignore the operation.
+ * Common errors are -ENOENT if there is no log and -ENOSPC if the log is full
+ */
+
+#if CONFIG_IS_ENABLED(EFI_LOG)
+
+/**
+ * efi_logs_testing() - Record a test call to an efi function
+ *
+ * @enum_val: enum value
+ * @int_val: integer value
+ * @buffer: place to write pointer address
+ * @memory: place to write memory address
+ * Return: log-offset of this new record, or -ve error code
+ */
+int efi_logs_testing(enum efil_test_t enum_val, efi_uintn_t int_value,
+ void *buffer, u64 *memory);
+
+/**
+ * efi_loge_testing() - Record a return from a test call
+ *
+ * This stores the value of the pointers also
+ *
+ * ofs: Offset of the record to end
+ * efi_ret: status code to record
+ */
+int efi_loge_testing(int ofs, efi_status_t efi_ret);
+
+#else /* !EFI_LOG */
+
+static inline int efi_logs_testing(enum efil_test_t enum_val,
+ efi_uintn_t int_value, void *buffer,
+ u64 *memory)
+{
+ return -ENOSYS;
+}
+
+static inline int efi_loge_testing(int ofs, efi_status_t efi_ret)
+{
+ return -ENOSYS;
+}
+
+#endif /* EFI_LOG */
+
+/* below are some general functions */
+
+/**
+ * efi_log_show() - Show the EFI log
+ *
+ * Displays the log of EFI boot-services calls which are so-far enabled for
+ * logging
+ *
+ * Return: 0 on success, or -ve error code
+ */
+int efi_log_show(void);
+
+/**
+ * efi_log_reset() - Reset the log, erasing all records
+ *
+ * Return 0 if OK, -ENOENT if the log could not be found
+
+ */
+int efi_log_reset(void);
+
+/**
+ * efi_log_init() - Create a log in the bloblist, then reset it
+ *
+ * Return 0 if OK, -ENOMEM if the bloblist is not large enough
+ */
+int efi_log_init(void);
+
+#endif /* __EFI_LOG_H */
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 69b2c9360d8..37572c82f54 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -531,6 +531,25 @@ config EFI_SCROLL_ON_CLEAR_SCREEN
to capture complete boot logs (except for interactive menus etc.)
and can ease debugging related issues.
+config EFI_LOG
+ bool "Enable logging of EFI operations"
+ select BLOBLIST
+ help
+ This enables maintaining a log of EFI boot-time services, useful for
+ debugging. It keeps track of some of the calls which are made, so far
+ just those related to memory allocation.
+
+config EFI_LOG_SIZE
+ hex "Size of EFI log"
+ depends on EFI_LOG
+ default 0x4000
+ help
+ Sets the size of the EFI log in bytes. The log is stored in the
+ bloblist so if its size is insufficient, U-Boot will raise an error.
+
+ The amount of space needed depends on the EFI app being run. When
+ space runes out, further EFI calls will not be logged.
+
endmenu
menu "EFI bootmanager"
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 00d18966f9e..8ec240cb864 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -38,6 +38,7 @@ obj-y += efi_file.o
obj-$(CONFIG_EFI_LOADER_HII) += efi_hii.o
obj-y += efi_image_loader.o
obj-y += efi_load_options.o
+obj-$(CONFIG_EFI_LOG) += efi_log.o
obj-y += efi_memory.o
obj-y += efi_root_node.o
obj-y += efi_runtime.o
diff --git a/lib/efi_loader/efi_log.c b/lib/efi_loader/efi_log.c
new file mode 100644
index 00000000000..01e495d3995
--- /dev/null
+++ b/lib/efi_loader/efi_log.c
@@ -0,0 +1,256 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Logging (to memory) of calls from an EFI app
+ *
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#define LOG_CATEGORY LOGC_EFI
+
+#include <bloblist.h>
+#include <efi_log.h>
+#include <errno.h>
+#include <log.h>
+
+/* names for enum efil_tag (abbreviated to keep output to a single line) */
+static const char *tag_name[EFILT_COUNT] = {
+ "testing",
+};
+
+/* names for error codes, trying to keep them short */
+static const char *error_name[EFI_ERROR_COUNT] = {
+ "OK",
+ "load",
+ "inval_param",
+ "unsupported",
+ "bad_buf_sz",
+ "buf_small",
+ "not_ready",
+ "device",
+ "write_prot",
+ "out_of_rsrc",
+ "vol_corrupt",
+ "vol_full",
+ "no_media",
+ "media_chg",
+ "not_found",
+ "no access",
+ "no_response",
+ "no_mapping",
+ "timeout",
+ "not_started",
+ "already",
+ "aborted",
+ "icmp",
+ "tftp",
+ "protocol",
+ "bad version",
+ "sec_violate",
+ "crc_error",
+ "end_media",
+ "end_file",
+ "inval_lang",
+ "compromised",
+ "ipaddr_busy",
+ "http",
+};
+
+static const char *test_enum_name[EFI_LOG_TEST_COUNT] = {
+ "test0",
+ "test1",
+};
+
+/**
+ * prep_rec() - prepare a new record in the log
+ *
+ * This creates a new record at the next available position, setting it up ready
+ * to hold data. The size and tag are set up.
+ *
+ * The log is updated so that the next record will start after this one
+ *
+ * @tag: tag of the EFI call to record
+ * @size: Number of bytes in the caller's struct
+ * @recp: Set to point to where the caller should add its data
+ * Return: Offset of this record (must be passed to finish_rec())
+ */
+static int prep_rec(enum efil_tag tag, uint str_size, void **recp)
+{
+ struct efil_hdr *hdr = bloblist_find(BLOBLISTT_EFI_LOG, 0);
+ struct efil_rec_hdr *rec_hdr;
+ int ofs, size;
+
+ if (!hdr)
+ return -ENOENT;
+ size = str_size + sizeof(struct efil_rec_hdr);
+ if (hdr->upto + size > hdr->size)
+ return -ENOSPC;
+
+ rec_hdr = (void *)hdr + hdr->upto;
+ rec_hdr->size = size;
+ rec_hdr->tag = tag;
+ rec_hdr->ended = false;
+ *recp = rec_hdr + 1;
+
+ ofs = hdr->upto;
+ hdr->upto += size;
+
+ return ofs;
+}
+
+/**
+ * finish_rec() - Finish a previously started record
+ *
+ * @ofs: Offset of record to finish
+ * @ret: Return code which is to be returned from the EFI function
+ * Return: Pointer to the structure where the caller should add its data
+ */
+static void *finish_rec(int ofs, efi_status_t ret)
+{
+ struct efil_hdr *hdr = bloblist_find(BLOBLISTT_EFI_LOG, 0);
+ struct efil_rec_hdr *rec_hdr;
+
+ if (!hdr || ofs < 0)
+ return NULL;
+ rec_hdr = (void *)hdr + ofs;
+ rec_hdr->ended = true;
+ rec_hdr->e_ret = ret;
+
+ return rec_hdr + 1;
+}
+
+int efi_logs_testing(enum efil_test_t enum_val, efi_uintn_t int_val,
+ void *buffer, u64 *memory)
+{
+ struct efil_testing *rec;
+ int ret;
+
+ ret = prep_rec(EFILT_TESTING, sizeof(*rec), (void **)&rec);
+ if (ret < 0)
+ return ret;
+
+ rec->int_val = int_val;
+ rec->buffer = buffer;
+ rec->memory = memory;
+ rec->e_buffer = NULL;
+ rec->e_memory = 0;
+
+ return ret;
+}
+
+int efi_loge_testing(int ofs, efi_status_t efi_ret)
+{
+ struct efil_testing *rec;
+
+ rec = finish_rec(ofs, efi_ret);
+ if (!rec)
+ return -ENOSPC;
+ rec->e_memory = *rec->memory;
+ rec->e_buffer = *rec->buffer;
+
+ return 0;
+}
+
+static void show_enum(const char *type_name[], int type)
+{
+ printf("%s ", type_name[type]);
+}
+
+static void show_ulong(const char *prompt, ulong val)
+{
+ printf("%s %lx", prompt, val);
+ if (val >= 10)
+ printf("/%ld", val);
+ printf(" ");
+}
+
+static void show_addr(const char *prompt, ulong addr)
+{
+ printf("%s %lx ", prompt, addr);
+}
+
+static void show_ret(efi_status_t ret)
+{
+ int code;
+
+ code = ret & ~EFI_ERROR_MASK;
+ if (code < ARRAY_SIZE(error_name))
+ printf("ret %s", error_name[ret]);
+ else
+ printf("ret %lx", ret);
+}
+
+void show_rec(int seq, struct efil_rec_hdr *rec_hdr)
+{
+ void *start = (void *)rec_hdr + sizeof(struct efil_rec_hdr);
+
+ printf("%3d %12s ", seq, tag_name[rec_hdr->tag]);
+ switch (rec_hdr->tag) {
+ case EFILT_TESTING: {
+ struct efil_testing *rec = start;
+
+ show_enum(test_enum_name, (int)rec->enum_val);
+ show_ulong("int", (ulong)rec->int_val);
+ show_addr("buf", map_to_sysmem(rec->buffer));
+ show_addr("mem", map_to_sysmem(rec->memory));
+ if (rec_hdr->ended) {
+ show_addr("*buf",
+ (ulong)map_to_sysmem((void *)rec->e_buffer));
+ show_addr("*mem",
+ (ulong)rec->e_memory);
+ show_ret(rec_hdr->e_ret);
+ }
+ }
+ case EFILT_COUNT:
+ break;
+ }
+ printf("\n");
+}
+
+int efi_log_show(void)
+{
+ struct efil_hdr *hdr = bloblist_find(BLOBLISTT_EFI_LOG, 0);
+ struct efil_rec_hdr *rec_hdr;
+ int i;
+
+ printf("EFI log (size %x)\n", hdr->upto);
+ if (!hdr)
+ return -ENOENT;
+ for (i = 0, rec_hdr = (void *)hdr + sizeof(*hdr);
+ (void *)rec_hdr - (void *)hdr < hdr->upto;
+ i++, rec_hdr = (void *)rec_hdr + rec_hdr->size)
+ show_rec(i, rec_hdr);
+ printf("%d records\n", i);
+
+ return 0;
+}
+
+int efi_log_reset(void)
+{
+ struct efil_hdr *hdr = bloblist_find(BLOBLISTT_EFI_LOG, 0);
+
+ if (!hdr)
+ return -ENOENT;
+ hdr->upto = sizeof(struct efil_hdr);
+ hdr->size = CONFIG_EFI_LOG_SIZE;
+
+ return 0;
+}
+
+int efi_log_init(void)
+{
+ struct efil_hdr *hdr;
+
+ hdr = bloblist_add(BLOBLISTT_EFI_LOG, CONFIG_EFI_LOG_SIZE, 0);
+ if (!hdr) {
+ /*
+ * Return -ENOMEM since we use -ENOSPC to mean that the log is
+ * full
+ */
+ log_warning("Failed to setup EFI log\n");
+ return log_msg_ret("eli", -ENOMEM);
+ }
+ efi_log_reset();
+
+ return 0;
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 10/15] efi_loader: Add support for logging EFI calls
2024-10-28 12:48 ` [PATCH 10/15] efi_loader: Add support for logging EFI calls Simon Glass
@ 2024-10-29 11:44 ` Heinrich Schuchardt
2024-10-29 22:19 ` Heinrich Schuchardt
1 sibling, 0 replies; 40+ messages in thread
From: Heinrich Schuchardt @ 2024-10-29 11:44 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, AKASHI Takahiro, Caleb Connolly,
Levi Yun, Marek Vasut, Mattijs Korpershoek, Michal Simek,
Oliver Gaskell, Patrick Rudolph, Raymond Mao, Robert Marko,
Sam Protsenko, Sumit Garg, Vincent Stehlé
Am 28. Oktober 2024 13:48:01 MEZ schrieb Simon Glass <sjg@chromium.org>:
>The current logging system suffers from some disadvantages, mainly that
>it writes its output to the console and cannot be easily reviewed.
>
>Add a dedicated log, storing records in a binary format and including
>the result codes and any return values from each call. The log is built
>sequentially in memory and can be reviewed after any EFI operation. It
>could potentially be written to media for later review, but that is not
>implemented so far.
We already have log_debug.
We should use it in EFI_ENTRY, EFI_PRINT, EFI_EXIT.
The missing piece might be a log driver to save the log wherever you want it for tests.
Best regards
Heinrich
>
>Signed-off-by: Simon Glass <sjg@chromium.org>
>---
>
> MAINTAINERS | 6 +
> include/bloblist.h | 1 +
> include/efi.h | 1 +
> include/efi_log.h | 169 ++++++++++++++++++++++++++
> lib/efi_loader/Kconfig | 19 +++
> lib/efi_loader/Makefile | 1 +
> lib/efi_loader/efi_log.c | 256 +++++++++++++++++++++++++++++++++++++++
> 7 files changed, 453 insertions(+)
> create mode 100644 include/efi_log.h
> create mode 100644 lib/efi_loader/efi_log.c
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 38c714cf46a..9ade0ca4bc3 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -1067,6 +1067,12 @@ F: lib/efi/efi_app.c
> F: scripts/build-efi.sh
> F: test/dm/efi_media.c
>
>+EFI LOGGING
>+M: Simon Glass <sjg@chromium.org>
>+S: Maintained
>+F: include/efi_log.h
>+F: lib/efi_loader/efi_log.c
>+
> EFI PAYLOAD
> M: Heinrich Schuchardt <xypron.glpk@gmx.de>
> M: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>diff --git a/include/bloblist.h b/include/bloblist.h
>index ff32d3fecfd..1e1ca34aa92 100644
>--- a/include/bloblist.h
>+++ b/include/bloblist.h
>@@ -153,6 +153,7 @@ enum bloblist_tag_t {
> BLOBLISTT_U_BOOT_SPL_HANDOFF = 0xfff000, /* Hand-off info from SPL */
> BLOBLISTT_VBE = 0xfff001, /* VBE per-phase state */
> BLOBLISTT_U_BOOT_VIDEO = 0xfff002, /* Video info from SPL */
>+ BLOBLISTT_EFI_LOG = 0xfff003, /* Log of EFI calls */
> };
>
> /**
>diff --git a/include/efi.h b/include/efi.h
>index 84640cf7b25..f7419b80d4d 100644
>--- a/include/efi.h
>+++ b/include/efi.h
>@@ -127,6 +127,7 @@ static inline void *guidcpy(void *dst, const void *src)
> #define EFI_COMPROMISED_DATA (EFI_ERROR_MASK | 33)
> #define EFI_IP_ADDRESS_CONFLICT (EFI_ERROR_MASK | 34)
> #define EFI_HTTP_ERROR (EFI_ERROR_MASK | 35)
>+#define EFI_ERROR_COUNT 36
>
> #define EFI_WARN_UNKNOWN_GLYPH 1
> #define EFI_WARN_DELETE_FAILURE 2
>diff --git a/include/efi_log.h b/include/efi_log.h
>new file mode 100644
>index 00000000000..1988e5f9df0
>--- /dev/null
>+++ b/include/efi_log.h
>@@ -0,0 +1,169 @@
>+/* SPDX-License-Identifier: GPL-2.0+ */
>+/*
>+ * Logging (to memory) of calls from an EFI app
>+ *
>+ * Copyright 2024 Google LLC
>+ * Written by Simon Glass <sjg@chromium.org>
>+ */
>+
>+#ifndef __EFI_LOG_H
>+#define __EFI_LOG_H
>+
>+#include <linux/types.h>
>+#include <efi.h>
>+
>+/**
>+ * enum efil_tag - Types of logging records which can be created
>+ */
>+enum efil_tag {
>+ EFILT_TESTING,
>+
>+ EFILT_COUNT,
>+};
>+
>+/**
>+ * struct efil_rec_hdr - Header for each logging record
>+ *
>+ * @tag: Tag which indicates the type of the record
>+ * @size: Size of the record in bytes
>+ * @ended: true if record has been completed (i.e. the function returned), false
>+ * if it is still pending
>+ * @e_ret: Records the return function from the logged function
>+ */
>+struct efil_rec_hdr {
>+ enum efil_tag tag;
>+ int size;
>+ bool ended;
>+ efi_status_t e_ret;
>+};
>+
>+/**
>+ * struct efil_hdr - Holds the header for the log
>+ *
>+ * @upto: Offset at which to store the next log record
>+ * @size: Total size of the log in bytes
>+ */
>+struct efil_hdr {
>+ int upto;
>+ int size;
>+};
>+
>+enum efil_test_t {
>+ EFI_LOG_TEST0,
>+ EFI_LOG_TEST1,
>+
>+ EFI_LOG_TEST_COUNT,
>+};
>+
>+/**
>+ * struct efil_testing - used for testing the log
>+ */
>+struct efil_testing {
>+ enum efil_test_t enum_val;
>+ efi_uintn_t int_val;
>+ u64 *memory;
>+ void **buffer;
>+ u64 e_memory;
>+ void *e_buffer;
>+};
>+
>+/**
>+ * struct efil_allocate_pages - holds info from efi_allocate_pages() call
>+ *
>+ * @e_memory: Contains the value of *@memory on return from the EFI function
>+ */
>+struct efil_allocate_pages {
>+ enum efi_allocate_type type;
>+ enum efi_memory_type memory_type;
>+ efi_uintn_t pages;
>+ u64 *memory;
>+ u64 e_memory;
>+};
>+
>+/*
>+ * The functions below are in pairs, with a 'start' and 'end' call for each EFI
>+ * function. The 'start' function (efi_logs_...) is called when the function is
>+ * started. It records all the arguments. The 'end' function (efi_loge_...) is
>+ * called when the function is ready to return. It records any output arguments
>+ * as well as the return value.
>+ *
>+ * The start function returns the offset of the log record. This must be passed
>+ * to the end function, so it can add the status code and any other useful
>+ * information. It is not possible for the end functions to remember the offset
>+ * from the associated start function, since EFI functions may be called in a
>+ * nested way and there is no obvious way to determine the log record to which
>+ * the end function refers.
>+ *
>+ * If the start function returns an error code (i.e. an offset < 0) then it is
>+ * safe to pass that to the end function. It will simply ignore the operation.
>+ * Common errors are -ENOENT if there is no log and -ENOSPC if the log is full
>+ */
>+
>+#if CONFIG_IS_ENABLED(EFI_LOG)
>+
>+/**
>+ * efi_logs_testing() - Record a test call to an efi function
>+ *
>+ * @enum_val: enum value
>+ * @int_val: integer value
>+ * @buffer: place to write pointer address
>+ * @memory: place to write memory address
>+ * Return: log-offset of this new record, or -ve error code
>+ */
>+int efi_logs_testing(enum efil_test_t enum_val, efi_uintn_t int_value,
>+ void *buffer, u64 *memory);
>+
>+/**
>+ * efi_loge_testing() - Record a return from a test call
>+ *
>+ * This stores the value of the pointers also
>+ *
>+ * ofs: Offset of the record to end
>+ * efi_ret: status code to record
>+ */
>+int efi_loge_testing(int ofs, efi_status_t efi_ret);
>+
>+#else /* !EFI_LOG */
>+
>+static inline int efi_logs_testing(enum efil_test_t enum_val,
>+ efi_uintn_t int_value, void *buffer,
>+ u64 *memory)
>+{
>+ return -ENOSYS;
>+}
>+
>+static inline int efi_loge_testing(int ofs, efi_status_t efi_ret)
>+{
>+ return -ENOSYS;
>+}
>+
>+#endif /* EFI_LOG */
>+
>+/* below are some general functions */
>+
>+/**
>+ * efi_log_show() - Show the EFI log
>+ *
>+ * Displays the log of EFI boot-services calls which are so-far enabled for
>+ * logging
>+ *
>+ * Return: 0 on success, or -ve error code
>+ */
>+int efi_log_show(void);
>+
>+/**
>+ * efi_log_reset() - Reset the log, erasing all records
>+ *
>+ * Return 0 if OK, -ENOENT if the log could not be found
>+
>+ */
>+int efi_log_reset(void);
>+
>+/**
>+ * efi_log_init() - Create a log in the bloblist, then reset it
>+ *
>+ * Return 0 if OK, -ENOMEM if the bloblist is not large enough
>+ */
>+int efi_log_init(void);
>+
>+#endif /* __EFI_LOG_H */
>diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>index 69b2c9360d8..37572c82f54 100644
>--- a/lib/efi_loader/Kconfig
>+++ b/lib/efi_loader/Kconfig
>@@ -531,6 +531,25 @@ config EFI_SCROLL_ON_CLEAR_SCREEN
> to capture complete boot logs (except for interactive menus etc.)
> and can ease debugging related issues.
>
>+config EFI_LOG
>+ bool "Enable logging of EFI operations"
>+ select BLOBLIST
>+ help
>+ This enables maintaining a log of EFI boot-time services, useful for
>+ debugging. It keeps track of some of the calls which are made, so far
>+ just those related to memory allocation.
>+
>+config EFI_LOG_SIZE
>+ hex "Size of EFI log"
>+ depends on EFI_LOG
>+ default 0x4000
>+ help
>+ Sets the size of the EFI log in bytes. The log is stored in the
>+ bloblist so if its size is insufficient, U-Boot will raise an error.
>+
>+ The amount of space needed depends on the EFI app being run. When
>+ space runes out, further EFI calls will not be logged.
>+
> endmenu
>
> menu "EFI bootmanager"
>diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>index 00d18966f9e..8ec240cb864 100644
>--- a/lib/efi_loader/Makefile
>+++ b/lib/efi_loader/Makefile
>@@ -38,6 +38,7 @@ obj-y += efi_file.o
> obj-$(CONFIG_EFI_LOADER_HII) += efi_hii.o
> obj-y += efi_image_loader.o
> obj-y += efi_load_options.o
>+obj-$(CONFIG_EFI_LOG) += efi_log.o
> obj-y += efi_memory.o
> obj-y += efi_root_node.o
> obj-y += efi_runtime.o
>diff --git a/lib/efi_loader/efi_log.c b/lib/efi_loader/efi_log.c
>new file mode 100644
>index 00000000000..01e495d3995
>--- /dev/null
>+++ b/lib/efi_loader/efi_log.c
>@@ -0,0 +1,256 @@
>+// SPDX-License-Identifier: GPL-2.0+
>+/*
>+ * Logging (to memory) of calls from an EFI app
>+ *
>+ * Copyright 2024 Google LLC
>+ * Written by Simon Glass <sjg@chromium.org>
>+ */
>+
>+#define LOG_CATEGORY LOGC_EFI
>+
>+#include <bloblist.h>
>+#include <efi_log.h>
>+#include <errno.h>
>+#include <log.h>
>+
>+/* names for enum efil_tag (abbreviated to keep output to a single line) */
>+static const char *tag_name[EFILT_COUNT] = {
>+ "testing",
>+};
>+
>+/* names for error codes, trying to keep them short */
>+static const char *error_name[EFI_ERROR_COUNT] = {
>+ "OK",
>+ "load",
>+ "inval_param",
>+ "unsupported",
>+ "bad_buf_sz",
>+ "buf_small",
>+ "not_ready",
>+ "device",
>+ "write_prot",
>+ "out_of_rsrc",
>+ "vol_corrupt",
>+ "vol_full",
>+ "no_media",
>+ "media_chg",
>+ "not_found",
>+ "no access",
>+ "no_response",
>+ "no_mapping",
>+ "timeout",
>+ "not_started",
>+ "already",
>+ "aborted",
>+ "icmp",
>+ "tftp",
>+ "protocol",
>+ "bad version",
>+ "sec_violate",
>+ "crc_error",
>+ "end_media",
>+ "end_file",
>+ "inval_lang",
>+ "compromised",
>+ "ipaddr_busy",
>+ "http",
>+};
>+
>+static const char *test_enum_name[EFI_LOG_TEST_COUNT] = {
>+ "test0",
>+ "test1",
>+};
>+
>+/**
>+ * prep_rec() - prepare a new record in the log
>+ *
>+ * This creates a new record at the next available position, setting it up ready
>+ * to hold data. The size and tag are set up.
>+ *
>+ * The log is updated so that the next record will start after this one
>+ *
>+ * @tag: tag of the EFI call to record
>+ * @size: Number of bytes in the caller's struct
>+ * @recp: Set to point to where the caller should add its data
>+ * Return: Offset of this record (must be passed to finish_rec())
>+ */
>+static int prep_rec(enum efil_tag tag, uint str_size, void **recp)
>+{
>+ struct efil_hdr *hdr = bloblist_find(BLOBLISTT_EFI_LOG, 0);
>+ struct efil_rec_hdr *rec_hdr;
>+ int ofs, size;
>+
>+ if (!hdr)
>+ return -ENOENT;
>+ size = str_size + sizeof(struct efil_rec_hdr);
>+ if (hdr->upto + size > hdr->size)
>+ return -ENOSPC;
>+
>+ rec_hdr = (void *)hdr + hdr->upto;
>+ rec_hdr->size = size;
>+ rec_hdr->tag = tag;
>+ rec_hdr->ended = false;
>+ *recp = rec_hdr + 1;
>+
>+ ofs = hdr->upto;
>+ hdr->upto += size;
>+
>+ return ofs;
>+}
>+
>+/**
>+ * finish_rec() - Finish a previously started record
>+ *
>+ * @ofs: Offset of record to finish
>+ * @ret: Return code which is to be returned from the EFI function
>+ * Return: Pointer to the structure where the caller should add its data
>+ */
>+static void *finish_rec(int ofs, efi_status_t ret)
>+{
>+ struct efil_hdr *hdr = bloblist_find(BLOBLISTT_EFI_LOG, 0);
>+ struct efil_rec_hdr *rec_hdr;
>+
>+ if (!hdr || ofs < 0)
>+ return NULL;
>+ rec_hdr = (void *)hdr + ofs;
>+ rec_hdr->ended = true;
>+ rec_hdr->e_ret = ret;
>+
>+ return rec_hdr + 1;
>+}
>+
>+int efi_logs_testing(enum efil_test_t enum_val, efi_uintn_t int_val,
>+ void *buffer, u64 *memory)
>+{
>+ struct efil_testing *rec;
>+ int ret;
>+
>+ ret = prep_rec(EFILT_TESTING, sizeof(*rec), (void **)&rec);
>+ if (ret < 0)
>+ return ret;
>+
>+ rec->int_val = int_val;
>+ rec->buffer = buffer;
>+ rec->memory = memory;
>+ rec->e_buffer = NULL;
>+ rec->e_memory = 0;
>+
>+ return ret;
>+}
>+
>+int efi_loge_testing(int ofs, efi_status_t efi_ret)
>+{
>+ struct efil_testing *rec;
>+
>+ rec = finish_rec(ofs, efi_ret);
>+ if (!rec)
>+ return -ENOSPC;
>+ rec->e_memory = *rec->memory;
>+ rec->e_buffer = *rec->buffer;
>+
>+ return 0;
>+}
>+
>+static void show_enum(const char *type_name[], int type)
>+{
>+ printf("%s ", type_name[type]);
>+}
>+
>+static void show_ulong(const char *prompt, ulong val)
>+{
>+ printf("%s %lx", prompt, val);
>+ if (val >= 10)
>+ printf("/%ld", val);
>+ printf(" ");
>+}
>+
>+static void show_addr(const char *prompt, ulong addr)
>+{
>+ printf("%s %lx ", prompt, addr);
>+}
>+
>+static void show_ret(efi_status_t ret)
>+{
>+ int code;
>+
>+ code = ret & ~EFI_ERROR_MASK;
>+ if (code < ARRAY_SIZE(error_name))
>+ printf("ret %s", error_name[ret]);
>+ else
>+ printf("ret %lx", ret);
>+}
>+
>+void show_rec(int seq, struct efil_rec_hdr *rec_hdr)
>+{
>+ void *start = (void *)rec_hdr + sizeof(struct efil_rec_hdr);
>+
>+ printf("%3d %12s ", seq, tag_name[rec_hdr->tag]);
>+ switch (rec_hdr->tag) {
>+ case EFILT_TESTING: {
>+ struct efil_testing *rec = start;
>+
>+ show_enum(test_enum_name, (int)rec->enum_val);
>+ show_ulong("int", (ulong)rec->int_val);
>+ show_addr("buf", map_to_sysmem(rec->buffer));
>+ show_addr("mem", map_to_sysmem(rec->memory));
>+ if (rec_hdr->ended) {
>+ show_addr("*buf",
>+ (ulong)map_to_sysmem((void *)rec->e_buffer));
>+ show_addr("*mem",
>+ (ulong)rec->e_memory);
>+ show_ret(rec_hdr->e_ret);
>+ }
>+ }
>+ case EFILT_COUNT:
>+ break;
>+ }
>+ printf("\n");
>+}
>+
>+int efi_log_show(void)
>+{
>+ struct efil_hdr *hdr = bloblist_find(BLOBLISTT_EFI_LOG, 0);
>+ struct efil_rec_hdr *rec_hdr;
>+ int i;
>+
>+ printf("EFI log (size %x)\n", hdr->upto);
>+ if (!hdr)
>+ return -ENOENT;
>+ for (i = 0, rec_hdr = (void *)hdr + sizeof(*hdr);
>+ (void *)rec_hdr - (void *)hdr < hdr->upto;
>+ i++, rec_hdr = (void *)rec_hdr + rec_hdr->size)
>+ show_rec(i, rec_hdr);
>+ printf("%d records\n", i);
>+
>+ return 0;
>+}
>+
>+int efi_log_reset(void)
>+{
>+ struct efil_hdr *hdr = bloblist_find(BLOBLISTT_EFI_LOG, 0);
>+
>+ if (!hdr)
>+ return -ENOENT;
>+ hdr->upto = sizeof(struct efil_hdr);
>+ hdr->size = CONFIG_EFI_LOG_SIZE;
>+
>+ return 0;
>+}
>+
>+int efi_log_init(void)
>+{
>+ struct efil_hdr *hdr;
>+
>+ hdr = bloblist_add(BLOBLISTT_EFI_LOG, CONFIG_EFI_LOG_SIZE, 0);
>+ if (!hdr) {
>+ /*
>+ * Return -ENOMEM since we use -ENOSPC to mean that the log is
>+ * full
>+ */
>+ log_warning("Failed to setup EFI log\n");
>+ return log_msg_ret("eli", -ENOMEM);
>+ }
>+ efi_log_reset();
>+
>+ return 0;
>+}
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 10/15] efi_loader: Add support for logging EFI calls
2024-10-28 12:48 ` [PATCH 10/15] efi_loader: Add support for logging EFI calls Simon Glass
2024-10-29 11:44 ` Heinrich Schuchardt
@ 2024-10-29 22:19 ` Heinrich Schuchardt
2024-10-31 18:01 ` Simon Glass
1 sibling, 1 reply; 40+ messages in thread
From: Heinrich Schuchardt @ 2024-10-29 22:19 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, AKASHI Takahiro, Caleb Connolly,
Levi Yun, Marek Vasut, Mattijs Korpershoek, Michal Simek,
Oliver Gaskell, Patrick Rudolph, Raymond Mao, Robert Marko,
Sam Protsenko, Sumit Garg, Vincent Stehlé
Am 28. Oktober 2024 13:48:01 MEZ schrieb Simon Glass <sjg@chromium.org>:
>The current logging system suffers from some disadvantages, mainly that
>it writes its output to the console and cannot be easily reviewed.
>
>Add a dedicated log, storing records in a binary format and including
>the result codes and any return values from each call. The log is built
>sequentially in memory and can be reviewed after any EFI operation. It
>could potentially be written to media for later review, but that is not
>implemented so far.
An EFI specific solution is not a good approach as it does not scale to other parts of the code. Please, implement a log driver to collect the messages that you are interested in.
Best regards
Heinrich
>
>Signed-off-by: Simon Glass <sjg@chromium.org>
>---
>
> MAINTAINERS | 6 +
> include/bloblist.h | 1 +
> include/efi.h | 1 +
> include/efi_log.h | 169 ++++++++++++++++++++++++++
> lib/efi_loader/Kconfig | 19 +++
> lib/efi_loader/Makefile | 1 +
> lib/efi_loader/efi_log.c | 256 +++++++++++++++++++++++++++++++++++++++
> 7 files changed, 453 insertions(+)
> create mode 100644 include/efi_log.h
> create mode 100644 lib/efi_loader/efi_log.c
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 38c714cf46a..9ade0ca4bc3 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -1067,6 +1067,12 @@ F: lib/efi/efi_app.c
> F: scripts/build-efi.sh
> F: test/dm/efi_media.c
>
>+EFI LOGGING
>+M: Simon Glass <sjg@chromium.org>
>+S: Maintained
>+F: include/efi_log.h
>+F: lib/efi_loader/efi_log.c
>+
> EFI PAYLOAD
> M: Heinrich Schuchardt <xypron.glpk@gmx.de>
> M: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>diff --git a/include/bloblist.h b/include/bloblist.h
>index ff32d3fecfd..1e1ca34aa92 100644
>--- a/include/bloblist.h
>+++ b/include/bloblist.h
>@@ -153,6 +153,7 @@ enum bloblist_tag_t {
> BLOBLISTT_U_BOOT_SPL_HANDOFF = 0xfff000, /* Hand-off info from SPL */
> BLOBLISTT_VBE = 0xfff001, /* VBE per-phase state */
> BLOBLISTT_U_BOOT_VIDEO = 0xfff002, /* Video info from SPL */
>+ BLOBLISTT_EFI_LOG = 0xfff003, /* Log of EFI calls */
> };
>
> /**
>diff --git a/include/efi.h b/include/efi.h
>index 84640cf7b25..f7419b80d4d 100644
>--- a/include/efi.h
>+++ b/include/efi.h
>@@ -127,6 +127,7 @@ static inline void *guidcpy(void *dst, const void *src)
> #define EFI_COMPROMISED_DATA (EFI_ERROR_MASK | 33)
> #define EFI_IP_ADDRESS_CONFLICT (EFI_ERROR_MASK | 34)
> #define EFI_HTTP_ERROR (EFI_ERROR_MASK | 35)
>+#define EFI_ERROR_COUNT 36
>
> #define EFI_WARN_UNKNOWN_GLYPH 1
> #define EFI_WARN_DELETE_FAILURE 2
>diff --git a/include/efi_log.h b/include/efi_log.h
>new file mode 100644
>index 00000000000..1988e5f9df0
>--- /dev/null
>+++ b/include/efi_log.h
>@@ -0,0 +1,169 @@
>+/* SPDX-License-Identifier: GPL-2.0+ */
>+/*
>+ * Logging (to memory) of calls from an EFI app
>+ *
>+ * Copyright 2024 Google LLC
>+ * Written by Simon Glass <sjg@chromium.org>
>+ */
>+
>+#ifndef __EFI_LOG_H
>+#define __EFI_LOG_H
>+
>+#include <linux/types.h>
>+#include <efi.h>
>+
>+/**
>+ * enum efil_tag - Types of logging records which can be created
>+ */
>+enum efil_tag {
>+ EFILT_TESTING,
>+
>+ EFILT_COUNT,
>+};
>+
>+/**
>+ * struct efil_rec_hdr - Header for each logging record
>+ *
>+ * @tag: Tag which indicates the type of the record
>+ * @size: Size of the record in bytes
>+ * @ended: true if record has been completed (i.e. the function returned), false
>+ * if it is still pending
>+ * @e_ret: Records the return function from the logged function
>+ */
>+struct efil_rec_hdr {
>+ enum efil_tag tag;
>+ int size;
>+ bool ended;
>+ efi_status_t e_ret;
>+};
>+
>+/**
>+ * struct efil_hdr - Holds the header for the log
>+ *
>+ * @upto: Offset at which to store the next log record
>+ * @size: Total size of the log in bytes
>+ */
>+struct efil_hdr {
>+ int upto;
>+ int size;
>+};
>+
>+enum efil_test_t {
>+ EFI_LOG_TEST0,
>+ EFI_LOG_TEST1,
>+
>+ EFI_LOG_TEST_COUNT,
>+};
>+
>+/**
>+ * struct efil_testing - used for testing the log
>+ */
>+struct efil_testing {
>+ enum efil_test_t enum_val;
>+ efi_uintn_t int_val;
>+ u64 *memory;
>+ void **buffer;
>+ u64 e_memory;
>+ void *e_buffer;
>+};
>+
>+/**
>+ * struct efil_allocate_pages - holds info from efi_allocate_pages() call
>+ *
>+ * @e_memory: Contains the value of *@memory on return from the EFI function
>+ */
>+struct efil_allocate_pages {
>+ enum efi_allocate_type type;
>+ enum efi_memory_type memory_type;
>+ efi_uintn_t pages;
>+ u64 *memory;
>+ u64 e_memory;
>+};
>+
>+/*
>+ * The functions below are in pairs, with a 'start' and 'end' call for each EFI
>+ * function. The 'start' function (efi_logs_...) is called when the function is
>+ * started. It records all the arguments. The 'end' function (efi_loge_...) is
>+ * called when the function is ready to return. It records any output arguments
>+ * as well as the return value.
>+ *
>+ * The start function returns the offset of the log record. This must be passed
>+ * to the end function, so it can add the status code and any other useful
>+ * information. It is not possible for the end functions to remember the offset
>+ * from the associated start function, since EFI functions may be called in a
>+ * nested way and there is no obvious way to determine the log record to which
>+ * the end function refers.
>+ *
>+ * If the start function returns an error code (i.e. an offset < 0) then it is
>+ * safe to pass that to the end function. It will simply ignore the operation.
>+ * Common errors are -ENOENT if there is no log and -ENOSPC if the log is full
>+ */
>+
>+#if CONFIG_IS_ENABLED(EFI_LOG)
>+
>+/**
>+ * efi_logs_testing() - Record a test call to an efi function
>+ *
>+ * @enum_val: enum value
>+ * @int_val: integer value
>+ * @buffer: place to write pointer address
>+ * @memory: place to write memory address
>+ * Return: log-offset of this new record, or -ve error code
>+ */
>+int efi_logs_testing(enum efil_test_t enum_val, efi_uintn_t int_value,
>+ void *buffer, u64 *memory);
>+
>+/**
>+ * efi_loge_testing() - Record a return from a test call
>+ *
>+ * This stores the value of the pointers also
>+ *
>+ * ofs: Offset of the record to end
>+ * efi_ret: status code to record
>+ */
>+int efi_loge_testing(int ofs, efi_status_t efi_ret);
>+
>+#else /* !EFI_LOG */
>+
>+static inline int efi_logs_testing(enum efil_test_t enum_val,
>+ efi_uintn_t int_value, void *buffer,
>+ u64 *memory)
>+{
>+ return -ENOSYS;
>+}
>+
>+static inline int efi_loge_testing(int ofs, efi_status_t efi_ret)
>+{
>+ return -ENOSYS;
>+}
>+
>+#endif /* EFI_LOG */
>+
>+/* below are some general functions */
>+
>+/**
>+ * efi_log_show() - Show the EFI log
>+ *
>+ * Displays the log of EFI boot-services calls which are so-far enabled for
>+ * logging
>+ *
>+ * Return: 0 on success, or -ve error code
>+ */
>+int efi_log_show(void);
>+
>+/**
>+ * efi_log_reset() - Reset the log, erasing all records
>+ *
>+ * Return 0 if OK, -ENOENT if the log could not be found
>+
>+ */
>+int efi_log_reset(void);
>+
>+/**
>+ * efi_log_init() - Create a log in the bloblist, then reset it
>+ *
>+ * Return 0 if OK, -ENOMEM if the bloblist is not large enough
>+ */
>+int efi_log_init(void);
>+
>+#endif /* __EFI_LOG_H */
>diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>index 69b2c9360d8..37572c82f54 100644
>--- a/lib/efi_loader/Kconfig
>+++ b/lib/efi_loader/Kconfig
>@@ -531,6 +531,25 @@ config EFI_SCROLL_ON_CLEAR_SCREEN
> to capture complete boot logs (except for interactive menus etc.)
> and can ease debugging related issues.
>
>+config EFI_LOG
>+ bool "Enable logging of EFI operations"
>+ select BLOBLIST
>+ help
>+ This enables maintaining a log of EFI boot-time services, useful for
>+ debugging. It keeps track of some of the calls which are made, so far
>+ just those related to memory allocation.
>+
>+config EFI_LOG_SIZE
>+ hex "Size of EFI log"
>+ depends on EFI_LOG
>+ default 0x4000
>+ help
>+ Sets the size of the EFI log in bytes. The log is stored in the
>+ bloblist so if its size is insufficient, U-Boot will raise an error.
>+
>+ The amount of space needed depends on the EFI app being run. When
>+ space runes out, further EFI calls will not be logged.
>+
> endmenu
>
> menu "EFI bootmanager"
>diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>index 00d18966f9e..8ec240cb864 100644
>--- a/lib/efi_loader/Makefile
>+++ b/lib/efi_loader/Makefile
>@@ -38,6 +38,7 @@ obj-y += efi_file.o
> obj-$(CONFIG_EFI_LOADER_HII) += efi_hii.o
> obj-y += efi_image_loader.o
> obj-y += efi_load_options.o
>+obj-$(CONFIG_EFI_LOG) += efi_log.o
> obj-y += efi_memory.o
> obj-y += efi_root_node.o
> obj-y += efi_runtime.o
>diff --git a/lib/efi_loader/efi_log.c b/lib/efi_loader/efi_log.c
>new file mode 100644
>index 00000000000..01e495d3995
>--- /dev/null
>+++ b/lib/efi_loader/efi_log.c
>@@ -0,0 +1,256 @@
>+// SPDX-License-Identifier: GPL-2.0+
>+/*
>+ * Logging (to memory) of calls from an EFI app
>+ *
>+ * Copyright 2024 Google LLC
>+ * Written by Simon Glass <sjg@chromium.org>
>+ */
>+
>+#define LOG_CATEGORY LOGC_EFI
>+
>+#include <bloblist.h>
>+#include <efi_log.h>
>+#include <errno.h>
>+#include <log.h>
>+
>+/* names for enum efil_tag (abbreviated to keep output to a single line) */
>+static const char *tag_name[EFILT_COUNT] = {
>+ "testing",
>+};
>+
>+/* names for error codes, trying to keep them short */
>+static const char *error_name[EFI_ERROR_COUNT] = {
>+ "OK",
>+ "load",
>+ "inval_param",
>+ "unsupported",
>+ "bad_buf_sz",
>+ "buf_small",
>+ "not_ready",
>+ "device",
>+ "write_prot",
>+ "out_of_rsrc",
>+ "vol_corrupt",
>+ "vol_full",
>+ "no_media",
>+ "media_chg",
>+ "not_found",
>+ "no access",
>+ "no_response",
>+ "no_mapping",
>+ "timeout",
>+ "not_started",
>+ "already",
>+ "aborted",
>+ "icmp",
>+ "tftp",
>+ "protocol",
>+ "bad version",
>+ "sec_violate",
>+ "crc_error",
>+ "end_media",
>+ "end_file",
>+ "inval_lang",
>+ "compromised",
>+ "ipaddr_busy",
>+ "http",
>+};
>+
>+static const char *test_enum_name[EFI_LOG_TEST_COUNT] = {
>+ "test0",
>+ "test1",
>+};
>+
>+/**
>+ * prep_rec() - prepare a new record in the log
>+ *
>+ * This creates a new record at the next available position, setting it up ready
>+ * to hold data. The size and tag are set up.
>+ *
>+ * The log is updated so that the next record will start after this one
>+ *
>+ * @tag: tag of the EFI call to record
>+ * @size: Number of bytes in the caller's struct
>+ * @recp: Set to point to where the caller should add its data
>+ * Return: Offset of this record (must be passed to finish_rec())
>+ */
>+static int prep_rec(enum efil_tag tag, uint str_size, void **recp)
>+{
>+ struct efil_hdr *hdr = bloblist_find(BLOBLISTT_EFI_LOG, 0);
>+ struct efil_rec_hdr *rec_hdr;
>+ int ofs, size;
>+
>+ if (!hdr)
>+ return -ENOENT;
>+ size = str_size + sizeof(struct efil_rec_hdr);
>+ if (hdr->upto + size > hdr->size)
>+ return -ENOSPC;
>+
>+ rec_hdr = (void *)hdr + hdr->upto;
>+ rec_hdr->size = size;
>+ rec_hdr->tag = tag;
>+ rec_hdr->ended = false;
>+ *recp = rec_hdr + 1;
>+
>+ ofs = hdr->upto;
>+ hdr->upto += size;
>+
>+ return ofs;
>+}
>+
>+/**
>+ * finish_rec() - Finish a previously started record
>+ *
>+ * @ofs: Offset of record to finish
>+ * @ret: Return code which is to be returned from the EFI function
>+ * Return: Pointer to the structure where the caller should add its data
>+ */
>+static void *finish_rec(int ofs, efi_status_t ret)
>+{
>+ struct efil_hdr *hdr = bloblist_find(BLOBLISTT_EFI_LOG, 0);
>+ struct efil_rec_hdr *rec_hdr;
>+
>+ if (!hdr || ofs < 0)
>+ return NULL;
>+ rec_hdr = (void *)hdr + ofs;
>+ rec_hdr->ended = true;
>+ rec_hdr->e_ret = ret;
>+
>+ return rec_hdr + 1;
>+}
>+
>+int efi_logs_testing(enum efil_test_t enum_val, efi_uintn_t int_val,
>+ void *buffer, u64 *memory)
>+{
>+ struct efil_testing *rec;
>+ int ret;
>+
>+ ret = prep_rec(EFILT_TESTING, sizeof(*rec), (void **)&rec);
>+ if (ret < 0)
>+ return ret;
>+
>+ rec->int_val = int_val;
>+ rec->buffer = buffer;
>+ rec->memory = memory;
>+ rec->e_buffer = NULL;
>+ rec->e_memory = 0;
>+
>+ return ret;
>+}
>+
>+int efi_loge_testing(int ofs, efi_status_t efi_ret)
>+{
>+ struct efil_testing *rec;
>+
>+ rec = finish_rec(ofs, efi_ret);
>+ if (!rec)
>+ return -ENOSPC;
>+ rec->e_memory = *rec->memory;
>+ rec->e_buffer = *rec->buffer;
>+
>+ return 0;
>+}
>+
>+static void show_enum(const char *type_name[], int type)
>+{
>+ printf("%s ", type_name[type]);
>+}
>+
>+static void show_ulong(const char *prompt, ulong val)
>+{
>+ printf("%s %lx", prompt, val);
>+ if (val >= 10)
>+ printf("/%ld", val);
>+ printf(" ");
>+}
>+
>+static void show_addr(const char *prompt, ulong addr)
>+{
>+ printf("%s %lx ", prompt, addr);
>+}
>+
>+static void show_ret(efi_status_t ret)
>+{
>+ int code;
>+
>+ code = ret & ~EFI_ERROR_MASK;
>+ if (code < ARRAY_SIZE(error_name))
>+ printf("ret %s", error_name[ret]);
>+ else
>+ printf("ret %lx", ret);
>+}
>+
>+void show_rec(int seq, struct efil_rec_hdr *rec_hdr)
>+{
>+ void *start = (void *)rec_hdr + sizeof(struct efil_rec_hdr);
>+
>+ printf("%3d %12s ", seq, tag_name[rec_hdr->tag]);
>+ switch (rec_hdr->tag) {
>+ case EFILT_TESTING: {
>+ struct efil_testing *rec = start;
>+
>+ show_enum(test_enum_name, (int)rec->enum_val);
>+ show_ulong("int", (ulong)rec->int_val);
>+ show_addr("buf", map_to_sysmem(rec->buffer));
>+ show_addr("mem", map_to_sysmem(rec->memory));
>+ if (rec_hdr->ended) {
>+ show_addr("*buf",
>+ (ulong)map_to_sysmem((void *)rec->e_buffer));
>+ show_addr("*mem",
>+ (ulong)rec->e_memory);
>+ show_ret(rec_hdr->e_ret);
>+ }
>+ }
>+ case EFILT_COUNT:
>+ break;
>+ }
>+ printf("\n");
>+}
>+
>+int efi_log_show(void)
>+{
>+ struct efil_hdr *hdr = bloblist_find(BLOBLISTT_EFI_LOG, 0);
>+ struct efil_rec_hdr *rec_hdr;
>+ int i;
>+
>+ printf("EFI log (size %x)\n", hdr->upto);
>+ if (!hdr)
>+ return -ENOENT;
>+ for (i = 0, rec_hdr = (void *)hdr + sizeof(*hdr);
>+ (void *)rec_hdr - (void *)hdr < hdr->upto;
>+ i++, rec_hdr = (void *)rec_hdr + rec_hdr->size)
>+ show_rec(i, rec_hdr);
>+ printf("%d records\n", i);
>+
>+ return 0;
>+}
>+
>+int efi_log_reset(void)
>+{
>+ struct efil_hdr *hdr = bloblist_find(BLOBLISTT_EFI_LOG, 0);
>+
>+ if (!hdr)
>+ return -ENOENT;
>+ hdr->upto = sizeof(struct efil_hdr);
>+ hdr->size = CONFIG_EFI_LOG_SIZE;
>+
>+ return 0;
>+}
>+
>+int efi_log_init(void)
>+{
>+ struct efil_hdr *hdr;
>+
>+ hdr = bloblist_add(BLOBLISTT_EFI_LOG, CONFIG_EFI_LOG_SIZE, 0);
>+ if (!hdr) {
>+ /*
>+ * Return -ENOMEM since we use -ENOSPC to mean that the log is
>+ * full
>+ */
>+ log_warning("Failed to setup EFI log\n");
>+ return log_msg_ret("eli", -ENOMEM);
>+ }
>+ efi_log_reset();
>+
>+ return 0;
>+}
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 10/15] efi_loader: Add support for logging EFI calls
2024-10-29 22:19 ` Heinrich Schuchardt
@ 2024-10-31 18:01 ` Simon Glass
2024-10-31 22:30 ` Heinrich Schuchardt
0 siblings, 1 reply; 40+ messages in thread
From: Simon Glass @ 2024-10-31 18:01 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: U-Boot Mailing List, Ilias Apalodimas, Tom Rini, AKASHI Takahiro,
Caleb Connolly, Levi Yun, Marek Vasut, Mattijs Korpershoek,
Michal Simek, Oliver Gaskell, Patrick Rudolph, Raymond Mao,
Robert Marko, Sam Protsenko, Sumit Garg, Vincent Stehlé
Hi Heinrich,
On Tue, 29 Oct 2024 at 23:32, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> Am 28. Oktober 2024 13:48:01 MEZ schrieb Simon Glass <sjg@chromium.org>:
> >The current logging system suffers from some disadvantages, mainly that
> >it writes its output to the console and cannot be easily reviewed.
> >
> >Add a dedicated log, storing records in a binary format and including
> >the result codes and any return values from each call. The log is built
> >sequentially in memory and can be reviewed after any EFI operation. It
> >could potentially be written to media for later review, but that is not
> >implemented so far.
>
> An EFI specific solution is not a good approach as it does not scale to other parts of the code. Please, implement a log driver to collect the messages that you are interested in.
>
I can do that too, but it isn't as easy to programmatically parse. I'd
like to track what calls are made and understand better what is going
on when Ubuntu boots, etc.
Regards,
Simon
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 10/15] efi_loader: Add support for logging EFI calls
2024-10-31 18:01 ` Simon Glass
@ 2024-10-31 22:30 ` Heinrich Schuchardt
2024-10-31 22:37 ` Tom Rini
0 siblings, 1 reply; 40+ messages in thread
From: Heinrich Schuchardt @ 2024-10-31 22:30 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Ilias Apalodimas, Tom Rini, AKASHI Takahiro,
Caleb Connolly, Levi Yun, Marek Vasut, Mattijs Korpershoek,
Michal Simek, Oliver Gaskell, Patrick Rudolph, Raymond Mao,
Robert Marko, Sam Protsenko, Sumit Garg, Vincent Stehlé
Am 31. Oktober 2024 19:01:47 MEZ schrieb Simon Glass <sjg@chromium.org>:
>Hi Heinrich,
>
>On Tue, 29 Oct 2024 at 23:32, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>>
>>
>> Am 28. Oktober 2024 13:48:01 MEZ schrieb Simon Glass <sjg@chromium.org>:
>> >The current logging system suffers from some disadvantages, mainly that
>> >it writes its output to the console and cannot be easily reviewed.
>> >
>> >Add a dedicated log, storing records in a binary format and including
>> >the result codes and any return values from each call. The log is built
>> >sequentially in memory and can be reviewed after any EFI operation. It
>> >could potentially be written to media for later review, but that is not
>> >implemented so far.
>>
>> An EFI specific solution is not a good approach as it does not scale to other parts of the code. Please, implement a log driver to collect the messages that you are interested in.
>>
>
>I can do that too, but it isn't as easy to programmatically parse. I'd
>like to track what calls are made and understand better what is going
>on when Ubuntu boots, etc.
What makes calls to log_debug hard to parse? We have __FILE__, __LINE__, __func__, and message text available as individual fields.
What information are you missing?
For tracking function calls we already have a trace capability in U-Boot.
Best regards
Heinrich
>
>Regards,
>Simon
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 10/15] efi_loader: Add support for logging EFI calls
2024-10-31 22:30 ` Heinrich Schuchardt
@ 2024-10-31 22:37 ` Tom Rini
2024-11-20 15:37 ` Simon Glass
0 siblings, 1 reply; 40+ messages in thread
From: Tom Rini @ 2024-10-31 22:37 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Simon Glass, U-Boot Mailing List, Ilias Apalodimas,
AKASHI Takahiro, Caleb Connolly, Levi Yun, Marek Vasut,
Mattijs Korpershoek, Michal Simek, Oliver Gaskell,
Patrick Rudolph, Raymond Mao, Robert Marko, Sam Protsenko,
Sumit Garg, Vincent Stehlé
[-- Attachment #1: Type: text/plain, Size: 1639 bytes --]
On Thu, Oct 31, 2024 at 11:30:58PM +0100, Heinrich Schuchardt wrote:
>
>
> Am 31. Oktober 2024 19:01:47 MEZ schrieb Simon Glass <sjg@chromium.org>:
> >Hi Heinrich,
> >
> >On Tue, 29 Oct 2024 at 23:32, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >>
> >>
> >> Am 28. Oktober 2024 13:48:01 MEZ schrieb Simon Glass <sjg@chromium.org>:
> >> >The current logging system suffers from some disadvantages, mainly that
> >> >it writes its output to the console and cannot be easily reviewed.
> >> >
> >> >Add a dedicated log, storing records in a binary format and including
> >> >the result codes and any return values from each call. The log is built
> >> >sequentially in memory and can be reviewed after any EFI operation. It
> >> >could potentially be written to media for later review, but that is not
> >> >implemented so far.
> >>
> >> An EFI specific solution is not a good approach as it does not scale to other parts of the code. Please, implement a log driver to collect the messages that you are interested in.
> >>
> >
> >I can do that too, but it isn't as easy to programmatically parse. I'd
> >like to track what calls are made and understand better what is going
> >on when Ubuntu boots, etc.
>
> What makes calls to log_debug hard to parse? We have __FILE__, __LINE__, __func__, and message text available as individual fields.
>
> What information are you missing?
>
> For tracking function calls we already have a trace capability in U-Boot.
Integrating EFI_LOADER with existing U-Boot logging / tracing
infrastructure sounds like a reasonable path forward to me.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 10/15] efi_loader: Add support for logging EFI calls
2024-10-31 22:37 ` Tom Rini
@ 2024-11-20 15:37 ` Simon Glass
0 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-11-20 15:37 UTC (permalink / raw)
To: Tom Rini
Cc: Heinrich Schuchardt, U-Boot Mailing List, Ilias Apalodimas,
AKASHI Takahiro, Caleb Connolly, Levi Yun, Marek Vasut,
Mattijs Korpershoek, Michal Simek, Oliver Gaskell,
Patrick Rudolph, Raymond Mao, Robert Marko, Sam Protsenko,
Sumit Garg, Vincent Stehlé
Hi Tom,
On Thu, 31 Oct 2024 at 16:38, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Oct 31, 2024 at 11:30:58PM +0100, Heinrich Schuchardt wrote:
> >
> >
> > Am 31. Oktober 2024 19:01:47 MEZ schrieb Simon Glass <sjg@chromium.org>:
> > >Hi Heinrich,
> > >
> > >On Tue, 29 Oct 2024 at 23:32, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > >>
> > >>
> > >>
> > >> Am 28. Oktober 2024 13:48:01 MEZ schrieb Simon Glass <sjg@chromium.org>:
> > >> >The current logging system suffers from some disadvantages, mainly that
> > >> >it writes its output to the console and cannot be easily reviewed.
> > >> >
> > >> >Add a dedicated log, storing records in a binary format and including
> > >> >the result codes and any return values from each call. The log is built
> > >> >sequentially in memory and can be reviewed after any EFI operation. It
> > >> >could potentially be written to media for later review, but that is not
> > >> >implemented so far.
> > >>
> > >> An EFI specific solution is not a good approach as it does not scale to other parts of the code. Please, implement a log driver to collect the messages that you are interested in.
> > >>
> > >
> > >I can do that too, but it isn't as easy to programmatically parse. I'd
> > >like to track what calls are made and understand better what is going
> > >on when Ubuntu boots, etc.
> >
> > What makes calls to log_debug hard to parse? We have __FILE__, __LINE__, __func__, and message text available as individual fields.
> >
> > What information are you missing?
> >
> > For tracking function calls we already have a trace capability in U-Boot.
>
> Integrating EFI_LOADER with existing U-Boot logging / tracing
> infrastructure sounds like a reasonable path forward to me.
Yes, we already have that. I replied to Ilias' response elsewhere.
Regards,
Simon
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 11/15] efi_loader: Create the log on startup
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
` (9 preceding siblings ...)
2024-10-28 12:48 ` [PATCH 10/15] efi_loader: Add support for logging EFI calls Simon Glass
@ 2024-10-28 12:48 ` Simon Glass
2024-10-28 12:48 ` [PATCH 12/15] efi_loader: Add a command to show the EFI log Simon Glass
` (4 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:48 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass,
Masahisa Kojima, Raymond Mao
Create an EFI log when the EFI subsystem is first touched. This happens
after relocation in board_init_f()
Signed-off-by: Simon Glass <sjg@chromium.org>
---
lib/efi_loader/efi_setup.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index aa59bc7779d..468156db813 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -8,6 +8,7 @@
#define LOG_CATEGORY LOGC_EFI
#include <efi_loader.h>
+#include <efi_log.h>
#include <efi_variable.h>
#include <log.h>
#include <asm-generic/unaligned.h>
@@ -186,6 +187,12 @@ int efi_init_early(void)
/* Allow unaligned memory access */
allow_unaligned();
+ if (IS_ENABLED(CONFIG_EFI_LOG)) {
+ ret = efi_log_init();
+ if (ret)
+ return -ENOSPC;
+ }
+
/* Initialize root node */
ret = efi_root_node_register();
if (ret != EFI_SUCCESS)
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 12/15] efi_loader: Add a command to show the EFI log
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
` (10 preceding siblings ...)
2024-10-28 12:48 ` [PATCH 11/15] efi_loader: Create the log on startup Simon Glass
@ 2024-10-28 12:48 ` Simon Glass
2024-10-28 12:48 ` [PATCH 13/15] test: efi_loader: Add a simple test for " Simon Glass
` (3 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:48 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass,
AKASHI Takahiro, Etienne Carriere, Francis Laniel, Hou Zhiqiang,
Masahisa Kojima, Maxim Moskalets, Sebastian Reichel
Add a simple command which lists the log. Avoid creating any new records
when this command is invoked.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
cmd/efidebug.c | 53 +++++++++++++++---
doc/usage/cmd/efidebug.rst | 109 +++++++++++++++++++++++++++++++++++++
doc/usage/index.rst | 1 +
3 files changed, 155 insertions(+), 8 deletions(-)
create mode 100644 doc/usage/cmd/efidebug.rst
diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index e040fe75fa1..bba984b2b75 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -10,6 +10,7 @@
#include <dm/device.h>
#include <efi_dt_fixup.h>
#include <efi_load_initrd.h>
+#include <efi_log.h>
#include <efi_loader.h>
#include <efi_rng.h>
#include <efi_variable.h>
@@ -511,6 +512,33 @@ static int do_efi_show_images(struct cmd_tbl *cmdtp, int flag,
return CMD_RET_SUCCESS;
}
+/**
+ * do_efi_show_log() - show UEFI log of boot-services calls
+ *
+ * @cmdtp: Command table
+ * @flag: Command flag
+ * @argc: Number of arguments
+ * @argv: Argument array
+ * Return: CMD_RET_SUCCESS on success, CMD_RET_RET_FAILURE on failure
+ *
+ * Implement efidebug "log" sub-command.
+ * Show UEFI log records.
+ */
+static int do_efi_show_log(struct cmd_tbl *cmdtp, int flag,
+ int argc, char *const argv[])
+{
+ if (!IS_ENABLED(CONFIG_EFI_LOG)) {
+ printf("Please enable CONFIG_EFI_LOG to use the log\n");
+ return CMD_RET_FAILURE;
+ }
+ if (efi_log_show()) {
+ printf("Failed\n");
+ return CMD_RET_FAILURE;
+ }
+
+ return CMD_RET_SUCCESS;
+}
+
static const char * const efi_mem_type_string[] = {
[EFI_RESERVED_MEMORY_TYPE] = "RESERVED",
[EFI_LOADER_CODE] = "LOADER CODE",
@@ -1563,6 +1591,7 @@ static struct cmd_tbl cmd_efidebug_sub[] = {
"", ""),
U_BOOT_CMD_MKENT(images, CONFIG_SYS_MAXARGS, 1, do_efi_show_images,
"", ""),
+ U_BOOT_CMD_MKENT(log, CONFIG_SYS_MAXARGS, 1, do_efi_show_log, "", ""),
U_BOOT_CMD_MKENT(memmap, CONFIG_SYS_MAXARGS, 1, do_efi_show_memmap,
"", ""),
U_BOOT_CMD_MKENT(tables, CONFIG_SYS_MAXARGS, 1, do_efi_show_tables,
@@ -1597,19 +1626,25 @@ static int do_efidebug(struct cmd_tbl *cmdtp, int flag,
argc--; argv++;
- /* Initialize UEFI drivers */
- r = efi_init_obj_list();
- if (r != EFI_SUCCESS) {
- printf("Error: Cannot initialize UEFI sub-system, r = %lu\n",
- r & ~EFI_ERROR_MASK);
- return CMD_RET_FAILURE;
- }
-
cp = find_cmd_tbl(argv[0], cmd_efidebug_sub,
ARRAY_SIZE(cmd_efidebug_sub));
if (!cp)
return CMD_RET_USAGE;
+ /*
+ * Calling efi_init_obj_list() can add log records, so avoid it if just
+ * showing the log
+ */
+ if (cp->cmd != do_efi_show_log) {
+ /* Initialize UEFI drivers */
+ r = efi_init_obj_list();
+ if (r != EFI_SUCCESS) {
+ printf("Error: Cannot initialize UEFI sub-system, r = %lu\n",
+ r & ~EFI_ERROR_MASK);
+ return CMD_RET_FAILURE;
+ }
+ }
+
return cp->cmd(cmdtp, flag, argc, argv);
}
@@ -1655,6 +1690,8 @@ U_BOOT_LONGHELP(efidebug,
" - show UEFI handles\n"
"efidebug images\n"
" - show loaded images\n"
+ "efidebug log\n"
+ " - show UEFI log\n"
"efidebug memmap\n"
" - show UEFI memory map\n"
"efidebug tables\n"
diff --git a/doc/usage/cmd/efidebug.rst b/doc/usage/cmd/efidebug.rst
new file mode 100644
index 00000000000..5eca4079f82
--- /dev/null
+++ b/doc/usage/cmd/efidebug.rst
@@ -0,0 +1,109 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright 2024 Google LLC
+.. Written by Simon Glass <sjg@chromium.org>
+
+.. index::
+ single: efidebug (command)
+
+efidebug command
+================
+
+Synopsis
+--------
+
+::
+
+ efidebug log
+
+Description
+-----------
+
+The *efidebug* command provides access to debugging features for the EFI-loader
+subsystem.
+
+Only one of the subcommands are documented at present.
+
+efidebug log
+~~~~~~~~~~~~
+
+This shows a log of EFI boot-services calls which have been handled since U-Boot
+started. This can be useful to see what the app is doing, or even what U-Boot
+itself has called.
+
+
+Example
+-------
+
+This shows checking the log, then using 'efidebug tables' to fully set up the
+EFI-loader subsystem, then checking the log again::
+
+ => efidebug log
+ EFI log (size 158)
+ 0 alloc_pool bt-data size 33/51 buf 7fffd8448ad0 *buf 7c20010 ret OK
+ 1 alloc_pages any-pages bt-data pgs 1 mem 7fffd8448a80 *mem 7c20000 ret OK
+ 2 alloc_pool bt-data size 60/96 buf 7fffd8448ac0 *buf 7c1f010 ret OK
+ 3 alloc_pages any-pages bt-data pgs 1 mem 7fffd8448a60 *mem 7c1f000 ret OK
+ 4 alloc_pool bt-data size 60/96 buf 7fffd8448ac0 *buf 7c1e010 ret OK
+ 5 alloc_pages any-pages bt-data pgs 1 mem 7fffd8448a60 *mem 7c1e000 ret OK
+ 6 records
+ => efidebug tables
+ efi_var_to_file() Cannot persist EFI variables without system partition
+ 0000000017bfc010 36122546-f7ef-4c8f-bd9b-eb8525b50c0b EFI Conformance Profiles Table
+ 0000000017bd4010 b122a263-3661-4f68-9929-78f8b0d62180 EFI System Resource Table
+ 0000000017bd8010 1e2ed096-30e2-4254-bd89-863bbef82325 TCG2 Final Events Table
+ 0000000017bd6010 eb66918a-7eef-402a-842e-931d21c38ae9 Runtime properties
+ 0000000008c49000 8868e871-e4f1-11d3-bc22-0080c73c8881 ACPI table
+ 0000000018c5b000 f2fd1544-9794-4a2c-992e-e5bbcf20e394 SMBIOS3 table
+ => efidebug log
+ EFI log (size a20)
+ 0 alloc_pool bt-data size 33/51 buf 7fffd8448ad0 *buf 7c20010 ret OK
+ 1 alloc_pages any-pages bt-data pgs 1 mem 7fffd8448a80 *mem 7c20000 ret OK
+ 2 alloc_pool bt-data size 60/96 buf 7fffd8448ac0 *buf 7c1f010 ret OK
+ 3 alloc_pages any-pages bt-data pgs 1 mem 7fffd8448a60 *mem 7c1f000 ret OK
+ 4 alloc_pool bt-data size 60/96 buf 7fffd8448ac0 *buf 7c1e010 ret OK
+ 5 alloc_pages any-pages bt-data pgs 1 mem 7fffd8448a60 *mem 7c1e000 ret OK
+ 6 alloc_pages any-pages rt-data pgs 20/32 mem 7fffd8448838 *mem 7bfe000 ret OK
+ 7 alloc_pool rt-data size 60/96 buf 7fffd84487e0 *buf 7bfd010 ret OK
+ 8 alloc_pages any-pages rt-data pgs 1 mem 7fffd8448780 *mem 7bfd000 ret OK
+ 9 alloc_pool rt-data size 180/384 buf 56f190ffd890 *buf 7bfc010 ret OK
+ 10 alloc_pages any-pages rt-data pgs 1 mem 7fffd8448800 *mem 7bfc000 ret OK
+ 11 alloc_pool bt-data size 4 buf 7fffd8448840 *buf 7bfb010 ret OK
+ 12 alloc_pages any-pages bt-data pgs 1 mem 7fffd84487f0 *mem 7bfb000 ret OK
+ 13 alloc_pool bt-data size 10/16 buf 7fffd8448728 *buf 7bfa010 ret OK
+ 14 alloc_pages any-pages bt-data pgs 1 mem 7fffd84486d0 *mem 7bfa000 ret OK
+ 15 alloc_pool bt-data size 60/96 buf 7fffd84487e0 *buf 7bf9010 ret OK
+ 16 alloc_pages any-pages bt-data pgs 1 mem 7fffd8448780 *mem 7bf9000 ret OK
+ 17 alloc_pool bt-data size 10000/65536 buf 56f19100fae0 *buf 7be8010 ret OK
+ 18 alloc_pages any-pages bt-data pgs 11/17 mem 7fffd84487d0 *mem 7be8000 ret OK
+ 19 alloc_pool acpi-nvs size 10000/65536 buf 56f19100fae8 *buf 7bd7010 ret OK
+ 20 alloc_pages any-pages acpi-nvs pgs 11/17 mem 7fffd84487d0 *mem 7bd7000 ret OK
+ 21 alloc_pool bt-data size 60/96 buf 7fffd84487d0 *buf 7bd6010 ret OK
+ 22 alloc_pages any-pages bt-data pgs 1 mem 7fffd8448770 *mem 7bd6000 ret OK
+ 23 alloc_pool rt-data size 8 buf 7fffd8448818 *buf 7bd5010 ret OK
+ 24 alloc_pages any-pages rt-data pgs 1 mem 7fffd84487c0 *mem 7bd5000 ret OK
+ 25 alloc_pool bt-data size 8 buf 7fffd8448360 *buf 7bd4010 ret OK
+ 26 alloc_pages any-pages bt-data pgs 1 mem 7fffd8448160 *mem 7bd4000 ret OK
+ 27 alloc_pool bt-data size f0/240 buf 7fffd8448378 *buf 7bd3010 ret OK
+ 28 alloc_pages any-pages bt-data pgs 1 mem 7fffd84482d0 *mem 7bd3000 ret OK
+ 29 free_pool buf 7bd3010 ret OK
+ 30 free_pages mem 7bd3000 pag 1 ret OK
+ 31 alloc_pool bt-data size 60/96 buf 7fffd84482d8 *buf 7bd3010 ret OK
+ 32 alloc_pages any-pages bt-data pgs 1 mem 7fffd8448280 *mem 7bd3000 ret OK
+ 33 free_pool buf 7bfa010 ret OK
+ 34 free_pages mem 7bfa000 pag 1 ret OK
+ 35 alloc_pool bt-data size f0/240 buf 7fffd8448380 *buf 7bfa010 ret OK
+ 36 alloc_pages any-pages bt-data pgs 1 mem 7fffd84482d0 *mem 7bfa000 ret OK
+ 37 free_pool buf 7bfa010 ret OK
+ 38 free_pages mem 7bfa000 pag 1 ret OK
+ 39 free_pool buf 7bd4010 ret OK
+ 40 free_pages mem 7bd4000 pag 1 ret OK
+ 41 alloc_pool bt-data size 61/97 buf 7fffd8448810 *buf 7bfa010 ret OK
+ 42 alloc_pages any-pages bt-data pgs 1 mem 7fffd84487c0 *mem 7bfa000 ret OK
+ 43 alloc_pool bt-data size 60/96 buf 7fffd8448800 *buf 7bd4010 ret OK
+ 44 alloc_pages any-pages bt-data pgs 1 mem 7fffd84487a0 *mem 7bd4000 ret OK
+ 45 alloc_pool bt-data size 60/96 buf 7fffd8448800 *buf 7bd2010 ret OK
+ 46 alloc_pages any-pages bt-data pgs 1 mem 7fffd84487a0 *mem 7bd2000 ret OK
+ 47 alloc_pool bt-data size 60/96 buf 7fffd8448810 *buf 7bd1010 ret OK
+ 48 alloc_pages any-pages bt-data pgs 1 mem 7fffd84487b0 *mem 7bd1000 ret OK
+ 49 records
+ =>
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index 24b2d2637b1..a2340d9a7e0 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -59,6 +59,7 @@ Shell commands
cmd/echo
cmd/efi
cmd/eficonfig
+ cmd/efidebug
cmd/env
cmd/event
cmd/exception
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 13/15] test: efi_loader: Add a simple test for the EFI log
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
` (11 preceding siblings ...)
2024-10-28 12:48 ` [PATCH 12/15] efi_loader: Add a command to show the EFI log Simon Glass
@ 2024-10-28 12:48 ` Simon Glass
2024-10-28 12:48 ` [PATCH 14/15] efi_loader: Use the log with memory-related functions Simon Glass
` (2 subsequent siblings)
15 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:48 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass,
Caleb Connolly, Dmitry Rokosov, Jonathan Humphreys,
Mattijs Korpershoek, Neil Armstrong, Oliver Gaskell, Robert Marko,
Sam Protsenko, Sean Anderson, Sumit Garg
Create a test which does some sample calls and checks the output.
Expand the size of the sandbox bloblist to accommodate the log.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
MAINTAINERS | 1 +
configs/sandbox_defconfig | 3 +++
lib/efi_loader/efi_log.c | 7 +++---
test/lib/Makefile | 1 +
test/lib/efi_log.c | 49 +++++++++++++++++++++++++++++++++++++++
5 files changed, 57 insertions(+), 4 deletions(-)
create mode 100644 test/lib/efi_log.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 9ade0ca4bc3..3e571174ad5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1072,6 +1072,7 @@ M: Simon Glass <sjg@chromium.org>
S: Maintained
F: include/efi_log.h
F: lib/efi_loader/efi_log.c
+F: test/lib/efi_log.c
EFI PAYLOAD
M: Heinrich Schuchardt <xypron.glpk@gmx.de>
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index d111858082d..afba6c78e28 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -17,6 +17,7 @@ CONFIG_EFI_CAPSULE_ON_DISK=y
CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
CONFIG_EFI_CAPSULE_AUTHENTICATE=y
CONFIG_EFI_CAPSULE_CRT_FILE="board/sandbox/capsule_pub_key_good.crt"
+CONFIG_EFI_LOG=y
CONFIG_BUTTON_CMD=y
CONFIG_FIT=y
CONFIG_FIT_RSASSA_PSS=y
@@ -50,6 +51,7 @@ CONFIG_LOG_DEFAULT_LEVEL=6
CONFIG_LOGF_FUNC=y
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_STACKPROTECTOR=y
+CONFIG_BLOBLIST_SIZE_RELOC=0x5000
CONFIG_CMD_CPU=y
CONFIG_CMD_LICENSE=y
CONFIG_CMD_SMBIOS=y
@@ -72,6 +74,7 @@ CONFIG_CMD_NVEDIT_SELECT=y
CONFIG_LOOPW=y
CONFIG_CMD_MD5SUM=y
CONFIG_CMD_MEMINFO=y
+CONFIG_CMD_MEMINFO=y
CONFIG_CMD_MEM_SEARCH=y
CONFIG_CMD_MX_CYCLIC=y
CONFIG_CMD_MEMTEST=y
diff --git a/lib/efi_loader/efi_log.c b/lib/efi_loader/efi_log.c
index 01e495d3995..780860d8b2f 100644
--- a/lib/efi_loader/efi_log.c
+++ b/lib/efi_loader/efi_log.c
@@ -129,6 +129,7 @@ int efi_logs_testing(enum efil_test_t enum_val, efi_uintn_t int_val,
if (ret < 0)
return ret;
+ rec->enum_val = enum_val;
rec->int_val = int_val;
rec->buffer = buffer;
rec->memory = memory;
@@ -194,10 +195,8 @@ void show_rec(int seq, struct efil_rec_hdr *rec_hdr)
show_addr("buf", map_to_sysmem(rec->buffer));
show_addr("mem", map_to_sysmem(rec->memory));
if (rec_hdr->ended) {
- show_addr("*buf",
- (ulong)map_to_sysmem((void *)rec->e_buffer));
- show_addr("*mem",
- (ulong)rec->e_memory);
+ show_addr("*buf", (ulong)map_to_sysmem(rec->e_buffer));
+ show_addr("*mem", (ulong)rec->e_memory);
show_ret(rec_hdr->e_ret);
}
}
diff --git a/test/lib/Makefile b/test/lib/Makefile
index 217c3baf881..050eaef4bb9 100644
--- a/test/lib/Makefile
+++ b/test/lib/Makefile
@@ -11,6 +11,7 @@ obj-y += abuf.o
obj-y += alist.o
obj-$(CONFIG_EFI_LOADER) += efi_device_path.o
obj-$(CONFIG_EFI_SECURE_BOOT) += efi_image_region.o
+obj-$(CONFIG_EFI_LOG) += efi_log.o
obj-y += hexdump.o
obj-$(CONFIG_SANDBOX) += kconfig.o
obj-y += lmb.o
diff --git a/test/lib/efi_log.c b/test/lib/efi_log.c
new file mode 100644
index 00000000000..414b7081bd9
--- /dev/null
+++ b/test/lib/efi_log.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <efi_log.h>
+#include <mapmem.h>
+#include <test/lib.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+/* basic test of logging */
+static int lib_test_efi_log_base(struct unit_test_state *uts)
+{
+ void **buf = map_sysmem(0x1000, 0);
+ u64 *addr = map_sysmem(0x1010, 0);
+ int ofs1, ofs2;
+
+ ut_assertok(efi_log_reset());
+
+ ofs1 = efi_logs_testing(EFI_LOG_TEST0, 123, &buf[0], &addr[0]);
+
+ ofs2 = efi_logs_testing(EFI_LOG_TEST1, 456, &buf[1], &addr[1]);
+
+ /* simulate an EFI call setting the return values */
+ addr[0] = 0x100;
+ buf[0] = map_sysmem(0x1100, 0);
+ addr[1] = 0x200;
+ buf[1] = map_sysmem(0x1200, 0);
+
+ ut_assertok(efi_loge_testing(ofs2, EFI_LOAD_ERROR));
+ ut_assertok(efi_loge_testing(ofs1, EFI_SUCCESS));
+
+ ut_assertok(efi_log_show());
+ ut_assert_nextline("EFI log (size 98)");
+ ut_assert_nextline(
+ " 0 testing test0 int 7b/123 buf 1000 mem 1010 *buf 1100 *mem 100 ret OK");
+ ut_assert_nextline(
+ " 1 testing test1 int 1c8/456 buf 1008 mem 1018 *buf 1200 *mem 200 ret load");
+ ut_assert_nextline("2 records");
+ ut_assert_console_end();
+
+ unmap_sysmem(buf);
+ unmap_sysmem(addr);
+
+ return 0;
+}
+LIB_TEST(lib_test_efi_log_base, UTF_CONSOLE);
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 14/15] efi_loader: Use the log with memory-related functions
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
` (12 preceding siblings ...)
2024-10-28 12:48 ` [PATCH 13/15] test: efi_loader: Add a simple test for " Simon Glass
@ 2024-10-28 12:48 ` Simon Glass
2024-10-28 12:48 ` [PATCH 15/15] efi_loader: Add documentation for the EFI log Simon Glass
2024-10-29 9:58 ` [PATCH 00/15] efi_loader: Add support for logging to a buffer Ilias Apalodimas
15 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:48 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass,
Sughosh Ganu
Update a few memory functions to log their inputs and outputs. To avoid
the use of 'goto', etc. the functions are turned into stubs, calling a
separate function to do the actual operation.
Add a few tests.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
include/efi_log.h | 147 ++++++++++++++++++++++++++++
lib/efi_loader/efi_log.c | 189 ++++++++++++++++++++++++++++++++++++
lib/efi_loader/efi_memory.c | 119 ++++++++++++++++-------
test/lib/efi_log.c | 44 +++++++++
4 files changed, 466 insertions(+), 33 deletions(-)
diff --git a/include/efi_log.h b/include/efi_log.h
index 1988e5f9df0..a4b707e57c5 100644
--- a/include/efi_log.h
+++ b/include/efi_log.h
@@ -16,6 +16,11 @@
* enum efil_tag - Types of logging records which can be created
*/
enum efil_tag {
+ EFILT_ALLOCATE_PAGES,
+ EFILT_FREE_PAGES,
+ EFILT_ALLOCATE_POOL,
+ EFILT_FREE_POOL,
+
EFILT_TESTING,
EFILT_COUNT,
@@ -80,6 +85,28 @@ struct efil_allocate_pages {
u64 e_memory;
};
+/** struct efil_free_pages - holds info from efi_free_pages() call */
+struct efil_free_pages {
+ u64 memory;
+ efi_uintn_t pages;
+};
+
+/** struct efil_allocate_pool - holds info from efi_allocate_pool() call
+ *
+ * @e_buffer: Contains the value of *@buffer on return from the EFI function
+ */
+struct efil_allocate_pool {
+ enum efi_memory_type pool_type;
+ efi_uintn_t size;
+ void **buffer;
+ void *e_buffer;
+};
+
+/** struct efil_free_pool - holds log-info from efi_free_pool() call */
+struct efil_free_pool {
+ void *buffer;
+};
+
/*
* The functions below are in pairs, with a 'start' and 'end' call for each EFI
* function. The 'start' function (efi_logs_...) is called when the function is
@@ -123,8 +150,128 @@ int efi_logs_testing(enum efil_test_t enum_val, efi_uintn_t int_value,
*/
int efi_loge_testing(int ofs, efi_status_t efi_ret);
+/**
+ * efi_logs_allocate_pages() - Record a call to efi_allocate_pages()
+ *
+ * @type: type of allocation to be performed
+ * @memory_type: usage type of the allocated memory
+ * @pages: number of pages to be allocated
+ * @memory: place to write address of allocated memory
+ * Return: log-offset of this new record, or -ve error code
+ */
+int efi_logs_allocate_pages(enum efi_allocate_type type,
+ enum efi_memory_type memory_type, efi_uintn_t pages,
+ u64 *memory);
+
+/**
+ * efi_loge_allocate_pages() - Record a return from efi_allocate_pages()
+ *
+ * This stores the value of the memory pointer also
+ *
+ * ofs: Offset of the record to end
+ * efi_ret: status code to record
+ */
+int efi_loge_allocate_pages(int ofs, efi_status_t efi_ret);
+
+/**
+ * efi_logs_free_pages() - Record a call to efi_free_pages()
+ *
+ * @memory: start of the memory area to be freed
+ * @pages: number of pages to be freed
+ * Return: log-offset of this new record, or -ve error code
+ */
+int efi_logs_free_pages(u64 memory, efi_uintn_t pages);
+
+/**
+ * efi_loge_free_pages() - Record a return from efi_free_pages()
+ *
+ * ofs: Offset of the record to end
+ * efi_ret: status code to record
+ */
+int efi_loge_free_pages(int ofs, efi_status_t efi_ret);
+
+/**
+ * efi_logs_allocate_pool() - Record a call to efi_allocate_pool()
+ *
+ * @pool_type: type of the pool from which memory is to be allocated
+ * @size: number of bytes to be allocated
+ * @buffer: place to hold pointer to allocated memory
+ * Return: log-offset of this new record, or -ve error code
+ */
+int efi_logs_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size,
+ void **buffer);
+
+/**
+ * efi_loge_allocate_pool() - Record a return from efi_allocate_pool()
+ *
+ * This stores the value of the buffer pointer also
+ *
+ * ofs: Offset of the record to end
+ * efi_ret: status code to record
+ */
+int efi_loge_allocate_pool(int ofs, efi_status_t efi_ret);
+
+/**
+ * efi_logs_free_pool() - Record a call to efi_free_pool()
+ *
+ * @buffer: start of memory to be freed
+ * Return: log-offset of this new record, or -ve error code
+ */
+int efi_logs_free_pool(void *buffer);
+
+/**
+ * efi_loge_free_pool() - Record a return from efi_free_pool()
+ *
+ * ofs: Offset of the record to end
+ * efi_ret: status code to record
+ */
+int efi_loge_free_pool(int ofs, efi_status_t efi_ret);
+
#else /* !EFI_LOG */
+static inline int efi_logs_allocate_pages(enum efi_allocate_type type,
+ enum efi_memory_type memory_type,
+ efi_uintn_t pages, u64 *memory)
+{
+ return -ENOSYS;
+}
+
+static inline int efi_loge_allocate_pages(int ofs, efi_status_t efi_ret)
+{
+ return -ENOSYS;
+}
+
+static inline int efi_logs_free_pages(u64 memory, efi_uintn_t pages)
+{
+ return -ENOSYS;
+}
+
+static inline int efi_loge_free_pages(int ofs, efi_status_t efi_ret)
+{
+ return -ENOSYS;
+}
+
+static inline int efi_logs_allocate_pool(enum efi_memory_type pool_type,
+ efi_uintn_t size, void **buffer)
+{
+ return -ENOSYS;
+}
+
+static inline int efi_loge_allocate_pool(int ofs, efi_status_t efi_ret)
+{
+ return -ENOSYS;
+}
+
+static inline int efi_logs_free_pool(void *buffer)
+{
+ return -ENOSYS;
+}
+
+static inline int efi_loge_free_pool(int ofs, efi_status_t efi_ret)
+{
+ return -ENOSYS;
+}
+
static inline int efi_logs_testing(enum efil_test_t enum_val,
efi_uintn_t int_value, void *buffer,
u64 *memory)
diff --git a/lib/efi_loader/efi_log.c b/lib/efi_loader/efi_log.c
index 780860d8b2f..4b9c6727fe7 100644
--- a/lib/efi_loader/efi_log.c
+++ b/lib/efi_loader/efi_log.c
@@ -15,9 +15,41 @@
/* names for enum efil_tag (abbreviated to keep output to a single line) */
static const char *tag_name[EFILT_COUNT] = {
+ "alloc_pages",
+ "free_pages",
+ "alloc_pool",
+ "free_pool",
+
"testing",
};
+/* names for enum efi_allocate_type */
+static const char *allocate_type_name[EFI_MAX_ALLOCATE_TYPE] = {
+ "any-pages",
+ "max-addr",
+ "alloc-addr",
+};
+
+/* names for enum efi_memory_type */
+static const char *memory_type_name[EFI_MAX_MEMORY_TYPE] = {
+ "reserved",
+ "ldr-code",
+ "ldr-data",
+ "bt-code",
+ "bt-data",
+ "rt-code",
+ "rt-data",
+ "convent",
+ "unusable",
+ "acpi-rec",
+ "acpi-nvs",
+ "mmap-io",
+ "mmap-iop",
+ "pal-code",
+ "persist",
+ "unaccept",
+};
+
/* names for error codes, trying to keep them short */
static const char *error_name[EFI_ERROR_COUNT] = {
"OK",
@@ -152,6 +184,119 @@ int efi_loge_testing(int ofs, efi_status_t efi_ret)
return 0;
}
+int efi_logs_allocate_pages(enum efi_allocate_type type,
+ enum efi_memory_type memory_type, efi_uintn_t pages,
+ u64 *memory)
+{
+ struct efil_allocate_pages *rec;
+ int ret;
+
+ ret = prep_rec(EFILT_ALLOCATE_PAGES, sizeof(*rec), (void **)&rec);
+ if (ret < 0)
+ return ret;
+
+ rec->type = type;
+ rec->memory_type = memory_type;
+ rec->pages = pages;
+ rec->memory = memory;
+ rec->e_memory = 0;
+
+ return ret;
+}
+
+int efi_loge_allocate_pages(int ofs, efi_status_t efi_ret)
+{
+ struct efil_allocate_pages *rec;
+
+ rec = finish_rec(ofs, efi_ret);
+ if (!rec)
+ return -ENOSPC;
+ rec->e_memory = *rec->memory;
+
+ return 0;
+}
+
+int efi_logs_free_pages(u64 memory, efi_uintn_t pages)
+{
+ struct efil_free_pages *rec;
+ int ret;
+
+ ret = prep_rec(EFILT_FREE_PAGES, sizeof(*rec), (void **)&rec);
+ if (ret < 0)
+ return ret;
+
+ rec->memory = memory;
+ rec->pages = pages;
+
+ return ret;
+}
+
+int efi_loge_free_pages(int ofs, efi_status_t efi_ret)
+{
+ struct efil_allocate_pages *rec;
+
+ rec = finish_rec(ofs, efi_ret);
+ if (!rec)
+ return -ENOSPC;
+
+ return 0;
+}
+
+int efi_logs_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size,
+ void **buffer)
+{
+ struct efil_allocate_pool *rec;
+ int ret;
+
+ ret = prep_rec(EFILT_ALLOCATE_POOL, sizeof(*rec), (void **)&rec);
+ if (ret < 0)
+ return ret;
+
+ rec->pool_type = pool_type;
+ rec->size = size;
+ rec->buffer = buffer;
+ rec->e_buffer = NULL;
+
+ return ret;
+}
+
+int efi_loge_allocate_pool(int ofs, efi_status_t efi_ret)
+{
+ struct efil_allocate_pool *rec;
+
+ rec = finish_rec(ofs, efi_ret);
+ if (!rec)
+ return -ENOSPC;
+ rec->e_buffer = *rec->buffer;
+
+ return 0;
+}
+
+int efi_logs_free_pool(void *buffer)
+{
+ struct efil_free_pool *rec;
+ int ret;
+
+ ret = prep_rec(EFILT_FREE_POOL, sizeof(*rec), (void **)&rec);
+ if (ret < 0)
+ return ret;
+
+ rec->buffer = buffer;
+
+ return ret;
+}
+
+int efi_loge_free_pool(int ofs, efi_status_t efi_ret)
+{
+ struct efil_free_pool *rec;
+
+ rec = finish_rec(ofs, efi_ret);
+ if (!rec)
+ return -ENOSPC;
+
+ return 0;
+}
+
static void show_enum(const char *type_name[], int type)
{
printf("%s ", type_name[type]);
@@ -187,6 +332,50 @@ void show_rec(int seq, struct efil_rec_hdr *rec_hdr)
printf("%3d %12s ", seq, tag_name[rec_hdr->tag]);
switch (rec_hdr->tag) {
+ case EFILT_ALLOCATE_PAGES: {
+ struct efil_allocate_pages *rec = start;
+
+ show_enum(allocate_type_name, rec->type);
+ show_enum(memory_type_name, rec->memory_type);
+ show_ulong("pgs", (ulong)rec->pages);
+ show_addr("mem", (ulong)rec->memory);
+ if (rec_hdr->ended) {
+ show_addr("*mem",
+ (ulong)map_to_sysmem((void *)rec->e_memory));
+ show_ret(rec_hdr->e_ret);
+ }
+ break;
+ }
+ case EFILT_FREE_PAGES: {
+ struct efil_free_pages *rec = start;
+
+ show_addr("mem", map_to_sysmem((void *)rec->memory));
+ show_ulong("pag", (ulong)rec->pages);
+ if (rec_hdr->ended)
+ show_ret(rec_hdr->e_ret);
+ break;
+ }
+ case EFILT_ALLOCATE_POOL: {
+ struct efil_allocate_pool *rec = start;
+
+ show_enum(memory_type_name, rec->pool_type);
+ show_ulong("size", (ulong)rec->size);
+ show_addr("buf", (ulong)rec->buffer);
+ if (rec_hdr->ended) {
+ show_addr("*buf",
+ (ulong)map_to_sysmem((void *)rec->e_buffer));
+ show_ret(rec_hdr->e_ret);
+ }
+ break;
+ }
+ case EFILT_FREE_POOL: {
+ struct efil_free_pool *rec = start;
+
+ show_addr("buf", map_to_sysmem(rec->buffer));
+ if (rec_hdr->ended)
+ show_ret(rec_hdr->e_ret);
+ break;
+ }
case EFILT_TESTING: {
struct efil_testing *rec = start;
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index 3d742fa1915..2982c45dc38 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -8,6 +8,7 @@
#define LOG_CATEGORY LOGC_EFI
#include <efi_loader.h>
+#include <efi_log.h>
#include <init.h>
#include <lmb.h>
#include <log.h>
@@ -438,18 +439,9 @@ static efi_status_t efi_check_allocated(u64 addr, bool must_be_allocated)
return EFI_NOT_FOUND;
}
-/**
- * efi_allocate_pages - allocate memory pages
- *
- * @type: type of allocation to be performed
- * @memory_type: usage type of the allocated memory
- * @pages: number of pages to be allocated
- * @memory: allocated memory
- * Return: status code
- */
-efi_status_t efi_allocate_pages(enum efi_allocate_type type,
- enum efi_memory_type memory_type,
- efi_uintn_t pages, uint64_t *memory)
+efi_status_t efi_allocate_pages_(enum efi_allocate_type type,
+ enum efi_memory_type memory_type,
+ efi_uintn_t pages, uint64_t *memory)
{
u64 len;
uint flags;
@@ -511,13 +503,29 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type type,
}
/**
- * efi_free_pages() - free memory pages
+ * efi_allocate_pages - allocate memory pages
*
- * @memory: start of the memory area to be freed
- * @pages: number of pages to be freed
- * Return: status code
+ * @type: type of allocation to be performed
+ * @memory_type: usage type of the allocated memory
+ * @pages: number of pages to be allocated
+ * @memory: allocated memory
+ * Return: status code
*/
-efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages)
+efi_status_t efi_allocate_pages(enum efi_allocate_type type,
+ enum efi_memory_type memory_type,
+ efi_uintn_t pages, uint64_t *memory)
+{
+ efi_status_t ret;
+ int ofs;
+
+ ofs = efi_logs_allocate_pages(type, memory_type, pages, memory);
+ ret = efi_allocate_pages_(type, memory_type, pages, memory);
+ efi_loge_allocate_pages(ofs, ret);
+
+ return ret;
+}
+
+efi_status_t efi_free_pages_(u64 memory, efi_uintn_t pages)
{
u64 len;
uint flags;
@@ -550,6 +558,25 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages)
return ret;
}
+/**
+ * efi_free_pages() - free memory pages
+ *
+ * @memory: start of the memory area to be freed
+ * @pages: number of pages to be freed
+ * Return: status code
+ */
+efi_status_t efi_free_pages(u64 memory, efi_uintn_t pages)
+{
+ efi_status_t ret;
+ int ofs;
+
+ ofs = efi_logs_free_pages(memory, pages);
+ ret = efi_free_pages_(memory, pages);
+ efi_loge_free_pages(ofs, ret);
+
+ return ret;
+}
+
/**
* efi_alloc_aligned_pages() - allocate aligned memory pages
*
@@ -602,15 +629,8 @@ void *efi_alloc_aligned_pages(u64 len, int memory_type, size_t align)
return (void *)(uintptr_t)aligned_mem;
}
-/**
- * efi_allocate_pool - allocate memory from pool
- *
- * @pool_type: type of the pool from which memory is to be allocated
- * @size: number of bytes to be allocated
- * @buffer: allocated memory
- * Return: status code
- */
-efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size, void **buffer)
+static efi_status_t efi_allocate_pool_(enum efi_memory_type pool_type,
+ efi_uintn_t size, void **buffer)
{
efi_status_t r;
u64 addr;
@@ -638,6 +658,27 @@ efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size,
return r;
}
+/**
+ * efi_allocate_pool - allocate memory from pool
+ *
+ * @pool_type: type of the pool from which memory is to be allocated
+ * @size: number of bytes to be allocated
+ * @buffer: allocated memory
+ * Return: status code
+ */
+efi_status_t efi_allocate_pool(enum efi_memory_type pool_type, efi_uintn_t size,
+ void **buffer)
+{
+ efi_status_t ret;
+ int ofs;
+
+ ofs = efi_logs_allocate_pool(pool_type, size, buffer);
+ ret = efi_allocate_pool_(pool_type, size, buffer);
+ efi_loge_allocate_pool(ofs, ret);
+
+ return ret;
+}
+
/**
* efi_alloc() - allocate boot services data pool memory
*
@@ -660,13 +701,7 @@ void *efi_alloc(size_t size)
return buf;
}
-/**
- * efi_free_pool() - free memory from pool
- *
- * @buffer: start of memory to be freed
- * Return: status code
- */
-efi_status_t efi_free_pool(void *buffer)
+efi_status_t efi_free_pool_(void *buffer)
{
efi_status_t ret;
struct efi_pool_allocation *alloc;
@@ -694,6 +729,24 @@ efi_status_t efi_free_pool(void *buffer)
return ret;
}
+/**
+ * efi_free_pool() - free memory from pool
+ *
+ * @buffer: start of memory to be freed
+ * Return: status code
+ */
+efi_status_t efi_free_pool(void *buffer)
+{
+ efi_status_t ret;
+ int ofs;
+
+ ofs = efi_logs_free_pool(buffer);
+ ret = efi_free_pool_(buffer);
+ efi_loge_free_pool(ofs, ret);
+
+ return ret;
+}
+
/**
* efi_get_memory_map() - get map describing memory usage.
*
diff --git a/test/lib/efi_log.c b/test/lib/efi_log.c
index 414b7081bd9..7c8f1cbcd06 100644
--- a/test/lib/efi_log.c
+++ b/test/lib/efi_log.c
@@ -47,3 +47,47 @@ static int lib_test_efi_log_base(struct unit_test_state *uts)
return 0;
}
LIB_TEST(lib_test_efi_log_base, UTF_CONSOLE);
+
+/* test the memory-function logging */
+static int lib_test_efi_log_mem(struct unit_test_state *uts)
+{
+ void **buf = map_sysmem(0x1000, 0);
+ u64 *addr = map_sysmem(0x1010, 0);
+ int ofs1, ofs2;
+
+ ut_assertok(efi_log_reset());
+
+ ofs1 = efi_logs_allocate_pool(EFI_BOOT_SERVICES_DATA, 100, buf);
+ ofs2 = efi_logs_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
+ EFI_BOOT_SERVICES_CODE, 10, addr);
+ ut_assertok(efi_loge_allocate_pages(ofs2, EFI_LOAD_ERROR));
+ ut_assertok(efi_loge_allocate_pool(ofs1, 0));
+
+ ofs1 = efi_logs_free_pool(*buf);
+ ut_assertok(efi_loge_free_pool(ofs1, EFI_INVALID_PARAMETER));
+
+ ofs2 = efi_logs_free_pages(*addr, 0);
+ ut_assertok(efi_loge_free_pool(ofs2, 0));
+
+ ut_assertok(efi_log_show());
+
+ ut_assert_nextline("EFI log (size c0)");
+
+ /*
+ * We end up with internal sandbox-addresses here since EFI_LOADER
+ * doesn't handle map_sysmem() correctly. So for now, only part of the
+ * string is matched.
+ */
+ ut_assert_nextlinen(" 0 alloc_pool bt-data size 64/100 buf 10002000 *buf");
+ ut_assert_nextlinen(" 1 alloc_pages any-pages bt-code pgs a/10 mem 10002010 *mem");
+ ut_assert_nextlinen(" 2 free_pool buf");
+ ut_assert_nextlinen(" 3 free_pages mem");
+
+ ut_assert_nextline("4 records");
+
+ unmap_sysmem(buf);
+ unmap_sysmem(addr);
+
+ return 0;
+}
+LIB_TEST(lib_test_efi_log_mem, UTF_CONSOLE);
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 15/15] efi_loader: Add documentation for the EFI log
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
` (13 preceding siblings ...)
2024-10-28 12:48 ` [PATCH 14/15] efi_loader: Use the log with memory-related functions Simon Glass
@ 2024-10-28 12:48 ` Simon Glass
2024-10-29 9:58 ` [PATCH 00/15] efi_loader: Add support for logging to a buffer Ilias Apalodimas
15 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-10-28 12:48 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Ilias Apalodimas, Tom Rini, Heinrich Schuchardt, Simon Glass,
Caleb Connolly, Jonathan Humphreys, Masahisa Kojima,
Wei Ming Chen
Add a note in the documentation about this feature.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
doc/develop/uefi/uefi.rst | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index 0760ca91d4f..eade13dcfa1 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -963,6 +963,28 @@ This driver is only available if U-Boot is configured with::
CONFIG_BLK=y
CONFIG_PARTITIONS=y
+Logging
+-------
+
+The EFI subsystem supports logging of boot-services calls to a memory area. The
+log can then be shown using the ``efidebug log`` command. Note that only a few
+calls are logged at present.
+
+To enable this feature::
+
+ CONFIG_EFI_LOG=y
+ CONFIG_EFI_LOG_SIZE=0x4000
+ CONFIG_BLOBLIST_SIZE_RELOC=0x5000
+
+The bloblist size must be set to larger than the EFI size. For now the log size
+is fixed at the start.
+
+Logging is performed within the internal U-Boot functions, not at the
+boot-services interface. This makes it possible to log EFI calls made by U-Boot
+itself, such as memory allocations.
+
+See the :doc:`/usage/cmd/efidebug` (*log* subcommand) for more information.
+
Miscellaneous
-------------
--
2.43.0
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer
2024-10-28 12:47 [PATCH 00/15] efi_loader: Add support for logging to a buffer Simon Glass
` (14 preceding siblings ...)
2024-10-28 12:48 ` [PATCH 15/15] efi_loader: Add documentation for the EFI log Simon Glass
@ 2024-10-29 9:58 ` Ilias Apalodimas
2024-10-29 15:45 ` Simon Glass
15 siblings, 1 reply; 40+ messages in thread
From: Ilias Apalodimas @ 2024-10-29 9:58 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Tom Rini, Heinrich Schuchardt,
AKASHI Takahiro, Caleb Connolly, Dmitry Rokosov, Emil Kronborg,
Etienne Carriere, Francis Laniel, Hou Zhiqiang, Igor Prusov,
Jonathan Humphreys, Levi Yun, Marek Vasut, Masahisa Kojima,
Mattijs Korpershoek, Maxim Moskalets, Michal Simek,
Neil Armstrong, Oliver Gaskell, Patrick Rudolph, Quentin Schulz,
Raymond Mao, Robert Marko, Sam Protsenko, Sean Anderson,
Sebastian Reichel, Sughosh Ganu, Sumit Garg,
Thomas Weißschuh, Vincent Stehlé, Wei Ming Chen
Hi Simon,
On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
>
> It is a bit of a pain to log EFI boot-services calls at present. The
> output goes to the console so cannot easily be inspected later. Also it
> would be useful to be able to store the log and review it later, perhaps
> after something has gone wrong.
>
> This series makes a start on implementing a log-to-buffer feature. It
> provides a simple 'efidebug log' command to inspect the buffer. For now,
> only memory allocations are logged.
Why is this problem specific to EFI and no U-Boot in general? Do we
have a similar machinery for malloc()?
Thanks
/Ilias
>
> This feature makes it possible to add tests to check which EFI calls are
> made by U-Boot itself. It may also make it easier to figure out what is
> needed for booting Windows.
>
> Some patches to help with debugging sandbox memory-mapping are included,
> but for now EFI's use of mapping is not adjusted.
>
>
> Simon Glass (15):
> log: Add a new category for tests
> test: Allow saving and restoring the bloblist
> bloblist: test: Mark tests with UTF_BLOBLIST
> sandbox: Convert sb command to use new macro
> doc: sandbox: Add docs for the sb command
> sandbox: Add a way to show the sandbox memory-mapping
> sandbox: Fix comment for nomap_sysmem() function
> lmb: Drop extra 16KB of stack space
> efi_loader: Fix free in ..._media_device_boot_option()
> efi_loader: Add support for logging EFI calls
> efi_loader: Create the log on startup
> efi_loader: Add a command to show the EFI log
> test: efi_loader: Add a simple test for the EFI log
> efi_loader: Use the log with memory-related functions
> efi_loader: Add documentation for the EFI log
>
> MAINTAINERS | 7 +
> arch/sandbox/cpu/cpu.c | 13 +
> arch/sandbox/include/asm/cpu.h | 3 +
> arch/sandbox/include/asm/io.h | 2 +-
> cmd/efidebug.c | 53 +++-
> cmd/sb.c | 42 ++--
> common/log.c | 1 +
> configs/sandbox_defconfig | 3 +
> doc/develop/uefi/uefi.rst | 22 ++
> doc/usage/cmd/efidebug.rst | 109 ++++++++
> doc/usage/cmd/sb.rst | 79 ++++++
> doc/usage/index.rst | 2 +
> include/bloblist.h | 1 +
> include/efi.h | 1 +
> include/efi_log.h | 316 +++++++++++++++++++++++
> include/log.h | 2 +
> include/test/test.h | 3 +
> lib/efi_loader/Kconfig | 19 ++
> lib/efi_loader/Makefile | 1 +
> lib/efi_loader/efi_bootmgr.c | 3 +-
> lib/efi_loader/efi_log.c | 444 +++++++++++++++++++++++++++++++++
> lib/efi_loader/efi_memory.c | 119 ++++++---
> lib/efi_loader/efi_setup.c | 7 +
> lib/lmb.c | 2 -
> test/common/bloblist.c | 28 +--
> test/lib/Makefile | 1 +
> test/lib/efi_log.c | 93 +++++++
> test/test-main.c | 13 +
> 28 files changed, 1305 insertions(+), 84 deletions(-)
> create mode 100644 doc/usage/cmd/efidebug.rst
> create mode 100644 doc/usage/cmd/sb.rst
> create mode 100644 include/efi_log.h
> create mode 100644 lib/efi_loader/efi_log.c
> create mode 100644 test/lib/efi_log.c
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer
2024-10-29 9:58 ` [PATCH 00/15] efi_loader: Add support for logging to a buffer Ilias Apalodimas
@ 2024-10-29 15:45 ` Simon Glass
2024-10-29 18:31 ` Ilias Apalodimas
0 siblings, 1 reply; 40+ messages in thread
From: Simon Glass @ 2024-10-29 15:45 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: U-Boot Mailing List, Tom Rini, Heinrich Schuchardt,
AKASHI Takahiro, Caleb Connolly, Dmitry Rokosov, Emil Kronborg,
Etienne Carriere, Francis Laniel, Hou Zhiqiang, Igor Prusov,
Jonathan Humphreys, Levi Yun, Marek Vasut, Masahisa Kojima,
Mattijs Korpershoek, Maxim Moskalets, Michal Simek,
Neil Armstrong, Oliver Gaskell, Patrick Rudolph, Quentin Schulz,
Raymond Mao, Robert Marko, Sam Protsenko, Sean Anderson,
Sebastian Reichel, Sughosh Ganu, Sumit Garg,
Thomas Weißschuh, Vincent Stehlé, Wei Ming Chen
Hi Ilias,
On Tue, 29 Oct 2024 at 10:58, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Simon,
>
> On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
> >
> > It is a bit of a pain to log EFI boot-services calls at present. The
> > output goes to the console so cannot easily be inspected later. Also it
> > would be useful to be able to store the log and review it later, perhaps
> > after something has gone wrong.
> >
> > This series makes a start on implementing a log-to-buffer feature. It
> > provides a simple 'efidebug log' command to inspect the buffer. For now,
> > only memory allocations are logged.
>
> Why is this problem specific to EFI and no U-Boot in general? Do we
> have a similar machinery for malloc()?
Mostly because an app can make EFI calls and we want to know what they
are, e.g. to debug them and figure out what might be wrong when
something doesn't boot.
Regards,
Simon
>
> Thanks
> /Ilias
> >
> > This feature makes it possible to add tests to check which EFI calls are
> > made by U-Boot itself. It may also make it easier to figure out what is
> > needed for booting Windows.
> >
> > Some patches to help with debugging sandbox memory-mapping are included,
> > but for now EFI's use of mapping is not adjusted.
> >
> >
> > Simon Glass (15):
> > log: Add a new category for tests
> > test: Allow saving and restoring the bloblist
> > bloblist: test: Mark tests with UTF_BLOBLIST
> > sandbox: Convert sb command to use new macro
> > doc: sandbox: Add docs for the sb command
> > sandbox: Add a way to show the sandbox memory-mapping
> > sandbox: Fix comment for nomap_sysmem() function
> > lmb: Drop extra 16KB of stack space
> > efi_loader: Fix free in ..._media_device_boot_option()
> > efi_loader: Add support for logging EFI calls
> > efi_loader: Create the log on startup
> > efi_loader: Add a command to show the EFI log
> > test: efi_loader: Add a simple test for the EFI log
> > efi_loader: Use the log with memory-related functions
> > efi_loader: Add documentation for the EFI log
> >
> > MAINTAINERS | 7 +
> > arch/sandbox/cpu/cpu.c | 13 +
> > arch/sandbox/include/asm/cpu.h | 3 +
> > arch/sandbox/include/asm/io.h | 2 +-
> > cmd/efidebug.c | 53 +++-
> > cmd/sb.c | 42 ++--
> > common/log.c | 1 +
> > configs/sandbox_defconfig | 3 +
> > doc/develop/uefi/uefi.rst | 22 ++
> > doc/usage/cmd/efidebug.rst | 109 ++++++++
> > doc/usage/cmd/sb.rst | 79 ++++++
> > doc/usage/index.rst | 2 +
> > include/bloblist.h | 1 +
> > include/efi.h | 1 +
> > include/efi_log.h | 316 +++++++++++++++++++++++
> > include/log.h | 2 +
> > include/test/test.h | 3 +
> > lib/efi_loader/Kconfig | 19 ++
> > lib/efi_loader/Makefile | 1 +
> > lib/efi_loader/efi_bootmgr.c | 3 +-
> > lib/efi_loader/efi_log.c | 444 +++++++++++++++++++++++++++++++++
> > lib/efi_loader/efi_memory.c | 119 ++++++---
> > lib/efi_loader/efi_setup.c | 7 +
> > lib/lmb.c | 2 -
> > test/common/bloblist.c | 28 +--
> > test/lib/Makefile | 1 +
> > test/lib/efi_log.c | 93 +++++++
> > test/test-main.c | 13 +
> > 28 files changed, 1305 insertions(+), 84 deletions(-)
> > create mode 100644 doc/usage/cmd/efidebug.rst
> > create mode 100644 doc/usage/cmd/sb.rst
> > create mode 100644 include/efi_log.h
> > create mode 100644 lib/efi_loader/efi_log.c
> > create mode 100644 test/lib/efi_log.c
> >
> > --
> > 2.43.0
> >
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer
2024-10-29 15:45 ` Simon Glass
@ 2024-10-29 18:31 ` Ilias Apalodimas
2024-10-31 18:01 ` Simon Glass
0 siblings, 1 reply; 40+ messages in thread
From: Ilias Apalodimas @ 2024-10-29 18:31 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Tom Rini, Heinrich Schuchardt,
AKASHI Takahiro, Caleb Connolly, Dmitry Rokosov, Emil Kronborg,
Etienne Carriere, Francis Laniel, Hou Zhiqiang, Igor Prusov,
Jonathan Humphreys, Levi Yun, Marek Vasut, Masahisa Kojima,
Mattijs Korpershoek, Maxim Moskalets, Michal Simek,
Neil Armstrong, Oliver Gaskell, Patrick Rudolph, Quentin Schulz,
Raymond Mao, Robert Marko, Sam Protsenko, Sean Anderson,
Sebastian Reichel, Sughosh Ganu, Sumit Garg,
Thomas Weißschuh, Vincent Stehlé, Wei Ming Chen
On Tue, 29 Oct 2024 at 17:45, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Ilias,
>
> On Tue, 29 Oct 2024 at 10:58, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > It is a bit of a pain to log EFI boot-services calls at present. The
> > > output goes to the console so cannot easily be inspected later. Also it
> > > would be useful to be able to store the log and review it later, perhaps
> > > after something has gone wrong.
> > >
> > > This series makes a start on implementing a log-to-buffer feature. It
> > > provides a simple 'efidebug log' command to inspect the buffer. For now,
> > > only memory allocations are logged.
> >
> > Why is this problem specific to EFI and no U-Boot in general? Do we
> > have a similar machinery for malloc()?
>
> Mostly because an app can make EFI calls and we want to know what they
> are, e.g. to debug them and figure out what might be wrong when
> something doesn't boot.
EFI_PRINT() has been proven pretty useful for this. I don't personally
see the point of adding ~1300 lines of code to replace a print.
What would make more sense is teach EFI_PRINT to log errors in a buffer.
Thanks
/Ilias
/Ilias
>
> Regards,
> Simon
>
>
> >
> > Thanks
> > /Ilias
> > >
> > > This feature makes it possible to add tests to check which EFI calls are
> > > made by U-Boot itself. It may also make it easier to figure out what is
> > > needed for booting Windows.
> > >
> > > Some patches to help with debugging sandbox memory-mapping are included,
> > > but for now EFI's use of mapping is not adjusted.
> > >
> > >
> > > Simon Glass (15):
> > > log: Add a new category for tests
> > > test: Allow saving and restoring the bloblist
> > > bloblist: test: Mark tests with UTF_BLOBLIST
> > > sandbox: Convert sb command to use new macro
> > > doc: sandbox: Add docs for the sb command
> > > sandbox: Add a way to show the sandbox memory-mapping
> > > sandbox: Fix comment for nomap_sysmem() function
> > > lmb: Drop extra 16KB of stack space
> > > efi_loader: Fix free in ..._media_device_boot_option()
> > > efi_loader: Add support for logging EFI calls
> > > efi_loader: Create the log on startup
> > > efi_loader: Add a command to show the EFI log
> > > test: efi_loader: Add a simple test for the EFI log
> > > efi_loader: Use the log with memory-related functions
> > > efi_loader: Add documentation for the EFI log
> > >
> > > MAINTAINERS | 7 +
> > > arch/sandbox/cpu/cpu.c | 13 +
> > > arch/sandbox/include/asm/cpu.h | 3 +
> > > arch/sandbox/include/asm/io.h | 2 +-
> > > cmd/efidebug.c | 53 +++-
> > > cmd/sb.c | 42 ++--
> > > common/log.c | 1 +
> > > configs/sandbox_defconfig | 3 +
> > > doc/develop/uefi/uefi.rst | 22 ++
> > > doc/usage/cmd/efidebug.rst | 109 ++++++++
> > > doc/usage/cmd/sb.rst | 79 ++++++
> > > doc/usage/index.rst | 2 +
> > > include/bloblist.h | 1 +
> > > include/efi.h | 1 +
> > > include/efi_log.h | 316 +++++++++++++++++++++++
> > > include/log.h | 2 +
> > > include/test/test.h | 3 +
> > > lib/efi_loader/Kconfig | 19 ++
> > > lib/efi_loader/Makefile | 1 +
> > > lib/efi_loader/efi_bootmgr.c | 3 +-
> > > lib/efi_loader/efi_log.c | 444 +++++++++++++++++++++++++++++++++
> > > lib/efi_loader/efi_memory.c | 119 ++++++---
> > > lib/efi_loader/efi_setup.c | 7 +
> > > lib/lmb.c | 2 -
> > > test/common/bloblist.c | 28 +--
> > > test/lib/Makefile | 1 +
> > > test/lib/efi_log.c | 93 +++++++
> > > test/test-main.c | 13 +
> > > 28 files changed, 1305 insertions(+), 84 deletions(-)
> > > create mode 100644 doc/usage/cmd/efidebug.rst
> > > create mode 100644 doc/usage/cmd/sb.rst
> > > create mode 100644 include/efi_log.h
> > > create mode 100644 lib/efi_loader/efi_log.c
> > > create mode 100644 test/lib/efi_log.c
> > >
> > > --
> > > 2.43.0
> > >
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer
2024-10-29 18:31 ` Ilias Apalodimas
@ 2024-10-31 18:01 ` Simon Glass
2024-11-01 11:31 ` Ilias Apalodimas
0 siblings, 1 reply; 40+ messages in thread
From: Simon Glass @ 2024-10-31 18:01 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: U-Boot Mailing List, Tom Rini, Heinrich Schuchardt,
AKASHI Takahiro, Caleb Connolly, Dmitry Rokosov, Emil Kronborg,
Etienne Carriere, Francis Laniel, Hou Zhiqiang, Igor Prusov,
Jonathan Humphreys, Levi Yun, Marek Vasut, Masahisa Kojima,
Mattijs Korpershoek, Maxim Moskalets, Michal Simek,
Neil Armstrong, Oliver Gaskell, Patrick Rudolph, Quentin Schulz,
Raymond Mao, Robert Marko, Sam Protsenko, Sean Anderson,
Sebastian Reichel, Sughosh Ganu, Sumit Garg,
Thomas Weißschuh, Vincent Stehlé, Wei Ming Chen
Hi Ilias,
On Tue, 29 Oct 2024 at 19:32, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> On Tue, 29 Oct 2024 at 17:45, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Ilias,
> >
> > On Tue, 29 Oct 2024 at 10:58, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > It is a bit of a pain to log EFI boot-services calls at present. The
> > > > output goes to the console so cannot easily be inspected later. Also it
> > > > would be useful to be able to store the log and review it later, perhaps
> > > > after something has gone wrong.
> > > >
> > > > This series makes a start on implementing a log-to-buffer feature. It
> > > > provides a simple 'efidebug log' command to inspect the buffer. For now,
> > > > only memory allocations are logged.
> > >
> > > Why is this problem specific to EFI and no U-Boot in general? Do we
> > > have a similar machinery for malloc()?
> >
> > Mostly because an app can make EFI calls and we want to know what they
> > are, e.g. to debug them and figure out what might be wrong when
> > something doesn't boot.
>
> EFI_PRINT() has been proven pretty useful for this. I don't personally
> see the point of adding ~1300 lines of code to replace a print.
> What would make more sense is teach EFI_PRINT to log errors in a buffer.
Is that a NAK? Please be clear if you are reviewing the code or just
rejecting the whole idea.
I am hoping to expand this into a debugging tool for figuring out how
to boot Windows and perhaps logging detailed information when things
go wrong, for later analysis. It might seem like overkill, but we will
see.
Regards,
Simon
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer
2024-10-31 18:01 ` Simon Glass
@ 2024-11-01 11:31 ` Ilias Apalodimas
2024-11-20 15:37 ` Simon Glass
0 siblings, 1 reply; 40+ messages in thread
From: Ilias Apalodimas @ 2024-11-01 11:31 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Tom Rini, Heinrich Schuchardt,
AKASHI Takahiro, Caleb Connolly, Dmitry Rokosov, Emil Kronborg,
Etienne Carriere, Francis Laniel, Hou Zhiqiang, Igor Prusov,
Jonathan Humphreys, Levi Yun, Marek Vasut, Masahisa Kojima,
Mattijs Korpershoek, Maxim Moskalets, Michal Simek,
Neil Armstrong, Oliver Gaskell, Patrick Rudolph, Quentin Schulz,
Raymond Mao, Robert Marko, Sam Protsenko, Sean Anderson,
Sebastian Reichel, Sughosh Ganu, Sumit Garg,
Thomas Weißschuh, Vincent Stehlé, Wei Ming Chen
Hi Simon,
On Thu, 31 Oct 2024 at 20:02, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Ilias,
>
> On Tue, 29 Oct 2024 at 19:32, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > On Tue, 29 Oct 2024 at 17:45, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Ilias,
> > >
> > > On Tue, 29 Oct 2024 at 10:58, Ilias Apalodimas
> > > <ilias.apalodimas@linaro.org> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > It is a bit of a pain to log EFI boot-services calls at present. The
> > > > > output goes to the console so cannot easily be inspected later. Also it
> > > > > would be useful to be able to store the log and review it later, perhaps
> > > > > after something has gone wrong.
> > > > >
> > > > > This series makes a start on implementing a log-to-buffer feature. It
> > > > > provides a simple 'efidebug log' command to inspect the buffer. For now,
> > > > > only memory allocations are logged.
> > > >
> > > > Why is this problem specific to EFI and no U-Boot in general? Do we
> > > > have a similar machinery for malloc()?
> > >
> > > Mostly because an app can make EFI calls and we want to know what they
> > > are, e.g. to debug them and figure out what might be wrong when
> > > something doesn't boot.
> >
> > EFI_PRINT() has been proven pretty useful for this. I don't personally
> > see the point of adding ~1300 lines of code to replace a print.
> > What would make more sense is teach EFI_PRINT to log errors in a buffer.
>
> Is that a NAK? Please be clear if you are reviewing the code or just
> rejecting the whole idea.
For the idea, no. But I don't think what's implemented here is what we want.
To track what EFI services are called, we already have EFI_ENTRY and EFI_EXIT.
Why don't we instead, add a logging service (and we already have
ftrace iirc) and plug it in the macros above?
That would make more sense not to mention way less code.
>
> I am hoping to expand this into a debugging tool for figuring out how
> to boot Windows and perhaps logging detailed information when things
> go wrong, for later analysis. It might seem like overkill, but we will
> see.
I've managed to run windows installers on QEMU & U-Boot. It's been
more than a year but windows boots and calls EBS(). The something
fails down the road.
I think I have a branch somewhere with the changes needed, I'll send
it over if I find it.
/Ilias
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer
2024-11-01 11:31 ` Ilias Apalodimas
@ 2024-11-20 15:37 ` Simon Glass
2024-11-20 15:55 ` Ilias Apalodimas
0 siblings, 1 reply; 40+ messages in thread
From: Simon Glass @ 2024-11-20 15:37 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: U-Boot Mailing List, Tom Rini, Heinrich Schuchardt,
AKASHI Takahiro, Caleb Connolly, Dmitry Rokosov, Emil Kronborg,
Etienne Carriere, Francis Laniel, Hou Zhiqiang, Igor Prusov,
Jonathan Humphreys, Levi Yun, Marek Vasut, Masahisa Kojima,
Mattijs Korpershoek, Maxim Moskalets, Michal Simek,
Neil Armstrong, Oliver Gaskell, Patrick Rudolph, Quentin Schulz,
Raymond Mao, Robert Marko, Sam Protsenko, Sean Anderson,
Sebastian Reichel, Sughosh Ganu, Sumit Garg,
Thomas Weißschuh, Vincent Stehlé, Wei Ming Chen
HI Ilias,
On Fri, 1 Nov 2024 at 05:32, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Simon,
>
> On Thu, 31 Oct 2024 at 20:02, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Ilias,
> >
> > On Tue, 29 Oct 2024 at 19:32, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > On Tue, 29 Oct 2024 at 17:45, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Ilias,
> > > >
> > > > On Tue, 29 Oct 2024 at 10:58, Ilias Apalodimas
> > > > <ilias.apalodimas@linaro.org> wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > It is a bit of a pain to log EFI boot-services calls at present. The
> > > > > > output goes to the console so cannot easily be inspected later. Also it
> > > > > > would be useful to be able to store the log and review it later, perhaps
> > > > > > after something has gone wrong.
> > > > > >
> > > > > > This series makes a start on implementing a log-to-buffer feature. It
> > > > > > provides a simple 'efidebug log' command to inspect the buffer. For now,
> > > > > > only memory allocations are logged.
> > > > >
> > > > > Why is this problem specific to EFI and no U-Boot in general? Do we
> > > > > have a similar machinery for malloc()?
> > > >
> > > > Mostly because an app can make EFI calls and we want to know what they
> > > > are, e.g. to debug them and figure out what might be wrong when
> > > > something doesn't boot.
> > >
> > > EFI_PRINT() has been proven pretty useful for this. I don't personally
> > > see the point of adding ~1300 lines of code to replace a print.
> > > What would make more sense is teach EFI_PRINT to log errors in a buffer.
> >
> > Is that a NAK? Please be clear if you are reviewing the code or just
> > rejecting the whole idea.
>
> For the idea, no. But I don't think what's implemented here is what we want.
>
> To track what EFI services are called, we already have EFI_ENTRY and EFI_EXIT.
> Why don't we instead, add a logging service (and we already have
> ftrace iirc) and plug it in the macros above?
> That would make more sense not to mention way less code.
I am wanting to programmatically log and manage what EFI_LOADER does,
so that bootstd can present a high-level view of what is going on,
e.g. which protocols are used, how much memory is allocated and where.
So this is not just about logging text output.
>
> >
> > I am hoping to expand this into a debugging tool for figuring out how
> > to boot Windows and perhaps logging detailed information when things
> > go wrong, for later analysis. It might seem like overkill, but we will
> > see.
>
> I've managed to run windows installers on QEMU & U-Boot. It's been
> more than a year but windows boots and calls EBS(). The something
> fails down the road.
> I think I have a branch somewhere with the changes needed, I'll send
> it over if I find it.
Did you find it?
Regards,
Simon
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer
2024-11-20 15:37 ` Simon Glass
@ 2024-11-20 15:55 ` Ilias Apalodimas
2024-11-20 18:06 ` Tom Rini
0 siblings, 1 reply; 40+ messages in thread
From: Ilias Apalodimas @ 2024-11-20 15:55 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Tom Rini, Heinrich Schuchardt,
AKASHI Takahiro, Caleb Connolly, Dmitry Rokosov, Emil Kronborg,
Etienne Carriere, Francis Laniel, Hou Zhiqiang, Igor Prusov,
Jonathan Humphreys, Levi Yun, Marek Vasut, Masahisa Kojima,
Mattijs Korpershoek, Maxim Moskalets, Michal Simek,
Neil Armstrong, Oliver Gaskell, Patrick Rudolph, Quentin Schulz,
Raymond Mao, Robert Marko, Sam Protsenko, Sean Anderson,
Sebastian Reichel, Sughosh Ganu, Sumit Garg,
Thomas Weißschuh, Vincent Stehlé, Wei Ming Chen
Hi Simon,
On Wed, 20 Nov 2024 at 17:37, Simon Glass <sjg@chromium.org> wrote:
>
> HI Ilias,
>
> On Fri, 1 Nov 2024 at 05:32, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Thu, 31 Oct 2024 at 20:02, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Ilias,
> > >
> > > On Tue, 29 Oct 2024 at 19:32, Ilias Apalodimas
> > > <ilias.apalodimas@linaro.org> wrote:
> > > >
> > > > On Tue, 29 Oct 2024 at 17:45, Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Ilias,
> > > > >
> > > > > On Tue, 29 Oct 2024 at 10:58, Ilias Apalodimas
> > > > > <ilias.apalodimas@linaro.org> wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
> > > > > > >
> > > > > > > It is a bit of a pain to log EFI boot-services calls at present. The
> > > > > > > output goes to the console so cannot easily be inspected later. Also it
> > > > > > > would be useful to be able to store the log and review it later, perhaps
> > > > > > > after something has gone wrong.
> > > > > > >
> > > > > > > This series makes a start on implementing a log-to-buffer feature. It
> > > > > > > provides a simple 'efidebug log' command to inspect the buffer. For now,
> > > > > > > only memory allocations are logged.
> > > > > >
> > > > > > Why is this problem specific to EFI and no U-Boot in general? Do we
> > > > > > have a similar machinery for malloc()?
> > > > >
> > > > > Mostly because an app can make EFI calls and we want to know what they
> > > > > are, e.g. to debug them and figure out what might be wrong when
> > > > > something doesn't boot.
> > > >
> > > > EFI_PRINT() has been proven pretty useful for this. I don't personally
> > > > see the point of adding ~1300 lines of code to replace a print.
> > > > What would make more sense is teach EFI_PRINT to log errors in a buffer.
> > >
> > > Is that a NAK? Please be clear if you are reviewing the code or just
> > > rejecting the whole idea.
> >
> > For the idea, no. But I don't think what's implemented here is what we want.
> >
> > To track what EFI services are called, we already have EFI_ENTRY and EFI_EXIT.
> > Why don't we instead, add a logging service (and we already have
> > ftrace iirc) and plug it in the macros above?
> > That would make more sense not to mention way less code.
>
> I am wanting to programmatically log and manage what EFI_LOADER does,
> so that bootstd can present a high-level view of what is going on,
> e.g. which protocols are used, how much memory is allocated and where.
> So this is not just about logging text output.
Why the EFI_LOADER only? Bootstd is supposed to cover more cases, so
why not a generic framework for all boot commands?
>
> >
> > >
> > > I am hoping to expand this into a debugging tool for figuring out how
> > > to boot Windows and perhaps logging detailed information when things
> > > go wrong, for later analysis. It might seem like overkill, but we will
> > > see.
> >
> > I've managed to run windows installers on QEMU & U-Boot. It's been
> > more than a year but windows boots and calls EBS(). The something
> > fails down the road.
> > I think I have a branch somewhere with the changes needed, I'll send
> > it over if I find it.
>
> Did you find it?
I didn't but Alexander has a tree on which I based my work on.
https://github.com/u-boot/u-boot/compare/master...agraf:u-boot:qemu-arm-win10
Cheers
/Ilias
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer
2024-11-20 15:55 ` Ilias Apalodimas
@ 2024-11-20 18:06 ` Tom Rini
2024-11-20 20:05 ` Heinrich Schuchardt
0 siblings, 1 reply; 40+ messages in thread
From: Tom Rini @ 2024-11-20 18:06 UTC (permalink / raw)
To: Ilias Apalodimas, Simon Glass
Cc: U-Boot Mailing List, Heinrich Schuchardt, AKASHI Takahiro,
Caleb Connolly, Dmitry Rokosov, Emil Kronborg, Etienne Carriere,
Francis Laniel, Hou Zhiqiang, Igor Prusov, Jonathan Humphreys,
Levi Yun, Marek Vasut, Masahisa Kojima, Mattijs Korpershoek,
Maxim Moskalets, Michal Simek, Neil Armstrong, Oliver Gaskell,
Patrick Rudolph, Quentin Schulz, Raymond Mao, Robert Marko,
Sam Protsenko, Sean Anderson, Sebastian Reichel, Sughosh Ganu,
Sumit Garg, Thomas Weißschuh, Vincent Stehlé,
Wei Ming Chen
[-- Attachment #1: Type: text/plain, Size: 3159 bytes --]
On Wed, Nov 20, 2024 at 05:55:18PM +0200, Ilias Apalodimas wrote:
> Hi Simon,
>
> On Wed, 20 Nov 2024 at 17:37, Simon Glass <sjg@chromium.org> wrote:
> >
> > HI Ilias,
> >
> > On Fri, 1 Nov 2024 at 05:32, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Thu, 31 Oct 2024 at 20:02, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Ilias,
> > > >
> > > > On Tue, 29 Oct 2024 at 19:32, Ilias Apalodimas
> > > > <ilias.apalodimas@linaro.org> wrote:
> > > > >
> > > > > On Tue, 29 Oct 2024 at 17:45, Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Ilias,
> > > > > >
> > > > > > On Tue, 29 Oct 2024 at 10:58, Ilias Apalodimas
> > > > > > <ilias.apalodimas@linaro.org> wrote:
> > > > > > >
> > > > > > > Hi Simon,
> > > > > > >
> > > > > > > On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
> > > > > > > >
> > > > > > > > It is a bit of a pain to log EFI boot-services calls at present. The
> > > > > > > > output goes to the console so cannot easily be inspected later. Also it
> > > > > > > > would be useful to be able to store the log and review it later, perhaps
> > > > > > > > after something has gone wrong.
> > > > > > > >
> > > > > > > > This series makes a start on implementing a log-to-buffer feature. It
> > > > > > > > provides a simple 'efidebug log' command to inspect the buffer. For now,
> > > > > > > > only memory allocations are logged.
> > > > > > >
> > > > > > > Why is this problem specific to EFI and no U-Boot in general? Do we
> > > > > > > have a similar machinery for malloc()?
> > > > > >
> > > > > > Mostly because an app can make EFI calls and we want to know what they
> > > > > > are, e.g. to debug them and figure out what might be wrong when
> > > > > > something doesn't boot.
> > > > >
> > > > > EFI_PRINT() has been proven pretty useful for this. I don't personally
> > > > > see the point of adding ~1300 lines of code to replace a print.
> > > > > What would make more sense is teach EFI_PRINT to log errors in a buffer.
> > > >
> > > > Is that a NAK? Please be clear if you are reviewing the code or just
> > > > rejecting the whole idea.
> > >
> > > For the idea, no. But I don't think what's implemented here is what we want.
> > >
> > > To track what EFI services are called, we already have EFI_ENTRY and EFI_EXIT.
> > > Why don't we instead, add a logging service (and we already have
> > > ftrace iirc) and plug it in the macros above?
> > > That would make more sense not to mention way less code.
> >
> > I am wanting to programmatically log and manage what EFI_LOADER does,
> > so that bootstd can present a high-level view of what is going on,
> > e.g. which protocols are used, how much memory is allocated and where.
> > So this is not just about logging text output.
>
> Why the EFI_LOADER only? Bootstd is supposed to cover more cases, so
> why not a generic framework for all boot commands?
This feels similar to the point I've made elsewhere in this overarching
series, why not do this at existing common points in the code path?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer
2024-11-20 18:06 ` Tom Rini
@ 2024-11-20 20:05 ` Heinrich Schuchardt
2024-11-21 2:19 ` Simon Glass
0 siblings, 1 reply; 40+ messages in thread
From: Heinrich Schuchardt @ 2024-11-20 20:05 UTC (permalink / raw)
To: Tom Rini, Ilias Apalodimas, Simon Glass
Cc: U-Boot Mailing List, AKASHI Takahiro, Caleb Connolly,
Dmitry Rokosov, Emil Kronborg, Etienne Carriere, Francis Laniel,
Hou Zhiqiang, Igor Prusov, Jonathan Humphreys, Levi Yun,
Marek Vasut, Masahisa Kojima, Mattijs Korpershoek,
Maxim Moskalets, Michal Simek, Neil Armstrong, Oliver Gaskell,
Patrick Rudolph, Quentin Schulz, Raymond Mao, Robert Marko,
Sam Protsenko, Sean Anderson, Sebastian Reichel, Sughosh Ganu,
Sumit Garg, Thomas Weißschuh, Vincent Stehlé,
Wei Ming Chen
Am 20. November 2024 19:06:33 MEZ schrieb Tom Rini <trini@konsulko.com>:
>On Wed, Nov 20, 2024 at 05:55:18PM +0200, Ilias Apalodimas wrote:
>> Hi Simon,
>>
>> On Wed, 20 Nov 2024 at 17:37, Simon Glass <sjg@chromium.org> wrote:
>> >
>> > HI Ilias,
>> >
>> > On Fri, 1 Nov 2024 at 05:32, Ilias Apalodimas
>> > <ilias.apalodimas@linaro.org> wrote:
>> > >
>> > > Hi Simon,
>> > >
>> > > On Thu, 31 Oct 2024 at 20:02, Simon Glass <sjg@chromium.org> wrote:
>> > > >
>> > > > Hi Ilias,
>> > > >
>> > > > On Tue, 29 Oct 2024 at 19:32, Ilias Apalodimas
>> > > > <ilias.apalodimas@linaro.org> wrote:
>> > > > >
>> > > > > On Tue, 29 Oct 2024 at 17:45, Simon Glass <sjg@chromium.org> wrote:
>> > > > > >
>> > > > > > Hi Ilias,
>> > > > > >
>> > > > > > On Tue, 29 Oct 2024 at 10:58, Ilias Apalodimas
>> > > > > > <ilias.apalodimas@linaro.org> wrote:
>> > > > > > >
>> > > > > > > Hi Simon,
>> > > > > > >
>> > > > > > > On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
>> > > > > > > >
>> > > > > > > > It is a bit of a pain to log EFI boot-services calls at present. The
>> > > > > > > > output goes to the console so cannot easily be inspected later. Also it
>> > > > > > > > would be useful to be able to store the log and review it later, perhaps
>> > > > > > > > after something has gone wrong.
>> > > > > > > >
>> > > > > > > > This series makes a start on implementing a log-to-buffer feature. It
>> > > > > > > > provides a simple 'efidebug log' command to inspect the buffer. For now,
>> > > > > > > > only memory allocations are logged.
>> > > > > > >
>> > > > > > > Why is this problem specific to EFI and no U-Boot in general? Do we
>> > > > > > > have a similar machinery for malloc()?
>> > > > > >
>> > > > > > Mostly because an app can make EFI calls and we want to know what they
>> > > > > > are, e.g. to debug them and figure out what might be wrong when
>> > > > > > something doesn't boot.
>> > > > >
>> > > > > EFI_PRINT() has been proven pretty useful for this. I don't personally
>> > > > > see the point of adding ~1300 lines of code to replace a print.
>> > > > > What would make more sense is teach EFI_PRINT to log errors in a buffer.
>> > > >
>> > > > Is that a NAK? Please be clear if you are reviewing the code or just
>> > > > rejecting the whole idea.
>> > >
>> > > For the idea, no. But I don't think what's implemented here is what we want.
>> > >
>> > > To track what EFI services are called, we already have EFI_ENTRY and EFI_EXIT.
>> > > Why don't we instead, add a logging service (and we already have
>> > > ftrace iirc) and plug it in the macros above?
>> > > That would make more sense not to mention way less code.
>> >
>> > I am wanting to programmatically log and manage what EFI_LOADER does,
>> > so that bootstd can present a high-level view of what is going on,
>> > e.g. which protocols are used, how much memory is allocated and where.
>> > So this is not just about logging text output.
>>
>> Why the EFI_LOADER only? Bootstd is supposed to cover more cases, so
>> why not a generic framework for all boot commands?
>
>This feels similar to the point I've made elsewhere in this overarching
>series, why not do this at existing common points in the code path?
>
The common code point is the log library. Just add an event there for which the test can register a handler.
With a log event you get:
function name
source location
message class
message text
message priority
and all of this with minimal invasiveness.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer
2024-11-20 20:05 ` Heinrich Schuchardt
@ 2024-11-21 2:19 ` Simon Glass
2024-12-02 0:12 ` Simon Glass
0 siblings, 1 reply; 40+ messages in thread
From: Simon Glass @ 2024-11-21 2:19 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Ilias Apalodimas, U-Boot Mailing List, AKASHI Takahiro,
Caleb Connolly, Dmitry Rokosov, Emil Kronborg, Etienne Carriere,
Francis Laniel, Hou Zhiqiang, Igor Prusov, Jonathan Humphreys,
Levi Yun, Marek Vasut, Masahisa Kojima, Mattijs Korpershoek,
Maxim Moskalets, Michal Simek, Neil Armstrong, Oliver Gaskell,
Patrick Rudolph, Quentin Schulz, Raymond Mao, Robert Marko,
Sam Protsenko, Sean Anderson, Sebastian Reichel, Sughosh Ganu,
Sumit Garg, Thomas Weißschuh, Vincent Stehlé,
Wei Ming Chen
Hi,
On Wed, 20 Nov 2024 at 13:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Am 20. November 2024 19:06:33 MEZ schrieb Tom Rini <trini@konsulko.com>:
> >On Wed, Nov 20, 2024 at 05:55:18PM +0200, Ilias Apalodimas wrote:
> >> Hi Simon,
> >>
> >> On Wed, 20 Nov 2024 at 17:37, Simon Glass <sjg@chromium.org> wrote:
> >> >
> >> > HI Ilias,
> >> >
> >> > On Fri, 1 Nov 2024 at 05:32, Ilias Apalodimas
> >> > <ilias.apalodimas@linaro.org> wrote:
> >> > >
> >> > > Hi Simon,
> >> > >
> >> > > On Thu, 31 Oct 2024 at 20:02, Simon Glass <sjg@chromium.org> wrote:
> >> > > >
> >> > > > Hi Ilias,
> >> > > >
> >> > > > On Tue, 29 Oct 2024 at 19:32, Ilias Apalodimas
> >> > > > <ilias.apalodimas@linaro.org> wrote:
> >> > > > >
> >> > > > > On Tue, 29 Oct 2024 at 17:45, Simon Glass <sjg@chromium.org> wrote:
> >> > > > > >
> >> > > > > > Hi Ilias,
> >> > > > > >
> >> > > > > > On Tue, 29 Oct 2024 at 10:58, Ilias Apalodimas
> >> > > > > > <ilias.apalodimas@linaro.org> wrote:
> >> > > > > > >
> >> > > > > > > Hi Simon,
> >> > > > > > >
> >> > > > > > > On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
> >> > > > > > > >
> >> > > > > > > > It is a bit of a pain to log EFI boot-services calls at present. The
> >> > > > > > > > output goes to the console so cannot easily be inspected later. Also it
> >> > > > > > > > would be useful to be able to store the log and review it later, perhaps
> >> > > > > > > > after something has gone wrong.
> >> > > > > > > >
> >> > > > > > > > This series makes a start on implementing a log-to-buffer feature. It
> >> > > > > > > > provides a simple 'efidebug log' command to inspect the buffer. For now,
> >> > > > > > > > only memory allocations are logged.
> >> > > > > > >
> >> > > > > > > Why is this problem specific to EFI and no U-Boot in general? Do we
> >> > > > > > > have a similar machinery for malloc()?
> >> > > > > >
> >> > > > > > Mostly because an app can make EFI calls and we want to know what they
> >> > > > > > are, e.g. to debug them and figure out what might be wrong when
> >> > > > > > something doesn't boot.
> >> > > > >
> >> > > > > EFI_PRINT() has been proven pretty useful for this. I don't personally
> >> > > > > see the point of adding ~1300 lines of code to replace a print.
> >> > > > > What would make more sense is teach EFI_PRINT to log errors in a buffer.
> >> > > >
> >> > > > Is that a NAK? Please be clear if you are reviewing the code or just
> >> > > > rejecting the whole idea.
> >> > >
> >> > > For the idea, no. But I don't think what's implemented here is what we want.
> >> > >
> >> > > To track what EFI services are called, we already have EFI_ENTRY and EFI_EXIT.
> >> > > Why don't we instead, add a logging service (and we already have
> >> > > ftrace iirc) and plug it in the macros above?
> >> > > That would make more sense not to mention way less code.
> >> >
> >> > I am wanting to programmatically log and manage what EFI_LOADER does,
> >> > so that bootstd can present a high-level view of what is going on,
> >> > e.g. which protocols are used, how much memory is allocated and where.
> >> > So this is not just about logging text output.
> >>
> >> Why the EFI_LOADER only? Bootstd is supposed to cover more cases, so
> >> why not a generic framework for all boot commands?
> >
> >This feels similar to the point I've made elsewhere in this overarching
> >series, why not do this at existing common points in the code path?
> >
>
> The common code point is the log library. Just add an event there for which the test can register a handler.
>
> With a log event you get:
>
> function name
> source location
> message class
> message text
> message priority
>
> and all of this with minimal invasiveness.
OK guys, I think I got the message :-)
I'm going to apply this to my tree for now. While I'm at it I think it
is time to go through my backlog and apply some other things that I'd
like in there.
Regards,
Simon
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer
2024-11-21 2:19 ` Simon Glass
@ 2024-12-02 0:12 ` Simon Glass
0 siblings, 0 replies; 40+ messages in thread
From: Simon Glass @ 2024-12-02 0:12 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Tom Rini, Ilias Apalodimas, U-Boot Mailing List, AKASHI Takahiro,
Caleb Connolly, Dmitry Rokosov, Emil Kronborg, Etienne Carriere,
Francis Laniel, Hou Zhiqiang, Igor Prusov, Jonathan Humphreys,
Levi Yun, Marek Vasut, Masahisa Kojima, Mattijs Korpershoek,
Maxim Moskalets, Michal Simek, Neil Armstrong, Oliver Gaskell,
Patrick Rudolph, Quentin Schulz, Raymond Mao, Robert Marko,
Sam Protsenko, Sean Anderson, Sebastian Reichel, Sughosh Ganu,
Sumit Garg, Thomas Weißschuh, Vincent Stehlé,
Wei Ming Chen
Hi,
On Wed, 20 Nov 2024 at 19:19, Simon Glass <sjg@chromium.org> wrote:
>
> Hi,
>
> On Wed, 20 Nov 2024 at 13:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > Am 20. November 2024 19:06:33 MEZ schrieb Tom Rini <trini@konsulko.com>:
> > >On Wed, Nov 20, 2024 at 05:55:18PM +0200, Ilias Apalodimas wrote:
> > >> Hi Simon,
> > >>
> > >> On Wed, 20 Nov 2024 at 17:37, Simon Glass <sjg@chromium.org> wrote:
> > >> >
> > >> > HI Ilias,
> > >> >
> > >> > On Fri, 1 Nov 2024 at 05:32, Ilias Apalodimas
> > >> > <ilias.apalodimas@linaro.org> wrote:
> > >> > >
> > >> > > Hi Simon,
> > >> > >
> > >> > > On Thu, 31 Oct 2024 at 20:02, Simon Glass <sjg@chromium.org> wrote:
> > >> > > >
> > >> > > > Hi Ilias,
> > >> > > >
> > >> > > > On Tue, 29 Oct 2024 at 19:32, Ilias Apalodimas
> > >> > > > <ilias.apalodimas@linaro.org> wrote:
> > >> > > > >
> > >> > > > > On Tue, 29 Oct 2024 at 17:45, Simon Glass <sjg@chromium.org> wrote:
> > >> > > > > >
> > >> > > > > > Hi Ilias,
> > >> > > > > >
> > >> > > > > > On Tue, 29 Oct 2024 at 10:58, Ilias Apalodimas
> > >> > > > > > <ilias.apalodimas@linaro.org> wrote:
> > >> > > > > > >
> > >> > > > > > > Hi Simon,
> > >> > > > > > >
> > >> > > > > > > On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg@chromium.org> wrote:
> > >> > > > > > > >
> > >> > > > > > > > It is a bit of a pain to log EFI boot-services calls at present. The
> > >> > > > > > > > output goes to the console so cannot easily be inspected later. Also it
> > >> > > > > > > > would be useful to be able to store the log and review it later, perhaps
> > >> > > > > > > > after something has gone wrong.
> > >> > > > > > > >
> > >> > > > > > > > This series makes a start on implementing a log-to-buffer feature. It
> > >> > > > > > > > provides a simple 'efidebug log' command to inspect the buffer. For now,
> > >> > > > > > > > only memory allocations are logged.
> > >> > > > > > >
> > >> > > > > > > Why is this problem specific to EFI and no U-Boot in general? Do we
> > >> > > > > > > have a similar machinery for malloc()?
> > >> > > > > >
> > >> > > > > > Mostly because an app can make EFI calls and we want to know what they
> > >> > > > > > are, e.g. to debug them and figure out what might be wrong when
> > >> > > > > > something doesn't boot.
> > >> > > > >
> > >> > > > > EFI_PRINT() has been proven pretty useful for this. I don't personally
> > >> > > > > see the point of adding ~1300 lines of code to replace a print.
> > >> > > > > What would make more sense is teach EFI_PRINT to log errors in a buffer.
> > >> > > >
> > >> > > > Is that a NAK? Please be clear if you are reviewing the code or just
> > >> > > > rejecting the whole idea.
> > >> > >
> > >> > > For the idea, no. But I don't think what's implemented here is what we want.
> > >> > >
> > >> > > To track what EFI services are called, we already have EFI_ENTRY and EFI_EXIT.
> > >> > > Why don't we instead, add a logging service (and we already have
> > >> > > ftrace iirc) and plug it in the macros above?
> > >> > > That would make more sense not to mention way less code.
> > >> >
> > >> > I am wanting to programmatically log and manage what EFI_LOADER does,
> > >> > so that bootstd can present a high-level view of what is going on,
> > >> > e.g. which protocols are used, how much memory is allocated and where.
> > >> > So this is not just about logging text output.
> > >>
> > >> Why the EFI_LOADER only? Bootstd is supposed to cover more cases, so
> > >> why not a generic framework for all boot commands?
> > >
> > >This feels similar to the point I've made elsewhere in this overarching
> > >series, why not do this at existing common points in the code path?
> > >
> >
> > The common code point is the log library. Just add an event there for which the test can register a handler.
> >
> > With a log event you get:
> >
> > function name
> > source location
> > message class
> > message text
> > message priority
> >
> > and all of this with minimal invasiveness.
>
> OK guys, I think I got the message :-)
>
> I'm going to apply this to my tree for now. While I'm at it I think it
> is time to go through my backlog and apply some other things that I'd
> like in there.
One point I didn't mention is that this series allows logging of all
calls, not just the ones that come in from the app. At present there
are quite a few efi_allocate_pages() calls which are entirely
internal.
Regards,
Simon
^ permalink raw reply [flat|nested] 40+ messages in thread