Re: [edk2-devel] [PATCH v6 09/14] MdeModulePkg: Connect VariablePolicy business logic to VariableServices

2020-07-02 Thread Bret Barkelew via groups.io 
  1.  [Dandan]: I see other APIs in the VariablePolicyProtocol are using the 
APIs in VariablePolicyLib directly, expect this one.
Could we make the IsVariablePolicyEnabled API aligned in protocol and Lib?
This is because of an incongruity between the protocol definition and the 
library definition. For a simpler interface the library returns a BOOLEAN, but 
the Protocol returns EFI_STATUS (to align with the majority of Protocol calls). 
Could update the library to match the protocol, if it’s important, but it would 
touch a number of places.

- 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 09/14] MdeModulePkg: Connect 
VariablePolicy business logic to VariableServices


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 09/14] MdeModulePkg: Connect
> VariablePolicy business logic to VariableServices
>
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522data=02%7C01%7CBret.Barkelew%40microsoft.com%7C3cebf618dafa4cfee99d08d81e2d7eea%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637292528041413678sdata=MqZbcBcrPmvajnjVnc8Ohmvq1s3u4OaUQB0d1wrdvLw%3Dreserved=0
>
> VariablePolicy is an updated interface to
> replace VarLock and VarCheckProtocol.
>
> Add connective code to publish the VariablePolicy protocol
> and wire it to either the SMM communication interface
> or directly into the VariablePolicyLib business logic.
>
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Liming Gao 
> Cc: Bret Barkelew 
> Signed-off-by: Bret Barkelew 
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c |  53
> ++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
> | 642 
>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
> c   |  14 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> |   2 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf   |   3
> +
>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.i
> nf |  10 +
>  6 files changed, 724 insertions(+)
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> index 7d2b6c8e1fad..d404d4763e54 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> @@ -5,18 +5,34 @@
>  Copyright (C) 2013, Red Hat, Inc.
>
>  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
>
>  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
>
> +Copyright (c) Microsoft Corporation.
>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
>
>  **/
>
>
>
>  #include "Variable.h"
>
>
>
> +#include 
>
> +#include 
>
> +
>
> +EFI_STATUS
>
> +EFIAPI
>
> +ProtocolIsVariablePolicyEnabled (
>
> +  OUT BOOLEAN *State
>
> +  );
>
> +
>
>  EFI_HANDLE  mHandle= NULL;
>
>  EFI_EVENT   mVirtualAddressChangeEvent = NULL;
>
>  VOID*mFtwRegistration  = NULL;
>
>  VOID***mVarCheckAddressPointer = NULL;
>
>  UINTN   mVarCheckAddressPointerCount = 0;
>
>  EDKII_VARIABLE_LOCK_PROTOCOLmVariableLock  =
> { VariableLockRequestToLock };
>
> +EDKII_VARIABLE_POLICY_PROTOCOL  mVariablePolicyProtocol=
> { EDKII_VARIABLE_POLICY_PROTOCOL_REVISION,
>
> +
> DisableVariablePolicy,
>
> +
> ProtocolIsVariablePolicyEnabled,
>
> +
> RegisterVariablePolicy,
>
> +
> DumpVariablePolicy,
>
> +
> LockVariablePolicy };
>
>  EDKII_VAR_CHECK_PROTOCOLmVarCheck  =
> {

Re: [edk2-devel] [PATCH v6 09/14] MdeModulePkg: Connect VariablePolicy business logic to VariableServices

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 09/14] MdeModulePkg: Connect
> VariablePolicy business logic to VariableServices
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2522
> 
> VariablePolicy is an updated interface to
> replace VarLock and VarCheckProtocol.
> 
> Add connective code to publish the VariablePolicy protocol
> and wire it to either the SMM communication interface
> or directly into the VariablePolicyLib business logic.
> 
> Cc: Jian J Wang 
> Cc: Hao A Wu 
> Cc: Liming Gao 
> Cc: Bret Barkelew 
> Signed-off-by: Bret Barkelew 
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c |  53
> ++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
> | 642 
> 
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
> c   |  14 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> |   2 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf   |   3
> +
> 
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.i
> nf |  10 +
>  6 files changed, 724 insertions(+)
> 
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> index 7d2b6c8e1fad..d404d4763e54 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> @@ -5,18 +5,34 @@
>  Copyright (C) 2013, Red Hat, Inc.
> 
>  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> 
>  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> 
> +Copyright (c) Microsoft Corporation.
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> 
> 
>  #include "Variable.h"
> 
> 
> 
> +#include 
> 
> +#include 
> 
> +
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +ProtocolIsVariablePolicyEnabled (
> 
> +  OUT BOOLEAN *State
> 
> +  );
> 
> +
> 
>  EFI_HANDLE  mHandle= NULL;
> 
>  EFI_EVENT   mVirtualAddressChangeEvent = NULL;
> 
>  VOID*mFtwRegistration  = NULL;
> 
>  VOID***mVarCheckAddressPointer = NULL;
> 
>  UINTN   mVarCheckAddressPointerCount = 0;
> 
>  EDKII_VARIABLE_LOCK_PROTOCOLmVariableLock  =
> { VariableLockRequestToLock };
> 
> +EDKII_VARIABLE_POLICY_PROTOCOL  mVariablePolicyProtocol=
> { EDKII_VARIABLE_POLICY_PROTOCOL_REVISION,
> 
> +
> DisableVariablePolicy,
> 
> +
> ProtocolIsVariablePolicyEnabled,
> 
> +
> RegisterVariablePolicy,
> 
> +
> DumpVariablePolicy,
> 
> +
> LockVariablePolicy };
> 
>  EDKII_VAR_CHECK_PROTOCOLmVarCheck  =
> { VarCheckRegisterSetVariableCheckHandler,
> 
>  
> VarCheckVariablePropertySet,
> 
>  
> VarCheckVariablePropertyGet };
> 
> @@ -303,6 +319,8 @@ OnReadyToBoot (
>  }
> 
>}
> 
> 
> 
> +  ASSERT_EFI_ERROR (LockVariablePolicy ());
> 
> +
> 
>gBS->CloseEvent (Event);
> 
>  }
> 
> 
> 
> @@ -466,6 +484,28 @@ FtwNotificationEvent (
>  }
> 
> 
> 
> 
> 
> +/**
> 
> +  This API function returns whether or not the policy engine is
> 
> +  currently being enforced.
> 
> +
> 
> +  @param[out]   State   Pointer to a return value for whether the policy
> enforcement
> 
> +is currently enabled.
> 
> +
> 
> +  @retval EFI_SUCCESS
> 
> +  @retval OthersAn error has prevented this command from
> completing.
> 
> +
> 
> +**/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +ProtocolIsVariablePolicyEnabled (
> 
> +  OUT BOOLEAN *State
> 
> +  )
> 
> +{
> 
> +  *State = IsVariablePolicyEnabled ();
>

[edk2-devel] [PATCH v6 09/14] MdeModulePkg: Connect VariablePolicy business logic to VariableServices

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

VariablePolicy is an updated interface to
replace VarLock and VarCheckProtocol.

Add connective code to publish the VariablePolicy protocol
and wire it to either the SMM communication interface
or directly into the VariablePolicyLib business logic.

Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Liming Gao 
Cc: Bret Barkelew 
Signed-off-by: Bret Barkelew 
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c |  53 ++
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c| 642 

 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   |  14 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf|   2 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf   |   3 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |  10 +
 6 files changed, 724 insertions(+)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
index 7d2b6c8e1fad..d404d4763e54 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
@@ -5,18 +5,34 @@
 Copyright (C) 2013, Red Hat, Inc.
 Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP
+Copyright (c) Microsoft Corporation.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
 #include "Variable.h"
 
+#include 
+#include 
+
+EFI_STATUS
+EFIAPI
+ProtocolIsVariablePolicyEnabled (
+  OUT BOOLEAN *State
+  );
+
 EFI_HANDLE  mHandle= NULL;
 EFI_EVENT   mVirtualAddressChangeEvent = NULL;
 VOID*mFtwRegistration  = NULL;
 VOID***mVarCheckAddressPointer = NULL;
 UINTN   mVarCheckAddressPointerCount = 0;
 EDKII_VARIABLE_LOCK_PROTOCOLmVariableLock  = { 
VariableLockRequestToLock };
+EDKII_VARIABLE_POLICY_PROTOCOL  mVariablePolicyProtocol= { 
EDKII_VARIABLE_POLICY_PROTOCOL_REVISION,
+
DisableVariablePolicy,
+
ProtocolIsVariablePolicyEnabled,
+
RegisterVariablePolicy,
+
DumpVariablePolicy,
+
LockVariablePolicy };
 EDKII_VAR_CHECK_PROTOCOLmVarCheck  = { 
VarCheckRegisterSetVariableCheckHandler,
 
VarCheckVariablePropertySet,
 
VarCheckVariablePropertyGet };
@@ -303,6 +319,8 @@ OnReadyToBoot (
 }
   }
 
+  ASSERT_EFI_ERROR (LockVariablePolicy ());
+
   gBS->CloseEvent (Event);
 }
 
@@ -466,6 +484,28 @@ FtwNotificationEvent (
 }
 
 
+/**
+  This API function returns whether or not the policy engine is
+  currently being enforced.
+
+  @param[out]   State   Pointer to a return value for whether the policy 
enforcement
+is currently enabled.
+
+  @retval EFI_SUCCESS
+  @retval OthersAn error has prevented this command from 
completing.
+
+**/
+EFI_STATUS
+EFIAPI
+ProtocolIsVariablePolicyEnabled (
+  OUT BOOLEAN *State
+  )
+{
+  *State = IsVariablePolicyEnabled ();
+  return EFI_SUCCESS;
+}
+
+
 /**
   Variable Driver main entry point. The Variable driver places the 4 EFI
   runtime services in the EFI System Table and installs arch protocols
@@ -576,6 +616,19 @@ VariableServiceInitialize (
   );
   ASSERT_EFI_ERROR (Status);
 
+  // Register and initialize the VariablePolicy engine.
+  Status = InitVariablePolicyLib (VariableServiceGetVariable);
+  ASSERT_EFI_ERROR (Status);
+  Status = VarCheckRegisterSetVariableCheckHandler (ValidateSetVariable);
+  ASSERT_EFI_ERROR (Status);
+  Status = gBS->InstallMultipleProtocolInterfaces (
+,
+,
+,
+NULL
+);
+  ASSERT_EFI_ERROR (Status);
+
   return EFI_SUCCESS;
 }
 
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
new file mode 100644
index ..e2d4cf4cec1a
--- /dev/null
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
@@ -0,0 +1,642 @@
+/** @file -- VariablePolicySmmDxe.c
+This protocol allows communication with Variable Policy Engine.
+
+Copyright (c) Microsoft Corporation.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include

[edk2-devel] [PATCH v6 09/14] MdeModulePkg: Connect VariablePolicy business logic to VariableServices

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

VariablePolicy is an updated interface to
replace VarLock and VarCheckProtocol.

Add connective code to publish the VariablePolicy protocol
and wire it to either the SMM communication interface
or directly into the VariablePolicyLib business logic.

Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Liming Gao 
Cc: Bret Barkelew 
Signed-off-by: Bret Barkelew 
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c |  53 ++
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c| 642 

 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   |  14 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf|   2 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf   |   3 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |  10 +
 6 files changed, 724 insertions(+)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
index 7d2b6c8e1fad..d404d4763e54 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
@@ -5,18 +5,34 @@
 Copyright (C) 2013, Red Hat, Inc.
 Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP
+Copyright (c) Microsoft Corporation.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
 #include "Variable.h"
 
+#include 
+#include 
+
+EFI_STATUS
+EFIAPI
+ProtocolIsVariablePolicyEnabled (
+  OUT BOOLEAN *State
+  );
+
 EFI_HANDLE  mHandle= NULL;
 EFI_EVENT   mVirtualAddressChangeEvent = NULL;
 VOID*mFtwRegistration  = NULL;
 VOID***mVarCheckAddressPointer = NULL;
 UINTN   mVarCheckAddressPointerCount = 0;
 EDKII_VARIABLE_LOCK_PROTOCOLmVariableLock  = { 
VariableLockRequestToLock };
+EDKII_VARIABLE_POLICY_PROTOCOL  mVariablePolicyProtocol= { 
EDKII_VARIABLE_POLICY_PROTOCOL_REVISION,
+
DisableVariablePolicy,
+
ProtocolIsVariablePolicyEnabled,
+
RegisterVariablePolicy,
+
DumpVariablePolicy,
+
LockVariablePolicy };
 EDKII_VAR_CHECK_PROTOCOLmVarCheck  = { 
VarCheckRegisterSetVariableCheckHandler,
 
VarCheckVariablePropertySet,
 
VarCheckVariablePropertyGet };
@@ -303,6 +319,8 @@ OnReadyToBoot (
 }
   }
 
+  ASSERT_EFI_ERROR (LockVariablePolicy ());
+
   gBS->CloseEvent (Event);
 }
 
@@ -466,6 +484,28 @@ FtwNotificationEvent (
 }
 
 
+/**
+  This API function returns whether or not the policy engine is
+  currently being enforced.
+
+  @param[out]   State   Pointer to a return value for whether the policy 
enforcement
+is currently enabled.
+
+  @retval EFI_SUCCESS
+  @retval OthersAn error has prevented this command from 
completing.
+
+**/
+EFI_STATUS
+EFIAPI
+ProtocolIsVariablePolicyEnabled (
+  OUT BOOLEAN *State
+  )
+{
+  *State = IsVariablePolicyEnabled ();
+  return EFI_SUCCESS;
+}
+
+
 /**
   Variable Driver main entry point. The Variable driver places the 4 EFI
   runtime services in the EFI System Table and installs arch protocols
@@ -576,6 +616,19 @@ VariableServiceInitialize (
   );
   ASSERT_EFI_ERROR (Status);
 
+  // Register and initialize the VariablePolicy engine.
+  Status = InitVariablePolicyLib (VariableServiceGetVariable);
+  ASSERT_EFI_ERROR (Status);
+  Status = VarCheckRegisterSetVariableCheckHandler (ValidateSetVariable);
+  ASSERT_EFI_ERROR (Status);
+  Status = gBS->InstallMultipleProtocolInterfaces (
+,
+,
+,
+NULL
+);
+  ASSERT_EFI_ERROR (Status);
+
   return EFI_SUCCESS;
 }
 
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
new file mode 100644
index ..e2d4cf4cec1a
--- /dev/null
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
@@ -0,0 +1,642 @@
+/** @file -- VariablePolicySmmDxe.c
+This protocol allows communication with Variable Policy Engine.
+
+Copyright (c) Microsoft Corporation.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include