MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c | 9 +++++++++ 1 file changed, 9 insertions(+)
From: Neal Gompa <ngompa@fedoraproject.org>
Currently, the ReadyToBoot event is only signaled when a formal Boot
Manager option is executed (in BmBoot.c -> EfiBootManagerBoot ()).
However, the introduction of Platform Recovery in UEFI 2.5 makes it
necessary to signal ReadyToBoot when a Platform Recovery boot loader
runs because otherwise it may lead to the execution of a boot loader
that has similar requirements to a regular one that is not launched
as a Boot Manager option.
This is especially critical to ensuring that the graphical console
is actually usable during platform recovery, as some platforms do
rely on the ConsolePrefDxe driver, which only performs console
initialization after ReadyToBoot is triggered.
This patch fixes that behavior by calling EfiSignalEventReadyToBoot ()
in EfiBootManagerProcessLoadOption (), which is the function that sets
up the platform recovery boot process.
The expected behavior has been clarified in the UEFI 2.10 specification
to explicitly indicate this behavior is required for correct operation.
This is a rebased version of the patch originally written by Pete Batard.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2831
Cc: Pete Batard <pete@akeo.ie>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Co-authored-by: Pete Batard <pete@akeo.ie>
Signed-off-by: Neal Gompa <ngompa@fedoraproject.org>
---
MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 2087f0b91d..31ed608817 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -1416,6 +1416,15 @@ EfiBootManagerProcessLoadOption (
return EFI_SUCCESS;
}
+ //
+ // Signal the EVT_SIGNAL_READY_TO_BOOT event when we are about to load and execute the boot option.
+ //
+ EfiSignalEventReadyToBoot ();
+ //
+ // Report Status Code to indicate ReadyToBoot was signalled
+ //
+ REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT));
+
//
// Load and start the load option.
//
--
2.41.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110104): https://edk2.groups.io/g/devel/message/110104
Mute This Topic: https://groups.io/mt/102200076/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 10/26/23 15:53, Neal Gompa wrote: > From: Neal Gompa <ngompa@fedoraproject.org> > > Currently, the ReadyToBoot event is only signaled when a formal Boot > Manager option is executed (in BmBoot.c -> EfiBootManagerBoot ()). > > However, the introduction of Platform Recovery in UEFI 2.5 makes it > necessary to signal ReadyToBoot when a Platform Recovery boot loader > runs because otherwise it may lead to the execution of a boot loader > that has similar requirements to a regular one that is not launched > as a Boot Manager option. > > This is especially critical to ensuring that the graphical console > is actually usable during platform recovery, as some platforms do > rely on the ConsolePrefDxe driver, which only performs console > initialization after ReadyToBoot is triggered. > > This patch fixes that behavior by calling EfiSignalEventReadyToBoot () > in EfiBootManagerProcessLoadOption (), which is the function that sets > up the platform recovery boot process. > > The expected behavior has been clarified in the UEFI 2.10 specification > to explicitly indicate this behavior is required for correct operation. > > This is a rebased version of the patch originally written by Pete Batard. > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2831 > > Cc: Pete Batard <pete@akeo.ie> > Cc: Daniel P. Berrangé <berrange@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com> > > Co-authored-by: Pete Batard <pete@akeo.ie> > Signed-off-by: Neal Gompa <ngompa@fedoraproject.org> > --- > MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c > index 2087f0b91d..31ed608817 100644 > --- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c > +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c > @@ -1416,6 +1416,15 @@ EfiBootManagerProcessLoadOption ( > return EFI_SUCCESS; > } > > + // > + // Signal the EVT_SIGNAL_READY_TO_BOOT event when we are about to load and execute the boot option. > + // > + EfiSignalEventReadyToBoot (); > + // > + // Report Status Code to indicate ReadyToBoot was signalled > + // > + REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT)); > + > // > // Load and start the load option. > // While the patch does the right thing for the latest UEFI spec language, it does a bit too much. The spec says (v2.10), under 7.1.2 "EFI_BOOT_SERVICES.CreateEventEx()": ----------- EFI_EVENT_GROUP_READY_TO_BOOT This event group is notified by the system right before notifying EFI_EVENT_GROUP_AFTER_READY_TO_BOOT event group when the Boot Manager is about to load and execute a boot option or a platform or OS recovery option. The event group presents the last chance to modify device or system configuration prior to passing control to a boot option. ----------- NB "boot option", or "platform or OS recovery option". However, EfiBootManagerProcessLoadOption() is also used for launching Driver#### and SysPrep#### options. EfiBootManagerProcessLoadOption() has two call sites: (1) in ProcessLoadOptions() [MdeModulePkg/Universal/BdsDxe/BdsEntry.c] (2) near the end of BdsEntry() [MdeModulePkg/Universal/BdsDxe/BdsEntry.c] Call site (2) bears the comment "When platform recovery is not enabled, still boot to platform default file path", so I figure signaling ready-to-boot at that point is fine. Call site (1) requires further investigation. Namely, ProcessLoadOptions() itself is called from three locations, all inside BdsEntry() [MdeModulePkg/Universal/BdsDxe/BdsEntry.c]: (1.1) under comment "Execute Driver Options", for "LoadOptionTypeDriver" type load options (1.2) under comment "Execute SysPrep####", for "LoadOptionTypeSysPrep" type load options (1.3) for "LoadOptionTypePlatformRecovery" type load options. I figure the intended extension is for case (1.3), but the patch, as proposed, will also affect (1.1) and (1.2); that is, when Driver#### and SysPrep#### options are processes. That's beyond what the spec says, in my opinion. Now, EfiBootManagerProcessLoadOption() takes a pointer to an EFI_BOOT_MANAGER_LOAD_OPTION structure. This structure has a field called OptionType (of type EFI_BOOT_MANAGER_LOAD_OPTION_TYPE). You might want to restrict the signaling and the status code reporting to when LoadOption->OptionType is either LoadOptionTypeBoot or LoadOptionTypePlatformRecovery (i.e., exclude LoadOptionTypeDriver and LoadOptionTypeSysPrep). ( I've made an attempt to check the above-noted four call paths (i.e., (1.1) through (1.3), and (2)), and I *think* the OptionType field is populated on all of them; thus, the check I recommend should be safe. Call paths (1.1) through (1.3) call EfiBootManagerGetLoadOptions(), which sets the OptionType field in each returned structure according to the input parameter "LoadOptionType" (note the tricky call to EfiBootManagerVariableToLoadOption()). And (2) relies on "PlatformDefaultBootOption", which is created in BdsEntry() with LoadOptionTypePlatformRecovery. ) Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110126): https://edk2.groups.io/g/devel/message/110126 Mute This Topic: https://groups.io/mt/102200076/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 10/26/23 18:19, Laszlo Ersek wrote: > On 10/26/23 15:53, Neal Gompa wrote: >> From: Neal Gompa <ngompa@fedoraproject.org> >> >> Currently, the ReadyToBoot event is only signaled when a formal Boot >> Manager option is executed (in BmBoot.c -> EfiBootManagerBoot ()). >> >> However, the introduction of Platform Recovery in UEFI 2.5 makes it >> necessary to signal ReadyToBoot when a Platform Recovery boot loader >> runs because otherwise it may lead to the execution of a boot loader >> that has similar requirements to a regular one that is not launched >> as a Boot Manager option. >> >> This is especially critical to ensuring that the graphical console >> is actually usable during platform recovery, as some platforms do >> rely on the ConsolePrefDxe driver, which only performs console >> initialization after ReadyToBoot is triggered. >> >> This patch fixes that behavior by calling EfiSignalEventReadyToBoot () >> in EfiBootManagerProcessLoadOption (), which is the function that sets >> up the platform recovery boot process. >> >> The expected behavior has been clarified in the UEFI 2.10 specification >> to explicitly indicate this behavior is required for correct operation. >> >> This is a rebased version of the patch originally written by Pete Batard. >> >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2831 >> >> Cc: Pete Batard <pete@akeo.ie> >> Cc: Daniel P. Berrangé <berrange@redhat.com> >> Cc: Gerd Hoffmann <kraxel@redhat.com> >> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com> >> >> Co-authored-by: Pete Batard <pete@akeo.ie> >> Signed-off-by: Neal Gompa <ngompa@fedoraproject.org> >> --- >> MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c >> index 2087f0b91d..31ed608817 100644 >> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c >> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c >> @@ -1416,6 +1416,15 @@ EfiBootManagerProcessLoadOption ( >> return EFI_SUCCESS; >> } >> >> + // >> + // Signal the EVT_SIGNAL_READY_TO_BOOT event when we are about to load and execute the boot option. >> + // >> + EfiSignalEventReadyToBoot (); >> + // >> + // Report Status Code to indicate ReadyToBoot was signalled >> + // >> + REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT)); >> + >> // >> // Load and start the load option. >> // > > While the patch does the right thing for the latest UEFI spec language, > it does a bit too much. > > The spec says (v2.10), under 7.1.2 "EFI_BOOT_SERVICES.CreateEventEx()": > > ----------- > EFI_EVENT_GROUP_READY_TO_BOOT > > This event group is notified by the system right before notifying > EFI_EVENT_GROUP_AFTER_READY_TO_BOOT event group when the Boot Manager is > about to load and execute a boot option or a platform or OS recovery > option. The event group presents the last chance to modify device or > system configuration prior to passing control to a boot option. > ----------- > > NB "boot option", or "platform or OS recovery option". > > However, EfiBootManagerProcessLoadOption() is also used for launching > Driver#### and SysPrep#### options. > > EfiBootManagerProcessLoadOption() has two call sites: > > (1) in ProcessLoadOptions() [MdeModulePkg/Universal/BdsDxe/BdsEntry.c] > > (2) near the end of BdsEntry() [MdeModulePkg/Universal/BdsDxe/BdsEntry.c] > > Call site (2) bears the comment "When platform recovery is not enabled, > still boot to platform default file path", so I figure signaling > ready-to-boot at that point is fine. > > Call site (1) requires further investigation. Namely, > ProcessLoadOptions() itself is called from three locations, all inside > BdsEntry() [MdeModulePkg/Universal/BdsDxe/BdsEntry.c]: > > (1.1) under comment "Execute Driver Options", for "LoadOptionTypeDriver" > type load options > > (1.2) under comment "Execute SysPrep####", for "LoadOptionTypeSysPrep" > type load options > > (1.3) for "LoadOptionTypePlatformRecovery" type load options. > > I figure the intended extension is for case (1.3), but the patch, as > proposed, will also affect (1.1) and (1.2); that is, when Driver#### and > SysPrep#### options are processes. That's beyond what the spec says, in > my opinion. > > Now, EfiBootManagerProcessLoadOption() takes a pointer to an > EFI_BOOT_MANAGER_LOAD_OPTION structure. This structure has a field > called OptionType (of type EFI_BOOT_MANAGER_LOAD_OPTION_TYPE). You might > want to restrict the signaling and the status code reporting to when > LoadOption->OptionType is either LoadOptionTypeBoot or > LoadOptionTypePlatformRecovery (i.e., exclude LoadOptionTypeDriver and > LoadOptionTypeSysPrep). In fact, EfiBootManagerProcessLoadOption() already checks "LoadOption->OptionType" higher up, and if its LoadOptionTypeBoot, then the function returns early, with EFI_UNSUPPORTED. Therefore, IMO, you need to restrict the logic you are proposing to LoadOption->OptionType == LoadOptionTypePlatformRecovery exclusively. Laszlo > > ( > > I've made an attempt to check the above-noted four call paths (i.e., > (1.1) through (1.3), and (2)), and I *think* the OptionType field is > populated on all of them; thus, the check I recommend should be safe. > > Call paths (1.1) through (1.3) call EfiBootManagerGetLoadOptions(), > which sets the OptionType field in each returned structure according to > the input parameter "LoadOptionType" (note the tricky call to > EfiBootManagerVariableToLoadOption()). > > And (2) relies on "PlatformDefaultBootOption", which is created in > BdsEntry() with LoadOptionTypePlatformRecovery. > > ) > > Laszlo > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110128): https://edk2.groups.io/g/devel/message/110128 Mute This Topic: https://groups.io/mt/102200076/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.