Re: [edk2-devel] [PATCH 0/5] ArmPkg/PlatformBootManagerLib: play nice without ConnectAll()

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

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.