Hi Sunny, thanks for looking into this.
On 2020.06.17 09:16, Wang, Sunny (HPS SW) wrote:
Hi Pete.
Since the EfiBootManagerProcessLoadOption is called by ProcessLoadOptions as
well, your change would also cause some unexpected behavior like:
1. Signal one more ReadyToBoot for the PlatformRecovery option which is an
application that calls EfiBootManagerBoot() to launch its recovered boot option.
I'm not sure I understand how this part is unwanted.
The point of this patch is to ensure that ReadyToBoot is signalled for
the PlatformRecovery option, so isn't what you describe above exactly
what we want?
Or is the "one more" the issue, meaning that it would get signalled more
than once?
2. Signal ReadyToBoot for SysPrep#### or Driver#### that is not really a "boot"
option.
Yes, I've been wondering about that, because BdsEntry.c's
ProcessLoadOptions(), which calls EfiBootManagerProcessLoadOption(),
mentions that it will load will load and start every Driver####,
SysPrep#### or PlatformRecovery####. But the comment about the while()
loop in EfiBootManagerProcessLoadOption() only mentions
PlatformRecovery####.
If needed, I guess we could amend the patch to detect the type of option
and only signal ReadyToBoot for PlatformRecovery####.
To solve your problem, creating a PlatformRecovery option with the smallest
option number and using it instead of default one (with short-form File Path
Media Device Path) looks like a simpler solution.
I don't mind trying an alternative approach, but I don't understand how
what you describe would help. Can you please be more specific about what
you have in mind?
Our main issue here is that we must have ReadyToBoot signalled so that
the ReadyToBoot() function callback from
EmbeddedPkg/Drivers/ConsolePrefDxe gets executed in order for the boot
loader invoked from PlatformRecovery#### to use a properly initialized
graphical console. So I'm not sure I quite get how switching from one
PlatformRecovery#### option to another would improve things.
If it helps, here is what we currently default to, in terms of boot
options, on a Raspberry Pi 4 platform with a newly build firmware:
[Bds]=============Begin Load Options Dumping ...=============
Driver Options:
SysPrep Options:
Boot Options:
Boot0000: UiApp 0x0109
Boot0001: UEFI Shell 0x0000
PlatformRecovery Options:
PlatformRecovery0000: Default PlatformRecovery 0x0001
[Bds]=============End Load Options Dumping=============
With this, PlatformRecovery0000 gets executed by default, which is what
we want, since it will pick /efi/boot/bootaa64.efi from either SD or USB
and run it, the only issue being that, because ReadyToBoot has not been
executed, the graphical console is not operative so users can't interact
with the OS installer.
So I'm really not sure how adding an extra PlatformRecovery#### would
help. And I'm also not too familiar with how one would go around to add
such an entry...
By the way, I also checked the UEFI specification. It looks making sense to
only signal ReadyToBoot for boot option (Boot####).
That's something I considered too, but I disagree with this conclusion.
My reasoning is that, if PlatformRecovery#### can execute a regular
bootloader like /efi/boot/boot####.efi from installation media, then it
should go through the same kind of initialization that happens for a
regular boot option, and that should include signalling the ReadyToBoot
event.
If there was a special bootloader for PlatformRecovery#### (e.g.
/efi/boot/recovery####.efi) then I would agree with only signalling
ReadyToBoot for a formal Boot#### option. But that isn't the case, so I
think it is reasonable to want to have ReadyToBoot also occur when a
/efi/boot/boot####.efi bootloader is executed from
PlatformRecovery####., especially when we know it can be crucial to
ensuring that the end user can actually use the graphical console.
Therefore, your change may also require specification change.
Yes, I mentioned that in the cover letter for this patch
(https://edk2.groups.io/g/devel/message/61327), which also describes the
issue we are trying to solve in greater details. This is what I wrote:
------------------------------------------------------------------------
Note however that this may require a specs update, as the current UEFI
specs for EFI_BOOT_SERVICES.CreateEventEx() have:
> EFI_EVENT_GROUP_READY_TO_BOOT
> This event group is notified by the system when the Boot Manager
> is about to load and execute a boot option.
and, once this patch has been applied, we may want to update this
section to mention that it applies to both Boot Manager and Platform
Recovery.
------------------------------------------------------------------------
Again, I don't have an issue with trying to use an alternate approach to
solve our problem (though I ultimately believe that, if
PlatformRecovery#### can launch a /efi/boot/boot####.efi bootloader then
we must update the specs and the code to have ReadyToBoot also signalled
then, because that's the logical thing to do). But right now, I'm not
seeing how to achieve that when PlatformRecovery#### is the option that
is used to launch the OS installation the bootloader. So if you can
provide mode details on how exactly you think creating an alternate
PlatformRecovery option would help, I would appreciate it.
Regards,
/Pete
Regards,
Sunny Wang
-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Pete
Batard
Sent: Tuesday, June 16, 2020 5:56 PM
To: devel@edk2.groups.io
Cc: zhichao....@intel.com; ray...@intel.com; ard.biesheu...@arm.com;
l...@nuviainc.com
Subject: [edk2-devel] [edk2][PATCH 1/1] MdeModulePkg/UefiBootManagerLib: Signal
ReadyToBoot on platform recovery
Currently, the ReadyToBoot event is only signaled when a formal Boot Manager
option is executed (in BmBoot.c -> EfiBootManagerBoot ()).
However, with the introduction of Platform Recovery in UEFI 2.5, which may lead
to the execution of a boot loader that has similar requirements to a regular
one, yet is not launched as a Boot Manager option, it also becomes necessary to
signal ReadyToBoot when a Platform Recovery boot loader runs.
Especially, this can be 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 behaviour by calling EfiSignalEventReadyToBoot () in
EfiBootManagerProcessLoadOption (), which is the function that sets up the
platform recovery boot process.
Signed-off-by: Pete Batard <p...@akeo.ie>
---
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 89372b3b97b8..117f1f5b124c 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -1376,6 +1376,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.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61400): https://edk2.groups.io/g/devel/message/61400
Mute This Topic: https://groups.io/mt/74912987/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
