public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/2] Use a pcd to control Platform Recovery behavior
@ 2019-04-02  5:49 Zhichao Gao
  2019-04-02  5:49 ` [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit Zhichao Gao
  2019-04-02  5:49 ` [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set " Zhichao Gao
  0 siblings, 2 replies; 10+ messages in thread
From: Zhichao Gao @ 2019-04-02  5:49 UTC (permalink / raw)
  To: edk2-devel

Add a pcd PcdPlatformRecoverySupport to control the variable
PlatformRecovery#### and the EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY
bit of variable "OsIndicationsSupported".

Bret Barkelew (2):
  MdeModulePkg: Add a pcd to set the OS indications bit
  MdeModulePkg/BdsDxe: Use a pcd to set OS indications bit

 MdeModulePkg/MdeModulePkg.dec            |  6 ++
 MdeModulePkg/Universal/BdsDxe/BdsDxe.inf |  1 +
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 74 +++++++++++++-----------
 3 files changed, 47 insertions(+), 34 deletions(-)

-- 
2.21.0.windows.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit
  2019-04-02  5:49 [PATCH 0/2] Use a pcd to control Platform Recovery behavior Zhichao Gao
@ 2019-04-02  5:49 ` Zhichao Gao
  2019-04-09  9:23   ` [edk2-devel] " Ni, Ray
  2019-04-02  5:49 ` [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set " Zhichao Gao
  1 sibling, 1 reply; 10+ messages in thread
From: Zhichao Gao @ 2019-04-02  5:49 UTC (permalink / raw)
  To: edk2-devel
  Cc: Bret Barkelew, Jian J Wang, Ray Ni, Star Zeng, Liming Gao,
	Sean Brogan, Michael Turner

From: Bret Barkelew <Bret.Barkelew@microsoft.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1678

Add a pcd PcdPlatformRecoverySupport to conditionally
set an OS indications bit.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdeModulePkg/MdeModulePkg.dec | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 43992b7f65..870f49749a 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1669,6 +1669,12 @@
   # @Prompt Reset on memory type information change.
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|TRUE|BOOLEAN|0x00010056
 
+  ## Indicates if the BDS supports Platform Recovery.<BR><BR>
+  #   TRUE  - Platform supports Platform Recovery.<BR>
+  #   FALSE - Does not add Boot Manager to boot order.<BR>
+  # @Prompt Support Platform Recovery.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPlatformRecoverySupport|TRUE|BOOLEAN|0x00010078
+
   ## Specify the foreground color for Subtile text in HII Form Browser. The default value is EFI_BLUE.
   #  Only following values defined in UEFI specification are valid:<BR><BR>
   #  0x00 (EFI_BLACK)<BR>
-- 
2.21.0.windows.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS indications bit
  2019-04-02  5:49 [PATCH 0/2] Use a pcd to control Platform Recovery behavior Zhichao Gao
  2019-04-02  5:49 ` [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit Zhichao Gao
@ 2019-04-02  5:49 ` Zhichao Gao
       [not found]   ` <734D49CCEBEEF84792F5B80ED585239D5C0DBCB4@SHSMSX104.ccr.corp.intel.com>
  1 sibling, 1 reply; 10+ messages in thread
From: Zhichao Gao @ 2019-04-02  5:49 UTC (permalink / raw)
  To: edk2-devel
  Cc: Bret Barkelew, Jian J Wang, Ray Ni, Star Zeng, Liming Gao,
	Sean Brogan, Michael Turner

From: Bret Barkelew <Bret.Barkelew@microsoft.com>

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 <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit
  2019-04-02  5:49 ` [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit Zhichao Gao
@ 2019-04-09  9:23   ` Ni, Ray
  0 siblings, 0 replies; 10+ messages in thread
From: Ni, Ray @ 2019-04-09  9:23 UTC (permalink / raw)
  To: Gao, Zhichao, devel

[-- Attachment #1: Type: text/plain, Size: 40 bytes --]

Reviewed-by: Ray Ni <ray.ni@intel.com>

[-- Attachment #2: Type: text/html, Size: 99 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS indications bit
       [not found]   ` <734D49CCEBEEF84792F5B80ED585239D5C0DBCB4@SHSMSX104.ccr.corp.intel.com>
@ 2019-04-09  9:32     ` Ni, Ray
  2019-04-10  0:44       ` Gao, Zhichao
  0 siblings, 1 reply; 10+ messages in thread
From: Ni, Ray @ 2019-04-09  9:32 UTC (permalink / raw)
  To: Gao, Zhichao, 'devel@edk2.groups.io'
  Cc: Bret Barkelew, Wang, Jian J, Zeng, Star, Gao, Liming, Sean Brogan,
	Michael Turner

Resend to groups.io.

> -----Original Message-----
> From: Ni, Ray
> Sent: Tuesday, April 9, 2019 5:31 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>; edk2-devel@lists.01.org
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> Michael Turner <Michael.Turner@microsoft.com>
> 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 <zhichao.gao@intel.com>
> > Sent: Tuesday, April 2, 2019 1:50 PM
> > To: edk2-devel@lists.01.org
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> > <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean Brogan
> > <sean.brogan@microsoft.com>; Michael Turner
> > <Michael.Turner@microsoft.com>
> > Subject: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS
> > indications bit
> >
> > From: Bret Barkelew <Bret.Barkelew@microsoft.com>
> >
> > 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 <jian.j.wang@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > Cc: Michael Turner <Michael.Turner@microsoft.com>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > ---
> >  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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS indications bit
  2019-04-09  9:32     ` Ni, Ray
@ 2019-04-10  0:44       ` Gao, Zhichao
  2019-04-10  1:59         ` Ni, Ray
  2019-04-10  2:00         ` Ni, Ray
  0 siblings, 2 replies; 10+ messages in thread
From: Gao, Zhichao @ 2019-04-10  0:44 UTC (permalink / raw)
  To: Ni, Ray, 'devel@edk2.groups.io'
  Cc: Bret Barkelew, Wang, Jian J, Zeng, Star, Gao, Liming, Sean Brogan,
	Michael Turner

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 <zhichao.gao@intel.com>; 'devel@edk2.groups.io'
> <devel@edk2.groups.io>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> Michael Turner <Michael.Turner@microsoft.com>
> 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 <zhichao.gao@intel.com>; edk2-devel@lists.01.org
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> > Michael Turner <Michael.Turner@microsoft.com>
> > 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 <zhichao.gao@intel.com>
> > > Sent: Tuesday, April 2, 2019 1:50 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> > > <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean
> > > Brogan <sean.brogan@microsoft.com>; Michael Turner
> > > <Michael.Turner@microsoft.com>
> > > Subject: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS
> > > indications bit
> > >
> > > From: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > >
> > > 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 <jian.j.wang@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Star Zeng <star.zeng@intel.com>
> > > Cc: Liming Gao <liming.gao@intel.com>
> > > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > > Cc: Michael Turner <Michael.Turner@microsoft.com>
> > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > > ---
> > >  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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS indications bit
  2019-04-10  0:44       ` Gao, Zhichao
@ 2019-04-10  1:59         ` Ni, Ray
  2019-04-10  2:00         ` Ni, Ray
  1 sibling, 0 replies; 10+ messages in thread
From: Ni, Ray @ 2019-04-10  1:59 UTC (permalink / raw)
  To: Gao, Zhichao, 'devel@edk2.groups.io'
  Cc: Bret Barkelew, Wang, Jian J, Zeng, Star, Gao, Liming, Sean Brogan,
	Michael Turner

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 <ray.ni@intel.com>; 'devel@edk2.groups.io' <devel@edk2.groups.io>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; Michael
> Turner <Michael.Turner@microsoft.com>
> 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 <zhichao.gao@intel.com>; 'devel@edk2.groups.io'
> > <devel@edk2.groups.io>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> > Michael Turner <Michael.Turner@microsoft.com>
> > 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 <zhichao.gao@intel.com>; edk2-devel@lists.01.org
> > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao,
> > > Liming <liming.gao@intel.com>; Sean Brogan
> > > <sean.brogan@microsoft.com>; Michael Turner
> > > <Michael.Turner@microsoft.com>
> > > 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 <zhichao.gao@intel.com>
> > > > Sent: Tuesday, April 2, 2019 1:50 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > > <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> > > > <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean
> > > > Brogan <sean.brogan@microsoft.com>; Michael Turner
> > > > <Michael.Turner@microsoft.com>
> > > > Subject: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS
> > > > indications bit
> > > >
> > > > From: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > > >
> > > > 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 <jian.j.wang@intel.com>
> > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > Cc: Star Zeng <star.zeng@intel.com>
> > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > > > Cc: Michael Turner <Michael.Turner@microsoft.com>
> > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > > > ---
> > > >  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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS indications bit
  2019-04-10  0:44       ` Gao, Zhichao
  2019-04-10  1:59         ` Ni, Ray
@ 2019-04-10  2:00         ` Ni, Ray
  2019-04-26  3:12           ` Gao, Zhichao
  1 sibling, 1 reply; 10+ messages in thread
From: Ni, Ray @ 2019-04-10  2:00 UTC (permalink / raw)
  To: Gao, Zhichao, 'devel@edk2.groups.io'
  Cc: Bret Barkelew, Wang, Jian J, Zeng, Star, Gao, Liming, Sean Brogan,
	Michael Turner

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 <zhichao.gao@intel.com>; 'devel@edk2.groups.io'
> <devel@edk2.groups.io>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; Michael
> Turner <Michael.Turner@microsoft.com>
> 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 <ray.ni@intel.com>; 'devel@edk2.groups.io'
> > <devel@edk2.groups.io>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> > Michael Turner <Michael.Turner@microsoft.com>
> > 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 <zhichao.gao@intel.com>; 'devel@edk2.groups.io'
> > > <devel@edk2.groups.io>
> > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao,
> > > Liming <liming.gao@intel.com>; Sean Brogan
> > > <sean.brogan@microsoft.com>; Michael Turner
> > > <Michael.Turner@microsoft.com>
> > > 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 <zhichao.gao@intel.com>; edk2-devel@lists.01.org
> > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao,
> > > > Liming <liming.gao@intel.com>; Sean Brogan
> > > > <sean.brogan@microsoft.com>; Michael Turner
> > > > <Michael.Turner@microsoft.com>
> > > > 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 <zhichao.gao@intel.com>
> > > > > Sent: Tuesday, April 2, 2019 1:50 PM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > > > <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star
> > > > > <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean
> > > > > Brogan <sean.brogan@microsoft.com>; Michael Turner
> > > > > <Michael.Turner@microsoft.com>
> > > > > Subject: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS
> > > > > indications bit
> > > > >
> > > > > From: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > > > >
> > > > > 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 <jian.j.wang@intel.com>
> > > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > > Cc: Star Zeng <star.zeng@intel.com>
> > > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > > > > Cc: Michael Turner <Michael.Turner@microsoft.com>
> > > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > > > > ---
> > > > >  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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS indications bit
  2019-04-10  2:00         ` Ni, Ray
@ 2019-04-26  3:12           ` Gao, Zhichao
  2019-04-26  5:55             ` Ni, Ray
  0 siblings, 1 reply; 10+ messages in thread
From: Gao, Zhichao @ 2019-04-26  3:12 UTC (permalink / raw)
  To: Ni, Ray, 'devel@edk2.groups.io'
  Cc: Bret Barkelew, Wang, Jian J, Zeng, Star, Gao, Liming, Sean Brogan,
	Michael Turner

Hi Ray,
 
Refer to the spec 2.8 section 3.4.3:
If system firmware supports boot option recovery as described in Section 3.4, system firmware must include a PlatformRecovery#### variable specifying a short-form File Path Media Device Path (see Section 3.1.2) containing the platform default file path for removable media (see Table 15).

Can we regard the "PlatformRecovery####" variable as optional? If so, I think it is fine to let the platform to control its behavior.

Thanks,
Zhichao

> -----Original Message-----
> From: Ni, Ray
> Sent: Wednesday, April 10, 2019 10:00 AM
> To: Gao, Zhichao <zhichao.gao@intel.com>; 'devel@edk2.groups.io'
> <devel@edk2.groups.io>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> Michael Turner <Michael.Turner@microsoft.com>
> Subject: RE: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS
> indications bit
> 
> 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 <zhichao.gao@intel.com>; 'devel@edk2.groups.io'
> > <devel@edk2.groups.io>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> > Michael Turner <Michael.Turner@microsoft.com>
> > 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 <ray.ni@intel.com>; 'devel@edk2.groups.io'
> > > <devel@edk2.groups.io>
> > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao,
> > > Liming <liming.gao@intel.com>; Sean Brogan
> > > <sean.brogan@microsoft.com>; Michael Turner
> > > <Michael.Turner@microsoft.com>
> > > 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 <zhichao.gao@intel.com>; 'devel@edk2.groups.io'
> > > > <devel@edk2.groups.io>
> > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao,
> > > > Liming <liming.gao@intel.com>; Sean Brogan
> > > > <sean.brogan@microsoft.com>; Michael Turner
> > > > <Michael.Turner@microsoft.com>
> > > > 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 <zhichao.gao@intel.com>;
> > > > > edk2-devel@lists.01.org
> > > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > > > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao,
> > > > > Liming <liming.gao@intel.com>; Sean Brogan
> > > > > <sean.brogan@microsoft.com>; Michael Turner
> > > > > <Michael.Turner@microsoft.com>
> > > > > 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 <zhichao.gao@intel.com>
> > > > > > Sent: Tuesday, April 2, 2019 1:50 PM
> > > > > > To: edk2-devel@lists.01.org
> > > > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > > > > <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng,
> > > > > > Star <star.zeng@intel.com>; Gao, Liming
> > > > > > <liming.gao@intel.com>; Sean Brogan
> > > > > > <sean.brogan@microsoft.com>; Michael Turner
> > > > > > <Michael.Turner@microsoft.com>
> > > > > > Subject: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS
> > > > > > indications bit
> > > > > >
> > > > > > From: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > > > > >
> > > > > > 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 <jian.j.wang@intel.com>
> > > > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > > > Cc: Star Zeng <star.zeng@intel.com>
> > > > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > > > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > > > > > Cc: Michael Turner <Michael.Turner@microsoft.com>
> > > > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > > > > > ---
> > > > > >  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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS indications bit
  2019-04-26  3:12           ` Gao, Zhichao
@ 2019-04-26  5:55             ` Ni, Ray
  0 siblings, 0 replies; 10+ messages in thread
From: Ni, Ray @ 2019-04-26  5:55 UTC (permalink / raw)
  To: Gao, Zhichao, 'devel@edk2.groups.io'
  Cc: Bret Barkelew, Wang, Jian J, Zeng, Star, Gao, Liming, Sean Brogan,
	Michael Turner

> -----Original Message-----
> From: Gao, Zhichao
> Sent: Thursday, April 25, 2019 8:13 PM
> To: Ni, Ray <ray.ni@intel.com>; 'devel@edk2.groups.io'
> <devel@edk2.groups.io>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> Michael Turner <Michael.Turner@microsoft.com>
> Subject: RE: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS
> indications bit
> 
> Hi Ray,
> 
> Refer to the spec 2.8 section 3.4.3:
> If system firmware supports boot option recovery as described in Section 3.4,
> system firmware must include a PlatformRecovery#### variable specifying a
> short-form File Path Media Device Path (see Section 3.1.2) containing the
> platform default file path for removable media (see Table 15).
> 
> Can we regard the "PlatformRecovery####" variable as optional? If so, I think
> it is fine to let the platform to control its behavior.
Can you explain a bit more? What's the meaning of "let platform control behavior"?

> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: Ni, Ray
> > Sent: Wednesday, April 10, 2019 10:00 AM
> > To: Gao, Zhichao <zhichao.gao@intel.com>; 'devel@edk2.groups.io'
> > <devel@edk2.groups.io>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> > Michael Turner <Michael.Turner@microsoft.com>
> > Subject: RE: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set OS
> > indications bit
> >
> > 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 <zhichao.gao@intel.com>; 'devel@edk2.groups.io'
> > > <devel@edk2.groups.io>
> > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao,
> > > Liming <liming.gao@intel.com>; Sean Brogan
> > > <sean.brogan@microsoft.com>; Michael Turner
> > > <Michael.Turner@microsoft.com>
> > > 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 <ray.ni@intel.com>; 'devel@edk2.groups.io'
> > > > <devel@edk2.groups.io>
> > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao,
> > > > Liming <liming.gao@intel.com>; Sean Brogan
> > > > <sean.brogan@microsoft.com>; Michael Turner
> > > > <Michael.Turner@microsoft.com>
> > > > 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 <zhichao.gao@intel.com>; 'devel@edk2.groups.io'
> > > > > <devel@edk2.groups.io>
> > > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > > > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao,
> > > > > Liming <liming.gao@intel.com>; Sean Brogan
> > > > > <sean.brogan@microsoft.com>; Michael Turner
> > > > > <Michael.Turner@microsoft.com>
> > > > > 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 <zhichao.gao@intel.com>;
> > > > > > edk2-devel@lists.01.org
> > > > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> > > > > > <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>;
> > > > > > Gao, Liming <liming.gao@intel.com>; Sean Brogan
> > > > > > <sean.brogan@microsoft.com>; Michael Turner
> > > > > > <Michael.Turner@microsoft.com>
> > > > > > 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 <zhichao.gao@intel.com>
> > > > > > > Sent: Tuesday, April 2, 2019 1:50 PM
> > > > > > > To: edk2-devel@lists.01.org
> > > > > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian
> > > > > > > J <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng,
> > > > > > > Star <star.zeng@intel.com>; Gao, Liming
> > > > > > > <liming.gao@intel.com>; Sean Brogan
> > > > > > > <sean.brogan@microsoft.com>; Michael Turner
> > > > > > > <Michael.Turner@microsoft.com>
> > > > > > > Subject: [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set
> > > > > > > OS indications bit
> > > > > > >
> > > > > > > From: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > > > > > >
> > > > > > > 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 <jian.j.wang@intel.com>
> > > > > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > > > > Cc: Star Zeng <star.zeng@intel.com>
> > > > > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > > > > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > > > > > > Cc: Michael Turner <Michael.Turner@microsoft.com>
> > > > > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > > > > > > ---
> > > > > > >  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


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-04-26  5:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-02  5:49 [PATCH 0/2] Use a pcd to control Platform Recovery behavior Zhichao Gao
2019-04-02  5:49 ` [PATCH 1/2] MdeModulePkg: Add a pcd to set the OS indications bit Zhichao Gao
2019-04-09  9:23   ` [edk2-devel] " Ni, Ray
2019-04-02  5:49 ` [PATCH 2/2] MdeModulePkg/BdsDxe: Use a pcd to set " Zhichao Gao
     [not found]   ` <734D49CCEBEEF84792F5B80ED585239D5C0DBCB4@SHSMSX104.ccr.corp.intel.com>
2019-04-09  9:32     ` Ni, Ray
2019-04-10  0:44       ` Gao, Zhichao
2019-04-10  1:59         ` Ni, Ray
2019-04-10  2:00         ` Ni, Ray
2019-04-26  3:12           ` Gao, Zhichao
2019-04-26  5:55             ` Ni, Ray

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox