Re: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol

["Wang, Sunny (HPS SW)" <sunnywang@hpe.com>]

Thanks for checking this, Ray. 

Platform may want to: 
 1. Refresh the static boot options (that are not created by BmEnumerateBootOptions) without a reboot. 
 2. Update some other static-informational data or configuration data right after calling EfiBootManagerRefreshAllBootOption. 
 3. Always skip calling EfiBootManagerRefreshAllBootOption for the cases like BOOT_ASSUMING_NO_CONFIGURATION_CHANGES.

I know these actions can be done by adding code to other places, but using hooks in EfiBootManagerRefreshAllBootOption would be an easier solution for the platform. We won't need to take care of all the EfiBootManagerRefreshAllBootOption callers. Therefore, If we don't have a concern about adding more hooks and want to give the platform more flexibility, we could add two more hooks (1 and 3) in the future to have three hooks as below: 
  1. BeginOfRefreshAllBootOptions
  2. RefreshAllBootOptions or RefreshEnumeratedBootOptions
  3. EndOfRefreshAllBootOption

By the way, the current change looks good enough to me. In case Ashish or others are in urgent need of this code change, we can discuss my comments later in a separated email.

Regards,
Sunny Wang

-----Original Message-----
From: Ni, Ray [mailto:ray.ni@intel.com] 
Sent: Tuesday, December 24, 2019 10:40 AM
To: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; devel@edk2.groups.io; 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>; 'Andrew Fish (afish@apple.com)' <afish@apple.com>
Cc: Spottswood, Jason <jason.spottswood@hpe.com>
Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol



> -----Original Message-----
> From: Wang, Sunny (HPS SW) <sunnywang@hpe.com>
> Sent: Friday, December 20, 2019 7:29 PM
> To: devel@edk2.groups.io; ashishsingha@nvidia.com; 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>; Kinney, 
> Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)'
> <afish@apple.com>
> Cc: Spottswood, Jason <jason.spottswood@hpe.com>; Wang, Sunny (HPS
> SW) <sunnywang@hpe.com>
> Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform 
> Boot Manager Protocol
> 
> Good point. The way you used is more robust. It can cover a mistake in 
> function's error handling. Thanks for clarifying this, Ashish.
> 
> In addition, the other naming suggestion just comes to mind. How about 
> we rename the function to a more generic one (based on location) like 
> AfterEnumerateBootOptions or a more specific one like 
> RefreshEnumeratedBootOptions? In the future, we may add the other hook 
> function in the EfiBootManagerRefreshAllBootOption to deal with the 
> boot options that are not created by BmEnumerateBootOptions. In this 
> case (two hook functions in EfiBootManagerRefreshAllBootOption), the 
> original function name "RefreshAllBootOptions" may cause some confusion.

Sunny,
What else feasibility do you think platform may require in future but this RefreshAllBootOptions cannot support?

Thanks,
Ray