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

[Tom Rini <trini@konsulko.com>]

[-- 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 --]