Currently, Armpkg's PlatformBootManagerLib connects all controller to
their drivers recursively, even if the active boot option does not
require it. There are some historical reasons for this, some of which are
being addressed in separate patches.

This series addresses the way ArmPkg's PlatformBootManagerLib implementation
deals with the UEFI Shell and the UiApp: currently, the shell is always
added as an ordinary boot option, which will be started if no other boot
options can be started, or if it is the first one in the boot order.

Once we remove the ConnectAll() call from PlatformBootManagerLib, those shells
will be launched without any block or other devices connected, which may
confuse novice users. So before doing so, let's make the handling a bit more
sane:
- add a 's' hotkey that enters the UEFI shell regardless of its priority
  in the BootOrder - this shell will be entered with no devices connected
  after patch #4
- enter the UiApp as a last resort if no boot options can be started
- make the UEFI Shell boot option hidden, so it is not started by default
  (only by hotkey or BootNext)
- remove the ConnectAll() call from PlatformBootManagerLib
- finally, add a plugin library for UiApp to expose the UEFI Shell via an
  ordinary main menu option (this works around the fact that patch #3 will
  result in the UEFI Shell disappearing from the Boot Manager list).
  Entering the shell this way will resemble the old situation, given that
  UiApp connects all devices and refreshes all boot options etc at launch.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>

Ard Biesheuvel (5):
  ArmPkg/PlatformBootManagerLib: register 's' as UEFI Shell hotkey
  ArmPkg/PlatformBootManagerLib: fall back to the UiApp on boot failure
  ArmPkg/PlatformBootManagerLib: hide UEFI Shell as a regular boot
    option
  ArmPkg/PlatformBootManagerLib: don't connect all devices on each boot
  ShellPkg: add BootManager library to add UEFI Shell menu option

 .../ShellBootManagerLib.inf                   |  45 +++
 .../ShellBootManagerLib/ShellBootManagerLib.h |  44 +++
 .../PlatformBootManagerLib/PlatformBm.c       |  37 ++-
 .../ShellBootManagerLib/ShellBootManagerLib.c | 258 ++++++++++++++++++
 .../ShellBootManagerLib/ShellBmStrings.uni    |  17 ++
 .../ShellBootManagerLib/ShellBmVfr.Vfr        |  37 +++
 6 files changed, 425 insertions(+), 13 deletions(-)
 create mode 100644 ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.inf
 create mode 100644 ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.h
 create mode 100644 ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.c
 create mode 100644 ShellPkg/Library/ShellBootManagerLib/ShellBmStrings.uni
 create mode 100644 ShellPkg/Library/ShellBootManagerLib/ShellBmVfr.Vfr

-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60288): https://edk2.groups.io/g/devel/message/60288
Mute This Topic: https://groups.io/mt/74481036/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Tue, May 26, 2020 at 18:13:54 +0200, Ard Biesheuvel wrote:
> Currently, Armpkg's PlatformBootManagerLib connects all controller to
> their drivers recursively, even if the active boot option does not
> require it. There are some historical reasons for this, some of which are
> being addressed in separate patches.
> 
> This series addresses the way ArmPkg's PlatformBootManagerLib implementation
> deals with the UEFI Shell and the UiApp: currently, the shell is always
> added as an ordinary boot option, which will be started if no other boot
> options can be started, or if it is the first one in the boot order.
> 
> Once we remove the ConnectAll() call from PlatformBootManagerLib, those shells
> will be launched without any block or other devices connected, which may
> confuse novice users. So before doing so, let's make the handling a bit more
> sane:
> - add a 's' hotkey that enters the UEFI shell regardless of its priority
>   in the BootOrder - this shell will be entered with no devices connected
>   after patch #4
> - enter the UiApp as a last resort if no boot options can be started
> - make the UEFI Shell boot option hidden, so it is not started by default
>   (only by hotkey or BootNext)
> - remove the ConnectAll() call from PlatformBootManagerLib
> - finally, add a plugin library for UiApp to expose the UEFI Shell via an
>   ordinary main menu option (this works around the fact that patch #3 will
>   result in the UEFI Shell disappearing from the Boot Manager list).
>   Entering the shell this way will resemble the old situation, given that
>   UiApp connects all devices and refreshes all boot options etc at launch.

I get why this set was sent in isolation, but could we also discuss
somewhat what we would plan to do with the edk2-platforms that make
use of the ArmPkg PlatformBootManagerLib?

Not attempting a fallback boot onto network is probably an acceptable
change to pick up, but having an undocumented hotkey as the only way
into a shell that now doesn't map devices could be a bit aggravating.

/
    Leif

> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> 
> Ard Biesheuvel (5):
>   ArmPkg/PlatformBootManagerLib: register 's' as UEFI Shell hotkey
>   ArmPkg/PlatformBootManagerLib: fall back to the UiApp on boot failure
>   ArmPkg/PlatformBootManagerLib: hide UEFI Shell as a regular boot
>     option
>   ArmPkg/PlatformBootManagerLib: don't connect all devices on each boot
>   ShellPkg: add BootManager library to add UEFI Shell menu option
> 
>  .../ShellBootManagerLib.inf                   |  45 +++
>  .../ShellBootManagerLib/ShellBootManagerLib.h |  44 +++
>  .../PlatformBootManagerLib/PlatformBm.c       |  37 ++-
>  .../ShellBootManagerLib/ShellBootManagerLib.c | 258 ++++++++++++++++++
>  .../ShellBootManagerLib/ShellBmStrings.uni    |  17 ++
>  .../ShellBootManagerLib/ShellBmVfr.Vfr        |  37 +++
>  6 files changed, 425 insertions(+), 13 deletions(-)
>  create mode 100644 ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.inf
>  create mode 100644 ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.h
>  create mode 100644 ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.c
>  create mode 100644 ShellPkg/Library/ShellBootManagerLib/ShellBmStrings.uni
>  create mode 100644 ShellPkg/Library/ShellBootManagerLib/ShellBmVfr.Vfr
> 
> -- 
> 2.17.1
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60301): https://edk2.groups.io/g/devel/message/60301
Mute This Topic: https://groups.io/mt/74481036/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 5/27/20 12:01 AM, Leif Lindholm via groups.io wrote:
> On Tue, May 26, 2020 at 18:13:54 +0200, Ard Biesheuvel wrote:
>> Currently, Armpkg's PlatformBootManagerLib connects all controller to
>> their drivers recursively, even if the active boot option does not
>> require it. There are some historical reasons for this, some of which are
>> being addressed in separate patches.
>>
>> This series addresses the way ArmPkg's PlatformBootManagerLib implementation
>> deals with the UEFI Shell and the UiApp: currently, the shell is always
>> added as an ordinary boot option, which will be started if no other boot
>> options can be started, or if it is the first one in the boot order.
>>
>> Once we remove the ConnectAll() call from PlatformBootManagerLib, those shells
>> will be launched without any block or other devices connected, which may
>> confuse novice users. So before doing so, let's make the handling a bit more
>> sane:
>> - add a 's' hotkey that enters the UEFI shell regardless of its priority
>>    in the BootOrder - this shell will be entered with no devices connected
>>    after patch #4
>> - enter the UiApp as a last resort if no boot options can be started
>> - make the UEFI Shell boot option hidden, so it is not started by default
>>    (only by hotkey or BootNext)
>> - remove the ConnectAll() call from PlatformBootManagerLib
>> - finally, add a plugin library for UiApp to expose the UEFI Shell via an
>>    ordinary main menu option (this works around the fact that patch #3 will
>>    result in the UEFI Shell disappearing from the Boot Manager list).
>>    Entering the shell this way will resemble the old situation, given that
>>    UiApp connects all devices and refreshes all boot options etc at launch.
> 
> I get why this set was sent in isolation, but could we also discuss
> somewhat what we would plan to do with the edk2-platforms that make
> use of the ArmPkg PlatformBootManagerLib?
> 
> Not attempting a fallback boot onto network is probably an acceptable
> change to pick up, but having an undocumented hotkey as the only way
> into a shell that now doesn't map devices could be a bit aggravating.
> 

It is not the only way, and it is not even the preferred way. Patch 5/5 
adds an option to the UiApp root menu to enter the UEFI Shell, in a way 
that is independent from boot option handling. Since you enter UiApp 
first, all handles will be connected and boot options refreshed as usual.

In cases where you want to avoid this from happening, you can use the 
's' key to drop into a shell directly.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60316): https://edk2.groups.io/g/devel/message/60316
Mute This Topic: https://groups.io/mt/74481036/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Wed, May 27, 2020 at 07:35:18 +0200, Ard Biesheuvel wrote:
> On 5/27/20 12:01 AM, Leif Lindholm via groups.io wrote:
> > On Tue, May 26, 2020 at 18:13:54 +0200, Ard Biesheuvel wrote:
> > > Currently, Armpkg's PlatformBootManagerLib connects all controller to
> > > their drivers recursively, even if the active boot option does not
> > > require it. There are some historical reasons for this, some of which are
> > > being addressed in separate patches.
> > > 
> > > This series addresses the way ArmPkg's PlatformBootManagerLib implementation
> > > deals with the UEFI Shell and the UiApp: currently, the shell is always
> > > added as an ordinary boot option, which will be started if no other boot
> > > options can be started, or if it is the first one in the boot order.
> > > 
> > > Once we remove the ConnectAll() call from PlatformBootManagerLib, those shells
> > > will be launched without any block or other devices connected, which may
> > > confuse novice users. So before doing so, let's make the handling a bit more
> > > sane:
> > > - add a 's' hotkey that enters the UEFI shell regardless of its priority
> > >    in the BootOrder - this shell will be entered with no devices connected
> > >    after patch #4
> > > - enter the UiApp as a last resort if no boot options can be started
> > > - make the UEFI Shell boot option hidden, so it is not started by default
> > >    (only by hotkey or BootNext)
> > > - remove the ConnectAll() call from PlatformBootManagerLib
> > > - finally, add a plugin library for UiApp to expose the UEFI Shell via an
> > >    ordinary main menu option (this works around the fact that patch #3 will
> > >    result in the UEFI Shell disappearing from the Boot Manager list).
> > >    Entering the shell this way will resemble the old situation, given that
> > >    UiApp connects all devices and refreshes all boot options etc at launch.
> > 
> > I get why this set was sent in isolation, but could we also discuss
> > somewhat what we would plan to do with the edk2-platforms that make
> > use of the ArmPkg PlatformBootManagerLib?
> > 
> > Not attempting a fallback boot onto network is probably an acceptable
> > change to pick up, but having an undocumented hotkey as the only way
> > into a shell that now doesn't map devices could be a bit aggravating.
> > 
> 
> It is not the only way, and it is not even the preferred way. Patch 5/5 adds
> an option to the UiApp root menu to enter the UEFI Shell, in a way that is
> independent from boot option handling. Since you enter UiApp first, all
> handles will be connected and boot options refreshed as usual.
> 
> In cases where you want to avoid this from happening, you can use the 's'
> key to drop into a shell directly.

Yes, that's exactly what I am referring to. But in order for the new
(and I agree improved) functionality to be available, the new Shell
library needs to be explicitly added to .dsc for the platforms affected.

I want an active decision to be made about how that is going to
happen, if it is going to happen, as part of the conversation about
*this* set. Merging this and *then* looking into it makes for too
harsh a break in behaviour.

/
    Leif

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60333): https://edk2.groups.io/g/devel/message/60333
Mute This Topic: https://groups.io/mt/74481036/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 5/27/20 12:43 PM, Leif Lindholm wrote:
> On Wed, May 27, 2020 at 07:35:18 +0200, Ard Biesheuvel wrote:
>> On 5/27/20 12:01 AM, Leif Lindholm via groups.io wrote:
>>> On Tue, May 26, 2020 at 18:13:54 +0200, Ard Biesheuvel wrote:
>>>> Currently, Armpkg's PlatformBootManagerLib connects all controller to
>>>> their drivers recursively, even if the active boot option does not
>>>> require it. There are some historical reasons for this, some of which are
>>>> being addressed in separate patches.
>>>>
>>>> This series addresses the way ArmPkg's PlatformBootManagerLib implementation
>>>> deals with the UEFI Shell and the UiApp: currently, the shell is always
>>>> added as an ordinary boot option, which will be started if no other boot
>>>> options can be started, or if it is the first one in the boot order.
>>>>
>>>> Once we remove the ConnectAll() call from PlatformBootManagerLib, those shells
>>>> will be launched without any block or other devices connected, which may
>>>> confuse novice users. So before doing so, let's make the handling a bit more
>>>> sane:
>>>> - add a 's' hotkey that enters the UEFI shell regardless of its priority
>>>>     in the BootOrder - this shell will be entered with no devices connected
>>>>     after patch #4
>>>> - enter the UiApp as a last resort if no boot options can be started
>>>> - make the UEFI Shell boot option hidden, so it is not started by default
>>>>     (only by hotkey or BootNext)
>>>> - remove the ConnectAll() call from PlatformBootManagerLib
>>>> - finally, add a plugin library for UiApp to expose the UEFI Shell via an
>>>>     ordinary main menu option (this works around the fact that patch #3 will
>>>>     result in the UEFI Shell disappearing from the Boot Manager list).
>>>>     Entering the shell this way will resemble the old situation, given that
>>>>     UiApp connects all devices and refreshes all boot options etc at launch.
>>>
>>> I get why this set was sent in isolation, but could we also discuss
>>> somewhat what we would plan to do with the edk2-platforms that make
>>> use of the ArmPkg PlatformBootManagerLib?
>>>
>>> Not attempting a fallback boot onto network is probably an acceptable
>>> change to pick up, but having an undocumented hotkey as the only way
>>> into a shell that now doesn't map devices could be a bit aggravating.
>>>
>>
>> It is not the only way, and it is not even the preferred way. Patch 5/5 adds
>> an option to the UiApp root menu to enter the UEFI Shell, in a way that is
>> independent from boot option handling. Since you enter UiApp first, all
>> handles will be connected and boot options refreshed as usual.
>>
>> In cases where you want to avoid this from happening, you can use the 's'
>> key to drop into a shell directly.
> 
> Yes, that's exactly what I am referring to. But in order for the new
> (and I agree improved) functionality to be available, the new Shell
> library needs to be explicitly added to .dsc for the platforms affected.
> 
> I want an active decision to be made about how that is going to
> happen, if it is going to happen, as part of the conversation about
> *this* set. Merging this and *then* looking into it makes for too
> harsh a break in behaviour.
> 

All existing platforms that incorporate ArmPkg's PlatformBootManagerLib 
must add this NULL library class resolution to UiApp first. So once we 
agree that this approach is acceptable (including the change to 
ShellPkg), I can prepare a patch for edk2-platforms implementing this 
for all such platforms living there. I suggest that we don't do the 
3-way handshake here (edk2 pre, edk2-platforms, edk2 post), given that 
the build won't break, it's just that if you pull and build your 
platform right at the minute between merging the edk2 changes and the 
edk2-platform ones, you will see the slightly unintuitive behavior if 
you also happen to clear your Boot#### variables at the same time.





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60334): https://edk2.groups.io/g/devel/message/60334
Mute This Topic: https://groups.io/mt/74481036/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Wed, May 27, 2020 at 12:50:53 +0200, Ard Biesheuvel wrote:
> > > > Not attempting a fallback boot onto network is probably an acceptable
> > > > change to pick up, but having an undocumented hotkey as the only way
> > > > into a shell that now doesn't map devices could be a bit aggravating.
> > > > 
> > > 
> > > It is not the only way, and it is not even the preferred way. Patch 5/5 adds
> > > an option to the UiApp root menu to enter the UEFI Shell, in a way that is
> > > independent from boot option handling. Since you enter UiApp first, all
> > > handles will be connected and boot options refreshed as usual.
> > > 
> > > In cases where you want to avoid this from happening, you can use the 's'
> > > key to drop into a shell directly.
> > 
> > Yes, that's exactly what I am referring to. But in order for the new
> > (and I agree improved) functionality to be available, the new Shell
> > library needs to be explicitly added to .dsc for the platforms affected.
> > 
> > I want an active decision to be made about how that is going to
> > happen, if it is going to happen, as part of the conversation about
> > *this* set. Merging this and *then* looking into it makes for too
> > harsh a break in behaviour.
> > 
> 
> All existing platforms that incorporate ArmPkg's PlatformBootManagerLib must
> add this NULL library class resolution to UiApp first. So once we agree that
> this approach is acceptable (including the change to ShellPkg), I can
> prepare a patch for edk2-platforms implementing this for all such platforms
> living there. I suggest that we don't do the 3-way handshake here (edk2 pre,
> edk2-platforms, edk2 post), given that the build won't break, it's just that
> if you pull and build your platform right at the minute between merging the
> edk2 changes and the edk2-platform ones, you will see the slightly
> unintuitive behavior if you also happen to clear your Boot#### variables at
> the same time.

Yeah, that's fine with me.
But I'd prefer that set to be ready to go immediately afer this one
hits.

(Insert generic note on benefit of *not* breaking the project up into
ever increasing number of repositories.)

Should I interpret this set more as RFC (in which case I may have
slightly overreacted) than the PATCH it says in the subject line?

/
    Leif

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60344): https://edk2.groups.io/g/devel/message/60344
Mute This Topic: https://groups.io/mt/74481036/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-