| Message ID | 20241028124815.47262-1-sjg@chromium.org |
|---|---|
| Headers | show |
| Series | efi_loader: Add support for logging to a buffer | expand |
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 >
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 > >
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 > > >
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
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