[PATCH v2 0/7] efi_loader: Add support for logging to a buffer

[Simon Glass]

Hi Tom,

On Mon, 2 Dec 2024 at 07:16, Tom Rini <trini at konsulko.com> wrote:
>
> On Mon, Dec 02, 2024 at 01:24:27AM +0100, Heinrich Schuchardt wrote:
> > Am 2. Dezember 2024 01:12:36 MEZ schrieb Simon Glass <sjg at chromium.org>:
> > >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.
> >
> > In my previous review I suggested to move your work to the level of the logging library lib/log.c.
> >
> > Why are you reiterating your series without exposing why an EFI specific solution. should be desireable?
>
> Even with
> https://lore.kernel.org/u-boot/CAFLszTgE_HZEbG7B09_oo=XfugvH07ix=_0eoSGaEQ+KhyA4Qw@mail.gmail.com/
> it is unclear why this patch series rather than either/both:
> - Expand CONFIG_LOG to have a "file" or "bloblist" option
> - Use CONFIG_TRACE
>
> Do not match your needs. Especially the second option.

>From my side I'd like to change the conversation a little, to how to
land code, rather than why we should bother. "Code needs to land"
should be the motto. If someone has taken the time to create
something, our bias should be towards getting it in, with sufficient
changes to make it fit the project. There are cases where something is
just a bad idea, or should be done another way, but for people working
on major features or changes, biasing towards not landing the code is
just going to make them go elsewhere.

CONFIG_LOG with a bloblist option would be a great idea, but it's hard
to programmatically scan text...plus only the external call sites are
actually logged.
CONFIG_TRACE - ick, you want me to write a test which does a trace and
then scans it to see what happened? The trace doesn't even include
function arguments...

Regards,
Simon