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

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

[Neal Gompa]

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.

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.
   //
-- 
2.41.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110438): https://edk2.groups.io/g/devel/message/110438
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]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Jeremy Linton]

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]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Laszlo Ersek]

On 10/31/23 18:37, 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 () 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.
> 
> 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.
>    //

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



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

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

[Laszlo Ersek]

On 10/31/23 23:27, Jeremy Linton wrote:
> 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?

What else is there, outside of this patch, in need of upstreaming?

> That seems a bit of an oversight since its been in the spec for a few years now.

"ready-to-boot for OsRecovery" is new in UEFI 2.10 (according to the
commit message), which is quite recent ("Aug 29, 2022").

I couldn't find the Mantis ticket in the Revision History (for 2.10) though.

Laszlo


> 
> 
>>
>> 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 (#110519): https://edk2.groups.io/g/devel/message/110519
Mute This Topic: https://groups.io/mt/102302654/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Neal Gompa]

On Thu, Nov 2, 2023 at 6:35 AM Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 10/31/23 23:27, Jeremy Linton wrote:
> > 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?
>
> What else is there, outside of this patch, in need of upstreaming?
>
> > That seems a bit of an oversight since its been in the spec for a few years now.
>
> "ready-to-boot for OsRecovery" is new in UEFI 2.10 (according to the
> commit message), which is quite recent ("Aug 29, 2022").
>
> I couldn't find the Mantis ticket in the Revision History (for 2.10) though.
>

Is there anything else holding up committing this patch? Jeremy and
you reviewed it earlier in the month...



--
真実はいつも一つ！/ Always, there's only one truth!


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111703): https://edk2.groups.io/g/devel/message/111703
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]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Neal Gompa]

On Fri, Nov 24, 2023 at 6:36 PM Neal Gompa <ngompa13@gmail.com> wrote:
>
> On Thu, Nov 2, 2023 at 6:35 AM Laszlo Ersek <lersek@redhat.com> wrote:
> >
> > On 10/31/23 23:27, Jeremy Linton wrote:
> > > 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?
> >
> > What else is there, outside of this patch, in need of upstreaming?
> >
> > > That seems a bit of an oversight since its been in the spec for a few years now.
> >
> > "ready-to-boot for OsRecovery" is new in UEFI 2.10 (according to the
> > commit message), which is quite recent ("Aug 29, 2022").
> >
> > I couldn't find the Mantis ticket in the Revision History (for 2.10) though.
> >
>
> Is there anything else holding up committing this patch? Jeremy and
> you reviewed it earlier in the month...
>

Friendly ping again? It's been a month now...



-- 
真実はいつも一つ！/ Always, there's only one truth!


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112157): https://edk2.groups.io/g/devel/message/112157
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]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Ard Biesheuvel]

(cc Liming)

On Thu, 7 Dec 2023 at 05:48, Neal Gompa <ngompa13@gmail.com> wrote:
>
> On Fri, Nov 24, 2023 at 6:36 PM Neal Gompa <ngompa13@gmail.com> wrote:
> >
> > On Thu, Nov 2, 2023 at 6:35 AM Laszlo Ersek <lersek@redhat.com> wrote:
> > >
> > > On 10/31/23 23:27, Jeremy Linton wrote:
> > > > 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?
> > >
> > > What else is there, outside of this patch, in need of upstreaming?
> > >
> > > > That seems a bit of an oversight since its been in the spec for a few years now.
> > >
> > > "ready-to-boot for OsRecovery" is new in UEFI 2.10 (according to the
> > > commit message), which is quite recent ("Aug 29, 2022").
> > >
> > > I couldn't find the Mantis ticket in the Revision History (for 2.10) though.
> > >
> >
> > Is there anything else holding up committing this patch? Jeremy and
> > you reviewed it earlier in the month...
> >
>
> Friendly ping again? It's been a month now...
>

Apologies for the delay - Liming, can we progress with this?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112165): https://edk2.groups.io/g/devel/message/112165
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]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Ard Biesheuvel]

(cc Mike, Leif)

On Thu, 7 Dec 2023 at 08:40, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> (cc Liming)
>
> On Thu, 7 Dec 2023 at 05:48, Neal Gompa <ngompa13@gmail.com> wrote:
> >
> > On Fri, Nov 24, 2023 at 6:36 PM Neal Gompa <ngompa13@gmail.com> wrote:
> > >
> > > On Thu, Nov 2, 2023 at 6:35 AM Laszlo Ersek <lersek@redhat.com> wrote:
> > > >
> > > > On 10/31/23 23:27, Jeremy Linton wrote:
> > > > > 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?
> > > >
> > > > What else is there, outside of this patch, in need of upstreaming?
> > > >
> > > > > That seems a bit of an oversight since its been in the spec for a few years now.
> > > >
> > > > "ready-to-boot for OsRecovery" is new in UEFI 2.10 (according to the
> > > > commit message), which is quite recent ("Aug 29, 2022").
> > > >
> > > > I couldn't find the Mantis ticket in the Revision History (for 2.10) though.
> > > >
> > >
> > > Is there anything else holding up committing this patch? Jeremy and
> > > you reviewed it earlier in the month...
> > >
> >
> > Friendly ping again? It's been a month now...
> >
>
> Apologies for the delay - Liming, can we progress with this?

Can we please make some progress with this? This has been in limbo for
far too long.

Thanks,


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112360): https://edk2.groups.io/g/devel/message/112360
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]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Ard Biesheuvel]

Hello all,

Same question again. Could we please make some progress on this?

Full thread here:
https://openfw.io/edk2-devel/20231031173700.647004-1-ngompa@fedoraproject.org/

If nobody objects, I will assume that the change is acceptable and
merge it by the end of the week.

Thanks,
Ard.



On Tue, 12 Dec 2023 at 09:11, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> (cc Mike, Leif)
>
> On Thu, 7 Dec 2023 at 08:40, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > (cc Liming)
> >
> > On Thu, 7 Dec 2023 at 05:48, Neal Gompa <ngompa13@gmail.com> wrote:
> > >
> > > On Fri, Nov 24, 2023 at 6:36 PM Neal Gompa <ngompa13@gmail.com> wrote:
> > > >
> > > > On Thu, Nov 2, 2023 at 6:35 AM Laszlo Ersek <lersek@redhat.com> wrote:
> > > > >
> > > > > On 10/31/23 23:27, Jeremy Linton wrote:
> > > > > > 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?
> > > > >
> > > > > What else is there, outside of this patch, in need of upstreaming?
> > > > >
> > > > > > That seems a bit of an oversight since its been in the spec for a few years now.
> > > > >
> > > > > "ready-to-boot for OsRecovery" is new in UEFI 2.10 (according to the
> > > > > commit message), which is quite recent ("Aug 29, 2022").
> > > > >
> > > > > I couldn't find the Mantis ticket in the Revision History (for 2.10) though.
> > > > >
> > > >
> > > > Is there anything else holding up committing this patch? Jeremy and
> > > > you reviewed it earlier in the month...
> > > >
> > >
> > > Friendly ping again? It's been a month now...
> > >
> >
> > Apologies for the delay - Liming, can we progress with this?
>
> Can we please make some progress with this? This has been in limbo for
> far too long.
>
> Thanks,


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112653): https://edk2.groups.io/g/devel/message/112653
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]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Leif Lindholm]

On Mon, Dec 18, 2023 at 22:55:21 +0100, Ard Biesheuvel wrote:
> Hello all,
> 
> Same question again. Could we please make some progress on this?
> 
> Full thread here:
> https://openfw.io/edk2-devel/20231031173700.647004-1-ngompa@fedoraproject.org/
> 
> If nobody objects, I will assume that the change is acceptable and
> merge it by the end of the week.

I'm OK with this.

The last comment from Liming in
https://bugzilla.tianocore.org/show_bug.cgi?id=2831
was that the fix could be merged after "the next UEFI is published",
which it was - in August 2022.

Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>

Regards,

Leif


> Thanks,
> Ard.
> 
> 
> 
> On Tue, 12 Dec 2023 at 09:11, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > (cc Mike, Leif)
> >
> > On Thu, 7 Dec 2023 at 08:40, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > (cc Liming)
> > >
> > > On Thu, 7 Dec 2023 at 05:48, Neal Gompa <ngompa13@gmail.com> wrote:
> > > >
> > > > On Fri, Nov 24, 2023 at 6:36 PM Neal Gompa <ngompa13@gmail.com> wrote:
> > > > >
> > > > > On Thu, Nov 2, 2023 at 6:35 AM Laszlo Ersek <lersek@redhat.com> wrote:
> > > > > >
> > > > > > On 10/31/23 23:27, Jeremy Linton wrote:
> > > > > > > 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?
> > > > > >
> > > > > > What else is there, outside of this patch, in need of upstreaming?
> > > > > >
> > > > > > > That seems a bit of an oversight since its been in the spec for a few years now.
> > > > > >
> > > > > > "ready-to-boot for OsRecovery" is new in UEFI 2.10 (according to the
> > > > > > commit message), which is quite recent ("Aug 29, 2022").
> > > > > >
> > > > > > I couldn't find the Mantis ticket in the Revision History (for 2.10) though.
> > > > > >
> > > > >
> > > > > Is there anything else holding up committing this patch? Jeremy and
> > > > > you reviewed it earlier in the month...
> > > > >
> > > >
> > > > Friendly ping again? It's been a month now...
> > > >
> > >
> > > Apologies for the delay - Liming, can we progress with this?
> >
> > Can we please make some progress with this? This has been in limbo for
> > far too long.
> >
> > Thanks,
> 
> 
> 
> 
> 


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

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

[gaoliming via groups.io]

Yes. The latest spec has clarified this behavior. So, this change is OK. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Leif Lindholm
> 发送时间: 2023年12月19日 19:51
> 收件人: devel@edk2.groups.io; ardb@kernel.org
> 抄送: ngompa13@gmail.com; Liming Gao (Byosoft address)
> <gaoliming@byosoft.com.cn>; Michael Kinney <michael.d.kinney@intel.com>;
> Laszlo Ersek <lersek@redhat.com>; Jeremy Linton <jeremy.linton@arm.com>;
> Pete Batard <pete@akeo.ie>; Daniel P . Berrangé <berrange@redhat.com>;
> Gerd Hoffmann <kraxel@redhat.com>; Samer El-Haj-Mahmoud
> <samer.el-haj-mahmoud@arm.com>
> 主题: Re: [edk2-devel] [PATCH v2] MdeModulePkg/UefiBootManagerLib:
> Signal ReadyToBoot on platform recovery
> 
> On Mon, Dec 18, 2023 at 22:55:21 +0100, Ard Biesheuvel wrote:
> > Hello all,
> >
> > Same question again. Could we please make some progress on this?
> >
> > Full thread here:
> >
> https://openfw.io/edk2-devel/20231031173700.647004-1-ngompa@fedorap
> roject.org/
> >
> > If nobody objects, I will assume that the change is acceptable and
> > merge it by the end of the week.
> 
> I'm OK with this.
> 
> The last comment from Liming in
> https://bugzilla.tianocore.org/show_bug.cgi?id=2831
> was that the fix could be merged after "the next UEFI is published",
> which it was - in August 2022.
> 
> Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
> 
> Regards,
> 
> Leif
> 
> 
> > Thanks,
> > Ard.
> >
> >
> >
> > On Tue, 12 Dec 2023 at 09:11, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > (cc Mike, Leif)
> > >
> > > On Thu, 7 Dec 2023 at 08:40, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > (cc Liming)
> > > >
> > > > On Thu, 7 Dec 2023 at 05:48, Neal Gompa <ngompa13@gmail.com>
> wrote:
> > > > >
> > > > > On Fri, Nov 24, 2023 at 6:36 PM Neal Gompa <ngompa13@gmail.com>
> wrote:
> > > > > >
> > > > > > On Thu, Nov 2, 2023 at 6:35 AM Laszlo Ersek <lersek@redhat.com>
> wrote:
> > > > > > >
> > > > > > > On 10/31/23 23:27, Jeremy Linton wrote:
> > > > > > > > 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?
> > > > > > >
> > > > > > > What else is there, outside of this patch, in need of upstreaming?
> > > > > > >
> > > > > > > > That seems a bit of an oversight since its been in the spec for a
> few years now.
> > > > > > >
> > > > > > > "ready-to-boot for OsRecovery" is new in UEFI 2.10 (according to
> the
> > > > > > > commit message), which is quite recent ("Aug 29, 2022").
> > > > > > >
> > > > > > > I couldn't find the Mantis ticket in the Revision History (for 2.10)
> though.
> > > > > > >
> > > > > >
> > > > > > Is there anything else holding up committing this patch? Jeremy and
> > > > > > you reviewed it earlier in the month...
> > > > > >
> > > > >
> > > > > Friendly ping again? It's been a month now...
> > > > >
> > > >
> > > > Apologies for the delay - Liming, can we progress with this?
> > >
> > > Can we please make some progress with this? This has been in limbo for
> > > far too long.
> > >
> > > Thanks,
> >
> >
> >
> >
> >
> 
> 
> 
> 





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

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

[Ard Biesheuvel]

On Tue, 19 Dec 2023 at 14:00, gaoliming via groups.io
<gaoliming=byosoft.com.cn@groups.io> wrote:
>
> Yes. The latest spec has clarified this behavior. So, this change is OK. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
>

Merged as #5165

Thanks all

> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Leif Lindholm
> > 发送时间: 2023年12月19日 19:51
> > 收件人: devel@edk2.groups.io; ardb@kernel.org
> > 抄送: ngompa13@gmail.com; Liming Gao (Byosoft address)
> > <gaoliming@byosoft.com.cn>; Michael Kinney <michael.d.kinney@intel.com>;
> > Laszlo Ersek <lersek@redhat.com>; Jeremy Linton <jeremy.linton@arm.com>;
> > Pete Batard <pete@akeo.ie>; Daniel P . Berrangé <berrange@redhat.com>;
> > Gerd Hoffmann <kraxel@redhat.com>; Samer El-Haj-Mahmoud
> > <samer.el-haj-mahmoud@arm.com>
> > 主题: Re: [edk2-devel] [PATCH v2] MdeModulePkg/UefiBootManagerLib:
> > Signal ReadyToBoot on platform recovery
> >
> > On Mon, Dec 18, 2023 at 22:55:21 +0100, Ard Biesheuvel wrote:
> > > Hello all,
> > >
> > > Same question again. Could we please make some progress on this?
> > >
> > > Full thread here:
> > >
> > https://openfw.io/edk2-devel/20231031173700.647004-1-ngompa@fedorap
> > roject.org/
> > >
> > > If nobody objects, I will assume that the change is acceptable and
> > > merge it by the end of the week.
> >
> > I'm OK with this.
> >
> > The last comment from Liming in
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2831
> > was that the fix could be merged after "the next UEFI is published",
> > which it was - in August 2022.
> >
> > Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
> >
> > Regards,
> >
> > Leif
> >
> >
> > > Thanks,
> > > Ard.
> > >
> > >
> > >
> > > On Tue, 12 Dec 2023 at 09:11, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > (cc Mike, Leif)
> > > >
> > > > On Thu, 7 Dec 2023 at 08:40, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >
> > > > > (cc Liming)
> > > > >
> > > > > On Thu, 7 Dec 2023 at 05:48, Neal Gompa <ngompa13@gmail.com>
> > wrote:
> > > > > >
> > > > > > On Fri, Nov 24, 2023 at 6:36 PM Neal Gompa <ngompa13@gmail.com>
> > wrote:
> > > > > > >
> > > > > > > On Thu, Nov 2, 2023 at 6:35 AM Laszlo Ersek <lersek@redhat.com>
> > wrote:
> > > > > > > >
> > > > > > > > On 10/31/23 23:27, Jeremy Linton wrote:
> > > > > > > > > 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?
> > > > > > > >
> > > > > > > > What else is there, outside of this patch, in need of upstreaming?
> > > > > > > >
> > > > > > > > > That seems a bit of an oversight since its been in the spec for a
> > few years now.
> > > > > > > >
> > > > > > > > "ready-to-boot for OsRecovery" is new in UEFI 2.10 (according to
> > the
> > > > > > > > commit message), which is quite recent ("Aug 29, 2022").
> > > > > > > >
> > > > > > > > I couldn't find the Mantis ticket in the Revision History (for 2.10)
> > though.
> > > > > > > >
> > > > > > >
> > > > > > > Is there anything else holding up committing this patch? Jeremy and
> > > > > > > you reviewed it earlier in the month...
> > > > > > >
> > > > > >
> > > > > > Friendly ping again? It's been a month now...
> > > > > >
> > > > >
> > > > > Apologies for the delay - Liming, can we progress with this?
> > > >
> > > > Can we please make some progress with this? This has been in limbo for
> > > > far too long.
> > > >
> > > > Thanks,
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
>
>
>
>
>
> 
>
>


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

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

[Samer El-Haj-Mahmoud]

Thank you all!

> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Tuesday, December 19, 2023 8:59 AM
> To: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> Cc: quic_llindhol@quicinc.com; ngompa13@gmail.com; Michael Kinney
> <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Jeremy Linton
> <Jeremy.Linton@arm.com>; Pete Batard <pete@akeo.ie>; Daniel P . Berrangé
> <berrange@redhat.com>; Gerd Hoffmann <kraxel@redhat.com>; Samer El-Haj-
> Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/UefiBootManagerLib:
> Signal ReadyToBoot on platform recovery
>
> On Tue, 19 Dec 2023 at 14:00, gaoliming via groups.io
> <gaoliming=byosoft.com.cn@groups.io> wrote:
> >
> > Yes. The latest spec has clarified this behavior. So, this change is OK. Reviewed-
> by: Liming Gao <gaoliming@byosoft.com.cn>
> >
>
> Merged as #5165
>
> Thanks all
>
> > > -----邮件原件-----
> > > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Leif Lindholm
> > > 发送时间: 2023年12月19日 19:51
> > > 收件人: devel@edk2.groups.io; ardb@kernel.org
> > > 抄送: ngompa13@gmail.com; Liming Gao (Byosoft address)
> > > <gaoliming@byosoft.com.cn>; Michael Kinney
> <michael.d.kinney@intel.com>;
> > > Laszlo Ersek <lersek@redhat.com>; Jeremy Linton <jeremy.linton@arm.com>;
> > > Pete Batard <pete@akeo.ie>; Daniel P . Berrangé <berrange@redhat.com>;
> > > Gerd Hoffmann <kraxel@redhat.com>; Samer El-Haj-Mahmoud
> > > <samer.el-haj-mahmoud@arm.com>
> > > 主题: Re: [edk2-devel] [PATCH v2] MdeModulePkg/UefiBootManagerLib:
> > > Signal ReadyToBoot on platform recovery
> > >
> > > On Mon, Dec 18, 2023 at 22:55:21 +0100, Ard Biesheuvel wrote:
> > > > Hello all,
> > > >
> > > > Same question again. Could we please make some progress on this?
> > > >
> > > > Full thread here:
> > > >
> > > https://openfw.io/edk2-devel/20231031173700.647004-1-
> ngompa@fedorap
> > > roject.org/
> > > >
> > > > If nobody objects, I will assume that the change is acceptable and
> > > > merge it by the end of the week.
> > >
> > > I'm OK with this.
> > >
> > > The last comment from Liming in
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=2831
> > > was that the fix could be merged after "the next UEFI is published",
> > > which it was - in August 2022.
> > >
> > > Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
> > >
> > > Regards,
> > >
> > > Leif
> > >
> > >
> > > > Thanks,
> > > > Ard.
> > > >
> > > >
> > > >
> > > > On Tue, 12 Dec 2023 at 09:11, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >
> > > > > (cc Mike, Leif)
> > > > >
> > > > > On Thu, 7 Dec 2023 at 08:40, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > >
> > > > > > (cc Liming)
> > > > > >
> > > > > > On Thu, 7 Dec 2023 at 05:48, Neal Gompa <ngompa13@gmail.com>
> > > wrote:
> > > > > > >
> > > > > > > On Fri, Nov 24, 2023 at 6:36 PM Neal Gompa
> <ngompa13@gmail.com>
> > > wrote:
> > > > > > > >
> > > > > > > > On Thu, Nov 2, 2023 at 6:35 AM Laszlo Ersek <lersek@redhat.com>
> > > wrote:
> > > > > > > > >
> > > > > > > > > On 10/31/23 23:27, Jeremy Linton wrote:
> > > > > > > > > > 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?
> > > > > > > > >
> > > > > > > > > What else is there, outside of this patch, in need of upstreaming?
> > > > > > > > >
> > > > > > > > > > That seems a bit of an oversight since its been in the spec for a
> > > few years now.
> > > > > > > > >
> > > > > > > > > "ready-to-boot for OsRecovery" is new in UEFI 2.10 (according to
> > > the
> > > > > > > > > commit message), which is quite recent ("Aug 29, 2022").
> > > > > > > > >
> > > > > > > > > I couldn't find the Mantis ticket in the Revision History (for 2.10)
> > > though.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Is there anything else holding up committing this patch? Jeremy and
> > > > > > > > you reviewed it earlier in the month...
> > > > > > > >
> > > > > > >
> > > > > > > Friendly ping again? It's been a month now...
> > > > > > >
> > > > > >
> > > > > > Apologies for the delay - Liming, can we progress with this?
> > > > >
> > > > > Can we please make some progress with this? This has been in limbo for
> > > > > far too long.
> > > > >
> > > > > Thanks,
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> > 
> >
> >
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 (#112731): https://edk2.groups.io/g/devel/message/112731
Mute This Topic: https://groups.io/mt/103262405/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

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

[Neal Gompa]

On Tue, Dec 19, 2023 at 9:11 AM Samer El-Haj-Mahmoud
<Samer.El-Haj-Mahmoud@arm.com> wrote:
>
> Thank you all!
>

Thank you everyone, I see that it has landed in the repository now:
https://github.com/tianocore/edk2/commit/8c1e9f9c6fa7b5137003b0cfa6d54a6bada16d8e



-- 
真実はいつも一つ！/ Always, there's only one truth!


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