From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mx.groups.io with SMTP id smtpd.web09.1145.1573111268034426198 for ; Wed, 06 Nov 2019 23:21:08 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.120, mailfrom: ray.ni@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Nov 2019 23:21:07 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,277,1569308400"; d="scan'208,217";a="377336456" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga005.jf.intel.com with ESMTP; 06 Nov 2019 23:21:06 -0800 Received: from fmsmsx119.amr.corp.intel.com (10.18.124.207) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 6 Nov 2019 23:21:06 -0800 Received: from shsmsx108.ccr.corp.intel.com (10.239.4.97) by FMSMSX119.amr.corp.intel.com (10.18.124.207) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 6 Nov 2019 23:21:05 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.127]) by SHSMSX108.ccr.corp.intel.com ([169.254.8.41]) with mapi id 14.03.0439.000; Thu, 7 Nov 2019 15:21:04 +0800 From: "Ni, Ray" To: "devel@edk2.groups.io" , "jbrasen@nvidia.com" , "afish@apple.com" CC: Ashish Singhal , Laszlo Ersek , "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//97lgCAAIoFAA== Date: Thu, 7 Nov 2019 07:21:04 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C352F7C@SHSMSX104.ccr.corp.intel.com> References: <1b91c052-f64c-1dca-98ff-a2777afd7f77@redhat.com> <734D49CCEBEEF84792F5B80ED585239D5C34F98A@SHSMSX104.ccr.corp.intel.com> <6766B443-E14A-4F57-984E-5A865FB22CC9@apple.com> <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> In-Reply-To: Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_6b558183-044c-4105-8d9c-cea02a2a3d86_Enabled=True;MSIP_Label_6b558183-044c-4105-8d9c-cea02a2a3d86_SiteId=43083d15-7273-40c1-b7db-39efd9ccc17a;MSIP_Label_6b558183-044c-4105-8d9c-cea02a2a3d86_SetDate=2019-11-07T04:11:42.0477204Z;MSIP_Label_6b558183-044c-4105-8d9c-cea02a2a3d86_Name=Unrestricted;MSIP_Label_6b558183-044c-4105-8d9c-cea02a2a3d86_ContentBits=11;MSIP_Label_6b558183-044c-4105-8d9c-cea02a2a3d86_Method=Unknown 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_734D49CCEBEEF84792F5B80ED585239D5C352F7CSHSMSX104ccrcor_" --_000_734D49CCEBEEF84792F5B80ED585239D5C352F7CSHSMSX104ccrcor_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable I treat the issue in this way: 1. Platform Boot Manager library does a good job. It doesn't always cal= l 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 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, Michae= l D Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeratio= n The issue is there are some auto created options we do not want on our pla= tform. Get Outlook for Android ________________________________ From: Ni, Ray > Sent: Wednesday, November 6, 2019 11:59:31 PM To: Jeff Brasen >; afish@app= le.com > Cc: devel@edk2.groups.io >; Ashish Singhal >; Laszlo Ersek >; Wang, Jian J >; Wu, Hao A >; Gao, Z= hichao >; Kinney, Micha= el D > Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeratio= n Jeff, RefreshAllBootOption() only modifies/creates the auto-created boot options= . 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 > 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, Michael D > Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeratio= n As the suggestions below made sense, we updated our platform boot manager = library to behave in this manner and for normal boots everything works well= . However the UiApp and boot maintenance applications in EDK2 also call Efi= BootManagerRefreshAllBootOption() when ever a user goes into the menu which= will re-create the skipped boot options with no place for the platform cod= e to intervene. What about a solution where we add a new Platform library function that al= lows for override of the behavior of BmEnumerateBootOptions? For example, e= ither a function or protocol that takes the same parameters as this functio= n and only if it returns NULL then we continue to the default enumeration c= ode. Or a function call inserted at the end that would modify the load opt= ion 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 >; Laszlo Ersek >; Wang, Jian J >; Wu, Hao A = >; Gao, Zhichao >; Mike= Kinney > Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeratio= n 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 guidanc= e 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 EfiBootManagerRefreshAl= lBootOption() only in full configuration boot path. The full configuration boot path is chosen when hardware changes happen. S= o 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<= mailto:jbrasen@nvidia.com> Cc: Ashish Singhal >; Laszlo Ersek >; Ni, Ray >; Wang, Jian J >; Wu, Hao A >; Gao, Zhichao >; Kinney, Michael D > Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeratio= n On Nov 5, 2019, at 7:34 PM, Jeff Brasen > wrote: Wouldn't having a variable that we create and delete on every boot put unn= ecessary stress on the SPI-NOR that the variable store lives on? What about the alternative approach where we allow the platform code to mo= dify the attributes of the auto created variable to disable it with hidden/= !active but still match for detection purposes so that it doesn't delete an= d 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 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 tha= t is not changing. The custom BDS could could only create the variable for this device if it = does not exist. [JB] The current flow with no changes in the boot manager would be as foll= ows 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 vari= able and update BootOrder 3. Platform code runs creates the options for the boot option it wants = 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 BmB= oot.c If you modify the variable to disable it with hidden/not active it would d= elete 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 attri= butes (at least allow for differences in active and hidden) then the when i= t 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 though= the patch changed BmEnumerateBootOptions(), so that is going to change how= EfiBootManagerRefreshAllBootOption() works. I'd also point out the patch a= s given is invalid as it changed the behavior of the public library API for= EfiBootManagerRefreshAllBootOption() [1] so for the patch to be valid it w= ould 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 sure t= he boot variables are in the correct state. That should happen before the B= oot Manager code runs, and it is not clear to me why the Boot Manager coul= d 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 varia= ble that Windows installs? If it works kind of the same way then the answer= is to have the BDS platform specific code write the boot variable. [1] /** The function creates boot options for all possible bootable medias in th= e following order: 1. Removable BlockIo - The boot option only points to the rem= ovable media device, like USB key, DVD, Floppy etc. 2. Fixed BlockIo - The boot option only points to a Fixed= blockIo device, like HardDisk. 3. Non-BlockIo SimpleFileSystem - The boot option points to a device sup= porting SimpleFileSystem Protocol, but not sup= porting BlockIo protocol. 4. LoadFile - The boot option points to the media su= pporting LoadFile protocol. Reference: UEFI Spec chapter 3.3 Boot Option Variables Default Boot Beha= vior 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 ma= y contain confidential information. Any unauthorized review, use, disclosu= re or distribution is prohibited. If you are not the intended recipient, p= lease contact the sender by reply email and destroy all copies of the origi= nal message. ________________________________ --_000_734D49CCEBEEF84792F5B80ED585239D5C352F7CSHSMSX104ccrcor_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

I treat the issue in this way:

  1. Platform Boot Manager library does a good job. It doesn’t alw= ays call RefreshAll() API to auto-create the boot options=
  2. But UiApp doesn’t. It constantly call RefreshAll().<= /o:p>

 

Do you think that we can fix UiApp instead? For ex= ample, introducing a PCD to control the boot option refresh behavior?<= /o:p>

 

Thanks,

Ray

 

From: devel@edk2.groups.io <deve= l@edk2.groups.io> On Behalf Of Jeff Brasen
Sent: Thursday, November 7, 2019 3:02 PM
To: Ni, Ray <ray.ni@intel.com>; afish@apple.com
Cc: devel@edk2.groups.io; Ashish Singhal <ashishsingha@nvidia.co= m>; Laszlo Ersek <lersek@redhat.com>; Wang, Jian J <jian.j.wang= @intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhic= hao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>=
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enu= meration

 

The issue= is there are some auto created options we do not want on our platform.

From: Ni, R= ay <ray.ni@intel.com>
Sent: Wednesday, November 6, 2019 11:59:31 PM
To: Jeff Brasen <jbrasen@n= vidia.com>; afish@apple.com <afish@apple.com>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>;= Ashish Singhal <ashishsingha= @nvidia.com>; Laszlo Ersek <= lersek@redhat.com>; Wang, Jian J <jian.j.wang@int= el.com>; Wu, Hao A <hao.a.w= u@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enu= meration

 

Jeff,

RefreshAllBootOption() only modifies/creates the a= uto-created boot options. For the boot options created by platform boot man= ager library, they stay with no one touches. And all auto-created boot opti= ons are appended in the end of boot option list (through BootOrder).

 

From: Jeff Brasen <jbrasen@nvidia.com>
Sent: Thursday, November 7, 2019 12:13 PM
To: afish@apple.com; Ni, Ray= <ray.ni@intel.com>
Cc: devel@edk2.groups.io; Ashish Singhal <ashishsing= ha@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.ga= o@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enu= meration

 

As the suggestions below made sense, we updated ou= r platform boot manager library to behave in this manner and for normal boo= ts everything works well. However the UiApp and boot maintenance applicatio= ns in EDK2 also call 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 that allows for override of the behavior of BmEnumerateBoo= tOptions? For example, either a function or protocol that takes the same pa= rameters as this function and only if it returns NULL then we continue to the default enumeration code. &nbs= p;Or a function call inserted at the end that would modify the load option = array after the system does the standard enumeration.

 

-Jeff

 

 

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 p= age to give some guidance on how to customize the BDS. 

 

Thanks,

 

Andrew Fish

 

On Nov 5, 2019, at 9:20 PM, Ni, Ray <ray.ni@intel.com> wrote:

 

Andrew,

I agree with your opinion.

It’s expected that Platform Boot Manager lib= calls EfiBootManagerRefreshAllBootOption() only in full configuration boot= path.

The full configuration boot path is chosen when ha= rdware changes happen. So it’s not expected EfiBootManagerRefreshR= 30;() 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 <afish@apple.com> 
Sent: Wednesday,= November 6, 2019 10:47 AM
To: devel@edk2.groups.io; jbrasen@nvidia.com
Cc: Ashish Singh= al <ashishsingha@nvidia.com>; Laszlo Ersek <lersek@redhat= .com>; Ni, Ray <ray.ni@intel.= com>; Wang, Jian J <jian.j.wang@int= el.com>; Wu, Hao A <hao.a.w= u@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [ed= k2-devel] [PATCH] Support skipping automatic BM enumeration

 

 

 

On Nov 5, 2019, at 7:34 PM, Jeff Brasen <jbrasen@nvidia= .com> wrote:

 


Wouldn't having a variable that we create and delete on every boot put unn= ecessary stress on the SPI-NOR that the variable store lives on?
What about the alternative approach where we allow the platform code to mo= dify the attributes of the auto created variable to disable it with hidden/= !active but still match for detection purposes so that it doesn't delete an= d 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 modified?<= span class=3D"xapple-converted-space"> 

I would assume the NOR driver is smart enough to not update a variable tha= t is not changing. 

The custom BDS could could only create the variable for this device if it = does not exist. 

[JB] The current flow with no changes in the boot manager would be as foll= ows

 

  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 variable and update BootOrder
  3. Platform code runs creates the options for the = boot option it wants 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-occu= r

 

The code that does this (1/2) is EfiBootManagerRefreshAllBo= otOption in BmBoot.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 reco= gnize that is the same boot option.

 

If however we modify EfiBootManagerFindLoadOption to not co= mpare the attributes (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 new variable in the store and thus we w= ouldn't have changes every boot.

 

 

Jeff,

 

Sorry if I'm a little off on the sequence of thing= s as the platform I work on day to day has a custom BDS and does not use th= is library..... I though the patch changed BmEnumerateBootOptions(), s= o that is going to change 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. <= /p>

 

I think Laszlo advocated having the BDS platform s= pecific code make sure the boot variables are in the correct state. That sh= ould happen before the Boot Manager code runs, and it is  not clear to= me why the Boot Manager 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 diffe= rent than the boot variable that Windows installs? If it works kind of the = same way then the answer is to have the BDS platform specific code write th= e boot variable. 

 

 

[1]

/**

  The function creates boot options for all p= ossible bootable medias in the following order:

  1. Removable BlockIo       &= nbsp;    - The boot option only points to the removable media

              &= nbsp;                    = device, like USB key, DVD, Floppy etc.

  2. Fixed BlockIo        = ;        - The boot option only points to a Fixed block= Io device,

              &= nbsp;                    = like HardDisk.

  3. Non-BlockIo SimpleFileSystem - The boot = option points to a device supporting

              &= nbsp;                    = SimpleFileSystem Protocol, but not supporting BlockIo

              &= nbsp;                    = protocol.

  4. LoadFile         &nb= sp;           - The boot option points to the medi= a supporting

              &= nbsp;                    = LoadFile protocol.

  Reference: UEFI Spec chapter 3.3 Boot Optio= n Variables Default Boot Behavior

 

  The function won't delete the boot option n= ot 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 in= tended recipient(s) and may contain confidential information.  Any una= uthorized review, use, disclosure or distribution is prohibited.  If you are not the intended recipient, please contact t= he sender by reply email and destroy all copies of the original message.

 

--_000_734D49CCEBEEF84792F5B80ED585239D5C352F7CSHSMSX104ccrcor_--