[PATCH 00/15] efi_loader: Add support for logging to a buffer

[Heinrich Schuchardt]

Am 20. November 2024 19:06:33 MEZ schrieb Tom Rini <trini at 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 at chromium.org> wrote:
>> >
>> > HI Ilias,
>> >
>> > On Fri, 1 Nov 2024 at 05:32, Ilias Apalodimas
>> > <ilias.apalodimas at linaro.org> wrote:
>> > >
>> > > Hi Simon,
>> > >
>> > > On Thu, 31 Oct 2024 at 20:02, Simon Glass <sjg at chromium.org> wrote:
>> > > >
>> > > > Hi Ilias,
>> > > >
>> > > > On Tue, 29 Oct 2024 at 19:32, Ilias Apalodimas
>> > > > <ilias.apalodimas at linaro.org> wrote:
>> > > > >
>> > > > > On Tue, 29 Oct 2024 at 17:45, Simon Glass <sjg at chromium.org> wrote:
>> > > > > >
>> > > > > > Hi Ilias,
>> > > > > >
>> > > > > > On Tue, 29 Oct 2024 at 10:58, Ilias Apalodimas
>> > > > > > <ilias.apalodimas at linaro.org> wrote:
>> > > > > > >
>> > > > > > > Hi Simon,
>> > > > > > >
>> > > > > > > On Mon, 28 Oct 2024 at 14:48, Simon Glass <sjg at 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