Re: [edk2-devel] [PATCH 5/5] ShellPkg: add BootManager library to add UEFI Shell menu option

["Laszlo Ersek" <lersek@redhat.com>]

On 05/26/20 18:13, Ard Biesheuvel wrote:
> Add a plug-in library for UiApp that creates a 'UEFI Shell' menu
> option at the root level which gives access to a form that allows
> the UEFI Shell to be launched.
>
> This gives the PlatformBootManagerLib implementation of the platform
> more flexibility in the way it handles boot options pointing to the
> UEFI Shell, which will typically be invoked with only the boot path
> connected on fast boots.
>
> This library may be incorporated to a platform build by adding a
> NULL resolution to the <LibraryClasses> section of the UiApp.inf
> {} block in the platform .DSC
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>  ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.inf |  45 ++++
>  ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.h   |  44 ++++
>  ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.c   | 258 ++++++++++++++++++++
>  ShellPkg/Library/ShellBootManagerLib/ShellBmStrings.uni      |  17 ++
>  ShellPkg/Library/ShellBootManagerLib/ShellBmVfr.Vfr          |  37 +++
>  5 files changed, 401 insertions(+)

I've had to go back to the blurb and re-read this part, to understand
the goal of this patch:

> - finally, add a plugin library for UiApp to expose the UEFI Shell via an
>   ordinary main menu option (this works around the fact that patch #3 will
>   result in the UEFI Shell disappearing from the Boot Manager list).
>   Entering the shell this way will resemble the old situation, given that
>   UiApp connects all devices and refreshes all boot options etc at launch.

If I understand correctly:

- patch #3 does two things: it clears LOAD_OPTION_ACTIVE (preventing the
  boot manager from auto-booting the shell), and sets LOAD_OPTION_HIDDEN
  (hiding the boot option from UiApp),

- patch #5 undoes LOAD_OPTION_HIDDEN, in effect -- it makes sure that we
  still see the shell option "somewhere" in UiApp (not among the boot
  options, but at the root level)

Can we:

- drop patch#5, and

- pass zero (0) as "Attributes" to PlatformRegisterFvBootOption() in
  patch#3, rather than LOAD_OPTION_HIDDEN?

Because, per spec, Attributes=0 should prevent the auto-booting of the
shell *without* hiding the shell boot option from the menu.

Thanks,
Laszlo