Re: [edk2-discuss] [edk2-devel] [PATCH] Support skipping automatic BM enumeration

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

Sunny,
Thanks for your comments.

Thanks,
Ray

> -----Original Message-----
> From: Wang, Sunny (HPS SW) <sunnywang@hpe.com>
> Sent: Wednesday, December 18, 2019 11:54 AM
> To: discuss@edk2.groups.io; ashishsingha@nvidia.com; Jeff Brasen <jbrasen@nvidia.com>; Ni, Ray <ray.ni@intel.com>;
> devel@edk2.groups.io; Laszlo Ersek <lersek@redhat.com>; afish@apple.com
> Cc: 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 <michael.d.kinney@intel.com>; Wang, Sunny (HPS SW) <sunnywang@hpe.com>
> Subject: RE: [edk2-discuss] [edk2-devel] [PATCH] Support skipping automatic BM enumeration
> 
> Sorry for the delay. Somehow I didn't catch the follow-up email. Thanks for checking my comments, Ray and Ashish.
> Yeah, agree. 2.b is better. I will review the code change.
> 
> Regards,
> Sunny Wang
> 
> -----Original Message-----
> From: discuss@edk2.groups.io [mailto:discuss@edk2.groups.io] On Behalf Of Ashish Singhal
> Sent: Wednesday, December 18, 2019 4:16 AM
> To: Jeff Brasen <jbrasen@nvidia.com>; Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Laszlo Ersek
> <lersek@redhat.com>; afish@apple.com; discuss@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>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-discuss] [edk2-devel] [PATCH] Support skipping automatic BM enumeration
> 
> I have submitted a patch based on 2.b as suggested by Ray. I am open to making changes in the structure of the protocol
> functions as well as the verbal description. Please let me know what you all think about it.
> 
> Thanks
> Ashish
> ________________________________
> From: Jeff Brasen <jbrasen@nvidia.com>
> Sent: Thursday, December 12, 2019 10:52 AM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com>;
> afish@apple.com <afish@apple.com>; discuss@edk2.groups.io <discuss@edk2.groups.io>
> Cc: Ashish Singhal <ashishsingha@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 <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration
> 
> Thanks for the summary Ray, for the problem summary only thing I would add would be that platform also wants to
> create/modify boot options when full enumeration is requested as well.
> 
> For solutions I prefer option 2 as we don't have to put the same logic everywhere of how to modify the default
> enumerated list. And if we do that 2b makes more sense as then we don't have to modify all of the existing platforms.
> 
> I see two things the platform would need to do.
> 
>   1.  Update list created in BmEnumerateBootOptions
>   2.  Delete any no longer valid platform created boot options
> 
> 
> Thanks,
> 
> Jeff
> 
> ________________________________
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Wednesday, December 11, 2019 7:00 AM
> To: Jeff Brasen <jbrasen@nvidia.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com>;
> afish@apple.com <afish@apple.com>; discuss@edk2.groups.io <discuss@edk2.groups.io>
> Cc: Ashish Singhal <ashishsingha@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 <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration
> 
> External email: Use caution opening links or attachments
> 
> 
> Jeff,
> 
> Tom from AMD booked the meeting for SEV discussion months ago. I am afraid there is no time for this discussion.
> 
> Let's try to resolve it in mails.
> 
> 
> 
> Firstly, let me rephase the problem and your proposed solutions here (subjective + verb + objective). Sunny's input is also
> included. Hope Mike K and others can provide inputs.
> 
> Personally, I agree with 2.b. It helps us to gradually migrate the PlatformBootManagerLib to PlatformBootManager
> protocol. Protocol with Revision field helps to reduce the impact to old platforms with new APIs added.
> 
> 
> 
> **Problem:
> 
>                Platform requires certain BlockIo/SimpleFileSystem/LoadFile instances don't cause Boot#### created. It's a need
> of platform customization.
> 
> 
> 
> **Details:
> 
>                Boot#### for BlockIo/SimpleFileSystem/LoadFile are created by API EfiBootMangerRefreshAllBootOptions(). There
> are 2 places that call this API:
> 
>   1.  Platform Boot Manager calls the API (usually in the full configuration boot path)
>   2.  UiApp calls the API when entering "Boot Manager" page and "Boot Maintenance Manager" page.
> 
> 
> 
> Platform can change Platform Boot Manager to remove the unneeded Boot#### in case #1.
> 
> But platform has no way to remove the Boot#### created in case #2 .
> 
> 
> 
> **Potential solutions:
> 
>   1.  Update UiApp
>      *   Define a new PCD and a new event group.
> 
> If PCD is TRUE, UiApp signals the event. Event callback creates the Boot####. Otherwise,
> EfiBootManagerRefreshAllBootOptions() is called.
> 
>      *   Add a new PlatformBootManagerLib API (implemented by platform).
> 
> UiApp calls the new API instead of EfiBootManagerRefreshAllBootOption. (need to coordinate rollout with updates to all
> platforms).
> 
>      *   Add a new protocol (implemented by platform).
> 
> UiApp calls the new protocol if it exists otherwise calls EfiBootManagerRefreshAllBootOption.
> 
> 
> 
>   1.  Update EfiBootManagerRefreshAllBootOptions()
>      *   Add a new library class (implemented by platform).
> 
> EfiBootManagerRefreshAllBootOption() calls the new library class.
> 
>      *   Add a new PlatformBootManager protocol (implemented by platform).
> 
> EfiBootManagerRefreshAllBootOption() calls the new protocol if it exists.
> 
> 
> 
> Thanks,
> 
> Ray
> 
> 
> 
> From: Jeff Brasen <jbrasen@nvidia.com>
> Sent: Wednesday, December 11, 2019 4:46 AM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Laszlo Ersek <lersek@redhat.com>; afish@apple.com;
> discuss@edk2.groups.io
> Cc: Ashish Singhal <ashishsingha@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 <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration
> 
> 
> 
> Can we discuss this at the design meeting this week (12/12)?
> 
> 
> 
> Thanks,
> 
> Jeff
> 
> ________________________________
> 
> From: Jeff Brasen
> Sent: Thursday, November 14, 2019 10:04 AM
> To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>;
> afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
> Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.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>>;
> Gao, Zhichao <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 enumeration
> 
> 
> 
> Yes, I think that would be good.
> 
> 
> 
> Summarizing everything in this thread
> 
> 
> 
> Problem: Platform needs to customize the boot options, this can be done for normal boot but the UiApp calls
> EfiBootManagerRefreshAllBootOption in a couple places.
> 
> 
> 
> Potential solutions:
> 
> 1 - Define new PCD and event group if PCD is set true then signal event instead of calling
> EfiBootManagerRefreshAllBootOption in UiApp
> 
> 2 - Add new function to boot manager library and replace call to EfiBootManagerRefreshAllBootOption in UiApp (need to
> coordinate rollout with updates to all platform.
> 
> 3 - Add new protocol with new function, if supported call this otherwise call EfiBootManagerRefreshAllBootOption as is
> done now
> 
> 4 - For 2/3 use  generic function so we don't need new APIs for future expansion
> 
> 5 - Update EfiBootManagerRefreshAllBootOption to call platform specific function.
> 
> 
> 
> Thanks,
> Jeff
> 
> 
> 
> 
> 
> From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Sent: Wednesday, November 13, 2019 7:09 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>;
> Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com>
> Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.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>>;
> Gao, Zhichao <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 enumeration
> 
> 
> 
> Jeff,
> 
> I think it's a good topic that we could discuss in the open design meeting.
> 
> Are you ok to present the problem you have and discuss the potential solutions in that meeting?
> 
> https://github.com/tianocore/tianocore.github.io/wiki/Design-Meeting
> 
> 
> 
> Thanks,
> 
> Ray
> 
> 
> 
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On
> Behalf Of Jeff Brasen
> Sent: Thursday, November 14, 2019 2:43 AM
> To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Laszlo
> Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com>
> Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.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>>;
> Gao, Zhichao <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 enumeration
> 
> 
> 
> Thinking about this more I think we could do this with a PCD and a new group event without having to define any new
> function interfaces.
> 
> 
> 
> We could change UiApp and BootManagerMenuApp (and any others that are relevant) from
> 
> 
> 
> EfiBootManagerRefreshAllBootOption ();
> 
> 
> 
> to
> 
> 
> 
> if (FeaturePcdGet (PcdEventBasedRefreshAllBootOptionSupport) {
> 
>   EFI_EVENT Event;
> 
>   gBS->CreateEventEx ( 0, 0, NULL, NULL, &gEventBasedRefreshGuid, &Event );
> 
>   gBS->SignalEvent (Event);
> 
>   gBS->CloseEvent (Event);
> 
> } else {
> 
>   EfiBootManagerRefreshAllBootOption ();
> 
> }
> 
> 
> 
> Then a platform that wants to do this on its own would just set this pcd and create a group event and do what it needs to
> do there.
> 
> 
> 
> Thanks,
> 
> Jeff
> 
> ________________________________
> 
> From: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>
> Sent: Monday, November 11, 2019 5:00 PM
> To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>;
> afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
> Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.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>>;
> Gao, Zhichao <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 enumeration
> 
> 
> 
> I am not sure a PCD would work (unless I am missing something) We do want to do a connect all and re-enumerate in
> UiApp but we need the platform code to be involved in that process.
> 
> 
> 
> Thanks,
> 
> Jeff
> 
> ________________________________
> 
> From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Sent: Monday, November 11, 2019 4:58 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Jeff
> Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Laszlo Ersek
> <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com>
> <afish@apple.com<mailto:afish@apple.com>>
> Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.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>>;
> Gao, Zhichao <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 enumeration
> 
> 
> 
> 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
> PlatformBootManagerLib.
> 
> 
> 
> Thanks,
> 
> Ray
> 
> 
> 
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On
> Behalf Of Jeff Brasen
> Sent: Tuesday, November 12, 2019 6:58 AM
> To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>;
> devel@edk2.groups.io<mailto:devel@edk2.groups.io>; afish@apple.com<mailto:afish@apple.com>
> Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.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>>;
> Gao, Zhichao <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 enumeration
> 
> 
> 
> 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.
> 
> 
> 
> 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 disruption on new customization hooks. Something like
> 
> 
> 
> EFI_STATUS PlatformBootNotify (CONST EFI_GUID *NotificationType, VOID *ContextData OPTIONAL)
> 
> 
> 
> Where Notification type describes where we are that we want platform to potentially handle and ContextData is per type
> caller allocated data that provides additional in/out data. This has the same issue of leaving the current default behavior
> in place for unsupported types as well as being a less than specific function to describe.
> 
> 
> 
> Thanks,
> 
> Jeff
> 
> 
> 
> ________________________________
> 
> From: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Sent: Friday, November 8, 2019 9:37 AM
> To: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>;
> devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>;
> afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
> Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.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>>;
> Gao, Zhichao <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 enumeration
> 
> 
> 
> 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 intended to do a full enumeration of the system and there may be platform specifics 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 time:
> 
> - 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://urldefense.proofpoint.com/v2/url?u=https-3A__bugzilla.tianocore.org_show-5Fbug.cgi-3Fid-3D982&d=DwIF-
> g&c=C5b8zRQO1miGmBeVZ2LFWg&r=Z9cLgEMdGZCI1_R0bW6KqOAGzCXLJUR24f8N3205AYw&m=EzQH7xR5u0kejdb3Oa18jq
> GGrV5AO_ROvd_Y_ajQQMk&s=cTZCJVE04A0qq4imb3Pma8jdJIrunDornyDXBty-1Uk&e=
> 
> Thanks,
> Laszlo
> 
> > From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> > Sent: Thursday, November 7, 2019 12:21 AM
> > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Jeff Brasen
> > <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>;
> > afish@apple.com<mailto:afish@apple.com>
> > Cc: 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>>; 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>>; Kinney, Michael
> > D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> > Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM
> > enumeration
> >
> > I treat the issue in this way:
> >
> >   1.  Platform Boot Manager library does a good job. It 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<mailto:devel@edk2.gro
> > ups.io%3cmailto:devel@edk2.groups.io>>
> > <devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gr
> > oups.io%3cmailto: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<mailto:ray.ni@intel.com%3cma
> > ilto:ray.ni@intel.com>>>; afish@apple.com<mailto:afish@apple.com>
> > Cc:
> > devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gro
> > ups.io%3cmailto:devel@edk2.groups.io>>; Ashish Singhal
> > <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishs
> > ingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek
> > <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3
> > cmailto:lersek@redhat.com>>>; Wang, Jian J
> > <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang
> > @intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A
> > <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.co
> > m%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao
> > <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao
> > @intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D
> > <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:m
> > ichael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
> > Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM
> > enumeration
> >
> > The issue is there are some auto created options we do not want on our platform.
> > Get Outlook for
> > Android<https://urldefense.proofpoint.com/v2/url?u=https-3A__aka.ms_gh
> > ei36&d=DwIF-g&c=C5b8zRQO1miGmBeVZ2LFWg&r=Z9cLgEMdGZCI1_R0bW6KqOAGzCXLJ
> > UR24f8N3205AYw&m=EzQH7xR5u0kejdb3Oa18jqGGrV5AO_ROvd_Y_ajQQMk&s=yaH1ZcO
> > LL9iVBCGMBGKtuwgPVbblPxtJooJMnpxn8P0&e= >
> >
> > ________________________________
> > From: Ni, Ray
> > <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cma
> > ilto:ray.ni@intel.com>>>
> > Sent: Wednesday, November 6, 2019 11:59:31 PM
> > To: Jeff Brasen
> > <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.co
> > m%3cmailto:jbrasen@nvidia.com>>>;
> > afish@apple.com<mailto:afish@apple.com>
> > <afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailt
> > o:afish@apple.com>>>
> > Cc:
> > devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gro
> > ups.io%3cmailto:devel@edk2.groups.io>>
> > <devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gr
> > oups.io%3cmailto:devel@edk2.groups.io>>>; Ashish Singhal
> > <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishs
> > ingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek
> > <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3
> > cmailto:lersek@redhat.com>>>; Wang, Jian J
> > <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang
> > @intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A
> > <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.co
> > m%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao
> > <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao
> > @intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D
> > <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:m
> > ichael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
> > Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM
> > enumeration
> >
> >
> > 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
> > <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.co
> > m%3cmailto:jbrasen@nvidia.com>>>
> > Sent: Thursday, November 7, 2019 12:13 PM
> > To: afish@apple.com<mailto:afish@apple.com>; Ni, Ray
> > <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cma
> > ilto:ray.ni@intel.com>>>
> > Cc:
> > devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gro
> > ups.io%3cmailto:devel@edk2.groups.io>>; Ashish Singhal
> > <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishs
> > ingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek
> > <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3
> > cmailto:lersek@redhat.com>>>; Wang, Jian J
> > <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang
> > @intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A
> > <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.co
> > m%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao
> > <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao
> > @intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D
> > <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:m
> > ichael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
> > Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM
> > enumeration
> >
> >
> >
> > 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
> 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
> BmEnumerateBootOptions? For example, either a function or protocol that takes the same parameters as this function
> 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 option array after the system does the standard enumeration.
> >
> >
> >
> > -Jeff
> >
> >
> >
> > From: afish@apple.com<mailto:afish@apple.com>
> > <afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailt
> > o:afish@apple.com>>>
> > Sent: Wednesday, November 6, 2019 9:20 AM
> > To: Ni, Ray
> > <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cma
> > ilto:ray.ni@intel.com>>>
> > Cc:
> > devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gro
> > ups.io%3cmailto:devel@edk2.groups.io>>; Jeff Brasen
> > <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.co
> > m%3cmailto:jbrasen@nvidia.com>>>; Ashish Singhal
> > <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishs
> > ingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek
> > <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3
> > cmailto:lersek@redhat.com>>>; Wang, Jian J
> > <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang
> > @intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A
> > <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.co
> > m%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao
> > <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao
> > @intel.com%3cmailto:zhichao.gao@intel.com>>>; Mike Kinney
> > <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:m
> > ichael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
> > Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM
> > enumeration
> >
> >
> >
> > 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 guidance 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.com<mailto:ray.ni@intel.com%3cmailto: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 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<mailto:afish@apple.com>
> > <afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailt
> > o:afish@apple.com>>>
> > Sent: Wednesday, November 6, 2019 10:47 AM
> > To:
> > devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gro
> > ups.io%3cmailto:devel@edk2.groups.io>>;
> > jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>
> > Cc: Ashish Singhal
> > <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishs
> > ingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek
> > <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3
> > cmailto:lersek@redhat.com>>>; Ni, Ray
> > <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cma
> > ilto:ray.ni@intel.com>>>; Wang, Jian J
> > <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang
> > @intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A
> > <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.co
> > m%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao
> > <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao
> > @intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D
> > <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:m
> > ichael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
> > Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM
> > enumeration
> >
> >
> >
> >
> >
> >
> >
> > On Nov 5, 2019, at 7:34 PM, Jeff Brasen
> <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>> wrote:
> >
> >
> >
> > Wouldn't having a variable that we create and delete on every boot put 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 hidden/!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 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 that 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
> > follows
> >
> >
> >
> >   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-occur
> >
> >
> >
> > The code that does this (1/2) is EfiBootManagerRefreshAllBootOption 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 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 when 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 though the patch changed BmEnumerateBootOptions(), so 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.
> >
> >
> >
> > 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 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 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 the following order:
> >
> >   1. Removable BlockIo            - The boot option only points to the removable 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
> > supporting
> >
> >                                     SimpleFileSystem Protocol, but not
> > supporting 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
> > Behavior
> >
> >
> >
> >   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, disclosure or distribution is prohibited.  If you are not the intended recipient, please contact the
> sender by reply email and destroy all copies of the original message.
> >
> > ________________________________
> >
> > >
> 
> 
> 
> 
>