Hi Ray,
The places for launching PlatformRecovery options and boot menu option
(for Successful boot) in this code change look good to me. I think this patch
can solve problem I mentioned in our previous discussion for
PlatformBootManagerDefaultBootFail related patch. Thanks for making this code
change.
In addition, I made four review comments in this patch. You can search
"[Sunny]" to find my comments out.
Regards,
Sunny Wang
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Monday, November 02, 2015 7:34 PM
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni; Eric Dong
Subject: [edk2] [Patch 11/11] MdeModulePkg: Enable PlatformRecovery in BdsDxe
driver
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
Cc: Eric Dong <eric.d...@intel.com>
---
MdeModulePkg/Universal/BdsDxe/Bds.h | 3 -
MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 405 ++++++++-----------------------
2 files changed, 95 insertions(+), 313 deletions(-)
diff --git a/MdeModulePkg/Universal/BdsDxe/Bds.h
b/MdeModulePkg/Universal/BdsDxe/Bds.h
index 2171d14..ac54482 100644
--- a/MdeModulePkg/Universal/BdsDxe/Bds.h
+++ b/MdeModulePkg/Universal/BdsDxe/Bds.h
@@ -24,9 +24,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
#include <Protocol/Bds.h>
#include <Protocol/LoadedImage.h>
#include <Protocol/VariableLock.h>
-#include <Protocol/BlockIo.h>
-#include <Protocol/LoadFile.h>
-#include <Protocol/SimpleFileSystem.h>
#include <Library/UefiDriverEntryPoint.h> #include <Library/DebugLib.h> diff
--git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index b632ada..05cdb73 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -51,10 +51,10 @@ CHAR16 *mReadOnlyVariables[] = {
CHAR16 *mBdsLoadOptionName[] = {
L"Driver",
L"SysPrep",
- L"Boot"
+ L"Boot",
+ L"PlatformRecovery"
};
-CHAR16 mRecoveryBoot[] = L"Recovery Boot";
/**
Event to Connect ConIn.
@@ -122,187 +122,6 @@ BdsInitialize (
}
/**
- Emuerate all possible bootable medias in the following order:
- 1. Removable BlockIo - The boot option only points to the
removable media
- device, like USB key, DVD, Floppy etc.
- 2. Fixed BlockIo - The boot option only points to a Fixed
blockIo device,
- like HardDisk.
- 3. Non-BlockIo SimpleFileSystem - The boot option points to a device
supporting
- SimpleFileSystem Protocol, but not
supporting BlockIo
- protocol.
- 4. LoadFile - The boot option points to the media
supporting
- LoadFile protocol.
- Reference: UEFI Spec chapter 3.3 Boot Option Variables Default Boot Behavior
-
- @param BootOptionCount Return the boot option count which has been found.
-
- @retval Pointer to the boot option array.
-**/
-EFI_BOOT_MANAGER_LOAD_OPTION *
-BdsEnumerateBootOptions (
- UINTN *BootOptionCount
- )
-{
- EFI_STATUS Status;
- EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions;
- UINTN HandleCount;
- EFI_HANDLE *Handles;
- EFI_BLOCK_IO_PROTOCOL *BlkIo;
- UINTN Removable;
- UINTN Index;
-
- ASSERT (BootOptionCount != NULL);
-
- *BootOptionCount = 0;
- BootOptions = NULL;
-
- //
- // Parse removable block io followed by fixed block io
- //
- gBS->LocateHandleBuffer (
- ByProtocol,
- &gEfiBlockIoProtocolGuid,
- NULL,
- &HandleCount,
- &Handles
- );
-
- for (Removable = 0; Removable < 2; Removable++) {
- for (Index = 0; Index < HandleCount; Index++) {
- Status = gBS->HandleProtocol (
- Handles[Index],
- &gEfiBlockIoProtocolGuid,
- (VOID **) &BlkIo
- );
- if (EFI_ERROR (Status)) {
- continue;
- }
-
- //
- // Skip the logical partitions
- //
- if (BlkIo->Media->LogicalPartition) {
- continue;
- }
-
- //
- // Skip the fixed block io then the removable block io
- //
- if (BlkIo->Media->RemovableMedia == ((Removable == 0) ? FALSE : TRUE)) {
- continue;
- }
-
- BootOptions = ReallocatePool (
- sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) *
(*BootOptionCount),
- sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) *
(*BootOptionCount + 1),
- BootOptions
- );
- ASSERT (BootOptions != NULL);
-
- Status = EfiBootManagerInitializeLoadOption (
- &BootOptions[(*BootOptionCount)++],
- LoadOptionNumberUnassigned,
- LoadOptionTypeBoot,
- LOAD_OPTION_ACTIVE,
- mRecoveryBoot,
- DevicePathFromHandle (Handles[Index]),
- NULL,
- 0
- );
- ASSERT_EFI_ERROR (Status);
- }
- }
-
- if (HandleCount != 0) {
- FreePool (Handles);
- }
-
- //
- // Parse simple file system not based on block io
- //
- gBS->LocateHandleBuffer (
- ByProtocol,
- &gEfiSimpleFileSystemProtocolGuid,
- NULL,
- &HandleCount,
- &Handles
- );
- for (Index = 0; Index < HandleCount; Index++) {
- Status = gBS->HandleProtocol (
- Handles[Index],
- &gEfiBlockIoProtocolGuid,
- (VOID **) &BlkIo
- );
- if (!EFI_ERROR (Status)) {
- //
- // Skip if the file system handle supports a BlkIo protocol, which
we've handled in above
- //
- continue;
- }
- BootOptions = ReallocatePool (
- sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) * (*BootOptionCount),
- sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) * (*BootOptionCount
+ 1),
- BootOptions
- );
- ASSERT (BootOptions != NULL);
-
- Status = EfiBootManagerInitializeLoadOption (
- &BootOptions[(*BootOptionCount)++],
- LoadOptionNumberUnassigned,
- LoadOptionTypeBoot,
- LOAD_OPTION_ACTIVE,
- mRecoveryBoot,
- DevicePathFromHandle (Handles[Index]),
- NULL,
- 0
- );
- ASSERT_EFI_ERROR (Status);
- }
-
- if (HandleCount != 0) {
- FreePool (Handles);
- }
-
- //
- // Parse load file, assuming UEFI Network boot option
- //
- gBS->LocateHandleBuffer (
- ByProtocol,
- &gEfiLoadFileProtocolGuid,
- NULL,
- &HandleCount,
- &Handles
- );
- for (Index = 0; Index < HandleCount; Index++) {
-
- BootOptions = ReallocatePool (
- sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) * (*BootOptionCount),
- sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) * (*BootOptionCount
+ 1),
- BootOptions
- );
- ASSERT (BootOptions != NULL);
-
- Status = EfiBootManagerInitializeLoadOption (
- &BootOptions[(*BootOptionCount)++],
- LoadOptionNumberUnassigned,
- LoadOptionTypeBoot,
- LOAD_OPTION_ACTIVE,
- mRecoveryBoot,
- DevicePathFromHandle (Handles[Index]),
- NULL,
- 0
- );
- ASSERT_EFI_ERROR (Status);
- }
-
- if (HandleCount != 0) {
- FreePool (Handles);
- }
-
- return BootOptions;
-}
-
-/**
Function waits for a given event to fire, or for an optional timeout to
expire.
@param Event The event to wait for
@@ -445,14 +264,16 @@ BdsWait (
@param BootOptions Input boot option array.
@param BootOptionCount Input boot option count.
+ @param BootManagerMenu Input boot manager menu.
@retval TRUE Successfully boot one of the boot options.
@retval FALSE Failed boot any of the boot options.
**/
BOOLEAN
-BootAllBootOptions (
+BootBootOptions (
IN EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions,
- IN UINTN BootOptionCount
+ IN UINTN BootOptionCount,
+ IN EFI_BOOT_MANAGER_LOAD_OPTION *BootManagerMenu
)
{
UINTN Index;
@@ -489,6 +310,7 @@ BootAllBootOptions (
// Successful boot breaks the loop, otherwise tries next boot option
[Sunny] It would be good to update the comment as well because it doesn't match
your modification. You can cut part of UEFI 2.5 Section 3.1.1 Boot Manager
Programming, the 5th paragraph below to update the comment.
-If the boot via Boot#### returns with a status of EFI_SUCCESS,
platform firmware supports boot manager menu, and if firmware is configured to
boot in an interactive mode, the boot manager will stop processing the
BootOrder variable and present a boot manager menu to the user.
if (BootOptions[Index].Status == EFI_SUCCESS) {
+ EfiBootManagerBoot (BootManagerMenu);
break;
}
}
@@ -497,72 +319,10 @@ BootAllBootOptions ( }
/**
- This function attempts to boot per the boot order specified by platform
policy.
-
- If the boot via Boot#### returns with a status of EFI_SUCCESS the boot
manager will stop
- processing the BootOrder variable and present a boot manager menu to the
user. If a boot via
- Boot#### returns a status other than EFI_SUCCESS, the boot has failed and
the next Boot####
- in the BootOrder variable will be tried until all possibilities are
exhausted.
- -- Chapter 3.1.1 Boot Manager Programming,
the 4th paragraph
-**/
-VOID
-DefaultBootBehavior (
- VOID
- )
-{
- UINTN BootOptionCount;
- EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions;
- EFI_BOOT_MANAGER_LOAD_OPTION BootManagerMenu;
-
- EfiBootManagerGetBootManagerMenu (&BootManagerMenu);
- //
- // BootManagerMenu always contains the correct information even the above
function returns failure.
- //
-
- BootOptions = EfiBootManagerGetLoadOptions (&BootOptionCount,
LoadOptionTypeBoot);
-
- if (BootAllBootOptions (BootOptions, BootOptionCount)) {
- //
- // Follow generic rule, Call BdsDxeOnConnectConInCallBack to connect ConIn
before enter UI
- //
- if (PcdGetBool (PcdConInConnectOnDemand)) {
- BdsDxeOnConnectConInCallBack (NULL, NULL);
- }
-
- //
- // Show the Boot Manager Menu after successful boot
- //
- EfiBootManagerBoot (&BootManagerMenu);
- } else {
- EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
- //
- // Re-scan all EFI boot options in case all the boot#### are deleted or
failed to boot
- //
- // If no valid boot options exist, the boot manager will enumerate all
removable media
- // devices followed by all fixed media devices. The order within each
group is undefined.
- // These new default boot options are not saved to non volatile
storage.The boot manger
- // will then attempt toboot from each boot option.
- // -- Chapter 3.3 Boot Manager Programming, the
2nd paragraph
- //
- EfiBootManagerConnectAll ();
- BootOptions = BdsEnumerateBootOptions (&BootOptionCount);
-
- if (!BootAllBootOptions (BootOptions, BootOptionCount)) {
- DEBUG ((EFI_D_ERROR, "[Bds]No bootable device!\n"));
- EfiBootManagerBoot (&BootManagerMenu);
- }
- }
-
- EfiBootManagerFreeLoadOption (&BootManagerMenu);
- EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount); -}
-
-/**
- The function will load and start every Driver####/SysPrep####.
+ The function will load and start every Driver####, SysPrep#### or
PlatformRecovery####.
@param LoadOptions Load option array.
@param LoadOptionCount Load option count.
-
**/
VOID
ProcessLoadOptions (
@@ -575,7 +335,7 @@ ProcessLoadOptions (
BOOLEAN ReconnectAll;
EFI_BOOT_MANAGER_LOAD_OPTION_TYPE LoadOptionType;
- ReconnectAll = FALSE;
+ ReconnectAll = FALSE;
LoadOptionType = LoadOptionTypeMax;
[Sunny] Just a suggestion. Why don't we directly set
LoadOptions[Index].OptionType to LoadOptionType here instead of
LoadOptionTypeMax so that we don't need to check Index in the for-loop?
//
@@ -585,16 +345,23 @@ ProcessLoadOptions (
//
// All the load options in the array should be of the same type.
//
- if (LoadOptionType == LoadOptionTypeMax) {
+ if (Index == 0) {
LoadOptionType = LoadOptions[Index].OptionType;
}
ASSERT (LoadOptionType == LoadOptions[Index].OptionType);
- ASSERT (LoadOptionType == LoadOptionTypeDriver || LoadOptionType ==
LoadOptionTypeSysPrep);
Status = EfiBootManagerProcessLoadOption (&LoadOptions[Index]);
- if (!EFI_ERROR (Status) && ((LoadOptions[Index].Attributes &
LOAD_OPTION_FORCE_RECONNECT) != 0)) {
- ReconnectAll = TRUE;
+ if (!EFI_ERROR (Status)) {
+ if (LoadOptionType == LoadOptionTypePlatformRecovery) {
+ //
+ // Stop processing if any entry is successful
+ //
+ break;
+ }
+ if ((LoadOptions[Index].Attributes & LOAD_OPTION_FORCE_RECONNECT) != 0) {
+ ReconnectAll = TRUE;
+ }
}
}
@@ -670,7 +437,7 @@ BdsFormalizeOSIndicationVariable (
//
// OS indicater support variable
//
- OsIndicationSupport = EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
+ OsIndicationSupport = EFI_OS_INDICATIONS_BOOT_TO_FW_UI |
EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY;
Status = gRT->SetVariable (
EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME,
&gEfiGlobalVariableGuid,
@@ -828,6 +595,8 @@ BdsEntry (
CHAR16 BootNextVariableName[sizeof ("Boot####")];
EFI_BOOT_MANAGER_LOAD_OPTION BootManagerMenu;
BOOLEAN BootFwUi;
+ BOOLEAN PlatformRecovery;
+ BOOLEAN BootSuccess;
EFI_DEVICE_PATH_PROTOCOL *FilePath;
HotkeyTriggered = NULL;
[Sunny] Do we need to initialize BootSuccess to FALSE here? It looks like the
uninitialized BootSuccess variable would cause build failure.
@@ -1038,9 +807,24 @@ BdsEntry (
PERF_START (NULL, "PlatformBootManagerAfterConsole", "BDS", 0);
PlatformBootManagerAfterConsole ();
PERF_END (NULL, "PlatformBootManagerAfterConsole", "BDS", 0);
+ //
+ // Boot to Boot Manager Menu when EFI_OS_INDICATIONS_BOOT_TO_FW_UI is set.
Skip HotkeyBoot
+ //
+ DataSize = sizeof (UINT64);
+ Status = gRT->GetVariable (
+ EFI_OS_INDICATIONS_VARIABLE_NAME,
+ &gEfiGlobalVariableGuid,
+ NULL,
+ &DataSize,
+ &OsIndication
+ );
+ if (EFI_ERROR (Status)) {
+ OsIndication = 0;
+ }
DEBUG_CODE (
EFI_BOOT_MANAGER_LOAD_OPTION_TYPE LoadOptionType;
+ DEBUG ((EFI_D_INFO, "[Bds]OsIndication: %016x\n", OsIndication));
DEBUG ((EFI_D_INFO, "[Bds]=============Begin Load Options Dumping
...=============\n"));
for (LoadOptionType = 0; LoadOptionType < LoadOptionTypeMax;
LoadOptionType++) {
DEBUG ((
@@ -1063,26 +847,17 @@ BdsEntry (
);
//
- // Boot to Boot Manager Menu when EFI_OS_INDICATIONS_BOOT_TO_FW_UI is set.
Skip HotkeyBoot
+ // BootManagerMenu always contains the correct information even call fails.
//
- DataSize = sizeof (UINT64);
- Status = gRT->GetVariable (
- EFI_OS_INDICATIONS_VARIABLE_NAME,
- &gEfiGlobalVariableGuid,
- NULL,
- &DataSize,
- &OsIndication
- );
- if (EFI_ERROR (Status)) {
- OsIndication = 0;
- }
+ EfiBootManagerGetBootManagerMenu (&BootManagerMenu);
- BootFwUi = (BOOLEAN) ((OsIndication & EFI_OS_INDICATIONS_BOOT_TO_FW_UI) !=
0);
+ BootFwUi = (BOOLEAN) ((OsIndication &
EFI_OS_INDICATIONS_BOOT_TO_FW_UI) != 0);
+ PlatformRecovery = (BOOLEAN) ((OsIndication &
EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY) != 0);
//
// Clear EFI_OS_INDICATIONS_BOOT_TO_FW_UI to acknowledge OS
//
- if (BootFwUi) {
- OsIndication &= ~((UINT64) EFI_OS_INDICATIONS_BOOT_TO_FW_UI);
+ if (BootFwUi || PlatformRecovery) {
+ OsIndication &= ~((UINT64) (EFI_OS_INDICATIONS_BOOT_TO_FW_UI |
EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY));
Status = gRT->SetVariable (
EFI_OS_INDICATIONS_VARIABLE_NAME,
&gEfiGlobalVariableGuid,
@@ -1109,62 +884,72 @@ BdsEntry (
//
// Directly enter the setup page.
- // BootManagerMenu always contains the correct information even call fails.
//
- EfiBootManagerGetBootManagerMenu (&BootManagerMenu);
EfiBootManagerBoot (&BootManagerMenu);
- EfiBootManagerFreeLoadOption (&BootManagerMenu);
}
- //
- // Execute SysPrep####
- //
- LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount,
LoadOptionTypeSysPrep);
- ProcessLoadOptions (LoadOptions, LoadOptionCount);
- EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
+ Status = EFI_SUCCESS;
[Sunny] May I know why we need to set Status to EFI_SUCCESS here? If we need to
do this, could you add comment here? :)
- //
- // Execute Key####
- //
- PERF_START (NULL, "BdsWait", "BDS", 0);
- BdsWait (HotkeyTriggered);
- PERF_END (NULL, "BdsWait", "BDS", 0);
+ if (!PlatformRecovery) {
+ //
+ // Execute SysPrep####
+ //
+ LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount,
LoadOptionTypeSysPrep);
+ ProcessLoadOptions (LoadOptions, LoadOptionCount);
+ EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
- //
- // BdsReadKeys() be removed after all keyboard drivers invoke callback in
timer callback.
- //
- BdsReadKeys ();
+ //
+ // Execute Key####
+ //
+ PERF_START (NULL, "BdsWait", "BDS", 0);
+ BdsWait (HotkeyTriggered);
+ PERF_END (NULL, "BdsWait", "BDS", 0);
- EfiBootManagerHotkeyBoot ();
+ //
+ // BdsReadKeys() can be removed after all keyboard drivers invoke callback
in timer callback.
+ //
+ BdsReadKeys ();
- //
- // Boot to "BootNext"
- //
- if (BootNext != NULL) {
- UnicodeSPrint (BootNextVariableName, sizeof (BootNextVariableName),
L"Boot%04x", *BootNext);
- Status = EfiBootManagerVariableToLoadOption (BootNextVariableName,
&LoadOption);
- if (!EFI_ERROR (Status)) {
- EfiBootManagerBoot (&LoadOption);
- EfiBootManagerFreeLoadOption (&LoadOption);
- if (LoadOption.Status == EFI_SUCCESS) {
- //
- // Boot to Boot Manager Menu upon EFI_SUCCESS
- //
- EfiBootManagerGetBootManagerMenu (&LoadOption);
+ EfiBootManagerHotkeyBoot ();
+
+ //
+ // Boot to "BootNext"
+ //
+ if (BootNext != NULL) {
+ UnicodeSPrint (BootNextVariableName, sizeof (BootNextVariableName),
L"Boot%04x", *BootNext);
+ Status = EfiBootManagerVariableToLoadOption (BootNextVariableName,
&LoadOption);
+ if (!EFI_ERROR (Status)) {
EfiBootManagerBoot (&LoadOption);
EfiBootManagerFreeLoadOption (&LoadOption);
+ if (LoadOption.Status == EFI_SUCCESS) {
+ //
+ // Boot to Boot Manager Menu upon EFI_SUCCESS
+ //
+ EfiBootManagerBoot (&BootManagerMenu);
+ }
}
}
+
+ do {
+ //
+ // Retry to boot if any of the boot succeeds
+ //
+ LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount,
LoadOptionTypeBoot);
+ BootSuccess = BootBootOptions (LoadOptions, LoadOptionCount,
&BootManagerMenu);
+ EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
+ } while (BootSuccess);
}
- while (TRUE) {
- //
- // BDS select the boot device to load OS
- // Try next upon boot failure
- // Show Boot Manager Menu upon boot success
- //
- DefaultBootBehavior ();
+ EfiBootManagerFreeLoadOption (&BootManagerMenu);
+
+ if (!BootSuccess || PlatformRecovery) {
+ LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount,
LoadOptionTypePlatformRecovery);
+ ProcessLoadOptions (LoadOptions, LoadOptionCount);
+ EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
}
+
+ DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n"));
+ CpuDeadLoop ();
}
/**
--
1.9.5.msysgit.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
