Re: Proposition of a BmEnumerateBootOptions() hook.

[Laszlo Ersek <lersek@redhat.com>]

On 05/14/18 21:00, Marvin H?user wrote:
> Hey Star, Eric and everyone else,
>
> I have seen that some platforms add a Boot Option for the UEFI Shell
> in "PlatformBootManagerBeforeConsole()", which is called as part of
> the regular boot flow. This is surely beneficial for development
> platforms that are supposed to boot to UEFI Shell by default when no
> other option has been registered,

(Side point: I sure wish *all* platforms included the UEFI shell, one
way or another. Debugging issues is the *default* state of any computing
system (all software has bugs), so debug tools must be a first class
citizen. Forums are full of end-users asking for the UEFI shell on their
physical platforms; frequently because their platform firmware gives
them an extremely limited interface to managing boot and driver options,
and the shell is seen as a way out of that, justifiedly.)

> however for retail platforms it usually makes more sense to show the
> UEFI Boot Menu,

Note that this is already solved in BdsDxe (if I understand your point
anyway); please refer to commit d1de487dd2e7 ("MdeModulePkg/BdsDxe: fall
back to a Boot Manager Menu loop before hanging", 2017-11-27).

> which renders adding the Shell Boot Option as part of the regular boot
> flow obsolete and just adds up to the boot time. Meanwhile, there is a
> function in the UefiBootManagerLib, "BmEnumerateBootOptions()", which
> is called prior to entering the Boot Menu and, in my opinion, would be
> the perfect place to introduce another PlatformBootManagerLib hook,
> which retrieves platform-specific boot options, such as an UEFI Shell
> or other utilities like a Memory Test application.

Hmmm, I'm not sure. The only function that calls
BmEnumerateBootOptions() is EfiBootManagerRefreshAllBootOption(). The
latter is a public UefiBootManagerLib API that several
PlatformBootManagerLib instances call, usually from
PlatformBootManagerAfterConsole(). PlatformBootManagerAfterConsole() is
already a PlatformBootManagerLib hook, so the suggestion would result in
one hook calling back into another hook, of the same library instance:

  BdsEntry()                               [MdeModulePkg/Universal/BdsDxe/BdsEntry.c]
    PlatformBootManagerAfterConsole()      [SomePlatformPkg/Library/PlatformBootManagerLib/...]
      EfiBootManagerRefreshAllBootOption() [MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c]
        BmEnumerateBootOptions()           [MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c]
          PlatformBootManagerAnotherHook() [SomePlatformPkg/Library/PlatformBootManagerLib/...]

If I'm being honest, I'd like to avoid this -- if I'm in
PlatformBootManagerBeforeConsole() or PlatformBootManagerAfterConsole(),
I'd just like to do whatever's necessary by directly calling either
public UefiBootManagerLib APIs, or STATIC functions from the same
PlatformBootManagerLib instance that I'm already inside of.

Now, what I could see as useful is a public helper function in
UefiBootManagerLib that registers the shell boot option. This
functionality is usually duplicated across several
PlatformBootManagerLib instances.

I'll also mention another interface that the edk2-platforms project uses
-- several platforms there use the same PlatformBootManagerLib instance,
and they abstract out just the default set of platform boot options.
Please see

  [PATCH v4 0/2] add platform boot manager protocol
  http://mid.mail-archive.com/1524464514-14454-1-git-send-email-haojian.zhuang@linaro.org

Thanks
Laszlo