One comment below: > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Gao, Zhichao > Sent: Tuesday, June 04, 2019 9:05 AM > To: devel@edk2.groups.io > Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Zeng, Star; Gao, Liming; Sean Brogan; > Michael Turner; Bret Barkelew > Subject: [edk2-devel] [PATCH V3 2/2] MdeModulePkg/BdsDxe: Use a pcd to > control PlatformRecovery > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1678 > > Use the PcdPlatformRecoverySupport to control the function > of platform recovery in BDS. > First, set the variable's ("OsIndicationsSupported") > EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY bit base on the pcd. > It would affect the variable "OsIndications". > While the platform does not support the platform recovery, > it is inappropriate to set a PlatformRecovery#### variable. So > skip setting the variable. But it should remain the behavior of > booting from a default file path (such as \EFI\BOOT\BOOTX64.EFI) > to be compatible with the previous version UEFI spec. > > Add memory check before build platform default boot option. If > fail to allocate memory for the defualt boot file path, put the > system into dead loop to indicate it is unable to boot. > > Cc: Jian J Wang <jian.j.w...@intel.com> > Cc: Hao Wu <hao.a...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Star Zeng <star.z...@intel.com> > Cc: Liming Gao <liming....@intel.com> > Cc: Sean Brogan <sean.bro...@microsoft.com> > Cc: Michael Turner <michael.tur...@microsoft.com> > Cc: Bret Barkelew <bret.barke...@microsoft.com> > Signed-off-by: Zhichao Gao <zhichao....@intel.com> > --- > MdeModulePkg/Universal/BdsDxe/BdsDxe.inf | 3 +- > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 71 +++++++++++++++------ > --- > 2 files changed, 47 insertions(+), 27 deletions(-) > > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf > b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf > index 6913389d34..7f94ca17df 100644 > --- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf > +++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf > @@ -5,7 +5,7 @@ > # gEfiBdsArchProtocolGuid. After DxeCore finish dispatching, DxeCore will > invoke Entry > # interface of protocol gEfiBdsArchProtocolGuid, then BDS phase is entered. > # > -# Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2008 - 2019, Intel Corporation. All rights reserved.<BR> > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > ## > @@ -95,6 +95,7 @@ > gEfiMdeModulePkgTokenSpaceGuid.PcdConInConnectOnDemand > ## CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdErrorCodeSetVariable ## > SOMETIMES_CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed ## > CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdPlatformRecoverySupport > ## CONSUMES > > [Depex] > TRUE > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > index 9d312bd982..8c8a0b5236 100644 > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > @@ -5,7 +5,7 @@ > After DxeCore finish DXE phase, gEfiBdsArchProtocolGuid->BdsEntry will be > invoked > to enter BDS phase. > > -Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2004 - 2019, Intel Corporation. All rights reserved.<BR> > (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR> > (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > @@ -546,10 +546,14 @@ BdsFormalizeOSIndicationVariable ( > // > Status = EfiBootManagerGetBootManagerMenu (&BootManagerMenu); > if (Status != EFI_NOT_FOUND) { > - OsIndicationSupport = EFI_OS_INDICATIONS_BOOT_TO_FW_UI | > EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY; > + OsIndicationSupport = EFI_OS_INDICATIONS_BOOT_TO_FW_UI; > EfiBootManagerFreeLoadOption (&BootManagerMenu); > } else { > - OsIndicationSupport = > EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY; > + OsIndicationSupport = 0; > + } > + > + if (PcdGetBool (PcdPlatformRecoverySupport)) { > + OsIndicationSupport |= > EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY; > } > > Status = gRT->SetVariable ( > @@ -662,6 +666,7 @@ BdsEntry ( > BOOLEAN BootSuccess; > EFI_DEVICE_PATH_PROTOCOL *FilePath; > EFI_STATUS BootManagerMenuStatus; > + EFI_BOOT_MANAGER_LOAD_OPTION PlatformDefaultBootOption; > > HotkeyTriggered = NULL; > Status = EFI_SUCCESS; > @@ -763,14 +768,13 @@ BdsEntry ( > // > InitializeLanguage (TRUE); > > - // > - // System firmware must include a PlatformRecovery#### variable > specifying > - // a short-form File Path Media Device Path containing the platform default > - // file path for removable media > - // > FilePath = FileDevicePath (NULL, EFI_REMOVABLE_MEDIA_FILE_NAME); > + if (FilePath == NULL) { > + DEBUG ((DEBUG_ERROR, "Fail to allocate memory for defualt boot file > path. Unable to boot.\n")); > + CpuDeadLoop (); > + } > Status = EfiBootManagerInitializeLoadOption ( > - &LoadOption, > + &PlatformDefaultBootOption, > LoadOptionNumberUnassigned, > LoadOptionTypePlatformRecovery, > LOAD_OPTION_ACTIVE, > @@ -780,24 +784,31 @@ BdsEntry ( > 0 > ); > ASSERT_EFI_ERROR (Status); > - LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, > LoadOptionTypePlatformRecovery); > - if (EfiBootManagerFindLoadOption (&LoadOption, LoadOptions, > LoadOptionCount) == -1) { > - for (Index = 0; Index < LoadOptionCount; Index++) { > - // > - // The PlatformRecovery#### options are sorted by OptionNumber. > - // Find the the smallest unused number as the new OptionNumber. > - // > - if (LoadOptions[Index].OptionNumber != Index) { > - break; > + > + // > + // System firmware must include a PlatformRecovery#### variable > specifying > + // a short-form File Path Media Device Path containing the platform > default > + // file path for removable media if the platform supports Platform > Recovery. > + // > + if (PcdGetBool (PcdPlatformRecoverySupport)) { > + LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, > LoadOptionTypePlatformRecovery); > + if (EfiBootManagerFindLoadOption (&LoadOption, LoadOptions, > LoadOptionCount) == -1) {
In the above 'if' statement, 'LoadOption' is not being initialized. Is this a typo here? Best Regards, Hao Wu > + for (Index = 0; Index < LoadOptionCount; Index++) { > + // > + // The PlatformRecovery#### options are sorted by OptionNumber. > + // Find the the smallest unused number as the new OptionNumber. > + // > + if (LoadOptions[Index].OptionNumber != Index) { > + break; > + } > } > + PlatformDefaultBootOption.OptionNumber = Index; > + Status = EfiBootManagerLoadOptionToVariable > (&PlatformDefaultBootOption); > + ASSERT_EFI_ERROR (Status); > } > - LoadOption.OptionNumber = Index; > - Status = EfiBootManagerLoadOptionToVariable (&LoadOption); > - ASSERT_EFI_ERROR (Status); > + EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); > } > - EfiBootManagerFreeLoadOption (&LoadOption); > FreePool (FilePath); > - EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); > > // > // Report Status Code to indicate connecting drivers will happen > @@ -1043,10 +1054,18 @@ BdsEntry ( > } > > if (!BootSuccess) { > - LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, > LoadOptionTypePlatformRecovery); > - ProcessLoadOptions (LoadOptions, LoadOptionCount); > - EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); > + if (PlatformRecovery) { > + LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, > LoadOptionTypePlatformRecovery); > + ProcessLoadOptions (LoadOptions, LoadOptionCount); > + EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); > + } else { > + // > + // When platform recovery is not enabled, still boot to platform > default > file path. > + // > + EfiBootManagerProcessLoadOption (&PlatformDefaultBootOption); > + } > } > + EfiBootManagerFreeLoadOption (&PlatformDefaultBootOption); > > DEBUG ((EFI_D_ERROR, "[Bds] Unable to boot!\n")); > PlatformBootManagerUnableToBoot (); > -- > 2.21.0.windows.1 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42333): https://edk2.groups.io/g/devel/message/42333 Mute This Topic: https://groups.io/mt/31917434/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-