* [PATCH v2 1/1] ArmPkg/Library: prevent endless reboot loop with emulated NV varstore
@ 2021-02-04 14:06 Leif Lindholm
2021-02-04 14:17 ` Laszlo Ersek
0 siblings, 1 reply; 4+ messages in thread
From: Leif Lindholm @ 2021-02-04 14:06 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Laszlo Ersek
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>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Leif Lindholm <leif@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);
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2 1/1] ArmPkg/Library: prevent endless reboot loop with emulated NV varstore
2021-02-04 14:06 [PATCH v2 1/1] ArmPkg/Library: prevent endless reboot loop with emulated NV varstore Leif Lindholm
@ 2021-02-04 14:17 ` Laszlo Ersek
2021-02-04 15:49 ` Ard Biesheuvel
0 siblings, 1 reply; 4+ messages in thread
From: Laszlo Ersek @ 2021-02-04 14:17 UTC (permalink / raw)
To: Leif Lindholm, devel; +Cc: Ard Biesheuvel
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@kernel.org>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Leif Lindholm <leif@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@redhat.com>
Ard should please review this patch for the logic change.
Laszlo
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2 1/1] ArmPkg/Library: prevent endless reboot loop with emulated NV varstore
2021-02-04 14:17 ` Laszlo Ersek
@ 2021-02-04 15:49 ` Ard Biesheuvel
2021-02-04 17:04 ` Leif Lindholm
0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2021-02-04 15:49 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: Leif Lindholm, devel, Ard Biesheuvel
On Thu, 4 Feb 2021 at 15:17, Laszlo Ersek <lersek@redhat.com> wrote:
>
> 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@kernel.org>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Signed-off-by: Leif Lindholm <leif@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@redhat.com>
>
> Ard should please review this patch for the logic change.
>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2 1/1] ArmPkg/Library: prevent endless reboot loop with emulated NV varstore
2021-02-04 15:49 ` Ard Biesheuvel
@ 2021-02-04 17:04 ` Leif Lindholm
0 siblings, 0 replies; 4+ messages in thread
From: Leif Lindholm @ 2021-02-04 17:04 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Laszlo Ersek, devel, Ard Biesheuvel
On Thu, Feb 04, 2021 at 16:49:03 +0100, Ard Biesheuvel wrote:
> > > + 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@redhat.com>
> >
> > Ard should please review this patch for the logic change.
> >
>
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Thanks!
Pushed as 1b6c3a94eca7.
/
Leif
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-02-04 17:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-04 14:06 [PATCH v2 1/1] ArmPkg/Library: prevent endless reboot loop with emulated NV varstore Leif Lindholm
2021-02-04 14:17 ` Laszlo Ersek
2021-02-04 15:49 ` Ard Biesheuvel
2021-02-04 17:04 ` Leif Lindholm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox