From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (NAM12-BN8-obe.outbound.protection.outlook.com [40.107.237.122]) by mx.groups.io with SMTP id smtpd.web12.1097.1593671795495876139 for ; Wed, 01 Jul 2020 23:36:36 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@microsoft.com header.s=selector2 header.b=XYIO+iEO; spf=pass (domain: microsoft.com, ip: 40.107.237.122, mailfrom: bret.barkelew@microsoft.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UCh0spyi/IalADJRigQDHnNstWYnQ0DbtGvtABadqE0/juQ/XtPaHtcqqa+sgtb2LimEk16j+uUnbiGAfJLy20KLJpo+9eeaUeLbgX9Qg1cUeV+ITWjNhx2U2lv+j4bH2CrqUHDyvilzvE7QtMmzhmTRxs5qnvee/9AAoFAEyujEmcJpVyoNkGT5VIv/guqDyNzSq5AHmlLJEGHf6SD19CQP2RuTh+l9YETJAg1OqAInDoF9VuWTaRCck5Jpmwbz8xay4tNOtefuflgpJgOYlcDdUFQnQFi0teTGdPwtwk7lU2Nkbv1Kv3txCg3/Ikrdke9xst+VpS2K6Wam1mNqjQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Zg1D5/q6vdsdEpBEPJlFe8w21q8RG47joM3/SY4SblU=; b=S9kBhLwPk53eqt8swH4v1UrKK/yUpE0qR2kxyC7o2O7vQUKzleAbMsUkvN/vdYPOfhwMqoPMhDAGjzTFVO98psBhpwOIkPsJ4R1urt/rkPply53x+Crfp0azj6K8YgV4EG1EoOjdRyuHNNl2JbuCumLpwBFOETJWFwfnpiLlB+dEneUdSdJB+mJvx9BnnHF7BAXLD4y2uiOQCXc2aRr/sz7P2mvA1fFJvmfeKw8+dydZx1vmVKFo7BDY8UB2Urc7tUgWjFgHFdyvu8Hqva8qSHIMeE3qHLM5CBR2fOc6FOo3RLrgahm+Rtq5p5S+eGEQqp0xM8+IWbjl2bnKoSIzzw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microsoft.com; dmarc=pass action=none header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Zg1D5/q6vdsdEpBEPJlFe8w21q8RG47joM3/SY4SblU=; b=XYIO+iEOTwTlr67GZ8ZJQ3Yx9mpR2FFudbVGe+8CbfmgXcbHwUD+6IcNQkjepckB2NqyCRzu8s/ik1f1q7tD2b4F0h/vSsVQsJyh0wvNlaedrGQNx82xwp0IxAIP09ZVRI/C6nyS+3tMEl74utoh8fA3fxrtN1/wfEQ39N12+Wc= Received: from CY4PR21MB0743.namprd21.prod.outlook.com (2603:10b6:903:b2::9) by CY4PR21MB0167.namprd21.prod.outlook.com (2603:10b6:903:b9::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3174.1; Thu, 2 Jul 2020 06:36:33 +0000 Received: from CY4PR21MB0743.namprd21.prod.outlook.com ([fe80::f112:82fb:d4fd:f7dd]) by CY4PR21MB0743.namprd21.prod.outlook.com ([fe80::f112:82fb:d4fd:f7dd%12]) with mapi id 15.20.3174.012; Thu, 2 Jul 2020 06:36:33 +0000 From: "Bret Barkelew" To: "devel@edk2.groups.io" , "dandan.bi@intel.com" , "bret@corthon.com" CC: "Wang, Jian J" , "Wu, Hao A" , liming.gao Subject: Re: [edk2-devel] [PATCH v6 13/14] MdeModulePkg: Drop VarLock from RuntimeDxe variable driver Thread-Topic: [edk2-devel] [PATCH v6 13/14] MdeModulePkg: Drop VarLock from RuntimeDxe variable driver Thread-Index: AQHWUBZZ8FId/2lgz0ONX8QDOTqscqjzyjc5 Date: Thu, 2 Jul 2020 06:36:32 +0000 Message-ID: References: <20200623064104.1908-1-brbarkel@microsoft.com> <20200623064104.1908-14-brbarkel@microsoft.com>, In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=True;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2020-07-02T05:54:05.2032707Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Privileged authentication-results: edk2.groups.io; dkim=none (message not signed) header.d=none;edk2.groups.io; dmarc=none action=none header.from=microsoft.com; x-originating-ip: [174.21.80.75] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 6aeca910-bdff-487e-2b20-08d81e5242d4 x-ms-traffictypediagnostic: CY4PR21MB0167: x-ld-processed: 72f988bf-86f1-41af-91ab-2d7cd011db47,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:6790; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 6XKmL8RCSKjl5oZaqWy/u6Eey6ZKqTmJS60Hiv3DCfpbLiVPkQ71aAzfrfH4D2ONbqieXQywUoG8fNljx0GHXs67ZhXfcvRIwgFe1J6pIHG2uBXKUlLbTaacccQloD24/E6ag6wqC5NhuDRShStnrVTXAn6q7CNN3DnnwiEXKjE8CVtwoG9rzPpN/CLmLX3bo4kGlFNlXm0DN+E8sXEDJBQhMAecjb3Z6fKYiQoLqYXjaHbopb5icQk5SIxF6EnOmz8SLSEzEfqft3mS+PaqbqJ3SctS4nN9lCoLCdInwmc+gmKAFE8Oft55n4OjUF2We5mv7QsULajaBhdoGYoXMPQsulfVhW0byfzIkUCk3MY4l906vIWQo48IeovhQgysOzdRC34T7Rhw7ilNkW5vAw== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CY4PR21MB0743.namprd21.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(4636009)(366004)(39860400002)(346002)(396003)(376002)(136003)(52536014)(478600001)(7696005)(8676002)(6506007)(53546011)(8936002)(54906003)(316002)(110136005)(5660300002)(66476007)(66446008)(66946007)(966005)(64756008)(10290500003)(2906002)(33656002)(66556008)(86362001)(76116006)(83380400001)(55016002)(9686003)(30864003)(166002)(71200400001)(186003)(4326008)(8990500004)(82950400001)(82960400001)(26005);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: AEibK7XUfi/F0dvYlzCVUoT1VPZLj77rTGt849zw6Lodk/eR5tiVnhjXzq9RWFyag1j0T8m+C4oF0PhcC5lEfipvdOE00h0u+dll9A4e/hNpgr+QyB4W2NKyv+2o3Y5fTgOfriqeCcjSonT62N3c6ELbqiywdyssiEn+NAc0XZlijNOaq40ZdNyvzxdJ6WLuVIuH/PLbAsioAvpvBY1wKOE2UPmKGiQxXK7wChydESLBqJYsLMJ90O0PMVl3mvlCLg0OLlFJe/ieYDKhkVCqH2YRbzPWGzOfCwmGCA7A2vOfJ/OXCfBeCk3nsLmdz95BS20ZzZfAC+AZzb7Zj00mOhQ6WN4GjIHOWb12sBPtrT4lcDVO9NOd/gUJ3Z4xjsleMb1agjbRkrs9Pk/eWFB8AkUAyPh1OlrM+FzUbw0DnYr50iIDQvtfka3vc3ko+yPYbuBKumT1S7WCR+WUJ+vnx38qp7X96ArJ+okgjSF61cE= x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CY4PR21MB0743.namprd21.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6aeca910-bdff-487e-2b20-08d81e5242d4 X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Jul 2020 06:36:32.9308 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: DDnMMVPz5SOefONKHJHbC06TaYSh805KlmBCLlck0BZ+br80DyvxCeJog7Tg3lrZU4N6nY97nCsTXoJCQgas4A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR21MB0167 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CY4PR21MB0743ABC80C60B15936AB3CCFEF6D0CY4PR21MB0743namp_" --_000_CY4PR21MB0743ABC80C60B15936AB3CCFEF6D0CY4PR21MB0743namp_ Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable 1.[Dandan]: You mentioned that this API is deprecated. So, you will retire = VarLock protocol and this API, and update caller to use VariablePolicy prot= ocol later, right? Yes, the ultimate plan is to retire VarLock once the references in the cor= e are moved to VariablePolicy. And I also see that VariablePolicy is an updated interface to replace VarL= ock and VarCheckProtocol, so will you also retire VarCheckProtocol later? B= ut in patch 9 VarCheckRegisterSetVariableCheckHandler seem still be used to= register SetVariable handler to do SetVariable check based on Variable Pol= icy. 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 an= d 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 lib= rary interface as well, but would prefer to leave this implementation as cl= ose to current functionality as possible since we=92ve had significant test= ing hours on it. - Bret From: Dandan Bi via groups.io Sent: Wednesday, July 1, 2020 7:13 PM To: devel@edk2.groups.io; bret@corthon.com Cc: Wang, Jian J; Wu, Hao A; liming.gao Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v6 13/14] MdeModulePkg: Drop V= arLock 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=3Dhttps%3A%2F%2Fbugz= illa.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&data=3D02%7C01%7Cbret.bar= kelew%40microsoft.com%7Cf9e4108a0dd543ed78dc08d81e2d7a6d%7C72f988bf86f141af= 91ab2d7cd011db47%7C1%7C0%7C637292527963905085&sdata=3D9Gqld4XSrVmEWJk02= FkZRdi2YYX6iy3Uo%2FxZB1bi80Y%3D&reserved=3D0 > > 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 phas= e 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 ma= de > read-only subsequently. > > - > > - @retval EFI_SUCCESS The variable specified by the VariableN= ame 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 =3D VarCheckLibVariablePropertyGet (VariableName, VendorGuid, > &Property); > > - if (!EFI_ERROR (Status)) { > > - Property.Property |=3D VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY; > > - } else { > > - Property.Revision =3D VAR_CHECK_VARIABLE_PROPERTY_REVISION; > > - Property.Property =3D VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY; > > - Property.Attributes =3D 0; > > - Property.MinSize =3D 1; > > - Property.MaxSize =3D MAX_UINTN; > > - } > > - Status =3D 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 > > + > > +#include > > +#include > > + > > +#include > > + > > +#include > > +#include > > +#include > > + > > + > > +/** > > + 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 pro= tocol later, right? And I also see that VariablePolicy is an updated interface to replace VarL= ock and VarCheckProtocol, so will you also retire VarCheckProtocol later? B= ut in patch 9 VarCheckRegisterSetVariableCheckHandler seem still be used to= register SetVariable handler to do SetVariable check based on Variable Pol= icy. > > + Mark a variable that will become read-only after leaving the DXE phas= e 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 ma= de > read-only subsequently. > > + > > + @retval EFI_SUCCESS The variable specified by the VariableN= ame 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 =3D NULL; > > + Status =3D 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 =3D 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 !=3D 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 > > > -=3D-=3D-=3D-=3D-=3D-=3D > Groups.io Links: You receive all messages sent to this group. > > View/Reply Online (#61587): https://nam06.safelinks.protection.outlook.c= om/?url=3Dhttps%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F61587&da= ta=3D02%7C01%7Cbret.barkelew%40microsoft.com%7Cf9e4108a0dd543ed78dc08d81e2d= 7a6d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637292527963905085&sd= ata=3DYi9Gph8o5bYQLXQSIQXEoQOnwxQzaNUY5gva8eSA4fM%3D&reserved=3D0 > Mute This Topic: https://nam06.safelinks.protection.outlook.com/?url=3Dh= ttps%3A%2F%2Fgroups.io%2Fmt%2F75057696%2F1768738&data=3D02%7C01%7Cbret.= barkelew%40microsoft.com%7Cf9e4108a0dd543ed78dc08d81e2d7a6d%7C72f988bf86f14= 1af91ab2d7cd011db47%7C1%7C0%7C637292527963905085&sdata=3DdMe0I355RiX4B%= 2BvIxpwE27TDvPuLnpwDa9Zrn9%2Frk60%3D&reserved=3D0 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://nam06.safelinks.protection.outlook.com/?url=3Dhttps= %3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&data=3D02%7C01%7Cbret.barke= lew%40microsoft.com%7Cf9e4108a0dd543ed78dc08d81e2d7a6d%7C72f988bf86f141af91= ab2d7cd011db47%7C1%7C0%7C637292527963905085&sdata=3DQcKjWaEqx7465u4jZSG= z2TBQEQoubdLsl0DmeHWA9%2FQ%3D&reserved=3D0 [dandan.bi@intel.com] > -=3D-=3D-=3D-=3D-=3D-=3D --_000_CY4PR21MB0743ABC80C60B15936AB3CCFEF6D0CY4PR21MB0743namp_ Content-Type: text/html; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable

1.[Dandan]: You mentioned that this API is deprecat= ed. 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 VariablePol= icy.


And I also see that VariablePolicy is an updated interface to replace VarL= ock and VarCheckProtocol, so will you also retire VarCheckProtocol later? B= ut 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 goa= l is to also get rid of VarCheckProtocol, but the VarCheck library interfac= e 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 discus= s an architecture that would do away with the library interface as well, bu= t would prefer to leave this implementation as close to current functionality as possible since we=92ve had significa= nt testing hours on it.

 

- Bret

 

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

 

1 comment inline, pl= ease 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.wang@intel.com>; Wu, Hao A <hao.a.w= u@intel.com>;
> Gao, Liming <liming.gao@intel.com>
> Subject: [edk2-devel] [PATCH v6 13/14] MdeModulePkg: Drop VarLock fro= m
> RuntimeDxe variable driver
>
> https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fbugzil= la.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&amp;data=3D02%7C01%7Cbret.b= arkelew%40microsoft.com%7Cf9e4108a0dd543ed78dc08d81e2d7a6d%7C72f988bf86f141= af91ab2d7cd011db47%7C1%7C0%7C637292527963905085&amp;sdata=3D9Gqld4XSrVm= EWJk02FkZRdi2YYX6iy3Uo%2FxZB1bi80Y%3D&amp;reserved=3D0
>
> 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.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Bret Barkelew <brbarkel@microsoft.com>
> Signed-off-by: Bret Barkelew <brbarkel@microsoft.com>
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c &nbs= p;            &= nbsp;  | 49 +-
> ------------
>
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock > .c | 71 ++++++++++++&= #43;+++++++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.i= nf
> |  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       &nbs= p;  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 vend= or GUID that will be made
> read-only subsequently.
>
> -
>
> -  @retval EFI_SUCCESS       =     The variable specified by the VariableName and
> the VendorGuid was marked
>
> -           &n= bsp;            = ;        as pending to be read-only.
>
> -  @retval EFI_INVALID_PARAMETER VariableName or VendorGuid is N= ULL.
>
> -           &n= bsp;            = ;        Or VariableName is an empty str= ing.
>
> -  @retval EFI_ACCESS_DENIED     EFI_END_OF_= DXE_EVENT_GROUP_GUID
> or EFI_EVENT_GROUP_READY_TO_BOOT has
>
> -           &n= bsp;            = ;        already been signaled.
>
> -  @retval EFI_OUT_OF_RESOURCES  There is not enough resour= ce to hold
> the lock request.
>
> -**/
>
> -EFI_STATUS
>
> -EFIAPI
>
> -VariableLockRequestToLock (
>
> -  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL *This,
>
> -  IN       CHAR16  &nbs= p;            &= nbsp;       *VariableName,
>
> -  IN       EFI_GUID  &n= bsp;            = ;      *VendorGuid
>
> -  )
>
> -{
>
> -  EFI_STATUS        &nb= sp;           Status;
>
> -  VAR_CHECK_VARIABLE_PROPERTY   Property;
>
> -
>
> -  AcquireLockOnlyAtBootTime (&mVariableModuleGlobal-
> >VariableGlobal.VariableServicesLock);
>
> -
>
> -  Status =3D VarCheckLibVariablePropertyGet (VariableName, Vend= orGuid,
> &Property);
>
> -  if (!EFI_ERROR (Status)) {
>
> -    Property.Property |=3D VAR_CHECK_VARIABLE_PROPERT= Y_READ_ONLY;
>
> -  } else {
>
> -    Property.Revision =3D VAR_CHECK_VARIABLE_PROPERTY= _REVISION;
>
> -    Property.Property =3D VAR_CHECK_VARIABLE_PROPERTY= _READ_ONLY;
>
> -    Property.Attributes =3D 0;
>
> -    Property.MinSize =3D 1;
>
> -    Property.MaxSize =3D MAX_UINTN;
>
> -  }
>
> -  Status =3D VarCheckLibVariablePropertySet (VariableName, Vend= orGuid,
> &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 dep= recated.
>
> +
>
> +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 PORT= ING.
1.[Dandan]: You mentioned that this API is deprecated. So, you will retire= VarLock protocol and this API, and update caller to use VariablePolicy pro= tocol later, right?
And I also see that VariablePolicy is an updated interface to replace VarL= ock and VarCheckProtocol, so will you also retire VarCheckProtocol later? B= ut 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 n= ame 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      &n= bsp;    The variable specified by the VariableName and
> the VendorGuid was marked
>
> +          &nbs= p;            &= nbsp;        as pending to be read-only.=
>
> +  @retval EFI_INVALID_PARAMETER VariableName or VendorGuid = is NULL.
>
> +          &nbs= p;            &= nbsp;        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
>
> +          &nbs= p;            &= nbsp;        already been signaled.
>
> +  @retval EFI_OUT_OF_RESOURCES  There is not enough re= source to hold
> the lock request.
>
> +**/
>
> +EFI_STATUS
>
> +EFIAPI
>
> +VariableLockRequestToLock (
>
> +  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL *This,
>
> +  IN       CHAR16  =             &nb= sp;        *VariableName,
>
> +  IN       EFI_GUID &nbs= p;            &= nbsp;      *VendorGuid
>
> +  )
>
> +{
>
> +  EFI_STATUS        = ;      Status;
>
> +  VARIABLE_POLICY_ENTRY   *NewPolicy;
>
> +
>
> +  NewPolicy =3D NULL;
>
> +  Status =3D CreateBasicVariablePolicy( VendorGuid,
>
> +          &nbs= p;            &= nbsp;           &nbs= p;  VariableName,
>
> +          &nbs= p;            &= nbsp;           &nbs= p;  VARIABLE_POLICY_NO_MIN_SIZE,
>
> +          &nbs= p;            &= nbsp;           &nbs= p;  VARIABLE_POLICY_NO_MAX_SIZE,
>
> +          &nbs= p;            &= nbsp;           &nbs= p;  VARIABLE_POLICY_NO_MUST_ATTR,
>
> +          &nbs= p;            &= nbsp;           &nbs= p;  VARIABLE_POLICY_NO_CANT_ATTR,
>
> +          &nbs= p;            &= nbsp;           &nbs= p;  VARIABLE_POLICY_TYPE_LOCK_NOW,
>
> +          &nbs= p;            &= nbsp;           &nbs= p;  &NewPolicy );
>
> +  if (!EFI_ERROR( Status )) {
>
> +    Status =3D RegisterVariablePolicy( NewPolicy = );
>
> +  }
>
> +  if (EFI_ERROR( Status )) {
>
> +    DEBUG(( DEBUG_ERROR, "%a - Failed to loc= k variable %s! %r\n",
> __FUNCTION__, VariableName, Status ));
>
> +    ASSERT_EFI_ERROR( Status );
>
> +  }
>
> +  if (NewPolicy !=3D 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/Variable= Smm.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
>
>
> -=3D-=3D-=3D-=3D-=3D-=3D
> Groups.io Links: You receive all messages sent to this group.
>
> View/Reply Online (#61587): https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fedk2.g= roups.io%2Fg%2Fdevel%2Fmessage%2F61587&amp;data=3D02%7C01%7Cbret.barkel= ew%40microsoft.com%7Cf9e4108a0dd543ed78dc08d81e2d7a6d%7C72f988bf86f141af91a= b2d7cd011db47%7C1%7C0%7C637292527963905085&amp;sdata=3DYi9Gph8o5bYQLXQS= IQXEoQOnwxQzaNUY5gva8eSA4fM%3D&amp;reserved=3D0
> Mute This Topic: https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fgroups= .io%2Fmt%2F75057696%2F1768738&amp;data=3D02%7C01%7Cbret.barkelew%40micr= osoft.com%7Cf9e4108a0dd543ed78dc08d81e2d7a6d%7C72f988bf86f141af91ab2d7cd011= db47%7C1%7C0%7C637292527963905085&amp;sdata=3DdMe0I355RiX4B%2BvIxpwE27T= DvPuLnpwDa9Zrn9%2Frk60%3D&amp;reserved=3D0
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fedk2.g= roups.io%2Fg%2Fdevel%2Funsub&amp;data=3D02%7C01%7Cbret.barkelew%40micro= soft.com%7Cf9e4108a0dd543ed78dc08d81e2d7a6d%7C72f988bf86f141af91ab2d7cd011d= b47%7C1%7C0%7C637292527963905085&amp;sdata=3DQcKjWaEqx7465u4jZSGz2TBQEQ= oubdLsl0DmeHWA9%2FQ%3D&amp;reserved=3D0  [dandan.bi@intel.com]
> -=3D-=3D-=3D-=3D-=3D-=3D


 

--_000_CY4PR21MB0743ABC80C60B15936AB3CCFEF6D0CY4PR21MB0743namp_--