Re: [PATCH] ArmPkg/PlatformBootManagerLib: regenerate boot options on boot failure

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

On 6/17/20 6:21 PM, Andrei Warkentin wrote:
> Thanks for working on this.
> 
> Reviewed-by: Andrei Warkentin <awarkentin@vmware.com>
> 

Thanks.

> A non-blocking question: is a reboot necessary? Would it be possible to 
> just retry the boot sequence?
> 

Not sure how we'd do that.

> A
> ------------------------------------------------------------------------
> *From:* Ard Biesheuvel <ard.biesheuvel@arm.com>
> *Sent:* Tuesday, June 16, 2020 12:48 PM
> *To:* devel@edk2.groups.io <devel@edk2.groups.io>
> *Cc:* leif@nuviainc.com <leif@nuviainc.com>; Ard Biesheuvel 
> <ard.biesheuvel@arm.com>; Pete Batard <pete@akeo.ie>; Andrei Warkentin 
> <awarkentin@vmware.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> *Subject:* [PATCH] ArmPkg/PlatformBootManagerLib: regenerate boot 
> options on boot failure
> One of the side effects of the recent changes to PlatformBootManagerLib
> changes to avoid connecting all devices on every boot is that we no
> longer default to network boot on a virgin boot, but end up in the
> UiApp menu. At this point, the autogenerated boot options that we used
> to rely on will be instantiated too, but it does break the unattended
> boot case where devices are expected to attempt a network boot on the
> very first power on.
> 
> Let's work around this by refreshing all boot options explicitly in
> the UnableToBoot() handler, and rebooting the system if doing so
> resulted in a change to the total number of configured boot options.
> This way, we ultimately end up in the UiApp as before if no boot
> options could be started, but only after all the autogenerated ones
> have been attempted as well.
> 
> 
> Cc: Pete Batard <pete@akeo.ie>
> 
> Cc: Andrei Warkentin (awarkentin@vmware.com) <awarkentin@vmware.com>
> 
> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>   ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 34 
> ++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c 
> b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> index 15c5cac1bea0..9905cad22908 100644
> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -820,6 +820,40 @@ PlatformBootManagerUnableToBoot (
>   {
> 
>     EFI_STATUS                   Status;
> 
>     EFI_BOOT_MANAGER_LOAD_OPTION BootManagerMenu;
> 
> +  EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions;
> 
> +  UINTN                        OldBootOptionCount;
> 
> +  UINTN                        NewBootOptionCount;
> 
> +
> 
> +  //
> 
> +  // Record the total number of boot configured boot options
> 
> +  //
> 
> +  BootOptions = EfiBootManagerGetLoadOptions (&OldBootOptionCount,
> 
> +                  LoadOptionTypeBoot);
> 
> +  EfiBootManagerFreeLoadOptions (BootOptions, OldBootOptionCount);
> 
> +
> 
> +  //
> 
> +  // Connect all devices, and regenerate all boot options
> 
> +  //
> 
> +  EfiBootManagerConnectAll ();
> 
> +  EfiBootManagerRefreshAllBootOption ();
> 
> +
> 
> +  //
> 
> +  // Record the updated number of boot configured boot options
> 
> +  //
> 
> +  BootOptions = EfiBootManagerGetLoadOptions (&NewBootOptionCount,
> 
> +                  LoadOptionTypeBoot);
> 
> +  EfiBootManagerFreeLoadOptions (BootOptions, NewBootOptionCount);
> 
> +
> 
> +  //
> 
> +  // If the number of configured boot options has changed, reboot
> 
> +  // the system so the new boot options will be taken into account
> 
> +  // while executing the ordinary BDS bootflow sequence.
> 
> +  //
> 
> +  if (NewBootOptionCount != OldBootOptionCount) {
> 
> +    DEBUG ((DEBUG_WARN, "%a: rebooting after refreshing all boot 
> options\n",
> 
> +      __FUNCTION__));
> 
> +    gRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
> 
> +  }
> 
> 
> 
>     Status = EfiBootManagerGetBootManagerMenu (&BootManagerMenu);
> 
>     if (EFI_ERROR (Status)) {
> 
> -- 
> 2.27.0
>