Re: [edk2-devel] [PATCH v2] MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform recovery

["Jeremy Linton" <jeremy.linton@arm.com>]

On 10/31/23 12:37, Neal Gompa via groups.io 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 () when invoking platform recovery,
> 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.

Took me a bit to swap in that whole conversation again, and recheck the 
spec's and code paths, but this all looks fine to me and should allow 
the PFTF build to drop the similar patch from Pete that has been carried 
downstream for the past couple years.

As for testing the previous patch has been in the field for a couple 
years now and i'm not aware of it causing any issues. The additional 
restriction of limiting it to platform recovery logically makes sense, 
and as far as I can see shouldn't cause any problems.

So,

Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>


As a PS: I also went to check the ready to boot behavior for OsRecovery 
and realized that apparently none of that support was ever merged? That 
seems a bit of an oversight since its been in the spec for a few years now.


> 
> 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>
> Cc: Laszlo Ersek <lersek@redhat.com>
> 
> Co-authored-by: Pete Batard <pete@akeo.ie>
> Signed-off-by: Neal Gompa <ngompa@fedoraproject.org>
> ---
>   .../Library/UefiBootManagerLib/BmLoadOption.c         | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> index 2087f0b91d..83a2f893e4 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> @@ -1416,6 +1416,17 @@ EfiBootManagerProcessLoadOption (
>       return EFI_SUCCESS;
>     }
>   
> +  if (LoadOption->OptionType == LoadOptionTypePlatformRecovery) {
> +    //
> +    // 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 signaled
> +    //
> +    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.
>     //



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110454): https://edk2.groups.io/g/devel/message/110454
Mute This Topic: https://groups.io/mt/102302654/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-