Re: [RFC 1/1] efi_loader: rename 'efidebug boot' 'bootefi'

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Heinrich

On Mon, Jan 17, 2022 at 10:25:01AM +0100, Heinrich Schuchardt wrote:
> On 1/17/22 08:58, Ilias Apalodimas wrote:
> > On Mon, Jan 17, 2022 at 10:16:51AM +0900, AKASHI Takahiro wrote:
> > > On Sat, Jan 15, 2022 at 01:49:07AM +0100, Heinrich Schuchardt wrote:
> > > > The efidebug command was conceived for testing purposes.
> > > 
> > > Well, I initially implemented the command as an alternative of
> > > "EFI shell" as the shell was not able to run on EFI U-Boot at that time.
> > > 
> > > > The manipulation of boot options does better fit to the bootefi command
> > > > that is used to invoke the boot manager.
> > > 
> > > I believe that it would be best to have those two features
> > > in separate commands(/applications) since the bootefi/bootmgr be focused
> > > on booting EFI images while efidebug/EFI shell provides a kind of
> > > user interfaces for manipulating the system.
> > > 
> > > *If* you dare to move the code to bootefi/bootmgr, I'd ask you to honor
> > > and add my copyright to the file as "efidebug boot" feature is a core part
> > > of efidebug. Or export sub-command functions from efidebug.c and import
> > > them in bootefi.c.
> > 
> > I think renaming the efidebug command is overall good idea,  since it does
> > way more that debugging.  OTOH I think moving it to 'bootefi' is the wrong
> > way to go.  I'd be much happier if we kept bootefi for booting related
> > commands and purposes and rename 'efidebug' to 'efi'.  Then we could split
> > off the debug related commands to 'efi debug xxxxxxxxx' and put it under a
> > Kconfig option.
> 
> For me the important thing is that we should be able enable boot options
> related commands without the rest of efidebug to limit code size increase.
> 

Yes,  but isn't that doable by what I suggested?

> efidebug boot is only needed if CONFIG_CMD_BOOTEFI_BOOTMGR=y.
> 
> efidebug capsule is only needed for testing on QEMU, Sandbox if capsules are
> enabled. I can't see that a normal user would ever use it.
> 
> efidebug devices, drivers, dh, images, memmap, query, tables is only needed
> for debugging and should be disabled by default.


Yes,  those would be the options we could place under a new Kconfig, which enables
debugging capabilities on the command

Cheers
/Ilias
> 
> efidebug test is only needed on the Sandbox.
> 
> All commands lack documentation in /doc/usage/
> 
> Best regards
> 
> Heinrich