Sorry I didn't realize the impact in the beginning. > -----Original Message----- > From: Ni, Ray > Sent: Wednesday, April 10, 2019 10:00 AM > 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 > > 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 (#38769): https://edk2.groups.io/g/devel/message/38769 Mute This Topic: https://groups.io/mt/30918709/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
