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...@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:
> Delete the auto-created option pointing to LoadFile instance
> Create your own one with customized description.
>
>
> From: af...@apple.com <af...@apple.com> 
> Sent: Wednesday, November 6, 2019 10:47 AM
> To: devel@edk2.groups.io; jbra...@nvidia.com
> Cc: Ashish Singhal <ashishsin...@nvidia.com>; Laszlo Ersek 
> <ler...@redhat.com>; Ni, Ray <ray...@intel.com>; Wang, Jian J 
> <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com>; Gao, Zhichao 
> <zhichao....@intel.com>; Kinney, Michael D <michael.d.kin...@intel.com>
> Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration
>
>
> 
> 
> On Nov 5, 2019, at 7:34 PM, Jeff Brasen <jbra...@nvidia.com 
> <mailto:jbra...@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
>
> 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 
> 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.
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#50069): https://edk2.groups.io/g/devel/message/50069
Mute This Topic: https://groups.io/mt/39747302/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to