From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from nwk-aaemail-lapp01.apple.com (nwk-aaemail-lapp01.apple.com [17.151.62.66]) by mx.groups.io with SMTP id smtpd.web09.10729.1573057182031234705 for ; Wed, 06 Nov 2019 08:19:42 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=HBlLvd99; spf=pass (domain: apple.com, ip: 17.151.62.66, mailfrom: afish@apple.com) Received: from pps.filterd (nwk-aaemail-lapp01.apple.com [127.0.0.1]) by nwk-aaemail-lapp01.apple.com (8.16.0.27/8.16.0.27) with SMTP id xA6GH79h030473; Wed, 6 Nov 2019 08:19:40 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=sender : from : message-id : content-type : mime-version : subject : date : in-reply-to : cc : to : references; s=20180706; bh=EPv0yqNEOZCCGoALqW5rZpOaoiFsk/aFqZOOOa2+qWw=; b=HBlLvd99i4uJFxDTcVL4TZw8PY++H1FKNNG0ValbuvomhcdgGtmgXAycajT2APgjYt/r GrA108b8alt4kvGcmRK1H3oYAVHRnBKsmHNjNar+7ps43KB73CXDOB6eIz9gkXdtgZE1 HosviMAu71CJb0V+he6SvDeqbOOF9oJccYyCeYmSAKb+LBEhmJpbTcBhDWPTyT4TPxuF GqKVA5LKmnFOtT97SzJFYl+wOhKfSDK2gZLYUUUYKx1vjUl6RqMlo1oTJ8cnl+GzMxA+ yTcQkVXTP6wn6envky6I3vDZ8Tzbb62lmQSjIdOzCYW/VR7LiUd+cEeKBvxstWO8yWLF qw== Received: from ma-mailsvcp-mta-lapp02.corp.apple.com (ma-mailsvcp-mta-lapp02.corp.apple.com [10.226.18.134]) by nwk-aaemail-lapp01.apple.com with ESMTP id 2w18r9g5k2-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Wed, 06 Nov 2019 08:19:40 -0800 Received: from ma-mailsvcp-mmp-lapp04.apple.com (ma-mailsvcp-mmp-lapp04.apple.com [17.32.222.17]) by ma-mailsvcp-mta-lapp02.corp.apple.com (Oracle Communications Messaging Server 8.1.0.1.20190704 64bit (built Jul 4 2019)) with ESMTPS id <0Q0K00WSZ2OR9C10@ma-mailsvcp-mta-lapp02.corp.apple.com>; Wed, 06 Nov 2019 08:19:39 -0800 (PST) Received: from process_milters-daemon.ma-mailsvcp-mmp-lapp04.apple.com by ma-mailsvcp-mmp-lapp04.apple.com (Oracle Communications Messaging Server 8.1.0.1.20190704 64bit (built Jul 4 2019)) id <0Q0K00S002LUO700@ma-mailsvcp-mmp-lapp04.apple.com>; Wed, 06 Nov 2019 08:19:39 -0800 (PST) X-Va-A: X-Va-T-CD: b12cca5f282280a5682824b8a989b68b X-Va-E-CD: be490e393b9a0fc46442780b7330e634 X-Va-R-CD: 080c83ebb59fdc40182a21f863857514 X-Va-CD: 0 X-Va-ID: b27c8f6c-c181-48bd-b4de-7abf0c0ce65e X-V-A: X-V-T-CD: b12cca5f282280a5682824b8a989b68b X-V-E-CD: be490e393b9a0fc46442780b7330e634 X-V-R-CD: 080c83ebb59fdc40182a21f863857514 X-V-CD: 0 X-V-ID: 1b260d4d-3608-42ed-92c6-442858c55b7a X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-11-06_05:,, signatures=0 Received: from [17.103.44.172] (unknown [17.103.44.172]) by ma-mailsvcp-mmp-lapp04.apple.com (Oracle Communications Messaging Server 8.1.0.1.20190704 64bit (built Jul 4 2019)) with ESMTPSA id <0Q0K007B92OPJE10@ma-mailsvcp-mmp-lapp04.apple.com>; Wed, 06 Nov 2019 08:19:38 -0800 (PST) Sender: afish@apple.com From: "Andrew Fish" Message-id: MIME-version: 1.0 (Mac OS X Mail 13.0 \(3594.4.17\)) Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration Date: Wed, 06 Nov 2019 10:19:37 -0600 In-reply-to: <734D49CCEBEEF84792F5B80ED585239D5C35125E@SHSMSX104.ccr.corp.intel.com> Cc: "devel@edk2.groups.io" , "jbrasen@nvidia.com" , Ashish Singhal , Laszlo Ersek , "Wang, Jian J" , "Wu, Hao A" , "Gao, Zhichao" , Mike Kinney To: "Ni, Ray" 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> X-Mailer: Apple Mail (2.3594.4.17) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-11-06_05:,, signatures=0 Content-type: multipart/alternative; boundary="Apple-Mail=_DFEAD3A2-B5DF-4E6B-8F2A-7ECEF2A59549" --Apple-Mail=_DFEAD3A2-B5DF-4E6B-8F2A-7ECEF2A59549 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Ray, Is there an obvious hook point we could point Jeff and Ashish at?=20 Long term it would be a good idea to have a Wiki page to give some guidanc= e on how to customize the BDS.=20 Thanks, Andrew Fish > On Nov 5, 2019, at 9:20 PM, Ni, Ray wrote: >=20 > Andrew, > I agree with your opinion. > It=E2=80=99s expected that Platform Boot Manager lib calls EfiBootManage= rRefreshAllBootOption() only in full configuration boot path. > The full configuration boot path is chosen when hardware changes happen.= So it=E2=80=99s not expected EfiBootManagerRefresh=E2=80=A6() be > called in every boot. > So you could: > Delete the auto-created option pointing to LoadFile instance > Create your own one with customized description. > > > From: afish@apple.com =20 > Sent: Wednesday, November 6, 2019 10:47 AM > To: devel@edk2.groups.io; jbrasen@nvidia.com > Cc: Ashish Singhal ; Laszlo Ersek ; Ni, Ray ; Wang, Jian J ; W= u, Hao A ; Gao, Zhichao ; Kinney= , Michael D > Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumerat= ion > > >=20 >=20 > On Nov 5, 2019, at 7:34 PM, Jeff Brasen > wrote: > >=20 > 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? >=20 > What changes every boot that forces the variable to need to get modified= ?=20 >=20 > I would assume the NOR driver is smart enough to not update a variable t= hat is not changing.=20 >=20 > The custom BDS could could only create the variable for this device if i= t does not exist.=20 >=20 > [JB] The current flow with no changes in the boot manager would be as fo= llows > > Scan for instance of the boot option in the variables > It will not be found, so create a new boot option store it to a variable= and update BootOrder > Platform code runs creates the options for the boot option it wants and = writes those to variable store > 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.=20 > > 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.= =20 > > 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.=20 > > > [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 >=20 >=20 >=20 > Thanks, >=20 > Andrew Fish >=20 > Thanks, >=20 > Jeff >=20 >=20 > 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. >=20 --Apple-Mail=_DFEAD3A2-B5DF-4E6B-8F2A-7ECEF2A59549 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Ray,

Is there an obvious hook point we could poi= nt Jeff and Ashish at? 

Long term it would be a good idea to have a Wiki page to give s= ome guidance on how to customize the BDS. 

Thanks,

=
Andrew Fish

On Nov 5, 2019, at 9:20 PM, N= i, Ray <ray.ni@intel.com<= /a>> wrote:

Andrew,
I agree with your opinion.
It=E2=80=99s expected that Platform Boot Manager lib c= alls EfiBootManagerRefreshAllBootOption() only in full configuration boot p= ath.
The full config= uration boot path is chosen when hardware changes happen. So it=E2=80=99s n= ot expected EfiBootManagerRefresh=E2=80=A6() be
called in eve= ry boot.
So you coul= d:
  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: dev= el@edk2.groups.io; jbr= asen@nvidia.com
Cc: Ashish Singhal <ashishsingha@nvidia.com>; Laszlo Ersek = <lersek@redhat.com&g= t;; Ni, Ray <ray.ni@intel= .com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com&= gt;; Kinney, Michael D <michael.d.kinney@intel.com>
Sub= ject: Re: [edk2-devel= ] [PATCH] Support skipping automatic BM enumeration
 
 


On Nov 5, 2019, a= t 7:34 PM, Jeff Brasen <jbrasen@nvidia.com> wrote:
<= o:p class=3D""> 

Wouldn't having a variable that we cr= eate and delete on every boot put unnecessary stress on the SPI-NOR that th= e variable store lives on?
What about the alternative approac= h where we allow the platform code to modify the attributes of the auto cre= ated variable to disable it with hidden/!active but still match for detecti= on purposes so that it doesn't delete and recreate the modified variable ea= ch boot? That way all the logic on what to disable can still be in the plat= form 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 i= s smart enough to not update a variable that is not changing. 

The cust= om 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 follows
 
=
  1. Scan for instance of the boot option in the variables
  2. It will no= t be found, so create a new boot option store it to a variable and update B= ootOrder
  3. Platform code runs creates the options for the boot option it wants and w= rites those to variable store
    4. <= li class=3D"MsoNormal" style=3D"margin: 0in 0in 0.0001pt; font-size: 11pt; = font-family: Calibri, sans-serif;">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 BmBoot.c
 
<= div style=3D"margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibr= i, sans-serif;" class=3D"">If you modify the variable to disable it with hidden/not act= ive it would delete that and create a new one as well as the code wouldn't = recognize that is the same boot option.
 
 
 
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 th= e patch as given is invalid as it changed the behavior of the public librar= y API for EfiBootManagerRefreshAllBootOption() [1] so for the patch to be v= alid 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 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 Manag= er could would need to run if you have a valid EFI nvram variable to boot f= rom. 
I think the question is how is your use case differen= t than the boot variable that Windows installs? If it works kind of the sam= e way then the answer is to have the BDS platform specific code write the b= oot 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 removable= media
                  &nb= sp;                 device, like US= B key, DVD, Floppy etc.
  2. Fixed BlockIo       &nbs= p;        - The boot option only points to a Fixed bloc= kIo device,
                 = ;                   like HardD= isk.
  3. Non-BlockIo SimpleFileSystem - The boot option points to a= device supporting
               = ;                     Sim= pleFileSystem Protocol, but not supporting BlockIo
      &= nbsp;                    =         protocol.
  4. LoadFile     &= nbsp;               - The boot option po= ints to the media supporting
            &n= bsp;                     =   LoadFile protocol.
=
  Reference: UEFI Spec chapter 3.3 Boot Op= tion Variables Default Boot Behavior
 
  The function won't d= elete the boot option not added by itself.
**/
VOID
EFIAPI
EfiBoo= tManagerRefreshAllBootOption (
  VOID
<= /div>
  );<= /o:p>
 
Thank= s,
 
Andrew Fish


=


Thanks,

Andrew = Fish

Thanks,
Jeff

=

This email message is for the sole use of the intended rec= ipient(s) and may contain confidential information.  Any unauthorized = review, use, disclosure or distribution is prohibited.  If you are not= the intended recipient, please contact the sender by reply email and destr= oy all copies of the original message.

--Apple-Mail=_DFEAD3A2-B5DF-4E6B-8F2A-7ECEF2A59549--