[EXTERNAL] Re: [edk2-devel] [edk2][PATCH 1/1] MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform recovery
Laszlo Ersek
lersek at redhat.com
Wed Feb 17 14:25:41 UTC 2021
On 02/17/21 13:18, Pete Batard wrote:
> Hi Leif,
>
> Thanks for trying to resurrect this issue.
>
> At this stage, and despite some initial pushback in the bugzilla ticket,
> I believe we can all agree with the consensus that UefiBootManagerLib is
> not in fact specs-compliant and therefore needs to be fixed, one way or
> another, to make it specs-compliant.
>
> My take on this is that, rather than propose a new patch, I'd much
> rather have the current maintainers agree on the course of action to fix
> the library (which, as Leif suggests, might very well be to split the
> library into a specs-compliant and non-specs-compliant version), as it
> would of course be better if the fix came from people who have better
> understanding of the ramifications we might face with trying to correct
> the current behaviour, and especially, who have knowledge of the
> platforms that might be impacted from making the lib specs-compliant.
>
> Especially, I don't think that the patch that I originally submitted for
> this, or the additional proposals we made, are still receivable, as they
> seem to fall short of fixing the issue in a manner that all platforms
> can be happy with. And that is why I'd like to hear from the maintainers
> on what their preferred approach would be.
A new Feature PCD could satisfy both sets of platforms, could it not?
(Sorry if the original patch already had such a PCD; I don't remember.)
Of course then we'd have a debate around the DEC default for the new PCD
-- I'd say the default value of the PCD should match the spec-mandated
behavior.
I don't recall any specifics, but a bug-compat pattern that's sometimes
used is this:
if (BugCompatEnabled) {
//
// do the right thing in the wrong place, for legacy platforms' sake
//
Foo ();
}
//
// Do some stuff.
//
Bar ();
if (!BugCompatEnabled) {
//
// do the right thing in the right place, for conformant platforms
//
Foo ();
}
Not sure if it applies here.
Thanks
Laszlo
> On 2021.02.17 11:42, Leif Lindholm wrote:
>> Hi Pete, +various
>>
>> Resurrecting this old thread since Ard pointed out an issue I ran into
>> myself had already been encountered by Pete.
>> And the bugzilla ticket (directly below this reply) has had no
>> relevant progress since August.
>>
>> Executive summary:
>> The current UefiBootManagerLib implementation of the PlatformRecovery
>> path does not notify the EFI_EVENT_GROUP_READY_TO_BOOT event.
>>
>> The argument has been made that since changing this would affect an
>> unnamed number of non-public platforms, the behaviour cannot be
>> changed even though it violates the UEFI specification.
>>
>> I disagree with that statement. If we want to fork UefiBootManagerLib
>> into a BrokenLegacyUefiBootManagerLib and an actually correct one, and
>> have those platforms move to the BrokenLegacy variant, I'm OK with
>> that.
>>
>> But using the default version should give specification-compliant
>> behaviour.
>>
>> /
>> Leif
>>
>> On Tue, Jun 30, 2020 at 18:17:10 +0100, Pete Batard wrote:
>>> Please note that I have created a bug report
>>> (https://bugzilla.tianocore.org/show_bug.cgi?id=2831) to address the
>>> non-compliance issue was raised during the course of the discussion
>>> below.
>>>
>>> Regards,
>>>
>>> /Pete
>>>
>>>
>>> On 2020.06.17 18:06, Samer El-Haj-Mahmoud wrote:
>>>> I worked with Pete offline on this..
>>>>
>>>> This code seems to be violating the UEFI Spec:
>>>>
>>>> https://github.com/tianocore/edk2/blob/a56af23f066e2816c67b7c6e64de7ddefcd70780/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c#L1763
>>>>
>>>>
>>>> //
>>>> // 3. Signal the EVT_SIGNAL_READY_TO_BOOT event when we are
>>>> about to load and execute
>>>> // the boot option.
>>>> //
>>>> if (BmIsBootManagerMenuFilePath (BootOption->FilePath)) {
>>>> DEBUG ((EFI_D_INFO, "[Bds] Booting Boot Manager Menu.\n"));
>>>> BmStopHotkeyService (NULL, NULL);
>>>> } else {
>>>> 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));
>>>> //
>>>> // 4. Repair system through DriverHealth protocol
>>>> //
>>>> BmRepairAllControllers (0);
>>>> }
>>>>
>>>> The UEFI Spec section 3.1.7 clearly states that Boot Options (and
>>>> their FilePathList) *shall not* be evaluated prior to the completion
>>>> of EFI_EVENT_GROUP_READY_TO_BOOT event group processing:
>>>>
>>>> "After all SysPrep#### variables have been launched and exited, the
>>>> platform shall notify EFI_EVENT_GROUP_READY_TO_BOOT event group and
>>>> begin to evaluate Boot#### variables with Attributes set to
>>>> LOAD_OPTION_CATEGORY_BOOT according to the order defined by
>>>> BootOrder. The FilePathList of variables marked
>>>> LOAD_OPTION_CATEGORY_BOOT shall not be evaluated prior to the
>>>> completion of EFI_EVENT_GROUP_READY_TO_BOOT event group processing."
>>>>
>>>> This is a prescriptive language that is stronger than the language
>>>> in section 7.1 which defines the ReadyToBoot event group in a
>>>> general way:
>>>>
>>>> "EFI_EVENT_GROUP_RESET_SYSTEM
>>>> This event group is notified by the system when ResetSystem() is
>>>> invoked and the system is about to be reset. The event group is only
>>>> notified prior to ExitBootServices() invocation."
>>>>
>>>> The EDK2 code in the else block above (to call
>>>> EfiSignalEventReadyToBoot() ) need to move before the code that is
>>>> processing BootOption->FilePath. In fact, why is this signaling even
>>>> a BootManager task? It should be a higher level BDS task (after
>>>> processing SysPrp and before processing Boot options, per the spec).
>>>> This would be somewhere around
>>>> https://github.com/tianocore/edk2/blob/b15646484eaffcf7cc464fdea0214498f26addc2/MdeModulePkg/Universal/BdsDxe/BdsEntry.c#L1007
>>>> where SysPrep is processed.
>>>>
>>>> This should also take care of the issue Pete reported in this
>>>> thread, without the need for explicitly signaling ReadyToBoot from
>>>> PlatformRecovery (or changing the UEFI spec).
>>>>
>>>> Thanks,
>>>> --Samer
>>>>
>>>>
>>>>
>>>> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Samer
>>>> El-Haj-Mahmoud via groups.io
>>>> Sent: Wednesday, June 17, 2020 12:42 PM
>>>> To: devel at edk2.groups.io; Andrei Warkentin (awarkentin at vmware.com)
>>>> <awarkentin at vmware.com>; Wang, Sunny (HPS SW) <sunnywang at hpe.com>;
>>>> pete at akeo.ie
>>>> Cc: zhichao.gao at intel.com; ray.ni at intel.com; Ard Biesheuvel
>>>> <Ard.Biesheuvel at arm.com>; leif at nuviainc.com; Samer El-Haj-Mahmoud
>>>> <Samer.El-Haj-Mahmoud at arm.com>
>>>> Subject: Re: [edk2-devel] [edk2][PATCH 1/1]
>>>> MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform
>>>> recovery
>>>>
>>>> The UEFI spec (3.1.7) says:
>>>>
>>>> "After all SysPrep#### variables have been launched and exited, the
>>>> platform shall notify EFI_EVENT_GROUP_READY_TO_BOOT event group and
>>>> begin to evaluate Boot#### variables with Attributes set to
>>>> LOAD_OPTION_CATEGORY_BOOT according to the order defined by
>>>> BootOrder. The FilePathList of variables marked
>>>> LOAD_OPTION_CATEGORY_BOOT shall not be evaluated prior to the
>>>> completion of EFI_EVENT_GROUP_READY_TO_BOOT event group processing."
>>>>
>>>> The way I read this, I expect ReadyToBoot to be signaled after
>>>> SysPrep#### (if any) are processed, but before Boot#### are
>>>> processed. Is my understanding correct that this language implies
>>>> ReadyToBoot need to be signaled even if BootOrder does not contain
>>>> any Boot#### options marked as LOAD_OPTION_CATEGORY_BOOT? And if so,
>>>> is EDK2 not doing this, which leads us to this patch (signaling it
>>>> in PlatformRecovery?)
>>>>
>>>>
>>>>
>>>> From: mailto:devel at edk2.groups.io <mailto:devel at edk2.groups.io> On
>>>> Behalf Of Andrei Warkentin via groups.io
>>>> Sent: Wednesday, June 17, 2020 12:37 PM
>>>> To: Wang, Sunny (HPS SW) <mailto:sunnywang at hpe.com>;
>>>> mailto:devel at edk2.groups.io; mailto:pete at akeo.ie
>>>> Cc: mailto:zhichao.gao at intel.com; mailto:ray.ni at intel.com; Ard
>>>> Biesheuvel <mailto:Ard.Biesheuvel at arm.com>; mailto:leif at nuviainc.com
>>>> Subject: Re: [edk2-devel] [edk2][PATCH 1/1]
>>>> MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform
>>>> recovery
>>>>
>>>> Thanks Pete.
>>>>
>>>> I think the question I have, that I hope Tiano veterans can chime
>>>> in, is whether we are doing the right thing, or if we should be
>>>> overriding the boot mode? I.e. is it normal that we boot up in
>>>> recovery until options are saved?
>>>>
>>>>
>>>> A
>>>>
>>>>
>>>> ________________________________________
>>>> From: mailto:devel at edk2.groups.io <mailto:devel at edk2.groups.io> on
>>>> behalf of Pete Batard via groups.io <mailto:pete=akeo.ie at groups.io>
>>>> Sent: Wednesday, June 17, 2020 11:34 AM
>>>> To: Wang, Sunny (HPS SW) <mailto:sunnywang at hpe.com>;
>>>> mailto:devel at edk2.groups.io <mailto:devel at edk2.groups.io>
>>>> Cc: mailto:zhichao.gao at intel.com <mailto:zhichao.gao at intel.com>;
>>>> mailto:ray.ni at intel.com <mailto:ray.ni at intel.com>;
>>>> mailto:ard.biesheuvel at arm.com <mailto:ard.biesheuvel at arm.com>;
>>>> mailto:leif at nuviainc.com <mailto:leif at nuviainc.com>
>>>> Subject: Re: [edk2-devel] [edk2][PATCH 1/1]
>>>> MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform
>>>> recovery
>>>>
>>>> On 2020.06.17 14:04, Wang, Sunny (HPS SW) wrote:
>>>>> Thanks for checking my comments, Pete.
>>>>>
>>>>>> Or is the "one more" the issue, meaning that it would get signaled
>>>>>> more than once?
>>>>> [Sunny] Yeah, it would get signaled more than once if the
>>>>> PlatformRecovery option (a UEFI application) calls
>>>>> EfiBootManagerBoot() to launch the recovered boot option inside of
>>>>> the application.
>>>>
>>>> Okay.
>>>>
>>>> One element that I'm going to point out is that, with the current EDK2
>>>> code (i.e. without this proposal applied), and after a user goes into
>>>> the setup to save their boot options in order for regular boot options
>>>> to get executed instead of PlaformRecovery, the OnReadyToBoot event is
>>>> actually called twice.
>>>>
>>>> So my understanding is that, while we of course want to avoid this and
>>>> any patch proposal should actively try to prevent it, it seems we
>>>> already have behaviour in EDK2 that can lead to OnReadyToBoot being
>>>> signalled more than once.
>>>>
>>>> At least the current Pi 4 platform does demonstrate this behaviour. For
>>>> instance, if you run DEBUG, you will see two instances of:
>>>>
>>>> RemoveDtStdoutPath: could not retrieve DT blob - Not Found
>>>>
>>>> which is a one-instance message generated from the ConsolePrefDxe's
>>>> OnReadyToBoot() call. I've also confirmed more specifically that
>>>> OnReadyToBoot() is indeed called twice.
>>>>
>>>> I don't recall us doing much of any special with regards to boot
>>>> options
>>>> for the Pi platform, so my guess is that it's probably not the only
>>>> platform where OnReadyToBoot might be signalled more than once, and
>>>> that
>>>> this might be tied to a default EDK2 behaviour. Therefore I don't see
>>>> having a repeated event as a major deal breaker (though, again, if we
>>>> can avoid that, we of course will want to).
>>>>
>>>>>> 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?
>>>>> [Sunny] Sure. I added more information below. If it is still not
>>>>> clear enough, feel free to let me know.
>>>>> 1. Create a UEFI application with the code to signal
>>>>> ReadyToBoot and pick /efi/boot/bootaa64.efi from either SD or USB
>>>>> and run it.
>>>>
>>>> So that would basically be adding code that duplicates, in part, what
>>>> Platform Recovery already does.
>>>>
>>>> I have to be honest: Even outside of the extra work this would require,
>>>> I don't really like the idea of having to write our own application, as
>>>> it will introduce new possible points of failures and require extra
>>>> maintenance (especially as we will want to be able to handle network
>>>> boot and other options, and before long, I fear that we're going to
>>>> have
>>>> to write our own Pi specific boot manager). Doing so simply because the
>>>> current Platform Recovery, which does suit our needs otherwise, is not
>>>> designed to call ReadyToBoot does not seem like the best course of
>>>> action in my book.
>>>>
>>>> Instead, I still logically believe that any option that calls a boot
>>>> loader should signal ReadyToBoot, regardless of whether it was launched
>>>> from Boot Manager or Platform Recovery, and that it shouldn't be
>>>> left to
>>>> each platform to work around that.
>>>>
>>>> Of course, I understand that this would require a specs change, and
>>>> that
>>>> it also may have ramifications for existing platforms that interpret
>>>> the
>>>> current specs pedantically. But to me, regardless of what the specs
>>>> appear to be limiting it to right now, the logic of a "ReadyToBoot"
>>>> event is that it should be signalled whenever a bootloader is about to
>>>> be executed, rather than only when a bootloader happened to be launched
>>>> through a formal Boot Manager option.
>>>>
>>>> I would therefore appreciate if other people could weigh in on this
>>>> matter, to see if I'm the only one who believes that we could
>>>> ultimately
>>>> have more to gain from signalling ReadyToBoot with PlatformRecovery
>>>> options than leaving things as they stand right now...
>>>>
>>>>> 2. Then, call EfiBootManagerInitializeLoadOption like the
>>>>> following in a DXE driver or other places before "Default
>>>>> PlatformRecovery" registration:
>>>>> Status = EfiBootManagerInitializeLoadOption (
>>>>> &LoadOption,
>>>>>
>>>>> 0,
>>>>> -> 0 is the OptionNumber to let application be load before "
>>>>> Default PlatformRecovery" option
>>>>> LoadOptionTypePlatformRecovery,
>>>>> LOAD_OPTION_ACTIVE,
>>>>> L"Application for recovering boot options",
>>>>>
>>>>> FilePath,
>>>>> -> FilePath is the Application's device path,
>>>>> NULL,
>>>>> 0
>>>>> );
>>>>>
>>>>>
>>>>>> 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
>>>>>> signaling the ReadyToBoot event.
>>>>> [Sunny] Thanks for clarifying this, and Sorry about that I missed
>>>>> your cover letter for this patch. I was just thinking that we may
>>>>> not really need to make this behavior change in both EDK II code
>>>>> and UEFI specification for solving the problem specific to the case
>>>>> that OS is loaded by "Default PlatformRecovery" option,
>>>>
>>>> The way I see it is that the Pi platform is unlikely to be the only one
>>>> where PlatformRecovery is seen as a means to install an OS. Granted,
>>>> this may seem like abusing the option, but since UEFI doesn't
>>>> provide an
>>>> "Initial OS Install" mode, I would assert that it as good a use of this
>>>> option as any.
>>>>
>>>> In other words, I don't think this improvement would only benefit
>>>> the Pi
>>>> platform.
>>>>
>>>>> and I'm also not sure if it is worth making this change to affect
>>>>> some of the system or BIOS vendors who have implemented their
>>>>> PlatformRecovery option.
>>>>
>>>> That's a legitimate concern, and I would agree the one major potential
>>>> pitfall of this proposal, if there happens to exist a system where an
>>>> OnReadyToBoot even before running the recovery option can have adverse
>>>> effects.
>>>>
>>>> I don't really believe that such a system exists, because I expect most
>>>> recovery boot loaders to also work (or at least have been designed to
>>>> work) as regular boot options. But I don't have enough experience with
>>>> platform recovery to know if that's a correct assertion to make...
>>>>
>>>>> If the alternative approach I mentioned works for you, I think that
>>>>> would be an easier solution.
>>>>
>>>> Right now, even as the patch proposal has multiple issues that require
>>>> it to be amended (Don't signal ReadyToBoot except for PlatformRecovery
>>>> + Prevent situations where ReadyToBoot could be signalled multiple
>>>> times) I still see it as both an easier solution than the alternative,
>>>> as well as one that *should* benefit people who design Platform
>>>> Recovery
>>>> UEFI applications in the long run. So that is why I am still trying to
>>>> advocate for it.
>>>>
>>>> But I very much hear your concerns, and I agree that specs changes are
>>>> better avoided when possible.
>>>>
>>>> Thus, at this stage, even as I don't want to drag this discussion much
>>>> further, I don't feel like I want to commit to one solution or the
>>>> other
>>>> before we have had a chance to hear other people, who may have their
>>>> own
>>>> opinion on the matter, express their views.
>>>>
>>>> Regards,
>>>>
>>>> /Pete
>>>>
>>>>
>>>>>
>>>>> Regards,
>>>>> Sunny Wang
>>>>>
>>>>> -----Original Message-----
>>>>> From: Pete Batard [mailto:pete at akeo.ie]
>>>>> Sent: Wednesday, June 17, 2020 6:59 PM
>>>>> To: Wang, Sunny (HPS SW) <mailto:sunnywang at hpe.com>;
>>>>> mailto:devel at edk2.groups.io
>>>>> Cc: mailto:zhichao.gao at intel.com; mailto:ray.ni at intel.com;
>>>>> mailto:ard.biesheuvel at arm.com; mailto:leif at nuviainc.com
>>>>> Subject: Re: [edk2-devel] [edk2][PATCH 1/1]
>>>>> MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform
>>>>> recovery
>>>>>
>>>>> 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://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F61327&data=02%7C01%7Cawarkentin%40vmware.com%7C5f90d077bc7949c1122f08d812dc48d3%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637280084611749324&sdata=2%2B%2FcvMkrmZGTRRLDGSuMsKbiyDOGtwYwZ7qSqMyMicc%3D&reserved=0
>>>>> ), 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: mailto:devel at edk2.groups.io [mailto:devel at edk2.groups.io] On
>>>>>> Behalf Of
>>>>>> Pete Batard
>>>>>> Sent: Tuesday, June 16, 2020 5:56 PM
>>>>>> To: mailto:devel at edk2.groups.io
>>>>>> Cc: mailto:zhichao.gao at intel.com; mailto:ray.ni at intel.com;
>>>>>> mailto:ard.biesheuvel at arm.com;
>>>>>> mailto:leif at 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 <mailto:pete at 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
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>> 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.
>>>>
>>>> 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 (#71746): https://edk2.groups.io/g/devel/message/71746
Mute This Topic: https://groups.io/mt/80701195/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list