Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: Convert BmLoadOption to Variable Policy

["Rebecca Cran" <quic_rcran@quicinc.com>]

It's been a week since I sent this out: could someone review it please?


Thanks.
Rebecca Cran

On 1/19/22 14:01, Rebecca Cran wrote:
> Since the Variable Lock protocol is deprecated, convert locking of
> PlatformRecovery#### in EfiBootManagerLoadOptionToVariable to use the
> Variable Policy protocol.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
>
> Signed-off-by: Rebecca Cran <quic_rcran@quicinc.com>
> ---
>   MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c         | 33 +++++++++++++-------
>   MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h           |  1 -
>   MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf |  3 +-
>   3 files changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> index 32a9cbb425fa..2087f0b91df7 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> @@ -9,6 +9,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   
>   #include "InternalBm.h"
>   
> +#include <Library/VariablePolicyHelperLib.h>
> +
>   GLOBAL_REMOVE_IF_UNREFERENCED
>   CHAR16  *mBmLoadOptionName[] = {
>     L"Driver",
> @@ -169,15 +171,15 @@ EfiBootManagerLoadOptionToVariable (
>     IN CONST EFI_BOOT_MANAGER_LOAD_OPTION  *Option
>     )
>   {
> -  EFI_STATUS                    Status;
> -  UINTN                         VariableSize;
> -  UINT8                         *Variable;
> -  UINT8                         *Ptr;
> -  CHAR16                        OptionName[BM_OPTION_NAME_LEN];
> -  CHAR16                        *Description;
> -  CHAR16                        NullChar;
> -  EDKII_VARIABLE_LOCK_PROTOCOL  *VariableLock;
> -  UINT32                        VariableAttributes;
> +  EFI_STATUS                      Status;
> +  UINTN                           VariableSize;
> +  UINT8                           *Variable;
> +  UINT8                           *Ptr;
> +  CHAR16                          OptionName[BM_OPTION_NAME_LEN];
> +  CHAR16                          *Description;
> +  CHAR16                          NullChar;
> +  EDKII_VARIABLE_POLICY_PROTOCOL  *VariablePolicy;
> +  UINT32                          VariableAttributes;
>   
>     if ((Option->OptionNumber == LoadOptionNumberUnassigned) ||
>         (Option->FilePath == NULL) ||
> @@ -242,9 +244,18 @@ structure.
>       //
>       // Lock the PlatformRecovery####
>       //
> -    Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL, (VOID **)&VariableLock);
> +    Status = gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid, NULL, (VOID **)&VariablePolicy);
>       if (!EFI_ERROR (Status)) {
> -      Status = VariableLock->RequestToLock (VariableLock, OptionName, &gEfiGlobalVariableGuid);
> +      Status = RegisterBasicVariablePolicy (
> +                 VariablePolicy,
> +                 &gEfiGlobalVariableGuid,
> +                 OptionName,
> +                 VARIABLE_POLICY_NO_MIN_SIZE,
> +                 VARIABLE_POLICY_NO_MAX_SIZE,
> +                 VARIABLE_POLICY_NO_MUST_ATTR,
> +                 VARIABLE_POLICY_NO_CANT_ATTR,
> +                 VARIABLE_POLICY_TYPE_LOCK_NOW
> +                 );
>         ASSERT_EFI_ERROR (Status);
>       }
>   
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> index a9b0d485cace..b7dfe2a7e0bd 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> @@ -39,7 +39,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   #include <Protocol/BootLogo.h>
>   #include <Protocol/DriverHealth.h>
>   #include <Protocol/FormBrowser2.h>
> -#include <Protocol/VariableLock.h>
>   #include <Protocol/RamDisk.h>
>   #include <Protocol/DeferredImageLoad.h>
>   #include <Protocol/PlatformBootManager.h>
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> index cf5908692fa7..fe05d5f1cc9d 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> @@ -62,6 +62,7 @@
>     PerformanceLib
>     HiiLib
>     SortLib
> +  VariablePolicyHelperLib
>   
>   [Guids]
>     ## SOMETIMES_CONSUMES ## SystemTable (The identifier of memory type information type in system table)
> @@ -99,7 +100,7 @@
>     gEfiDevicePathProtocolGuid                    ## SOMETIMES_CONSUMES
>     gEfiBootLogoProtocolGuid                      ## SOMETIMES_CONSUMES
>     gEfiSimpleTextInputExProtocolGuid             ## SOMETIMES_CONSUMES
> -  gEdkiiVariableLockProtocolGuid                ## SOMETIMES_CONSUMES
> +  gEdkiiVariablePolicyProtocolGuid              ## SOMETIMES_CONSUMES
>     gEfiGraphicsOutputProtocolGuid                ## SOMETIMES_CONSUMES
>     gEfiUsbIoProtocolGuid                         ## SOMETIMES_CONSUMES
>     gEfiNvmExpressPassThruProtocolGuid            ## SOMETIMES_CONSUMES