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 <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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cf9e4108a0dd543ed78dc08d81e2d7a6d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637292527963905085&sdata=9Gqld4XSrVmEWJk02FkZRdi2YYX6iy3Uo%2FxZB1bi80Y%3D&reserved=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 <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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F61587&data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cf9e4108a0dd543ed78dc08d81e2d7a6d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637292527963905085&sdata=Yi9Gph8o5bYQLXQSIQXEoQOnwxQzaNUY5gva8eSA4fM%3D&reserved=0 > Mute This Topic: > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F75057696%2F1768738&data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cf9e4108a0dd543ed78dc08d81e2d7a6d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637292527963905085&sdata=dMe0I355RiX4B%2BvIxpwE27TDvPuLnpwDa9Zrn9%2Frk60%3D&reserved=0 > Group Owner: devel+ow...@edk2.groups.io > Unsubscribe: > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cf9e4108a0dd543ed78dc08d81e2d7a6d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637292527963905085&sdata=QcKjWaEqx7465u4jZSGz2TBQEQoubdLsl0DmeHWA9%2FQ%3D&reserved=0 > [dandan...@intel.com] > -=-=-=-=-=-= -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61954): https://edk2.groups.io/g/devel/message/61954 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] -=-=-=-=-=-=-=-=-=-=-=-
