1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH v2 0/5] ArmPkg/PlatformBootManagerLib: play nice without ConnectAll()
  4. View patch

[edk2-devel] [PATCH v2 3/5] MdeModulePkg/BootManagerUiLib: show inactive boot options

Ard Biesheuvel posted 5 patches 5 years, 8 months ago
[edk2-devel] [PATCH v2 3/5] MdeModulePkg/BootManagerUiLib: show inactive boot options
Posted by Ard Biesheuvel 5 years, 8 months ago 
UEFI boot options may exist but have the LOAD_OPTION_ACTIVE flag
cleared. This means that the boot option should not be selected
by default, but it does not mean it should be omitted from the
boot selection presented by the boot manager: for this purpose,
another flag LOAD_OPTION_HIDDEN exists.

Given that the latter flag exists solely for the purpose of omitting
boot options from the boot selection menu, and LOAD_OPTION_XXX flags
can be combined if desired, hiding inactive boot options as well is
a mistake, and violates the intent of paragraph 3.1.3 of the UEFI
specification (revision 2.8 errata A). Let's fix this by dropping
the LOAD_OPTION_ACTIVE check from the code that populates the boot
selection menu.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 MdeModulePkg/Library/BootManagerUiLib/BootManager.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/BootManagerUiLib/BootManager.c b/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
index 13b40e11b396..4b2c4c77a124 100644
--- a/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
+++ b/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
@@ -535,9 +535,9 @@ UpdateBootManager (
     mKeyInput++;
 
     //
-    // Don't display the hidden/inactive boot option
+    // Don't display hidden boot options, but retain inactive ones.
     //
-    if (((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0) || ((BootOption[Index].Attributes & LOAD_OPTION_ACTIVE) == 0)) {
+    if ((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0) {
       continue;
     }
 
-- 
2.17.1


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

View/Reply Online (#60390): https://edk2.groups.io/g/devel/message/60390
Mute This Topic: https://groups.io/mt/74518313/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/5] MdeModulePkg/BootManagerUiLib: show inactive boot options
Posted by Leif Lindholm 5 years, 8 months ago 
On Thu, May 28, 2020 at 11:17:39 +0200, Ard Biesheuvel wrote:
> UEFI boot options may exist but have the LOAD_OPTION_ACTIVE flag
> cleared. This means that the boot option should not be selected
> by default, but it does not mean it should be omitted from the
> boot selection presented by the boot manager: for this purpose,
> another flag LOAD_OPTION_HIDDEN exists.
> 
> Given that the latter flag exists solely for the purpose of omitting
> boot options from the boot selection menu, and LOAD_OPTION_XXX flags
> can be combined if desired, hiding inactive boot options as well is
> a mistake, and violates the intent of paragraph 3.1.3 of the UEFI
> specification (revision 2.8 errata A). Let's fix this by dropping
> the LOAD_OPTION_ACTIVE check from the code that populates the boot
> selection menu.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Acked-by: Leif Lindholm <leif@nuviainc.com>

> ---
>  MdeModulePkg/Library/BootManagerUiLib/BootManager.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/BootManagerUiLib/BootManager.c b/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> index 13b40e11b396..4b2c4c77a124 100644
> --- a/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> +++ b/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> @@ -535,9 +535,9 @@ UpdateBootManager (
>      mKeyInput++;
>  
>      //
> -    // Don't display the hidden/inactive boot option
> +    // Don't display hidden boot options, but retain inactive ones.
>      //
> -    if (((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0) || ((BootOption[Index].Attributes & LOAD_OPTION_ACTIVE) == 0)) {
> +    if ((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0) {
>        continue;
>      }
>  
> -- 
> 2.17.1
> 

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

View/Reply Online (#60505): https://edk2.groups.io/g/devel/message/60505
Mute This Topic: https://groups.io/mt/74518313/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/5] MdeModulePkg/BootManagerUiLib: show inactive boot options
Posted by Ard Biesheuvel 5 years, 8 months ago 
Ray, Zhichao,

Do you have any comments on this patch?


On 5/28/20 11:17 AM, Ard Biesheuvel wrote:
> UEFI boot options may exist but have the LOAD_OPTION_ACTIVE flag
> cleared. This means that the boot option should not be selected
> by default, but it does not mean it should be omitted from the
> boot selection presented by the boot manager: for this purpose,
> another flag LOAD_OPTION_HIDDEN exists.
> 
> Given that the latter flag exists solely for the purpose of omitting
> boot options from the boot selection menu, and LOAD_OPTION_XXX flags
> can be combined if desired, hiding inactive boot options as well is
> a mistake, and violates the intent of paragraph 3.1.3 of the UEFI
> specification (revision 2.8 errata A). Let's fix this by dropping
> the LOAD_OPTION_ACTIVE check from the code that populates the boot
> selection menu.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>   MdeModulePkg/Library/BootManagerUiLib/BootManager.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/BootManagerUiLib/BootManager.c b/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> index 13b40e11b396..4b2c4c77a124 100644
> --- a/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> +++ b/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> @@ -535,9 +535,9 @@ UpdateBootManager (
>       mKeyInput++;
>   
>       //
> -    // Don't display the hidden/inactive boot option
> +    // Don't display hidden boot options, but retain inactive ones.
>       //
> -    if (((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0) || ((BootOption[Index].Attributes & LOAD_OPTION_ACTIVE) == 0)) {
> +    if ((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0) {
>         continue;
>       }
>   
> 


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

View/Reply Online (#60563): https://edk2.groups.io/g/devel/message/60563
Mute This Topic: https://groups.io/mt/74518313/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/5] MdeModulePkg/BootManagerUiLib: show inactive boot options
Posted by Gao, Zhichao 5 years, 8 months ago 
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.

'''
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.

Thanks,
Zhichao

> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Sent: Tuesday, June 2, 2020 4:26 PM
> To: 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>; Gao, Zhichao
> <zhichao.gao@intel.com>
> Subject: Re: [PATCH v2 3/5] MdeModulePkg/BootManagerUiLib: show
> inactive boot options
> 
> Ray, Zhichao,
> 
> Do you have any comments on this patch?
> 
> 
> On 5/28/20 11:17 AM, Ard Biesheuvel wrote:
> > UEFI boot options may exist but have the LOAD_OPTION_ACTIVE flag
> > cleared. This means that the boot option should not be selected by
> > default, but it does not mean it should be omitted from the boot
> > selection presented by the boot manager: for this purpose, another
> > flag LOAD_OPTION_HIDDEN exists.
> >
> > Given that the latter flag exists solely for the purpose of omitting
> > boot options from the boot selection menu, and LOAD_OPTION_XXX flags
> > can be combined if desired, hiding inactive boot options as well is a
> > mistake, and violates the intent of paragraph 3.1.3 of the UEFI
> > specification (revision 2.8 errata A). Let's fix this by dropping the
> > LOAD_OPTION_ACTIVE check from the code that populates the boot
> > selection menu.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > ---
> >   MdeModulePkg/Library/BootManagerUiLib/BootManager.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> > b/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> > index 13b40e11b396..4b2c4c77a124 100644
> > --- a/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> > +++ b/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> > @@ -535,9 +535,9 @@ UpdateBootManager (
> >       mKeyInput++;
> >
> >       //
> > -    // Don't display the hidden/inactive boot option
> > +    // Don't display hidden boot options, but retain inactive ones.
> >       //
> > -    if (((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0) ||
> ((BootOption[Index].Attributes & LOAD_OPTION_ACTIVE) == 0)) {
> > +    if ((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0) {
> >         continue;
> >       }
> >
> >


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

View/Reply Online (#60565): https://edk2.groups.io/g/devel/message/60565
Mute This Topic: https://groups.io/mt/74518313/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/5] MdeModulePkg/BootManagerUiLib: show inactive boot options
Posted by Ard Biesheuvel 5 years, 8 months ago 

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.


> '''
> 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?


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

View/Reply Online (#60567): https://edk2.groups.io/g/devel/message/60567
Mute This Topic: https://groups.io/mt/74518313/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/5] MdeModulePkg/BootManagerUiLib: show inactive boot options
Posted by Gao, Zhichao 5 years, 8 months ago 

> -----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


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

View/Reply Online (#60661): https://edk2.groups.io/g/devel/message/60661
Mute This Topic: https://groups.io/mt/74518313/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 3/5] MdeModulePkg/BootManagerUiLib: show inactive boot options
Posted by Laszlo Ersek 5 years, 8 months ago 
On 05/28/20 11:17, Ard Biesheuvel wrote:
> UEFI boot options may exist but have the LOAD_OPTION_ACTIVE flag
> cleared. This means that the boot option should not be selected
> by default, but it does not mean it should be omitted from the
> boot selection presented by the boot manager: for this purpose,
> another flag LOAD_OPTION_HIDDEN exists.
> 
> Given that the latter flag exists solely for the purpose of omitting
> boot options from the boot selection menu, and LOAD_OPTION_XXX flags
> can be combined if desired, hiding inactive boot options as well is
> a mistake, and violates the intent of paragraph 3.1.3 of the UEFI
> specification (revision 2.8 errata A). Let's fix this by dropping
> the LOAD_OPTION_ACTIVE check from the code that populates the boot
> selection menu.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>  MdeModulePkg/Library/BootManagerUiLib/BootManager.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/BootManagerUiLib/BootManager.c b/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> index 13b40e11b396..4b2c4c77a124 100644
> --- a/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> +++ b/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> @@ -535,9 +535,9 @@ UpdateBootManager (
>      mKeyInput++;
>  
>      //
> -    // Don't display the hidden/inactive boot option
> +    // Don't display hidden boot options, but retain inactive ones.
>      //
> -    if (((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0) || ((BootOption[Index].Attributes & LOAD_OPTION_ACTIVE) == 0)) {
> +    if ((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0) {
>        continue;
>      }
>  
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


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

View/Reply Online (#60410): https://edk2.groups.io/g/devel/message/60410
Mute This Topic: https://groups.io/mt/74518313/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.