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)) {
--
2.27.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61347): https://edk2.groups.io/g/devel/message/61347
Mute This Topic: https://groups.io/mt/74921613/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Tue, Jun 16, 2020 at 19:48:34 +0200, 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,
The passive voice is confusing me a bit here - who does the updating,
and when specifically?
/
Leif
> 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)) {
> --
> 2.27.0
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61402): https://edk2.groups.io/g/devel/message/61402
Mute This Topic: https://groups.io/mt/74921613/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 6/17/20 1:12 PM, Leif Lindholm wrote: > On Tue, Jun 16, 2020 at 19:48:34 +0200, 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, > > The passive voice is confusing me a bit here - who does the updating, > and when specifically? > Originally, the ArmPkg PlatformBmLib would always refresh all boot options, but now, only the UiApp does that upon entry, at which point your sitting in the menu idly, and so automated network boot no longer works. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61404): https://edk2.groups.io/g/devel/message/61404 Mute This Topic: https://groups.io/mt/74921613/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Wed, Jun 17, 2020 at 13:32:36 +0200, Ard Biesheuvel wrote:
> On 6/17/20 1:12 PM, Leif Lindholm wrote:
> > On Tue, Jun 16, 2020 at 19:48:34 +0200, 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,
> >
> > The passive voice is confusing me a bit here - who does the updating,
> > and when specifically?
> >
>
> Originally, the ArmPkg PlatformBmLib would always refresh all boot options,
> but now, only the UiApp does that upon entry, at which point your sitting in
> the menu idly, and so automated network boot no longer works.
Sure. But the message should contain some description of agency.
Something like:
"On entry, the UiApp instantiates the autogenerated boot options that
we used to rely on - but it does not consume them. This breaks the
unattended..."
I assume the UiApp only ever *adds* entries, which is why checking
number of entries is sufficient?
With that/if so:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
/
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61407): https://edk2.groups.io/g/devel/message/61407
Mute This Topic: https://groups.io/mt/74921613/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 6/17/20 2:16 PM, Leif Lindholm wrote: > On Wed, Jun 17, 2020 at 13:32:36 +0200, Ard Biesheuvel wrote: >> On 6/17/20 1:12 PM, Leif Lindholm wrote: >>> On Tue, Jun 16, 2020 at 19:48:34 +0200, 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, >>> >>> The passive voice is confusing me a bit here - who does the updating, >>> and when specifically? >>> >> >> Originally, the ArmPkg PlatformBmLib would always refresh all boot options, >> but now, only the UiApp does that upon entry, at which point your sitting in >> the menu idly, and so automated network boot no longer works. > > Sure. But the message should contain some description of agency. > > Something like: > "On entry, the UiApp instantiates the autogenerated boot options that > we used to rely on - but it does not consume them. This breaks the > unattended..." > OK > I assume the UiApp only ever *adds* entries, which is why checking > number of entries is sufficient? > It only manages entries that it instantiated itself, but it may also remove entries if the underlying hardware has disappeared. > With that/if so: > Reviewed-by: Leif Lindholm <leif@nuviainc.com> > > / > Leif > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61408): https://edk2.groups.io/g/devel/message/61408 Mute This Topic: https://groups.io/mt/74921613/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Wed, Jun 17, 2020 at 14:28:04 +0200, Ard Biesheuvel wrote:
> On 6/17/20 2:16 PM, Leif Lindholm wrote:
> > On Wed, Jun 17, 2020 at 13:32:36 +0200, Ard Biesheuvel wrote:
> > > On 6/17/20 1:12 PM, Leif Lindholm wrote:
> > > > On Tue, Jun 16, 2020 at 19:48:34 +0200, 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,
> > > >
> > > > The passive voice is confusing me a bit here - who does the updating,
> > > > and when specifically?
> > > >
> > >
> > > Originally, the ArmPkg PlatformBmLib would always refresh all boot options,
> > > but now, only the UiApp does that upon entry, at which point your sitting in
> > > the menu idly, and so automated network boot no longer works.
> >
> > Sure. But the message should contain some description of agency.
> >
> > Something like:
> > "On entry, the UiApp instantiates the autogenerated boot options that
> > we used to rely on - but it does not consume them. This breaks the
> > unattended..."
>
> OK
>
> > I assume the UiApp only ever *adds* entries, which is why checking
> > number of entries is sufficient?
> >
>
> It only manages entries that it instantiated itself, but it may also remove
> entries if the underlying hardware has disappeared.
Hmm, that's a bit trickier then. I mean, it's unlikely, but I'm sure
there's situations that could happen.
Would we run the risk of getting bug reports like "system fails to
boot from Ethernet when inifiniband switch powered off"? Or if some
virtual devices presented by a BMC appear/disappear?
/
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61410): https://edk2.groups.io/g/devel/message/61410
Mute This Topic: https://groups.io/mt/74921613/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 6/17/20 2:40 PM, Leif Lindholm wrote: > On Wed, Jun 17, 2020 at 14:28:04 +0200, Ard Biesheuvel wrote: >> On 6/17/20 2:16 PM, Leif Lindholm wrote: >>> On Wed, Jun 17, 2020 at 13:32:36 +0200, Ard Biesheuvel wrote: >>>> On 6/17/20 1:12 PM, Leif Lindholm wrote: >>>>> On Tue, Jun 16, 2020 at 19:48:34 +0200, 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, >>>>> >>>>> The passive voice is confusing me a bit here - who does the updating, >>>>> and when specifically? >>>>> >>>> >>>> Originally, the ArmPkg PlatformBmLib would always refresh all boot options, >>>> but now, only the UiApp does that upon entry, at which point your sitting in >>>> the menu idly, and so automated network boot no longer works. >>> >>> Sure. But the message should contain some description of agency. >>> >>> Something like: >>> "On entry, the UiApp instantiates the autogenerated boot options that >>> we used to rely on - but it does not consume them. This breaks the >>> unattended..." >> >> OK >> >>> I assume the UiApp only ever *adds* entries, which is why checking >>> number of entries is sufficient? >>> >> >> It only manages entries that it instantiated itself, but it may also remove >> entries if the underlying hardware has disappeared. > > Hmm, that's a bit trickier then. I mean, it's unlikely, but I'm sure > there's situations that could happen. > Would we run the risk of getting bug reports like "system fails to > boot from Ethernet when inifiniband switch powered off"? Or if some > virtual devices presented by a BMC appear/disappear? > If the boot entries are not refreshed, you will retain the old ones. So the only way this could lead to a boot failure is when you rely on automatically generated boot entries to device that disappear and reappear in a different place, e.g., move a Ethernet PCIe card to a different slot. Note that USB devices plugged into a different port will still work fine, though, as they rely on the removable boot path in this case, which will be attempted anyway before doing the UnableToBoot(). Note that the failure mode here is being dropped into the menu, where before you were always dropped into the Shell. The case we are trying to address here is zero intervention network boot after putting the device into circulation, and that should work correctly with this change: if the network boot path did not exist before, it will be added, in which case the number of boot options will increase. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61413): https://edk2.groups.io/g/devel/message/61413 Mute This Topic: https://groups.io/mt/74921613/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Wed, Jun 17, 2020 at 14:59:45 +0200, Ard Biesheuvel wrote:
> > > > Something like:
> > > > "On entry, the UiApp instantiates the autogenerated boot options that
> > > > we used to rely on - but it does not consume them. This breaks the
> > > > unattended..."
> > >
> > > OK
> > >
> > > > I assume the UiApp only ever *adds* entries, which is why checking
> > > > number of entries is sufficient?
> > > >
> > >
> > > It only manages entries that it instantiated itself, but it may also remove
> > > entries if the underlying hardware has disappeared.
> >
> > Hmm, that's a bit trickier then. I mean, it's unlikely, but I'm sure
> > there's situations that could happen.
> > Would we run the risk of getting bug reports like "system fails to
> > boot from Ethernet when inifiniband switch powered off"? Or if some
> > virtual devices presented by a BMC appear/disappear?
> >
>
> If the boot entries are not refreshed, you will retain the old ones. So the
> only way this could lead to a boot failure is when you rely on automatically
> generated boot entries to device that disappear and reappear in a different
> place, e.g., move a Ethernet PCIe card to a different slot. Note that USB
> devices plugged into a different port will still work fine, though, as they
> rely on the removable boot path in this case, which will be attempted anyway
> before doing the UnableToBoot().
>
> Note that the failure mode here is being dropped into the menu, where before
> you were always dropped into the Shell. The case we are trying to address
> here is zero intervention network boot after putting the device into
> circulation, and that should work correctly with this change: if the network
> boot path did not exist before, it will be added, in which case the number
> of boot options will increase.
OK. I'm not convinced we're not going to see a report of this
somewhere down the line, but I think you've managed to convince me
it's an unlikely enough situation, and a fallback, that we can bump it
to then (and it *is* a behavioural improvement in all other cases).
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
(with the commit message update)
/
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61417): https://edk2.groups.io/g/devel/message/61417
Mute This Topic: https://groups.io/mt/74921613/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 6/17/20 4:35 PM, Leif Lindholm wrote: > On Wed, Jun 17, 2020 at 14:59:45 +0200, Ard Biesheuvel wrote: >>>>> Something like: >>>>> "On entry, the UiApp instantiates the autogenerated boot options that >>>>> we used to rely on - but it does not consume them. This breaks the >>>>> unattended..." >>>> >>>> OK >>>> >>>>> I assume the UiApp only ever *adds* entries, which is why checking >>>>> number of entries is sufficient? >>>>> >>>> >>>> It only manages entries that it instantiated itself, but it may also remove >>>> entries if the underlying hardware has disappeared. >>> >>> Hmm, that's a bit trickier then. I mean, it's unlikely, but I'm sure >>> there's situations that could happen. >>> Would we run the risk of getting bug reports like "system fails to >>> boot from Ethernet when inifiniband switch powered off"? Or if some >>> virtual devices presented by a BMC appear/disappear? >>> >> >> If the boot entries are not refreshed, you will retain the old ones. So the >> only way this could lead to a boot failure is when you rely on automatically >> generated boot entries to device that disappear and reappear in a different >> place, e.g., move a Ethernet PCIe card to a different slot. Note that USB >> devices plugged into a different port will still work fine, though, as they >> rely on the removable boot path in this case, which will be attempted anyway >> before doing the UnableToBoot(). >> >> Note that the failure mode here is being dropped into the menu, where before >> you were always dropped into the Shell. The case we are trying to address >> here is zero intervention network boot after putting the device into >> circulation, and that should work correctly with this change: if the network >> boot path did not exist before, it will be added, in which case the number >> of boot options will increase. > > OK. I'm not convinced we're not going to see a report of this > somewhere down the line, but I think you've managed to convince me > it's an unlikely enough situation, and a fallback, that we can bump it > to then (and it *is* a behavioural improvement in all other cases). > > Reviewed-by: Leif Lindholm <leif@nuviainc.com> > (with the commit message update) > Merged as 2d233af64b8f73d1b1e138b302e6344f7c2e0f4e Thanks all, -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61438): https://edk2.groups.io/g/devel/message/61438 Mute This Topic: https://groups.io/mt/74921613/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Thanks for working on this.
Reviewed-by: Andrei Warkentin <awarkentin@vmware.com>
A non-blocking question: is a reboot necessary? Would it be possible to just retry the boot sequence?
A
________________________________
From: Ard Biesheuvel <ard.biesheuvel@arm.com>
Sent: Tuesday, June 16, 2020 12:48 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: leif@nuviainc.com <leif@nuviainc.com>; Ard Biesheuvel <ard.biesheuvel@arm.com>; Pete Batard <pete@akeo.ie>; Andrei Warkentin <awarkentin@vmware.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Subject: [PATCH] ArmPkg/PlatformBootManagerLib: regenerate boot options on boot failure
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)) {
--
2.27.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61425): https://edk2.groups.io/g/devel/message/61425
Mute This Topic: https://groups.io/mt/74921613/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 6/17/20 6:21 PM, Andrei Warkentin wrote:
> Thanks for working on this.
>
> Reviewed-by: Andrei Warkentin <awarkentin@vmware.com>
>
Thanks.
> A non-blocking question: is a reboot necessary? Would it be possible to
> just retry the boot sequence?
>
Not sure how we'd do that.
> A
> ------------------------------------------------------------------------
> *From:* Ard Biesheuvel <ard.biesheuvel@arm.com>
> *Sent:* Tuesday, June 16, 2020 12:48 PM
> *To:* devel@edk2.groups.io <devel@edk2.groups.io>
> *Cc:* leif@nuviainc.com <leif@nuviainc.com>; Ard Biesheuvel
> <ard.biesheuvel@arm.com>; Pete Batard <pete@akeo.ie>; Andrei Warkentin
> <awarkentin@vmware.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> *Subject:* [PATCH] ArmPkg/PlatformBootManagerLib: regenerate boot
> options on boot failure
> 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)) {
>
> --
> 2.27.0
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61436): https://edk2.groups.io/g/devel/message/61436
Mute This Topic: https://groups.io/mt/74921613/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61415): https://edk2.groups.io/g/devel/message/61415
Mute This Topic: https://groups.io/mt/74921613/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Sent: Tuesday, June 16, 2020 1:49 PM
> To: devel@edk2.groups.io
> Cc: leif@nuviainc.com; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Pete
> Batard <pete@akeo.ie>; Andrei Warkentin (awarkentin@vmware.com)
> <awarkentin@vmware.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
> Mahmoud@arm.com>
> Subject: [PATCH] ArmPkg/PlatformBootManagerLib: regenerate boot
> options on boot failure
>
> 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)) {--
> 2.27.0
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61360): https://edk2.groups.io/g/devel/message/61360
Mute This Topic: https://groups.io/mt/74921613/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.