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

[Laszlo Ersek]

On 02/04/21 15:06, 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 at kernel.org>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Signed-off-by: Leif Lindholm <leif at nuviainc.com>
> ---
> 
> Changes in v2:
> - Fix indentation.
> - Add missing space in commit message.
> 
>  ArmPkg/Library/PlatformBootManagerLib/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..5ceb23d822e5 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__));
> +      gRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
> +    }
>    }
>  
>    Status = EfiBootManagerGetBootManagerMenu (&BootManagerMenu);
> 

At the level where I commented on v1 -- i.e., totally superficially --:

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

Ard should please review this patch for the logic change.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71250): https://edk2.groups.io/g/devel/message/71250
Mute This Topic: https://groups.io/mt/80379806/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-