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

["Laszlo Ersek" <lersek@redhat.com>]

On 06/16/20 19:48, Ard Biesheuvel wrote:
> 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)) {
> 

This looks like a very nice trick, and a very good utilization of the
PlatformBootManagerUnableToBoot() hook, for physical machines.

FWIW:

Acked-by: Laszlo Ersek <lersek@redhat.com>