Re: [edk2-devel] [PATCH 1/1] ArmPkg/Library: prevent endless reboot loop with emulated NV varstore

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

On 02/04/21 13:54, Leif Lindholm wrote:
> If no valid boot options were found, PlatformBootManagerLib refreshes a
> set of sane default options and then reboots. However, if there is in
> fact no persistent varstore,the same thing happens again on next boot,
> and we end up in an endlessly rebooting loop.
> 
> So when PcdEmuVariableNvModeEnable is TRUE, skip the reboot step and
> enter the setup menu instead.
> 
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Leif Lindholm <leif@nuviainc.com>
> ---
>  .../PlatformBootManagerLib.inf                       |  1 +
>  ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c   | 12 ++++++++----
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 2f726d117d7d..353d7a967b76 100644
> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -55,6 +55,7 @@ [FeaturePcd]
>    gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport
>  
>  [FixedPcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> index 9905cad22908..9564ab817f4c 100644
> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -848,11 +848,15 @@ PlatformBootManagerUnableToBoot (
>    // 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.
> +  // *Unless* persistent varstore is being emulated, since we would
> +  // then end up in an endless reboot loop.
>    //
> -  if (NewBootOptionCount != OldBootOptionCount) {
> -    DEBUG ((DEBUG_WARN, "%a: rebooting after refreshing all boot options\n",
> -      __FUNCTION__));
> -    gRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
> +  if (!PcdGetBool (PcdEmuVariableNvModeEnable)) {
> +    if (NewBootOptionCount != OldBootOptionCount) {
> +      DEBUG ((DEBUG_WARN, "%a: rebooting after refreshing all boot options\n",
> +              __FUNCTION__));

Incorrect indentation :P

Laszlo

> +      gRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
> +    }
>    }
>  
>    Status = EfiBootManagerGetBootManagerMenu (&BootManagerMenu);
>