Re: [edk2-devel][PATCH v2] FmpDevicePkg/FmpDxe: Use new Variable Lock interface

["Xu, Wei6" <wei6.xu@intel.com>]

Jie:
  Thanks for the update. Reviewed-by: Wei6 Xu <wei6.xu@intel.com>

BR,
Wei

-----Original Message-----
From: Yang, Jie <jie.yang@intel.com> 
Sent: Tuesday, October 19, 2021 11:11 AM
To: devel@edk2.groups.io
Cc: gaoliming@byosoft.com.cn; Kinney, Michael D <michael.d.kinney@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; Xu, Wei6 <wei6.xu@intel.com>; Yang, Jie <jie.yang@intel.com>
Subject: [edk2-devel][PATCH v2] FmpDevicePkg/FmpDxe: Use new Variable Lock interface

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3655
The code in FmpDevicePkg call the deprecated interface VariableLockRequestToLockc. So I changed the code in FmpDevicePkg using RegisterBasicVariablePolicy, instead of the deprecated interface.
Signed-off-by: Yang Jie <jie.yang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Wei6 Xu <wei6.xu@intel.com>
---
 FmpDevicePkg/FmpDevicePkg.dsc         |  1 +
 FmpDevicePkg/FmpDxe/FmpDxe.h          |  4 +-
 FmpDevicePkg/FmpDxe/FmpDxe.inf        |  5 +-
 FmpDevicePkg/FmpDxe/VariableSupport.c | 69 +++++++++++++--------------
 4 files changed, 39 insertions(+), 40 deletions(-)

diff --git a/FmpDevicePkg/FmpDevicePkg.dsc b/FmpDevicePkg/FmpDevicePkg.dsc index b420f52a08..7b1af285dd 100644
--- a/FmpDevicePkg/FmpDevicePkg.dsc
+++ b/FmpDevicePkg/FmpDevicePkg.dsc
@@ -53,6 +53,7 @@
   DebugLib|MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf   DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf+  VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf !ifdef CONTINUOUS_INTEGRATION   BaseCryptLib|CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf !elsediff --git a/FmpDevicePkg/FmpDxe/FmpDxe.h b/FmpDevicePkg/FmpDxe/FmpDxe.h
index 1177b1828e..4d94a925b6 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.h
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.h
@@ -4,7 +4,7 @@
   information provided through PCDs and libraries.    Copyright (c) Microsoft Corporation.<BR>-  Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>+  Copyright (c) 2018 - 2021, Intel Corporation. All rights reserved.<BR>    SPDX-License-Identifier: BSD-2-Clause-Patent @@ -33,11 +33,11 @@
 #include <Library/FmpDependencyDeviceLib.h> #include <Protocol/FirmwareManagement.h> #include <Protocol/FirmwareManagementProgress.h>-#include <Protocol/VariableLock.h> #include <Guid/SystemResourceTable.h> #include <Guid/EventGroup.h> #include <LastAttemptStatus.h> #include <FmpLastAttemptStatus.h>+#include <Library/VariablePolicyHelperLib.h>  #define VERSION_STRING_NOT_SUPPORTED  L"VERSION STRING NOT SUPPORTED" #define VERSION_STRING_NOT_AVAILABLE  L"VERSION STRING NOT AVAILABLE"diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.inf b/FmpDevicePkg/FmpDxe/FmpDxe.inf index eeb904a091..1c296388b0 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.inf
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.inf
@@ -4,7 +4,7 @@
 #  information provided through PCDs and libraries. # #  Copyright (c) 2016, Microsoft Corporation. All rights reserved.<BR>-#  Copyright (c) 2018 - 2020, Intel Corporation. All rights reserved.<BR>+#  Copyright (c) 2018 - 2021, Intel Corporation. All rights reserved.<BR> # #  SPDX-License-Identifier: BSD-2-Clause-Patent ##@@ -55,14 +55,15 @@
   FmpDependencyLib   FmpDependencyCheckLib   FmpDependencyDeviceLib+  VariablePolicyHelperLib  [Guids]   gEfiEndOfDxeEventGroupGuid  [Protocols]-  gEdkiiVariableLockProtocolGuid                ## CONSUMES   gEfiFirmwareManagementProtocolGuid            ## PRODUCES   gEdkiiFirmwareManagementProgressProtocolGuid  ## PRODUCES+  gEdkiiVariablePolicyProtocolGuid              ## CONSUMES  [Pcd]   gFmpDevicePkgTokenSpaceGuid.PcdFmpDeviceStorageAccessEnable              ## CONSUMESdiff --git a/FmpDevicePkg/FmpDxe/VariableSupport.c b/FmpDevicePkg/FmpDxe/VariableSupport.c
index 86dd5b203b..c4b72a2ff9 100644
--- a/FmpDevicePkg/FmpDxe/VariableSupport.c
+++ b/FmpDevicePkg/FmpDxe/VariableSupport.c
@@ -3,7 +3,7 @@
   firmware updates.    Copyright (c) 2016, Microsoft Corporation. All rights reserved.<BR>-  Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>+  Copyright (c) 2018 - 2021, Intel Corporation. All rights reserved.<BR>    SPDX-License-Identifier: BSD-2-Clause-Patent @@ -729,29 +729,30 @@ SetLastAttemptVersionInVariable (
 static EFI_STATUS LockFmpVariable (-  IN EFI_STATUS                    PreviousStatus,-  IN EDKII_VARIABLE_LOCK_PROTOCOL  *VariableLock,-  IN CHAR16                        *VariableName+  IN EFI_STATUS                      PreviousStatus,+  IN EDKII_VARIABLE_POLICY_PROTOCOL  *VariablePolicy,+  IN CHAR16                          *VariableName   ) {   EFI_STATUS  Status; -  Status = VariableLock->RequestToLock (-                           VariableLock,-                           VariableName,-                           &gEfiCallerIdGuid-                           );-  if (!EFI_ERROR (Status)) {-    return PreviousStatus;+  // If success, go ahead and set the policies to protect the target variables.+  Status = RegisterBasicVariablePolicy (VariablePolicy,+                                        &gEfiCallerIdGuid,+                                        VariableName,+                                        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);+  if (EFI_ERROR (Status)) {+    DEBUG ((DEBUG_ERROR, "FmpDxe(%s): Failed to lock variable %g %s. Status = %r\n",+            mImageIdName,+            &gEfiCallerIdGuid,+            VariableName,+            Status+           ));   }--  DEBUG ((DEBUG_ERROR, "FmpDxe(%s): Failed to lock variable %g %s.  Status = %r\n",-    mImageIdName,-    &gEfiCallerIdGuid,-    VariableName,-    Status-    ));-   if (EFI_ERROR (PreviousStatus)) {     return PreviousStatus;   }@@ -773,26 +774,22 @@ LockAllFmpVariables (
   FIRMWARE_MANAGEMENT_PRIVATE_DATA  *Private   ) {-  EFI_STATUS                    Status;-  EDKII_VARIABLE_LOCK_PROTOCOL  *VariableLock;--  VariableLock = NULL;-  Status = gBS->LocateProtocol (-                  &gEdkiiVariableLockProtocolGuid,-                  NULL,-                  (VOID **)&VariableLock-                  );-  if (EFI_ERROR (Status) || VariableLock == NULL) {-    DEBUG ((DEBUG_ERROR, "FmpDxe(%s): Failed to locate Variable Lock Protocol (%r).\n", mImageIdName, Status));-    return EFI_UNSUPPORTED;+  EFI_STATUS                        Status;+  EDKII_VARIABLE_POLICY_PROTOCOL    *VariablePolicy;++  // Locate the VariablePolicy protocol.+  Status = gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid, NULL, (VOID**)&VariablePolicy );+  if (EFI_ERROR (Status)) {+    DEBUG ((DEBUG_ERROR, "FmpDxe %a - Could not locate VariablePolicy protocol! %r\n", __FUNCTION__, Status));+    return Status;   }    Status = EFI_SUCCESS;-  Status = LockFmpVariable (Status, VariableLock, Private->VersionVariableName);-  Status = LockFmpVariable (Status, VariableLock, Private->LsvVariableName);-  Status = LockFmpVariable (Status, VariableLock, Private->LastAttemptStatusVariableName);-  Status = LockFmpVariable (Status, VariableLock, Private->LastAttemptVersionVariableName);-  Status = LockFmpVariable (Status, VariableLock, Private->FmpStateVariableName);+  Status = LockFmpVariable (Status, VariablePolicy, Private->VersionVariableName);+  Status = LockFmpVariable (Status, VariablePolicy, Private->LsvVariableName);+  Status = LockFmpVariable (Status, VariablePolicy, Private->LastAttemptStatusVariableName);+  Status = LockFmpVariable (Status, VariablePolicy, Private->LastAttemptVersionVariableName);+  Status = LockFmpVariable (Status, VariablePolicy, Private->FmpStateVariableName);    return Status; }-- 
2.26.2.windows.1