Re: [PATCH] MdeModulePkg/UefiBootManagerLib: handle ultimate boot failure

["Ni, Ruiyu" <ruiyu.ni@Intel.com>]

On 6/28/2018 11:04 PM, Laszlo Ersek wrote:
> Personally I'd be very happy with the current version of the patch as
> well, but I agree Sunny's request makes sense. How about this, for
> BdsDxe:
> 
>> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> index 3191a986304b..cb4196a56c87 100644
>> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> @@ -1088,11 +1088,26 @@ BdsEntry (
>>       EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
>>     }
>>
>> -  //
>> -  // If BootManagerMenu is available, fall back to it indefinitely.
>> -  //
>> -  if (BootManagerMenuStatus != EFI_NOT_FOUND) {
>> -    BdsBootManagerMenuLoop (&BootManagerMenu);
>> +  if (BootManagerMenuStatus == EFI_NOT_FOUND) {
>> +    //
>> +    // Inform the platform that we're unable to boot, and that there's no
>> +    // BootManagerMenu.
>> +    //
>> +    EfiBootManagerUnableToBoot (NULL);
>> +  } else {
>> +    //
>> +    // Inform the platform that we're unable to boot. The platform may enter
>> +    // BootManagerMenu with the public EfiBootManagerBoot() interface, if so
>> +    // desired.
>> +    //
>> +    Status = EfiBootManagerUnableToBoot (&BootManagerMenu);
>> +    if (EFI_ERROR (Status)) {
>> +      //
>> +      // The platform didn't register a callback; fall back to BootManagerMenu
>> +      // internally, indefinitely.
>> +      //
>> +      BdsBootManagerMenuLoop (&BootManagerMenu);
>> +    }
>>     }
>>
>>     DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n"));
> Note that this requires changing the declaration of
> EfiBootManagerUnableBoot(), so that it takes the parameter
> 
>    IN EFI_BOOT_MANAGER_LOAD_OPTION *BootManagerMenu OPTIONAL
> 
> The structure EFI_BOOT_MANAGER_LOAD_OPTION is from
> "MdeModulePkg/Include/Library/UefiBootManagerLib.h", so it is OK to
> expose to platforms.
> 
> Just an idea, of course.

Platform can use EfiBootManagerGetBootManagerMenu() to get the boot 
manager menu. So there is no need to add an extra parameter for 
EfiBootManagerUnableToBoot().
I agree with your idea to only pop up boot manager menu as the default 
behavior.

> 
> --*--
> 
> Anyway, for a v2, I have some superficial reuqests / questions for Ray:
> 
> * Please replace "UNABLE_BOOT" with "UNABLE_TO_BOOT". Same for
>    "UnableBoot" and "UnableToBoot".
> 
>    (Compare: READY_TO_BOOT, ReadyToBoot.)
> 
>    Note that this affects the commit message too.
OK.
> 
> * Should we split the BdsDxe modification to a separate patch?
OK.
> 
> * Can you please reference
>    <https://bugzilla.tianocore.org/show_bug.cgi?id=982>  in the commit
>    message?
OK.
> 
> Thank you very much Ray for doing this!
> Laszlo


-- 
Thanks,
Ray