Re: [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy

["Yao, Jiewen" <jiewen.yao@intel.com>]

Thanks. The patch loos good to me.

Would you please share with us, how you validate the patch?



> -----Original Message-----
> From: Liu, Linus <linus.liu@intel.com>
> Sent: Tuesday, April 11, 2023 5:55 PM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; FST-FIR-PRC <fst-fir-
> prc@intel.com>; FST FIR Server <fst.fir.server@intel.com>; Chu, Maggie
> <maggie.chu@intel.com>
> Subject: [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to
> use Variable Policy
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4408
> 
> Change-Id: I3c4b466ef318766d6d70c9f73e36b94b5f10832c
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: FST-FIR-PRC <fst-fir-prc@intel.com>
> Cc: FST FIR Server C <fst.fir.server@intel.com>
> Cc: Maggie Chu <maggie.chu@intel.com>
> Signed-off-by: Linus Liu <linus.liu@intel.com>
> ---
>  SecurityPkg/HddPassword/HddPasswordDxe.c   | 16 +++++++++++-----
>  SecurityPkg/HddPassword/HddPasswordDxe.h   |  1 -
>  SecurityPkg/HddPassword/HddPasswordDxe.inf |  3 ++-
>  SecurityPkg/SecurityPkg.dsc                |  1 +
>  4 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.c
> b/SecurityPkg/HddPassword/HddPasswordDxe.c
> index a1a63b67a4..c20fdbe83f 100644
> --- a/SecurityPkg/HddPassword/HddPasswordDxe.c
> +++ b/SecurityPkg/HddPassword/HddPasswordDxe.c
> @@ -9,6 +9,7 @@
>  **/
> 
> 
> 
>  #include "HddPasswordDxe.h"
> 
> +#include <Library/VariablePolicyHelperLib.h>
> 
> 
> 
>  EFI_GUID    mHddPasswordVendorGuid          =
> HDD_PASSWORD_CONFIG_GUID;
> 
>  CHAR16      mHddPasswordVendorStorageName[] =
> L"HDD_PASSWORD_CONFIG";
> 
> @@ -2822,7 +2823,7 @@ HddPasswordDxeInit (
>    HDD_PASSWORD_DXE_PRIVATE_DATA  *Private;
> 
>    VOID                           *Registration;
> 
>    EFI_EVENT                      EndOfDxeEvent;
> 
> -  EDKII_VARIABLE_LOCK_PROTOCOL   *VariableLock;
> 
> +  EDKII_VARIABLE_POLICY_PROTOCOL *VariablePolicy;
> 
> 
> 
>    Private = NULL;
> 
> 
> 
> @@ -2858,12 +2859,17 @@ HddPasswordDxeInit (
>    //
> 
>    // Make HDD_PASSWORD_VARIABLE_NAME variable read-only.
> 
>    //
> 
> -  Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL,
> (VOID **)&VariableLock);
> 
> +  Status = gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid, NULL,
> (VOID **)&VariablePolicy);
> 
>    if (!EFI_ERROR (Status)) {
> 
> -    Status = VariableLock->RequestToLock (
> 
> -                             VariableLock,
> 
> +    Status = RegisterBasicVariablePolicy (
> 
> +                             VariablePolicy,
> 
> +                             &mHddPasswordVendorGuid,
> 
>                               HDD_PASSWORD_VARIABLE_NAME,
> 
> -                             &mHddPasswordVendorGuid
> 
> +                             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
> 
>                               );
> 
>      DEBUG ((DEBUG_INFO, "%a(): Lock %s variable (%r)\n", __FUNCTION__,
> HDD_PASSWORD_VARIABLE_NAME, Status));
> 
>      ASSERT_EFI_ERROR (Status);
> 
> diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.h
> b/SecurityPkg/HddPassword/HddPasswordDxe.h
> index 231533e737..049a208794 100644
> --- a/SecurityPkg/HddPassword/HddPasswordDxe.h
> +++ b/SecurityPkg/HddPassword/HddPasswordDxe.h
> @@ -17,7 +17,6 @@
>  #include <Protocol/AtaPassThru.h>
> 
>  #include <Protocol/PciIo.h>
> 
>  #include <Protocol/HiiConfigAccess.h>
> 
> -#include <Protocol/VariableLock.h>
> 
> 
> 
>  #include <Guid/MdeModuleHii.h>
> 
>  #include <Guid/EventGroup.h>
> 
> diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.inf
> b/SecurityPkg/HddPassword/HddPasswordDxe.inf
> index 06e8755ffc..2c0ebbcc78 100644
> --- a/SecurityPkg/HddPassword/HddPasswordDxe.inf
> +++ b/SecurityPkg/HddPassword/HddPasswordDxe.inf
> @@ -50,6 +50,7 @@
>    PrintLib
> 
>    UefiLib
> 
>    LockBoxLib
> 
> +  VariablePolicyHelperLib
> 
>    S3BootScriptLib
> 
>    PciLib
> 
>    BaseCryptLib
> 
> @@ -63,7 +64,7 @@
>    gEfiHiiConfigAccessProtocolGuid               ## PRODUCES
> 
>    gEfiAtaPassThruProtocolGuid                   ## CONSUMES
> 
>    gEfiPciIoProtocolGuid                         ## CONSUMES
> 
> -  gEdkiiVariableLockProtocolGuid                ## CONSUMES
> 
> +  gEdkiiVariablePolicyProtocolGuid              ## CONSUMES
> 
> 
> 
>  [Pcd]
> 
>    gEfiSecurityPkgTokenSpaceGuid.PcdSkipHddPasswordPrompt  ##
> CONSUMES
> 
> diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
> index 3bad5375c0..3c62205162 100644
> --- a/SecurityPkg/SecurityPkg.dsc
> +++ b/SecurityPkg/SecurityPkg.dsc
> @@ -74,6 +74,7 @@
> 
> PlatformPKProtectionLib|SecurityPkg/Library/PlatformPKProtectionLibVarPo
> licy/PlatformPKProtectionLibVarPolicy.inf
> 
> 
> SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariablePro
> visionLib/SecureBootVariableProvisionLib.inf
> 
>    TdxLib|MdePkg/Library/TdxLib/TdxLib.inf
> 
> +
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Var
> iablePolicyHelperLib.inf
> 
> 
> 
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> 
>    #
> 
> --
> 2.33.1.windows.1