From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from foss.arm.com (foss.arm.com [217.140.110.172])
 by mx.groups.io with SMTP id smtpd.web11.6281.1591086347786967068
 for <devel@edk2.groups.io>;
 Tue, 02 Jun 2020 01:25:48 -0700
Authentication-Results: mx.groups.io;
 dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com)
Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14])
	by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id AA2ED1FB;
	Tue,  2 Jun 2020 01:25:46 -0700 (PDT)
Received: from [192.168.1.69] (unknown [10.37.8.209])
	by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 666443F305;
	Tue,  2 Jun 2020 01:25:45 -0700 (PDT)
Subject: Re: [PATCH v2 3/5] MdeModulePkg/BootManagerUiLib: show inactive boot options
To: devel@edk2.groups.io
Cc: jon@solid-run.com, Laszlo Ersek <lersek@redhat.com>,
 Leif Lindholm <leif@nuviainc.com>, Ray Ni <ray.ni@intel.com>,
 Zhichao Gao <zhichao.gao@intel.com>
References: <20200528091741.14610-1-ard.biesheuvel@arm.com>
 <20200528091741.14610-4-ard.biesheuvel@arm.com>
From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
Message-ID: <bc7c985d-7b02-a864-9376-455e157e9f5c@arm.com>
Date: Tue, 2 Jun 2020 10:25:43 +0200
User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:68.0) Gecko/20100101
 Thunderbird/68.8.1
MIME-Version: 1.0
In-Reply-To: <20200528091741.14610-4-ard.biesheuvel@arm.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit

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