Thanks,

Jeff

From: Laszlo Ersek <ler= sek@redhat.com>
Sent: Friday, November 8, 2019 9:37 AM
To: Jeff Brasen <jbrasen@nvidia.com>; Ni, Ray <ray.ni@inte= l.com>; devel@edk2.groups.io <devel@edk2.groups.io>; afish@apple.c= om <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

On 11/07/19 18:46, Jeff 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.com>
> Sent: Thursday, November 7, 2019 12:21 AM
> To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; afi= sh@apple.com
> Cc: Ashish Singhal <ashishsingha@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.com>; K= inney, 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> <dev= el@edk2.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@apple.com<mailto:afish@apple.co= m>
> Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ashish S= inghal <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>&= gt;; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Ga= o, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>= ;; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kin= ney@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&g= t;>; 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 E= rsek <lersek@redhat.com<mailto:lersek@redhat.com>>; Wang, Jian = 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>>; Ga= o, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>= ;; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kin= ney@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= >>
> 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.co= m>>
> Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ashish S= inghal <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>&= gt;; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Ga= o, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>= ;; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kin= ney@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 Bra= sen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Ashish Sin= ghal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>;= Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Wa= ng, Jian 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= , Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>= ; Mike Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@int= el.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:ra= y.ni@intel.com>> 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@= nvidia.com<mailto:jbrasen@nvidia.c= om>
> Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha= @nvidia.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@re= dhat.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com&g= t;>; Wang, Jian 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>>; Ga= o, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>= ;; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kin= ney@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<mai= lto:jbrasen@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.
>
> ________________________________
>
>=20 >