1 comment inline, please check.
Thanks, Dandan > -----Original Message----- > From: devel@edk2.groups.io <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 <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com>; > Gao, Liming <liming....@intel.com> > 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 <jian.j.w...@intel.com> > Cc: Hao A Wu <hao.a...@intel.com> > Cc: Liming Gao <liming....@intel.com> > Cc: Bret Barkelew <brbar...@microsoft.com> > Signed-off-by: Bret Barkelew <brbar...@microsoft.com> > --- > 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.<BR> > > +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] 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_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_STATUS Status; > > - VAR_CHECK_VARIABLE_PROPERTY Property; > > - > > - AcquireLockOnlyAtBootTime (&mVariableModuleGlobal- > >VariableGlobal.VariableServicesLock); > > - > > - Status = VarCheckLibVariablePropertyGet (VariableName, VendorGuid, > &Property); > > - 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, > &Property); > > - > > - DEBUG ((EFI_D_INFO, "[Variable] Lock: %g:%s %r\n", VendorGuid, > VariableName, Status)); > > - > > - ReleaseLockOnlyAtBootTime (&mVariableModuleGlobal- > >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 000000000000..1f7f0b7ef06c > --- /dev/null > +++ > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLo > ck.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 <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> > > + > > + > > +/** > > + DEPRECATED. THIS IS ONLY HERE AS A CONVENIENCE WHILE PORTING. 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? 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. > > + 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. > > + > > + @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_STATUS Status; > > + VARIABLE_POLICY_ENTRY *NewPolicy; > > + > > + 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 ); > > + if (!EFI_ERROR( Status )) { > > + Status = RegisterVariablePolicy( NewPolicy ); > > + } > > + 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 ); > > + } > > + > > + return Status; > > +} > > diff --git > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > index 8debc560e6dc..3005e9617423 100644 > --- > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > +++ > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > @@ -49,6 +49,7 @@ [Sources] > VarCheck.c > > VariableExLib.c > > SpeculationBarrierDxe.c > > + VariableLockRequstToLock.c > > > > [Packages] > > MdePkg/MdePkg.dec > > diff --git > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf > index bbc8d2080193..26fbad97339f 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf > @@ -58,6 +58,7 @@ [Sources] > VariableExLib.c > > TcgMorLockSmm.c > > SpeculationBarrierSmm.c > > + VariableLockRequstToLock.c > > > > [Packages] > > MdePkg/MdePkg.dec > > diff --git > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i > nf > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm. > inf > index 62f2f9252f43..7c6fdf4d65fd 100644 > --- > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i > nf > +++ > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm. > inf > @@ -58,6 +58,7 @@ [Sources] > VariableExLib.c > > TcgMorLockSmm.c > > SpeculationBarrierSmm.c > > + VariableLockRequstToLock.c > > > > [Packages] > > MdePkg/MdePkg.dec > > -- > 2.26.2.windows.1.8.g01c50adf56.20200515075929 > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > > View/Reply Online (#61587): https://edk2.groups.io/g/devel/message/61587 > Mute This Topic: https://groups.io/mt/75057696/1768738 > Group Owner: devel+ow...@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [dandan...@intel.com] > -=-=-=-=-=-= -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61930): https://edk2.groups.io/g/devel/message/61930 Mute This Topic: https://groups.io/mt/75057696/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
