public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch v4 0/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior
@ 2020-12-11  8:01 Michael D Kinney
  2020-12-11  8:01 ` [Patch v4 1/2] " Michael D Kinney
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Michael D Kinney @ 2020-12-11  8:01 UTC (permalink / raw)
  To: devel; +Cc: Hao A Wu, Liming Gao, Bret Barkelew

New in V4
==========
* Fix spelling in unit tests
* Call ValidateSetVariable() with DataSize=0, Attributes=0

New in V3
==========
* Split into 2 patches.  One for code change.  Second for unit tests.
* Remove duplicate code and use ValidateSetVariable() to detect if a
  variable is already locked.

https://bugzilla.tianocore.org/show_bug.cgi?id=3111

The VariableLock shim currently fails if called twice because the
underlying Variable Policy engine returns an error if a policy is set
on an existing variable.

This breaks existing code which expect it to silently pass if a variable
is locked multiple times (because it should "be locked").

Refactor the shim to confirm that the variable is indeed locked and then
change the error to EFI_SUCCESS and generate a DEBUG_ERROR message so
the duplicate lock can be reported in a debug log and removed.

Add host based unit tests for the multiple lock case using Variable Lock
Protocol, Variable Policy Protocol, and mixes of Variable Lock Protocol
and Variable Policy Protocol.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Bret Barkelew <Bret.Barkelew@microsoft.com>

Bret Barkelew (1):
  MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol
    behavior

Michael D Kinney (1):
  MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit
    Tests

 MdeModulePkg/Test/MdeModulePkgHostTest.dsc    |  11 +
 .../VariableLockRequestToLockUnitTest.c       | 565 ++++++++++++++++++
 .../VariableLockRequestToLockUnitTest.inf     |  36 ++
 .../RuntimeDxe/VariableLockRequestToLock.c    |  95 +--
 4 files changed, 671 insertions(+), 36 deletions(-)
 create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.c
 create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.inf

-- 
2.29.2.windows.2


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

* [Patch v4 1/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior
  2020-12-11  8:01 [Patch v4 0/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior Michael D Kinney
@ 2020-12-11  8:01 ` Michael D Kinney
  2020-12-14  1:39   ` 回复: [edk2-devel] " gaoliming
  2020-12-11  8:01 ` [Patch v4 2/2] MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit Tests Michael D Kinney
  2020-12-11  8:12 ` [Patch v4 0/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior Wu, Hao A
  2 siblings, 1 reply; 8+ messages in thread
From: Michael D Kinney @ 2020-12-11  8:01 UTC (permalink / raw)
  To: devel; +Cc: Bret Barkelew, Hao A Wu, Liming Gao, Bret Barkelew

From: Bret Barkelew <bret.barkelew@microsoft.com>

https://bugzilla.tianocore.org/show_bug.cgi?id=3111

The VariableLock shim currently fails if called twice because the
underlying Variable Policy engine returns an error if a policy is set
on an existing variable.

This breaks existing code which expect it to silently pass if a variable
is locked multiple times (because it should "be locked").

Refactor the shim to confirm that the variable is indeed locked and then
change the error to EFI_SUCCESS and generate a DEBUG_ERROR message so
the duplicate lock can be reported in a debug log and removed.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 .../RuntimeDxe/VariableLockRequestToLock.c    | 95 ++++++++++++-------
 1 file changed, 59 insertions(+), 36 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c
index 4aa854aaf260..7d87e50efdcd 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c
@@ -1,67 +1,90 @@
-/** @file -- VariableLockRequestToLock.c
-Temporary location of the RequestToLock shim code while
-projects are moved to VariablePolicy. Should be removed when deprecated.
+/** @file
+  Temporary location of the RequestToLock shim code while projects
+  are moved to VariablePolicy. Should be removed when deprecated.
 
-Copyright (c) Microsoft Corporation.
-SPDX-License-Identifier: BSD-2-Clause-Patent
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
 #include <Uefi.h>
-
 #include <Library/DebugLib.h>
 #include <Library/MemoryAllocationLib.h>
-
-#include <Protocol/VariableLock.h>
-
-#include <Protocol/VariablePolicy.h>
 #include <Library/VariablePolicyLib.h>
 #include <Library/VariablePolicyHelperLib.h>
-
+#include <Protocol/VariableLock.h>
 
 /**
   DEPRECATED. THIS IS ONLY HERE AS A CONVENIENCE WHILE PORTING.
-  Mark a variable that will become read-only after leaving the DXE phase of execution.
-  Write request coming from SMM environment through EFI_SMM_VARIABLE_PROTOCOL is allowed.
+  Mark a variable that will become read-only after leaving the DXE phase of
+  execution. Write request coming from SMM environment through
+  EFI_SMM_VARIABLE_PROTOCOL is allowed.
 
   @param[in] This          The VARIABLE_LOCK_PROTOCOL instance.
-  @param[in] VariableName  A pointer to the variable name that will be made read-only subsequently.
-  @param[in] VendorGuid    A pointer to the vendor GUID that will be made read-only subsequently.
+  @param[in] VariableName  A pointer to the variable name that will be made
+                           read-only subsequently.
+  @param[in] VendorGuid    A pointer to the vendor GUID that will be made
+                           read-only subsequently.
 
-  @retval EFI_SUCCESS           The variable specified by the VariableName and the VendorGuid was marked
-                                as pending to be read-only.
+  @retval EFI_SUCCESS           The variable specified by the VariableName and
+                                the VendorGuid was marked as pending to be
+                                read-only.
   @retval EFI_INVALID_PARAMETER VariableName or VendorGuid is NULL.
                                 Or VariableName is an empty string.
-  @retval EFI_ACCESS_DENIED     EFI_END_OF_DXE_EVENT_GROUP_GUID or EFI_EVENT_GROUP_READY_TO_BOOT has
-                                already been signaled.
-  @retval EFI_OUT_OF_RESOURCES  There is not enough resource to hold the lock request.
+  @retval EFI_ACCESS_DENIED     EFI_END_OF_DXE_EVENT_GROUP_GUID or
+                                EFI_EVENT_GROUP_READY_TO_BOOT has already been
+                                signaled.
+  @retval EFI_OUT_OF_RESOURCES  There is not enough resource to hold the lock
+                                request.
 **/
 EFI_STATUS
 EFIAPI
 VariableLockRequestToLock (
-  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL *This,
-  IN       CHAR16                       *VariableName,
-  IN       EFI_GUID                     *VendorGuid
+  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL  *This,
+  IN CHAR16                              *VariableName,
+  IN EFI_GUID                            *VendorGuid
   )
 {
-  EFI_STATUS              Status;
-  VARIABLE_POLICY_ENTRY   *NewPolicy;
+  EFI_STATUS             Status;
+  VARIABLE_POLICY_ENTRY  *NewPolicy;
+
+  DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!! %a() will go away soon!\n", __FUNCTION__));
+  DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!! Please move to use Variable Policy!\n"));
+  DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!! Variable: %g %s\n", VendorGuid, VariableName));
 
   NewPolicy = NULL;
-  Status = CreateBasicVariablePolicy( VendorGuid,
-                                      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,
-                                      &NewPolicy );
+  Status = CreateBasicVariablePolicy(
+             VendorGuid,
+             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,
+             &NewPolicy
+             );
   if (!EFI_ERROR( Status )) {
-    Status = RegisterVariablePolicy( NewPolicy );
+    Status = RegisterVariablePolicy (NewPolicy);
+
+    //
+    // If the error returned is EFI_ALREADY_STARTED, we need to check the
+    // current database for the variable and see whether it's locked. If it's
+    // locked, we're still fine, but also generate a DEBUG_ERROR message so the
+    // duplicate lock can be removed.
+    //
+    if (Status == EFI_ALREADY_STARTED) {
+      Status = ValidateSetVariable (VariableName, VendorGuid, 0, 0, NULL);
+      if (Status == EFI_WRITE_PROTECTED) {
+        DEBUG ((DEBUG_ERROR, "  Variable: %g %s is already locked!\n", VendorGuid, VariableName));
+        Status = EFI_SUCCESS;
+      } else {
+        DEBUG ((DEBUG_ERROR, "  Variable: %g %s can not be locked!\n", VendorGuid, VariableName));
+        Status = EFI_ACCESS_DENIED;
+      }
+    }
   }
-  if (EFI_ERROR( Status )) {
+  if (EFI_ERROR (Status)) {
     DEBUG(( DEBUG_ERROR, "%a - Failed to lock variable %s! %r\n", __FUNCTION__, VariableName, Status ));
-    ASSERT_EFI_ERROR( Status );
   }
   if (NewPolicy != NULL) {
     FreePool( NewPolicy );
-- 
2.29.2.windows.2


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

* [Patch v4 2/2] MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit Tests
  2020-12-11  8:01 [Patch v4 0/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior Michael D Kinney
  2020-12-11  8:01 ` [Patch v4 1/2] " Michael D Kinney
@ 2020-12-11  8:01 ` Michael D Kinney
  2020-12-14  1:56   ` 回复: [edk2-devel] " gaoliming
  2020-12-11  8:12 ` [Patch v4 0/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior Wu, Hao A
  2 siblings, 1 reply; 8+ messages in thread
From: Michael D Kinney @ 2020-12-11  8:01 UTC (permalink / raw)
  To: devel; +Cc: Hao A Wu, Liming Gao, Bret Barkelew

https://bugzilla.tianocore.org/show_bug.cgi?id=3111

Add host based unit tests for the multiple lock case using Variable Lock
Protocol, Variable Policy Protocol, and mixes of Variable Lock Protocol
and Variable Policy Protocol.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Bret Barkelew <Bret.Barkelew@microsoft.com>
---
 MdeModulePkg/Test/MdeModulePkgHostTest.dsc    |  11 +
 .../VariableLockRequestToLockUnitTest.c       | 565 ++++++++++++++++++
 .../VariableLockRequestToLockUnitTest.inf     |  36 ++
 3 files changed, 612 insertions(+)
 create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.c
 create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.inf

diff --git a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
index 72a119db4568..4da4692c8451 100644
--- a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
+++ b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
@@ -19,6 +19,9 @@ [Defines]
 
 !include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc
 
+[LibraryClasses]
+  SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
+
 [Components]
   MdeModulePkg/Library/DxeResetSystemLib/UnitTest/MockUefiRuntimeServicesTableLib.inf
 
@@ -30,3 +33,11 @@ [Components]
       ResetSystemLib|MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
       UefiRuntimeServicesTableLib|MdeModulePkg/Library/DxeResetSystemLib/UnitTest/MockUefiRuntimeServicesTableLib.inf
   }
+
+  MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.inf {
+    <LibraryClasses>
+      VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
+      VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+    <PcdsFixedAtBuild>
+      gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisable|TRUE
+  }
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.c
new file mode 100644
index 000000000000..44d70e639d77
--- /dev/null
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.c
@@ -0,0 +1,565 @@
+/** @file
+  This is a host-based unit test for the VariableLockRequestToLock shim.
+
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <stdio.h>
+#include <string.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <Uefi.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UnitTestLib.h>
+#include <Library/VariablePolicyLib.h>
+#include <Library/VariablePolicyHelperLib.h>
+
+#include <Protocol/VariableLock.h>
+
+#define UNIT_TEST_NAME        "VarPol/VarLock Shim Unit Test"
+#define UNIT_TEST_VERSION     "1.0"
+
+///=== CODE UNDER TEST ===========================================================================
+
+EFI_STATUS
+EFIAPI
+VariableLockRequestToLock (
+  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL  *This,
+  IN       CHAR16                        *VariableName,
+  IN       EFI_GUID                      *VendorGuid
+  );
+
+///=== TEST DATA ==================================================================================
+
+//
+// Test GUID 1 {F955BA2D-4A2C-480C-BFD1-3CC522610592}
+//
+EFI_GUID  mTestGuid1 = {
+  0xf955ba2d, 0x4a2c, 0x480c, {0xbf, 0xd1, 0x3c, 0xc5, 0x22, 0x61, 0x5, 0x92}
+};
+
+//
+// Test GUID 2 {2DEA799E-5E73-43B9-870E-C945CE82AF3A}
+//
+EFI_GUID  mTestGuid2 = {
+  0x2dea799e, 0x5e73, 0x43b9, {0x87, 0xe, 0xc9, 0x45, 0xce, 0x82, 0xaf, 0x3a}
+};
+
+//
+// Test GUID 3 {698A2BFD-A616-482D-B88C-7100BD6682A9}
+//
+EFI_GUID  mTestGuid3 = {
+  0x698a2bfd, 0xa616, 0x482d, {0xb8, 0x8c, 0x71, 0x0, 0xbd, 0x66, 0x82, 0xa9}
+};
+
+#define TEST_VAR_1_NAME              L"TestVar1"
+#define TEST_VAR_2_NAME              L"TestVar2"
+#define TEST_VAR_3_NAME              L"TestVar3"
+
+#define TEST_POLICY_ATTRIBUTES_NULL  0
+#define TEST_POLICY_MIN_SIZE_NULL    0
+#define TEST_POLICY_MAX_SIZE_NULL    MAX_UINT32
+
+#define TEST_POLICY_MIN_SIZE_10      10
+#define TEST_POLICY_MAX_SIZE_200     200
+
+///=== HELPER FUNCTIONS ===========================================================================
+
+/**
+  Mocked version of GetVariable, for testing.
+
+  @param  VariableName
+  @param  VendorGuid
+  @param  Attributes
+  @param  DataSize
+  @param  Data
+**/
+EFI_STATUS
+EFIAPI
+StubGetVariableNull (
+  IN     CHAR16    *VariableName,
+  IN     EFI_GUID  *VendorGuid,
+  OUT    UINT32    *Attributes,  OPTIONAL
+  IN OUT UINTN     *DataSize,
+  OUT    VOID      *Data         OPTIONAL
+  )
+{
+  UINT32      MockedAttr;
+  UINTN       MockedDataSize;
+  VOID        *MockedData;
+  EFI_STATUS  MockedReturn;
+
+  check_expected_ptr (VariableName);
+  check_expected_ptr (VendorGuid);
+  check_expected_ptr (DataSize);
+
+  MockedAttr     = (UINT32)mock();
+  MockedDataSize = (UINTN)mock();
+  MockedData     = (VOID*)(UINTN)mock();
+  MockedReturn   = (EFI_STATUS)mock();
+
+  if (Attributes != NULL) {
+    *Attributes = MockedAttr;
+  }
+  if (Data != NULL && !EFI_ERROR (MockedReturn)) {
+    CopyMem (Data, MockedData, MockedDataSize);
+  }
+
+  *DataSize = MockedDataSize;
+
+  return MockedReturn;
+}
+
+//
+// Anything you think might be helpful that isn't a test itself.
+//
+
+/**
+  This is a common setup function that will ensure the library is always
+  initialized with the stubbed GetVariable.
+
+  Not used by all test cases, but by most.
+
+  @param[in]  Context  Unit test case context
+**/
+STATIC
+UNIT_TEST_STATUS
+EFIAPI
+LibInitMocked (
+  IN UNIT_TEST_CONTEXT  Context
+  )
+{
+  return EFI_ERROR (InitVariablePolicyLib (StubGetVariableNull)) ? UNIT_TEST_ERROR_PREREQUISITE_NOT_MET : UNIT_TEST_PASSED;
+}
+
+/**
+  Common cleanup function to make sure that the library is always de-initialized
+  prior to the next test case.
+
+  @param[in]  Context  Unit test case context
+**/
+STATIC
+VOID
+EFIAPI
+LibCleanup (
+  IN UNIT_TEST_CONTEXT  Context
+  )
+{
+  DeinitVariablePolicyLib();
+}
+
+///=== TEST CASES =================================================================================
+
+///===== SHIM SUITE ===========================================================
+
+/**
+  Test Case that locks a single variable using the Variable Lock Protocol.
+  The call is expected to succeed.
+
+  @param[in]  Context  Unit test case context
+**/
+UNIT_TEST_STATUS
+EFIAPI
+LockingWithoutAnyPoliciesShouldSucceed (
+  IN UNIT_TEST_CONTEXT  Context
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Test Case that locks the same variable twice using the Variable Lock Protocol.
+  Both calls are expected to succeed.
+
+  @param[in]  Context  Unit test case context
+  **/
+UNIT_TEST_STATUS
+EFIAPI
+LockingTwiceShouldSucceed (
+  IN UNIT_TEST_CONTEXT  Context
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Test Case that locks a variable using the Variable Policy Protocol then locks
+  the same variable using the Variable Lock Protocol.
+  Both calls are expected to succeed.
+
+  @param[in]  Context  Unit test case context
+  **/
+UNIT_TEST_STATUS
+EFIAPI
+LockingALockedVariableShouldSucceed (
+  IN UNIT_TEST_CONTEXT  Context
+  )
+{
+  EFI_STATUS             Status;
+  VARIABLE_POLICY_ENTRY  *NewEntry;
+
+  //
+  // Create a variable policy that locks the variable.
+  //
+  Status = CreateBasicVariablePolicy (
+             &mTestGuid1,
+             TEST_VAR_1_NAME,
+             TEST_POLICY_MIN_SIZE_NULL,
+             TEST_POLICY_MAX_SIZE_200,
+             TEST_POLICY_ATTRIBUTES_NULL,
+             TEST_POLICY_ATTRIBUTES_NULL,
+             VARIABLE_POLICY_TYPE_LOCK_NOW,
+             &NewEntry
+             );
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  //
+  // Register the new policy.
+  //
+  Status = RegisterVariablePolicy (NewEntry);
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  FreePool (NewEntry);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Test Case that locks a variable using the Variable Policy Protocol with a
+  policy other than LOCK_NOW then attempts to lock the same variable using the
+  Variable Lock Protocol.  The call to Variable Policy is expected to succeed
+  and the call to Variable Lock is expected to fail.
+
+  @param[in]  Context  Unit test case context
+  **/
+UNIT_TEST_STATUS
+EFIAPI
+LockingAnUnlockedVariableShouldFail (
+  IN UNIT_TEST_CONTEXT      Context
+  )
+{
+  EFI_STATUS                Status;
+  VARIABLE_POLICY_ENTRY     *NewEntry;
+
+  // Create a variable policy that locks the variable.
+  Status = CreateVarStateVariablePolicy (&mTestGuid1,
+                                         TEST_VAR_1_NAME,
+                                         TEST_POLICY_MIN_SIZE_NULL,
+                                         TEST_POLICY_MAX_SIZE_200,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         &mTestGuid2,
+                                         1,
+                                         TEST_VAR_2_NAME,
+                                         &NewEntry);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  // Register the new policy.
+  Status = RegisterVariablePolicy (NewEntry);
+
+ // Configure the stub to not care about parameters. We're testing errors.
+  expect_any_always( StubGetVariableNull, VariableName );
+  expect_any_always( StubGetVariableNull, VendorGuid );
+  expect_any_always( StubGetVariableNull, DataSize );
+
+  // With a policy, make sure that writes still work, since the variable doesn't exist.
+  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );    // Attributes
+  will_return( StubGetVariableNull, 0 );                              // Size
+  will_return( StubGetVariableNull, NULL );                           // DataPtr
+  will_return( StubGetVariableNull, EFI_NOT_FOUND);                   // Status
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_TRUE (EFI_ERROR (Status));
+
+  FreePool (NewEntry);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Test Case that locks a variable using the Variable Policy Protocol with a
+  policy other than LOCK_NOW, but is currently locked.  Then attempts to lock
+  the same variable using the Variable Lock Protocol.  The call to Variable
+  Policy is expected to succeed and the call to Variable Lock also expected to
+  succeed.
+
+  @param[in]  Context  Unit test case context
+  **/
+UNIT_TEST_STATUS
+EFIAPI
+LockingALockedVariableWithMatchingDataShouldSucceed (
+  IN UNIT_TEST_CONTEXT      Context
+  )
+{
+  EFI_STATUS                Status;
+  VARIABLE_POLICY_ENTRY     *NewEntry;
+  UINT8                     Data;
+
+  // Create a variable policy that locks the variable.
+  Status = CreateVarStateVariablePolicy (&mTestGuid1,
+                                         TEST_VAR_1_NAME,
+                                         TEST_POLICY_MIN_SIZE_NULL,
+                                         TEST_POLICY_MAX_SIZE_200,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         &mTestGuid2,
+                                         1,
+                                         TEST_VAR_2_NAME,
+                                         &NewEntry);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  // Register the new policy.
+  Status = RegisterVariablePolicy (NewEntry);
+
+ // Configure the stub to not care about parameters. We're testing errors.
+  expect_any_always( StubGetVariableNull, VariableName );
+  expect_any_always( StubGetVariableNull, VendorGuid );
+  expect_any_always( StubGetVariableNull, DataSize );
+
+  // With a policy, make sure that writes still work, since the variable doesn't exist.
+  Data = 1;
+  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );    // Attributes
+  will_return( StubGetVariableNull, sizeof (Data) );                  // Size
+  will_return( StubGetVariableNull, &Data );                          // DataPtr
+  will_return( StubGetVariableNull, EFI_SUCCESS);                     // Status
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_TRUE (!EFI_ERROR (Status));
+
+  FreePool (NewEntry);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Test Case that locks a variable using the Variable Policy Protocol with a
+  policy other than LOCK_NOW, but variable data does not match.  Then attempts
+  to lock the same variable using the Variable Lock Protocol.  The call to
+  Variable Policy is expected to succeed and the call to Variable Lock is
+  expected to fail.
+
+  @param[in]  Context  Unit test case context
+  **/
+UNIT_TEST_STATUS
+EFIAPI
+LockingALockedVariableWithNonMatchingDataShouldFail (
+  IN UNIT_TEST_CONTEXT      Context
+  )
+{
+  EFI_STATUS                Status;
+  VARIABLE_POLICY_ENTRY     *NewEntry;
+  UINT8                     Data;
+
+  // Create a variable policy that locks the variable.
+  Status = CreateVarStateVariablePolicy (&mTestGuid1,
+                                         TEST_VAR_1_NAME,
+                                         TEST_POLICY_MIN_SIZE_NULL,
+                                         TEST_POLICY_MAX_SIZE_200,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         &mTestGuid2,
+                                         1,
+                                         TEST_VAR_2_NAME,
+                                         &NewEntry);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  // Register the new policy.
+  Status = RegisterVariablePolicy (NewEntry);
+
+ // Configure the stub to not care about parameters. We're testing errors.
+  expect_any_always( StubGetVariableNull, VariableName );
+  expect_any_always( StubGetVariableNull, VendorGuid );
+  expect_any_always( StubGetVariableNull, DataSize );
+
+  // With a policy, make sure that writes still work, since the variable doesn't exist.
+  Data = 2;
+  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );    // Attributes
+  will_return( StubGetVariableNull, sizeof (Data) );                  // Size
+  will_return( StubGetVariableNull, &Data );                          // DataPtr
+  will_return( StubGetVariableNull, EFI_SUCCESS);                     // Status
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_TRUE (EFI_ERROR (Status));
+
+  FreePool (NewEntry);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Test Case that locks a variable using Variable Lock Protocol Policy Protocol
+  then and then attempts to lock the same variable using the Variable Policy
+  Protocol.  The call to Variable Lock is expected to succeed and the call to
+  Variable Policy is expected to fail.
+
+  @param[in]  Context  Unit test case context
+  **/
+UNIT_TEST_STATUS
+EFIAPI
+SettingPolicyForALockedVariableShouldFail (
+  IN UNIT_TEST_CONTEXT      Context
+  )
+{
+  EFI_STATUS                Status;
+  VARIABLE_POLICY_ENTRY     *NewEntry;
+
+  // Lock the variable.
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  // Create a variable policy that locks the variable.
+  Status = CreateVarStateVariablePolicy (&mTestGuid1,
+                                         TEST_VAR_1_NAME,
+                                         TEST_POLICY_MIN_SIZE_NULL,
+                                         TEST_POLICY_MAX_SIZE_200,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         &mTestGuid2,
+                                         1,
+                                         TEST_VAR_2_NAME,
+                                         &NewEntry);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  // Register the new policy.
+  Status = RegisterVariablePolicy (NewEntry);
+  UT_ASSERT_TRUE (EFI_ERROR (Status));
+
+  FreePool (NewEntry);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Main entry point to this unit test application.
+
+  Sets up and runs the test suites.
+**/
+VOID
+EFIAPI
+UnitTestMain (
+  VOID
+  )
+{
+  EFI_STATUS                  Status;
+  UNIT_TEST_FRAMEWORK_HANDLE  Framework;
+  UNIT_TEST_SUITE_HANDLE      ShimTests;
+
+  Framework = NULL;
+
+  DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_NAME, UNIT_TEST_VERSION));
+
+  //
+  // Start setting up the test framework for running the tests.
+  //
+  Status = InitUnitTestFramework (&Framework, UNIT_TEST_NAME, gEfiCallerBaseName, UNIT_TEST_VERSION);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed in InitUnitTestFramework. Status = %r\n", Status));
+    goto EXIT;
+  }
+
+  //
+  // Add all test suites and tests.
+  //
+  Status = CreateUnitTestSuite (
+             &ShimTests, Framework,
+             "Variable Lock Shim Tests", "VarPolicy.VarLockShim", NULL, NULL
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for ShimTests\n"));
+    Status = EFI_OUT_OF_RESOURCES;
+    goto EXIT;
+  }
+  AddTestCase (
+    ShimTests,
+    "Locking a variable with no matching policies should always work", "EmptyPolicies",
+    LockingWithoutAnyPoliciesShouldSucceed, LibInitMocked, LibCleanup, NULL
+    );
+  AddTestCase (
+    ShimTests,
+    "Locking a variable twice should always work", "DoubleLock",
+    LockingTwiceShouldSucceed, LibInitMocked, LibCleanup, NULL
+    );
+  AddTestCase (
+    ShimTests,
+    "Locking a variable that's already locked by another policy should work", "LockAfterPolicy",
+    LockingALockedVariableShouldSucceed, LibInitMocked, LibCleanup, NULL
+    );
+  AddTestCase (
+    ShimTests,
+    "Locking a variable that already has an unlocked policy should fail", "LockAfterUnlockedPolicy",
+    LockingAnUnlockedVariableShouldFail, LibInitMocked, LibCleanup, NULL
+    );
+  AddTestCase (
+    ShimTests,
+    "Locking a variable that already has an locked policy should succeed", "LockAfterLockedPolicyMatchingData",
+    LockingALockedVariableWithMatchingDataShouldSucceed, LibInitMocked, LibCleanup, NULL
+    );
+  AddTestCase (
+    ShimTests,
+    "Locking a variable that already has an locked policy with matching data should succeed", "LockAfterLockedPolicyNonMatchingData",
+    LockingALockedVariableWithNonMatchingDataShouldFail, LibInitMocked, LibCleanup, NULL
+    );
+  AddTestCase (
+    ShimTests,
+    "Adding a policy for a variable that has previously been locked should always fail", "SetPolicyAfterLock",
+    SettingPolicyForALockedVariableShouldFail, LibInitMocked, LibCleanup, NULL
+    );
+
+  //
+  // Execute the tests.
+  //
+  Status = RunAllTestSuites (Framework);
+
+EXIT:
+  if (Framework != NULL) {
+    FreeUnitTestFramework (Framework);
+  }
+
+  return;
+}
+
+///
+/// Avoid ECC error for function name that starts with lower case letter
+///
+#define Main main
+
+/**
+  Standard POSIX C entry point for host based unit test execution.
+
+  @param[in] Argc  Number of arguments
+  @param[in] Argv  Array of pointers to arguments
+
+  @retval 0      Success
+  @retval other  Error
+**/
+INT32
+Main (
+  IN INT32  Argc,
+  IN CHAR8  *Argv[]
+  )
+{
+  UnitTestMain ();
+  return 0;
+}
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.inf
new file mode 100644
index 000000000000..2a659d7e1370
--- /dev/null
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.inf
@@ -0,0 +1,36 @@
+## @file
+# This is a host-based unit test for the VariableLockRequestToLock shim.
+#
+# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION         = 0x00010017
+  BASE_NAME           = VariableLockRequestToLockUnitTest
+  FILE_GUID           = A7388B6C-7274-4717-9649-BDC5DFD1FCBE
+  VERSION_STRING      = 1.0
+  MODULE_TYPE         = HOST_APPLICATION
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
+#
+
+[Sources]
+  VariableLockRequestToLockUnitTest.c
+  ../VariableLockRequestToLock.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
+
+[LibraryClasses]
+  UnitTestLib
+  DebugLib
+  VariablePolicyLib
+  VariablePolicyHelperLib
+  BaseMemoryLib
+  MemoryAllocationLib
-- 
2.29.2.windows.2


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

* Re: [Patch v4 0/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior
  2020-12-11  8:01 [Patch v4 0/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior Michael D Kinney
  2020-12-11  8:01 ` [Patch v4 1/2] " Michael D Kinney
  2020-12-11  8:01 ` [Patch v4 2/2] MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit Tests Michael D Kinney
@ 2020-12-11  8:12 ` Wu, Hao A
  2 siblings, 0 replies; 8+ messages in thread
From: Wu, Hao A @ 2020-12-11  8:12 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io; +Cc: Liming Gao, Bret Barkelew

For the series:
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu

> -----Original Message-----
> From: Michael D Kinney <michael.d.kinney@intel.com>
> Sent: Friday, December 11, 2020 4:01 PM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: [Patch v4 0/2] MdeModulePkg/Variable/RuntimeDxe: Restore
> Variable Lock Protocol behavior
> 
> New in V4
> ==========
> * Fix spelling in unit tests
> * Call ValidateSetVariable() with DataSize=0, Attributes=0
> 
> New in V3
> ==========
> * Split into 2 patches.  One for code change.  Second for unit tests.
> * Remove duplicate code and use ValidateSetVariable() to detect if a
>   variable is already locked.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=3111
> 
> The VariableLock shim currently fails if called twice because the underlying
> Variable Policy engine returns an error if a policy is set on an existing variable.
> 
> This breaks existing code which expect it to silently pass if a variable is locked
> multiple times (because it should "be locked").
> 
> Refactor the shim to confirm that the variable is indeed locked and then
> change the error to EFI_SUCCESS and generate a DEBUG_ERROR message so
> the duplicate lock can be reported in a debug log and removed.
> 
> Add host based unit tests for the multiple lock case using Variable Lock
> Protocol, Variable Policy Protocol, and mixes of Variable Lock Protocol and
> Variable Policy Protocol.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Bret Barkelew <Bret.Barkelew@microsoft.com>
> 
> Bret Barkelew (1):
>   MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol
>     behavior
> 
> Michael D Kinney (1):
>   MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit
>     Tests
> 
>  MdeModulePkg/Test/MdeModulePkgHostTest.dsc    |  11 +
>  .../VariableLockRequestToLockUnitTest.c       | 565 ++++++++++++++++++
>  .../VariableLockRequestToLockUnitTest.inf     |  36 ++
>  .../RuntimeDxe/VariableLockRequestToLock.c    |  95 +--
>  4 files changed, 671 insertions(+), 36 deletions(-)  create mode 100644
> MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> ableLockRequestToLockUnitTest.c
>  create mode 100644
> MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> ableLockRequestToLockUnitTest.inf
> 
> --
> 2.29.2.windows.2


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

* 回复: [edk2-devel] [Patch v4 1/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior
  2020-12-11  8:01 ` [Patch v4 1/2] " Michael D Kinney
@ 2020-12-14  1:39   ` gaoliming
  0 siblings, 0 replies; 8+ messages in thread
From: gaoliming @ 2020-12-14  1:39 UTC (permalink / raw)
  To: devel, michael.d.kinney; +Cc: 'Bret Barkelew', 'Hao A Wu'

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

> -----邮件原件-----
> 发件人: bounce+27952+68702+4905953+8761045@groups.io
> <bounce+27952+68702+4905953+8761045@groups.io> 代表 Michael D
> Kinney
> 发送时间: 2020年12月11日 16:01
> 收件人: devel@edk2.groups.io
> 抄送: Bret Barkelew <bret.barkelew@microsoft.com>; Hao A Wu
> <hao.a.wu@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Bret
> Barkelew <Bret.Barkelew@microsoft.com>
> 主题: [edk2-devel] [Patch v4 1/2] MdeModulePkg/Variable/RuntimeDxe:
> Restore Variable Lock Protocol behavior
> 
> From: Bret Barkelew <bret.barkelew@microsoft.com>
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=3111
> 
> The VariableLock shim currently fails if called twice because the
> underlying Variable Policy engine returns an error if a policy is set
> on an existing variable.
> 
> This breaks existing code which expect it to silently pass if a variable
> is locked multiple times (because it should "be locked").
> 
> Refactor the shim to confirm that the variable is indeed locked and then
> change the error to EFI_SUCCESS and generate a DEBUG_ERROR message so
> the duplicate lock can be reported in a debug log and removed.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Bret Barkelew <Bret.Barkelew@microsoft.com>
> ---
>  .../RuntimeDxe/VariableLockRequestToLock.c    | 95 ++++++++++++-------
>  1 file changed, 59 insertions(+), 36 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo
> ck.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo
> ck.c
> index 4aa854aaf260..7d87e50efdcd 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo
> ck.c
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo
> ck.c
> @@ -1,67 +1,90 @@
> -/** @file -- VariableLockRequestToLock.c
> -Temporary location of the RequestToLock shim code while
> -projects are moved to VariablePolicy. Should be removed when deprecated.
> +/** @file
> +  Temporary location of the RequestToLock shim code while projects
> +  are moved to VariablePolicy. Should be removed when deprecated.
> 
> -Copyright (c) Microsoft Corporation.
> -SPDX-License-Identifier: BSD-2-Clause-Patent
> +  Copyright (c) Microsoft Corporation.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
>  #include <Uefi.h>
> -
>  #include <Library/DebugLib.h>
>  #include <Library/MemoryAllocationLib.h>
> -
> -#include <Protocol/VariableLock.h>
> -
> -#include <Protocol/VariablePolicy.h>
>  #include <Library/VariablePolicyLib.h>
>  #include <Library/VariablePolicyHelperLib.h>
> -
> +#include <Protocol/VariableLock.h>
> 
>  /**
>    DEPRECATED. THIS IS ONLY HERE AS A CONVENIENCE WHILE PORTING.
> -  Mark a variable that will become read-only after leaving the DXE phase
of
> execution.
> -  Write request coming from SMM environment through
> EFI_SMM_VARIABLE_PROTOCOL is allowed.
> +  Mark a variable that will become read-only after leaving the DXE phase
of
> +  execution. Write request coming from SMM environment through
> +  EFI_SMM_VARIABLE_PROTOCOL is allowed.
> 
>    @param[in] This          The VARIABLE_LOCK_PROTOCOL instance.
> -  @param[in] VariableName  A pointer to the variable name that will be
> made read-only subsequently.
> -  @param[in] VendorGuid    A pointer to the vendor GUID that will be
> made read-only subsequently.
> +  @param[in] VariableName  A pointer to the variable name that will be
> made
> +                           read-only subsequently.
> +  @param[in] VendorGuid    A pointer to the vendor GUID that will be
> made
> +                           read-only subsequently.
> 
> -  @retval EFI_SUCCESS           The variable specified by the
> VariableName and the VendorGuid was marked
> -                                as pending to be read-only.
> +  @retval EFI_SUCCESS           The variable specified by the
> VariableName and
> +                                the VendorGuid was marked as
> pending to be
> +                                read-only.
>    @retval EFI_INVALID_PARAMETER VariableName or VendorGuid is NULL.
>                                  Or VariableName is an empty string.
> -  @retval EFI_ACCESS_DENIED
> EFI_END_OF_DXE_EVENT_GROUP_GUID or
> EFI_EVENT_GROUP_READY_TO_BOOT has
> -                                already been signaled.
> -  @retval EFI_OUT_OF_RESOURCES  There is not enough resource to hold
> the lock request.
> +  @retval EFI_ACCESS_DENIED
> EFI_END_OF_DXE_EVENT_GROUP_GUID or
> +
> EFI_EVENT_GROUP_READY_TO_BOOT has already been
> +                                signaled.
> +  @retval EFI_OUT_OF_RESOURCES  There is not enough resource to hold
> the lock
> +                                request.
>  **/
>  EFI_STATUS
>  EFIAPI
>  VariableLockRequestToLock (
> -  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL *This,
> -  IN       CHAR16                       *VariableName,
> -  IN       EFI_GUID                     *VendorGuid
> +  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL  *This,
> +  IN CHAR16                              *VariableName,
> +  IN EFI_GUID                            *VendorGuid
>    )
>  {
> -  EFI_STATUS              Status;
> -  VARIABLE_POLICY_ENTRY   *NewPolicy;
> +  EFI_STATUS             Status;
> +  VARIABLE_POLICY_ENTRY  *NewPolicy;
> +
> +  DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!! %a() will go
> away soon!\n", __FUNCTION__));
> +  DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!! Please move to
> use Variable Policy!\n"));
> +  DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!!
> Variable: %g %s\n", VendorGuid, VariableName));
> 
>    NewPolicy = NULL;
> -  Status = CreateBasicVariablePolicy( VendorGuid,
> -                                      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,
> -                                      &NewPolicy );
> +  Status = CreateBasicVariablePolicy(
> +             VendorGuid,
> +             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,
> +             &NewPolicy
> +             );
>    if (!EFI_ERROR( Status )) {
> -    Status = RegisterVariablePolicy( NewPolicy );
> +    Status = RegisterVariablePolicy (NewPolicy);
> +
> +    //
> +    // If the error returned is EFI_ALREADY_STARTED, we need to check the
> +    // current database for the variable and see whether it's locked. If
it's
> +    // locked, we're still fine, but also generate a DEBUG_ERROR message
> so the
> +    // duplicate lock can be removed.
> +    //
> +    if (Status == EFI_ALREADY_STARTED) {
> +      Status = ValidateSetVariable (VariableName, VendorGuid, 0, 0,
NULL);
> +      if (Status == EFI_WRITE_PROTECTED) {
> +        DEBUG ((DEBUG_ERROR, "  Variable: %g %s is already locked!\n",
> VendorGuid, VariableName));
> +        Status = EFI_SUCCESS;
> +      } else {
> +        DEBUG ((DEBUG_ERROR, "  Variable: %g %s can not be
> locked!\n", VendorGuid, VariableName));
> +        Status = EFI_ACCESS_DENIED;
> +      }
> +    }
>    }
> -  if (EFI_ERROR( Status )) {
> +  if (EFI_ERROR (Status)) {
>      DEBUG(( DEBUG_ERROR, "%a - Failed to lock variable %s! %r\n",
> __FUNCTION__, VariableName, Status ));
> -    ASSERT_EFI_ERROR( Status );
>    }
>    if (NewPolicy != NULL) {
>      FreePool( NewPolicy );
> --
> 2.29.2.windows.2
> 
> 
> 
> 
> 




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

* 回复: [edk2-devel] [Patch v4 2/2] MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit Tests
  2020-12-11  8:01 ` [Patch v4 2/2] MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit Tests Michael D Kinney
@ 2020-12-14  1:56   ` gaoliming
  2020-12-14 17:52     ` Michael D Kinney
  0 siblings, 1 reply; 8+ messages in thread
From: gaoliming @ 2020-12-14  1:56 UTC (permalink / raw)
  To: devel, michael.d.kinney; +Cc: 'Hao A Wu', 'Bret Barkelew'

Mike and Bret:
  Seemly, this is the first unit test case for the logic in the driver. This
unit test module needs to include the driver source file. Then, its INF file
has to use the below style to include the source file. But, edk2 module INF
doesn't recommend to specify the source file with the relative upper
directory (..). How about add VariableLockRequestToLockUnitTest.inf into
MdeModulePkg\Universal\Variable\RuntimeDxe\ directory? If so, this INF can
directly specify the source file in [Sources] section. 

> +[Sources]
> +  VariableLockRequestToLockUnitTest.c
> +  ../VariableLockRequestToLock.c
> + 

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+68703+4905953+8761045@groups.io
> <bounce+27952+68703+4905953+8761045@groups.io> 代表 Michael D
> Kinney
> 发送时间: 2020年12月11日 16:01
> 收件人: devel@edk2.groups.io
> 抄送: Hao A Wu <hao.a.wu@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Bret Barkelew <Bret.Barkelew@microsoft.com>
> 主题: [edk2-devel] [Patch v4 2/2] MdeModulePkg/Variable/RuntimeDxe: Add
> Variable Lock Protocol Unit Tests
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=3111
> 
> Add host based unit tests for the multiple lock case using Variable Lock
> Protocol, Variable Policy Protocol, and mixes of Variable Lock Protocol
> and Variable Policy Protocol.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Bret Barkelew <Bret.Barkelew@microsoft.com>
> ---
>  MdeModulePkg/Test/MdeModulePkgHostTest.dsc    |  11 +
>  .../VariableLockRequestToLockUnitTest.c       | 565
> ++++++++++++++++++
>  .../VariableLockRequestToLockUnitTest.inf     |  36 ++
>  3 files changed, 612 insertions(+)
>  create mode 100644
> MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Variab
> leLockRequestToLockUnitTest.c
>  create mode 100644
> MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Variab
> leLockRequestToLockUnitTest.inf
> 
> diff --git a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> index 72a119db4568..4da4692c8451 100644
> --- a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> +++ b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> @@ -19,6 +19,9 @@ [Defines]
> 
>  !include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc
> 
> +[LibraryClasses]
> +  SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> +
>  [Components]
> 
> MdeModulePkg/Library/DxeResetSystemLib/UnitTest/MockUefiRuntimeServi
> cesTableLib.inf
> 
> @@ -30,3 +33,11 @@ [Components]
> 
> ResetSystemLib|MdeModulePkg/Library/DxeResetSystemLib/DxeResetSyste
> mLib.inf
> 
> UefiRuntimeServicesTableLib|MdeModulePkg/Library/DxeResetSystemLib/U
> nitTest/MockUefiRuntimeServicesTableLib.inf
>    }
> +
> +
> MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Variab
> leLockRequestToLockUnitTest.inf {
> +    <LibraryClasses>
> +
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib
> .inf
> +
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Vari
> ablePolicyHelperLib.inf
> +    <PcdsFixedAtBuild>
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisab
> le|TRUE
> +  }
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> ableLockRequestToLockUnitTest.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> ableLockRequestToLockUnitTest.c
> new file mode 100644
> index 000000000000..44d70e639d77
> --- /dev/null
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> ableLockRequestToLockUnitTest.c
> @@ -0,0 +1,565 @@
> +/** @file
> +  This is a host-based unit test for the VariableLockRequestToLock shim.
> +
> +  Copyright (c) Microsoft Corporation.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdarg.h>
> +#include <stddef.h>
> +#include <setjmp.h>
> +#include <cmocka.h>
> +
> +#include <Uefi.h>
> +#include <Library/DebugLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UnitTestLib.h>
> +#include <Library/VariablePolicyLib.h>
> +#include <Library/VariablePolicyHelperLib.h>
> +
> +#include <Protocol/VariableLock.h>
> +
> +#define UNIT_TEST_NAME        "VarPol/VarLock Shim Unit Test"
> +#define UNIT_TEST_VERSION     "1.0"
> +
> +///=== CODE UNDER TEST
> ==============================================================
> =============
> +
> +EFI_STATUS
> +EFIAPI
> +VariableLockRequestToLock (
> +  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL  *This,
> +  IN       CHAR16                        *VariableName,
> +  IN       EFI_GUID                      *VendorGuid
> +  );
> +
> +///=== TEST DATA
> ==============================================================
> ====================
> +
> +//
> +// Test GUID 1 {F955BA2D-4A2C-480C-BFD1-3CC522610592}
> +//
> +EFI_GUID  mTestGuid1 = {
> +  0xf955ba2d, 0x4a2c, 0x480c, {0xbf, 0xd1, 0x3c, 0xc5, 0x22, 0x61, 0x5,
> 0x92}
> +};
> +
> +//
> +// Test GUID 2 {2DEA799E-5E73-43B9-870E-C945CE82AF3A}
> +//
> +EFI_GUID  mTestGuid2 = {
> +  0x2dea799e, 0x5e73, 0x43b9, {0x87, 0xe, 0xc9, 0x45, 0xce, 0x82, 0xaf,
> 0x3a}
> +};
> +
> +//
> +// Test GUID 3 {698A2BFD-A616-482D-B88C-7100BD6682A9}
> +//
> +EFI_GUID  mTestGuid3 = {
> +  0x698a2bfd, 0xa616, 0x482d, {0xb8, 0x8c, 0x71, 0x0, 0xbd, 0x66, 0x82,
> 0xa9}
> +};
> +
> +#define TEST_VAR_1_NAME              L"TestVar1"
> +#define TEST_VAR_2_NAME              L"TestVar2"
> +#define TEST_VAR_3_NAME              L"TestVar3"
> +
> +#define TEST_POLICY_ATTRIBUTES_NULL  0
> +#define TEST_POLICY_MIN_SIZE_NULL    0
> +#define TEST_POLICY_MAX_SIZE_NULL    MAX_UINT32
> +
> +#define TEST_POLICY_MIN_SIZE_10      10
> +#define TEST_POLICY_MAX_SIZE_200     200
> +
> +///=== HELPER FUNCTIONS
> ==============================================================
> =============
> +
> +/**
> +  Mocked version of GetVariable, for testing.
> +
> +  @param  VariableName
> +  @param  VendorGuid
> +  @param  Attributes
> +  @param  DataSize
> +  @param  Data
> +**/
> +EFI_STATUS
> +EFIAPI
> +StubGetVariableNull (
> +  IN     CHAR16    *VariableName,
> +  IN     EFI_GUID  *VendorGuid,
> +  OUT    UINT32    *Attributes,  OPTIONAL
> +  IN OUT UINTN     *DataSize,
> +  OUT    VOID      *Data         OPTIONAL
> +  )
> +{
> +  UINT32      MockedAttr;
> +  UINTN       MockedDataSize;
> +  VOID        *MockedData;
> +  EFI_STATUS  MockedReturn;
> +
> +  check_expected_ptr (VariableName);
> +  check_expected_ptr (VendorGuid);
> +  check_expected_ptr (DataSize);
> +
> +  MockedAttr     = (UINT32)mock();
> +  MockedDataSize = (UINTN)mock();
> +  MockedData     = (VOID*)(UINTN)mock();
> +  MockedReturn   = (EFI_STATUS)mock();
> +
> +  if (Attributes != NULL) {
> +    *Attributes = MockedAttr;
> +  }
> +  if (Data != NULL && !EFI_ERROR (MockedReturn)) {
> +    CopyMem (Data, MockedData, MockedDataSize);
> +  }
> +
> +  *DataSize = MockedDataSize;
> +
> +  return MockedReturn;
> +}
> +
> +//
> +// Anything you think might be helpful that isn't a test itself.
> +//
> +
> +/**
> +  This is a common setup function that will ensure the library is always
> +  initialized with the stubbed GetVariable.
> +
> +  Not used by all test cases, but by most.
> +
> +  @param[in]  Context  Unit test case context
> +**/
> +STATIC
> +UNIT_TEST_STATUS
> +EFIAPI
> +LibInitMocked (
> +  IN UNIT_TEST_CONTEXT  Context
> +  )
> +{
> +  return EFI_ERROR (InitVariablePolicyLib (StubGetVariableNull)) ?
> UNIT_TEST_ERROR_PREREQUISITE_NOT_MET : UNIT_TEST_PASSED;
> +}
> +
> +/**
> +  Common cleanup function to make sure that the library is always
> de-initialized
> +  prior to the next test case.
> +
> +  @param[in]  Context  Unit test case context
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +LibCleanup (
> +  IN UNIT_TEST_CONTEXT  Context
> +  )
> +{
> +  DeinitVariablePolicyLib();
> +}
> +
> +///=== TEST CASES
> ==============================================================
> ===================
> +
> +///===== SHIM SUITE
> ===========================================================
> +
> +/**
> +  Test Case that locks a single variable using the Variable Lock
Protocol.
> +  The call is expected to succeed.
> +
> +  @param[in]  Context  Unit test case context
> +**/
> +UNIT_TEST_STATUS
> +EFIAPI
> +LockingWithoutAnyPoliciesShouldSucceed (
> +  IN UNIT_TEST_CONTEXT  Context
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> &mTestGuid1);
> +  UT_ASSERT_NOT_EFI_ERROR (Status);
> +
> +  return UNIT_TEST_PASSED;
> +}
> +
> +/**
> +  Test Case that locks the same variable twice using the Variable Lock
> Protocol.
> +  Both calls are expected to succeed.
> +
> +  @param[in]  Context  Unit test case context
> +  **/
> +UNIT_TEST_STATUS
> +EFIAPI
> +LockingTwiceShouldSucceed (
> +  IN UNIT_TEST_CONTEXT  Context
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> &mTestGuid1);
> +  UT_ASSERT_NOT_EFI_ERROR (Status);
> +
> +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> &mTestGuid1);
> +  UT_ASSERT_NOT_EFI_ERROR (Status);
> +
> +  return UNIT_TEST_PASSED;
> +}
> +
> +/**
> +  Test Case that locks a variable using the Variable Policy Protocol then
locks
> +  the same variable using the Variable Lock Protocol.
> +  Both calls are expected to succeed.
> +
> +  @param[in]  Context  Unit test case context
> +  **/
> +UNIT_TEST_STATUS
> +EFIAPI
> +LockingALockedVariableShouldSucceed (
> +  IN UNIT_TEST_CONTEXT  Context
> +  )
> +{
> +  EFI_STATUS             Status;
> +  VARIABLE_POLICY_ENTRY  *NewEntry;
> +
> +  //
> +  // Create a variable policy that locks the variable.
> +  //
> +  Status = CreateBasicVariablePolicy (
> +             &mTestGuid1,
> +             TEST_VAR_1_NAME,
> +             TEST_POLICY_MIN_SIZE_NULL,
> +             TEST_POLICY_MAX_SIZE_200,
> +             TEST_POLICY_ATTRIBUTES_NULL,
> +             TEST_POLICY_ATTRIBUTES_NULL,
> +             VARIABLE_POLICY_TYPE_LOCK_NOW,
> +             &NewEntry
> +             );
> +  UT_ASSERT_NOT_EFI_ERROR (Status);
> +
> +  //
> +  // Register the new policy.
> +  //
> +  Status = RegisterVariablePolicy (NewEntry);
> +
> +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> &mTestGuid1);
> +  UT_ASSERT_NOT_EFI_ERROR (Status);
> +
> +  FreePool (NewEntry);
> +
> +  return UNIT_TEST_PASSED;
> +}
> +
> +/**
> +  Test Case that locks a variable using the Variable Policy Protocol with
a
> +  policy other than LOCK_NOW then attempts to lock the same variable
> using the
> +  Variable Lock Protocol.  The call to Variable Policy is expected to
succeed
> +  and the call to Variable Lock is expected to fail.
> +
> +  @param[in]  Context  Unit test case context
> +  **/
> +UNIT_TEST_STATUS
> +EFIAPI
> +LockingAnUnlockedVariableShouldFail (
> +  IN UNIT_TEST_CONTEXT      Context
> +  )
> +{
> +  EFI_STATUS                Status;
> +  VARIABLE_POLICY_ENTRY     *NewEntry;
> +
> +  // Create a variable policy that locks the variable.
> +  Status = CreateVarStateVariablePolicy (&mTestGuid1,
> +                                         TEST_VAR_1_NAME,
> +
> TEST_POLICY_MIN_SIZE_NULL,
> +
> TEST_POLICY_MAX_SIZE_200,
> +
> TEST_POLICY_ATTRIBUTES_NULL,
> +
> TEST_POLICY_ATTRIBUTES_NULL,
> +                                         &mTestGuid2,
> +                                         1,
> +                                         TEST_VAR_2_NAME,
> +                                         &NewEntry);
> +  UT_ASSERT_NOT_EFI_ERROR (Status);
> +
> +  // Register the new policy.
> +  Status = RegisterVariablePolicy (NewEntry);
> +
> + // Configure the stub to not care about parameters. We're testing
errors.
> +  expect_any_always( StubGetVariableNull, VariableName );
> +  expect_any_always( StubGetVariableNull, VendorGuid );
> +  expect_any_always( StubGetVariableNull, DataSize );
> +
> +  // With a policy, make sure that writes still work, since the variable
> doesn't exist.
> +  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );
> // Attributes
> +  will_return( StubGetVariableNull, 0 );
> // Size
> +  will_return( StubGetVariableNull, NULL );
> // DataPtr
> +  will_return( StubGetVariableNull, EFI_NOT_FOUND);
> // Status
> +
> +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> &mTestGuid1);
> +  UT_ASSERT_TRUE (EFI_ERROR (Status));
> +
> +  FreePool (NewEntry);
> +
> +  return UNIT_TEST_PASSED;
> +}
> +
> +/**
> +  Test Case that locks a variable using the Variable Policy Protocol with
a
> +  policy other than LOCK_NOW, but is currently locked.  Then attempts to
> lock
> +  the same variable using the Variable Lock Protocol.  The call to
Variable
> +  Policy is expected to succeed and the call to Variable Lock also
expected
> to
> +  succeed.
> +
> +  @param[in]  Context  Unit test case context
> +  **/
> +UNIT_TEST_STATUS
> +EFIAPI
> +LockingALockedVariableWithMatchingDataShouldSucceed (
> +  IN UNIT_TEST_CONTEXT      Context
> +  )
> +{
> +  EFI_STATUS                Status;
> +  VARIABLE_POLICY_ENTRY     *NewEntry;
> +  UINT8                     Data;
> +
> +  // Create a variable policy that locks the variable.
> +  Status = CreateVarStateVariablePolicy (&mTestGuid1,
> +                                         TEST_VAR_1_NAME,
> +
> TEST_POLICY_MIN_SIZE_NULL,
> +
> TEST_POLICY_MAX_SIZE_200,
> +
> TEST_POLICY_ATTRIBUTES_NULL,
> +
> TEST_POLICY_ATTRIBUTES_NULL,
> +                                         &mTestGuid2,
> +                                         1,
> +                                         TEST_VAR_2_NAME,
> +                                         &NewEntry);
> +  UT_ASSERT_NOT_EFI_ERROR (Status);
> +
> +  // Register the new policy.
> +  Status = RegisterVariablePolicy (NewEntry);
> +
> + // Configure the stub to not care about parameters. We're testing
errors.
> +  expect_any_always( StubGetVariableNull, VariableName );
> +  expect_any_always( StubGetVariableNull, VendorGuid );
> +  expect_any_always( StubGetVariableNull, DataSize );
> +
> +  // With a policy, make sure that writes still work, since the variable
> doesn't exist.
> +  Data = 1;
> +  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );
> // Attributes
> +  will_return( StubGetVariableNull, sizeof (Data) );                  //
> Size
> +  will_return( StubGetVariableNull, &Data );
> // DataPtr
> +  will_return( StubGetVariableNull, EFI_SUCCESS);
> // Status
> +
> +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> &mTestGuid1);
> +  UT_ASSERT_TRUE (!EFI_ERROR (Status));
> +
> +  FreePool (NewEntry);
> +
> +  return UNIT_TEST_PASSED;
> +}
> +
> +/**
> +  Test Case that locks a variable using the Variable Policy Protocol with
a
> +  policy other than LOCK_NOW, but variable data does not match.  Then
> attempts
> +  to lock the same variable using the Variable Lock Protocol.  The call
to
> +  Variable Policy is expected to succeed and the call to Variable Lock is
> +  expected to fail.
> +
> +  @param[in]  Context  Unit test case context
> +  **/
> +UNIT_TEST_STATUS
> +EFIAPI
> +LockingALockedVariableWithNonMatchingDataShouldFail (
> +  IN UNIT_TEST_CONTEXT      Context
> +  )
> +{
> +  EFI_STATUS                Status;
> +  VARIABLE_POLICY_ENTRY     *NewEntry;
> +  UINT8                     Data;
> +
> +  // Create a variable policy that locks the variable.
> +  Status = CreateVarStateVariablePolicy (&mTestGuid1,
> +                                         TEST_VAR_1_NAME,
> +
> TEST_POLICY_MIN_SIZE_NULL,
> +
> TEST_POLICY_MAX_SIZE_200,
> +
> TEST_POLICY_ATTRIBUTES_NULL,
> +
> TEST_POLICY_ATTRIBUTES_NULL,
> +                                         &mTestGuid2,
> +                                         1,
> +                                         TEST_VAR_2_NAME,
> +                                         &NewEntry);
> +  UT_ASSERT_NOT_EFI_ERROR (Status);
> +
> +  // Register the new policy.
> +  Status = RegisterVariablePolicy (NewEntry);
> +
> + // Configure the stub to not care about parameters. We're testing
errors.
> +  expect_any_always( StubGetVariableNull, VariableName );
> +  expect_any_always( StubGetVariableNull, VendorGuid );
> +  expect_any_always( StubGetVariableNull, DataSize );
> +
> +  // With a policy, make sure that writes still work, since the variable
> doesn't exist.
> +  Data = 2;
> +  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );
> // Attributes
> +  will_return( StubGetVariableNull, sizeof (Data) );                  //
> Size
> +  will_return( StubGetVariableNull, &Data );
> // DataPtr
> +  will_return( StubGetVariableNull, EFI_SUCCESS);
> // Status
> +
> +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> &mTestGuid1);
> +  UT_ASSERT_TRUE (EFI_ERROR (Status));
> +
> +  FreePool (NewEntry);
> +
> +  return UNIT_TEST_PASSED;
> +}
> +
> +/**
> +  Test Case that locks a variable using Variable Lock Protocol Policy
Protocol
> +  then and then attempts to lock the same variable using the Variable
Policy
> +  Protocol.  The call to Variable Lock is expected to succeed and the
call to
> +  Variable Policy is expected to fail.
> +
> +  @param[in]  Context  Unit test case context
> +  **/
> +UNIT_TEST_STATUS
> +EFIAPI
> +SettingPolicyForALockedVariableShouldFail (
> +  IN UNIT_TEST_CONTEXT      Context
> +  )
> +{
> +  EFI_STATUS                Status;
> +  VARIABLE_POLICY_ENTRY     *NewEntry;
> +
> +  // Lock the variable.
> +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> &mTestGuid1);
> +  UT_ASSERT_NOT_EFI_ERROR (Status);
> +
> +  // Create a variable policy that locks the variable.
> +  Status = CreateVarStateVariablePolicy (&mTestGuid1,
> +                                         TEST_VAR_1_NAME,
> +
> TEST_POLICY_MIN_SIZE_NULL,
> +
> TEST_POLICY_MAX_SIZE_200,
> +
> TEST_POLICY_ATTRIBUTES_NULL,
> +
> TEST_POLICY_ATTRIBUTES_NULL,
> +                                         &mTestGuid2,
> +                                         1,
> +                                         TEST_VAR_2_NAME,
> +                                         &NewEntry);
> +  UT_ASSERT_NOT_EFI_ERROR (Status);
> +
> +  // Register the new policy.
> +  Status = RegisterVariablePolicy (NewEntry);
> +  UT_ASSERT_TRUE (EFI_ERROR (Status));
> +
> +  FreePool (NewEntry);
> +
> +  return UNIT_TEST_PASSED;
> +}
> +
> +/**
> +  Main entry point to this unit test application.
> +
> +  Sets up and runs the test suites.
> +**/
> +VOID
> +EFIAPI
> +UnitTestMain (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                  Status;
> +  UNIT_TEST_FRAMEWORK_HANDLE  Framework;
> +  UNIT_TEST_SUITE_HANDLE      ShimTests;
> +
> +  Framework = NULL;
> +
> +  DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_NAME,
> UNIT_TEST_VERSION));
> +
> +  //
> +  // Start setting up the test framework for running the tests.
> +  //
> +  Status = InitUnitTestFramework (&Framework, UNIT_TEST_NAME,
> gEfiCallerBaseName, UNIT_TEST_VERSION);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Failed in InitUnitTestFramework. Status
> = %r\n", Status));
> +    goto EXIT;
> +  }
> +
> +  //
> +  // Add all test suites and tests.
> +  //
> +  Status = CreateUnitTestSuite (
> +             &ShimTests, Framework,
> +             "Variable Lock Shim Tests", "VarPolicy.VarLockShim", NULL,
> NULL
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for
> ShimTests\n"));
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto EXIT;
> +  }
> +  AddTestCase (
> +    ShimTests,
> +    "Locking a variable with no matching policies should always work",
> "EmptyPolicies",
> +    LockingWithoutAnyPoliciesShouldSucceed, LibInitMocked, LibCleanup,
> NULL
> +    );
> +  AddTestCase (
> +    ShimTests,
> +    "Locking a variable twice should always work", "DoubleLock",
> +    LockingTwiceShouldSucceed, LibInitMocked, LibCleanup, NULL
> +    );
> +  AddTestCase (
> +    ShimTests,
> +    "Locking a variable that's already locked by another policy should
work",
> "LockAfterPolicy",
> +    LockingALockedVariableShouldSucceed, LibInitMocked, LibCleanup,
> NULL
> +    );
> +  AddTestCase (
> +    ShimTests,
> +    "Locking a variable that already has an unlocked policy should fail",
> "LockAfterUnlockedPolicy",
> +    LockingAnUnlockedVariableShouldFail, LibInitMocked, LibCleanup,
> NULL
> +    );
> +  AddTestCase (
> +    ShimTests,
> +    "Locking a variable that already has an locked policy should
succeed",
> "LockAfterLockedPolicyMatchingData",
> +    LockingALockedVariableWithMatchingDataShouldSucceed,
> LibInitMocked, LibCleanup, NULL
> +    );
> +  AddTestCase (
> +    ShimTests,
> +    "Locking a variable that already has an locked policy with matching
> data should succeed", "LockAfterLockedPolicyNonMatchingData",
> +    LockingALockedVariableWithNonMatchingDataShouldFail,
> LibInitMocked, LibCleanup, NULL
> +    );
> +  AddTestCase (
> +    ShimTests,
> +    "Adding a policy for a variable that has previously been locked
should
> always fail", "SetPolicyAfterLock",
> +    SettingPolicyForALockedVariableShouldFail, LibInitMocked, LibCleanup,
> NULL
> +    );
> +
> +  //
> +  // Execute the tests.
> +  //
> +  Status = RunAllTestSuites (Framework);
> +
> +EXIT:
> +  if (Framework != NULL) {
> +    FreeUnitTestFramework (Framework);
> +  }
> +
> +  return;
> +}
> +
> +///
> +/// Avoid ECC error for function name that starts with lower case letter
> +///
> +#define Main main
> +
> +/**
> +  Standard POSIX C entry point for host based unit test execution.
> +
> +  @param[in] Argc  Number of arguments
> +  @param[in] Argv  Array of pointers to arguments
> +
> +  @retval 0      Success
> +  @retval other  Error
> +**/
> +INT32
> +Main (
> +  IN INT32  Argc,
> +  IN CHAR8  *Argv[]
> +  )
> +{
> +  UnitTestMain ();
> +  return 0;
> +}
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> ableLockRequestToLockUnitTest.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> ableLockRequestToLockUnitTest.inf
> new file mode 100644
> index 000000000000..2a659d7e1370
> --- /dev/null
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> ableLockRequestToLockUnitTest.inf
> @@ -0,0 +1,36 @@
> +## @file
> +# This is a host-based unit test for the VariableLockRequestToLock shim.
> +#
> +# Copyright (c) Microsoft Corporation.
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> +  INF_VERSION         = 0x00010017
> +  BASE_NAME           = VariableLockRequestToLockUnitTest
> +  FILE_GUID           = A7388B6C-7274-4717-9649-BDC5DFD1FCBE
> +  VERSION_STRING      = 1.0
> +  MODULE_TYPE         = HOST_APPLICATION
> +
> +#
> +# The following information is for reference only and not required by the
> build tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
> +#
> +
> +[Sources]
> +  VariableLockRequestToLockUnitTest.c
> +  ../VariableLockRequestToLock.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
> +
> +[LibraryClasses]
> +  UnitTestLib
> +  DebugLib
> +  VariablePolicyLib
> +  VariablePolicyHelperLib
> +  BaseMemoryLib
> +  MemoryAllocationLib
> --
> 2.29.2.windows.2
> 
> 
> 
> 
> 




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

* Re: [edk2-devel] [Patch v4 2/2] MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit Tests
  2020-12-14  1:56   ` 回复: [edk2-devel] " gaoliming
@ 2020-12-14 17:52     ` Michael D Kinney
  2020-12-15  0:56       ` 回复: " gaoliming
  0 siblings, 1 reply; 8+ messages in thread
From: Michael D Kinney @ 2020-12-14 17:52 UTC (permalink / raw)
  To: devel@edk2.groups.io, gaoliming@byosoft.com.cn, Kinney, Michael D
  Cc: Wu, Hao A, 'Bret Barkelew'

Hi Liming,

I agree it breaks the rules to use .. in the [Sources] section of an INF file.

However, I do think it is important to isolate all unit test related code (including INFs)
into their own Test directories so developers can easily distinguish source code and INFs
associated with FW implementations from unit tests and potentially set up sparse check out
rules or filters so code searches can be configured to only search FW code artifacts.

There is a host based unit test feature gap that needs to be addresses for module scoped
testing vs library scoped testing.  Library scoped testing can use lib class/lib instance
mapping in host based unit test DSC file to pull lib interfaces into a host based unit test.
For module scoped testing today, we do not have access to the module code as a library, so
you see the example here of referencing a module source file from a unit test.

I would prefer we allow this code remain "as is" today and enter 2 new BZs:

* Add better support for module scoped testing to UnitTestFrameworkPkg that avoids the
  need to use .. in [Sources] sections of host based unit test INF files.

* Remove use of .. from INF in host based unit test of Variable Lock Protocol and 
  Variable Policy Protocol in MdeModulePkg/Universal/Variable/RuntimeDxe/Test.

Best regards,

Mike


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
> Sent: Sunday, December 13, 2020 5:56 PM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Wu, Hao A <hao.a.wu@intel.com>; 'Bret Barkelew' <Bret.Barkelew@microsoft.com>
> Subject: 回复: [edk2-devel] [Patch v4 2/2] MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit Tests
> 
> Mike and Bret:
>   Seemly, this is the first unit test case for the logic in the driver. This
> unit test module needs to include the driver source file. Then, its INF file
> has to use the below style to include the source file. But, edk2 module INF
> doesn't recommend to specify the source file with the relative upper
> directory (..). How about add VariableLockRequestToLockUnitTest.inf into
> MdeModulePkg\Universal\Variable\RuntimeDxe\ directory? If so, this INF can
> directly specify the source file in [Sources] section.
> 
> > +[Sources]
> > +  VariableLockRequestToLockUnitTest.c
> > +  ../VariableLockRequestToLock.c
> > +
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: bounce+27952+68703+4905953+8761045@groups.io
> > <bounce+27952+68703+4905953+8761045@groups.io> 代表 Michael D
> > Kinney
> > 发送时间: 2020年12月11日 16:01
> > 收件人: devel@edk2.groups.io
> > 抄送: Hao A Wu <hao.a.wu@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>; Bret Barkelew <Bret.Barkelew@microsoft.com>
> > 主题: [edk2-devel] [Patch v4 2/2] MdeModulePkg/Variable/RuntimeDxe: Add
> > Variable Lock Protocol Unit Tests
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=3111
> >
> > Add host based unit tests for the multiple lock case using Variable Lock
> > Protocol, Variable Policy Protocol, and mixes of Variable Lock Protocol
> > and Variable Policy Protocol.
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Signed-off-by: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > ---
> >  MdeModulePkg/Test/MdeModulePkgHostTest.dsc    |  11 +
> >  .../VariableLockRequestToLockUnitTest.c       | 565
> > ++++++++++++++++++
> >  .../VariableLockRequestToLockUnitTest.inf     |  36 ++
> >  3 files changed, 612 insertions(+)
> >  create mode 100644
> > MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Variab
> > leLockRequestToLockUnitTest.c
> >  create mode 100644
> > MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Variab
> > leLockRequestToLockUnitTest.inf
> >
> > diff --git a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> > b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> > index 72a119db4568..4da4692c8451 100644
> > --- a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> > +++ b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> > @@ -19,6 +19,9 @@ [Defines]
> >
> >  !include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc
> >
> > +[LibraryClasses]
> > +  SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> > +
> >  [Components]
> >
> > MdeModulePkg/Library/DxeResetSystemLib/UnitTest/MockUefiRuntimeServi
> > cesTableLib.inf
> >
> > @@ -30,3 +33,11 @@ [Components]
> >
> > ResetSystemLib|MdeModulePkg/Library/DxeResetSystemLib/DxeResetSyste
> > mLib.inf
> >
> > UefiRuntimeServicesTableLib|MdeModulePkg/Library/DxeResetSystemLib/U
> > nitTest/MockUefiRuntimeServicesTableLib.inf
> >    }
> > +
> > +
> > MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Variab
> > leLockRequestToLockUnitTest.inf {
> > +    <LibraryClasses>
> > +
> > VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib
> > .inf
> > +
> > VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Vari
> > ablePolicyHelperLib.inf
> > +    <PcdsFixedAtBuild>
> > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisab
> > le|TRUE
> > +  }
> > diff --git
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> > ableLockRequestToLockUnitTest.c
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> > ableLockRequestToLockUnitTest.c
> > new file mode 100644
> > index 000000000000..44d70e639d77
> > --- /dev/null
> > +++
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> > ableLockRequestToLockUnitTest.c
> > @@ -0,0 +1,565 @@
> > +/** @file
> > +  This is a host-based unit test for the VariableLockRequestToLock shim.
> > +
> > +  Copyright (c) Microsoft Corporation.
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <stdarg.h>
> > +#include <stddef.h>
> > +#include <setjmp.h>
> > +#include <cmocka.h>
> > +
> > +#include <Uefi.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/BaseMemoryLib.h>
> > +#include <Library/MemoryAllocationLib.h>
> > +#include <Library/UnitTestLib.h>
> > +#include <Library/VariablePolicyLib.h>
> > +#include <Library/VariablePolicyHelperLib.h>
> > +
> > +#include <Protocol/VariableLock.h>
> > +
> > +#define UNIT_TEST_NAME        "VarPol/VarLock Shim Unit Test"
> > +#define UNIT_TEST_VERSION     "1.0"
> > +
> > +///=== CODE UNDER TEST
> > ==============================================================
> > =============
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +VariableLockRequestToLock (
> > +  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL  *This,
> > +  IN       CHAR16                        *VariableName,
> > +  IN       EFI_GUID                      *VendorGuid
> > +  );
> > +
> > +///=== TEST DATA
> > ==============================================================
> > ====================
> > +
> > +//
> > +// Test GUID 1 {F955BA2D-4A2C-480C-BFD1-3CC522610592}
> > +//
> > +EFI_GUID  mTestGuid1 = {
> > +  0xf955ba2d, 0x4a2c, 0x480c, {0xbf, 0xd1, 0x3c, 0xc5, 0x22, 0x61, 0x5,
> > 0x92}
> > +};
> > +
> > +//
> > +// Test GUID 2 {2DEA799E-5E73-43B9-870E-C945CE82AF3A}
> > +//
> > +EFI_GUID  mTestGuid2 = {
> > +  0x2dea799e, 0x5e73, 0x43b9, {0x87, 0xe, 0xc9, 0x45, 0xce, 0x82, 0xaf,
> > 0x3a}
> > +};
> > +
> > +//
> > +// Test GUID 3 {698A2BFD-A616-482D-B88C-7100BD6682A9}
> > +//
> > +EFI_GUID  mTestGuid3 = {
> > +  0x698a2bfd, 0xa616, 0x482d, {0xb8, 0x8c, 0x71, 0x0, 0xbd, 0x66, 0x82,
> > 0xa9}
> > +};
> > +
> > +#define TEST_VAR_1_NAME              L"TestVar1"
> > +#define TEST_VAR_2_NAME              L"TestVar2"
> > +#define TEST_VAR_3_NAME              L"TestVar3"
> > +
> > +#define TEST_POLICY_ATTRIBUTES_NULL  0
> > +#define TEST_POLICY_MIN_SIZE_NULL    0
> > +#define TEST_POLICY_MAX_SIZE_NULL    MAX_UINT32
> > +
> > +#define TEST_POLICY_MIN_SIZE_10      10
> > +#define TEST_POLICY_MAX_SIZE_200     200
> > +
> > +///=== HELPER FUNCTIONS
> > ==============================================================
> > =============
> > +
> > +/**
> > +  Mocked version of GetVariable, for testing.
> > +
> > +  @param  VariableName
> > +  @param  VendorGuid
> > +  @param  Attributes
> > +  @param  DataSize
> > +  @param  Data
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +StubGetVariableNull (
> > +  IN     CHAR16    *VariableName,
> > +  IN     EFI_GUID  *VendorGuid,
> > +  OUT    UINT32    *Attributes,  OPTIONAL
> > +  IN OUT UINTN     *DataSize,
> > +  OUT    VOID      *Data         OPTIONAL
> > +  )
> > +{
> > +  UINT32      MockedAttr;
> > +  UINTN       MockedDataSize;
> > +  VOID        *MockedData;
> > +  EFI_STATUS  MockedReturn;
> > +
> > +  check_expected_ptr (VariableName);
> > +  check_expected_ptr (VendorGuid);
> > +  check_expected_ptr (DataSize);
> > +
> > +  MockedAttr     = (UINT32)mock();
> > +  MockedDataSize = (UINTN)mock();
> > +  MockedData     = (VOID*)(UINTN)mock();
> > +  MockedReturn   = (EFI_STATUS)mock();
> > +
> > +  if (Attributes != NULL) {
> > +    *Attributes = MockedAttr;
> > +  }
> > +  if (Data != NULL && !EFI_ERROR (MockedReturn)) {
> > +    CopyMem (Data, MockedData, MockedDataSize);
> > +  }
> > +
> > +  *DataSize = MockedDataSize;
> > +
> > +  return MockedReturn;
> > +}
> > +
> > +//
> > +// Anything you think might be helpful that isn't a test itself.
> > +//
> > +
> > +/**
> > +  This is a common setup function that will ensure the library is always
> > +  initialized with the stubbed GetVariable.
> > +
> > +  Not used by all test cases, but by most.
> > +
> > +  @param[in]  Context  Unit test case context
> > +**/
> > +STATIC
> > +UNIT_TEST_STATUS
> > +EFIAPI
> > +LibInitMocked (
> > +  IN UNIT_TEST_CONTEXT  Context
> > +  )
> > +{
> > +  return EFI_ERROR (InitVariablePolicyLib (StubGetVariableNull)) ?
> > UNIT_TEST_ERROR_PREREQUISITE_NOT_MET : UNIT_TEST_PASSED;
> > +}
> > +
> > +/**
> > +  Common cleanup function to make sure that the library is always
> > de-initialized
> > +  prior to the next test case.
> > +
> > +  @param[in]  Context  Unit test case context
> > +**/
> > +STATIC
> > +VOID
> > +EFIAPI
> > +LibCleanup (
> > +  IN UNIT_TEST_CONTEXT  Context
> > +  )
> > +{
> > +  DeinitVariablePolicyLib();
> > +}
> > +
> > +///=== TEST CASES
> > ==============================================================
> > ===================
> > +
> > +///===== SHIM SUITE
> > ===========================================================
> > +
> > +/**
> > +  Test Case that locks a single variable using the Variable Lock
> Protocol.
> > +  The call is expected to succeed.
> > +
> > +  @param[in]  Context  Unit test case context
> > +**/
> > +UNIT_TEST_STATUS
> > +EFIAPI
> > +LockingWithoutAnyPoliciesShouldSucceed (
> > +  IN UNIT_TEST_CONTEXT  Context
> > +  )
> > +{
> > +  EFI_STATUS  Status;
> > +
> > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > &mTestGuid1);
> > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > +
> > +  return UNIT_TEST_PASSED;
> > +}
> > +
> > +/**
> > +  Test Case that locks the same variable twice using the Variable Lock
> > Protocol.
> > +  Both calls are expected to succeed.
> > +
> > +  @param[in]  Context  Unit test case context
> > +  **/
> > +UNIT_TEST_STATUS
> > +EFIAPI
> > +LockingTwiceShouldSucceed (
> > +  IN UNIT_TEST_CONTEXT  Context
> > +  )
> > +{
> > +  EFI_STATUS  Status;
> > +
> > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > &mTestGuid1);
> > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > +
> > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > &mTestGuid1);
> > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > +
> > +  return UNIT_TEST_PASSED;
> > +}
> > +
> > +/**
> > +  Test Case that locks a variable using the Variable Policy Protocol then
> locks
> > +  the same variable using the Variable Lock Protocol.
> > +  Both calls are expected to succeed.
> > +
> > +  @param[in]  Context  Unit test case context
> > +  **/
> > +UNIT_TEST_STATUS
> > +EFIAPI
> > +LockingALockedVariableShouldSucceed (
> > +  IN UNIT_TEST_CONTEXT  Context
> > +  )
> > +{
> > +  EFI_STATUS             Status;
> > +  VARIABLE_POLICY_ENTRY  *NewEntry;
> > +
> > +  //
> > +  // Create a variable policy that locks the variable.
> > +  //
> > +  Status = CreateBasicVariablePolicy (
> > +             &mTestGuid1,
> > +             TEST_VAR_1_NAME,
> > +             TEST_POLICY_MIN_SIZE_NULL,
> > +             TEST_POLICY_MAX_SIZE_200,
> > +             TEST_POLICY_ATTRIBUTES_NULL,
> > +             TEST_POLICY_ATTRIBUTES_NULL,
> > +             VARIABLE_POLICY_TYPE_LOCK_NOW,
> > +             &NewEntry
> > +             );
> > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > +
> > +  //
> > +  // Register the new policy.
> > +  //
> > +  Status = RegisterVariablePolicy (NewEntry);
> > +
> > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > &mTestGuid1);
> > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > +
> > +  FreePool (NewEntry);
> > +
> > +  return UNIT_TEST_PASSED;
> > +}
> > +
> > +/**
> > +  Test Case that locks a variable using the Variable Policy Protocol with
> a
> > +  policy other than LOCK_NOW then attempts to lock the same variable
> > using the
> > +  Variable Lock Protocol.  The call to Variable Policy is expected to
> succeed
> > +  and the call to Variable Lock is expected to fail.
> > +
> > +  @param[in]  Context  Unit test case context
> > +  **/
> > +UNIT_TEST_STATUS
> > +EFIAPI
> > +LockingAnUnlockedVariableShouldFail (
> > +  IN UNIT_TEST_CONTEXT      Context
> > +  )
> > +{
> > +  EFI_STATUS                Status;
> > +  VARIABLE_POLICY_ENTRY     *NewEntry;
> > +
> > +  // Create a variable policy that locks the variable.
> > +  Status = CreateVarStateVariablePolicy (&mTestGuid1,
> > +                                         TEST_VAR_1_NAME,
> > +
> > TEST_POLICY_MIN_SIZE_NULL,
> > +
> > TEST_POLICY_MAX_SIZE_200,
> > +
> > TEST_POLICY_ATTRIBUTES_NULL,
> > +
> > TEST_POLICY_ATTRIBUTES_NULL,
> > +                                         &mTestGuid2,
> > +                                         1,
> > +                                         TEST_VAR_2_NAME,
> > +                                         &NewEntry);
> > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > +
> > +  // Register the new policy.
> > +  Status = RegisterVariablePolicy (NewEntry);
> > +
> > + // Configure the stub to not care about parameters. We're testing
> errors.
> > +  expect_any_always( StubGetVariableNull, VariableName );
> > +  expect_any_always( StubGetVariableNull, VendorGuid );
> > +  expect_any_always( StubGetVariableNull, DataSize );
> > +
> > +  // With a policy, make sure that writes still work, since the variable
> > doesn't exist.
> > +  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );
> > // Attributes
> > +  will_return( StubGetVariableNull, 0 );
> > // Size
> > +  will_return( StubGetVariableNull, NULL );
> > // DataPtr
> > +  will_return( StubGetVariableNull, EFI_NOT_FOUND);
> > // Status
> > +
> > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > &mTestGuid1);
> > +  UT_ASSERT_TRUE (EFI_ERROR (Status));
> > +
> > +  FreePool (NewEntry);
> > +
> > +  return UNIT_TEST_PASSED;
> > +}
> > +
> > +/**
> > +  Test Case that locks a variable using the Variable Policy Protocol with
> a
> > +  policy other than LOCK_NOW, but is currently locked.  Then attempts to
> > lock
> > +  the same variable using the Variable Lock Protocol.  The call to
> Variable
> > +  Policy is expected to succeed and the call to Variable Lock also
> expected
> > to
> > +  succeed.
> > +
> > +  @param[in]  Context  Unit test case context
> > +  **/
> > +UNIT_TEST_STATUS
> > +EFIAPI
> > +LockingALockedVariableWithMatchingDataShouldSucceed (
> > +  IN UNIT_TEST_CONTEXT      Context
> > +  )
> > +{
> > +  EFI_STATUS                Status;
> > +  VARIABLE_POLICY_ENTRY     *NewEntry;
> > +  UINT8                     Data;
> > +
> > +  // Create a variable policy that locks the variable.
> > +  Status = CreateVarStateVariablePolicy (&mTestGuid1,
> > +                                         TEST_VAR_1_NAME,
> > +
> > TEST_POLICY_MIN_SIZE_NULL,
> > +
> > TEST_POLICY_MAX_SIZE_200,
> > +
> > TEST_POLICY_ATTRIBUTES_NULL,
> > +
> > TEST_POLICY_ATTRIBUTES_NULL,
> > +                                         &mTestGuid2,
> > +                                         1,
> > +                                         TEST_VAR_2_NAME,
> > +                                         &NewEntry);
> > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > +
> > +  // Register the new policy.
> > +  Status = RegisterVariablePolicy (NewEntry);
> > +
> > + // Configure the stub to not care about parameters. We're testing
> errors.
> > +  expect_any_always( StubGetVariableNull, VariableName );
> > +  expect_any_always( StubGetVariableNull, VendorGuid );
> > +  expect_any_always( StubGetVariableNull, DataSize );
> > +
> > +  // With a policy, make sure that writes still work, since the variable
> > doesn't exist.
> > +  Data = 1;
> > +  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );
> > // Attributes
> > +  will_return( StubGetVariableNull, sizeof (Data) );                  //
> > Size
> > +  will_return( StubGetVariableNull, &Data );
> > // DataPtr
> > +  will_return( StubGetVariableNull, EFI_SUCCESS);
> > // Status
> > +
> > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > &mTestGuid1);
> > +  UT_ASSERT_TRUE (!EFI_ERROR (Status));
> > +
> > +  FreePool (NewEntry);
> > +
> > +  return UNIT_TEST_PASSED;
> > +}
> > +
> > +/**
> > +  Test Case that locks a variable using the Variable Policy Protocol with
> a
> > +  policy other than LOCK_NOW, but variable data does not match.  Then
> > attempts
> > +  to lock the same variable using the Variable Lock Protocol.  The call
> to
> > +  Variable Policy is expected to succeed and the call to Variable Lock is
> > +  expected to fail.
> > +
> > +  @param[in]  Context  Unit test case context
> > +  **/
> > +UNIT_TEST_STATUS
> > +EFIAPI
> > +LockingALockedVariableWithNonMatchingDataShouldFail (
> > +  IN UNIT_TEST_CONTEXT      Context
> > +  )
> > +{
> > +  EFI_STATUS                Status;
> > +  VARIABLE_POLICY_ENTRY     *NewEntry;
> > +  UINT8                     Data;
> > +
> > +  // Create a variable policy that locks the variable.
> > +  Status = CreateVarStateVariablePolicy (&mTestGuid1,
> > +                                         TEST_VAR_1_NAME,
> > +
> > TEST_POLICY_MIN_SIZE_NULL,
> > +
> > TEST_POLICY_MAX_SIZE_200,
> > +
> > TEST_POLICY_ATTRIBUTES_NULL,
> > +
> > TEST_POLICY_ATTRIBUTES_NULL,
> > +                                         &mTestGuid2,
> > +                                         1,
> > +                                         TEST_VAR_2_NAME,
> > +                                         &NewEntry);
> > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > +
> > +  // Register the new policy.
> > +  Status = RegisterVariablePolicy (NewEntry);
> > +
> > + // Configure the stub to not care about parameters. We're testing
> errors.
> > +  expect_any_always( StubGetVariableNull, VariableName );
> > +  expect_any_always( StubGetVariableNull, VendorGuid );
> > +  expect_any_always( StubGetVariableNull, DataSize );
> > +
> > +  // With a policy, make sure that writes still work, since the variable
> > doesn't exist.
> > +  Data = 2;
> > +  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );
> > // Attributes
> > +  will_return( StubGetVariableNull, sizeof (Data) );                  //
> > Size
> > +  will_return( StubGetVariableNull, &Data );
> > // DataPtr
> > +  will_return( StubGetVariableNull, EFI_SUCCESS);
> > // Status
> > +
> > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > &mTestGuid1);
> > +  UT_ASSERT_TRUE (EFI_ERROR (Status));
> > +
> > +  FreePool (NewEntry);
> > +
> > +  return UNIT_TEST_PASSED;
> > +}
> > +
> > +/**
> > +  Test Case that locks a variable using Variable Lock Protocol Policy
> Protocol
> > +  then and then attempts to lock the same variable using the Variable
> Policy
> > +  Protocol.  The call to Variable Lock is expected to succeed and the
> call to
> > +  Variable Policy is expected to fail.
> > +
> > +  @param[in]  Context  Unit test case context
> > +  **/
> > +UNIT_TEST_STATUS
> > +EFIAPI
> > +SettingPolicyForALockedVariableShouldFail (
> > +  IN UNIT_TEST_CONTEXT      Context
> > +  )
> > +{
> > +  EFI_STATUS                Status;
> > +  VARIABLE_POLICY_ENTRY     *NewEntry;
> > +
> > +  // Lock the variable.
> > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > &mTestGuid1);
> > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > +
> > +  // Create a variable policy that locks the variable.
> > +  Status = CreateVarStateVariablePolicy (&mTestGuid1,
> > +                                         TEST_VAR_1_NAME,
> > +
> > TEST_POLICY_MIN_SIZE_NULL,
> > +
> > TEST_POLICY_MAX_SIZE_200,
> > +
> > TEST_POLICY_ATTRIBUTES_NULL,
> > +
> > TEST_POLICY_ATTRIBUTES_NULL,
> > +                                         &mTestGuid2,
> > +                                         1,
> > +                                         TEST_VAR_2_NAME,
> > +                                         &NewEntry);
> > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > +
> > +  // Register the new policy.
> > +  Status = RegisterVariablePolicy (NewEntry);
> > +  UT_ASSERT_TRUE (EFI_ERROR (Status));
> > +
> > +  FreePool (NewEntry);
> > +
> > +  return UNIT_TEST_PASSED;
> > +}
> > +
> > +/**
> > +  Main entry point to this unit test application.
> > +
> > +  Sets up and runs the test suites.
> > +**/
> > +VOID
> > +EFIAPI
> > +UnitTestMain (
> > +  VOID
> > +  )
> > +{
> > +  EFI_STATUS                  Status;
> > +  UNIT_TEST_FRAMEWORK_HANDLE  Framework;
> > +  UNIT_TEST_SUITE_HANDLE      ShimTests;
> > +
> > +  Framework = NULL;
> > +
> > +  DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_NAME,
> > UNIT_TEST_VERSION));
> > +
> > +  //
> > +  // Start setting up the test framework for running the tests.
> > +  //
> > +  Status = InitUnitTestFramework (&Framework, UNIT_TEST_NAME,
> > gEfiCallerBaseName, UNIT_TEST_VERSION);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "Failed in InitUnitTestFramework. Status
> > = %r\n", Status));
> > +    goto EXIT;
> > +  }
> > +
> > +  //
> > +  // Add all test suites and tests.
> > +  //
> > +  Status = CreateUnitTestSuite (
> > +             &ShimTests, Framework,
> > +             "Variable Lock Shim Tests", "VarPolicy.VarLockShim", NULL,
> > NULL
> > +             );
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for
> > ShimTests\n"));
> > +    Status = EFI_OUT_OF_RESOURCES;
> > +    goto EXIT;
> > +  }
> > +  AddTestCase (
> > +    ShimTests,
> > +    "Locking a variable with no matching policies should always work",
> > "EmptyPolicies",
> > +    LockingWithoutAnyPoliciesShouldSucceed, LibInitMocked, LibCleanup,
> > NULL
> > +    );
> > +  AddTestCase (
> > +    ShimTests,
> > +    "Locking a variable twice should always work", "DoubleLock",
> > +    LockingTwiceShouldSucceed, LibInitMocked, LibCleanup, NULL
> > +    );
> > +  AddTestCase (
> > +    ShimTests,
> > +    "Locking a variable that's already locked by another policy should
> work",
> > "LockAfterPolicy",
> > +    LockingALockedVariableShouldSucceed, LibInitMocked, LibCleanup,
> > NULL
> > +    );
> > +  AddTestCase (
> > +    ShimTests,
> > +    "Locking a variable that already has an unlocked policy should fail",
> > "LockAfterUnlockedPolicy",
> > +    LockingAnUnlockedVariableShouldFail, LibInitMocked, LibCleanup,
> > NULL
> > +    );
> > +  AddTestCase (
> > +    ShimTests,
> > +    "Locking a variable that already has an locked policy should
> succeed",
> > "LockAfterLockedPolicyMatchingData",
> > +    LockingALockedVariableWithMatchingDataShouldSucceed,
> > LibInitMocked, LibCleanup, NULL
> > +    );
> > +  AddTestCase (
> > +    ShimTests,
> > +    "Locking a variable that already has an locked policy with matching
> > data should succeed", "LockAfterLockedPolicyNonMatchingData",
> > +    LockingALockedVariableWithNonMatchingDataShouldFail,
> > LibInitMocked, LibCleanup, NULL
> > +    );
> > +  AddTestCase (
> > +    ShimTests,
> > +    "Adding a policy for a variable that has previously been locked
> should
> > always fail", "SetPolicyAfterLock",
> > +    SettingPolicyForALockedVariableShouldFail, LibInitMocked, LibCleanup,
> > NULL
> > +    );
> > +
> > +  //
> > +  // Execute the tests.
> > +  //
> > +  Status = RunAllTestSuites (Framework);
> > +
> > +EXIT:
> > +  if (Framework != NULL) {
> > +    FreeUnitTestFramework (Framework);
> > +  }
> > +
> > +  return;
> > +}
> > +
> > +///
> > +/// Avoid ECC error for function name that starts with lower case letter
> > +///
> > +#define Main main
> > +
> > +/**
> > +  Standard POSIX C entry point for host based unit test execution.
> > +
> > +  @param[in] Argc  Number of arguments
> > +  @param[in] Argv  Array of pointers to arguments
> > +
> > +  @retval 0      Success
> > +  @retval other  Error
> > +**/
> > +INT32
> > +Main (
> > +  IN INT32  Argc,
> > +  IN CHAR8  *Argv[]
> > +  )
> > +{
> > +  UnitTestMain ();
> > +  return 0;
> > +}
> > diff --git
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> > ableLockRequestToLockUnitTest.inf
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> > ableLockRequestToLockUnitTest.inf
> > new file mode 100644
> > index 000000000000..2a659d7e1370
> > --- /dev/null
> > +++
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> > ableLockRequestToLockUnitTest.inf
> > @@ -0,0 +1,36 @@
> > +## @file
> > +# This is a host-based unit test for the VariableLockRequestToLock shim.
> > +#
> > +# Copyright (c) Microsoft Corporation.
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent
> > +##
> > +
> > +[Defines]
> > +  INF_VERSION         = 0x00010017
> > +  BASE_NAME           = VariableLockRequestToLockUnitTest
> > +  FILE_GUID           = A7388B6C-7274-4717-9649-BDC5DFD1FCBE
> > +  VERSION_STRING      = 1.0
> > +  MODULE_TYPE         = HOST_APPLICATION
> > +
> > +#
> > +# The following information is for reference only and not required by the
> > build tools.
> > +#
> > +#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
> > +#
> > +
> > +[Sources]
> > +  VariableLockRequestToLockUnitTest.c
> > +  ../VariableLockRequestToLock.c
> > +
> > +[Packages]
> > +  MdePkg/MdePkg.dec
> > +  MdeModulePkg/MdeModulePkg.dec
> > +  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
> > +
> > +[LibraryClasses]
> > +  UnitTestLib
> > +  DebugLib
> > +  VariablePolicyLib
> > +  VariablePolicyHelperLib
> > +  BaseMemoryLib
> > +  MemoryAllocationLib
> > --
> > 2.29.2.windows.2
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 
> 
> 


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

* 回复: [edk2-devel] [Patch v4 2/2] MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit Tests
  2020-12-14 17:52     ` Michael D Kinney
@ 2020-12-15  0:56       ` gaoliming
  0 siblings, 0 replies; 8+ messages in thread
From: gaoliming @ 2020-12-15  0:56 UTC (permalink / raw)
  To: devel, michael.d.kinney; +Cc: 'Wu, Hao A', 'Bret Barkelew'

Mike:
  I understand unit test purpose. I am OK to record this issue first. I have no other comments. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+68804+4905953+8761045@groups.io
> <bounce+27952+68804+4905953+8761045@groups.io> 代表 Michael D
> Kinney
> 发送时间: 2020年12月15日 1:53
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Kinney, Michael
> D <michael.d.kinney@intel.com>
> 抄送: Wu, Hao A <hao.a.wu@intel.com>; 'Bret Barkelew'
> <Bret.Barkelew@microsoft.com>
> 主题: Re: [edk2-devel] [Patch v4 2/2] MdeModulePkg/Variable/RuntimeDxe:
> Add Variable Lock Protocol Unit Tests
> 
> Hi Liming,
> 
> I agree it breaks the rules to use .. in the [Sources] section of an INF file.
> 
> However, I do think it is important to isolate all unit test related code
> (including INFs)
> into their own Test directories so developers can easily distinguish source
> code and INFs
> associated with FW implementations from unit tests and potentially set up
> sparse check out
> rules or filters so code searches can be configured to only search FW code
> artifacts.
> 
> There is a host based unit test feature gap that needs to be addresses for
> module scoped
> testing vs library scoped testing.  Library scoped testing can use lib class/lib
> instance
> mapping in host based unit test DSC file to pull lib interfaces into a host based
> unit test.
> For module scoped testing today, we do not have access to the module code
> as a library, so
> you see the example here of referencing a module source file from a unit test.
> 
> I would prefer we allow this code remain "as is" today and enter 2 new BZs:
> 
> * Add better support for module scoped testing to UnitTestFrameworkPkg that
> avoids the
>   need to use .. in [Sources] sections of host based unit test INF files.
> 
> * Remove use of .. from INF in host based unit test of Variable Lock Protocol
> and
>   Variable Policy Protocol in
> MdeModulePkg/Universal/Variable/RuntimeDxe/Test.
> 
> Best regards,
> 
> Mike
> 
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> gaoliming
> > Sent: Sunday, December 13, 2020 5:56 PM
> > To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; 'Bret Barkelew'
> <Bret.Barkelew@microsoft.com>
> > Subject: 回复: [edk2-devel] [Patch v4 2/2]
> MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit Tests
> >
> > Mike and Bret:
> >   Seemly, this is the first unit test case for the logic in the driver. This
> > unit test module needs to include the driver source file. Then, its INF file
> > has to use the below style to include the source file. But, edk2 module INF
> > doesn't recommend to specify the source file with the relative upper
> > directory (..). How about add VariableLockRequestToLockUnitTest.inf into
> > MdeModulePkg\Universal\Variable\RuntimeDxe\ directory? If so, this INF
> can
> > directly specify the source file in [Sources] section.
> >
> > > +[Sources]
> > > +  VariableLockRequestToLockUnitTest.c
> > > +  ../VariableLockRequestToLock.c
> > > +
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: bounce+27952+68703+4905953+8761045@groups.io
> > > <bounce+27952+68703+4905953+8761045@groups.io> 代表 Michael D
> > > Kinney
> > > 发送时间: 2020年12月11日 16:01
> > > 收件人: devel@edk2.groups.io
> > > 抄送: Hao A Wu <hao.a.wu@intel.com>; Liming Gao
> > > <gaoliming@byosoft.com.cn>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> > > 主题: [edk2-devel] [Patch v4 2/2] MdeModulePkg/Variable/RuntimeDxe:
> Add
> > > Variable Lock Protocol Unit Tests
> > >
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=3111
> > >
> > > Add host based unit tests for the multiple lock case using Variable Lock
> > > Protocol, Variable Policy Protocol, and mixes of Variable Lock Protocol
> > > and Variable Policy Protocol.
> > >
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > Signed-off-by: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > > ---
> > >  MdeModulePkg/Test/MdeModulePkgHostTest.dsc    |  11 +
> > >  .../VariableLockRequestToLockUnitTest.c       | 565
> > > ++++++++++++++++++
> > >  .../VariableLockRequestToLockUnitTest.inf     |  36 ++
> > >  3 files changed, 612 insertions(+)
> > >  create mode 100644
> > >
> MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Variab
> > > leLockRequestToLockUnitTest.c
> > >  create mode 100644
> > >
> MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Variab
> > > leLockRequestToLockUnitTest.inf
> > >
> > > diff --git a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> > > b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> > > index 72a119db4568..4da4692c8451 100644
> > > --- a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> > > +++ b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
> > > @@ -19,6 +19,9 @@ [Defines]
> > >
> > >  !include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc
> > >
> > > +[LibraryClasses]
> > > +  SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> > > +
> > >  [Components]
> > >
> > >
> MdeModulePkg/Library/DxeResetSystemLib/UnitTest/MockUefiRuntimeServi
> > > cesTableLib.inf
> > >
> > > @@ -30,3 +33,11 @@ [Components]
> > >
> > >
> ResetSystemLib|MdeModulePkg/Library/DxeResetSystemLib/DxeResetSyste
> > > mLib.inf
> > >
> > >
> UefiRuntimeServicesTableLib|MdeModulePkg/Library/DxeResetSystemLib/U
> > > nitTest/MockUefiRuntimeServicesTableLib.inf
> > >    }
> > > +
> > > +
> > >
> MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Variab
> > > leLockRequestToLockUnitTest.inf {
> > > +    <LibraryClasses>
> > > +
> > >
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib
> > > .inf
> > > +
> > >
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Vari
> > > ablePolicyHelperLib.inf
> > > +    <PcdsFixedAtBuild>
> > > +
> > >
> gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisab
> > > le|TRUE
> > > +  }
> > > diff --git
> > >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> > > ableLockRequestToLockUnitTest.c
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> > > ableLockRequestToLockUnitTest.c
> > > new file mode 100644
> > > index 000000000000..44d70e639d77
> > > --- /dev/null
> > > +++
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> > > ableLockRequestToLockUnitTest.c
> > > @@ -0,0 +1,565 @@
> > > +/** @file
> > > +  This is a host-based unit test for the VariableLockRequestToLock shim.
> > > +
> > > +  Copyright (c) Microsoft Corporation.
> > > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +
> > > +#include <stdio.h>
> > > +#include <string.h>
> > > +#include <stdarg.h>
> > > +#include <stddef.h>
> > > +#include <setjmp.h>
> > > +#include <cmocka.h>
> > > +
> > > +#include <Uefi.h>
> > > +#include <Library/DebugLib.h>
> > > +#include <Library/BaseMemoryLib.h>
> > > +#include <Library/MemoryAllocationLib.h>
> > > +#include <Library/UnitTestLib.h>
> > > +#include <Library/VariablePolicyLib.h>
> > > +#include <Library/VariablePolicyHelperLib.h>
> > > +
> > > +#include <Protocol/VariableLock.h>
> > > +
> > > +#define UNIT_TEST_NAME        "VarPol/VarLock Shim Unit Test"
> > > +#define UNIT_TEST_VERSION     "1.0"
> > > +
> > > +///=== CODE UNDER TEST
> > >
> ==============================================================
> > > =============
> > > +
> > > +EFI_STATUS
> > > +EFIAPI
> > > +VariableLockRequestToLock (
> > > +  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL  *This,
> > > +  IN       CHAR16                        *VariableName,
> > > +  IN       EFI_GUID                      *VendorGuid
> > > +  );
> > > +
> > > +///=== TEST DATA
> > >
> ==============================================================
> > > ====================
> > > +
> > > +//
> > > +// Test GUID 1 {F955BA2D-4A2C-480C-BFD1-3CC522610592}
> > > +//
> > > +EFI_GUID  mTestGuid1 = {
> > > +  0xf955ba2d, 0x4a2c, 0x480c, {0xbf, 0xd1, 0x3c, 0xc5, 0x22, 0x61, 0x5,
> > > 0x92}
> > > +};
> > > +
> > > +//
> > > +// Test GUID 2 {2DEA799E-5E73-43B9-870E-C945CE82AF3A}
> > > +//
> > > +EFI_GUID  mTestGuid2 = {
> > > +  0x2dea799e, 0x5e73, 0x43b9, {0x87, 0xe, 0xc9, 0x45, 0xce, 0x82, 0xaf,
> > > 0x3a}
> > > +};
> > > +
> > > +//
> > > +// Test GUID 3 {698A2BFD-A616-482D-B88C-7100BD6682A9}
> > > +//
> > > +EFI_GUID  mTestGuid3 = {
> > > +  0x698a2bfd, 0xa616, 0x482d, {0xb8, 0x8c, 0x71, 0x0, 0xbd, 0x66, 0x82,
> > > 0xa9}
> > > +};
> > > +
> > > +#define TEST_VAR_1_NAME              L"TestVar1"
> > > +#define TEST_VAR_2_NAME              L"TestVar2"
> > > +#define TEST_VAR_3_NAME              L"TestVar3"
> > > +
> > > +#define TEST_POLICY_ATTRIBUTES_NULL  0
> > > +#define TEST_POLICY_MIN_SIZE_NULL    0
> > > +#define TEST_POLICY_MAX_SIZE_NULL    MAX_UINT32
> > > +
> > > +#define TEST_POLICY_MIN_SIZE_10      10
> > > +#define TEST_POLICY_MAX_SIZE_200     200
> > > +
> > > +///=== HELPER FUNCTIONS
> > >
> ==============================================================
> > > =============
> > > +
> > > +/**
> > > +  Mocked version of GetVariable, for testing.
> > > +
> > > +  @param  VariableName
> > > +  @param  VendorGuid
> > > +  @param  Attributes
> > > +  @param  DataSize
> > > +  @param  Data
> > > +**/
> > > +EFI_STATUS
> > > +EFIAPI
> > > +StubGetVariableNull (
> > > +  IN     CHAR16    *VariableName,
> > > +  IN     EFI_GUID  *VendorGuid,
> > > +  OUT    UINT32    *Attributes,  OPTIONAL
> > > +  IN OUT UINTN     *DataSize,
> > > +  OUT    VOID      *Data         OPTIONAL
> > > +  )
> > > +{
> > > +  UINT32      MockedAttr;
> > > +  UINTN       MockedDataSize;
> > > +  VOID        *MockedData;
> > > +  EFI_STATUS  MockedReturn;
> > > +
> > > +  check_expected_ptr (VariableName);
> > > +  check_expected_ptr (VendorGuid);
> > > +  check_expected_ptr (DataSize);
> > > +
> > > +  MockedAttr     = (UINT32)mock();
> > > +  MockedDataSize = (UINTN)mock();
> > > +  MockedData     = (VOID*)(UINTN)mock();
> > > +  MockedReturn   = (EFI_STATUS)mock();
> > > +
> > > +  if (Attributes != NULL) {
> > > +    *Attributes = MockedAttr;
> > > +  }
> > > +  if (Data != NULL && !EFI_ERROR (MockedReturn)) {
> > > +    CopyMem (Data, MockedData, MockedDataSize);
> > > +  }
> > > +
> > > +  *DataSize = MockedDataSize;
> > > +
> > > +  return MockedReturn;
> > > +}
> > > +
> > > +//
> > > +// Anything you think might be helpful that isn't a test itself.
> > > +//
> > > +
> > > +/**
> > > +  This is a common setup function that will ensure the library is always
> > > +  initialized with the stubbed GetVariable.
> > > +
> > > +  Not used by all test cases, but by most.
> > > +
> > > +  @param[in]  Context  Unit test case context
> > > +**/
> > > +STATIC
> > > +UNIT_TEST_STATUS
> > > +EFIAPI
> > > +LibInitMocked (
> > > +  IN UNIT_TEST_CONTEXT  Context
> > > +  )
> > > +{
> > > +  return EFI_ERROR (InitVariablePolicyLib (StubGetVariableNull)) ?
> > > UNIT_TEST_ERROR_PREREQUISITE_NOT_MET : UNIT_TEST_PASSED;
> > > +}
> > > +
> > > +/**
> > > +  Common cleanup function to make sure that the library is always
> > > de-initialized
> > > +  prior to the next test case.
> > > +
> > > +  @param[in]  Context  Unit test case context
> > > +**/
> > > +STATIC
> > > +VOID
> > > +EFIAPI
> > > +LibCleanup (
> > > +  IN UNIT_TEST_CONTEXT  Context
> > > +  )
> > > +{
> > > +  DeinitVariablePolicyLib();
> > > +}
> > > +
> > > +///=== TEST CASES
> > >
> ==============================================================
> > > ===================
> > > +
> > > +///===== SHIM SUITE
> > > ===========================================================
> > > +
> > > +/**
> > > +  Test Case that locks a single variable using the Variable Lock
> > Protocol.
> > > +  The call is expected to succeed.
> > > +
> > > +  @param[in]  Context  Unit test case context
> > > +**/
> > > +UNIT_TEST_STATUS
> > > +EFIAPI
> > > +LockingWithoutAnyPoliciesShouldSucceed (
> > > +  IN UNIT_TEST_CONTEXT  Context
> > > +  )
> > > +{
> > > +  EFI_STATUS  Status;
> > > +
> > > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > > &mTestGuid1);
> > > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > > +
> > > +  return UNIT_TEST_PASSED;
> > > +}
> > > +
> > > +/**
> > > +  Test Case that locks the same variable twice using the Variable Lock
> > > Protocol.
> > > +  Both calls are expected to succeed.
> > > +
> > > +  @param[in]  Context  Unit test case context
> > > +  **/
> > > +UNIT_TEST_STATUS
> > > +EFIAPI
> > > +LockingTwiceShouldSucceed (
> > > +  IN UNIT_TEST_CONTEXT  Context
> > > +  )
> > > +{
> > > +  EFI_STATUS  Status;
> > > +
> > > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > > &mTestGuid1);
> > > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > > +
> > > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > > &mTestGuid1);
> > > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > > +
> > > +  return UNIT_TEST_PASSED;
> > > +}
> > > +
> > > +/**
> > > +  Test Case that locks a variable using the Variable Policy Protocol then
> > locks
> > > +  the same variable using the Variable Lock Protocol.
> > > +  Both calls are expected to succeed.
> > > +
> > > +  @param[in]  Context  Unit test case context
> > > +  **/
> > > +UNIT_TEST_STATUS
> > > +EFIAPI
> > > +LockingALockedVariableShouldSucceed (
> > > +  IN UNIT_TEST_CONTEXT  Context
> > > +  )
> > > +{
> > > +  EFI_STATUS             Status;
> > > +  VARIABLE_POLICY_ENTRY  *NewEntry;
> > > +
> > > +  //
> > > +  // Create a variable policy that locks the variable.
> > > +  //
> > > +  Status = CreateBasicVariablePolicy (
> > > +             &mTestGuid1,
> > > +             TEST_VAR_1_NAME,
> > > +             TEST_POLICY_MIN_SIZE_NULL,
> > > +             TEST_POLICY_MAX_SIZE_200,
> > > +             TEST_POLICY_ATTRIBUTES_NULL,
> > > +             TEST_POLICY_ATTRIBUTES_NULL,
> > > +             VARIABLE_POLICY_TYPE_LOCK_NOW,
> > > +             &NewEntry
> > > +             );
> > > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > > +
> > > +  //
> > > +  // Register the new policy.
> > > +  //
> > > +  Status = RegisterVariablePolicy (NewEntry);
> > > +
> > > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > > &mTestGuid1);
> > > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > > +
> > > +  FreePool (NewEntry);
> > > +
> > > +  return UNIT_TEST_PASSED;
> > > +}
> > > +
> > > +/**
> > > +  Test Case that locks a variable using the Variable Policy Protocol with
> > a
> > > +  policy other than LOCK_NOW then attempts to lock the same variable
> > > using the
> > > +  Variable Lock Protocol.  The call to Variable Policy is expected to
> > succeed
> > > +  and the call to Variable Lock is expected to fail.
> > > +
> > > +  @param[in]  Context  Unit test case context
> > > +  **/
> > > +UNIT_TEST_STATUS
> > > +EFIAPI
> > > +LockingAnUnlockedVariableShouldFail (
> > > +  IN UNIT_TEST_CONTEXT      Context
> > > +  )
> > > +{
> > > +  EFI_STATUS                Status;
> > > +  VARIABLE_POLICY_ENTRY     *NewEntry;
> > > +
> > > +  // Create a variable policy that locks the variable.
> > > +  Status = CreateVarStateVariablePolicy (&mTestGuid1,
> > > +                                         TEST_VAR_1_NAME,
> > > +
> > > TEST_POLICY_MIN_SIZE_NULL,
> > > +
> > > TEST_POLICY_MAX_SIZE_200,
> > > +
> > > TEST_POLICY_ATTRIBUTES_NULL,
> > > +
> > > TEST_POLICY_ATTRIBUTES_NULL,
> > > +                                         &mTestGuid2,
> > > +                                         1,
> > > +                                         TEST_VAR_2_NAME,
> > > +                                         &NewEntry);
> > > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > > +
> > > +  // Register the new policy.
> > > +  Status = RegisterVariablePolicy (NewEntry);
> > > +
> > > + // Configure the stub to not care about parameters. We're testing
> > errors.
> > > +  expect_any_always( StubGetVariableNull, VariableName );
> > > +  expect_any_always( StubGetVariableNull, VendorGuid );
> > > +  expect_any_always( StubGetVariableNull, DataSize );
> > > +
> > > +  // With a policy, make sure that writes still work, since the variable
> > > doesn't exist.
> > > +  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );
> > > // Attributes
> > > +  will_return( StubGetVariableNull, 0 );
> > > // Size
> > > +  will_return( StubGetVariableNull, NULL );
> > > // DataPtr
> > > +  will_return( StubGetVariableNull, EFI_NOT_FOUND);
> > > // Status
> > > +
> > > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > > &mTestGuid1);
> > > +  UT_ASSERT_TRUE (EFI_ERROR (Status));
> > > +
> > > +  FreePool (NewEntry);
> > > +
> > > +  return UNIT_TEST_PASSED;
> > > +}
> > > +
> > > +/**
> > > +  Test Case that locks a variable using the Variable Policy Protocol with
> > a
> > > +  policy other than LOCK_NOW, but is currently locked.  Then attempts
> to
> > > lock
> > > +  the same variable using the Variable Lock Protocol.  The call to
> > Variable
> > > +  Policy is expected to succeed and the call to Variable Lock also
> > expected
> > > to
> > > +  succeed.
> > > +
> > > +  @param[in]  Context  Unit test case context
> > > +  **/
> > > +UNIT_TEST_STATUS
> > > +EFIAPI
> > > +LockingALockedVariableWithMatchingDataShouldSucceed (
> > > +  IN UNIT_TEST_CONTEXT      Context
> > > +  )
> > > +{
> > > +  EFI_STATUS                Status;
> > > +  VARIABLE_POLICY_ENTRY     *NewEntry;
> > > +  UINT8                     Data;
> > > +
> > > +  // Create a variable policy that locks the variable.
> > > +  Status = CreateVarStateVariablePolicy (&mTestGuid1,
> > > +                                         TEST_VAR_1_NAME,
> > > +
> > > TEST_POLICY_MIN_SIZE_NULL,
> > > +
> > > TEST_POLICY_MAX_SIZE_200,
> > > +
> > > TEST_POLICY_ATTRIBUTES_NULL,
> > > +
> > > TEST_POLICY_ATTRIBUTES_NULL,
> > > +                                         &mTestGuid2,
> > > +                                         1,
> > > +                                         TEST_VAR_2_NAME,
> > > +                                         &NewEntry);
> > > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > > +
> > > +  // Register the new policy.
> > > +  Status = RegisterVariablePolicy (NewEntry);
> > > +
> > > + // Configure the stub to not care about parameters. We're testing
> > errors.
> > > +  expect_any_always( StubGetVariableNull, VariableName );
> > > +  expect_any_always( StubGetVariableNull, VendorGuid );
> > > +  expect_any_always( StubGetVariableNull, DataSize );
> > > +
> > > +  // With a policy, make sure that writes still work, since the variable
> > > doesn't exist.
> > > +  Data = 1;
> > > +  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );
> > > // Attributes
> > > +  will_return( StubGetVariableNull, sizeof (Data) );
> //
> > > Size
> > > +  will_return( StubGetVariableNull, &Data );
> > > // DataPtr
> > > +  will_return( StubGetVariableNull, EFI_SUCCESS);
> > > // Status
> > > +
> > > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > > &mTestGuid1);
> > > +  UT_ASSERT_TRUE (!EFI_ERROR (Status));
> > > +
> > > +  FreePool (NewEntry);
> > > +
> > > +  return UNIT_TEST_PASSED;
> > > +}
> > > +
> > > +/**
> > > +  Test Case that locks a variable using the Variable Policy Protocol with
> > a
> > > +  policy other than LOCK_NOW, but variable data does not match.
> Then
> > > attempts
> > > +  to lock the same variable using the Variable Lock Protocol.  The call
> > to
> > > +  Variable Policy is expected to succeed and the call to Variable Lock is
> > > +  expected to fail.
> > > +
> > > +  @param[in]  Context  Unit test case context
> > > +  **/
> > > +UNIT_TEST_STATUS
> > > +EFIAPI
> > > +LockingALockedVariableWithNonMatchingDataShouldFail (
> > > +  IN UNIT_TEST_CONTEXT      Context
> > > +  )
> > > +{
> > > +  EFI_STATUS                Status;
> > > +  VARIABLE_POLICY_ENTRY     *NewEntry;
> > > +  UINT8                     Data;
> > > +
> > > +  // Create a variable policy that locks the variable.
> > > +  Status = CreateVarStateVariablePolicy (&mTestGuid1,
> > > +                                         TEST_VAR_1_NAME,
> > > +
> > > TEST_POLICY_MIN_SIZE_NULL,
> > > +
> > > TEST_POLICY_MAX_SIZE_200,
> > > +
> > > TEST_POLICY_ATTRIBUTES_NULL,
> > > +
> > > TEST_POLICY_ATTRIBUTES_NULL,
> > > +                                         &mTestGuid2,
> > > +                                         1,
> > > +                                         TEST_VAR_2_NAME,
> > > +                                         &NewEntry);
> > > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > > +
> > > +  // Register the new policy.
> > > +  Status = RegisterVariablePolicy (NewEntry);
> > > +
> > > + // Configure the stub to not care about parameters. We're testing
> > errors.
> > > +  expect_any_always( StubGetVariableNull, VariableName );
> > > +  expect_any_always( StubGetVariableNull, VendorGuid );
> > > +  expect_any_always( StubGetVariableNull, DataSize );
> > > +
> > > +  // With a policy, make sure that writes still work, since the variable
> > > doesn't exist.
> > > +  Data = 2;
> > > +  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );
> > > // Attributes
> > > +  will_return( StubGetVariableNull, sizeof (Data) );
> //
> > > Size
> > > +  will_return( StubGetVariableNull, &Data );
> > > // DataPtr
> > > +  will_return( StubGetVariableNull, EFI_SUCCESS);
> > > // Status
> > > +
> > > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > > &mTestGuid1);
> > > +  UT_ASSERT_TRUE (EFI_ERROR (Status));
> > > +
> > > +  FreePool (NewEntry);
> > > +
> > > +  return UNIT_TEST_PASSED;
> > > +}
> > > +
> > > +/**
> > > +  Test Case that locks a variable using Variable Lock Protocol Policy
> > Protocol
> > > +  then and then attempts to lock the same variable using the Variable
> > Policy
> > > +  Protocol.  The call to Variable Lock is expected to succeed and the
> > call to
> > > +  Variable Policy is expected to fail.
> > > +
> > > +  @param[in]  Context  Unit test case context
> > > +  **/
> > > +UNIT_TEST_STATUS
> > > +EFIAPI
> > > +SettingPolicyForALockedVariableShouldFail (
> > > +  IN UNIT_TEST_CONTEXT      Context
> > > +  )
> > > +{
> > > +  EFI_STATUS                Status;
> > > +  VARIABLE_POLICY_ENTRY     *NewEntry;
> > > +
> > > +  // Lock the variable.
> > > +  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME,
> > > &mTestGuid1);
> > > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > > +
> > > +  // Create a variable policy that locks the variable.
> > > +  Status = CreateVarStateVariablePolicy (&mTestGuid1,
> > > +                                         TEST_VAR_1_NAME,
> > > +
> > > TEST_POLICY_MIN_SIZE_NULL,
> > > +
> > > TEST_POLICY_MAX_SIZE_200,
> > > +
> > > TEST_POLICY_ATTRIBUTES_NULL,
> > > +
> > > TEST_POLICY_ATTRIBUTES_NULL,
> > > +                                         &mTestGuid2,
> > > +                                         1,
> > > +                                         TEST_VAR_2_NAME,
> > > +                                         &NewEntry);
> > > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > > +
> > > +  // Register the new policy.
> > > +  Status = RegisterVariablePolicy (NewEntry);
> > > +  UT_ASSERT_TRUE (EFI_ERROR (Status));
> > > +
> > > +  FreePool (NewEntry);
> > > +
> > > +  return UNIT_TEST_PASSED;
> > > +}
> > > +
> > > +/**
> > > +  Main entry point to this unit test application.
> > > +
> > > +  Sets up and runs the test suites.
> > > +**/
> > > +VOID
> > > +EFIAPI
> > > +UnitTestMain (
> > > +  VOID
> > > +  )
> > > +{
> > > +  EFI_STATUS                  Status;
> > > +  UNIT_TEST_FRAMEWORK_HANDLE  Framework;
> > > +  UNIT_TEST_SUITE_HANDLE      ShimTests;
> > > +
> > > +  Framework = NULL;
> > > +
> > > +  DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_NAME,
> > > UNIT_TEST_VERSION));
> > > +
> > > +  //
> > > +  // Start setting up the test framework for running the tests.
> > > +  //
> > > +  Status = InitUnitTestFramework (&Framework, UNIT_TEST_NAME,
> > > gEfiCallerBaseName, UNIT_TEST_VERSION);
> > > +  if (EFI_ERROR (Status)) {
> > > +    DEBUG ((DEBUG_ERROR, "Failed in InitUnitTestFramework. Status
> > > = %r\n", Status));
> > > +    goto EXIT;
> > > +  }
> > > +
> > > +  //
> > > +  // Add all test suites and tests.
> > > +  //
> > > +  Status = CreateUnitTestSuite (
> > > +             &ShimTests, Framework,
> > > +             "Variable Lock Shim Tests", "VarPolicy.VarLockShim",
> NULL,
> > > NULL
> > > +             );
> > > +  if (EFI_ERROR (Status)) {
> > > +    DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for
> > > ShimTests\n"));
> > > +    Status = EFI_OUT_OF_RESOURCES;
> > > +    goto EXIT;
> > > +  }
> > > +  AddTestCase (
> > > +    ShimTests,
> > > +    "Locking a variable with no matching policies should always work",
> > > "EmptyPolicies",
> > > +    LockingWithoutAnyPoliciesShouldSucceed, LibInitMocked,
> LibCleanup,
> > > NULL
> > > +    );
> > > +  AddTestCase (
> > > +    ShimTests,
> > > +    "Locking a variable twice should always work", "DoubleLock",
> > > +    LockingTwiceShouldSucceed, LibInitMocked, LibCleanup, NULL
> > > +    );
> > > +  AddTestCase (
> > > +    ShimTests,
> > > +    "Locking a variable that's already locked by another policy should
> > work",
> > > "LockAfterPolicy",
> > > +    LockingALockedVariableShouldSucceed, LibInitMocked, LibCleanup,
> > > NULL
> > > +    );
> > > +  AddTestCase (
> > > +    ShimTests,
> > > +    "Locking a variable that already has an unlocked policy should fail",
> > > "LockAfterUnlockedPolicy",
> > > +    LockingAnUnlockedVariableShouldFail, LibInitMocked, LibCleanup,
> > > NULL
> > > +    );
> > > +  AddTestCase (
> > > +    ShimTests,
> > > +    "Locking a variable that already has an locked policy should
> > succeed",
> > > "LockAfterLockedPolicyMatchingData",
> > > +    LockingALockedVariableWithMatchingDataShouldSucceed,
> > > LibInitMocked, LibCleanup, NULL
> > > +    );
> > > +  AddTestCase (
> > > +    ShimTests,
> > > +    "Locking a variable that already has an locked policy with matching
> > > data should succeed", "LockAfterLockedPolicyNonMatchingData",
> > > +    LockingALockedVariableWithNonMatchingDataShouldFail,
> > > LibInitMocked, LibCleanup, NULL
> > > +    );
> > > +  AddTestCase (
> > > +    ShimTests,
> > > +    "Adding a policy for a variable that has previously been locked
> > should
> > > always fail", "SetPolicyAfterLock",
> > > +    SettingPolicyForALockedVariableShouldFail, LibInitMocked,
> LibCleanup,
> > > NULL
> > > +    );
> > > +
> > > +  //
> > > +  // Execute the tests.
> > > +  //
> > > +  Status = RunAllTestSuites (Framework);
> > > +
> > > +EXIT:
> > > +  if (Framework != NULL) {
> > > +    FreeUnitTestFramework (Framework);
> > > +  }
> > > +
> > > +  return;
> > > +}
> > > +
> > > +///
> > > +/// Avoid ECC error for function name that starts with lower case letter
> > > +///
> > > +#define Main main
> > > +
> > > +/**
> > > +  Standard POSIX C entry point for host based unit test execution.
> > > +
> > > +  @param[in] Argc  Number of arguments
> > > +  @param[in] Argv  Array of pointers to arguments
> > > +
> > > +  @retval 0      Success
> > > +  @retval other  Error
> > > +**/
> > > +INT32
> > > +Main (
> > > +  IN INT32  Argc,
> > > +  IN CHAR8  *Argv[]
> > > +  )
> > > +{
> > > +  UnitTestMain ();
> > > +  return 0;
> > > +}
> > > diff --git
> > >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> > > ableLockRequestToLockUnitTest.inf
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> > > ableLockRequestToLockUnitTest.inf
> > > new file mode 100644
> > > index 000000000000..2a659d7e1370
> > > --- /dev/null
> > > +++
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> > > ableLockRequestToLockUnitTest.inf
> > > @@ -0,0 +1,36 @@
> > > +## @file
> > > +# This is a host-based unit test for the VariableLockRequestToLock shim.
> > > +#
> > > +# Copyright (c) Microsoft Corporation.
> > > +# SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +##
> > > +
> > > +[Defines]
> > > +  INF_VERSION         = 0x00010017
> > > +  BASE_NAME           = VariableLockRequestToLockUnitTest
> > > +  FILE_GUID           =
> A7388B6C-7274-4717-9649-BDC5DFD1FCBE
> > > +  VERSION_STRING      = 1.0
> > > +  MODULE_TYPE         = HOST_APPLICATION
> > > +
> > > +#
> > > +# The following information is for reference only and not required by the
> > > build tools.
> > > +#
> > > +#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
> > > +#
> > > +
> > > +[Sources]
> > > +  VariableLockRequestToLockUnitTest.c
> > > +  ../VariableLockRequestToLock.c
> > > +
> > > +[Packages]
> > > +  MdePkg/MdePkg.dec
> > > +  MdeModulePkg/MdeModulePkg.dec
> > > +  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
> > > +
> > > +[LibraryClasses]
> > > +  UnitTestLib
> > > +  DebugLib
> > > +  VariablePolicyLib
> > > +  VariablePolicyHelperLib
> > > +  BaseMemoryLib
> > > +  MemoryAllocationLib
> > > --
> > > 2.29.2.windows.2
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 




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

end of thread, other threads:[~2020-12-15  0:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-11  8:01 [Patch v4 0/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior Michael D Kinney
2020-12-11  8:01 ` [Patch v4 1/2] " Michael D Kinney
2020-12-14  1:39   ` 回复: [edk2-devel] " gaoliming
2020-12-11  8:01 ` [Patch v4 2/2] MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit Tests Michael D Kinney
2020-12-14  1:56   ` 回复: [edk2-devel] " gaoliming
2020-12-14 17:52     ` Michael D Kinney
2020-12-15  0:56       ` 回复: " gaoliming
2020-12-11  8:12 ` [Patch v4 0/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior Wu, Hao A

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