Proposition of a BmEnumerateBootOptions() hook.

Proposition of a BmEnumerateBootOptions() hook.

[Marvin H?user]

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, however for retail platforms it usually makes more sense to show the UEFI Boot Menu, 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.
If you have a few spare minutes, I'll be happy for feedback.

Thanks in advance for your time.

Best regards,
Marvin

Re: Proposition of a BmEnumerateBootOptions() hook.

[Zeng, Star]

Cc Ray.

Thanks,
Star
From: Marvin H?user [mailto:Marvin.Haeuser@outlook.com]
Sent: Tuesday, May 15, 2018 3:00 AM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: Proposition of a BmEnumerateBootOptions() hook.

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, however for retail platforms it usually makes more sense to show the UEFI Boot Menu, 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.
If you have a few spare minutes, I'll be happy for feedback.

Thanks in advance for your time.

Best regards,
Marvin

Re: Proposition of a BmEnumerateBootOptions() hook.

[Laszlo Ersek]

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

Re: Proposition of a BmEnumerateBootOptions() hook.

[Marvin Häuser]

Hi Laszlo, thanks for your feedback!
Comments are inline.

I also CC'd Jordan and Ard because of OVMF references in the third comment.
Thanks in advance for your time!

I have rewritten a few parts multiple times, so, if something doesn't make sense (because I forgot to rework it in the context), please feel free to ask for clarification.

Best regards,
Marvin

> -----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:
> > 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.)

Definitely, sadly capacity and OEM mentality are still issues.
My point is just that a common user should not land in UEFI Shell by accident, but it should be explicitly launched.

> 
> > 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).

Oh interesting, thanks! That might resolve it, but I didn't check it out yet.

> 
> > 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()

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. 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.
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.
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).
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.

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.
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. 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.

> 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'm not sure if this is needed, but I guess it could help sharing code.

> 
> 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

I'm not sure whether a protocol is the best way to go, as I suppose only BDS is using it in the end.

> 
> Thanks
> Laszlo

Re: Proposition of a BmEnumerateBootOptions() hook.

[Laszlo Ersek]

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

Re: Proposition of a BmEnumerateBootOptions() hook.

[Gabriel L. Somlo]

On Tue, May 15, 2018 at 03:52:34PM +0200, Laszlo Ersek wrote:
>
> [...]
>
> > 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).

For whatever it's worth:

The size of the out-of-tree patchset (available at github.com/gsomlo/edk2,
branches gls-hfsplus -> gls-macboot -> gls-miscopt, with the latter
containing everything, cumulatively) is mainly due to the HFS+ driver
needed to access OSX disk images. The 'macboot' bits are from a GSoC
project by Reza Jelveh, and I haven't had a chance to really understand
how they work yet, since I think HFS+ support in a form acceptable to EDK2
are the bigger priority (and "gls-hfsplus" is not in a form acceptable
to EDK2 at the moment :)

Anyhow, the patched OVMF can boot Sierra right now, there's an
only-slightly-outdated writeup about it at
www.contrib.andrew.cmu.edu/~somlo/OSXKVM/

My long-term wish is to get everything cleaned up and palatable for
upstream inclusion, but haven't found time to really get into it over
the last couple of years.

Cheers,
--Gabriel

Re: Proposition of a BmEnumerateBootOptions() hook.

[Marvin Häuser]

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, May 15, 2018 3:53 PM
> To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
> devel@lists.01.org
> Cc: eric.dong@intel.com; star.zeng@intel.com; jordan.l.justen@intel.com;
> ard.biesheuvel@linaro.org; ruiyu.ni@intel.com; Gabriel L. Somlo (GMail)
> <gsomlo@gmail.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: Re: [edk2] Proposition of a BmEnumerateBootOptions() hook.
> 
> 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.

It surely is possible today, the proposition is more concerned about performance and "best practice".
I simply don't see a good reason to perform scanning and boot option writing when there is no code to read that (yet).
Of course this makes it come down to maintainer's preference because there is no significant demand.

> 
> (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.

That is perfectly valid for the Status Quo, however if my NV vs volatile comment should be considered, it will not be because it relies on a proactive BootMenuApp call (which happens to be to EfiBootManagerRefreshAllBootOption()).

> 
> > 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.

I think I might have been unclear here, I was solely referring to the Boot options created by the enumeration process and not Boot#### in general.
I wouldn't know of anything restricting all Boot#### variables to be NV, do you happen to have a quote?

> 
> > 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.

This is definitely true, however I'm not sure about the difference "all currently valid options" and "all currently set options".
If that boot option happens to be a Boot Menu editor of some sort, the proposed behavior would remain fine, in my opinion.
Otherwise, you're right with your comment, such as if a custom Boot Menu GUI is invoked and relies on all Boot#### to be set.
I don't actually think the specification demands all options to be up-to-date, so maybe the idea of a protocol handling such things is great after all?

> 
> 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.

Oh, I was just talking about the possibility, because the current code does remove options under certain conditions.
However, for USB devices as you have mentioned, I do see this good practice so long as the option is volatile.
There is no point in exposing a boot option to a removable device that is not attached if not to prevent unnecessary Flash cycles as far as I can think.
Do you happen to have worries about a scenario I did not consider?

> 
> > 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.

Sure, that comment was mostly for end-user platforms.

> 
> > 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.

Yes, those options would basically to be rewritten or at least verified on every boot (which requires a boot option refresh).
Verification has the disadvantage that it might catch user-created options, however I saw the BM code attaching own OptionalData, which might be sufficient to distinguish.

> 
> > 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 think for a Boot#### option, it is always invalid, because the default path is defined to be used in the "scanning phase" once no Boot#### variable could be found.
Also, unfortunately the path is also stored via bless and definitely never the UEFI standard one, so supporting this vendor-specific scenario definitely requires special handling.
Do you have any opinion on the proposed second hook?

> 
> > 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.

Thank you that you are ready to merge changes you cannot even really test.
This is probably off to some point in the future to make sure to supply well-tested patches.

> 
> 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

Re: Proposition of a BmEnumerateBootOptions() hook.

[Laszlo Ersek]

On 05/15/18 17:38, Marvin Häuser wrote:
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Tuesday, May 15, 2018 3:53 PM
>> To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
>> devel@lists.01.org
>> Cc: eric.dong@intel.com; star.zeng@intel.com; jordan.l.justen@intel.com;
>> ard.biesheuvel@linaro.org; ruiyu.ni@intel.com; Gabriel L. Somlo (GMail)
>> <gsomlo@gmail.com>; Gerd Hoffmann <kraxel@redhat.com>
>> Subject: Re: [edk2] Proposition of a BmEnumerateBootOptions() hook.
>>
>> On 05/15/18 15:02, Marvin Häuser wrote:

>>> 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.
> 
> I think I might have been unclear here, I was solely referring to the Boot options created by the enumeration process and not Boot#### in general.
> I wouldn't know of anything restricting all Boot#### variables to be NV, do you happen to have a quote?

See "3.3 Globally Defined Variables" and "Table 10. Global Variables" in
the UEFI-2.7 spec:

  [...] The variables with an attribute of NV are nonvolatile. [...]

  [...]

  Variable Name  Attribute   Description
  -------------  ---------   -----------
  [...]
  Boot####       NV, BS, RT  [...]
  [...]

>> 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.
> 
> Oh, I was just talking about the possibility, because the current code does remove options under certain conditions.
> However, for USB devices as you have mentioned, I do see this good practice so long as the option is volatile.
> There is no point in exposing a boot option to a removable device that is not attached if not to prevent unnecessary Flash cycles as far as I can think.
> Do you happen to have worries about a scenario I did not consider?

Sure; it's a simple scenario: the user might want the system to
automatically boot off of a USB drive whenever they connect it, before
powering on or rebooting the system, rather than boot from the hard drive.

(For USB devices, "USB WWID" and "USB Class" device path nodes are
defined by the spec, so the user isn't even expected to plug the drive
into the same USB port, for the drive to be found uniquely.)

>>> 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 think for a Boot#### option, it is always invalid,

I disagree. I'll have to defer to Ray here, but in my experience, edk2
practice is that if you have a boot option whose devpath ends in a HD()
node, then an UEFI image on that partition under the filepath
\EFI\BOOT\BOOT<arch>.efi will be tried for booting. As far as I can see,
this is neither mandated nor forbidden by the spec. Anyway, for Ray to
answer authoritatively.

> because the default path is defined to be used in the "scanning phase" once no Boot#### variable could be found.
> Also, unfortunately the path is also stored via bless and definitely never the UEFI standard one, so supporting this vendor-specific scenario definitely requires special handling.
> Do you have any opinion on the proposed second hook?

Personally I remain unconvinced that the second hook is needed (to be
invoked from core BdsDxe or UefiBootManagerLib code). But, that's just
my opinion :)

In general I find that library APIs (especially a *set* of library APIs)
are very hard to design (because we can't see the future). So, "organic
growth" works better in practice (even if it leads to "less well
organized" sets of APIs). This is to say that, if you can propose a hook
and immediately demonstrate that it saves code for, and/or simplifies,
multiple platforms, then the new API might be justified. (Personally, I
usually argue for "three call sites" as a minimum. Two might be enough
if the code extracted is particularly baroque, or undergoes frequent
changes.)

>> 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.
> 
> Thank you that you are ready to merge changes you cannot even really test.

I definitely intend to regression test any such changes :)

OTOH, in any larger project, any given maintainer is surely unable to
integration-test *all* features in the subsystem that they co-maintain.
What's important is that each major feature has at least one maintainer
that can integration-test it (possibly with the help of an automated
test suite).

For example, I totally cannot test Xen code in OvmfPkg or ArmVirtPkg,
but that's why we have several permanent Reviewers from the Xen project,
for OvmfPkg and ArmVirtPkg.

Thanks,
Laszlo

Re: Proposition of a BmEnumerateBootOptions() hook.

[Marvin Häuser]

Thanks for your feedback!
Comments inline

Regards,
Marvin.

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, May 15, 2018 6:12 PM
> To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
> devel@lists.01.org
> Cc: gsomlo@gmail.com; ard.biesheuvel@linaro.org; ruiyu.ni@intel.com;
> eric.dong@intel.com; star.zeng@intel.com; jordan.l.justen@intel.com
> Subject: Re: [edk2] Proposition of a BmEnumerateBootOptions() hook.
> 
> On 05/15/18 17:38, Marvin Häuser wrote:
> >> -----Original Message-----
> >> From: Laszlo Ersek <lersek@redhat.com>
> >> Sent: Tuesday, May 15, 2018 3:53 PM
> >> To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
> >> devel@lists.01.org
> >> Cc: eric.dong@intel.com; star.zeng@intel.com;
> >> jordan.l.justen@intel.com; ard.biesheuvel@linaro.org;
> >> ruiyu.ni@intel.com; Gabriel L. Somlo (GMail) <gsomlo@gmail.com>; Gerd
> >> Hoffmann <kraxel@redhat.com>
> >> Subject: Re: [edk2] Proposition of a BmEnumerateBootOptions() hook.
> >>
> >> On 05/15/18 15:02, Marvin Häuser wrote:
> 
> >>> 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.
> >
> > I think I might have been unclear here, I was solely referring to the Boot
> options created by the enumeration process and not Boot#### in general.
> > I wouldn't know of anything restricting all Boot#### variables to be NV, do
> you happen to have a quote?
> 
> See "3.3 Globally Defined Variables" and "Table 10. Global Variables" in the
> UEFI-2.7 spec:
> 
>   [...] The variables with an attribute of NV are nonvolatile. [...]
> 
>   [...]
> 
>   Variable Name  Attribute   Description
>   -------------  ---------   -----------
>   [...]
>   Boot####       NV, BS, RT  [...]
>   [...]

I had hoped it was a recommendation, but indeed it seems to be mandatory.
What's your opinion on this fact and how do you think the chances are NV could be made optional?

> 
> >> 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.
> >
> > Oh, I was just talking about the possibility, because the current code does
> remove options under certain conditions.
> > However, for USB devices as you have mentioned, I do see this good
> practice so long as the option is volatile.
> > There is no point in exposing a boot option to a removable device that is
> not attached if not to prevent unnecessary Flash cycles as far as I can think.
> > Do you happen to have worries about a scenario I did not consider?
> 
> Sure; it's a simple scenario: the user might want the system to automatically
> boot off of a USB drive whenever they connect it, before powering on or
> rebooting the system, rather than boot from the hard drive.

Remember that point was made in the context of enumeration. What I was saying is that boot options created by the enum code, for removable drives, are fine to be gone (as they are volatile, or proactively removed when NV) after a reboot.
If a user wants to boot from such an USB device by default, in my opinion they should manually create a boot option for it. Otherwise, if I do not misunderstand your point, you are suggesting the firmware to keep track of every single bootable USB device ever attached.

> 
> (For USB devices, "USB WWID" and "USB Class" device path nodes are
> defined by the spec, so the user isn't even expected to plug the drive into
> the same USB port, for the drive to be found uniquely.)

Actually didn't know that, thanks for the fact!

> 
> >>> 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 think for a Boot#### option, it is always invalid,
> 
> I disagree. I'll have to defer to Ray here, but in my experience, edk2 practice
> is that if you have a boot option whose devpath ends in a HD() node, then an
> UEFI image on that partition under the filepath \EFI\BOOT\BOOT<arch>.efi
> will be tried for booting. As far as I can see, this is neither mandated nor
> forbidden by the spec. Anyway, for Ray to answer authoritatively.

Indeed it is neither explicitly allowed nor forbidden, I concluded it being invalid from the strictness of how the booting protocol is defined.
If it is forbidden, it is implicit, but if it's not the case, the better it is for this purpose.

> 
> > because the default path is defined to be used in the "scanning phase"
> once no Boot#### variable could be found.
> > Also, unfortunately the path is also stored via bless and definitely never the
> UEFI standard one, so supporting this vendor-specific scenario definitely
> requires special handling.
> > Do you have any opinion on the proposed second hook?
> 
> Personally I remain unconvinced that the second hook is needed (to be
> invoked from core BdsDxe or UefiBootManagerLib code). But, that's just my
> opinion :)

Well, the only alternative I see are modifying the entry pre-BDS (slightly unclean), forking off (unpleasant) or not supporting it at all (unpleasant too, but support is not terribly important, although would be nice to have).

> 
> In general I find that library APIs (especially a *set* of library APIs) are very
> hard to design (because we can't see the future). So, "organic growth" works
> better in practice (even if it leads to "less well organized" sets of APIs). This is
> to say that, if you can propose a hook and immediately demonstrate that it
> saves code for, and/or simplifies, multiple platforms, then the new API might
> be justified. (Personally, I usually argue for "three call sites" as a minimum.
> Two might be enough if the code extracted is particularly baroque, or
> undergoes frequent
> changes.)

The "problem" is that both hooks are not introduced for sharing code but for giving the platform developer more freedom without forking off.
Whether such are accepted ends up being entirely the maintainer's preference, I guess.

> 
> >> 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.
> >
> > Thank you that you are ready to merge changes you cannot even really
> test.
> 
> I definitely intend to regression test any such changes :)

Sure, I just often come across a "I don't merge what I cannot test" mentality. :)

> 
> OTOH, in any larger project, any given maintainer is surely unable to
> integration-test *all* features in the subsystem that they co-maintain.
> What's important is that each major feature has at least one maintainer that
> can integration-test it (possibly with the help of an automated test suite).
> 
> For example, I totally cannot test Xen code in OvmfPkg or ArmVirtPkg, but
> that's why we have several permanent Reviewers from the Xen project, for
> OvmfPkg and ArmVirtPkg.
> 
> Thanks,
> Laszlo

Re: Proposition of a BmEnumerateBootOptions() hook.

[Laszlo Ersek]

On 05/15/18 19:14, Marvin Häuser wrote:
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Tuesday, May 15, 2018 6:12 PM
>> To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
>> devel@lists.01.org
>> Cc: gsomlo@gmail.com; ard.biesheuvel@linaro.org; ruiyu.ni@intel.com;
>> eric.dong@intel.com; star.zeng@intel.com; jordan.l.justen@intel.com
>> Subject: Re: [edk2] Proposition of a BmEnumerateBootOptions() hook.

>> See "3.3 Globally Defined Variables" and "Table 10. Global Variables"
>> in the UEFI-2.7 spec:
>>
>>   [...] The variables with an attribute of NV are nonvolatile. [...]
>>
>>   [...]
>>
>>   Variable Name  Attribute   Description
>>   -------------  ---------   -----------
>>   [...]
>>   Boot####       NV, BS, RT  [...]
>>   [...]
>
> I had hoped it was a recommendation, but indeed it seems to be
> mandatory.
> What's your opinion on this fact

I have always accepted it as a fact, and haven't really thought about
volatile Boot#### options ever :)

> and how do you think the chances are NV could be made optional?

I wouldn't like to guess :)

>>>> 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.
>>>
>>> Oh, I was just talking about the possibility, because the current
>>> code does remove options under certain conditions.
>>> However, for USB devices as you have mentioned, I do see this good
>>> practice so long as the option is volatile.
>>> There is no point in exposing a boot option to a removable device
>>> that is not attached if not to prevent unnecessary Flash cycles as
>>> far as I can think.
>>> Do you happen to have worries about a scenario I did not consider?
>>
>> Sure; it's a simple scenario: the user might want the system to
>> automatically boot off of a USB drive whenever they connect it,
>> before powering on or rebooting the system, rather than boot from the
>> hard drive.
>
> Remember that point was made in the context of enumeration. What I was
> saying is that boot options created by the enum code, for removable
> drives, are fine to be gone (as they are volatile, or proactively
> removed when NV) after a reboot.
> If a user wants to boot from such an USB device by default, in my
> opinion they should manually create a boot option for it. Otherwise,
> if I do not misunderstand your point, you are suggesting the firmware
> to keep track of every single bootable USB device ever attached.

Thanks for the clarification.

I see this all as platform policy. It's up to the platform to
auto-generate a boot option for a USB drive (either by calling
EfiBootManagerRefreshAllBootOption() or by other means). It's also up to
the platform to remove preexistent non-volatile options that it deems
unnecessary, or wrong.

I'm not suggesting that the platform keep track of every single bootable
USB device ever attached. The scenario I described was indeed that the
user once adds such a boot option manually, and then the system should
likely not remove it automatically. I see now that your point was about
auto-generated options only. About those, I think that if a platform
calls EfiBootManagerRefreshAllBootOption(), then the platform is also
responsible for pruning the auto-generated options that are no longer
valid.

In OVMF we do both the auto-generation and the (quite heavy-handed!)
pruning as well. It's mostly very platform-specific code. One example
that I can name without needing many details is the
RemoveStaleFvFileOptions() function.

>> In general I find that library APIs (especially a *set* of library
>> APIs) are very hard to design (because we can't see the future). So,
>> "organic growth" works better in practice (even if it leads to "less
>> well organized" sets of APIs). This is to say that, if you can
>> propose a hook and immediately demonstrate that it saves code for,
>> and/or simplifies, multiple platforms, then the new API might be
>> justified. (Personally, I usually argue for "three call sites" as a
>> minimum. Two might be enough if the code extracted is particularly
>> baroque, or undergoes frequent changes.)
>
> The "problem" is that both hooks are not introduced for sharing code
> but for giving the platform developer more freedom without forking
> off.
> Whether such are accepted ends up being entirely the maintainer's
> preference, I guess.

You could show the maintainer the two implementations side-by-side: what
it takes for the platform in question to implement the desired
functionality without the hook (if that's possible at all), and with the
hook. Double work, yes, but sometimes nothing else enlightens
maintainers. (I know this from both the contributor and the maintainer
sides.)

Thanks,
Laszlo

Re: Proposition of a BmEnumerateBootOptions() hook.

[Ni, Ruiyu]

Thanks/Ray

> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Marvin
> Häuser
> Sent: Wednesday, May 16, 2018 1:15 AM
> To: Laszlo Ersek <lersek@redhat.com>; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>;
> ard.biesheuvel@linaro.org; Justen, Jordan L <jordan.l.justen@intel.com>;
> gsomlo@gmail.com; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] Proposition of a BmEnumerateBootOptions() hook.
> 
> Thanks for your feedback!
> Comments inline
> 
> Regards,
> Marvin.
> 
> > -----Original Message-----
> > From: Laszlo Ersek <lersek@redhat.com>
> > Sent: Tuesday, May 15, 2018 6:12 PM
> > To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
> > devel@lists.01.org
> > Cc: gsomlo@gmail.com; ard.biesheuvel@linaro.org; ruiyu.ni@intel.com;
> > eric.dong@intel.com; star.zeng@intel.com; jordan.l.justen@intel.com
> > Subject: Re: [edk2] Proposition of a BmEnumerateBootOptions() hook.
> >
> > On 05/15/18 17:38, Marvin Häuser wrote:
> > >> -----Original Message-----
> > >> From: Laszlo Ersek <lersek@redhat.com>
> > >> Sent: Tuesday, May 15, 2018 3:53 PM
> > >> To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
> > >> devel@lists.01.org
> > >> Cc: eric.dong@intel.com; star.zeng@intel.com;
> > >> jordan.l.justen@intel.com; ard.biesheuvel@linaro.org;
> > >> ruiyu.ni@intel.com; Gabriel L. Somlo (GMail) <gsomlo@gmail.com>;
> > >> Gerd Hoffmann <kraxel@redhat.com>
> > >> Subject: Re: [edk2] Proposition of a BmEnumerateBootOptions() hook.
> > >>
> > >> On 05/15/18 15:02, Marvin Häuser wrote:
> >
> > >>> 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.
> > >
> > > I think I might have been unclear here, I was solely referring to
> > > the Boot
> > options created by the enumeration process and not Boot#### in general.
> > > I wouldn't know of anything restricting all Boot#### variables to be
> > > NV, do
> > you happen to have a quote?
> >
> > See "3.3 Globally Defined Variables" and "Table 10. Global Variables"
> > in the
> > UEFI-2.7 spec:
> >
> >   [...] The variables with an attribute of NV are nonvolatile. [...]
> >
> >   [...]
> >
> >   Variable Name  Attribute   Description
> >   -------------  ---------   -----------
> >   [...]
> >   Boot####       NV, BS, RT  [...]
> >   [...]
> 
> I had hoped it was a recommendation, but indeed it seems to be mandatory.
> What's your opinion on this fact and how do you think the chances are NV
> could be made optional?
> 
> >
> > >> 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.
> > >
> > > Oh, I was just talking about the possibility, because the current
> > > code does
> > remove options under certain conditions.
> > > However, for USB devices as you have mentioned, I do see this good
> > practice so long as the option is volatile.
> > > There is no point in exposing a boot option to a removable device
> > > that is
> > not attached if not to prevent unnecessary Flash cycles as far as I can think.
> > > Do you happen to have worries about a scenario I did not consider?
> >
> > Sure; it's a simple scenario: the user might want the system to
> > automatically boot off of a USB drive whenever they connect it, before
> > powering on or rebooting the system, rather than boot from the hard drive.
> 
> Remember that point was made in the context of enumeration. What I was
> saying is that boot options created by the enum code, for removable drives,
> are fine to be gone (as they are volatile, or proactively removed when NV)
> after a reboot.
> If a user wants to boot from such an USB device by default, in my opinion
> they should manually create a boot option for it. Otherwise, if I do not
> misunderstand your point, you are suggesting the firmware to keep track of
> every single bootable USB device ever attached.
> 
> >
> > (For USB devices, "USB WWID" and "USB Class" device path nodes are
> > defined by the spec, so the user isn't even expected to plug the drive
> > into the same USB port, for the drive to be found uniquely.)
> 
> Actually didn't know that, thanks for the fact!
> 
> >
> > >>> 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 think for a Boot#### option, it is always invalid,
> >
> > I disagree. I'll have to defer to Ray here, but in my experience, edk2
> > practice is that if you have a boot option whose devpath ends in a
> > HD() node, then an UEFI image on that partition under the filepath
> > \EFI\BOOT\BOOT<arch>.efi will be tried for booting. As far as I can
> > see, this is neither mandated nor forbidden by the spec. Anyway, for Ray
> to answer authoritatively.
> 
> Indeed it is neither explicitly allowed nor forbidden, I concluded it being
> invalid from the strictness of how the booting protocol is defined.
> If it is forbidden, it is implicit, but if it's not the case, the better it is for this
> purpose.
> 
> >
> > > because the default path is defined to be used in the "scanning phase"
> > once no Boot#### variable could be found.
> > > Also, unfortunately the path is also stored via bless and definitely
> > > never the
> > UEFI standard one, so supporting this vendor-specific scenario
> > definitely requires special handling.
> > > Do you have any opinion on the proposed second hook?
> >
> > Personally I remain unconvinced that the second hook is needed (to be
> > invoked from core BdsDxe or UefiBootManagerLib code). But, that's just
> > my opinion :)
> 
> Well, the only alternative I see are modifying the entry pre-BDS (slightly
> unclean), forking off (unpleasant) or not supporting it at all (unpleasant too,
> but support is not terribly important, although would be nice to have).
> 
> >
> > In general I find that library APIs (especially a *set* of library
> > APIs) are very hard to design (because we can't see the future). So,
> > "organic growth" works better in practice (even if it leads to "less
> > well organized" sets of APIs). This is to say that, if you can propose
> > a hook and immediately demonstrate that it saves code for, and/or
> > simplifies, multiple platforms, then the new API might be justified.
> (Personally, I usually argue for "three call sites" as a minimum.
> > Two might be enough if the code extracted is particularly baroque, or
> > undergoes frequent
> > changes.)
> 
> The "problem" is that both hooks are not introduced for sharing code but for
> giving the platform developer more freedom without forking off.
> Whether such are accepted ends up being entirely the maintainer's
> preference, I guess.

Forking off is never an option. In my opinion, if a generic library/driver needs
forking, that's a failure indicator of design.
Freedom is very important. I have seen many internal platform BDS
implementations. The platform requirements are very different and strange sometimes.
So the beforeconsole/afterconsole hook points were used.
They are very natural. First hook point is when the platform doesn't have mouth and ears.
The other hook point is when platform can speak and hear.

> 
> >
> > >> 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.
> > >
> > > Thank you that you are ready to merge changes you cannot even really
> > test.
> >
> > I definitely intend to regression test any such changes :)
> 
> Sure, I just often come across a "I don't merge what I cannot test"
> mentality. :)
> 
> >
> > OTOH, in any larger project, any given maintainer is surely unable to
> > integration-test *all* features in the subsystem that they co-maintain.
> > What's important is that each major feature has at least one
> > maintainer that can integration-test it (possibly with the help of an
> automated test suite).
> >
> > For example, I totally cannot test Xen code in OvmfPkg or ArmVirtPkg,
> > but that's why we have several permanent Reviewers from the Xen
> > project, for OvmfPkg and ArmVirtPkg.
> >
> > Thanks,
> > Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: Proposition of a BmEnumerateBootOptions() hook.

[Marvin Häuser]

Thanks for your comment,
Comments are inline

Regards,
Marvin.

> -----Original Message-----
> From: Ni, Ruiyu <ruiyu.ni@intel.com>
> Sent: Thursday, May 17, 2018 9:58 AM
> To: Marvin Häuser <Marvin.Haeuser@outlook.com>; Laszlo Ersek
> <lersek@redhat.com>; edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; ard.biesheuvel@linaro.org; Justen,
> Jordan L <jordan.l.justen@intel.com>; gsomlo@gmail.com; Zeng, Star
> <star.zeng@intel.com>
> Subject: RE: [edk2] Proposition of a BmEnumerateBootOptions() hook.
> 
> 
> 
> Thanks/Ray
> 
> > -----Original Message-----
> > From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Marvin
> > Häuser
> > Sent: Wednesday, May 16, 2018 1:15 AM
> > To: Laszlo Ersek <lersek@redhat.com>; edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>;
> > ard.biesheuvel@linaro.org; Justen, Jordan L
> > <jordan.l.justen@intel.com>; gsomlo@gmail.com; Zeng, Star
> > <star.zeng@intel.com>
> > Subject: Re: [edk2] Proposition of a BmEnumerateBootOptions() hook.
> >
> > Thanks for your feedback!
> > Comments inline
> >
> > Regards,
> > Marvin.
> >
> > > -----Original Message-----
> > > From: Laszlo Ersek <lersek@redhat.com>
> > > Sent: Tuesday, May 15, 2018 6:12 PM
> > > To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
> > > devel@lists.01.org
> > > Cc: gsomlo@gmail.com; ard.biesheuvel@linaro.org; ruiyu.ni@intel.com;
> > > eric.dong@intel.com; star.zeng@intel.com; jordan.l.justen@intel.com
> > > Subject: Re: [edk2] Proposition of a BmEnumerateBootOptions() hook.
> > >
> > > On 05/15/18 17:38, Marvin Häuser wrote:
> > > >> -----Original Message-----
> > > >> From: Laszlo Ersek <lersek@redhat.com>
> > > >> Sent: Tuesday, May 15, 2018 3:53 PM
> > > >> To: Marvin Häuser <Marvin.Haeuser@outlook.com>; edk2-
> > > >> devel@lists.01.org
> > > >> Cc: eric.dong@intel.com; star.zeng@intel.com;
> > > >> jordan.l.justen@intel.com; ard.biesheuvel@linaro.org;
> > > >> ruiyu.ni@intel.com; Gabriel L. Somlo (GMail) <gsomlo@gmail.com>;
> > > >> Gerd Hoffmann <kraxel@redhat.com>
> > > >> Subject: Re: [edk2] Proposition of a BmEnumerateBootOptions()
> hook.
> > > >>
> > > >> On 05/15/18 15:02, Marvin Häuser wrote:
> > >
> > > >>> 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.
> > > >
> > > > I think I might have been unclear here, I was solely referring to
> > > > the Boot
> > > options created by the enumeration process and not Boot#### in
> general.
> > > > I wouldn't know of anything restricting all Boot#### variables to
> > > > be NV, do
> > > you happen to have a quote?
> > >
> > > See "3.3 Globally Defined Variables" and "Table 10. Global Variables"
> > > in the
> > > UEFI-2.7 spec:
> > >
> > >   [...] The variables with an attribute of NV are nonvolatile. [...]
> > >
> > >   [...]
> > >
> > >   Variable Name  Attribute   Description
> > >   -------------  ---------   -----------
> > >   [...]
> > >   Boot####       NV, BS, RT  [...]
> > >   [...]
> >
> > I had hoped it was a recommendation, but indeed it seems to be
> mandatory.
> > What's your opinion on this fact and how do you think the chances are
> > NV could be made optional?
> >
> > >
> > > >> 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.
> > > >
> > > > Oh, I was just talking about the possibility, because the current
> > > > code does
> > > remove options under certain conditions.
> > > > However, for USB devices as you have mentioned, I do see this good
> > > practice so long as the option is volatile.
> > > > There is no point in exposing a boot option to a removable device
> > > > that is
> > > not attached if not to prevent unnecessary Flash cycles as far as I can
> think.
> > > > Do you happen to have worries about a scenario I did not consider?
> > >
> > > Sure; it's a simple scenario: the user might want the system to
> > > automatically boot off of a USB drive whenever they connect it,
> > > before powering on or rebooting the system, rather than boot from the
> hard drive.
> >
> > Remember that point was made in the context of enumeration. What I was
> > saying is that boot options created by the enum code, for removable
> > drives, are fine to be gone (as they are volatile, or proactively
> > removed when NV) after a reboot.
> > If a user wants to boot from such an USB device by default, in my
> > opinion they should manually create a boot option for it. Otherwise,
> > if I do not misunderstand your point, you are suggesting the firmware
> > to keep track of every single bootable USB device ever attached.
> >
> > >
> > > (For USB devices, "USB WWID" and "USB Class" device path nodes are
> > > defined by the spec, so the user isn't even expected to plug the
> > > drive into the same USB port, for the drive to be found uniquely.)
> >
> > Actually didn't know that, thanks for the fact!
> >
> > >
> > > >>> 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 think for a Boot#### option, it is always invalid,
> > >
> > > I disagree. I'll have to defer to Ray here, but in my experience,
> > > edk2 practice is that if you have a boot option whose devpath ends
> > > in a
> > > HD() node, then an UEFI image on that partition under the filepath
> > > \EFI\BOOT\BOOT<arch>.efi will be tried for booting. As far as I can
> > > see, this is neither mandated nor forbidden by the spec. Anyway, for
> > > Ray
> > to answer authoritatively.
> >
> > Indeed it is neither explicitly allowed nor forbidden, I concluded it
> > being invalid from the strictness of how the booting protocol is defined.
> > If it is forbidden, it is implicit, but if it's not the case, the
> > better it is for this purpose.
> >
> > >
> > > > because the default path is defined to be used in the "scanning phase"
> > > once no Boot#### variable could be found.
> > > > Also, unfortunately the path is also stored via bless and
> > > > definitely never the
> > > UEFI standard one, so supporting this vendor-specific scenario
> > > definitely requires special handling.
> > > > Do you have any opinion on the proposed second hook?
> > >
> > > Personally I remain unconvinced that the second hook is needed (to
> > > be invoked from core BdsDxe or UefiBootManagerLib code). But, that's
> > > just my opinion :)
> >
> > Well, the only alternative I see are modifying the entry pre-BDS
> > (slightly unclean), forking off (unpleasant) or not supporting it at
> > all (unpleasant too, but support is not terribly important, although would
> be nice to have).
> >
> > >
> > > In general I find that library APIs (especially a *set* of library
> > > APIs) are very hard to design (because we can't see the future). So,
> > > "organic growth" works better in practice (even if it leads to "less
> > > well organized" sets of APIs). This is to say that, if you can
> > > propose a hook and immediately demonstrate that it saves code for,
> > > and/or simplifies, multiple platforms, then the new API might be
> justified.
> > (Personally, I usually argue for "three call sites" as a minimum.
> > > Two might be enough if the code extracted is particularly baroque,
> > > or undergoes frequent
> > > changes.)
> >
> > The "problem" is that both hooks are not introduced for sharing code
> > but for giving the platform developer more freedom without forking off.
> > Whether such are accepted ends up being entirely the maintainer's
> > preference, I guess.
> 
> Forking off is never an option. In my opinion, if a generic library/driver needs
> forking, that's a failure indicator of design.

I think so too, hence my proposition for PlatformAcpiLib a few days ago. However the failure of design can be with both the generic driver and the platform side.
For the first hook, I believe it's a design flaw of edk2 because there is no (known to me) way of adding boot options "on demand", but just via the Before/AfterConsole hooks, which execute unconditioned.
Regarding the second hook - I do not like the idea myself, but it is the only I can think of to support the described scenario properly.

> Freedom is very important. I have seen many internal platform BDS
> implementations. The platform requirements are very different and strange
> sometimes.
> So the beforeconsole/afterconsole hook points were used.
> They are very natural. First hook point is when the platform doesn't have
> mouth and ears.
> The other hook point is when platform can speak and hear.
> 
> >
> > >
> > > >> 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.
> > > >
> > > > Thank you that you are ready to merge changes you cannot even
> > > > really
> > > test.
> > >
> > > I definitely intend to regression test any such changes :)
> >
> > Sure, I just often come across a "I don't merge what I cannot test"
> > mentality. :)
> >
> > >
> > > OTOH, in any larger project, any given maintainer is surely unable
> > > to integration-test *all* features in the subsystem that they co-maintain.
> > > What's important is that each major feature has at least one
> > > maintainer that can integration-test it (possibly with the help of
> > > an
> > automated test suite).
> > >
> > > For example, I totally cannot test Xen code in OvmfPkg or
> > > ArmVirtPkg, but that's why we have several permanent Reviewers from
> > > the Xen project, for OvmfPkg and ArmVirtPkg.
> > >
> > > Thanks,
> > > Laszlo
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel