public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel][PATCH v4] MdeModulePkg/DxeCapsuleLibFmp: Use new Variable Lock interface
@ 2021-10-29  3:10 Yang Jie
  2021-11-02  1:40 ` 回复: " gaoliming
  2021-11-02  6:11 ` Wang, Jian J
  0 siblings, 2 replies; 3+ messages in thread
From: Yang Jie @ 2021-10-29  3:10 UTC (permalink / raw)
  To: devel; +Cc: jian.j.wang, guomin.jiang, gaoliming, Yang Jie

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3699
The code in MdeModulePkg\Library\DxeCapsuleLibFmp call the deprecated 
interface VariableLockRequestToLock.c. So I changed the code in
FmpDevicePkg using RegisterBasicVariablePolicy, instead of the 
deprecated interface.

Signed-off-by: Yang Jie <jie.yang@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jian J Wang <jian.j.wang@intel.com>
---
 .../DxeCapsuleLibFmp/DxeCapsuleLib.inf        |  5 +-
 .../DxeCapsuleLibFmp/DxeCapsuleReportLib.c    | 87 +++++++++++++------
 2 files changed, 62 insertions(+), 30 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
index 05de4299fb..9212c81d68 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
@@ -3,7 +3,7 @@
 #
 #  Capsule library instance for DXE_DRIVER module types.
 #
-#  Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -51,6 +51,7 @@
   DisplayUpdateProgressLib
   FileHandleLib
   UefiBootManagerLib
+  VariablePolicyHelperLib
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleMax                               ## CONSUMES
@@ -71,11 +72,11 @@
 [Protocols]
   gEsrtManagementProtocolGuid                   ## CONSUMES
   gEfiFirmwareManagementProtocolGuid            ## CONSUMES
-  gEdkiiVariableLockProtocolGuid                ## SOMETIMES_CONSUMES
   gEdkiiFirmwareManagementProgressProtocolGuid  ## SOMETIMES_CONSUMES
   gEfiSimpleFileSystemProtocolGuid              ## SOMETIMES_CONSUMES
   gEfiBlockIoProtocolGuid                       ## CONSUMES
   gEfiDiskIoProtocolGuid                        ## CONSUMES
+  gEdkiiVariablePolicyProtocolGuid              ## CONSUMES
 
 [Guids]
   gEfiFmpCapsuleGuid                      ## SOMETIMES_CONSUMES ## GUID
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
index 0ec5f20676..d90f131879 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
@@ -1,14 +1,13 @@
 /** @file
   DXE capsule report related function.
 
-  Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
 #include <PiDxe.h>
 #include <Protocol/FirmwareManagement.h>
-#include <Protocol/VariableLock.h>
 #include <Guid/CapsuleReport.h>
 #include <Guid/FmpCapsule.h>
 #include <Guid/CapsuleVendor.h>
@@ -26,6 +25,7 @@
 #include <Library/ReportStatusCodeLib.h>
 #include <Library/DevicePathLib.h>
 #include <Library/CapsuleLib.h>
+#include <Library/VariablePolicyHelperLib.h>
 
 #include <IndustryStandard/WindowsUxCapsule.h>
 
@@ -94,6 +94,39 @@ GetNewCapsuleResultIndex (
   return CurrentIndex + 1;
 }
 
+/**
+  Lock Variable by variable policy
+
+  @param[in] VariableGuid         The Guid of the variable to be locked
+  @param[in] VariableName         The name of the variable to be locked
+  @param[in] VariablePolicy       The pointer of variable lock policy
+**/
+VOID LockVaraible (
+    IN CONST  EFI_GUID                 VariableGuid,
+    IN CHAR16                          *VariableName,
+    IN EDKII_VARIABLE_POLICY_PROTOCOL  *VariablePolicy
+  )
+{
+  EFI_STATUS                       Status;
+
+  // Set the policies to protect the target variables
+  Status = RegisterBasicVariablePolicy (VariablePolicy,
+                                        &VariableGuid,
+                                        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, "DxeCapsuleLibFmp: Failed to lock variable %g %s.  Status = %r\n",
+            &VariableGuid,
+            VariableName,
+            Status));
+    ASSERT_EFI_ERROR (Status);
+  }
+}
+
 /**
   Write a new capsule status variable.
 
@@ -269,16 +302,17 @@ RecordFmpCapsuleStatusVariable (
 
 /**
   Initialize CapsuleMax variables.
+
+  @param[in] VariablePolicy       The pointer of variable lock policy
 **/
 VOID
 InitCapsuleMaxVariable (
-  VOID
+  EDKII_VARIABLE_POLICY_PROTOCOL   *VariablePolicy
   )
 {
   EFI_STATUS                       Status;
   UINTN                            Size;
   CHAR16                           CapsuleMaxStr[sizeof("Capsule####")];
-  EDKII_VARIABLE_LOCK_PROTOCOL     *VariableLock;
 
   UnicodeSPrint(
     CapsuleMaxStr,
@@ -297,25 +331,22 @@ InitCapsuleMaxVariable (
                   );
   if (!EFI_ERROR(Status)) {
     // Lock it per UEFI spec.
-    Status = gBS->LocateProtocol(&gEdkiiVariableLockProtocolGuid, NULL, (VOID **)&VariableLock);
-    if (!EFI_ERROR(Status)) {
-      Status = VariableLock->RequestToLock(VariableLock, L"CapsuleMax", &gEfiCapsuleReportGuid);
-      ASSERT_EFI_ERROR(Status);
-    }
+    LockVaraible (gEfiCapsuleReportGuid, L"CapsuleMax", VariablePolicy);
   }
 }
 
 /**
   Initialize CapsuleLast variables.
+
+  @param[in] VariablePolicy       The pointer of variable lock policy
 **/
 VOID
 InitCapsuleLastVariable (
-  VOID
+  EDKII_VARIABLE_POLICY_PROTOCOL   *VariablePolicy
   )
 {
   EFI_STATUS                       Status;
   EFI_BOOT_MODE                    BootMode;
-  EDKII_VARIABLE_LOCK_PROTOCOL     *VariableLock;
   VOID                             *CapsuleResult;
   UINTN                            Size;
   CHAR16                           CapsuleLastStr[sizeof("Capsule####")];
@@ -372,11 +403,7 @@ InitCapsuleLastVariable (
     }
 
     // Lock it in normal boot path per UEFI spec.
-    Status = gBS->LocateProtocol(&gEdkiiVariableLockProtocolGuid, NULL, (VOID **)&VariableLock);
-    if (!EFI_ERROR(Status)) {
-      Status = VariableLock->RequestToLock(VariableLock, L"CapsuleLast", &gEfiCapsuleReportGuid);
-      ASSERT_EFI_ERROR(Status);
-    }
+    LockVaraible (gEfiCapsuleReportGuid, L"CapsuleLast", VariablePolicy);
   }
 }
 
@@ -430,26 +457,21 @@ InitCapsuleUpdateVariable (
 
 /**
   Initialize capsule relocation info variable.
+
+  @param[in] VariablePolicy       The pointer of variable lock policy
 **/
 VOID
 InitCapsuleRelocationInfo (
-  VOID
+  EDKII_VARIABLE_POLICY_PROTOCOL   *VariablePolicy
   )
 {
-  EFI_STATUS                   Status;
-  EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock;
-
   CoDClearCapsuleRelocationInfo();
 
   //
   // Unlock Capsule On Disk relocation Info variable only when Capsule On Disk flag is enabled
   //
   if (!CoDCheckCapsuleOnDiskFlag()) {
-    Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL, (VOID **) &VariableLock);
-    if (!EFI_ERROR (Status)) {
-      Status = VariableLock->RequestToLock (VariableLock, COD_RELOCATION_INFO_VAR_NAME, &gEfiCapsuleVendorGuid);
-      ASSERT_EFI_ERROR (Status);
-    }
+    LockVaraible (gEfiCapsuleVendorGuid, COD_RELOCATION_INFO_VAR_NAME, VariablePolicy);
   }
 }
 
@@ -461,10 +483,19 @@ InitCapsuleVariable (
   VOID
   )
 {
+  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, "DxeCapsuleReportLib %a - Could not locate VariablePolicy protocol! %r\n", __FUNCTION__, Status));
+    ASSERT_EFI_ERROR (Status);
+  }
   InitCapsuleUpdateVariable();
-  InitCapsuleMaxVariable();
-  InitCapsuleLastVariable();
-  InitCapsuleRelocationInfo();
+  InitCapsuleMaxVariable (VariablePolicy);
+  InitCapsuleLastVariable (VariablePolicy);
+  InitCapsuleRelocationInfo (VariablePolicy);
 
   //
   // No need to clear L"Capsule####", because OS/APP should refer L"CapsuleLast"
-- 
2.26.2.windows.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* 回复: [edk2-devel][PATCH v4] MdeModulePkg/DxeCapsuleLibFmp: Use new Variable Lock interface
  2021-10-29  3:10 [edk2-devel][PATCH v4] MdeModulePkg/DxeCapsuleLibFmp: Use new Variable Lock interface Yang Jie
@ 2021-11-02  1:40 ` gaoliming
  2021-11-02  6:11 ` Wang, Jian J
  1 sibling, 0 replies; 3+ messages in thread
From: gaoliming @ 2021-11-02  1:40 UTC (permalink / raw)
  To: devel, jie.yang; +Cc: jian.j.wang, guomin.jiang

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Yang Jie
> 发送时间: 2021年10月29日 11:10
> 收件人: devel@edk2.groups.io
> 抄送: jian.j.wang@intel.com; guomin.jiang@intel.com;
> gaoliming@byosoft.com.cn; Yang Jie <jie.yang@intel.com>
> 主题: [edk2-devel][PATCH v4] MdeModulePkg/DxeCapsuleLibFmp: Use new
> Variable Lock interface
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3699
> The code in MdeModulePkg\Library\DxeCapsuleLibFmp call the deprecated
> interface VariableLockRequestToLock.c. So I changed the code in
> FmpDevicePkg using RegisterBasicVariablePolicy, instead of the
> deprecated interface.
> 
> Signed-off-by: Yang Jie <jie.yang@intel.com>
> Cc: Guomin Jiang <guomin.jiang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> ---
>  .../DxeCapsuleLibFmp/DxeCapsuleLib.inf        |  5 +-
>  .../DxeCapsuleLibFmp/DxeCapsuleReportLib.c    | 87
> +++++++++++++------
>  2 files changed, 62 insertions(+), 30 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> index 05de4299fb..9212c81d68 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> @@ -3,7 +3,7 @@
>  #
> 
>  #  Capsule library instance for DXE_DRIVER module types.
> 
>  #
> 
> -#  Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +#  Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #
> 
>  ##
> 
> @@ -51,6 +51,7 @@
>    DisplayUpdateProgressLib
> 
>    FileHandleLib
> 
>    UefiBootManagerLib
> 
> +  VariablePolicyHelperLib
> 
> 
> 
>  [Pcd]
> 
>    gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleMax
> ## CONSUMES
> 
> @@ -71,11 +72,11 @@
>  [Protocols]
> 
>    gEsrtManagementProtocolGuid                   ## CONSUMES
> 
>    gEfiFirmwareManagementProtocolGuid            ## CONSUMES
> 
> -  gEdkiiVariableLockProtocolGuid                ##
> SOMETIMES_CONSUMES
> 
>    gEdkiiFirmwareManagementProgressProtocolGuid  ##
> SOMETIMES_CONSUMES
> 
>    gEfiSimpleFileSystemProtocolGuid              ##
> SOMETIMES_CONSUMES
> 
>    gEfiBlockIoProtocolGuid                       ## CONSUMES
> 
>    gEfiDiskIoProtocolGuid                        ## CONSUMES
> 
> +  gEdkiiVariablePolicyProtocolGuid              ## CONSUMES
> 
> 
> 
>  [Guids]
> 
>    gEfiFmpCapsuleGuid                      ##
> SOMETIMES_CONSUMES ## GUID
> 
> diff --git
> a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> index 0ec5f20676..d90f131879 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> @@ -1,14 +1,13 @@
>  /** @file
> 
>    DXE capsule report related function.
> 
> 
> 
> -  Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +  Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> 
> 
>  #include <PiDxe.h>
> 
>  #include <Protocol/FirmwareManagement.h>
> 
> -#include <Protocol/VariableLock.h>
> 
>  #include <Guid/CapsuleReport.h>
> 
>  #include <Guid/FmpCapsule.h>
> 
>  #include <Guid/CapsuleVendor.h>
> 
> @@ -26,6 +25,7 @@
>  #include <Library/ReportStatusCodeLib.h>
> 
>  #include <Library/DevicePathLib.h>
> 
>  #include <Library/CapsuleLib.h>
> 
> +#include <Library/VariablePolicyHelperLib.h>
> 
> 
> 
>  #include <IndustryStandard/WindowsUxCapsule.h>
> 
> 
> 
> @@ -94,6 +94,39 @@ GetNewCapsuleResultIndex (
>    return CurrentIndex + 1;
> 
>  }
> 
> 
> 
> +/**
> 
> +  Lock Variable by variable policy
> 
> +
> 
> +  @param[in] VariableGuid         The Guid of the variable to be locked
> 
> +  @param[in] VariableName         The name of the variable to be
> locked
> 
> +  @param[in] VariablePolicy       The pointer of variable lock policy
> 
> +**/
> 
> +VOID LockVaraible (
> 
> +    IN CONST  EFI_GUID                 VariableGuid,
> 
> +    IN CHAR16                          *VariableName,
> 
> +    IN EDKII_VARIABLE_POLICY_PROTOCOL  *VariablePolicy
> 
> +  )
> 
> +{
> 
> +  EFI_STATUS                       Status;
> 
> +
> 
> +  // Set the policies to protect the target variables
> 
> +  Status = RegisterBasicVariablePolicy (VariablePolicy,
> 
> +                                        &VariableGuid,
> 
> +                                        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, "DxeCapsuleLibFmp: Failed to lock
> variable %g %s.  Status = %r\n",
> 
> +            &VariableGuid,
> 
> +            VariableName,
> 
> +            Status));
> 
> +    ASSERT_EFI_ERROR (Status);
> 
> +  }
> 
> +}
> 
> +
> 
>  /**
> 
>    Write a new capsule status variable.
> 
> 
> 
> @@ -269,16 +302,17 @@ RecordFmpCapsuleStatusVariable (
> 
> 
>  /**
> 
>    Initialize CapsuleMax variables.
> 
> +
> 
> +  @param[in] VariablePolicy       The pointer of variable lock policy
> 
>  **/
> 
>  VOID
> 
>  InitCapsuleMaxVariable (
> 
> -  VOID
> 
> +  EDKII_VARIABLE_POLICY_PROTOCOL   *VariablePolicy
> 
>    )
> 
>  {
> 
>    EFI_STATUS                       Status;
> 
>    UINTN                            Size;
> 
>    CHAR16
> CapsuleMaxStr[sizeof("Capsule####")];
> 
> -  EDKII_VARIABLE_LOCK_PROTOCOL     *VariableLock;
> 
> 
> 
>    UnicodeSPrint(
> 
>      CapsuleMaxStr,
> 
> @@ -297,25 +331,22 @@ InitCapsuleMaxVariable (
>                    );
> 
>    if (!EFI_ERROR(Status)) {
> 
>      // Lock it per UEFI spec.
> 
> -    Status = gBS->LocateProtocol(&gEdkiiVariableLockProtocolGuid, NULL,
> (VOID **)&VariableLock);
> 
> -    if (!EFI_ERROR(Status)) {
> 
> -      Status = VariableLock->RequestToLock(VariableLock, L"CapsuleMax",
> &gEfiCapsuleReportGuid);
> 
> -      ASSERT_EFI_ERROR(Status);
> 
> -    }
> 
> +    LockVaraible (gEfiCapsuleReportGuid, L"CapsuleMax", VariablePolicy);
> 
>    }
> 
>  }
> 
> 
> 
>  /**
> 
>    Initialize CapsuleLast variables.
> 
> +
> 
> +  @param[in] VariablePolicy       The pointer of variable lock policy
> 
>  **/
> 
>  VOID
> 
>  InitCapsuleLastVariable (
> 
> -  VOID
> 
> +  EDKII_VARIABLE_POLICY_PROTOCOL   *VariablePolicy
> 
>    )
> 
>  {
> 
>    EFI_STATUS                       Status;
> 
>    EFI_BOOT_MODE                    BootMode;
> 
> -  EDKII_VARIABLE_LOCK_PROTOCOL     *VariableLock;
> 
>    VOID                             *CapsuleResult;
> 
>    UINTN                            Size;
> 
>    CHAR16
> CapsuleLastStr[sizeof("Capsule####")];
> 
> @@ -372,11 +403,7 @@ InitCapsuleLastVariable (
>      }
> 
> 
> 
>      // Lock it in normal boot path per UEFI spec.
> 
> -    Status = gBS->LocateProtocol(&gEdkiiVariableLockProtocolGuid, NULL,
> (VOID **)&VariableLock);
> 
> -    if (!EFI_ERROR(Status)) {
> 
> -      Status = VariableLock->RequestToLock(VariableLock, L"CapsuleLast",
> &gEfiCapsuleReportGuid);
> 
> -      ASSERT_EFI_ERROR(Status);
> 
> -    }
> 
> +    LockVaraible (gEfiCapsuleReportGuid, L"CapsuleLast", VariablePolicy);
> 
>    }
> 
>  }
> 
> 
> 
> @@ -430,26 +457,21 @@ InitCapsuleUpdateVariable (
> 
> 
>  /**
> 
>    Initialize capsule relocation info variable.
> 
> +
> 
> +  @param[in] VariablePolicy       The pointer of variable lock policy
> 
>  **/
> 
>  VOID
> 
>  InitCapsuleRelocationInfo (
> 
> -  VOID
> 
> +  EDKII_VARIABLE_POLICY_PROTOCOL   *VariablePolicy
> 
>    )
> 
>  {
> 
> -  EFI_STATUS                   Status;
> 
> -  EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock;
> 
> -
> 
>    CoDClearCapsuleRelocationInfo();
> 
> 
> 
>    //
> 
>    // Unlock Capsule On Disk relocation Info variable only when Capsule On
> Disk flag is enabled
> 
>    //
> 
>    if (!CoDCheckCapsuleOnDiskFlag()) {
> 
> -    Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL,
> (VOID **) &VariableLock);
> 
> -    if (!EFI_ERROR (Status)) {
> 
> -      Status = VariableLock->RequestToLock (VariableLock,
> COD_RELOCATION_INFO_VAR_NAME, &gEfiCapsuleVendorGuid);
> 
> -      ASSERT_EFI_ERROR (Status);
> 
> -    }
> 
> +    LockVaraible (gEfiCapsuleVendorGuid,
> COD_RELOCATION_INFO_VAR_NAME, VariablePolicy);
> 
>    }
> 
>  }
> 
> 
> 
> @@ -461,10 +483,19 @@ InitCapsuleVariable (
>    VOID
> 
>    )
> 
>  {
> 
> +  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, "DxeCapsuleReportLib %a - Could not locate
> VariablePolicy protocol! %r\n", __FUNCTION__, Status));
> 
> +    ASSERT_EFI_ERROR (Status);
> 
> +  }
> 
>    InitCapsuleUpdateVariable();
> 
> -  InitCapsuleMaxVariable();
> 
> -  InitCapsuleLastVariable();
> 
> -  InitCapsuleRelocationInfo();
> 
> +  InitCapsuleMaxVariable (VariablePolicy);
> 
> +  InitCapsuleLastVariable (VariablePolicy);
> 
> +  InitCapsuleRelocationInfo (VariablePolicy);
> 
> 
> 
>    //
> 
>    // No need to clear L"Capsule####", because OS/APP should refer
> L"CapsuleLast"
> 
> --
> 2.26.2.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#82904): https://edk2.groups.io/g/devel/message/82904
> Mute This Topic: https://groups.io/mt/86669161/4905953
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaoliming@byosoft.com.cn]
> -=-=-=-=-=-=
> 




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [edk2-devel][PATCH v4] MdeModulePkg/DxeCapsuleLibFmp: Use new Variable Lock interface
  2021-10-29  3:10 [edk2-devel][PATCH v4] MdeModulePkg/DxeCapsuleLibFmp: Use new Variable Lock interface Yang Jie
  2021-11-02  1:40 ` 回复: " gaoliming
@ 2021-11-02  6:11 ` Wang, Jian J
  1 sibling, 0 replies; 3+ messages in thread
From: Wang, Jian J @ 2021-11-02  6:11 UTC (permalink / raw)
  To: devel@edk2.groups.io, Yang, Jie; +Cc: Jiang, Guomin, gaoliming@byosoft.com.cn

Just a format issue (see inline comment). With it addressed,

Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

Regards,
Jian

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yang Jie
> Sent: Friday, October 29, 2021 11:10 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Jiang, Guomin
> <guomin.jiang@intel.com>; gaoliming@byosoft.com.cn; Yang, Jie
> <jie.yang@intel.com>
> Subject: [edk2-devel][PATCH v4] MdeModulePkg/DxeCapsuleLibFmp: Use new
> Variable Lock interface
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3699
> The code in MdeModulePkg\Library\DxeCapsuleLibFmp call the deprecated
> interface VariableLockRequestToLock.c. So I changed the code in
> FmpDevicePkg using RegisterBasicVariablePolicy, instead of the
> deprecated interface.
> 
> Signed-off-by: Yang Jie <jie.yang@intel.com>
> Cc: Guomin Jiang <guomin.jiang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> ---
>  .../DxeCapsuleLibFmp/DxeCapsuleLib.inf        |  5 +-
>  .../DxeCapsuleLibFmp/DxeCapsuleReportLib.c    | 87 +++++++++++++------
>  2 files changed, 62 insertions(+), 30 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> index 05de4299fb..9212c81d68 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
> @@ -3,7 +3,7 @@
>  #
> 
>  #  Capsule library instance for DXE_DRIVER module types.
> 
>  #
> 
> -#  Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +#  Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  #
> 
>  ##
> 
> @@ -51,6 +51,7 @@
>    DisplayUpdateProgressLib
> 
>    FileHandleLib
> 
>    UefiBootManagerLib
> 
> +  VariablePolicyHelperLib
> 
> 
> 
>  [Pcd]
> 
>    gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleMax                               ##
> CONSUMES
> 
> @@ -71,11 +72,11 @@
>  [Protocols]
> 
>    gEsrtManagementProtocolGuid                   ## CONSUMES
> 
>    gEfiFirmwareManagementProtocolGuid            ## CONSUMES
> 
> -  gEdkiiVariableLockProtocolGuid                ## SOMETIMES_CONSUMES
> 
>    gEdkiiFirmwareManagementProgressProtocolGuid  ##
> SOMETIMES_CONSUMES
> 
>    gEfiSimpleFileSystemProtocolGuid              ## SOMETIMES_CONSUMES
> 
>    gEfiBlockIoProtocolGuid                       ## CONSUMES
> 
>    gEfiDiskIoProtocolGuid                        ## CONSUMES
> 
> +  gEdkiiVariablePolicyProtocolGuid              ## CONSUMES
> 
> 
> 
>  [Guids]
> 
>    gEfiFmpCapsuleGuid                      ## SOMETIMES_CONSUMES ## GUID
> 
> diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> index 0ec5f20676..d90f131879 100644
> --- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> +++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleReportLib.c
> @@ -1,14 +1,13 @@
>  /** @file
> 
>    DXE capsule report related function.
> 
> 
> 
> -  Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
> 
> +  Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> 
> 
>  #include <PiDxe.h>
> 
>  #include <Protocol/FirmwareManagement.h>
> 
> -#include <Protocol/VariableLock.h>
> 
>  #include <Guid/CapsuleReport.h>
> 
>  #include <Guid/FmpCapsule.h>
> 
>  #include <Guid/CapsuleVendor.h>
> 
> @@ -26,6 +25,7 @@
>  #include <Library/ReportStatusCodeLib.h>
> 
>  #include <Library/DevicePathLib.h>
> 
>  #include <Library/CapsuleLib.h>
> 
> +#include <Library/VariablePolicyHelperLib.h>
> 
> 
> 
>  #include <IndustryStandard/WindowsUxCapsule.h>
> 
> 
> 
> @@ -94,6 +94,39 @@ GetNewCapsuleResultIndex (
>    return CurrentIndex + 1;
> 
>  }
> 
> 
> 
> +/**
> 
> +  Lock Variable by variable policy
> 
> +
> 
> +  @param[in] VariableGuid         The Guid of the variable to be locked
> 
> +  @param[in] VariableName         The name of the variable to be locked
> 
> +  @param[in] VariablePolicy       The pointer of variable lock policy
> 
> +**/
> 
> +VOID LockVaraible (
> 
> +    IN CONST  EFI_GUID                 VariableGuid,
> 
> +    IN CHAR16                          *VariableName,
> 
> +    IN EDKII_VARIABLE_POLICY_PROTOCOL  *VariablePolicy
> 

The alignment in above lines are not correct (four spaces, should be two spaces here).

> +  )
> 
> +{
> 
> +  EFI_STATUS                       Status;
> 
> +
> 
> +  // Set the policies to protect the target variables
> 
> +  Status = RegisterBasicVariablePolicy (VariablePolicy,
> 
> +                                        &VariableGuid,
> 
> +                                        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, "DxeCapsuleLibFmp: Failed to lock variable %g %s.
> Status = %r\n",
> 
> +            &VariableGuid,
> 
> +            VariableName,
> 
> +            Status));
> 
> +    ASSERT_EFI_ERROR (Status);
> 
> +  }
> 
> +}
> 
> +
> 
>  /**
> 
>    Write a new capsule status variable.
> 
> 
> 
> @@ -269,16 +302,17 @@ RecordFmpCapsuleStatusVariable (
> 
> 
>  /**
> 
>    Initialize CapsuleMax variables.
> 
> +
> 
> +  @param[in] VariablePolicy       The pointer of variable lock policy
> 
>  **/
> 
>  VOID
> 
>  InitCapsuleMaxVariable (
> 
> -  VOID
> 
> +  EDKII_VARIABLE_POLICY_PROTOCOL   *VariablePolicy
> 
>    )
> 
>  {
> 
>    EFI_STATUS                       Status;
> 
>    UINTN                            Size;
> 
>    CHAR16                           CapsuleMaxStr[sizeof("Capsule####")];
> 
> -  EDKII_VARIABLE_LOCK_PROTOCOL     *VariableLock;
> 
> 
> 
>    UnicodeSPrint(
> 
>      CapsuleMaxStr,
> 
> @@ -297,25 +331,22 @@ InitCapsuleMaxVariable (
>                    );
> 
>    if (!EFI_ERROR(Status)) {
> 
>      // Lock it per UEFI spec.
> 
> -    Status = gBS->LocateProtocol(&gEdkiiVariableLockProtocolGuid, NULL,
> (VOID **)&VariableLock);
> 
> -    if (!EFI_ERROR(Status)) {
> 
> -      Status = VariableLock->RequestToLock(VariableLock, L"CapsuleMax",
> &gEfiCapsuleReportGuid);
> 
> -      ASSERT_EFI_ERROR(Status);
> 
> -    }
> 
> +    LockVaraible (gEfiCapsuleReportGuid, L"CapsuleMax", VariablePolicy);
> 
>    }
> 
>  }
> 
> 
> 
>  /**
> 
>    Initialize CapsuleLast variables.
> 
> +
> 
> +  @param[in] VariablePolicy       The pointer of variable lock policy
> 
>  **/
> 
>  VOID
> 
>  InitCapsuleLastVariable (
> 
> -  VOID
> 
> +  EDKII_VARIABLE_POLICY_PROTOCOL   *VariablePolicy
> 
>    )
> 
>  {
> 
>    EFI_STATUS                       Status;
> 
>    EFI_BOOT_MODE                    BootMode;
> 
> -  EDKII_VARIABLE_LOCK_PROTOCOL     *VariableLock;
> 
>    VOID                             *CapsuleResult;
> 
>    UINTN                            Size;
> 
>    CHAR16                           CapsuleLastStr[sizeof("Capsule####")];
> 
> @@ -372,11 +403,7 @@ InitCapsuleLastVariable (
>      }
> 
> 
> 
>      // Lock it in normal boot path per UEFI spec.
> 
> -    Status = gBS->LocateProtocol(&gEdkiiVariableLockProtocolGuid, NULL,
> (VOID **)&VariableLock);
> 
> -    if (!EFI_ERROR(Status)) {
> 
> -      Status = VariableLock->RequestToLock(VariableLock, L"CapsuleLast",
> &gEfiCapsuleReportGuid);
> 
> -      ASSERT_EFI_ERROR(Status);
> 
> -    }
> 
> +    LockVaraible (gEfiCapsuleReportGuid, L"CapsuleLast", VariablePolicy);
> 
>    }
> 
>  }
> 
> 
> 
> @@ -430,26 +457,21 @@ InitCapsuleUpdateVariable (
> 
> 
>  /**
> 
>    Initialize capsule relocation info variable.
> 
> +
> 
> +  @param[in] VariablePolicy       The pointer of variable lock policy
> 
>  **/
> 
>  VOID
> 
>  InitCapsuleRelocationInfo (
> 
> -  VOID
> 
> +  EDKII_VARIABLE_POLICY_PROTOCOL   *VariablePolicy
> 
>    )
> 
>  {
> 
> -  EFI_STATUS                   Status;
> 
> -  EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock;
> 
> -
> 
>    CoDClearCapsuleRelocationInfo();
> 
> 
> 
>    //
> 
>    // Unlock Capsule On Disk relocation Info variable only when Capsule On Disk
> flag is enabled
> 
>    //
> 
>    if (!CoDCheckCapsuleOnDiskFlag()) {
> 
> -    Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL,
> (VOID **) &VariableLock);
> 
> -    if (!EFI_ERROR (Status)) {
> 
> -      Status = VariableLock->RequestToLock (VariableLock,
> COD_RELOCATION_INFO_VAR_NAME, &gEfiCapsuleVendorGuid);
> 
> -      ASSERT_EFI_ERROR (Status);
> 
> -    }
> 
> +    LockVaraible (gEfiCapsuleVendorGuid, COD_RELOCATION_INFO_VAR_NAME,
> VariablePolicy);
> 
>    }
> 
>  }
> 
> 
> 
> @@ -461,10 +483,19 @@ InitCapsuleVariable (
>    VOID
> 
>    )
> 
>  {
> 
> +  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, "DxeCapsuleReportLib %a - Could not locate
> VariablePolicy protocol! %r\n", __FUNCTION__, Status));
> 
> +    ASSERT_EFI_ERROR (Status);
> 
> +  }
> 
>    InitCapsuleUpdateVariable();
> 
> -  InitCapsuleMaxVariable();
> 
> -  InitCapsuleLastVariable();
> 
> -  InitCapsuleRelocationInfo();
> 
> +  InitCapsuleMaxVariable (VariablePolicy);
> 
> +  InitCapsuleLastVariable (VariablePolicy);
> 
> +  InitCapsuleRelocationInfo (VariablePolicy);
> 
> 
> 
>    //
> 
>    // No need to clear L"Capsule####", because OS/APP should refer
> L"CapsuleLast"
> 
> --
> 2.26.2.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#82904): https://edk2.groups.io/g/devel/message/82904
> Mute This Topic: https://groups.io/mt/86669161/1768734
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jian.j.wang@intel.com]
> -=-=-=-=-=-=
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-11-02  6:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-29  3:10 [edk2-devel][PATCH v4] MdeModulePkg/DxeCapsuleLibFmp: Use new Variable Lock interface Yang Jie
2021-11-02  1:40 ` 回复: " gaoliming
2021-11-02  6:11 ` Wang, Jian J

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox