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 (#38698): https://edk2.groups.io/g/devel/message/38698
Mute This Topic: https://groups.io/mt/30918709/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to