From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=yKfTtswA; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: michael.a.kubacki@intel.com) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by groups.io with SMTP; Thu, 03 Oct 2019 14:53:40 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Oct 2019 14:53:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.67,253,1566889200"; d="scan'208";a="204093914" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga002.jf.intel.com with ESMTP; 03 Oct 2019 14:53:39 -0700 Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 3 Oct 2019 14:53:38 -0700 Received: from FMSEDG002.ED.cps.intel.com (10.1.192.134) by FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 3 Oct 2019 14:53:38 -0700 Received: from NAM04-SN1-obe.outbound.protection.outlook.com (104.47.44.55) by edgegateway.intel.com (192.55.55.69) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 3 Oct 2019 14:53:38 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=lIrUs68wqB7HI4TrirxgB3AE3Rv6KamCWDcb0lozwTAHSP4vOabYOGwDaC1SlWMIKRk8prsc6nIuNB4uKn6NUvnB8ybGf+am/VATP9vy9hlRMJdCy8aVJEc5AgH98QlN/HU28YWgtDJrxxg4GxX/4xxltbU4bg8DUlX1w6KfYljIBgm+vK1iRsFwIyNr56zFJRxwDhkpTMGEk11HYseduw5BNKBC2xmAP7FwhX/1Bt254GHc0+xcOmF8lNU3d7vjSvmqEw9+P1tGNLdsma3ci7G5GeK+QiV4kxYwvC8y51PDVL9Q4Biw9JolmbBokjSn28hLU/5AKv8LfHhOaqUJSw== 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=Xxo4cPFZensNOwbqz916C16dFCAWiqMVLCVks6By6hY=; b=cwbrSfbxIts5aY+P60SeMSHnFcPdPSK63h6QKjPnHiXgYui5L3gkstaspCoEky3kB4ONpQTwci70QzkQw8qwOSgV7lxvJH8gnWP29S16Jg4OkmgN3nXdXuvbpFjd/s0X/2wbnp2VBbeCneQqCsqHBOO4rWOJUdvcZrGQMgrSAJuN1QhLp9ducEe9WS99d7H1YZe1FGd+Zj2vYir9wzf2WgF+OnIBSdyaYWWcwksE7ROcBkZgZFZkxv3ZODarFridEnFZkqVL+y5vEu64lUIeVf30IiGkwvGpXCzs4QH/6xV6yN4jsav2zKERPLxuODOcpABcVNRtNmi4Njq6rGRFkA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Xxo4cPFZensNOwbqz916C16dFCAWiqMVLCVks6By6hY=; b=yKfTtswAr9zd/rprII6h8NjHPTMMucG3Ptf5RMUt2csLvVqJswGWpSxeTVpyjHdKEMh6LlCKaKl67xCMUunfJl4Qe6F38GYY8zIPuLJ70g95m3+0iwHqWzQr67cGUIP1V3JfonmINu95pm+3feLWoks2J7pEYJAMTIUYWYF4bh0= Received: from DM6PR11MB3834.namprd11.prod.outlook.com (20.179.17.87) by DM6PR11MB3434.namprd11.prod.outlook.com (20.177.220.27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2305.22; Thu, 3 Oct 2019 21:53:34 +0000 Received: from DM6PR11MB3834.namprd11.prod.outlook.com ([fe80::59cc:8a30:6b9e:584e]) by DM6PR11MB3834.namprd11.prod.outlook.com ([fe80::59cc:8a30:6b9e:584e%3]) with mapi id 15.20.2305.023; Thu, 3 Oct 2019 21:53:34 +0000 From: "Kubacki, Michael A" To: "Wu, Hao A" , "devel@edk2.groups.io" CC: "Bi, Dandan" , Ard Biesheuvel , "Dong, Eric" , Laszlo Ersek , "Gao, Liming" , "Kinney, Michael D" , "Ni, Ray" , "Wang, Jian J" , "Yao, Jiewen" Subject: Re: [PATCH V2 7/9] MdeModulePkg/Variable: Add RT GetVariable() cache support Thread-Topic: [PATCH V2 7/9] MdeModulePkg/Variable: Add RT GetVariable() cache support Thread-Index: AQHVdZ68oSYHi0EfNkyFbThKCzvSRqdDzt8ggAV9wYA= Date: Thu, 3 Oct 2019 21:53:34 +0000 Message-ID: References: <20190928014717.31372-1-michael.a.kubacki@intel.com> <20190928014717.31372-8-michael.a.kubacki@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOGVmOGU2MTUtNDQ3Zi00YWE2LWE1OGEtNzg2ZmFkYzY0YzY2IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiWE5OT2lFdStkdXhVM0VtM0dobTFmTTdLZllEUElYNlRQckZoaWZsM2lEZEw0UmhveWllcjdHbU15Vm9CSVFPcSJ9 dlp-reaction: no-action dlp-version: 11.2.0.6 authentication-results: spf=none (sender IP is ) smtp.mailfrom=michael.a.kubacki@intel.com; x-originating-ip: [134.134.136.217] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 19a104c3-5d3e-4860-5d09-08d7484c2371 x-ms-traffictypediagnostic: DM6PR11MB3434: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 01792087B6 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(366004)(136003)(376002)(396003)(346002)(39850400004)(51874003)(13464003)(189003)(199004)(11346002)(446003)(33656002)(256004)(14444005)(3846002)(54906003)(305945005)(7736002)(316002)(110136005)(6116002)(2906002)(486006)(476003)(14454004)(74316002)(66066001)(71200400001)(6246003)(26005)(71190400001)(107886003)(53546011)(8676002)(6506007)(86362001)(81156014)(6436002)(55016002)(9686003)(102836004)(229853002)(25786009)(81166006)(5660300002)(6306002)(7696005)(52536014)(8936002)(64756008)(66556008)(66476007)(66946007)(30864003)(99286004)(2501003)(76116006)(66446008)(4326008)(76176011)(478600001)(186003)(567084001)(559001)(579004)(569006);DIR:OUT;SFP:1102;SCL:1;SRVR:DM6PR11MB3434;H:DM6PR11MB3834.namprd11.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Uh0TooAVPWOPDBxDwirNrpg0egniOQfhcdvzcd4zcF6jDpo5JNO2a5LfkvrBZNaHyU5+agIyz69kFJ7JvTk647/CsWStP64mk/cQ9IJoVFLthGNUmTvj4Y4lHa4FSDVBwv6u7q4zj14Ncmwpx8oUSBlzXTen4nUUBG6fbHD52IOysxXLIXsGesLA9dJShITyZ/CGpGIU4bZYKacnSezEhJQhXDzGMY/YcGfqhV8Ro9xz9iodqltUK5m6+iG1Hex6N7KyHwcMNhRHGYiqOPOzF92Gm3x1YQNT0DMeN38cD4g/75oY0eQLCJeOdoDleO9+Oz5Lw5+LEbJNKPlqK1twtayZ0TbgOhrK0nybPVfYJGNWdoQmjAIsXezx3mMjgTy51qF+DqkLLng+d0Tisew3MyC4kc1L+2rzO6iJ+WLvFSg= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 19a104c3-5d3e-4860-5d09-08d7484c2371 X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Oct 2019 21:53:34.0603 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: mpODUdqSSYWnKsv0q6je1T25CovIGMO7Ofzx2RSjW+j8i1a6iUUxmIgdIu+NPt5VuyWBqVGT7RgLPBuVPtubp1glIo38deIYHCyX++/mvIc= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB3434 Return-Path: michael.a.kubacki@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable #1 - The plan is to remove the polling entirely in V3. #2 - I'd prefer to take a definitive direction and reduce validation and ma= intenance effort but you and Laszlo both requested this so I'll add a Feature= PCD to control activation of the runtime cache in this patch series. Perhaps this = can be removed in the future. #3 - Will be done in V3.=20 Other replies are inline. > -----Original Message----- > From: Wu, Hao A > Sent: Thursday, October 3, 2019 1:05 AM > To: Kubacki, Michael A ; > devel@edk2.groups.io > Cc: Bi, Dandan ; Ard Biesheuvel > ; Dong, Eric ; Laszlo Ers= ek > ; Gao, Liming ; Kinney, Michael > D ; Ni, Ray ; Wang, Jian J > ; Yao, Jiewen > Subject: RE: [PATCH V2 7/9] MdeModulePkg/Variable: Add RT GetVariable() > cache support >=20 > Before any comment on the patch, since I am not experienced in the > Variable > driver, I would like to ask for help from other reviewers to look into th= is > patch and provide feedbacks as well. Thanks in advance. >=20 > With the above fact, some comments provided below maybe wrong. So > please help > to kindly correct me. >=20 >=20 > Some general comments: > 1. I am not sure if bringing the TimerLib dependency (delay in acquiring = the > runtime cache read lock) to variable driver (a software driver for the= most > part) is a good idea. >=20 > Hope other reviewers can provide some feedbacks for this. Thanks in > advance. >=20 > 2. In my opinion, I prefer a switch can be provided for platform owners t= o > choose between using the runtime cache and going through SMM for > GetVariable > (and for GetNextVariableName in the next patch as well). >=20 > If platform owners feel uncomfortable with using the runtime cache wit= h > regard to the security perspective, they can switch to the origin solu= tion. >=20 > 3. Please help to remove the 'EFIAPI' keyword for new driver internal > functions; >=20 >=20 > Inline comments below: >=20 >=20 > > -----Original Message----- > > From: Kubacki, Michael A > > Sent: Saturday, September 28, 2019 9:47 AM > > To: devel@edk2.groups.io > > Cc: Bi, Dandan; Ard Biesheuvel; Dong, Eric; Laszlo Ersek; Gao, Liming; > Kinney, > > Michael D; Ni, Ray; Wang, Jian J; Wu, Hao A; Yao, Jiewen > > Subject: [PATCH V2 7/9] MdeModulePkg/Variable: Add RT GetVariable() > > cache support > > > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2220 > > > > This change reduces SMIs for GetVariable () by maintaining a > > UEFI variable cache in Runtime DXE in addition to the pre- > > existing cache in SMRAM. When the Runtime Service GetVariable() > > is invoked, a Runtime DXE cache is used instead of triggering an > > SMI to VariableSmm. This can improve overall system performance > > by servicing variable read requests without rendezvousing all > > cores into SMM. > > > > The following are important points regarding this change. > > > > 1. All of the non-volatile storage contents are loaded into the > > cache upon driver load. This one time load operation from storage > > is preferred as opposed to building the cache on demand. An on- > > demand cache would require a fallback SMI to load data into the > > cache as variables are requested. > > > > 2. SetVariable () requests will continue to always trigger an SMI. > > This occurs regardless of whether the variable is volatile or > > non-volatile. > > > > 3. Both volatile and non-volatile variables are cached in a runtime > > buffer. As is the case in the current EDK II variable driver, they > > continue to be cached in separate buffers. > > > > 4. The cache in Runtime DXE and SMM are intended to be exact copies > > of one another. All SMM variable accesses only return data from the > > SMM cache. The runtime caches are only updated after the variable I/= O > > operation is successful in SMM. The runtime caches are only updated > > from SMM. > > > > 5. Synchronization mechanisms are in place to ensure the runtime cache > > content integrity with the SMM cache. These may result in updates to > > runtime cache that are the same in content but different in offset a= nd > > size from updates to the SMM cache. > > > > When using SMM variables, two caches will now be present. > > 1. "Runtime Cache" - Maintained in VariableSmmRuntimeDxe. Used to > > service > > Runtime Services GetVariable () and GetNextVariableName () callers. > > 2. "SMM Cache" - Maintained in VariableSmm to service SMM GetVariable > () > > and GetNextVariableName () callers. > > a. This cache is retained so SMM modules do not operate on data outs= ide > > SMRAM. > > > > It is possible to view UEFI variable read and write statistics by setti= ng > > the gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics > > FeaturePcd > > to TRUE and using the VariableInfo UEFI application in MdeModulePkg to > > dump > > variable statistics to the console. By doing so, a user can view the nu= mber > > of GetVariable () hits from the Runtime DXE variable driver (Runtime Ca= che > > hits) and the SMM variable driver (SMM Cache hits). SMM Cache hits for > > GetVariable () will occur when SMM modules invoke GetVariable (). > > > > Cc: Dandan Bi > > Cc: Ard Biesheuvel > > Cc: Eric Dong > > Cc: Laszlo Ersek > > Cc: Liming Gao > > Cc: Michael D Kinney > > Cc: Ray Ni > > Cc: Jian J Wang > > Cc: Hao A Wu > > Cc: Jiewen Yao > > Signed-off-by: Michael Kubacki > > --- > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > > | 2 + > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | > 2 > > + > > > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.i > > nf | 31 +- > > > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf > > | 2 + > > MdeModulePkg/Include/Guid/SmmVariableCommon.h |= 29 +- > > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h |= 39 > +- > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.h > > | 47 ++ > > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c |= 44 > +- > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c > > | 153 +++++ > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | > 114 > > +++- > > > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe. > > c | 608 +++++++++++++++++--- > > 11 files changed, 966 insertions(+), 105 deletions(-) > > > > diff --git > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > > index 08a5490787..ceea5d1ff9 100644 > > --- > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > > +++ > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf > > @@ -40,6 +40,8 @@ > > VariableNonVolatile.h > > VariableParsing.c > > VariableParsing.h > > + VariableRuntimeCache.c > > + VariableRuntimeCache.h >=20 >=20 > Per my understanding, the module specified by VariableRuntimeDxe.inf > does not > involve SMM/SMI for variable services (like GetVariable). It looks weird = to > me > for this INF to include the newly introduced runtime cache codes (below > source > header files): >=20 > VariableRuntimeCache.c > VariableRuntimeCache.h >=20 >=20 This is because Variable.c is common to the runtime DXE and SMM variable driver and it contains the code to update variable caches. The runtime cach= e synchronization function (SynchronizeRuntimeVariableCache ()) will return if the runtime cache pointer is NULL. > > PrivilegePolymorphic.h > > Measurement.c > > TcgMorLockDxe.c > > diff --git > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf > > index 6dc2721b81..bc3033588d 100644 > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf > > @@ -49,6 +49,8 @@ > > VariableNonVolatile.h > > VariableParsing.c > > VariableParsing.h > > + VariableRuntimeCache.c > > + VariableRuntimeCache.h > > VarCheck.c > > Variable.h > > PrivilegePolymorphic.h > > diff --git > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > > e.inf > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > > e.inf > > index 14894e6f13..70837ac6e0 100644 > > --- > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > > e.inf > > +++ > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > > e.inf > > @@ -13,7 +13,7 @@ > > # may not be modified without authorization. If platform fails to pro= tect > > these resources, > > # the authentication service provided in this driver will be broken, = and the > > behavior is undefined. > > # > > -# Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved. > > +# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved. > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > # > > ## > > @@ -39,6 +39,10 @@ > > VariableSmmRuntimeDxe.c > > PrivilegePolymorphic.h > > Measurement.c > > + VariableParsing.c > > + VariableParsing.h > > + VariableRuntimeCache.c > > + VariableRuntimeCache.h > > > > [Packages] > > MdePkg/MdePkg.dec > > @@ -49,6 +53,7 @@ > > BaseLib > > UefiBootServicesTableLib > > DebugLib > > + TimerLib > > UefiRuntimeLib > > DxeServicesTableLib > > UefiDriverEntryPoint > > @@ -65,7 +70,29 @@ > > gEdkiiVariableLockProtocolGuid ## PRODUCES > > gEdkiiVarCheckProtocolGuid ## PRODUCES > > > > +[Pcd] > > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize = ## > > CONSUMES > > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize > ## > > CONSUMES > > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize > > ## CONSUMES > > + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize = ## > > CONSUMES > > + gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize = ## > > CONSUMES > > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxUserNvVariableSpaceSize > > ## CONSUMES > > + > > > gEfiMdeModulePkgTokenSpaceGuid.PcdBoottimeReservedNvVariableSpace > > Size ## CONSUMES >=20 >=20 > Not sure if the above PCDs are really needed by VariableSmmRuntimeDxe. >=20 >=20 I will double check and remove any not required. > > + > > +[FeaturePcd] > > + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics > ## > > CONSUMES > > + > > [Guids] > > + ## PRODUCES ## GUID # Signature of Variable store header > > + ## CONSUMES ## GUID # Signature of Variable store header > > + ## SOMETIMES_PRODUCES ## SystemTable > > + gEfiAuthenticatedVariableGuid > > + > > + ## PRODUCES ## GUID # Signature of Variable store header > > + ## CONSUMES ## GUID # Signature of Variable store header > > + ## SOMETIMES_PRODUCES ## SystemTable > > + gEfiVariableGuid > > + > > gEfiEventVirtualAddressChangeGuid ## CONSUMES ## Event > > gEfiEventExitBootServicesGuid ## CONSUMES ## Event > > ## CONSUMES ## GUID # Locate protocol > > @@ -82,6 +109,8 @@ > > ## SOMETIMES_CONSUMES ## Variable:L"dbt" > > gEfiImageSecurityDatabaseGuid > > > > + gEdkiiPiSmmCommunicationRegionTableGuid ## > > SOMETIMES_CONSUMES ## SystemTable > > + > > [Depex] > > gEfiSmmCommunicationProtocolGuid > > > > diff --git > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i > > nf > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm. > > inf > > index ca9d23ce9f..95c5310c0b 100644 > > --- > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i > > nf > > +++ > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm. > > inf > > @@ -49,6 +49,8 @@ > > VariableNonVolatile.h > > VariableParsing.c > > VariableParsing.h > > + VariableRuntimeCache.c > > + VariableRuntimeCache.h > > VarCheck.c > > Variable.h > > PrivilegePolymorphic.h > > diff --git a/MdeModulePkg/Include/Guid/SmmVariableCommon.h > > b/MdeModulePkg/Include/Guid/SmmVariableCommon.h > > index c527a59891..ceef44dfd2 100644 > > --- a/MdeModulePkg/Include/Guid/SmmVariableCommon.h > > +++ b/MdeModulePkg/Include/Guid/SmmVariableCommon.h > > @@ -1,7 +1,7 @@ > > /** @file > > The file defined some common structures used for communicating > > between SMM variable module and SMM variable wrapper module. > > > > -Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
> > +Copyright (c) 2011 - 2019, Intel Corporation. All rights reserved.
> > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > **/ > > @@ -9,6 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #ifndef _SMM_VARIABLE_COMMON_H_ > > #define _SMM_VARIABLE_COMMON_H_ > > > > +#include > > #include > > > > #define EFI_SMM_VARIABLE_WRITE_GUID \ > > @@ -66,6 +67,16 @@ typedef struct { > > #define > > SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET 10 > > > > #define SMM_VARIABLE_FUNCTION_GET_PAYLOAD_SIZE 11 > > +// > > +// The payload for this function is > > SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT > > +// > > +#define > > SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT > > 12 > > + > > +#define SMM_VARIABLE_FUNCTION_SYNC_RUNTIME_CACHE > 13 > > +// > > +// The payload for this function is > > SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO > > +// > > +#define SMM_VARIABLE_FUNCTION_GET_RUNTIME_CACHE_INFO > > 14 > > > > /// > > /// Size of SMM communicate header, without including the payload. > > @@ -120,4 +131,20 @@ typedef struct { > > UINTN VariablePayloadSize; > > } SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE; > > > > +typedef struct { > > + BOOLEAN *ReadLock; > > + BOOLEAN *PendingUpdate; > > + BOOLEAN *HobFlushComplete; > > + VARIABLE_STORE_HEADER *RuntimeHobCache; > > + VARIABLE_STORE_HEADER *RuntimeNvCache; > > + VARIABLE_STORE_HEADER *RuntimeVolatileCache; > > +} > > > SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT; > > + > > +typedef struct { > > + UINTN TotalHobStorageSize; > > + UINTN TotalNvStorageSize; > > + UINTN TotalVolatileStorageSize; > > + BOOLEAN AuthenticatedVariableUsage; > > +} SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO; > > + > > #endif // _SMM_VARIABLE_COMMON_H_ > > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h > > index fb574b2e32..b9723c0250 100644 > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h > > @@ -57,6 +57,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > /// > > #define ISO_639_2_ENTRY_SIZE 3 > > > > +/// > > +/// The timeout to in 10us units to wait for the > > +/// variable runtime cache read lock to be acquired. > > +/// > > +#define VARIABLE_RT_CACHE_READ_LOCK_TIMEOUT 200000 > > + > > typedef enum { > > VariableStoreTypeVolatile, > > VariableStoreTypeHob, > > @@ -64,6 +70,21 @@ typedef enum { > > VariableStoreTypeMax > > } VARIABLE_STORE_TYPE; > > > > +typedef struct { > > + UINT32 PendingUpdateOffset; > > + UINT32 PendingUpdateLength; > > + VARIABLE_STORE_HEADER *Store; > > +} VARIABLE_RUNTIME_CACHE; > > + > > +typedef struct { > > + BOOLEAN *ReadLock; > > + BOOLEAN *PendingUpdate; > > + BOOLEAN *HobFlushComplete; > > + VARIABLE_RUNTIME_CACHE VariableRuntimeHobCache; > > + VARIABLE_RUNTIME_CACHE VariableRuntimeNvCache; > > + VARIABLE_RUNTIME_CACHE VariableRuntimeVolatileCache; > > +} VARIABLE_RUNTIME_CACHE_CONTEXT; > > + > > typedef struct { > > VARIABLE_HEADER *CurrPtr; > > // > > @@ -79,14 +100,16 @@ typedef struct { > > } VARIABLE_POINTER_TRACK; > > > > typedef struct { > > - EFI_PHYSICAL_ADDRESS HobVariableBase; > > - EFI_PHYSICAL_ADDRESS VolatileVariableBase; > > - EFI_PHYSICAL_ADDRESS NonVolatileVariableBase; > > - EFI_LOCK VariableServicesLock; > > - UINT32 ReentrantState; > > - BOOLEAN AuthFormat; > > - BOOLEAN AuthSupport; > > - BOOLEAN EmuNvMode; > > + EFI_PHYSICAL_ADDRESS HobVariableBase; > > + EFI_PHYSICAL_ADDRESS HobVariableBackupBase; >=20 >=20 > I do not see any usage of the new field "HobVariableBackupBase". > Could you help to double confirm? >=20 >=20 You are correct. I removed usage of this variable before sending the patch series and the global variable here needs to be cleaned up. > > + EFI_PHYSICAL_ADDRESS VolatileVariableBase; > > + EFI_PHYSICAL_ADDRESS NonVolatileVariableBase; > > + VARIABLE_RUNTIME_CACHE_CONTEXT VariableRuntimeCacheContext; > > + EFI_LOCK VariableServicesLock; > > + UINT32 ReentrantState; > > + BOOLEAN AuthFormat; > > + BOOLEAN AuthSupport; > > + BOOLEAN EmuNvMode; > > } VARIABLE_GLOBAL; > > > > typedef struct { > > diff --git > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.h > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache. > > h > > new file mode 100644 > > index 0000000000..09b83eb215 > > --- /dev/null > > +++ > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache. > > h > > @@ -0,0 +1,47 @@ > > +/** @file > > + The common variable volatile store routines shared by the > DXE_RUNTIME > > variable > > + module and the DXE_SMM variable module. > > + > > +Copyright (c) 2019, Intel Corporation. All rights reserved.
> > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#ifndef _VARIABLE_RUNTIME_CACHE_H_ > > +#define _VARIABLE_RUNTIME_CACHE_H_ > > + > > +#include "Variable.h" > > + > > +/** > > + Copies any pending updates to runtime variable caches. > > + > > + @retval EFI_UNSUPPORTED The volatile store to be updated is = not > > initialized properly. > > + @retval EFI_SUCCESS The volatile store was updated succe= ssfully. > > + > > +**/ > > +EFI_STATUS > > +SynchronizeRuntimeVariableCacheEx ( > > + VOID > > + ); > > + > > +/** > > + Synchronizes the runtime variable caches with all pending updates > outside > > runtime. > > + > > + Ensures all conditions are met to maintain coherency for runtime cac= he > > updates. > > + > > + @param[in] VariableRuntimeCache Variable runtime cache structure for > > the runtime cache being synchronized. > > + @param[in] Offset Offset in bytes to apply the update. > > + @param[in] Length Length of data in bytes of the updat= e. > > + > > + @retval EFI_UNSUPPORTED The volatile store to be updated is = not > > initialized properly. > > + @retval EFI_SUCCESS The volatile store was updated succe= ssfully. > > + > > +**/ > > +EFI_STATUS > > +SynchronizeRuntimeVariableCache ( > > + IN VARIABLE_RUNTIME_CACHE *VariableRuntimeCache, > > + IN UINTN Offset, > > + IN UINTN Length > > + ); > > + > > +#endif > > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > > index 5da2354aa5..bb2fa3fc19 100644 > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > > @@ -25,6 +25,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #include "Variable.h" > > #include "VariableNonVolatile.h" > > #include "VariableParsing.h" > > +#include "VariableRuntimeCache.h" > > > > VARIABLE_MODULE_GLOBAL *mVariableModuleGlobal; > > > > @@ -332,6 +333,12 @@ RecordVarErrorFlag ( > > // Update the data in NV cache. > > // > > *VarErrFlag =3D TempFlag; > > + Status =3D SynchronizeRuntimeVariableCache ( > > + &mVariableModuleGlobal- > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache, > > + (UINTN) VarErrFlag - (UINTN) mNvVariableCache + (UIN= TN) > > mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase, > > + sizeof (TempFlag) > > + ); > > + ASSERT_EFI_ERROR (Status); > > } > > } > > } > > @@ -755,12 +762,24 @@ Reclaim ( > > > > Done: > > if (IsVolatile || mVariableModuleGlobal->VariableGlobal.EmuNvMode) { > > + Status =3D SynchronizeRuntimeVariableCache ( > > + &mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCach > > e, > > + 0, > > + VariableStoreHeader->Size > > + ); > > + ASSERT_EFI_ERROR (Status); > > FreePool (ValidBuffer); > > } else { > > // > > // For NV variable reclaim, we use mNvVariableCache as the buffer,= so > > copy the data back. > > // > > - CopyMem (mNvVariableCache, (UINT8 *)(UINTN)VariableBase, > > VariableStoreHeader->Size); > > + CopyMem (mNvVariableCache, (UINT8 *) (UINTN) VariableBase, > > VariableStoreHeader->Size); > > + Status =3D SynchronizeRuntimeVariableCache ( > > + &(mVariableModuleGlobal- > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache), > > + 0, > > + VariableStoreHeader->Size > > + ); > > + ASSERT_EFI_ERROR (Status); > > } > > > > return Status; > > @@ -1592,6 +1611,7 @@ UpdateVariable ( > > VARIABLE_POINTER_TRACK *Variable; > > VARIABLE_POINTER_TRACK NvVariable; > > VARIABLE_STORE_HEADER *VariableStoreHeader; > > + VARIABLE_RUNTIME_CACHE *VolatileCacheInstance; > > UINT8 *BufferForMerge; > > UINTN MergedBufSize; > > BOOLEAN DataReady; > > @@ -2235,6 +2255,21 @@ UpdateVariable ( > > } > > > > Done: > > + if (!EFI_ERROR (Status)) { > > + if (Variable->Volatile) { > > + VolatileCacheInstance =3D &(mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCach > > e); > > + } else { > > + VolatileCacheInstance =3D &(mVariableModuleGlobal- > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache); > > + } > > + > > + Status =3D SynchronizeRuntimeVariableCache ( > > + VolatileCacheInstance, > > + 0, > > + VolatileCacheInstance->Store->Size > > + ); > > + ASSERT_EFI_ERROR (Status); > > + } > > + > > return Status; > > } > > > > @@ -3409,6 +3444,12 @@ FlushHobVariableToFlash ( > > ErrorFlag =3D TRUE; > > } > > } > > + Status =3D SynchronizeRuntimeVariableCache ( > > + &mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeHobCache, > > + 0, > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeHobCache.S > > tore->Size > > + ); > > + ASSERT_EFI_ERROR (Status); > > if (ErrorFlag) { > > // > > // We still have HOB variable(s) not flushed in flash. > > @@ -3419,6 +3460,7 @@ FlushHobVariableToFlash ( > > // All HOB variables have been flushed in flash. > > // > > DEBUG ((EFI_D_INFO, "Variable driver: all HOB variables have bee= n > > flushed in flash.\n")); > > + *(mVariableModuleGlobal- > > >VariableGlobal.VariableRuntimeCacheContext.HobFlushComplete) =3D > TRUE; > > if (!AtRuntime ()) { > > FreePool ((VOID *) VariableStoreHeader); > > } > > diff --git > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c > > new file mode 100644 > > index 0000000000..2642d9b000 > > --- /dev/null > > +++ > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c > > @@ -0,0 +1,153 @@ > > +/** @file > > + The common variable volatile store routines shared by the > DXE_RUNTIME > > variable > > + module and the DXE_SMM variable module. > > + > > + Caution: This module requires additional review when modified. > > + This driver will have external input - variable data. They may be in= put in > > SMM mode. > > + This external input must be validated carefully to avoid security is= sue like > > + buffer overflow, integer overflow. > > + > > +Copyright (c) 2019, Intel Corporation. All rights reserved.
> > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#include "VariableParsing.h" > > +#include "VariableRuntimeCache.h" > > + > > +extern VARIABLE_MODULE_GLOBAL *mVariableModuleGlobal; > > +extern VARIABLE_STORE_HEADER *mNvVariableCache; > > + > > +/** > > + Copies any pending updates to runtime variable caches. > > + > > + @retval EFI_UNSUPPORTED The volatile store to be updated is = not > > initialized properly. > > + @retval EFI_SUCCESS The volatile store was updated succe= ssfully. > > + > > +**/ > > +EFI_STATUS > > +SynchronizeRuntimeVariableCacheEx ( >=20 >=20 > It is not clear to me why this function is named as the "Ex" version of f= unction > SynchronizeRuntimeVariableCache(). For me, this function looks more like = a > basic > version. >=20 > I would suggest a name change for the functions provided in file > VariableRuntimeCache.c to better reflect their usage model. >=20 >=20 I'll rename it in V3. > > + VOID > > + ) > > +{ >=20 >=20 > I would recommend that at least a local variable should be introduced to > reduce > the duplications of: >=20 > "mVariableModuleGlobal->VariableGlobal.VariableRuntimeCacheContext" >=20 > in this function in order to make it easier to read. >=20 >=20 I'll add it in V3. > > + if ( > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache.St > > ore =3D=3D NULL || > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCach > > e.Store =3D=3D NULL || > > + mVariableModuleGlobal- > > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdate =3D=3D NULL > > + ) { > > + return EFI_UNSUPPORTED; > > + } > > + > > + if (*(mVariableModuleGlobal- > > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdate)) { > > + if ( > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeHobCache.S > > tore !=3D NULL && > > + mVariableModuleGlobal->VariableGlobal.HobVariableBase > 0 > > + ) { > > + CopyMem ( > > + (VOID *) ( > > + ((UINT8 *) (UINTN) mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeHobCache.S > > tore) + > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeHobCache.P > > endingUpdateOffset > > + ), > > + (VOID *) ( > > + ((UINT8 *) (UINTN) mVariableModuleGlobal- > > >VariableGlobal.HobVariableBase) + > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeHobCache.P > > endingUpdateOffset > > + ), > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeHobCache.P > > endingUpdateLength > > + ); > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeHobCache.P > > endingUpdateLength =3D 0; > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeHobCache.P > > endingUpdateOffset =3D 0; > > + } > > + > > + CopyMem ( > > + (VOID *) ( > > + ((UINT8 *) (UINTN) mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache.St > > ore) + > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache.Pe > > ndingUpdateOffset > > + ), > > + (VOID *) ( > > + ((UINT8 *) (UINTN) mNvVariableCache) + > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache.Pe > > ndingUpdateOffset > > + ), > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache.Pe > > ndingUpdateLength > > + ); > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache.Pe > > ndingUpdateLength =3D 0; > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache.Pe > > ndingUpdateOffset =3D 0; > > + > > + CopyMem ( > > + (VOID *) ( > > + ((UINT8 *) (UINTN) mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCach > > e.Store) + > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCach > > e.PendingUpdateOffset > > + ), > > + (VOID *) ( > > + ((UINT8 *) (UINTN) mVariableModuleGlobal- > > >VariableGlobal.VolatileVariableBase) + > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCach > > e.PendingUpdateOffset > > + ), > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCach > > e.PendingUpdateLength > > + ); > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCach > > e.PendingUpdateLength =3D 0; > > + mVariableModuleGlobal- > > > >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCach > > e.PendingUpdateOffset =3D 0; > > + *(mVariableModuleGlobal- > > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdate) =3D FALSE; > > + } > > + > > + return EFI_SUCCESS; > > +} > > + > > +/** > > + Synchronizes the runtime variable caches with all pending updates > outside > > runtime. > > + > > + Ensures all conditions are met to maintain coherency for runtime cac= he > > updates. > > + > > + @param[in] VariableRuntimeCache Variable runtime cache structure for > > the runtime cache being synchronized. > > + @param[in] Offset Offset in bytes to apply the update. > > + @param[in] Length Length of data in bytes of the updat= e. > > + > > + @retval EFI_UNSUPPORTED The volatile store to be updated is = not > > initialized properly. > > + @retval EFI_SUCCESS The volatile store was updated succe= ssfully. > > + > > +**/ > > +EFI_STATUS > > +SynchronizeRuntimeVariableCache ( > > + IN VARIABLE_RUNTIME_CACHE *VariableRuntimeCache, > > + IN UINTN Offset, > > + IN UINTN Length > > + ) > > +{ > > + if (VariableRuntimeCache =3D=3D NULL) { > > + return EFI_INVALID_PARAMETER; > > + } else if (VariableRuntimeCache->Store =3D=3D NULL) { > > + // Runtime cache is not available yet at this point, > > + // Return EFI_SUCCESS instead of EFI_NOT_AVAILABLE_YET to let it > > progress > > + return EFI_SUCCESS; > > + } > > + > > + if ( > > + mVariableModuleGlobal- > > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdate =3D=3D NULL > || > > + mVariableModuleGlobal- > > >VariableGlobal.VariableRuntimeCacheContext.ReadLock =3D=3D NULL > > + ) { > > + return EFI_UNSUPPORTED; > > + } > > + > > + if ( > > + *(mVariableModuleGlobal- > > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdate) && > > + VariableRuntimeCache->PendingUpdateLength > 0 > > + ) { > > + VariableRuntimeCache->PendingUpdateLength =3D > > + (UINT32) ( > > + MAX ( > > + (UINTN) (VariableRuntimeCache->PendingUpdateOffset + > > VariableRuntimeCache->PendingUpdateLength), > > + Offset + Length > > + ) - MIN ((UINTN) VariableRuntimeCache->PendingUpdateOffset, > Offset) > > + ); > > + VariableRuntimeCache->PendingUpdateOffset =3D > > + (UINT32) MIN ((UINTN) VariableRuntimeCache- > >PendingUpdateOffset, > > Offset); > > + } else { > > + VariableRuntimeCache->PendingUpdateLength =3D (UINT32) Length; > > + VariableRuntimeCache->PendingUpdateOffset =3D (UINT32) Offset; > > + } > > + *(mVariableModuleGlobal- > > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdate) =3D TRUE; > > + > > + if (*(mVariableModuleGlobal- > > >VariableGlobal.VariableRuntimeCacheContext.ReadLock) =3D=3D FALSE) { > > + return SynchronizeRuntimeVariableCacheEx (); > > + } > > + > > + return EFI_SUCCESS; > > +} > > diff --git > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > > index ce409f22a3..8d767f75ac 100644 > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c > > @@ -31,6 +31,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #include > > #include "Variable.h" > > #include "VariableParsing.h" > > +#include "VariableRuntimeCache.h" > > + > > +extern VARIABLE_STORE_HEADER *mNvVariableCache= ; > > > > BOOLEAN mAtRuntime = =3D FALSE; > > UINT8 *mVariableBufferP= ayload =3D NULL; > > @@ -451,25 +454,29 @@ SmmVariableGetStatistics ( > > EFI_STATUS > > EFIAPI > > SmmVariableHandler ( > > - IN EFI_HANDLE DispatchHandle, > > - IN CONST VOID *RegisterContext, > > - IN OUT VOID *CommBuffer, > > - IN OUT UINTN *CommBufferSize > > + IN EFI_HANDLE DispatchHand= le, > > + IN CONST VOID *RegisterCon= text, > > + IN OUT VOID *CommBuffer, > > + IN OUT UINTN *CommBufferS= ize > > ) > > { > > - EFI_STATUS Status; > > - SMM_VARIABLE_COMMUNICATE_HEADER > > *SmmVariableFunctionHeader; > > - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE > > *SmmVariableHeader; > > - SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME > > *GetNextVariableName; > > - SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO > > *QueryVariableInfo; > > - SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE > > *GetPayloadSize; > > - VARIABLE_INFO_ENTRY *VariableInfo; > > - SMM_VARIABLE_COMMUNICATE_LOCK_VARIABLE > *VariableToLock; > > - SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY > > *CommVariableProperty; > > - UINTN InfoSize; > > - UINTN NameBufferSize; > > - UINTN CommBufferPayloadSi= ze; > > - UINTN TempCommBufferSize; > > + EFI_STATUS Status; > > + SMM_VARIABLE_COMMUNICATE_HEADER > > *SmmVariableFunctionHeader; > > + SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE > > *SmmVariableHeader; > > + SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME > > *GetNextVariableName; > > + SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO > > *QueryVariableInfo; > > + SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE > > *GetPayloadSize; > > + > > SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT > > *RuntimeVariableCacheContext; > > + SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO > > *GetRuntimeCacheInfo; > > + SMM_VARIABLE_COMMUNICATE_LOCK_VARIABLE > > *VariableToLock; > > + SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY > > *CommVariableProperty; > > + VARIABLE_INFO_ENTRY *VariableInf= o; > > + VARIABLE_RUNTIME_CACHE_CONTEXT > > *VariableCacheContext; > > + VARIABLE_STORE_HEADER *VariableCac= he; > > + UINTN InfoSize; > > + UINTN NameBufferSi= ze; > > + UINTN CommBufferPa= yloadSize; > > + UINTN TempCommBuff= erSize; > > > > // > > // If input is invalid, stop processing this SMI > > @@ -789,6 +796,79 @@ SmmVariableHandler ( > > ); > > CopyMem (SmmVariableFunctionHeader->Data, > mVariableBufferPayload, > > CommBufferPayloadSize); > > break; > > + case > > > SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT: > > + if (CommBufferPayloadSize < sizeof > > > (SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT) > ) > > { >=20 >=20 > The above check is not correct, I think it should be: >=20 > if (CommBufferPayloadSize < sizeof > (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT) > ) { >=20 > Please help to double confirm. > Also, I recommend some security tests should be performed to these new > cases in > the variable SMI handler. >=20 >=20 You're right. The wrong macro was simply copied. > > + DEBUG ((DEBUG_ERROR, "InitRuntimeVariableCacheContext: SMM > > communication buffer size invalid!\n")); > > + } else if (mEndOfDxe) { > > + DEBUG ((DEBUG_ERROR, "InitRuntimeVariableCacheContext: Cannot > > init context after end of DXE!\n")); > > + } else { > > + RuntimeVariableCacheContext =3D > > > (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT > > *) SmmVariableFunctionHeader->Data; >=20 >=20 > Not sure on this one: >=20 > Do you think it is necessary to copy the contents in the comm buffer to t= he > pre-allocated SMM variable buffer payload 'mVariableBufferPayload' to > avoid > TOCTOU issue? Since there are some tests (sort of, a couple of ASSERTs) > based > on the comm buffer content. >=20 >=20 I understand the TOCTOU observation. But is this still a concern with all t= he cores rendezvoused in SMM prior to end of DXE? > > + VariableCacheContext =3D &mVariableModuleGlobal- > > >VariableGlobal.VariableRuntimeCacheContext; > > + > > + ASSERT (RuntimeVariableCacheContext->RuntimeVolatileCache !=3D > > NULL); > > + ASSERT (RuntimeVariableCacheContext->RuntimeNvCache !=3D NULL)= ; > > + ASSERT (RuntimeVariableCacheContext->PendingUpdate !=3D NULL); > > + ASSERT (RuntimeVariableCacheContext->ReadLock !=3D NULL); > > + ASSERT (RuntimeVariableCacheContext->HobFlushComplete !=3D > NULL); > > + > > + VariableCacheContext->VariableRuntimeHobCache.Store =3D > > RuntimeVariableCacheContext->RuntimeHobCache; > > + VariableCacheContext->VariableRuntimeVolatileCache.Store =3D > > RuntimeVariableCacheContext->RuntimeVolatileCache; > > + VariableCacheContext->VariableRuntimeNvCache.Store =3D > > RuntimeVariableCacheContext->RuntimeNvCache; > > + VariableCacheContext->PendingUpdate =3D > > RuntimeVariableCacheContext->PendingUpdate; > > + VariableCacheContext->ReadLock =3D > > RuntimeVariableCacheContext->ReadLock; > > + VariableCacheContext->HobFlushComplete =3D > > RuntimeVariableCacheContext->HobFlushComplete; > > + > > + // Set up the intial pending request since the RT cache needs = to be in > > sync with SMM cache > > + if (mVariableModuleGlobal->VariableGlobal.HobVariableBase =3D= =3D 0) { > > + VariableCacheContext- > > >VariableRuntimeHobCache.PendingUpdateOffset =3D 0; > > + VariableCacheContext- > > >VariableRuntimeHobCache.PendingUpdateLength =3D 0; > > + } else { > > + VariableCache =3D (VARIABLE_STORE_HEADER *) (UINTN) > > mVariableModuleGlobal->VariableGlobal.HobVariableBase; > > + VariableCacheContext- > > >VariableRuntimeHobCache.PendingUpdateOffset =3D 0; > > + VariableCacheContext- > > >VariableRuntimeHobCache.PendingUpdateLength =3D (UINT32) ((UINTN) > > GetEndPointer (VariableCache) - (UINTN) VariableCache); > > + CopyGuid (&(VariableCacheContext- > > >VariableRuntimeHobCache.Store->Signature), &(VariableCache- > > >Signature)); > > + } > > + VariableCache =3D (VARIABLE_STORE_HEADER *) (UINTN) > > mVariableModuleGlobal->VariableGlobal.VolatileVariableBase; > > + VariableCacheContext- > > >VariableRuntimeVolatileCache.PendingUpdateOffset =3D 0; > > + VariableCacheContext- > > >VariableRuntimeVolatileCache.PendingUpdateLength =3D (UINT32) > ((UINTN) > > GetEndPointer (VariableCache) - (UINTN) VariableCache); > > + CopyGuid (&(VariableCacheContext- > > >VariableRuntimeVolatileCache.Store->Signature), &(VariableCache- > > >Signature)); > > + > > + VariableCache =3D (VARIABLE_STORE_HEADER *) (UINTN) > > mNvVariableCache; > > + VariableCacheContext- > > >VariableRuntimeNvCache.PendingUpdateOffset =3D 0; > > + VariableCacheContext- > > >VariableRuntimeNvCache.PendingUpdateLength =3D (UINT32) ((UINTN) > > GetEndPointer (VariableCache) - (UINTN) VariableCache); > > + CopyGuid (&(VariableCacheContext- > >VariableRuntimeNvCache.Store- > > >Signature), &(VariableCache->Signature)); > > + > > + *(VariableCacheContext->PendingUpdate) =3D TRUE; > > + *(VariableCacheContext->ReadLock) =3D FALSE; > > + *(VariableCacheContext->HobFlushComplete) =3D FALSE; > > + } > > + Status =3D EFI_SUCCESS; > > + break; > > + case SMM_VARIABLE_FUNCTION_SYNC_RUNTIME_CACHE: > > + Status =3D SynchronizeRuntimeVariableCacheEx (); > > + break; > > + case SMM_VARIABLE_FUNCTION_GET_RUNTIME_CACHE_INFO: > > + if (CommBufferPayloadSize < sizeof > > (SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO)) { > > + DEBUG ((DEBUG_ERROR, "GetRuntimeCacheInfo: SMM > communication > > buffer size invalid!\n")); > > + return EFI_SUCCESS; > > + } > > + GetRuntimeCacheInfo =3D > > (SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO *) > > SmmVariableFunctionHeader->Data; > > + > > + if (mVariableModuleGlobal->VariableGlobal.HobVariableBase > 0) { > > + VariableCache =3D (VARIABLE_STORE_HEADER *) (UINTN) > > mVariableModuleGlobal->VariableGlobal.HobVariableBase; > > + GetRuntimeCacheInfo->TotalHobStorageSize =3D VariableCache->Si= ze; > > + } else { > > + GetRuntimeCacheInfo->TotalHobStorageSize =3D 0; > > + } > > + > > + VariableCache =3D (VARIABLE_STORE_HEADER *) (UINTN) > > mVariableModuleGlobal->VariableGlobal.VolatileVariableBase; > > + GetRuntimeCacheInfo->TotalVolatileStorageSize =3D VariableCache- > >Size; > > + VariableCache =3D (VARIABLE_STORE_HEADER *) (UINTN) > > mNvVariableCache; > > + GetRuntimeCacheInfo->TotalNvStorageSize =3D (UINTN) VariableCach= e- > > >Size; > > + GetRuntimeCacheInfo->AuthenticatedVariableUsage =3D > > mVariableModuleGlobal->VariableGlobal.AuthFormat; > > + > > + Status =3D EFI_SUCCESS; > > + break; > > > > default: > > Status =3D EFI_UNSUPPORTED; > > diff --git > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > > e.c > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > > e.c > > index 0a1888e5ef..46f69765a4 100644 > > --- > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > > e.c > > +++ > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > > e.c > > @@ -13,7 +13,7 @@ > > > > InitCommunicateBuffer() is really function to check the variable dat= a size. > > > > -Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
> > +Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
> > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > **/ > > @@ -32,13 +32,16 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #include > > #include > > #include > > +#include > > #include > > #include > > > > #include > > +#include > > #include > > > > #include "PrivilegePolymorphic.h" > > +#include "VariableParsing.h" > > > > EFI_HANDLE mHandle =3D NULL; > > EFI_SMM_VARIABLE_PROTOCOL *mSmmVariable =3D NULL; > > @@ -46,8 +49,19 @@ EFI_EVENT mVirtualAddressChan= geEvent > =3D > > NULL; > > EFI_SMM_COMMUNICATION_PROTOCOL *mSmmCommunication =3D > > NULL; > > UINT8 *mVariableBuffer =3D NULL; > > UINT8 *mVariableBufferPhysical =3D NULL; > > +VARIABLE_INFO_ENTRY *mVariableInfo =3D NULL; > > +VARIABLE_STORE_HEADER *mVariableRuntimeHobCacheBuffer > =3D > > NULL; > > +VARIABLE_STORE_HEADER *mVariableRuntimeNvCacheBuffer > =3D > > NULL; > > +VARIABLE_STORE_HEADER *mVariableRuntimeVolatileCacheBuffer > > =3D NULL; > > UINTN mVariableBufferSize; > > +UINTN mVariableRuntimeHobCacheBufferSize; > > +UINTN mVariableRuntimeNvCacheBufferSize; > > +UINTN mVariableRuntimeVolatileCacheBufferSi= ze; > > UINTN mVariableBufferPayloadSize; > > +BOOLEAN mVariableRuntimeCachePendingUpdate; > > +BOOLEAN mVariableRuntimeCacheReadLock; > > +BOOLEAN mVariableAuthFormat; > > +BOOLEAN mHobFlushComplete; > > EFI_LOCK mVariableServicesLock; > > EDKII_VARIABLE_LOCK_PROTOCOL mVariableLock; > > EDKII_VAR_CHECK_PROTOCOL mVarCheck; > > @@ -107,6 +121,73 @@ ReleaseLockOnlyAtBootTime ( > > } > > } > > > > +/** > > + Return TRUE if ExitBootServices () has been called. > > + > > + @retval TRUE If ExitBootServices () has been called. > > +**/ > > +BOOLEAN > > +AtRuntime ( > > + VOID > > + ) >=20 >=20 > I think we can either: > 1. Use EfiAtRuntime() for VariableSmmRuntimeDxe > 2. Move AtRuntime() to VariableParsing.c so that the function can be shar= ed > with VariableRuntimeDxe & VariableSmmRuntimeDxe. And then update > the > EfiAtRuntime() usages to AtRuntime() for VariableSmmRuntimeDxe. >=20 >=20 #1 will work fine. > > +{ > > + return EfiAtRuntime (); > > +} > > + > > +/** > > + Initialize the variable cache buffer as an empty variable store. > > + > > + @param[out] VariableCacheBuffer A pointer to pointer of a ca= che > > variable store. > > + @param[in,out] TotalVariableCacheSize On input, the minimum size > > needed for the UEFI variable store cache > > + buffer that is allocated. On= output, the actual size of > > the buffer allocated. > > + If TotalVariableCacheSize is= zero, a buffer will not be > > allocated and the > > + function will return with EF= I_SUCCESS. > > + > > + @retval EFI_SUCCESS The variable cache was allocated and > initialized > > successfully. > > + @retval EFI_INVALID_PARAMETER A given pointer is NULL or an invali= d > > variable store size was specified. > > + @retval EFI_OUT_OF_RESOURCES Insufficient resources are available > to > > allocate the variable store cache buffer. > > + > > +**/ > > +EFI_STATUS > > +EFIAPI > > +InitVariableCache ( > > + OUT VARIABLE_STORE_HEADER **VariableCacheBuffer, > > + IN OUT UINTN *TotalVariableCacheSize > > + ) > > +{ > > + VARIABLE_STORE_HEADER *VariableCacheStorePtr; > > + > > + if (TotalVariableCacheSize =3D=3D NULL) { > > + return EFI_INVALID_PARAMETER; > > + } > > + if (*TotalVariableCacheSize =3D=3D 0) { > > + return EFI_SUCCESS; > > + } > > + if (VariableCacheBuffer =3D=3D NULL || *TotalVariableCacheSize < siz= eof > > (VARIABLE_STORE_HEADER)) { > > + return EFI_INVALID_PARAMETER; > > + } > > + *TotalVariableCacheSize =3D ALIGN_VALUE (*TotalVariableCacheSize, > sizeof > > (UINT32)); > > + > > + // > > + // Allocate NV Storage Cache and initialize it to all 1's (like an e= rased FV) > > + // > > + *VariableCacheBuffer =3D (VARIABLE_STORE_HEADER *) > > AllocateRuntimePages ( > > + EFI_SIZE_TO_PAGES (*TotalVariableCacheSize= ) > > + ); > > + if (*VariableCacheBuffer =3D=3D NULL) { > > + return EFI_OUT_OF_RESOURCES; > > + } > > + VariableCacheStorePtr =3D *VariableCacheBuffer; > > + SetMem32 ((VOID *) VariableCacheStorePtr, *TotalVariableCacheSize, > > (UINT32) 0xFFFFFFFF); > > + > > + ZeroMem ((VOID *) VariableCacheStorePtr, sizeof > > (VARIABLE_STORE_HEADER)); > > + VariableCacheStorePtr->Size =3D (UINT32) *TotalVariableCacheSize; > > + VariableCacheStorePtr->Format =3D VARIABLE_STORE_FORMATTED; > > + VariableCacheStorePtr->State =3D VARIABLE_STORE_HEALTHY; > > + > > + return EFI_SUCCESS; > > +} > > + > > /** > > Initialize the communicate buffer using DataSize and Function. > > > > @@ -153,6 +234,69 @@ InitCommunicateBuffer ( > > } > > > > > > +/** > > + Gets a SMM communicate buffer from the > > EDKII_PI_SMM_COMMUNICATION_REGION_TABLE installed as an entry in > > the UEFI > > + system configuration table. A generic SMM communication buffer DXE > > driver may install the table or a custom table > > + may be installed by a platform-specific driver. > > + > > + The communicate size is: SMM_COMMUNICATE_HEADER_SIZE + > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + > > + DataSize. > > + > > + @param[in,out] CommBufferSize On input, the minimum size needed > > for the communication buffer. > > + On output, the SMM buffer size ava= ilable at > CommBuffer. > > + @param[out] CommBuffer A pointer to an SMM communication > > buffer pointer. > > + > > + @retval EFI_SUCCESS The communication buffer was found > > successfully. > > + @retval EFI_INVALID_PARAMETER A given pointer is NULL or the > > CommBufferSize is zero. > > + @retval EFI_NOT_FOUND The > > EDKII_PI_SMM_COMMUNICATION_REGION_TABLE was not found. > > + @retval EFI_OUT_OF_RESOURCES A valid SMM communicate buffer > for > > the requested size is not available. > > + > > +**/ > > +EFI_STATUS > > +GetCommunicateBuffer ( > > + IN OUT UINTN *CommBufferSize, > > + OUT UINT8 **CommBuffer > > + ) >=20 >=20 > Minor comment: >=20 > I found that the consumers of the above function are: > GetRuntimeCacheInfo() > SendRuntimeVariableCacheContextToSmm() >=20 > Both of them get called within SmmVariableReady() when the SMM variable > driver > finished initialization. I am wondering if they can simply use the pre-al= located > comm buffer (via InitCommunicateBuffer() and using 'mVariableBuffer'), > instead > of looking into the configuration table. >=20 > In my opinion, this function can be dropped. >=20 >=20 I did that initially. It was recommended to use this method. > > +{ > > + EFI_STATUS Status; > > + EDKII_PI_SMM_COMMUNICATION_REGION_TABLE > > *PiSmmCommunicationRegionTable; > > + EFI_MEMORY_DESCRIPTOR *Entry; > > + UINTN EntrySize; > > + UINT32 Index; > > + > > + if (CommBuffer =3D=3D NULL || CommBufferSize =3D=3D NULL || > > *CommBufferSize =3D=3D 0) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + Status =3D EfiGetSystemConfigurationTable ( > > + &gEdkiiPiSmmCommunicationRegionTableGuid, > > + (VOID **) &PiSmmCommunicationRegionTable > > + ); > > + if (EFI_ERROR (Status) || PiSmmCommunicationRegionTable =3D=3D NULL)= { > > + return EFI_NOT_FOUND; > > + } > > + > > + Entry =3D (EFI_MEMORY_DESCRIPTOR *) > (PiSmmCommunicationRegionTable > > + 1); > > + EntrySize =3D 0; > > + for (Index =3D 0; Index < PiSmmCommunicationRegionTable- > > >NumberOfEntries; Index++) { > > + if (Entry->Type =3D=3D EfiConventionalMemory) { > > + EntrySize =3D EFI_PAGES_TO_SIZE ((UINTN) Entry->NumberOfPages); > > + if (EntrySize >=3D *CommBufferSize) { > > + break; > > + } > > + } > > + Entry =3D (EFI_MEMORY_DESCRIPTOR *) ((UINT8 *) Entry + > > PiSmmCommunicationRegionTable->DescriptorSize); > > + } > > + > > + if (Index < PiSmmCommunicationRegionTable->NumberOfEntries) { > > + *CommBufferSize =3D EntrySize; > > + *CommBuffer =3D (UINT8 *) (UINTN) Entry->PhysicalStart; > > + return EFI_SUCCESS; > > + } > > + > > + return EFI_OUT_OF_RESOURCES; > > +} > > + > > /** > > Send the data in communicate buffer to SMM. > > > > @@ -424,6 +568,171 @@ Done: > > return Status; > > } > > > > +/** > > + Signals SMM to synchronize any pending variable updates with the > > runtime cache(s). > > + > > +**/ > > +VOID > > +EFIAPI > > +SyncRuntimeCache ( > > + VOID > > + ) > > +{ > > + // > > + // Init the communicate buffer. The buffer data size is: > > + // SMM_COMMUNICATE_HEADER_SIZE + > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE. > > + // > > + InitCommunicateBuffer (NULL, 0, > > SMM_VARIABLE_FUNCTION_SYNC_RUNTIME_CACHE); > > + > > + // > > + // Send data to SMM. > > + // > > + SendCommunicateBuffer (0); > > +} > > + > > +/** > > + Check whether a SMI must be triggered to retrieve pending cache > updates. > > + > > + If the variable HOB was finished being flushed since the last check = for a > > runtime cache update, this function > > + will prevent the HOB cache from being used for future runtime cache > hits. > > + > > +**/ > > +VOID > > +EFIAPI > > +CheckForRuntimeCacheSync ( > > + VOID > > + ) > > +{ > > + if (mVariableRuntimeCachePendingUpdate) { > > + SyncRuntimeCache (); > > + } > > + ASSERT (!mVariableRuntimeCachePendingUpdate); > > + > > + // > > + // The HOB variable data may have finished being flushed in the runt= ime > > cache sync update > > + // > > + if (mHobFlushComplete && mVariableRuntimeHobCacheBuffer !=3D NULL) > { > > + if (!AtRuntime ()) { > > + FreePool (mVariableRuntimeHobCacheBuffer); > > + } > > + mVariableRuntimeHobCacheBuffer =3D NULL; > > + } > > +} > > + > > +/** > > + This code finds variable in a volatile memory store. > > + > > + Caution: This function may receive untrusted input. > > + The data size is external input, so this function will validate it c= arefully to > > avoid buffer overflow. > > + > > + @param[in] VariableName Name of Variable to be found. > > + @param[in] VendorGuid Variable vendor GUID. > > + @param[out] Attributes Attribute value of the variable f= ound. > > + @param[in, out] DataSize Size of Data found. If size is le= ss than the > > + data, this value contains the req= uired size. > > + @param[out] Data Data pointer. > > + > > + @retval EFI_SUCCESS Found the specified variable. > > + @retval EFI_INVALID_PARAMETER Invalid parameter. > > + @retval EFI_NOT_FOUND The specified variable could not = be > found. > > + > > +**/ > > +EFI_STATUS > > +EFIAPI > > +FindVariableInRuntimeCache ( > > + IN CHAR16 *VariableName, > > + IN EFI_GUID *VendorGuid, > > + OUT UINT32 *Attributes OPTIONAL, > > + IN OUT UINTN *DataSize, > > + OUT VOID *Data OPTIONAL > > + ) > > +{ > > + EFI_STATUS Status; > > + UINTN DelayIndex; > > + UINTN TempDataSize; > > + VARIABLE_POINTER_TRACK RtPtrTrack; > > + VARIABLE_STORE_TYPE StoreType; > > + VARIABLE_STORE_HEADER *VariableStoreList[VariableStoreTypeMax]; > > + > > + Status =3D EFI_NOT_FOUND; > > + > > + if (VariableName =3D=3D NULL || VendorGuid =3D=3D NULL || DataSize = =3D=3D NULL) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + for (DelayIndex =3D 0; mVariableRuntimeCacheReadLock && DelayIndex < > > VARIABLE_RT_CACHE_READ_LOCK_TIMEOUT; DelayIndex++) { > > + MicroSecondDelay (10); > > + } > > + if (DelayIndex < VARIABLE_RT_CACHE_READ_LOCK_TIMEOUT) { > > + ASSERT (!mVariableRuntimeCacheReadLock); > > + > > + mVariableRuntimeCacheReadLock =3D TRUE; > > + CheckForRuntimeCacheSync (); > > + > > + if (!mVariableRuntimeCachePendingUpdate) { > > + // > > + // 0: Volatile, 1: HOB, 2: Non-Volatile. > > + // The index and attributes mapping must be kept in this order a= s > > FindVariable > > + // makes use of this mapping to implement search algorithm. > > + // > > + VariableStoreList[VariableStoreTypeVolatile] =3D > > mVariableRuntimeVolatileCacheBuffer; > > + VariableStoreList[VariableStoreTypeHob] =3D > > mVariableRuntimeHobCacheBuffer; > > + VariableStoreList[VariableStoreTypeNv] =3D > > mVariableRuntimeNvCacheBuffer; > > + > > + for (StoreType =3D (VARIABLE_STORE_TYPE) 0; StoreType < > > VariableStoreTypeMax; StoreType++) { > > + if (VariableStoreList[StoreType] =3D=3D NULL) { > > + continue; > > + } > > + > > + RtPtrTrack.StartPtr =3D GetStartPointer (VariableStoreList[Sto= reType]); > > + RtPtrTrack.EndPtr =3D GetEndPointer (VariableStoreList[Sto= reType]); > > + RtPtrTrack.Volatile =3D (BOOLEAN) (StoreType =3D=3D > > VariableStoreTypeVolatile); > > + > > + Status =3D FindVariableEx (VariableName, VendorGuid, FALSE, > > &RtPtrTrack); > > + if (!EFI_ERROR (Status)) { > > + break; > > + } > > + } > > + > > + if (!EFI_ERROR (Status)) { > > + // > > + // Get data size > > + // > > + TempDataSize =3D DataSizeOfVariable (RtPtrTrack.CurrPtr); > > + ASSERT (TempDataSize !=3D 0); > > + > > + if (*DataSize >=3D TempDataSize) { > > + if (Data =3D=3D NULL) { > > + Status =3D EFI_INVALID_PARAMETER; > > + goto Done; > > + } > > + > > + CopyMem (Data, GetVariableDataPtr (RtPtrTrack.CurrPtr), > > TempDataSize); > > + if (Attributes !=3D NULL) { > > + *Attributes =3D RtPtrTrack.CurrPtr->Attributes; > > + } > > + > > + *DataSize =3D TempDataSize; > > + > > + UpdateVariableInfo (VariableName, VendorGuid, > RtPtrTrack.Volatile, > > TRUE, FALSE, FALSE, TRUE, &mVariableInfo); > > + > > + Status =3D EFI_SUCCESS; > > + goto Done; > > + } else { > > + *DataSize =3D TempDataSize; > > + Status =3D EFI_BUFFER_TOO_SMALL; > > + goto Done; > > + } > > + } > > + } > > + } > > + > > +Done: > > + mVariableRuntimeCacheReadLock =3D FALSE; >=20 >=20 > If timeout occurs when acquiring the read lock, should this flag be set t= o > FALSE > in such case? >=20 Please see reply to patch #8. > Best Regards, > Hao Wu >=20 >=20 > > + > > + return Status; > > +} > > + > > /** > > This code finds variable in storage blocks (Volatile or Non-Volatile= ). > > > > @@ -454,91 +763,21 @@ RuntimeServiceGetVariable ( > > ) > > { > > EFI_STATUS Status; > > - UINTN PayloadSize; > > - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE > > *SmmVariableHeader; > > - UINTN TempDataSize; > > - UINTN VariableNameSize; > > > > if (VariableName =3D=3D NULL || VendorGuid =3D=3D NULL || DataSize = =3D=3D NULL) { > > return EFI_INVALID_PARAMETER; > > } > > - > > - TempDataSize =3D *DataSize; > > - VariableNameSize =3D StrSize (VariableName); > > - SmmVariableHeader =3D NULL; > > - > > - // > > - // If VariableName exceeds SMM payload limit. Return failure > > - // > > - if (VariableNameSize > mVariableBufferPayloadSize - OFFSET_OF > > (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) { > > - return EFI_INVALID_PARAMETER; > > - } > > - > > - AcquireLockOnlyAtBootTime(&mVariableServicesLock); > > - > > - // > > - // Init the communicate buffer. The buffer data size is: > > - // SMM_COMMUNICATE_HEADER_SIZE + > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize. > > - // > > - if (TempDataSize > mVariableBufferPayloadSize - OFFSET_OF > > (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - > > VariableNameSize) { > > - // > > - // If output data buffer exceed SMM payload limit. Trim output buf= fer to > > SMM payload size > > - // > > - TempDataSize =3D mVariableBufferPayloadSize - OFFSET_OF > > (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) - > > VariableNameSize; > > - } > > - PayloadSize =3D OFFSET_OF > > (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) + > > VariableNameSize + TempDataSize; > > - > > - Status =3D InitCommunicateBuffer ((VOID **)&SmmVariableHeader, > > PayloadSize, SMM_VARIABLE_FUNCTION_GET_VARIABLE); > > - if (EFI_ERROR (Status)) { > > - goto Done; > > - } > > - ASSERT (SmmVariableHeader !=3D NULL); > > - > > - CopyGuid (&SmmVariableHeader->Guid, VendorGuid); > > - SmmVariableHeader->DataSize =3D TempDataSize; > > - SmmVariableHeader->NameSize =3D VariableNameSize; > > - if (Attributes =3D=3D NULL) { > > - SmmVariableHeader->Attributes =3D 0; > > - } else { > > - SmmVariableHeader->Attributes =3D *Attributes; > > - } > > - CopyMem (SmmVariableHeader->Name, VariableName, > > SmmVariableHeader->NameSize); > > - > > - // > > - // Send data to SMM. > > - // > > - Status =3D SendCommunicateBuffer (PayloadSize); > > - > > - // > > - // Get data from SMM. > > - // > > - if (Status =3D=3D EFI_SUCCESS || Status =3D=3D EFI_BUFFER_TOO_SMALL)= { > > - // > > - // SMM CommBuffer DataSize can be a trimed value > > - // Only update DataSize when needed > > - // > > - *DataSize =3D SmmVariableHeader->DataSize; > > - } > > - if (Attributes !=3D NULL) { > > - *Attributes =3D SmmVariableHeader->Attributes; > > - } > > - > > - if (EFI_ERROR (Status)) { > > - goto Done; > > - } > > - > > - if (Data !=3D NULL) { > > - CopyMem (Data, (UINT8 *)SmmVariableHeader->Name + > > SmmVariableHeader->NameSize, SmmVariableHeader->DataSize); > > - } else { > > - Status =3D EFI_INVALID_PARAMETER; > > + if (VariableName[0] =3D=3D 0) { > > + return EFI_NOT_FOUND; > > } > > > > -Done: > > + AcquireLockOnlyAtBootTime (&mVariableServicesLock); > > + Status =3D FindVariableInRuntimeCache (VariableName, VendorGuid, > > Attributes, DataSize, Data); > > ReleaseLockOnlyAtBootTime (&mVariableServicesLock); > > + > > return Status; > > } > > > > - > > /** > > This code Finds the Next available variable. > > > > @@ -870,6 +1109,17 @@ OnReadyToBoot ( > > // > > SendCommunicateBuffer (0); > > > > + // > > + // Install the system configuration table for variable info data cap= tured > > + // > > + if (FeaturePcdGet (PcdVariableCollectStatistics)) { > > + if (mVariableAuthFormat) { > > + gBS->InstallConfigurationTable (&gEfiAuthenticatedVariableGuid, > > mVariableInfo); > > + } else { > > + gBS->InstallConfigurationTable (&gEfiVariableGuid, mVariableInfo= ); > > + } > > + } > > + > > gBS->CloseEvent (Event); > > } > > > > @@ -893,6 +1143,9 @@ VariableAddressChangeEvent ( > > { > > EfiConvertPointer (0x0, (VOID **) &mVariableBuffer); > > EfiConvertPointer (0x0, (VOID **) &mSmmCommunication); > > + EfiConvertPointer (0x0, (VOID **) &mVariableRuntimeHobCacheBuffer); > > + EfiConvertPointer (0x0, (VOID **) &mVariableRuntimeNvCacheBuffer); > > + EfiConvertPointer (0x0, (VOID **) > > &mVariableRuntimeVolatileCacheBuffer); > > } > > > > /** > > @@ -969,6 +1222,173 @@ Done: > > return Status; > > } > > > > +/** > > + This code gets information needed from SMM for runtime cache > > initialization. > > + > > + @param[out] TotalHobStorageSize Output pointer for the total= HOB > > storage size in bytes. > > + @param[out] TotalNvStorageSize Output pointer for the total= non- > > volatile storage size in bytes. > > + @param[out] TotalVolatileStorageSize Output pointer for the total > > volatile storage size in bytes. > > + @param[out] AuthenticatedVariableUsage Output pointer that indicate= s > if > > authenticated variables are to be used. > > + > > + @retval EFI_SUCCESS Retrieved the size successfu= lly. > > + @retval EFI_INVALID_PARAMETER TotalNvStorageSize parameter= is > > NULL. > > + @retval EFI_OUT_OF_RESOURCES Could not allocate a > CommBuffer. > > + @retval Others Could not retrieve the size = successfully.; > > + > > +**/ > > +EFI_STATUS > > +EFIAPI > > +GetRuntimeCacheInfo ( > > + OUT UINTN *TotalHobStorageSize, > > + OUT UINTN *TotalNvStorageSize, > > + OUT UINTN *TotalVolatileStorageSize, > > + OUT BOOLEAN *AuthenticatedVariableUsage > > + ) > > +{ > > + EFI_STATUS Status; > > + SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO > > *SmmGetRuntimeCacheInfo; > > + EFI_SMM_COMMUNICATE_HEADER > > *SmmCommunicateHeader; > > + SMM_VARIABLE_COMMUNICATE_HEADER > > *SmmVariableFunctionHeader; > > + UINTN CommSize; > > + UINTN CommBufferSize; > > + UINT8 *CommBuffer; > > + > > + SmmGetRuntimeCacheInfo =3D NULL; > > + CommBuffer =3D NULL; > > + > > + if (TotalHobStorageSize =3D=3D NULL || TotalNvStorageSize =3D=3D NUL= L || > > TotalVolatileStorageSize =3D=3D NULL || AuthenticatedVariableUsage =3D= =3D NULL) > { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + AcquireLockOnlyAtBootTime (&mVariableServicesLock); > > + > > + CommSize =3D SMM_COMMUNICATE_HEADER_SIZE + > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + sizeof > > (SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO); > > + CommBufferSize =3D CommSize; > > + Status =3D GetCommunicateBuffer (&CommBufferSize, &CommBuffer); > > + if (EFI_ERROR (Status)) { > > + goto Done; > > + } > > + if (CommBuffer =3D=3D NULL) { > > + Status =3D EFI_OUT_OF_RESOURCES; > > + goto Done; > > + } > > + ZeroMem (CommBuffer, CommBufferSize); > > + > > + SmmCommunicateHeader =3D (EFI_SMM_COMMUNICATE_HEADER *) > > CommBuffer; > > + CopyGuid (&SmmCommunicateHeader->HeaderGuid, > > &gEfiSmmVariableProtocolGuid); > > + SmmCommunicateHeader->MessageLength =3D > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + sizeof > > (SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO); > > + > > + SmmVariableFunctionHeader =3D > > (SMM_VARIABLE_COMMUNICATE_HEADER *) SmmCommunicateHeader- > > >Data; > > + SmmVariableFunctionHeader->Function =3D > > SMM_VARIABLE_FUNCTION_GET_RUNTIME_CACHE_INFO; > > + SmmGetRuntimeCacheInfo =3D > > (SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO *) > > SmmVariableFunctionHeader->Data; > > + > > + // > > + // Send data to SMM. > > + // > > + Status =3D mSmmCommunication->Communicate (mSmmCommunication, > > CommBuffer, &CommSize); > > + ASSERT_EFI_ERROR (Status); > > + if (CommSize <=3D SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) { > > + Status =3D EFI_BAD_BUFFER_SIZE; > > + goto Done; > > + } > > + > > + Status =3D SmmVariableFunctionHeader->ReturnStatus; > > + if (EFI_ERROR (Status)) { > > + goto Done; > > + } > > + > > + // > > + // Get data from SMM. > > + // > > + *TotalHobStorageSize =3D SmmGetRuntimeCacheInfo- > >TotalHobStorageSize; > > + *TotalNvStorageSize =3D SmmGetRuntimeCacheInfo->TotalNvStorageSize; > > + *TotalVolatileStorageSize =3D SmmGetRuntimeCacheInfo- > > >TotalVolatileStorageSize; > > + *AuthenticatedVariableUsage =3D SmmGetRuntimeCacheInfo- > > >AuthenticatedVariableUsage; > > + > > +Done: > > + ReleaseLockOnlyAtBootTime (&mVariableServicesLock); > > + return Status; > > +} > > + > > +/** > > + Sends the runtime variable cache context information to SMM. > > + > > + @retval EFI_SUCCESS Retrieved the size successfully. > > + @retval EFI_INVALID_PARAMETER TotalNvStorageSize parameter is > > NULL. > > + @retval EFI_OUT_OF_RESOURCES Could not allocate a CommBuffer. > > + @retval Others Could not retrieve the size succes= sfully.; > > + > > +**/ > > +EFI_STATUS > > +EFIAPI > > +SendRuntimeVariableCacheContextToSmm ( > > + VOID > > + ) > > +{ > > + EFI_STATUS Status; > > + > > SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT > > *SmmRuntimeVarCacheContext; > > + EFI_SMM_COMMUNICATE_HEADER > > *SmmCommunicateHeader; > > + SMM_VARIABLE_COMMUNICATE_HEADER > > *SmmVariableFunctionHeader; > > + UINTN CommSize; > > + UINTN CommBuffer= Size; > > + UINT8 *CommBuffe= r; > > + > > + SmmRuntimeVarCacheContext =3D NULL; > > + CommBuffer =3D NULL; > > + > > + AcquireLockOnlyAtBootTime (&mVariableServicesLock); > > + > > + // > > + // Init the communicate buffer. The buffer data size is: > > + // SMM_COMMUNICATE_HEADER_SIZE + > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + sizeof > > > (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT) > ; > > + // > > + CommSize =3D SMM_COMMUNICATE_HEADER_SIZE + > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + sizeof > > > (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT) > ; > > + CommBufferSize =3D CommSize; > > + Status =3D GetCommunicateBuffer (&CommBufferSize, &CommBuffer); > > + if (EFI_ERROR (Status)) { > > + goto Done; > > + } > > + if (CommBuffer =3D=3D NULL) { > > + Status =3D EFI_OUT_OF_RESOURCES; > > + goto Done; > > + } > > + ZeroMem (CommBuffer, CommBufferSize); > > + > > + SmmCommunicateHeader =3D (EFI_SMM_COMMUNICATE_HEADER *) > > CommBuffer; > > + CopyGuid (&SmmCommunicateHeader->HeaderGuid, > > &gEfiSmmVariableProtocolGuid); > > + SmmCommunicateHeader->MessageLength =3D > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + sizeof > > > (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT) > ; > > + > > + SmmVariableFunctionHeader =3D > > (SMM_VARIABLE_COMMUNICATE_HEADER *) SmmCommunicateHeader- > > >Data; > > + SmmVariableFunctionHeader->Function =3D > > > SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT; > > + SmmRuntimeVarCacheContext =3D > > > (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT > > *) SmmVariableFunctionHeader->Data; > > + > > + SmmRuntimeVarCacheContext->RuntimeHobCache =3D > > mVariableRuntimeHobCacheBuffer; > > + SmmRuntimeVarCacheContext->RuntimeVolatileCache =3D > > mVariableRuntimeVolatileCacheBuffer; > > + SmmRuntimeVarCacheContext->RuntimeNvCache =3D > > mVariableRuntimeNvCacheBuffer; > > + SmmRuntimeVarCacheContext->PendingUpdate =3D > > &mVariableRuntimeCachePendingUpdate; > > + SmmRuntimeVarCacheContext->ReadLock =3D > > &mVariableRuntimeCacheReadLock; > > + SmmRuntimeVarCacheContext->HobFlushComplete =3D > > &mHobFlushComplete; > > + > > + // > > + // Send data to SMM. > > + // > > + Status =3D mSmmCommunication->Communicate (mSmmCommunication, > > CommBuffer, &CommSize); > > + ASSERT_EFI_ERROR (Status); > > + if (CommSize <=3D SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) { > > + Status =3D EFI_BAD_BUFFER_SIZE; > > + goto Done; > > + } > > + > > + Status =3D SmmVariableFunctionHeader->ReturnStatus; > > + if (EFI_ERROR (Status)) { > > + goto Done; > > + } > > + > > +Done: > > + ReleaseLockOnlyAtBootTime (&mVariableServicesLock); > > + return Status; > > +} > > + > > /** > > Initialize variable service and install Variable Architectural proto= col. > > > > @@ -985,7 +1405,7 @@ SmmVariableReady ( > > { > > EFI_STATUS Status; > > > > - Status =3D gBS->LocateProtocol (&gEfiSmmVariableProtocolGuid, NULL, > > (VOID **)&mSmmVariable); > > + Status =3D gBS->LocateProtocol (&gEfiSmmVariableProtocolGuid, NULL, > > (VOID **) &mSmmVariable); > > if (EFI_ERROR (Status)) { > > return; > > } > > @@ -1007,6 +1427,40 @@ SmmVariableReady ( > > // > > mVariableBufferPhysical =3D mVariableBuffer; > > > > + // > > + // Allocate runtime variable cache memory buffers. > > + // > > + Status =3D GetRuntimeCacheInfo ( > > + &mVariableRuntimeHobCacheBufferSize, > > + &mVariableRuntimeNvCacheBufferSize, > > + &mVariableRuntimeVolatileCacheBufferSize, > > + &mVariableAuthFormat > > + ); > > + if (!EFI_ERROR (Status)) { > > + Status =3D InitVariableCache (&mVariableRuntimeHobCacheBuffer, > > &mVariableRuntimeHobCacheBufferSize); > > + if (!EFI_ERROR (Status)) { > > + Status =3D InitVariableCache (&mVariableRuntimeNvCacheBuffer, > > &mVariableRuntimeNvCacheBufferSize); > > + if (!EFI_ERROR (Status)) { > > + Status =3D InitVariableCache (&mVariableRuntimeVolatileCacheBu= ffer, > > &mVariableRuntimeVolatileCacheBufferSize); > > + if (!EFI_ERROR (Status)) { > > + Status =3D InitVariableParsing (mVariableAuthFormat); > > + ASSERT_EFI_ERROR (Status); > > + > > + Status =3D SendRuntimeVariableCacheContextToSmm (); > > + if (!EFI_ERROR (Status)) { > > + SyncRuntimeCache (); > > + } > > + } > > + } > > + } > > + if (EFI_ERROR (Status)) { > > + mVariableRuntimeHobCacheBuffer =3D NULL; > > + mVariableRuntimeNvCacheBuffer =3D NULL; > > + mVariableRuntimeVolatileCacheBuffer =3D NULL; > > + } > > + } > > + ASSERT_EFI_ERROR (Status); > > + > > gRT->GetVariable =3D RuntimeServiceGetVariable; > > gRT->GetNextVariableName =3D RuntimeServiceGetNextVariableName; > > gRT->SetVariable =3D RuntimeServiceSetVariable; > > -- > > 2.16.2.windows.1 >=20