Re: [edk2-devel] [PATCH] MdeModulePkg: Retrive boot manager menu from any fv

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

Hao, I agree with you on the performance evaluation and function header update. Besides that, I don't have other comments.

> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: Tuesday, May 11, 2021 9:20 AM
> To: Liu, Zhiguang <zhiguang.liu@intel.com>; Gao, Zhichao
> <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>;
> devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Retrive boot manager
> menu from any fv
> 
> > -----Original Message-----
> > From: Liu, Zhiguang <zhiguang.liu@intel.com>
> > Sent: Monday, May 10, 2021 5:00 PM
> > To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Zhichao
> > <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Retrive boot manager
> menu
> > from any fv
> >
> > Hi Hao,
> >
> > I don't have the data about the boot performance.
> > However, I think it has little impact for the originally working platforms.
> > In function GetSectionFromAnyFv, It will first search section in current FV.
> > If it finds the section, it will return succuss.
> > Only if it can't find the section, it will take extra time to search in other FV.
> This
> > case won't happen in originally working platforms
> 
> 
> Thanks Zhiguang.
> The implementation of GetSectionFromAnyFv() will try searching the caller's
> FFS first, so there should be no impact to currently working platforms.
> 
> Could you help to update the function comments for
> BmRegisterBootManagerMenu() to reflect this change (first searching the
> same FV, if fails then all FVs).
> With this handled:
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> 
> Hello Zhichao and Ray, do you have comments for this patch? Thanks in
> advance.
> 
> Best Regards,
> Hao Wu
> 
> 
> >
> > Thanks
> > Zhiguang
> >
> > > -----Original Message-----
> > > From: Wu, Hao A <hao.a.wu@intel.com>
> > > Sent: Monday, May 10, 2021 4:42 PM
> > > To: devel@edk2.groups.io; Liu, Zhiguang <zhiguang.liu@intel.com>
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Zhichao
> > > <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg: Retrive boot
> manager
> > > menu from any fv
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > Zhiguang Liu
> > > > Sent: Monday, May 10, 2021 4:16 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > > > <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Ni,
> Ray
> > > > <ray.ni@intel.com>
> > > > Subject: [edk2-devel] [PATCH] MdeModulePkg: Retrive boot manager
> > > menu
> > > > from any fv
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3384
> > > >
> > > > Currently, UefiBootManagerLib has the below assumption:
> > > >   Assume the BootManagerMenuFile is in the same FV as the module
> > > > links to this library.
> > > > It has some limitation now, so remove the assumption.
> > >
> > >
> > > Hello Zhiguang,
> > >
> > > Sorry for a question. Do you know what is the boot performance impact
> > > if 'GetSectionFromAnyFv()' is being used?
> > > Any data on the impact for originally working platforms?
> > >
> > > Best Regards,
> > > Hao Wu
> > >
> > >
> > > >
> > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> > > > ---
> > > >  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 7 +++----
> > > >  1 file changed, 3 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > > > b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > > > index 6cc34d29c0..b8df4d590f 100644
> > > > --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > > > +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> > > > @@ -2,7 +2,7 @@
> > > >    Library functions which relates with booting.  Copyright (c)
> > > > 2019, NVIDIA CORPORATION. All rights reserved.-Copyright (c) 2011 -
> > > > 2020, Intel Corporation. All rights reserved.<BR>+Copyright (c) 2011
> > > > - 2021, Intel Corporation. All rights reserved.<BR> (C) Copyright
> > > > 2015-2021 Hewlett Packard Enterprise Development LP<BR> SPDX-
> License-
> > Identifier:
> > > > BSD-2- Clause-Patent @@ -2385,7 +2385,6 @@
> > > > EfiBootManagerRefreshAllBootOption (
> > > >    This function is called to get or create the boot option for the
> > > > Boot
> > > Manager
> > > > Menu.    The Boot Manager Menu is shown after successfully booting a
> > > boot
> > > > option.-  Assume the BootManagerMenuFile is in the same FV as the
> > > module
> > > > links to this library.    @param  BootOption    Return the boot option of
> the
> > > > Boot Manager Menu @@ -2437,7 +2436,7 @@
> > > BmRegisterBootManagerMenu (
> > > >     if (DevicePath == NULL) {     Data = NULL;-    Status =
> GetSectionFromFv (+
> > > > Status = GetSectionFromAnyFv (                PcdGetPtr
> > > > (PcdBootManagerMenuFile),                EFI_SECTION_PE32,                0,@@
> -
> > > > 2455,7 +2454,7 @@ BmRegisterBootManagerMenu (
> > > >      //     // Get BootManagerMenu application's description from EFI
> User
> > > > Interface Section.     //-    Status = GetSectionFromFv (+    Status =
> > > > GetSectionFromAnyFv (                PcdGetPtr (PcdBootManagerMenuFile),
> > > > EFI_SECTION_USER_INTERFACE,                0,--
> > > > 2.30.0.windows.2
> > > >
> > > >
> > > >
> > > > -=-=-=-=-=-=
> > > > Groups.io Links: You receive all messages sent to this group.
> > > > View/Reply Online (#74859):
> > > > https://edk2.groups.io/g/devel/message/74859
> > > > Mute This Topic: https://groups.io/mt/82714887/1768737
> > > > Group Owner: devel+owner@edk2.groups.io
> > > > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > > > [hao.a.wu@intel.com]
> > > > - =-=-=-=-=-=
> > > >