public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 1/1] ArmPkg/Library: prevent endless reboot loop with emulated NV varstore
@ 2021-02-04 12:54 Leif Lindholm
  2021-02-04 13:06 ` [edk2-devel] " Laszlo Ersek
  0 siblings, 1 reply; 3+ messages in thread
From: Leif Lindholm @ 2021-02-04 12:54 UTC (permalink / raw)
  To: devel; +Cc: Ard Biesheuvel

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__));
+      gRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
+    }
   }
 
   Status = EfiBootManagerGetBootManagerMenu (&BootManagerMenu);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] ArmPkg/Library: prevent endless reboot loop with emulated NV varstore
  2021-02-04 12:54 [PATCH 1/1] ArmPkg/Library: prevent endless reboot loop with emulated NV varstore Leif Lindholm
@ 2021-02-04 13:06 ` Laszlo Ersek
  2021-02-04 13:50   ` Leif Lindholm
  0 siblings, 1 reply; 3+ messages in thread
From: Laszlo Ersek @ 2021-02-04 13:06 UTC (permalink / raw)
  To: devel, leif; +Cc: Ard Biesheuvel

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);
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [PATCH 1/1] ArmPkg/Library: prevent endless reboot loop with emulated NV varstore
  2021-02-04 13:06 ` [edk2-devel] " Laszlo Ersek
@ 2021-02-04 13:50   ` Leif Lindholm
  0 siblings, 0 replies; 3+ messages in thread
From: Leif Lindholm @ 2021-02-04 13:50 UTC (permalink / raw)
  To: devel, lersek; +Cc: Ard Biesheuvel

On Thu, Feb 04, 2021 at 14:06:01 +0100, Laszlo Ersek wrote:
> 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

Doh!
Emacs, you have betrayed me!
*Hangs head in shame*

/
    Leif

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-02-04 13:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-04 12:54 [PATCH 1/1] ArmPkg/Library: prevent endless reboot loop with emulated NV varstore Leif Lindholm
2021-02-04 13:06 ` [edk2-devel] " Laszlo Ersek
2021-02-04 13:50   ` Leif Lindholm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox