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

["Ni, Ray" <ray.ni@intel.com>]

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