Re: [PATCH v2 3/5] MdeModulePkg/BootManagerUiLib: show inactive boot options

["Gao, Zhichao" <zhichao.gao@intel.com>]

> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Sent: Tuesday, June 2, 2020 5:27 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> Cc: jon@solid-run.com; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm
> <leif@nuviainc.com>; Ni, Ray <ray.ni@intel.com>
> Subject: Re: [PATCH v2 3/5] MdeModulePkg/BootManagerUiLib: show
> inactive boot options
> 
> 
> 
> On 6/2/20 11:11 AM, Gao, Zhichao wrote:
> > Hi Ard,
> >
> > Form the Uefi spec:
> > '''
> > If a load option is marked as LOAD_OPTION_ACTIVE, the boot manager
> > will attempt to boot automatically using the device path information
> > in the load option. This provides an easy way to disable or enable load
> options without needing to delete and re-add them.
> > '''
> > The ACTIVE flag seems to enable/disable the option.
> >
> 
> Sure. But this change does not affect what gets booted automatically, it
> changes what gets presented as available options in the UiApp boot manager
> menu.
> 
> To hide options from this menu, a different flag HIDDEN is provided. The UEFI
> spec clearly mentions that
> - the ACTIVE flag defines which boot options may be booted *automatically*
> - the HIDDEN flag defines which boot options are hidden from the menu
> which allows options to be booted *manually*
> 
> So preventing ACTIVE options from being booted manually is incorrect.
> That is what the patch fixes.

I don't see the boot function prevent no-ACTIVE option to boot. So I agree with your point.

> 
> 
> > '''
> > Boot#### load options with LOAD_OPTION_CATEGORY set to
> > LOAD_OPTION_CATEGORY_APP are executables which are not part of the
> > normal boot processing but can be optionally chosen for execution if
> > boot menu is provided, or via Hot Keys. See Section 3.1.6 for details '''
> > There is another flag to implement your function. The option with ACTIVE |
> CATEGORY | CATEGORY_APP should show in the boot menu but not in the
> normal boot flow.
> >
> 
> But this prevents me from setting BootNext to the Shell option, and boot it
> automatically on the next boot, right?

Sorry, I miss the requirement of next boot. Next boot would called by EfiBootManagerBoot.
And the APP flag only affect BmSetMemoryTypeInformationVariable result. I don't think it would affect the next boot running.

Anyway, I agree with your patch and thanks for your explain.

Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao