The PlatformRecovery#### can be set by PlatformBootManagerLib. We cannot execute these if the feature is not supported.
Wait a sec, can I get clarification on the PCD's meaning a bit more: Why do we need such a configuration flag? Turning it off will break the functionality to boot default boot option like EFI\BOOT\BOOTIA32.EFI EFI\BOOT\BOOTx64.EFI ... Is that expected? If it's not, we still need to support booting from these boot options even the PCD is OFF. Thanks, Ray > -----Original Message----- > From: Gao, Zhichao > Sent: Wednesday, April 10, 2019 8:45 AM > To: Ni, Ray <[email protected]>; '[email protected]' <[email protected]> > Cc: Bret Barkelew <[email protected]>; Wang, Jian J > <[email protected]>; Zeng, Star <[email protected]>; Gao, Liming > <[email protected]>; Sean Brogan <[email protected]>; Michael > Turner <[email protected]> > Subject: RE: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS > indications bit > > This section can work fine even through the L"PlatformRecovery####" is not > set. > But if we do not support it, it should be better not to run it. > Thanks for your comments, I will update it later. > > Thanks, > Zhichao > > > -----Original Message----- > > From: Ni, Ray > > Sent: Tuesday, April 9, 2019 5:32 PM > > To: Gao, Zhichao <[email protected]>; '[email protected]' > > <[email protected]> > > Cc: Bret Barkelew <[email protected]>; Wang, Jian J > > <[email protected]>; Zeng, Star <[email protected]>; Gao, Liming > > <[email protected]>; Sean Brogan <[email protected]>; > > Michael Turner <[email protected]> > > Subject: RE: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS > > indications bit > > > > Resend to groups.io. > > > > > -----Original Message----- > > > From: Ni, Ray > > > Sent: Tuesday, April 9, 2019 5:31 PM > > > To: Gao, Zhichao <[email protected]>; [email protected] > > > Cc: Bret Barkelew <[email protected]>; Wang, Jian J > > > <[email protected]>; Zeng, Star <[email protected]>; Gao, > > > Liming <[email protected]>; Sean Brogan > > > <[email protected]>; Michael Turner > > > <[email protected]> > > > Subject: RE: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS > > > indications bit > > > > > > Zhichao, > > > In the very bottom of BdsEntry(), below code is always executed no > > > matter the PCD is true or false. > > > I don't think that's the right behavior. > > > > > > if (!BootSuccess) { > > > LoadOptions = EfiBootManagerGetLoadOptions (&LoadOptionCount, > > > LoadOptionTypePlatformRecovery); > > > ProcessLoadOptions (LoadOptions, LoadOptionCount); > > > EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); > > > } > > > > > > > -----Original Message----- > > > > From: Gao, Zhichao <[email protected]> > > > > Sent: Tuesday, April 2, 2019 1:50 PM > > > > To: [email protected] > > > > Cc: Bret Barkelew <[email protected]>; Wang, Jian J > > > > <[email protected]>; Ni, Ray <[email protected]>; Zeng, Star > > > > <[email protected]>; Gao, Liming <[email protected]>; Sean > > > > Brogan <[email protected]>; Michael Turner > > > > <[email protected]> > > > > Subject: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS > > > > indications bit > > > > > > > > From: Bret Barkelew <[email protected]> > > > > > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1678 > > > > > > > > Use the pcd PcdPlatformRecoverySupport to control whether to set > > > > the PlatformRecovery#### variable and whether to set the > > > > EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY bit of variable > > > > "OsIndicationsSupported". > > > > > > > > Cc: Jian J Wang <[email protected]> > > > > Cc: Ray Ni <[email protected]> > > > > Cc: Star Zeng <[email protected]> > > > > Cc: Liming Gao <[email protected]> > > > > Cc: Sean Brogan <[email protected]> > > > > Cc: Michael Turner <[email protected]> > > > > Cc: Bret Barkelew <[email protected]> > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > > Signed-off-by: Zhichao Gao <[email protected]> > > > > --- > > > > MdeModulePkg/Universal/BdsDxe/BdsDxe.inf | 1 + > > > > MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 74 +++++++++++++------ > > -- > > > -- > > > > - > > > > 2 files changed, 41 insertions(+), 34 deletions(-) > > > > > > > > diff --git a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf > > > > b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf > > > > index 82eb8aafc6..9caabbce7f 100644 > > > > --- a/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf > > > > +++ b/MdeModulePkg/Universal/BdsDxe/BdsDxe.inf > > > > @@ -101,6 +101,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 8946d79ab2..ade77adb7d 100644 > > > > --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > > > > +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c > > > > @@ -552,10 +552,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 ( > > > > @@ -769,41 +773,43 @@ 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); > > > > - Status = EfiBootManagerInitializeLoadOption ( > > > > - &LoadOption, > > > > - LoadOptionNumberUnassigned, > > > > - LoadOptionTypePlatformRecovery, > > > > - LOAD_OPTION_ACTIVE, > > > > - L"Default PlatformRecovery", > > > > - FilePath, > > > > - NULL, > > > > - 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; > > > > + if (PcdGetBool (PcdPlatformRecoverySupport)) { > > > > + // > > > > + // 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); > > > > + Status = EfiBootManagerInitializeLoadOption ( > > > > + &LoadOption, > > > > + LoadOptionNumberUnassigned, > > > > + LoadOptionTypePlatformRecovery, > > > > + LOAD_OPTION_ACTIVE, > > > > + L"Default PlatformRecovery", > > > > + FilePath, > > > > + NULL, > > > > + 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; > > > > + } > > > > } > > > > + LoadOption.OptionNumber = Index; > > > > + Status = EfiBootManagerLoadOptionToVariable (&LoadOption); > > > > + ASSERT_EFI_ERROR (Status); > > > > } > > > > - LoadOption.OptionNumber = Index; > > > > - Status = EfiBootManagerLoadOptionToVariable (&LoadOption); > > > > - ASSERT_EFI_ERROR (Status); > > > > + EfiBootManagerFreeLoadOption (&LoadOption); > > > > + FreePool (FilePath); > > > > + EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); > > > > } > > > > - EfiBootManagerFreeLoadOption (&LoadOption); > > > > - FreePool (FilePath); > > > > - EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); > > > > > > > > // > > > > // Report Status Code to indicate connecting drivers will > > > > happen > > > > -- > > > > 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#38768): https://edk2.groups.io/g/devel/message/38768 Mute This Topic: https://groups.io/mt/30918709/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
