Re: Proposition of a BmEnumerateBootOptions() hook.

[Laszlo Ersek <lersek@redhat.com>]

On 05/15/18 15:02, Marvin Häuser wrote:
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Tuesday, May 15, 2018 10:22 AM
>> To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
>> devel@lists.01.org
>> Cc: eric.dong@intel.com; star.zeng@intel.com
>> Subject: Re: [edk2] Proposition of a BmEnumerateBootOptions() hook.
>>
>> On 05/14/18 21:00, Marvin Häuser wrote:
>>> [...] 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()
>
> I have misunderstood the current edk2 logic to be what I expected
> (partially), however I have just discovered that scanned boot options
> are added to NV and to BootOrder.

This is not a coincidence. That's what the
EfiBootManagerRefreshAllBootOption() API does, by design, and that's the
functionality for which platforms call the API.

> Furthermore, I did not mention another usage case for the proposed
> hook as I thought it was practically irrelevant to the edk2 community:
> Supporting Apple's bless boot concept. (For those who wonder,
> virtualizing macOS on Apple hardware is perfectly legal). So, I will
> have to explain a bit more in detail, in two parts.
>
> UEFI BOOT
>
> What I imagined to be the boot flow, by the specification and by my
> personal taste, is the following:
> 1. Boot BootOrder[] one-by-one. This connects the referenced drive and
>    attempts to start the specified file. This is the case today.
> 2. Boot *Recovery[]. This is the case today.
> 3. There are two alternatives here, based on platform preference.
> 3.1. Boot enumerated drives (via the algorithm defined in the UEFI
>      spec). I think this is not the case today.
> 3.2. Enumerate the drives (via the algorithm defined in the UEFI spec)
>      and show them in BootManagerMenuApp. I think this is done today
>      and is what I would prefer too.
>
> What I basically do not agree with in the list above are two nuances.
> 3.1 and 3.2: I would like to see support for a platform-defined logic
> of enumerating in addition to the one defined in the UEFI spec. This
> can be used for UEFI Shell, platform diagnostic apps and Apple bless.

I think platform-specific boot option generation -- enumeration of
various protocols that stand for different kinds of devices, and
generating boot options from them -- is already possible today.
Platforms call EfiBootManagerRefreshAllBootOption() because (a) it's
easy to call, (b) it generally does the right thing.

(Note that this API does not connect devices; it just scans devices and
turns whatever it finds into boot options.)

Nothing stops a platform from *not* calling
EfiBootManagerRefreshAllBootOption() -- the platform can simply skip
that and generate other options. Or the platform may generate options in
addition to those that are generated by the API.

> 3.2: I think adding them as volatile variables is the better approach,
> except for platform-specific code, which should be capable of adding
> NV options. The reasoning behind this is that usually the found
> volumes are removable volumes (such as USB Flash Drives) and will be
> cleaned away soon again anyway. "Extra wishes" for boot options should
> be added via the UEFI Setup menu or UEFI Shell.

I can't really comment on this -- the fact that Boot#### options are
non-volatile comes from the UEFI spec. I'm not immediately sure what I
think of their suggested volatility, but this is likely something for
the USWG to discuss.

> The reason why I still wish for platform code to be able to add NV
> options as part of enumeration is that FV-embedded tools are indeed NV
> (Shell etc.) and will not need to be cleaned up, as well as that we're
> not in an utopia. Sometimes an unwanted purge of boot option happens
> ("CMOS reset" on boards, etc) and consumer platform provides do not
> want to leave their users with a constant boot to BootMenu or no
> option at all. Platform code often does currently and probably should
> add a NV Windows boot option in the proposed hook (this should of
> course not be part of edk2). Furthermore, if an OS boot option is
> rendered invalid, usually there is no OsRecovery option available as
> proposed in the specification.
>
> Considering the boot protocol I just described, I think that the call
> in PlatformBootManagerAfterConsole() is not a good idea, because it
> refreshes boot options unconditionally. When the first element of
> BootOrder[] can be booted, there is no reason to do so and it costs
> time. Such a refresh makes sense when booting the first option fails
> though (to make sure it is purged to not delay boot again).

I think there are two possibilities for disagreement here. First, if the
first boot option cares about the full set of Boot#### options (for
whatever reasons), then this is an incompatible change.

Second, just because a boot option fails, it should not be removed. For
example, a netboot option could fail, or a USB drive might not be
connected (temporarily). I don't think that's grounds enough for
summarily removing a boot option, in shared reference code.

> The function is also called by BootManagerMenuApp in the entry point,
> which makes a lot of sense. In the scenario described above, "scanned"
> boot options are added only when the NV ones cannot be booted and
> MenuApp is rendered in consequence. It basically moves the scanning
> delay from the "NV phase" to the following "scanning phase", which is
> not supposed to be entered when booting a "sane" platform.

For OVMF specifically, calling EfiBootManagerRefreshAllBootOption() at
every boot is necessary. The set of bootable devices and their desired
boot order may change at every boot, and the boot order spec that comes
from QEMU isn't expressive enough for the *generation* of boot options;
it's expressive enough only for the *filtering* and reordering of boot
options.

> APPLE BLESS
>
> This might be interesting for the OVMF maintainers.
> I did not want to mention this concept at first, because I don't think
> there is a terrible huge demand or interest, however I would like to
> be able to implement Apple bless support for OVMF without having to
> fork and modify edk2 drivers.
> I did not check about a concrete way of implementation, however I will
> shortly describe what needs to be involved.
>
> Secondary partitions: Supporting this will be easy when accepting the
> hook proposed above and considering my comments regarding NV vs
> volatile variables. No boot options are proactively added for those
> installs and they are scanned on demand, which can be done entirely in
> the proposed hook function.

I think it could also be done in AfterConsole(). I realize it might
incur more pflash traffic -- like any other Boot#### option generation
-- than what you might consider optimal, but functionally that shouldn't
be a barrier.

> Primary partition: The so-called "Startup Volume" unfortunately is a
> bit trickier. For it, a practically invalid Boot Option is added,
> which is an expanded device path to the volume to be booted, however
> without having a File Device Path Node appended.

This doesn't immediately seem invalid -- if memory serves, you can have
\EFI\BOOT\BOOT<arch>.efi on fixed media as well, and if a boot option
only names the HD() in question, that default boot path will be launched
off of it.

> I had hoped to be able to use EFI_LOAD_FILE_PROTOCOL, however
> obviously EFI_SIMPLE_FILE_SYSTEM_PROTOCOL will attach to the mentioned
> DevicePath, which means LoadFile will not be called. Support for this
> would need to be done via a platform-specific error handler for when
> the DevicePath is determined to be invalid, which can then either fix
> it up and return success or fail as well. I have not looked into this
> in detail and I can understand if you are not interested in supporting
> such a scenario. However if you do, I will look into it as soon as
> possible and probably perform a few tests in OVMF.

I have a more general comment in the end: I doubt I could legally test
an OSX guest on non-Apple hardware, so I won't try (and I'll most likely
not buy or otherwise procure OSX, let alone Apple hardware, just for
this). That means, if you post patches, my main focus will be on keeping
the current behavior regression-free, and (secondarily) the
implementation preferably simple & separated.

I've added Gabriel and Gerd to the CC list because I believe they might
be interested in OSX guests (I seem to remember that a sizeable
out-of-tree patchset is necessary for OSX guests anyway).

Thanks,
Laszlo