From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by mx.groups.io with SMTP id smtpd.web09.3607.1573516699827418930 for ; Mon, 11 Nov 2019 15:58:19 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.43, mailfrom: ray.ni@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Nov 2019 15:58:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,294,1569308400"; d="scan'208,217";a="197852768" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga008.jf.intel.com with ESMTP; 11 Nov 2019 15:58:16 -0800 Received: from fmsmsx114.amr.corp.intel.com (10.18.116.8) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 11 Nov 2019 15:58:15 -0800 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX114.amr.corp.intel.com (10.18.116.8) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 11 Nov 2019 15:58:15 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.127]) by shsmsx102.ccr.corp.intel.com ([169.254.2.108]) with mapi id 14.03.0439.000; Tue, 12 Nov 2019 07:58:14 +0800 From: "Ni, Ray" To: "devel@edk2.groups.io" , "jbrasen@nvidia.com" , Laszlo Ersek , "afish@apple.com" CC: Ashish Singhal , "Wang, Jian J" , "Wu, Hao A" , "Gao, Zhichao" , "Kinney, Michael D" Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration Thread-Topic: [edk2-devel] [PATCH] Support skipping automatic BM enumeration Thread-Index: AQHVjtTBfs56XIdrLUS3rD3UlLLaAqd0BEOAgAfitPD//4ZKgIAAGusAgAABkQCAAARWgIAABc2AgAAJNQCAAD03AIAAdMAAgAAS3ACAABdZAIAAX/iAgAAHo4CAABRiAIAAixowgABX2YCAAMcwAIAAs8oA//97lgCAAIoFAIAAKeMAgAF/U4CABSEZAIAAlohA Date: Mon, 11 Nov 2019 23:58:14 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C358AD3@SHSMSX104.ccr.corp.intel.com> References: <37D801DD-41E8-452E-9F24-ADF52BFDB676@apple.com> <72ce1d71-2a65-a6c0-1dd8-7628429c5a3c@redhat.com> <746A8D5E-DC45-4D39-9C4D-97A10BE2E0B0@apple.com> <734D49CCEBEEF84792F5B80ED585239D5C35125E@SHSMSX104.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C352ECA@SHSMSX104.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C352F7C@SHSMSX104.ccr.corp.intel.com> ,<897d405a-b164-773c-5784-47fb8c4f9040@redhat.com> In-Reply-To: Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNzE3NmRlN2UtYjI3OC00ZTQwLWI1MTAtNTgzMThlNTBiZjg1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoidG9xWFwvSnlKbEdHNVQxK3NcLzVSdTFUVTZ3UE1iRkg5aHprd050THNLalVHNWRqanhjWHBLUmE1WFFqM2Rqc3JqIn0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_734D49CCEBEEF84792F5B80ED585239D5C358AD3SHSMSX104ccrcor_" --_000_734D49CCEBEEF84792F5B80ED585239D5C358AD3SHSMSX104ccrcor_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Jeff, If adding a PCD to control UiApp can meet the real needs, I prefer to do i= n that way instead of adding new APIs to PlatformBootManagerLib. Thanks, Ray From: devel@edk2.groups.io On Behalf Of Jeff Brasen Sent: Tuesday, November 12, 2019 6:58 AM To: Laszlo Ersek ; Ni, Ray ; devel@ed= k2.groups.io; afish@apple.com Cc: Ashish Singhal ; Wang, Jian J ; Wu, Hao A ; Gao, Zhichao ; Kinney, Michael D Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeratio= n If we are concerned about deploying this and breaking builds we could do t= his via a new protocol instead. In that case though we would leave the old = default behavior in the code to handle platforms that didn't implement the = new protocol, so this might not be the cleanest way to deploy this. We could also look at adding a generic platform boot hook function (either= as a library function or protocol) if we wanted to limit the number of dis= ruption on new customization hooks. Something like EFI_STATUS PlatformBootNotify (CONST EFI_GUID *NotificationType, VOID *Con= textData OPTIONAL) Where Notification type describes where we are that we want platform to po= tentially handle and ContextData is per type caller allocated data that pro= vides additional in/out data. This has the same issue of leaving the curren= t default behavior in place for unsupported types as well as being a less t= han specific function to describe. Thanks, Jeff ________________________________ From: Laszlo Ersek > Sent: Friday, November 8, 2019 9:37 AM To: Jeff Brasen >; Ni, Ray <= ray.ni@intel.com>; devel@edk2.groups.io >; afi= sh@apple.com > Cc: Ashish Singhal >; Wang, Jian J >; Wu,= Hao A >; Gao, Zhichao >; Kinney, Michael D > Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeratio= n On 11/07/19 18:46, Jeff Brasen wrote: > Fixing UiApp seems reasonable, I do think we would want a hook to the pl= atform library in here as the enumeration that occurs in the UiApp is inten= ded to do a full enumeration of the system and there may be platform specif= ics to how that occurs. Fully agreed -- entering UiApp should expose everything bootable in the system, unless (perhaps) PlatformBootManagerLib specifically thinks otherwise. Of course, then we arrive (again) at the problem that a call in UefiBootManagerLib, to a *new* PlatformBootManagerLib API, will break tens of out-of-tree platforms. :) I think that can be prevented, as follows; but it will take quite some tim= e: - introduce the new function declaration in "PlatformBootManagerLib.h", - modify all platforms (in tree and out of tree) to implement (define) the new function, - call the new function from UefiBootManagerLib For some history / background on this kind of problem, I suggest reading through: https://bugzilla.tianocore.org/show_bug.cgi?id=3D982 Thanks, Laszlo > From: Ni, Ray > > Sent: Thursday, November 7, 2019 12:21 AM > To: devel@edk2.groups.io; Jeff Brasen >; afish@apple.com > Cc: Ashish Singhal >; Laszlo Ersek >; Wang, Jia= n J >; Wu, Hao A >; Gao, Zhichao >; Kinney, Michael D > > Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumerat= ion > > I treat the issue in this way: > > 1. Platform Boot Manager library does a good job. It doesn't always c= all RefreshAll() API to auto-create the boot options > 2. But UiApp doesn't. It constantly call RefreshAll(). > > Do you think that we can fix UiApp instead? For example, introducing a P= CD to control the boot option refresh behavior? > > Thanks, > Ray > > From: devel@edk2.groups.io> >> On Behalf Of Jeff Brasen > Sent: Thursday, November 7, 2019 3:02 PM > To: Ni, Ray >>; afish@apple.com > Cc: devel@edk2.groups.io>; Ashish Singhal >>; Laszlo Ersek >>; Wang, = Jian J >>; Wu, Hao A >>; Gao, Zhichao >>; Kinney, M= ichael D >> > Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumerat= ion > > The issue is there are some auto created options we do not want on our p= latform. > Get Outlook for Android > > ________________________________ > From: Ni, Ray >> > Sent: Wednesday, November 6, 2019 11:59:31 PM > To: Jeff Brasen >>; afish@apple.com >> > Cc: devel@edk2.groups.io> >= >; Ashish Singhal >>; Laszlo E= rsek >>; Wang, Jian J >>; Wu, Hao A >>; Gao, Zhichao >>; Kinney, Michael D >> > Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumerat= ion > > > Jeff, > > RefreshAllBootOption() only modifies/creates the auto-created boot optio= ns. For the boot options created by platform boot manager library, they sta= y with no one touches. And all auto-created boot options are appended in th= e end of boot option list (through BootOrder). > > > > From: Jeff Brasen >> > Sent: Thursday, November 7, 2019 12:13 PM > To: afish@apple.com; Ni, Ray >> > Cc: devel@edk2.groups.io>; Ashish Singhal >>; Laszlo Ersek >>; Wang, = Jian J >>; Wu, Hao A >>; Gao, Zhichao >>; Kinney, M= ichael D >> > Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumerat= ion > > > > As the suggestions below made sense, we updated our platform boot manage= r library to behave in this manner and for normal boots everything works we= ll. However the UiApp and boot maintenance applications in EDK2 also call E= fiBootManagerRefreshAllBootOption() when ever a user goes into the menu whi= ch will re-create the skipped boot options with no place for the platform c= ode to intervene. > > > > What about a solution where we add a new Platform library function that = allows for override of the behavior of BmEnumerateBootOptions? For example,= either a function or protocol that takes the same parameters as this funct= ion and only if it returns NULL then we continue to the default enumeration= code. Or a function call inserted at the end that would modify the load o= ption array after the system does the standard enumeration. > > > > -Jeff > > > > From: afish@apple.com >> > Sent: Wednesday, November 6, 2019 9:20 AM > To: Ni, Ray >> > Cc: devel@edk2.groups.io>; Jeff Brasen >>; Ashish Singhal >>; Las= zlo Ersek >>; Wang, Jian J >>; Wu, Hao A >>; Gao, Zhichao >>; Mike Kinney >> > Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumerat= ion > > > > Ray, > > > > Is there an obvious hook point we could point Jeff and Ashish at? > > > > Long term it would be a good idea to have a Wiki page to give some guida= nce on how to customize the BDS. > > > > Thanks, > > > > Andrew Fish > > > > On Nov 5, 2019, at 9:20 PM, Ni, Ray >> wrote: > > > > Andrew, > > I agree with your opinion. > > It's expected that Platform Boot Manager lib calls EfiBootManagerRefresh= AllBootOption() only in full configuration boot path. > > The full configuration boot path is chosen when hardware changes happen.= So it's not expected EfiBootManagerRefresh...() be > called in every boot. > > So you could: > > 1. Delete the auto-created option pointing to LoadFile instance > 2. Create your own one with customized description. > > > > > > From: afish@apple.com >> > Sent: Wednesday, November 6, 2019 10:47 AM > To: devel@edk2.groups.io>; jbrasen@nvidia.com > Cc: Ashish Singhal >>; Lasz= lo Ersek >>; Ni, Ray >>; Wang, Jian J= >>; Wu, Hao A >>; Gao, Zhichao >>; Kinney, Michael= D >> > Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumerat= ion > > > > > > > > On Nov 5, 2019, at 7:34 PM, Jeff Brasen >> wrot= e: > > > > Wouldn't having a variable that we create and delete on every boot put u= nnecessary stress on the SPI-NOR that the variable store lives on? > What about the alternative approach where we allow the platform code to = modify the attributes of the auto created variable to disable it with hidde= n/!active but still match for detection purposes so that it doesn't delete = and recreate the modified variable each boot? That way all the logic on wha= t to disable can still be in the platform code and all the existing logic i= n the boot manager can stay basically the same? > > What changes every boot that forces the variable to need to get modified= ? > > I would assume the NOR driver is smart enough to not update a variable t= hat is not changing. > > The custom BDS could could only create the variable for this device if i= t does not exist. > > [JB] The current flow with no changes in the boot manager would be as fo= llows > > > > 1. Scan for instance of the boot option in the variables > 2. It will not be found, so create a new boot option store it to a va= riable and update BootOrder > 3. Platform code runs creates the options for the boot option it want= s and writes those to variable store > 4. Delete/disable the boot option in the variable store > > > > When you reboot it won't find the variable so 1/2/4 will re-occur > > > > The code that does this (1/2) is EfiBootManagerRefreshAllBootOption in B= mBoot.c > > > > If you modify the variable to disable it with hidden/not active it would= delete that and create a new one as well as the code wouldn't recognize th= at is the same boot option. > > > > If however we modify EfiBootManagerFindLoadOption to not compare the att= ributes (at least allow for differences in active and hidden) then the when= it refreshes every thing it would see the match and not delete/create a ne= w variable in the store and thus we wouldn't have changes every boot. > > > > > > Jeff, > > > > Sorry if I'm a little off on the sequence of things as the platform I wo= rk on day to day has a custom BDS and does not use this library..... I thou= gh the patch changed BmEnumerateBootOptions(), so that is going to change h= ow EfiBootManagerRefreshAllBootOption() works. I'd also point out the patch= as given is invalid as it changed the behavior of the public library API f= or EfiBootManagerRefreshAllBootOption() [1] so for the patch to be valid it= would need to change the comments to reflect the new behavior. This is kin= d of what Laszlo's technical debt comment was about. > > > > I think Laszlo advocated having the BDS platform specific code make sure= the boot variables are in the correct state. That should happen before the= Boot Manager code runs, and it is not clear to me why the Boot Manager co= uld would need to run if you have a valid EFI nvram variable to boot from. > > > > I think the question is how is your use case different than the boot var= iable that Windows installs? If it works kind of the same way then the answ= er is to have the BDS platform specific code write the boot variable. > > > > > > [1] > > /** > > The function creates boot options for all possible bootable medias in = the following order: > > 1. Removable BlockIo - The boot option only points to the r= emovable media > > device, like USB key, DVD, Floppy et= c. > > 2. Fixed BlockIo - The boot option only points to a Fix= ed blockIo device, > > like HardDisk. > > 3. Non-BlockIo SimpleFileSystem - The boot option points to a device s= upporting > > SimpleFileSystem Protocol, but not s= upporting BlockIo > > protocol. > > 4. LoadFile - The boot option points to the media = supporting > > LoadFile protocol. > > Reference: UEFI Spec chapter 3.3 Boot Option Variables Default Boot Be= havior > > > > The function won't delete the boot option not added by itself. > > **/ > > VOID > > EFIAPI > > EfiBootManagerRefreshAllBootOption ( > > VOID > > ); > > > > Thanks, > > > > Andrew Fish > > > > Thanks, > > Andrew Fish > > Thanks, > > Jeff > > ________________________________ > > This email message is for the sole use of the intended recipient(s) and = may contain confidential information. Any unauthorized review, use, disclo= sure or distribution is prohibited. If you are not the intended recipient,= please contact the sender by reply email and destroy all copies of the ori= ginal message. > > ________________________________ > > > --_000_734D49CCEBEEF84792F5B80ED585239D5C358AD3SHSMSX104ccrcor_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

Jeff,

If adding a PCD to control UiApp can meet the real = needs, I prefer to do in that way instead of adding new APIs to PlatformBoo= tManagerLib.

 

Thanks,

Ray

 

From: de= vel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeff Brasen
Sent: Tuesday, November 12, 2019 6:58 AM
To: Laszlo Ersek <lersek@redhat.com>; Ni, Ray <ray.ni@inte= l.com>; devel@edk2.groups.io; afish@apple.com
Cc: Ashish Singhal <ashishsingha@nvidia.com>; Wang, Jian J &l= t;jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhi= chao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@= intel.com>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enu= meration

 

If we = are concerned about deploying this and breaking builds we could do this via= a new protocol instead. In that case though we would leave the old default= behavior in the code to handle platforms that didn't implement the new protocol, so this might not be the cleanest= way to deploy this.

&= nbsp;

We cou= ld also look at adding a generic platform boot hook function (either as a l= ibrary function or protocol) if we wanted to limit the number of disruption= on new customization hooks. Something like

&= nbsp;

EFI_ST= ATUS PlatformBootNotify (CONST EFI_GUID *NotificationType, VOID *ContextDat= a OPTIONAL)

&= nbsp;

Where = Notification type describes where we are that we want platform to potential= ly handle and ContextData is per type caller allocated data that provides a= dditional in/out data. This has the same issue of leaving the current default behavior in place for unsupported ty= pes as well as being a less than specific function to describe.<= /span>

&= nbsp;

Thanks,

Jeff

&= nbsp;

From: Laszlo Ersek <lersek@redhat.com>
Sent: Friday, November 8, 2019 9:37 AM
To: Jeff Brasen <jbrasen@n= vidia.com>; Ni, Ray <ray.ni@i= ntel.com>; devel@edk2.groups.io <devel@edk2.groups.io>; afish@apple.com <afish@apple.com>
Cc: Ashish Singhal <a= shishsingha@nvidia.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>= ;; Kinney, Michael D <mich= ael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enu= meration

 

On 11/07/19 18:46, J= eff Brasen wrote:
> Fixing UiApp seems reasonable, I do think we would want a hook to the= platform library in here as the enumeration that occurs in the UiApp is in= tended to do a full enumeration of the system and there may be platform spe= cifics to how that occurs.

Fully agreed -- entering UiApp should expose everything bootable in the system, unless (perhaps) PlatformBootManagerLib specifically thinks
otherwise.

Of course, then we arrive (again) at the problem that a call in
UefiBootManagerLib, to a *new* PlatformBootManagerLib API, will break
tens of out-of-tree platforms. :)

I think that can be prevented, as follows; but it will take quite some tim= e:

- introduce the new function declaration in "PlatformBootManagerLib.h= ",
- modify all platforms (in tree and out of tree) to implement (define)
the new function,
- call the new function from UefiBootManagerLib

For some history / background on this kind of problem, I suggest reading through:

  ht= tps://bugzilla.tianocore.org/show_bug.cgi?id=3D982

Thanks,
Laszlo

> From: Ni, Ray <ray.ni@intel.co= m>
> Sent: Thursday, November 7, 2019 12:21 AM
> To: devel@edk2.groups.io;= Jeff Brasen <jbrasen@nvidia.com>; afish@apple.com
> Cc: Ashish Singhal <ash= ishsingha@nvidia.com>; Laszlo Ersek <lersek@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao= , Zhichao <zhichao.gao@intel.co= m>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enume= ration
>
> I treat the issue in this way:
>
>   1.  Platform Boot Manager library does a good job. I= t doesn't always call RefreshAll() API to auto-create the boot options
>   2.  But UiApp doesn't. It constantly call RefreshAll= ().
>
> Do you think that we can fix UiApp instead? For example, introducing = a PCD to control the boot option refresh behavior?
>
> Thanks,
> Ray
>
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk= 2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Jeff Brasen
> Sent: Thursday, November 7, 2019 3:02 PM
> To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; afish@app= le.com<mailto:afish@apple.com>=
> Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ashish Si= nghal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>= ;>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Wang, Ji= an J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zh= ichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael= .d.kinney@intel.com>>
> Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enume= ration
>
> The issue is there are some auto created options we do not want on ou= r platform.
> Get Outlook for Android<https://= aka.ms/ghei36>
>
> ________________________________
> From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Sent: Wednesday, November 6, 2019 11:59:31 PM
> To: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>= ;; afish@apple.com<mailto:afish@apple= .com> <afish@apple.com<mailto:afish@apple.com>>
> Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.= groups.io<mailto:devel@edk2.groups.io>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com&g= t;>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com&g= t;>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:m= ichael.d.kinney@intel.com>>
> Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enume= ration
>
>
> Jeff,
>
> RefreshAllBootOption() only modifies/creates the auto-created boot op= tions. For the boot options created by platform boot manager library, they = stay with no one touches. And all auto-created boot options are appended in= the end of boot option list (through BootOrder).
>
>
>
> From: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>&= gt;
> Sent: Thursday, November 7, 2019 12:13 PM
> To: afish@apple.com<mailto:afis= h@apple.com>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>=
> Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ashish Si= nghal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>= ;>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Wang, Ji= an J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zh= ichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael= .d.kinney@intel.com>>
> Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enume= ration
>
>
>
> As the suggestions below made sense, we updated our platform boot man= ager library to behave in this manner and for normal boots everything works= well. However the UiApp and boot maintenance applications in EDK2 also cal= l EfiBootManagerRefreshAllBootOption() when ever a user goes into the menu which will re-create the skipped boot= options with no place for the platform code to intervene.
>
>
>
> What about a solution where we add a new Platform library function th= at allows for override of the behavior of BmEnumerateBootOptions? For examp= le, either a function or protocol that takes the same parameters as this fu= nction and only if it returns NULL then we continue to the default enumeration code.  Or a function call ins= erted at the end that would modify the load option array after the system d= oes the standard enumeration.
>
>
>
> -Jeff
>
>
>
> From: afish@apple.com<mailto:af= ish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
> Sent: Wednesday, November 6, 2019 9:20 AM
> To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Jeff Bras= en <jb= rasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia= .com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com&g= t;>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com&g= t;>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com&g= t;>; Mike Kinney <michael.d.kinney@intel.com<mailto:michael= .d.kinney@intel.com>>
> Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enume= ration
>
>
>
> Ray,
>
>
>
> Is there an obvious hook point we could point Jeff and Ashish at?
>
>
>
> Long term it would be a good idea to have a Wiki page to give some gu= idance on how to customize the BDS.
>
>
>
> Thanks,
>
>
>
> Andrew Fish
>
>
>
> On Nov 5, 2019, at 9:20 PM, Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.co= m>> wrote:
>
>
>
> Andrew,
>
> I agree with your opinion.
>
> It's expected that Platform Boot Manager lib calls EfiBootManagerRefr= eshAllBootOption() only in full configuration boot path.
>
> The full configuration boot path is chosen when hardware changes happ= en. So it's not expected EfiBootManagerRefresh...() be
> called in every boot.
>
> So you could:
>
>   1.  Delete the auto-created option pointing to LoadF= ile instance
>   2.  Create your own one with customized description.=
>
>
>
>
>
> From: afish@apple.com<mailto:af= ish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
> Sent: Wednesday, November 6, 2019 10:47 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; jbrasen@n= vidia.com<mailto:jbrasen@nvidia.co= m>
> Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingh= a@nvidia.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.c= om>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Wang, Jian J <= jia= n.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zh= ichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael= .d.kinney@intel.com>>
> Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enume= ration
>
>
>
>
>
>
>
> On Nov 5, 2019, at 7:34 PM, Jeff Brasen <jbrasen@nvidia.com<mailto:jbras= en@nvidia.com>> wrote:
>
>
>
> Wouldn't having a variable that we create and delete on every boot pu= t unnecessary stress on the SPI-NOR that the variable store lives on?
> What about the alternative approach where we allow the platform code = to modify the attributes of the auto created variable to disable it with hi= dden/!active but still match for detection purposes so that it doesn't dele= te and recreate the modified variable each boot? That way all the logic on what to disable can still be in the = platform code and all the existing logic in the boot manager can stay basic= ally the same?
>
> What changes every boot that forces the variable to need to get modif= ied?
>
> I would assume the NOR driver is smart enough to not update a variabl= e that is not changing.
>
> The custom BDS could could only create the variable for this device i= f it does not exist.
>
> [JB] The current flow with no changes in the boot manager would be as= follows
>
>
>
>   1.  Scan for instance of the boot option in the vari= ables
>   2.  It will not be found, so create a new boot optio= n store it to a variable and update BootOrder
>   3.  Platform code runs creates the options for the b= oot option it wants and writes those to variable store
>   4.  Delete/disable the boot option in the variable s= tore
>
>
>
> When you reboot it won't find the variable so 1/2/4 will re-occur
>
>
>
> The code that does this (1/2) is EfiBootManagerRefreshAllBootOption i= n BmBoot.c
>
>
>
> If you modify the variable to disable it with hidden/not active it wo= uld delete that and create a new one as well as the code wouldn't recognize= that is the same boot option.
>
>
>
> If however we modify EfiBootManagerFindLoadOption to not compare the = attributes (at least allow for differences in active and hidden) then the w= hen it refreshes every thing it would see the match and not delete/create a= new variable in the store and thus we wouldn't have changes every boot.
>
>
>
>
>
> Jeff,
>
>
>
> Sorry if I'm a little off on the sequence of things as the platform I= work on day to day has a custom BDS and does not use this library..... I t= hough the patch changed BmEnumerateBootOptions(), so that is going to chang= e how EfiBootManagerRefreshAllBootOption() works. I'd also point out the patch as given is invalid as it changed the= behavior of the public library API for EfiBootManagerRefreshAllBootOption(= ) [1] so for the patch to be valid it would need to change the comments to = reflect the new behavior. This is kind of what Laszlo's technical debt comment was about.
>
>
>
> I think Laszlo advocated having the BDS platform specific code make s= ure the boot variables are in the correct state. That should happen before = the Boot Manager code runs, and it is  not clear to me why the Boot Ma= nager could would need to run if you have a valid EFI nvram variable to boot from.
>
>
>
> I think the question is how is your use case different than the boot = variable that Windows installs? If it works kind of the same way then the a= nswer is to have the BDS platform specific code write the boot variable. >
>
>
>
>
> [1]
>
> /**
>
>   The function creates boot options for all possible bootab= le medias in the following order:
>
>   1. Removable BlockIo      &= nbsp;     - The boot option only points to the removabl= e media
>
>           &nbs= p;            &= nbsp;            dev= ice, like USB key, DVD, Floppy etc.
>
>   2. Fixed BlockIo       = ;         - The boot option only po= ints to a Fixed blockIo device,
>
>           &nbs= p;            &= nbsp;            lik= e HardDisk.
>
>   3. Non-BlockIo SimpleFileSystem - The boot option points = to a device supporting
>
>           &nbs= p;            &= nbsp;            Sim= pleFileSystem Protocol, but not supporting BlockIo
>
>           &nbs= p;            &= nbsp;            pro= tocol.
>
>   4. LoadFile       &nbs= p;             = - The boot option points to the media supporting
>
>           &nbs= p;            &= nbsp;            Loa= dFile protocol.
>
>   Reference: UEFI Spec chapter 3.3 Boot Option Variables De= fault Boot Behavior
>
>
>
>   The function won't delete the boot option not added by it= self.
>
> **/
>
> VOID
>
> EFIAPI
>
> EfiBootManagerRefreshAllBootOption (
>
>   VOID
>
>   );
>
>
>
> Thanks,
>
>
>
> Andrew Fish
>
>
>
> Thanks,
>
> Andrew Fish
>
> Thanks,
>
> Jeff
>
> ________________________________
>
> This email message is for the sole use of the intended recipient(s) a= nd may contain confidential information.  Any unauthorized review, use= , disclosure or distribution is prohibited.  If you are not the intend= ed recipient, please contact the sender by reply email and destroy all copies of the original message.
>
> ________________________________
>
> >

--_000_734D49CCEBEEF84792F5B80ED585239D5C358AD3SHSMSX104ccrcor_--