Zhichao: please evaluate this issue.

Any chance for an update on this?

It's been close to a month since we reported this bug and this specs compliance issue is hindering the ability of Raspberry Pi users to install Linux distributions, such as Debian, on account that improper evaluation of EFI_EVENT_GROUP_READY_TO_BOOT makes the console default to serial instead of graphical, unless the user goes to their firmware options first.

Thank you.

Zhichao: can you give the response?

I didn't add the issue in my list and miss it for a month. Sorry about that.

Firstly, I am not a native english user. I don't totally understand the meaning of "evaluate Boot####". My understand is that evaluate the Boot#### if it is bootable or not. If so, the code does not compliance the spec. The check is after the ready_to_boot event signal.

The signal function put into the BootManager function is because of the description in UEFI spec 2.8 Section 7.1 BootService.CreateEventEx:
'''
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 for the console device change, it is usually done at the PlatformBootManagerBeforeConsole fucntion by the platform code.

(In reply to Zhichao Gao from comment #4)
> I didn't add the issue in my list and miss it for a month. Sorry about that.
> 
> Firstly, I am not a native english user. I don't totally understand the
> meaning of "evaluate Boot####". My understand is that evaluate the Boot####
> if it is bootable or not. If so, the code does not compliance the spec. The
> check is after the ready_to_boot event signal.
> 
> The signal function put into the BootManager function is because of the
> description in UEFI spec 2.8 Section 7.1 BootService.CreateEventEx:
> '''
> 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 for the console device change, it is usually done at the
> PlatformBootManagerBeforeConsole fucntion by the platform code.

Fix the typo "the code is compliant with the spec".

I'm afraid I'm going to have to dispute the interpretation that the current code actually complies with the specs on account that:

1. When specs indicate that "After X occurs, Y should occur and do Z", the expectation is that X will occur fully before any part of Y is started, then Y will occur fully before any part of Z is started, then Z will occur fully". In this case, the immediate interpretation of X, Y and Z for someone reading the specs would be:

X = Process all SysPrep#### variables
Y = Send an EFI_EVENT_GROUP_READY_TO_BOOT event group to every option that can receive it and wait for it to complete
Z = Process ("evaluate") Boot#### variables.

On the other hand, your interpretation would force Y to start before Z, since the code at https://github.com/tianocore/edk2/blob/a56af23f066e2816c67b7c6e64de7ddefcd70780/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c#L1792-L1845 shows that we are already in the process of "evaluating" Boot#### variables.

2. I would assert that most people will that look at the specs will read "evaluate" as a synonymous of "process" that was only used there as a means to avoid generic repetition as well as a way to provides some hints with regards to what this process mostly does. Especially, "evaluate" is a very generic term, that I don't think suits itself to a narrow interpretation like the one you are trying to apply to it. If a narrow interpretation was intended, "evaluate" would not be used, or it would be used with a qualifier that narrows it, which isn't the case.

3. The fact that the word "begin" is used before "to evaluate" infers that the "evaluation" is a moderately complex process, that should only "begin" after another moderately complex process has completed, further hinting that we are in fact talking about the whole process of handling the Boot#### variables, and not a sub-section of that process.

At any rate, we really need to consider that it makes no logical sense for Platform Recovery options not to be signalled with EFI_EVENT_GROUP_READY_TO_BOOT, unless we go great length to twist the specs in a manner that narrows them down to signalling that event only after the processing ("evaluation") of Boot#### has, in effect, already begun, which I don't think is in anybody's interest, and especially not the developers who are sure to miss this potential very narrow interpretation.

In other words, I don't see any way for anybody who reads these sections to infer from them that only Boot#### should receive a EFI_EVENT_GROUP_READY_TO_BOOT, and that Platform Recovery shouldn't, because the only way to figure that out, right now, is to actually look at the code... 

So let me try to invoke some logic and common sense here:

1. Unless the specs state otherwise, an event called EFI_EVENT_GROUP_READY_TO_BOOT should naturally be signalled to all options that are able to boot the platform, because it will of course be used by platform developers to perform important initialization that they want to see happen right before boot. This means that, not only it should apply to Boot#### options but it should also apply to Platform Recovery options, as all of these options will boot the platform. I really don't see any scenario where a developer, that has a Platform Recovery entry, will want EFI_EVENT_GROUP_READY_TO_BOOT to be signalled to Boot#### options, and not also Platform Recovery.

2. This *expected* interpretation of the specs' intention is further corroborated by the description of EFI_EVENT_GROUP_READY_TO_BOOT, which you quoted, that states "This event group is notified by the system when the Boot Manager is about to load and execute a boot option". The fact that it states "boot option" in lowercase, and not "Boot#### option", indicates that the event is meant to be generic and not limited to the "Boot####" entries.

3. If we believe the specs are subject to interpretation, then we should go with the element of least surprise, and I have to strongly assert here that *NOT* sending EFI_EVENT_GROUP_READY_TO_BOOT to a Platform Recovery boot option, on account that we can squeeze the specs into a meaning that goes contrary to what developers who read them would expect, goes contrary to the element of least surprise.

In other words, if we really want the specs to be interpreted to mean that "Only Boot#### shall receive the EFI_EVENT_GROUP_READY_TO_BOOT event and Platform Recovery shall not", then we need to alter them to make that clear as well as provide a justification for why that should be the case (that goes further than "this is being done so that we don't have to modify our current code").

If, on the other hand, we consider that the specs don't need to be altered when it comes to whether Platform Recovery should receive the event, then I don't think we have any choice but to alter our code, so that Platform Recovery is signalled, because, when the chapter that deals with when and how EFI_EVENT_GROUP_READY_TO_BOOT should be signalled says "the platform shall notify EFI_EVENT_GROUP_READY_TO_BOOT event group and begin to evaluate Boot#### variables", and it makes no mention of excluding Platform Recovery, then the logical expectation is that all options that can be signalled with EFI_EVENT_GROUP_READY_TO_BOOT will be signalled before the rest of the processing happens with regards to the Boot#### variables, and therefore that Platform Recovery will be signalled.

So, can we please go with a bit of logic and common sense here, and modify our code so that, as opposed to what is the case now, Platform Recovery entries do receive the EFI_EVENT_GROUP_READY_TO_BOOT event?

I think it might also be worth reframing the context to help people understand the problem better, so that we can come with a resolution. So let me try to do that as well.

The origin of this bug report is that we should expect some platforms to use the Platform Recovery to launch a boot loader located on some media. After all, it makes sense that, if formal Boot#### options have not been defined or cannot be used, and Platform Recovery is triggered, one could see it as the Platform Recovery's job to launch an OS installation boot loader located on removable media for instance.

In that sense, one would therefore expect that the initialization that occurs before a formal Boot#### option that points to a /efi/boot/boot###.efi is launched also occurs when a /efi/boot/boot###.efi is launched as part of Platform Recovery.

However, if part of that initialization is performed through the EFI_EVENT_GROUP_READY_TO_BOOT event, then, when launching /efi/boot/boot###.efi through Platform Recovery, this initialization will not happen because EFI_EVENT_GROUP_READY_TO_BOOT is only currently signalled for Boot#### options.

In the case of the Raspberry Pi platform this is especially problematic as the setting up of the serial or graphical console occurs in the event handler for EFI_EVENT_GROUP_READY_TO_BOOT, with the result that some OS installers, such as Debian's, are not able to use the graphical console by default. In other words, on account of EFI_EVENT_GROUP_READY_TO_BOOT not being signalled for Platform Recovery, users are left unable to perform platform recovery!

But even outside of the Raspberry Pi platform specifics, we do see the launching of a /efi/boot/boot###.efi from Platform Recovery as something a lot of platform developers might be tempted to do, along with the expectation that EFI_EVENT_GROUP_READY_TO_BOOT would be signalled before the booting of their Platform Recovery option.

So I think there are 3 ways we can go with regards to this issue:

1. Agree that EFI_EVENT_GROUP_READY_TO_BOOT should be signalled for both Boot#### and Platform Recovery options, in which case our code should be modified in the manner requested above.

2. If not, and we believe that EFI_EVENT_GROUP_READY_TO_BOOT should only apply to formal Boot#### options, agree that a new EFI_EVENT_GROUP_READY_TO_RECOVER_PLATFORM event should be introduced to perform the pre-boot initialization that a Platform Recovery option might require.

3. If not, formally clarify why we don't think Platform Recovery qualifies to any initialization that other Boot#### options currently have, and what makes Platform Recovery so "special" that it doesn't qualify for any pre-boot custom initialization.

So, even if a very narrow interpretation of the specs (that I personally don't see) could technically let us get away with 3, it still leaves us very short of providing any logic as to why this is a desirable thing, apart from "It allows us keep the current code as is". If keeping the code as it stands is what we plan on doing, then platform developers who may want to use Platform Recovery to launch an OS installation boot loader, will still require some formal justification on why we logically think that Platform Recovery should not qualify to being signalled the EFI_EVENT_GROUP_READY_TO_BOOT event at all...

 SysPrep#### and EFI_EVENT_GROUP_READY_TO_BOOT behavior is defined by the UEFI Spec:

3.1.7 Required System Preparation Applications
A load option of the form SysPrep#### is intended to designate a UEFI application that is required to execute in order to complete system preparation prior to processing of any Boot#### variables. The execution order of SysPrep#### applications is determined by the contents of the variable SysPrepOrder in a way directly analogous to the ordering of Boot#### options by BootOrder.

The platform is required to examine all SysPrep#### variables referenced in SysPrepOrder. If Attributes bit LOAD_OPTION_ACTIVE is set, and the application referenced by FilePathList[0] is present, the UEFI Applications thus identified must be loaded and launched in the order they appear in SysPrepOrder and prior to the launch of any load options of type Boot####.

When launched, the platform is required to provide the application loaded by SysPrep####, with the same services such as console and network as are normally provided at launch to applications referenced by a Boot#### variable. SysPrep#### application must exit and may not call ExitBootServices(). Processing of any Error Code returned at exit is according to system policy and does not necessarily change processing of following boot options. Any driver portion of the feature supported by SysPrep#### boot option that is required to remain resident should be loaded by use of Driver#### variable.

The Attributes option LOAD_OPTION_FORCE_RECONNECT is ignored for SysPrep#### variables, and in the event that an application so launched performs some action that adds to the available hardware or drivers, the system preparation application shall itself utilize appropriate calls to ConnectController() or DisconnectController() to revise connections between drivers and hardware.

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.

Andrew,

We're talking about Platform Recovery and EFI_EVENT_GROUP_READY_TO_BOOT, not SysPrep#### and EFI_EVENT_GROUP_READY_TO_BOOT.

Unless I'm very mistaken, Platform Recovery != SysPrep####, and SysPrep#### does apply before Platform Recovery in the same way as it applies before executing Boot####, so this is not our issue here. In one sense, one could consider Platform Recovery as a "special" Boot#### option, that has nothing to do with SysPrep#### (like Boot####, it only occurs after SysPrep#### processing is completed) and that is geared specifically towards platform recovery.

The issue is that, after SysPrep#### has been processed, we have both Boot#### and Platform Recovery to process (if any of those exist), and the specs don't appear to formally indicate that EFI_EVENT_GROUP_READY_TO_BOOT should not be signalled before Platform Recovery is launched, in the same manner as it is signalled when a Boot#### is launched.

And that is the core of the issue, because we believe that, in the absence of anything to the contrary, after all the SysPrep#### options have been processed, and if Platform Recovery is launched instead of Boot####, then EFI_EVENT_GROUP_READY_TO_BOOT should still be signalled before it launches. And that is not the case with the current code where EFI_EVENT_GROUP_READY_TO_BOOT is only signalled for a Boot#### option and not a Platform Recovery option. And because the specs you quoted don't mention Platform Recovery specifically, one cannot definitely state what their intent is with EFI_EVENT_GROUP_READY_TO_BOOT and Platform Recovery (though I'm trying to argue that, logically, one would expect the specs to treat Platform Recovery in the same manner as it treats Boot#### in that respect).

At least that' my current understanding of SysPrep####, Boot####, Platform Recovery and EFI_EVENT_GROUP_READY_TO_BOOT.

Form 3.4.3:

If system firmware supports boot option recovery as described in Section 3.4, system firmware must include a PlatformRecovery#### variable specifying a short-form File Path Media Device Path (see Section 3.1.2) containing the platform default file path for removable media (see Table 15). It is recommended for maximal compatibility with prior versions of this specification that this entry be the first such variable, though it may be at any position within the list.

It is expected that this default boot will load an operating system or a maintenance utility. If this is an operating system setup program it is then responsible for setting the requisite environment variables for subsequent boots. The platform firmware may also decide to recover or set to a known set of boot
options.

Given the current spec language I think the practical implication is SysPrep##### is not by its self a recovery path, but a fixup to make Boot#### function. So it is an App that gets launched. 

The spec states the OsRecovery#### and PlatformRecovery##### are processed like Boot####. 

LoadOptionTypeDriver[1], and LoadOptionTypeSysPrep happen prior to EFI_EVENT_GROUP_READY_TO_BOOT being signaled. The difference with LoadOptionTypeSysPrep is the consoles have been connected at that point.

I noticed LoadOptionTypeBoot is processed via BootBootOptions() while LoadOptionTypeDriver, LoadOptionTypeSysPrep, LoadOptionTypePlatformRecovery, are processed via ProcessLoadOptions(). So it looks to me like the bug is not processing LoadOptionTypePlatformRecovery via BootBootOptions(). There are things like setting L"BootCurrent" that will be missed by calling ProcessLoadOptions() so I think we have more than one spec bug due to this flow. 

I agree with Pete that we need to fix the BDS flow. The platform I work on day to day has a custom BDS, so I'm not totally up on the current BDS so I may be missing something, but it seems to me anything that could boot should be going down the BootBootOptions() path. 

[1]
typedef enum {
  LoadOptionTypeDriver,
  LoadOptionTypeSysPrep,
  LoadOptionTypeBoot,
  LoadOptionTypePlatformRecovery,
  LoadOptionTypeMax
} EFI_BOOT_MANAGER_LOAD_OPTION_TYPE;

Pete sorry I initially got confused over the issue. I looked into the BDS code and I agree with your statement that it violates the spec. I actually found other violations and suggested an alternate fix.

Thanks Andrew for the feedback. In addition to being a UEFI spec implementation bug, this is a show-stopped for Debian support on the RPi4 (as seen here https://github.com/pftf/RPi4/issues/76).

Also, can we mark this as required for edk2-stable202008 ?

===Background===
I am sorry that the implementation was initially done by me before the SysPrep#### is added to UEFI spec. If you check UEFI spec 2.0, the only wording it contains is "This event group is notified by the system when the Boot Manager is about to load and execute a boot option." I guess that was the reason I chose to signal ReadyToBoot for each boot option.

Later someone (maybe Microsoft) added SysPrep####, later Peter Johns (from Redhat) added OsRecovery and PlatformRecovery and did a document refactoring.
The spec was changed a lot due to these new features. The conflict wording was added then. I should have payed more attention to the spec content when implementing the code. I admit that I did have limitations at that moment.
1. I set my goal to understand the spec and then implement new features.
2. I knew the BDS implementation very well so when reading the spec I just understand it based on my knowledge.


===Opinion===
I am aware of some platform implementation that relies on ReadyToBoot event to get the current boot option through L"BootCurrent". So, changing the BDS code would break all of those platforms.

Let's not discuss about how spec should be interpreted and discuss about:
1. How to solve the problem you are facing (without breaking existing platforms).
2. How to change spec wording to better reflect the code behavior.

I know that you may argue spec should drive the code but not vice versa. But if you look at what EDKII have done in the past, it is a common practice to clarify the spec instead of blindly changing the code without considering impacts when implementation was there for quite a long time creating lots of dependencies.

===What's platform recovery===
Because platform recovery is provided by the platform vendors, I don't see a need to signal a certain event (like readytoboot) to platform. As soon as the platform recovery option gets executed, platform vendor takes the control.
Please don't consider that BootIa32/X64.efi as a usually platform recovery option. The platform recovery option is usually provided by platform vendor to help to recovery the platform. Thinking about Dell is selling a laptop, the platform recovery option gives Dell the opportunity to reduce the customer support effort by automatically launching platform recovery activities when system cannot boot.

I really can't see myself going with the suggested approach.

We've already wasted tons of time on this, and I consider that the logic of trying to justify that Platform Recovery, that is clearly designed to execute a boot###.efi bootloader off disk media, should not be signalled with the same event as a regular boot###.efi bootloader that executes off disk media, is deeply flawed.

I'm afraid I have a completely different approach from you on fixing flawed behaviour when it comes the pre-existence of software that may rely on said flawed behaviour, especially considering that we're are dealing with Open Source software, which is that either that downstream software can and should be fixed, or that it is clearly in a state of not being maintained any longer, with all that implies in terms of addressing security risks and reported user issues, and therefore should no longer be used at all.

In other words, I see 2 options with regards to the platform(s) you are talking about (but unfortunately fail to name):

1. They are no longer being maintained, in which case updating EDK2 doesn't matter, since whoever was maintaining them is not going to produce new builds based on the updated EDK2/updated specs. In that case, there's really no issue updating with the earlier proposed approach.

2. They are being maintained, in which case, along with addressing security flaws and reported user issues, I consider it part of the established mandate of the maintainers to handle changes like the one we propose, because that is exactly what maintaining software means. In fact, I am very concerned about any software that doesn't happen to be nimble enough to take such changes in its stride, because, to me, it means that that software is close to falling into the "unmaintained" category.

Be it for security matters or otherwise, I have never seen the use of "I know you should logically fix this, but we have downstream application that are designed around the old flawed behaviour, so could you please keep it that way?" ever be beneficial for end users, which what we should first and foremost care about. Instead, I've almost always seen it used as a means to justify not having to maintain software that, for the sake of its users, should either be maintained or weeded out.

Especially as a proponent of Open Source software that is eager to see *unmaintained* closed source software being exposed for what it is (a liability, since software that is unlikely/unwilling to be updated for non-security related changes or evolving practices is also unlikely to be updated for those) and phased out, I can't see myself agreeing with having to sink more time adding workarounds we shouldn't have to add into Open Source software that is itself actively being maintained, on account that someone, somewhere, appears to be unwilling to allocate the resources they should rightfully allocate to maintain theirs.

Here, I have to apologize for seeming unwilling to negotiate but I a genuinely see no long term benefits for end users, for going with what you suggest. Instead, I only see short term ones for people who, in my view, should know better than be unwilling to maintain their software.

As such, in part on account of the time I've already had to sink in in this and from seeing that the logical resolution is once again being snatched away, I think I'm just going to step away from this specific matter and let others handle it as they see fit. But mostly it's because I can't see myself being coerced into maintaining an illogical status-quo that I don't see benefiting anyone, including the users of your unnamed platforms...

/Pete

Ray or Zhichao, any update on this issue? 

We cannot expect the implementation to not match the spec, and just keep the status quo simply because "there are existing products using this".

Samer, such behavior was there since EDK (I checked the edk source code I saved in 2008). I don't think changing it makes sense.

Hi Ray,

You do understand that this is like saying "Well, this bug has been present in the source code for more than 10 years so, since it's that old, I don't think we should fix it", which doesn't really strike me as a sane approach to software development.

At this stage, we've had multiple EDK2 contributors agree that the current code behaviour **SHOULD** be fixed because it doesn't match what we consider to be the logical/expected approach as to when and how the event should be signalled. And, if you re-read this bug thread, you will see that these were justified by more than a terse "I don't think it makes sense"...

Thus, whereas we understand that altering the current code **may** have ramifications for people who rely on the current improper behaviour (though, as I explained earlier, I will assert that platforms that do are unlikely to be updated to latest EDK2 hence unlikely to ever end up broken if we change the existing behaviour) and will likely require some time and effort to fix, that you'd rather use elsewhere, I don't think we can continue with the current "Let's just pretend there's nothing wrong so that we don't have to do" approach.

The current behaviour of not signalling EFI_EVENT_GROUP_READY_TO_BOOT for Platform Recovery is illogical and also runs contrary to what someone who reads the specs will expect.
Therefore the current EDK2 code is wrong and should be fixed, regardless of how long ago this improper/unexepected behaviour was introduced.

So can we please stop stalling on this issue and start to address it in earnest?

What's going on with this?

Can we settle on either EFI_EVENT_GROUP_READY_TO_RECOVER_PLATFORM or EFI_EVENT_GROUP_READY_TO_BOOT approaches?

Can someone comment on why the original patch (https://edk2.groups.io/g/devel/message/61328) of signaling ReadyToBoot in the recovery boot path (through EfiBootManagerProcessLoadOption() ) is not a reasonable request?

In order to make progress on this, I opened a code-first ECR  against the UEFI spec to clarify the language around the Boot#### options processing.

Code First ECR: https://bugzilla.tianocore.org/show_bug.cgi?id=3336

Feedback on the proposed language is appreciated. Please provide the feedback directly in the BZ above.

I will update the thread/BZ with results from USWG.

The UEFI Forum approved the ECR submitted in https://bugzilla.tianocore.org/show_bug.cgi?id=3336 as an errata / clarification for the UEFI specification

Implementations may proceed with the approved spec clarification change.

This means we can now update the TianoCore EDK2 / EDK2-Platforms code to signal EFI_EVENT_GROUP_READY_TO_BOOT (and EFI_EVENT_GROUP_AFTER_READY_TO_BOOT when it gets implemented) on platform recovery path, similar to what is done on normal boot path.

This is consistent with the original patch Pete submitted: https://edk2.groups.io/g/devel/topic/74912987#61328

Pete: after new UEFI is published, your patch can be merged.

Gao,

Why can't implementations proceed with the USWG approved UEFI ECR text (as documented in https://bugzilla.tianocore.org/show_bug.cgi?id=3336) now? Why do we need a UEFI spec published before the patch (which is based on an approved ECR) can be accepted?

Can we land the patch now? It's been over a year now... I see that a new UEFI version was published in August 2022, does that include the expected erratum?

I don’t see why not. The spec is published so the patch can proceed. Someone will need to resubmit it though, and reference this BZ and the UEFI spec change.

I can do that.

Done: https://edk2.groups.io/g/devel/message/110104

[PATCH v2] MdeModulePkg/UefiBootManagerLib: Signal ReadyToBoot on platform recovery
https://edk2.groups.io/g/devel/message/110438
mid: <20231031173700.647004-1-ngompa@fedoraproject.org>

"in progress", due to patches being on the list