Re: [edk2-devel] [PATCH v6 13/14] MdeModulePkg: Drop VarLock from RuntimeDxe variable driver

2020-07-02 Thread Bret Barkelew via groups.io 
1.[Dandan]: You mentioned that this API is deprecated. So, you will retire 
VarLock protocol and this API, and update caller to use VariablePolicy protocol 
later, right?
Yes, the ultimate plan is to retire VarLock once the references in the core are 
moved to VariablePolicy.

And I also see that VariablePolicy is an updated interface to replace VarLock 
and VarCheckProtocol, so will you also retire VarCheckProtocol later? But in 
patch 9 VarCheckRegisterSetVariableCheckHandler seem still be used to register 
SetVariable handler to do SetVariable check based on Variable Policy.
I think that yes, the goal is to also get rid of VarCheckProtocol, but the 
VarCheck library interface is still used and useful for things like MOR and 
UefiGlobalVars.
I was planning on leaving the library interface because it seemed useful. Would 
be willing to discuss an architecture that would do away with the library 
interface as well, but would prefer to leave this implementation as close to 
current functionality as possible since we’ve had significant testing hours on 
it.

- Bret

From: Dandan Bi via groups.io<mailto:dandan.bi=intel@groups.io>
Sent: Wednesday, July 1, 2020 7:13 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; 
b...@corthon.com<mailto:b...@corthon.com>
Cc: Wang, Jian J<mailto:jian.j.w...@intel.com>; Wu, Hao 
A<mailto:hao.a...@intel.com>; liming.gao<mailto:liming@intel.com>
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v6 13/14] MdeModulePkg: Drop 
VarLock from RuntimeDxe variable driver

1 comment inline, please check.


Thanks,
Dandan
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Bret
> Barkelew
> Sent: Tuesday, June 23, 2020 2:41 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J ; Wu, Hao A ;
> Gao, Liming 
> Subject: [edk2-devel] [PATCH v6 13/14] MdeModulePkg: Drop VarLock from
> RuntimeDxe variable driver
>
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cf9e4108a0dd543ed78dc08d81e2d7a6d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637292527963905085sdata=9Gqld4XSrVmEWJk02FkZRdi2YYX6iy3Uo%2FxZB1bi80Y%3Dreserved=0
>
> Now that everything should be moved to
> VariablePolicy, drop support for the
> deprecated VarLock SMI interface and
> associated functions from variable RuntimeDxe.
>
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Liming Gao 
> Cc: Bret Barkelew 
> Signed-off-by: Bret Barkelew 
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c | 49 +-
> 
>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock
> .c | 71 
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> |  1 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf|  1
> +
>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> |  1 +
>  5 files changed, 75 insertions(+), 48 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
> index f15219df5eb8..486d85b022e1 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
> @@ -3,60 +3,13 @@
>and variable lock protocol based on VarCheckLib.
>
>
>
>  Copyright (c) 2015, Intel Corporation. All rights reserved.
>
> +Copyright (c) Microsoft Corporation.
>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
>
>  **/
>
>
>
>  #include "Variable.h"
>
>
>
> -/**
>
> -  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] VendorGuidA 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_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.

Re: [edk2-devel] [PATCH v6 13/14] MdeModulePkg: Drop VarLock from RuntimeDxe variable driver

2020-07-01 Thread Dandan Bi 
1 comment inline, please check.


Thanks,
Dandan
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Bret
> Barkelew
> Sent: Tuesday, June 23, 2020 2:41 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J ; Wu, Hao A ;
> Gao, Liming 
> Subject: [edk2-devel] [PATCH v6 13/14] MdeModulePkg: Drop VarLock from
> RuntimeDxe variable driver
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2522
> 
> Now that everything should be moved to
> VariablePolicy, drop support for the
> deprecated VarLock SMI interface and
> associated functions from variable RuntimeDxe.
> 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Liming Gao 
> Cc: Bret Barkelew 
> Signed-off-by: Bret Barkelew 
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c | 49 +-
> 
> 
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock
> .c | 71 
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> |  1 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf|  1
> +
> 
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> |  1 +
>  5 files changed, 75 insertions(+), 48 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
> index f15219df5eb8..486d85b022e1 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
> @@ -3,60 +3,13 @@
>and variable lock protocol based on VarCheckLib.
> 
> 
> 
>  Copyright (c) 2015, Intel Corporation. All rights reserved.
> 
> +Copyright (c) Microsoft Corporation.
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> 
> 
>  #include "Variable.h"
> 
> 
> 
> -/**
> 
> -  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] VendorGuidA 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_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.
> 
> -**/
> 
> -EFI_STATUS
> 
> -EFIAPI
> 
> -VariableLockRequestToLock (
> 
> -  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL *This,
> 
> -  IN   CHAR16   *VariableName,
> 
> -  IN   EFI_GUID *VendorGuid
> 
> -  )
> 
> -{
> 
> -  EFI_STATUSStatus;
> 
> -  VAR_CHECK_VARIABLE_PROPERTY   Property;
> 
> -
> 
> -  AcquireLockOnlyAtBootTime (
> >VariableGlobal.VariableServicesLock);
> 
> -
> 
> -  Status = VarCheckLibVariablePropertyGet (VariableName, VendorGuid,
> );
> 
> -  if (!EFI_ERROR (Status)) {
> 
> -Property.Property |= VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY;
> 
> -  } else {
> 
> -Property.Revision = VAR_CHECK_VARIABLE_PROPERTY_REVISION;
> 
> -Property.Property = VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY;
> 
> -Property.Attributes = 0;
> 
> -Property.MinSize = 1;
> 
> -Property.MaxSize = MAX_UINTN;
> 
> -  }
> 
> -  Status = VarCheckLibVariablePropertySet (VariableName, VendorGuid,
> );
> 
> -
> 
> -  DEBUG ((EFI_D_INFO, "[Variable] Lock: %g:%s %r\n", VendorGuid,
> VariableName, Status));
> 
> -
> 
> -  ReleaseLockOnlyAtBootTime (
> >VariableGlobal.VariableServicesLock);
> 
> -
> 
> -  return Status;
> 
> -}
> 
> -
> 
>  /**
> 
>Register SetVariable check handler.
> 
> 
> 
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLo
> ck.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLo
> ck.c
> new file mode 100644
> index ..1f7f0b7ef06c
> --- /dev/null
&g

[edk2-devel] [PATCH v6 13/14] MdeModulePkg: Drop VarLock from RuntimeDxe variable driver

2020-06-23 Thread Bret Barkelew 
https://bugzilla.tianocore.org/show_bug.cgi?id=2522

Now that everything should be moved to
VariablePolicy, drop support for the
deprecated VarLock SMI interface and
associated functions from variable RuntimeDxe.

Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Liming Gao 
Cc: Bret Barkelew 
Signed-off-by: Bret Barkelew 
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c | 49 
+-
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c | 71 

 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf |  1 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf|  1 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf   |  1 +
 5 files changed, 75 insertions(+), 48 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
index f15219df5eb8..486d85b022e1 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
@@ -3,60 +3,13 @@
   and variable lock protocol based on VarCheckLib.
 
 Copyright (c) 2015, Intel Corporation. All rights reserved.
+Copyright (c) Microsoft Corporation.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
 #include "Variable.h"
 
-/**
-  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] VendorGuidA 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_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.
-**/
-EFI_STATUS
-EFIAPI
-VariableLockRequestToLock (
-  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL *This,
-  IN   CHAR16   *VariableName,
-  IN   EFI_GUID *VendorGuid
-  )
-{
-  EFI_STATUSStatus;
-  VAR_CHECK_VARIABLE_PROPERTY   Property;
-
-  AcquireLockOnlyAtBootTime 
(>VariableGlobal.VariableServicesLock);
-
-  Status = VarCheckLibVariablePropertyGet (VariableName, VendorGuid, 
);
-  if (!EFI_ERROR (Status)) {
-Property.Property |= VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY;
-  } else {
-Property.Revision = VAR_CHECK_VARIABLE_PROPERTY_REVISION;
-Property.Property = VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY;
-Property.Attributes = 0;
-Property.MinSize = 1;
-Property.MaxSize = MAX_UINTN;
-  }
-  Status = VarCheckLibVariablePropertySet (VariableName, VendorGuid, 
);
-
-  DEBUG ((EFI_D_INFO, "[Variable] Lock: %g:%s %r\n", VendorGuid, VariableName, 
Status));
-
-  ReleaseLockOnlyAtBootTime 
(>VariableGlobal.VariableServicesLock);
-
-  return Status;
-}
-
 /**
   Register SetVariable check handler.
 
diff --git 
a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c
new file mode 100644
index ..1f7f0b7ef06c
--- /dev/null
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c
@@ -0,0 +1,71 @@
+/** @file -- VariableLockRequstToLock.c
+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
+
+**/
+
+#include 
+
+#include 
+#include 
+
+#include 
+
+#include 
+#include 
+#include 
+
+
+/**
+  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.
+
+  @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] VendorGuidA 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_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