Re: [PATCH 6/8] cmd: efidebug: simplify printing GUIDs

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Mon, Jan 24, 2022 at 09:25:34AM +0100, Heinrich Schuchardt wrote:
> On 1/24/22 08:23, AKASHI Takahiro wrote:
> > On Fri, Jan 21, 2022 at 05:03:03PM +0100, Heinrich Schuchardt wrote:
> > > On 1/21/22 16:20, Simon Glass wrote:
> > > > Hi Heinrich,
> > > > 
> > > > On Sun, 16 Jan 2022 at 08:14, Heinrich Schuchardt
> > > > <heinrich.schuchardt@canonical.com> wrote:
> > > > > 
> > > > > Use "%pS" to print text representations of GUIDs.
> > > > > 
> > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > > > > ---
> > > > >    cmd/efidebug.c         | 160 ++---------------------------------------
> > > > >    include/efi_api.h      |   8 +++
> > > > >    include/efi_dt_fixup.h |   4 --
> > > > >    include/efi_rng.h      |   4 --
> > > > >    lib/uuid.c             | 116 ++++++++++++++++++++++++++++++
> > > > >    5 files changed, 128 insertions(+), 164 deletions(-)
> > > > > 
> > > > 
> > > > Does this blow up the image size? These strings only in the debug side before.
> > > 
> > > It was to avoid image size increase that I added
> > > +#ifdef CONFIG_CMD_EFIDEBUG
> > > 
> > > > 
> > > > Having said that, I would much rather see a string than a guid, which
> > > > I consider to be little more than an obfuscation.
> > > 
> > > That was my motivation. When debugging a boot failure I set DEBUG in
> > > lib/efi_loader/efi_boottime.c and reading GUIDs in the debug output was not
> > > helpful.
> > 
> > But setting DEBUG in efi_boottime.c doesn't lead to CONFIG_CMD_EFIDEBUG
> > being on in uuid.c. Do we want to have a more direct CONFIG?
> 
> We could use CONFIG_LOGLEVEL >= LOGL_DEBUG because once that loglevel is
> set you can use the log command to log one of the EFI related functions.

I'm not sure the log level is the best choice for controlling messages.
Showing a human-readable GUID would also be a help in case of errors.

> Overall the EFI debugging requires rethinking:

I definitely agree, no specific idea though.

-Takahiro Akashi

> If you simply add '#define DEFINE 1' to the top of
> lib/efi_loader/efi_boottime your screen will be flooded by function
> calls related to timer events. Probably those high frequency events
> should use LOGL_DEBUG_IO.
> 
> Best regards
> 
> Heinrich
> 
> > 
> > -Takahiro Akashi
> > 
> > > Best regards
> > > 
> > > Heinrich
> > > 
> > > > 
> > > > Regards,
> > > > Simon
>