public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gao, Zhichao" <zhichao.gao@intel.com>
To: "Ni, Ray" <ray.ni@intel.com>,
	"devel@edk2.groups.io" <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
Date: Wed, 17 Jul 2019 08:28:37 +0000	[thread overview]
Message-ID: <3CE959C139B4C44DBEA1810E3AA6F9000B8085BB@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C23325A@SHSMSX104.ccr.corp.intel.com>

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


  reply	other threads:[~2019-07-17  8:28 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-17  7:50 [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control whether BootManager is in BootOrder Gao, Zhichao
2019-07-17  7:50 ` [PATCH 1/4] MdeModulePkg: Add PcdBootManagerInBootOrder to dec file Gao, Zhichao
2019-07-17  7:50 ` [PATCH 2/4] MdeModulePkg: Add prompt and help string for PcdBootManagerInBootOrder Gao, Zhichao
2019-07-17  7:50 ` [PATCH 3/4] MdeModulePkg: Use pcd to control if BootManager in BootOrder Gao, Zhichao
2019-07-17  7:50 ` [PATCH 4/4] MdeModulePkg: PcdBootManagerInBootOrder should only affect BootOrder Gao, Zhichao
2019-07-17  8:07 ` [PATCH 0/4] Add a pcd PcdBootManagerInBootOrder to control whether BootManager is in BootOrder Ni, Ray
2019-07-17  8:28   ` Gao, Zhichao [this message]
2019-07-17  8:42     ` Ni, Ray
2019-07-17  8:44   ` Gao, Zhichao
2019-07-18  0:50     ` Ni, Ray
2019-07-17 12:59   ` [edk2-devel] " Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3CE959C139B4C44DBEA1810E3AA6F9000B8085BB@SHSMSX101.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox