[PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control whether BootManager is in BootOrder

[PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control whether BootManager is in BootOrder

[Gao, Zhichao]

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

Add a pcd to control whether the BootManager is in BootOrder variable.
>From a UI perspective, it never want to see a Front Page UI in the boot
order list. So add a pcd PcdBootManagerInBootOrder to control it. If it
is FALSE, the BootManager App wouldn't be added to BootOrder. And it
wouldn't be shown in the BootManagerMenuApp.

Notes:
BootManager means the application with file guid PcdBootManagerMenuFile.
It's default value is EEC25BDC-67F2-4D95-B1D5-F81B2039D11D, and users
usually change the pcd to UiApp.inf's file guid. BootManagerMenuApp means
uefi application with file guid EEC25BDC-67F2-4D95-B1D5-F81B2039D11D.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@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>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>

Bret Barkelew (2):
  MdeModulePkg: Add PcdBootManagerInBootOrder to dec file
  MdeModulePkg: Use pcd to control if BootManager in BootOrder

Zhichao Gao (2):
  MdeModulePkg: Add prompt and help string for PcdBootManagerInBootOrder
  MdeModulePkg: PcdBootManagerInBootOrder should only affect BootOrder

 .../Library/UefiBootManagerLib/BmBoot.c       | 29 +++++++++++++++----
 .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
 MdeModulePkg/MdeModulePkg.dec                 |  6 ++++
 MdeModulePkg/MdeModulePkg.uni                 |  6 ++++
 4 files changed, 36 insertions(+), 6 deletions(-)

-- 
2.21.0.windows.1

[PATCH 1/4] MdeModulePkg: Add PcdBootManagerInBootOrder to dec file

[Gao, Zhichao]

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

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

Add defination of PcdBootManagerInBootOrder to MdeModulePkg.dec.
This pcd indicates if the Boot Manager should be in the Boot
Order list.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@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>
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 12e0bbf579..839569aafd 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1666,6 +1666,12 @@
   # @Prompt Reset on memory type information change.
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|TRUE|BOOLEAN|0x00010056
 
+  ## Indicates if the Boot Manager should be in the Boot Order list.<BR><BR>
+  #   TRUE  - Add Boot Manager to boot order.<BR>
+  #   FALSE - Does not add Boot Manager to boot order.<BR>
+  # @Prompt Store Boot Manager in BootOrder.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerInBootOrder|TRUE|BOOLEAN|0x00010077
+
   ## Indicates if the BDS supports Platform Recovery.<BR><BR>
   #   TRUE  - BDS supports Platform Recovery.<BR>
   #   FALSE - BDS does not support Platform Recovery.<BR>
-- 
2.21.0.windows.1

[PATCH 2/4] MdeModulePkg: Add prompt and help string for PcdBootManagerInBootOrder

[Gao, Zhichao]

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

Add prompt and help string for PcdBootManagerInBootOrder.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@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>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdeModulePkg/MdeModulePkg.uni | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index 2bd1ad29f2..aa38ba1df3 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -445,6 +445,12 @@
                                                                                                        "TRUE  - Resets system when memory type information changes.<BR>\n"
                                                                                                        "FALSE - Does not reset system when memory type information changes.<BR>"
 
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdBootManagerInBootOrder_PROMPT  #language en-US "Store Boot Manager in BootOrder"
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdBootManagerInBootOrder_HELP  #language en-US "Indicates if the Boot Manager should be in the Boot Order list.<BR><BR>\n"
+                                                                                           "TRUE  - Add Boot Manager to boot order.<BR>"
+                                                                                           "FALSE - Does not add Boot Manager to boot order.<BR>"
+
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPlatformRecoverySupport_PROMPT  #language en-US "Support Platform Recovery"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdPlatformRecoverySupport_HELP  #language en-US "Indicates if the BDS supports Platform Recovery.<BR><BR>\n"
-- 
2.21.0.windows.1

[PATCH 3/4] MdeModulePkg: Use pcd to control if BootManager in BootOrder

[Gao, Zhichao]

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

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

Use pcd PcdBootManagerInBootOrder to control whether BootManager
is in BootOrder. Also add EFI_UNSUPPORTED to indicate the
BootManager is not in the BootOrder.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@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>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 .../Library/UefiBootManagerLib/BmBoot.c       | 20 +++++++++++++------
 .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 952033fc82..611acb91e2 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -2320,11 +2320,13 @@ EfiBootManagerRefreshAllBootOption (
 
   @param  BootOption    Return the boot option of the Boot Manager Menu
 
-  @retval EFI_SUCCESS   Successfully register the Boot Manager Menu.
-  @retval EFI_NOT_FOUND The Boot Manager Menu cannot be found.
-  @retval others        Return status of gRT->SetVariable (). BootOption still points
-                        to the Boot Manager Menu even the Status is not EFI_SUCCESS
-                        and EFI_NOT_FOUND.
+  @retval EFI_SUCCESS       Successfully register the Boot Manager Menu.
+  @retval EFI_NOT_FOUND     The Boot Manager Menu cannot be found.
+  @retval EFI_UNSUPPORTED   The Boot Manager Menu isn't register to Boot Order because of
+                            PcdBootManagerInBootOrder.
+  @retval others            Return status of gRT->SetVariable (). BootOption still points
+                            to the Boot Manager Menu even the Status is not EFI_SUCCESS
+                            and EFI_NOT_FOUND.
 **/
 EFI_STATUS
 BmRegisterBootManagerMenu (
@@ -2436,7 +2438,13 @@ BmRegisterBootManagerMenu (
     EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
     );
 
-  return EfiBootManagerAddLoadOptionVariable (BootOption, 0);
+  if (PcdGetBool (PcdBootManagerInBootOrder)) {
+    Status = EfiBootManagerAddLoadOptionVariable (BootOption, 0);
+  } else {
+    Status = EFI_UNSUPPORTED;
+  }
+
+  return Status;
 }
 
 /**
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
index ed6b4679eb..cfc01f57c6 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
@@ -116,3 +116,4 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile                     ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdDriverHealthConfigureForm               ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxRepairCount                          ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerInBootOrder                  ## CONSUMES
-- 
2.21.0.windows.1

[PATCH 4/4] MdeModulePkg: PcdBootManagerInBootOrder should only affect BootOrder

[Gao, Zhichao]

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

The PcdBootManagerInBootOrder should only control whether
BootManager is in BootOrder. It shouldn't affect the Hotkey
service with BootManager.
Acctually, setting this pcd hides the boot option from BootOrder.
But it didn't disable the boot option.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@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>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 611acb91e2..8bdac924b4 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -2322,8 +2322,6 @@ EfiBootManagerRefreshAllBootOption (
 
   @retval EFI_SUCCESS       Successfully register the Boot Manager Menu.
   @retval EFI_NOT_FOUND     The Boot Manager Menu cannot be found.
-  @retval EFI_UNSUPPORTED   The Boot Manager Menu isn't register to Boot Order because of
-                            PcdBootManagerInBootOrder.
   @retval others            Return status of gRT->SetVariable (). BootOption still points
                             to the Boot Manager Menu even the Status is not EFI_SUCCESS
                             and EFI_NOT_FOUND.
@@ -2344,6 +2342,7 @@ BmRegisterBootManagerMenu (
   UINTN                              Index;
   VOID                               *Data;
   UINTN                              DataSize;
+  UINT16                             OptionNumber;
 
   DevicePath = NULL;
   Description = NULL;
@@ -2441,7 +2440,17 @@ BmRegisterBootManagerMenu (
   if (PcdGetBool (PcdBootManagerInBootOrder)) {
     Status = EfiBootManagerAddLoadOptionVariable (BootOption, 0);
   } else {
-    Status = EFI_UNSUPPORTED;
+    //
+    // Get the free option number if the option number is unassigned
+    //
+    if (BootOption->OptionNumber == LoadOptionNumberUnassigned) {
+      Status = BmGetFreeOptionNumber (BootOption->OptionType, &OptionNumber);
+      if (EFI_ERROR (Status)) {
+        return Status;
+      }
+      BootOption->OptionNumber = OptionNumber;
+    }
+    Status = EfiBootManagerLoadOptionToVariable (BootOption);
   }
 
   return Status;
-- 
2.21.0.windows.1

Re: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control whether BootManager is in BootOrder

[Ni, Ray]

Zhichao,

Firstly, patch 4/4 is a fix to patch 3/4, why not merge them into one patch? Do we have a policy to strictly cherry-pick the patches from MU?

Secondly, the change that skips to add the #### to BootOrder may cause the Boot#### pointing to the BootManagerMenu be overwritten later. Reason is BmGetFreeOptionNumber() replies on BootOrder/BootNext to get a free option number.

If the requirement is to let BootManagerMenuApp hide the BootManagerMenu, we could change the BootManagerMenuApp using a PCD to control.

Thanks,
Ray

> -----Original Message-----
> From: Gao, Zhichao
> Sent: Wednesday, July 17, 2019 3:50 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@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>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control
> whether BootManager is in BootOrder
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1979
> 
> Add a pcd to control whether the BootManager is in BootOrder variable.
> From a UI perspective, it never want to see a Front Page UI in the boot order
> list. So add a pcd PcdBootManagerInBootOrder to control it. If it is FALSE, the
> BootManager App wouldn't be added to BootOrder. And it wouldn't be
> shown in the BootManagerMenuApp.
> 
> Notes:
> BootManager means the application with file guid PcdBootManagerMenuFile.
> It's default value is EEC25BDC-67F2-4D95-B1D5-F81B2039D11D, and users
> usually change the pcd to UiApp.inf's file guid. BootManagerMenuApp
> means uefi application with file guid EEC25BDC-67F2-4D95-B1D5-
> F81B2039D11D.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@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>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> 
> Bret Barkelew (2):
>   MdeModulePkg: Add PcdBootManagerInBootOrder to dec file
>   MdeModulePkg: Use pcd to control if BootManager in BootOrder
> 
> Zhichao Gao (2):
>   MdeModulePkg: Add prompt and help string for
> PcdBootManagerInBootOrder
>   MdeModulePkg: PcdBootManagerInBootOrder should only affect
> BootOrder
> 
>  .../Library/UefiBootManagerLib/BmBoot.c       | 29 +++++++++++++++----
>  .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
>  MdeModulePkg/MdeModulePkg.dec                 |  6 ++++
>  MdeModulePkg/MdeModulePkg.uni                 |  6 ++++
>  4 files changed, 36 insertions(+), 6 deletions(-)
> 
> --
> 2.21.0.windows.1

Re: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control whether BootManager is in BootOrder

[Gao, Zhichao]

Ray,

You are right. I didn't view the implementation of BmGetFreeOptionNumber. Only use it base on its comments.
If new options are added after BootManagerMenuApp with the pcd's value FALSE then it would be overwriten.

3/4 would remove the BootManagerMenuApp option from Boot#### and BootOrder. But adding it to BootMenu (i.e. BootOrder) is done by platform code, platform usually add it at PlatformBootManagerAfterConsole function. So I think we are able to choose whether to add it or not. Both changing the pcd or do not add it in the platform (actually PlatformBootManagerLib) can match this requirement.

So I think the requirement is keeping all the features of BootMenuApp (such as hotkey boot, boot next and so on) except showing it in the BootMenu.

This patch is inappropriate. But firstly, did I misunderstand the requirement?

Thanks,
Zhichao

> -----Original Message-----
> From: Ni, Ray
> Sent: Wednesday, July 17, 2019 4:07 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@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>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: RE: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control
> whether BootManager is in BootOrder
> 
> Zhichao,
> 
> Firstly, patch 4/4 is a fix to patch 3/4, why not merge them into one patch? Do
> we have a policy to strictly cherry-pick the patches from MU?
> 
> Secondly, the change that skips to add the #### to BootOrder may cause the
> Boot#### pointing to the BootManagerMenu be overwritten later. Reason is
> BmGetFreeOptionNumber() replies on BootOrder/BootNext to get a free
> option number.
> 
> If the requirement is to let BootManagerMenuApp hide the
> BootManagerMenu, we could change the BootManagerMenuApp using a
> PCD to control.
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: Gao, Zhichao
> > Sent: Wednesday, July 17, 2019 3:50 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@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>; Bret Barkelew
> > <Bret.Barkelew@microsoft.com>
> > Subject: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control
> > whether BootManager is in BootOrder
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1979
> >
> > Add a pcd to control whether the BootManager is in BootOrder variable.
> > From a UI perspective, it never want to see a Front Page UI in the
> > boot order list. So add a pcd PcdBootManagerInBootOrder to control it.
> > If it is FALSE, the BootManager App wouldn't be added to BootOrder.
> > And it wouldn't be shown in the BootManagerMenuApp.
> >
> > Notes:
> > BootManager means the application with file guid
> PcdBootManagerMenuFile.
> > It's default value is EEC25BDC-67F2-4D95-B1D5-F81B2039D11D, and users
> > usually change the pcd to UiApp.inf's file guid. BootManagerMenuApp
> > means uefi application with file guid EEC25BDC-67F2-4D95-B1D5-
> > F81B2039D11D.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@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>
> > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> >
> > Bret Barkelew (2):
> >   MdeModulePkg: Add PcdBootManagerInBootOrder to dec file
> >   MdeModulePkg: Use pcd to control if BootManager in BootOrder
> >
> > Zhichao Gao (2):
> >   MdeModulePkg: Add prompt and help string for
> > PcdBootManagerInBootOrder
> >   MdeModulePkg: PcdBootManagerInBootOrder should only affect
> BootOrder
> >
> >  .../Library/UefiBootManagerLib/BmBoot.c       | 29 +++++++++++++++----
> >  .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
> >  MdeModulePkg/MdeModulePkg.dec                 |  6 ++++
> >  MdeModulePkg/MdeModulePkg.uni                 |  6 ++++
> >  4 files changed, 36 insertions(+), 6 deletions(-)
> >
> > --
> > 2.21.0.windows.1

Re: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control whether BootManager is in BootOrder

[Ni, Ray]

I don't know the requirement. That's just my guess so I used "if".
You need to clarify with Bret Barkelew and get real requirement.

> -----Original Message-----
> From: Gao, Zhichao
> Sent: Wednesday, July 17, 2019 4:29 PM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@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>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: RE: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control
> whether BootManager is in BootOrder
> 
> Ray,
> 
> You are right. I didn't view the implementation of BmGetFreeOptionNumber.
> Only use it base on its comments.
> If new options are added after BootManagerMenuApp with the pcd's value
> FALSE then it would be overwriten.
> 
> 3/4 would remove the BootManagerMenuApp option from Boot#### and
> BootOrder. But adding it to BootMenu (i.e. BootOrder) is done by platform
> code, platform usually add it at PlatformBootManagerAfterConsole function.
> So I think we are able to choose whether to add it or not. Both changing the
> pcd or do not add it in the platform (actually PlatformBootManagerLib) can
> match this requirement.
> 
> So I think the requirement is keeping all the features of BootMenuApp (such
> as hotkey boot, boot next and so on) except showing it in the BootMenu.
> 
> This patch is inappropriate. But firstly, did I misunderstand the requirement?
> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: Ni, Ray
> > Sent: Wednesday, July 17, 2019 4:07 PM
> > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@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>; Bret Barkelew
> > <Bret.Barkelew@microsoft.com>
> > Subject: RE: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to
> > control whether BootManager is in BootOrder
> >
> > Zhichao,
> >
> > Firstly, patch 4/4 is a fix to patch 3/4, why not merge them into one
> > patch? Do we have a policy to strictly cherry-pick the patches from MU?
> >
> > Secondly, the change that skips to add the #### to BootOrder may cause
> > the Boot#### pointing to the BootManagerMenu be overwritten later.
> > Reason is
> > BmGetFreeOptionNumber() replies on BootOrder/BootNext to get a free
> > option number.
> >
> > If the requirement is to let BootManagerMenuApp hide the
> > BootManagerMenu, we could change the BootManagerMenuApp using a
> PCD to
> > control.
> >
> > Thanks,
> > Ray
> >
> > > -----Original Message-----
> > > From: Gao, Zhichao
> > > Sent: Wednesday, July 17, 2019 3:50 PM
> > > To: devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > <hao.a.wu@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>; Bret Barkelew
> > > <Bret.Barkelew@microsoft.com>
> > > Subject: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control
> > > whether BootManager is in BootOrder
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1979
> > >
> > > Add a pcd to control whether the BootManager is in BootOrder variable.
> > > From a UI perspective, it never want to see a Front Page UI in the
> > > boot order list. So add a pcd PcdBootManagerInBootOrder to control it.
> > > If it is FALSE, the BootManager App wouldn't be added to BootOrder.
> > > And it wouldn't be shown in the BootManagerMenuApp.
> > >
> > > Notes:
> > > BootManager means the application with file guid
> > PcdBootManagerMenuFile.
> > > It's default value is EEC25BDC-67F2-4D95-B1D5-F81B2039D11D, and
> > > users usually change the pcd to UiApp.inf's file guid.
> > > BootManagerMenuApp means uefi application with file guid
> > > EEC25BDC-67F2-4D95-B1D5- F81B2039D11D.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Hao A Wu <hao.a.wu@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>
> > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > >
> > > Bret Barkelew (2):
> > >   MdeModulePkg: Add PcdBootManagerInBootOrder to dec file
> > >   MdeModulePkg: Use pcd to control if BootManager in BootOrder
> > >
> > > Zhichao Gao (2):
> > >   MdeModulePkg: Add prompt and help string for
> > > PcdBootManagerInBootOrder
> > >   MdeModulePkg: PcdBootManagerInBootOrder should only affect
> > BootOrder
> > >
> > >  .../Library/UefiBootManagerLib/BmBoot.c       | 29 +++++++++++++++----
> > >  .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
> > >  MdeModulePkg/MdeModulePkg.dec                 |  6 ++++
> > >  MdeModulePkg/MdeModulePkg.uni                 |  6 ++++
> > >  4 files changed, 36 insertions(+), 6 deletions(-)
> > >
> > > --
> > > 2.21.0.windows.1

Re: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control whether BootManager is in BootOrder

[Gao, Zhichao]

Sorry to miss the first question. I used to send a patch to the community to merge my change and the changes from Mu.
And Leif suggests to separate them. That would distinguish the contributor when encounter copyright issue. And also make a history of it.
Here is the message link: https://edk2.groups.io/g/devel/message/41834.

Thanks,
Zhichao

> -----Original Message-----
> From: Ni, Ray
> Sent: Wednesday, July 17, 2019 4:07 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@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>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: RE: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control
> whether BootManager is in BootOrder
> 
> Zhichao,
> 
> Firstly, patch 4/4 is a fix to patch 3/4, why not merge them into one patch? Do
> we have a policy to strictly cherry-pick the patches from MU?
> 
> Secondly, the change that skips to add the #### to BootOrder may cause the
> Boot#### pointing to the BootManagerMenu be overwritten later. Reason is
> BmGetFreeOptionNumber() replies on BootOrder/BootNext to get a free
> option number.
> 
> If the requirement is to let BootManagerMenuApp hide the
> BootManagerMenu, we could change the BootManagerMenuApp using a
> PCD to control.
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: Gao, Zhichao
> > Sent: Wednesday, July 17, 2019 3:50 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@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>; Bret Barkelew
> > <Bret.Barkelew@microsoft.com>
> > Subject: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control
> > whether BootManager is in BootOrder
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1979
> >
> > Add a pcd to control whether the BootManager is in BootOrder variable.
> > From a UI perspective, it never want to see a Front Page UI in the
> > boot order list. So add a pcd PcdBootManagerInBootOrder to control it.
> > If it is FALSE, the BootManager App wouldn't be added to BootOrder.
> > And it wouldn't be shown in the BootManagerMenuApp.
> >
> > Notes:
> > BootManager means the application with file guid
> PcdBootManagerMenuFile.
> > It's default value is EEC25BDC-67F2-4D95-B1D5-F81B2039D11D, and users
> > usually change the pcd to UiApp.inf's file guid. BootManagerMenuApp
> > means uefi application with file guid EEC25BDC-67F2-4D95-B1D5-
> > F81B2039D11D.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@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>
> > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> >
> > Bret Barkelew (2):
> >   MdeModulePkg: Add PcdBootManagerInBootOrder to dec file
> >   MdeModulePkg: Use pcd to control if BootManager in BootOrder
> >
> > Zhichao Gao (2):
> >   MdeModulePkg: Add prompt and help string for
> > PcdBootManagerInBootOrder
> >   MdeModulePkg: PcdBootManagerInBootOrder should only affect
> BootOrder
> >
> >  .../Library/UefiBootManagerLib/BmBoot.c       | 29 +++++++++++++++----
> >  .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
> >  MdeModulePkg/MdeModulePkg.dec                 |  6 ++++
> >  MdeModulePkg/MdeModulePkg.uni                 |  6 ++++
> >  4 files changed, 36 insertions(+), 6 deletions(-)
> >
> > --
> > 2.21.0.windows.1

Re: [edk2-devel] [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control whether BootManager is in BootOrder

[Laszlo Ersek]

On 07/17/19 10:07, Ni, Ray wrote:
> Zhichao,
> 
> Firstly, patch 4/4 is a fix to patch 3/4, why not merge them into one patch? Do we have a policy to strictly cherry-pick the patches from MU?
> 
> Secondly, the change that skips to add the #### to BootOrder may cause the Boot#### pointing to the BootManagerMenu be overwritten later. Reason is BmGetFreeOptionNumber() replies on BootOrder/BootNext to get a free option number.
> 
> If the requirement is to let BootManagerMenuApp hide the BootManagerMenu, we could change the BootManagerMenuApp using a PCD to control.

This would be safer, in my opinion, because those platforms that don't
consume BootManagerMenuApp, would not depend on the new PCD at all.

(Not sure if the proposal satisfies the requirement though.)

Thanks!
Laszlo

> 
> Thanks,
> Ray
> 
>> -----Original Message-----
>> From: Gao, Zhichao
>> Sent: Wednesday, July 17, 2019 3:50 PM
>> To: devel@edk2.groups.io
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@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>; Bret Barkelew
>> <Bret.Barkelew@microsoft.com>
>> Subject: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control
>> whether BootManager is in BootOrder
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1979
>>
>> Add a pcd to control whether the BootManager is in BootOrder variable.
>> From a UI perspective, it never want to see a Front Page UI in the boot order
>> list. So add a pcd PcdBootManagerInBootOrder to control it. If it is FALSE, the
>> BootManager App wouldn't be added to BootOrder. And it wouldn't be
>> shown in the BootManagerMenuApp.
>>
>> Notes:
>> BootManager means the application with file guid PcdBootManagerMenuFile.
>> It's default value is EEC25BDC-67F2-4D95-B1D5-F81B2039D11D, and users
>> usually change the pcd to UiApp.inf's file guid. BootManagerMenuApp
>> means uefi application with file guid EEC25BDC-67F2-4D95-B1D5-
>> F81B2039D11D.
>>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Hao A Wu <hao.a.wu@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>
>> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>>
>> Bret Barkelew (2):
>>   MdeModulePkg: Add PcdBootManagerInBootOrder to dec file
>>   MdeModulePkg: Use pcd to control if BootManager in BootOrder
>>
>> Zhichao Gao (2):
>>   MdeModulePkg: Add prompt and help string for
>> PcdBootManagerInBootOrder
>>   MdeModulePkg: PcdBootManagerInBootOrder should only affect
>> BootOrder
>>
>>  .../Library/UefiBootManagerLib/BmBoot.c       | 29 +++++++++++++++----
>>  .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
>>  MdeModulePkg/MdeModulePkg.dec                 |  6 ++++
>>  MdeModulePkg/MdeModulePkg.uni                 |  6 ++++
>>  4 files changed, 36 insertions(+), 6 deletions(-)
>>
>> --
>> 2.21.0.windows.1
> 
> 
> 
> 

Re: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control whether BootManager is in BootOrder

[Ni, Ray]

I see. I have no concerns on the 3 &4 in different patches.

> -----Original Message-----
> From: Gao, Zhichao
> Sent: Wednesday, July 17, 2019 4:44 PM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@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>; Bret
> Barkelew <Bret.Barkelew@microsoft.com>
> Subject: RE: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control whether BootManager is in BootOrder
> 
> Sorry to miss the first question. I used to send a patch to the community to merge my change and the changes from Mu.
> And Leif suggests to separate them. That would distinguish the contributor when encounter copyright issue. And also make a
> history of it.
> Here is the message link: https://edk2.groups.io/g/devel/message/41834.
> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: Ni, Ray
> > Sent: Wednesday, July 17, 2019 4:07 PM
> > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@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>; Bret Barkelew
> > <Bret.Barkelew@microsoft.com>
> > Subject: RE: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control
> > whether BootManager is in BootOrder
> >
> > Zhichao,
> >
> > Firstly, patch 4/4 is a fix to patch 3/4, why not merge them into one patch? Do
> > we have a policy to strictly cherry-pick the patches from MU?
> >
> > Secondly, the change that skips to add the #### to BootOrder may cause the
> > Boot#### pointing to the BootManagerMenu be overwritten later. Reason is
> > BmGetFreeOptionNumber() replies on BootOrder/BootNext to get a free
> > option number.
> >
> > If the requirement is to let BootManagerMenuApp hide the
> > BootManagerMenu, we could change the BootManagerMenuApp using a
> > PCD to control.
> >
> > Thanks,
> > Ray
> >
> > > -----Original Message-----
> > > From: Gao, Zhichao
> > > Sent: Wednesday, July 17, 2019 3:50 PM
> > > To: devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > <hao.a.wu@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>; Bret Barkelew
> > > <Bret.Barkelew@microsoft.com>
> > > Subject: [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control
> > > whether BootManager is in BootOrder
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1979
> > >
> > > Add a pcd to control whether the BootManager is in BootOrder variable.
> > > From a UI perspective, it never want to see a Front Page UI in the
> > > boot order list. So add a pcd PcdBootManagerInBootOrder to control it.
> > > If it is FALSE, the BootManager App wouldn't be added to BootOrder.
> > > And it wouldn't be shown in the BootManagerMenuApp.
> > >
> > > Notes:
> > > BootManager means the application with file guid
> > PcdBootManagerMenuFile.
> > > It's default value is EEC25BDC-67F2-4D95-B1D5-F81B2039D11D, and users
> > > usually change the pcd to UiApp.inf's file guid. BootManagerMenuApp
> > > means uefi application with file guid EEC25BDC-67F2-4D95-B1D5-
> > > F81B2039D11D.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Hao A Wu <hao.a.wu@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>
> > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > >
> > > Bret Barkelew (2):
> > >   MdeModulePkg: Add PcdBootManagerInBootOrder to dec file
> > >   MdeModulePkg: Use pcd to control if BootManager in BootOrder
> > >
> > > Zhichao Gao (2):
> > >   MdeModulePkg: Add prompt and help string for
> > > PcdBootManagerInBootOrder
> > >   MdeModulePkg: PcdBootManagerInBootOrder should only affect
> > BootOrder
> > >
> > >  .../Library/UefiBootManagerLib/BmBoot.c       | 29 +++++++++++++++----
> > >  .../UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
> > >  MdeModulePkg/MdeModulePkg.dec                 |  6 ++++
> > >  MdeModulePkg/MdeModulePkg.uni                 |  6 ++++
> > >  4 files changed, 36 insertions(+), 6 deletions(-)
> > >
> > > --
> > > 2.21.0.windows.1