From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web12.2169.1571100589305644998 for ; Mon, 14 Oct 2019 17:49:49 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: liming.gao@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Oct 2019 17:49:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.67,297,1566889200"; d="scan'208";a="346918207" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga004.jf.intel.com with ESMTP; 14 Oct 2019 17:49:48 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 14 Oct 2019 17:49:48 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Mon, 14 Oct 2019 17:49:47 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Mon, 14 Oct 2019 17:49:47 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.166]) by shsmsx102.ccr.corp.intel.com ([169.254.2.176]) with mapi id 14.03.0439.000; Tue, 15 Oct 2019 08:49:45 +0800 From: "Liming Gao" To: "Kubacki, Michael A" , "devel@edk2.groups.io" CC: "Bi, Dandan" , Ard Biesheuvel , "Dong, Eric" , Laszlo Ersek , "Kinney, Michael D" , "Ni, Ray" , "Wang, Jian J" , "Wu, Hao A" , "Yao, Jiewen" Subject: Re: [PATCH V4 00/10] UEFI Variable SMI Reduction Thread-Topic: [PATCH V4 00/10] UEFI Variable SMI Reduction Thread-Index: AQHVgudgxIA7bJW0AkCqKtYHJx16MKda3uag Date: Tue, 15 Oct 2019 00:49:44 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E517150@SHSMSX104.ccr.corp.intel.com> References: <20191014233001.33024-1-michael.a.kubacki@intel.com> In-Reply-To: <20191014233001.33024-1-michael.a.kubacki@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: liming.gao@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Michael: I add this feature into edk2-stable2019011 tag planning. Is it ok to you? Thanks Liming >-----Original Message----- >From: Kubacki, Michael A >Sent: Tuesday, October 15, 2019 7:30 AM >To: devel@edk2.groups.io >Cc: Bi, Dandan ; Ard Biesheuvel >; Dong, Eric ; Laszlo Erse= k >; Gao, Liming ; Kinney, Michael >D ; Ni, Ray ; Wang, Jian J >; Wu, Hao A ; Yao, Jiewen > >Subject: [PATCH V4 00/10] UEFI Variable SMI Reduction > >REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2220 > >V4 Changes: > [PATCH V3 7/9] MdeModulePkg/Variable: Add RT GetVariable() cache >support > * Set gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache >to FALSE > by default in MdeModulePkg.dec. > > * Added a new patch to set >gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache > to TRUE at the end of the patch series. This allows someone to bisect a= n >issue at > patch #7 or patch #8 in the series with no change in variable caching b= ehavior. >The > runtime cache variable logic would be applied explicitly in V4 patch #1= 0. > >V3 Changes: > [PATCH V2 1/9] MdeModulePkg/Variable: Consolidate common parsing >functions > * Removed GUIDs added to VariableStandaloneMm.inf that are not required. > * Added more details to the commit message describing the criteria of > moving the chosen functions to VariableParsing.c. > > [PATCH V2 2/9] MdeModulePkg/Variable: Parameterize GetNextVariableEx() >store list > * RenamedGetNextVariableEx () to VariableServiceGetNextVariableInternal (= ) > * Updated comments in VariableServiceGetNextVariableInternal () to refer >to > "FindVariablEx ()" instead of "FindVariable ()" since FindVariableEx () > is not used in the function. > > [PATCH V2 3/9] MdeModulePkg/Variable: Parameterize >VARIABLE_INFO_ENTRY buffer > * Updated the commit message to clarify the message "structure can be >updated > outside the fixed global variable". > > [edk2-devel] [PATCH V2 4/9] MdeModulePkg/Variable: Add local auth status >in VariableParsing > * Remove the function InitVariableParsing () > * Remove the mAuthFormat global variable and instead add a BOOLEAN >parameter > to all functions that require variable authentication information to > indicate if authenticated variables are used. > * This allows the authenticated variable status to be tracked in one pl= ace in > each variable driver in the SMM variable solution (VariableSmmRuntime= Dxe > and VariableSmm). > > [edk2-devel] [PATCH V2 5/9] MdeModulePkg/Variable: Add a file for NV >variable functions > * Added the following non-volatile related functions to VariableNonVolati= le.c > from Variable.c: > * InitRealNonVolatileVariableStore () > * InitEmuNonVolatileVariableStore () > * InitNonVolatileVariableStore () > > [edk2-devel] [PATCH V2 7/9] MdeModulePkg/Variable: Add RT GetVariable() >cache support > * Added a FeaturePCD to control enabling the runtime variable cache - > gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache. > * Removed usage of the TimerLib and the wait to acquire > mVariableRuntimeCacheReadLock. Can rely on the UEFI specification > restrictions on Runtime Services callers. > * Removed the EFIAPI keyword from internal functions. > * Removed PCDs in VariableSmmRuntimeDxe.inf not required. > * Removed the HobVariableBackupBase variable no longer required. > * Renamed SynchronizeRuntimeVariableCacheEx () to better reflect usage. > * Renamed functions in VariableRuntimeCache.c to better reflect usage. > * Introduced a local variable in FlushPendingRuntimeVariableCacheUpdates = () > to reduce duplication of > mVariableModuleGlobal->VariableGlobal.VariableRuntimeCacheContext. > * Corrected the macro used in SmmVariableHandler () to > SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT. > * Remove usage of the EDKII_PI_SMM_COMMUNICATION_REGION_TABLE >to acquire a > CommBuffer from the EFI System Table and use the same runtime >CommBuffer > allocated for variable SMM communicate calls. > > [PATCH V2 8/9] MdeModulePkg/Variable: Add RT GetNextVariableName() >cache support > * Removed usage of the TimerLib and the wait to acquire > mVariableRuntimeCacheReadLock. Can rely on the UEFI specification >restrictions > on Runtime Services callers. > > * Added a new patch to disable >gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache > for all OvmfPkg package builds as requested by maintainer Laszlo Ersek. > >V2 Changes: > >Patch #1 in V1 both moved functions to VariableParsing.c and modified some >functionality in those functions. In V2, the functions are first moved and >then functionality is modified in subsequent patches. This resulted in the >following new patches in the V2 patch series: > > 1. MdeModulePkg/Variable: Parameterize GetNextVariableEx() store list > 2. MdeModulePkg/Variable: Parameterize VARIABLE_INFO_ENTRY buffer > 3. MdeModulePkg/Variable: Add local auth status in VariableParsing > 4. MdeModulePkg/Variable: Add a file for NV variable functions > >Apart from this refactor in the patches, no functionally impacting changes >were made. > >Overview >--------- >This patch series reduces SMM usage when using VariableSmmRuntimeDxe >with >VariableSmm. It does so by eliminating SMM usage for runtime service >GetVariable () and GetNextVariableName () invocations. Most UEFI variable >usage in typical systems after the variable store is initialized >(e.g. manufacturing boots) is due to GetVariable ( ) and >GetNextVariableName () not SetVariable (). GetVariable () calls can regula= rly >exceed 100 per boot while SetVariable () calls typically remain less than = 10 >per boot. By focusing on the common case, the majority of overhead >associated >with SMM can be avoided while still using existing and proven code for >operations such as variable authentication that require an isolated execut= ion >environment. > > * Advantage: Reduces overall system SMM usage > * Disadvantage: Requires more Runtime data memory usage > >The runtime cache behavior described for this patch series is enabled by >default but can be disabled with a new FeaturePCD added to >MdeModulePkg.dec: > gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache > >The reaminder of this blurb describes the changes and behavior when the >PCD >is set to TRUE. > >Initial Performance Observations >--------------------------------- > * With these proposed changes, an Intel Atom based SoC saw GetVariable ( = ) > time for an existing variable reduce from ~220us to ~5us. > >Major Changes >-------------- > 1. Two UEFI variable caches will be maintained. > a. "Runtime Cache" - Maintained in VariableSmmRuntimeDxe. Used to >serve > runtime service GetVariable () and GetNextVariableName () callers= . > b. "SMM cache" - Maintained in VariableSmm to service SMM GetVariable= () > and GetNextVariableName () callers. > i. A cache in SMRAM is retained so SMM modules do not operate on = data > outside SMRAM. > 2. A new UEFI variable read and write flow will be used as described belo= w. > >At any given time, the two caches would be coherent. On a variable write, = the >runtime cache is only updated after validation in SMM and, in the case of = a >non-volatile UEFI variable, the variable must also be successfully written= to >non-volatile storage. > >Prior RFC Feedback Addressed >----------------------------- >RFC sent Sept. 5, 2019: https://edk2.groups.io/g/devel/message/46939 > >1. UEFI variable data retrieval from a ring 0 buffer > > A common concern with this proposed set of changes is the potential >security > threat presented by serving runtime services callers from a ring 0 memo= ry > buffer of EfiRuntimeServicesData type. The conclusion was that this cha= nge > does not fundamentally alter the attack surface. The UEFI variable Runt= ime > Services are invoked from ring 0 and the data already travels through r= ing > 0 buffers (such as the SMM communicate buffer) to reach the caller. Eve= n > today if ring 0 is assumed to be malicious, the malicious code may keep= one > AP in a loop to monitor the communication data, when the BSP gets an > (authenticated) variable. When the communication buffer is updated and >the > status is set to EFI_SUCCESS, the AP may modify the communication buffe= r > contents such the tampered data is returned to the BSP caller. Or an > interrupt handler on the BSP may alter the communication buffer content= s > before the data is returned to the caller. In summary, this was not fou= nd to > introduce any attack not possible today. > >2. VarCheckLib impact > > VarCheckLib plays a role in SetVariable () calls. This patch series onl= y > changes GetVariable () behavior. Therefore, VarCheckLib is expected to > have no impact due to these changes. > >Testing Performed >------------------ >This code was tested with the master branch of edk2 on an Intel Kaby Lake = U >and Intel Whiskey Lake U reference validation platform. The set of tests >performed included: > >1. Boot from S5 to Windows 10 OS with SMM variables enabled and > the variable runtime cache enabled. >2. Boot from S5 to Ubuntu 18.04.1 LTS with SMM variable enabled > and the variable runtime cache enabled. >3. Boot from S5 to Windows 10 OS with SMM variables enabled and > the variable runtime cache disabled. >4. Boot from S5 to Ubuntu 18.04.1 LTS with SMM variable enabled > and the variable runtime cache disabled. >5. Boot from S5 to EFI shell with DXE variables enabled. > (commit 2de1f611be broke OvmfPkgIa32X64 boot regardless of > this patch series; testing without this commit was successful) >6. Dump UEFI variable store at shell with dmpstore to verify contents. >7. Dump NvStorage FV from SPI flash after boot to verify contents written= . >8. Dump UEFI variable statistics with VariableInfo at shell. >9. Boot with emulated variables enabled. >10. Cycles of adding and deleting a UEFI variable to verify cache correctn= ess. >11. Set OsIndications to stop at FW UI to verify cache load of non-volatil= e > contents. > >Why Keep SMM on Variable Writes >-------------------------------- > * SMM provides a ubiquitous isolated execution environment in x86 for > authenticated UEFI variables. > * BIOS region SPI flash write restrictions to SMM in platforms today can > be retained. > >Today's UEFI Variable Cache (for reference) >-------------------------------------------- > * Maintained in SMRAM via VariableSmm. > * A "write-through" cache of variable data in the form of a UEFI variable > store. > * Non-volatile and volatile variables are maintained in separate buffers > (variable stores). > >Runtime & SMM Cache Coherency >------------------------------ >The non-volatile cache should always accurately reflect non-volatile stora= ge >contents (done today) and the "SMM cache" and "Runtime cache" should >always be >coherent on access. The runtime cache is updated by VariableSmm. > >Updating both caches from within a SMM SetVariable () operation is fairly >straightforward but a race condition can occur if an SMI occurs during the >execution of runtime code reading from the runtime cache. To handle this >case, >a runtime cache read lock is introduced that explicitly moves pending upda= tes >from SMM to the runtime cache if an SMM update occurs while the runtime >cache >is locked. Note that it is not expected a Runtime services call will inter= rupt >SMM processing since all CPU cores rendezvous in SMM. > >New Key Elements for Coherence >------------------------------- >Runtime DXE (VariableSmmRuntimeDxe) > 1. RuntimeCacheReadLock - A global lock used to lock read access to the > runtime cache. > 2. RuntimeCachePendingUpdate - A global flag used to notify runtime code = of >a > pending cache update in SMM. > >SMM (VariableSmm) > 1. FlushRuntimeCachePendingUpdate SMI - A SW SMI handler that >synchronizes > the runtime cache buffer with the= SMM > cache buffer. > >Proposed Runtime DXE Read Flow >------------------------------- > 1. Acquire RuntimeCacheReadLock > 2. If RuntimeCachePendingUpdate flag (rare) is set then: > 2.a. Trigger FlushRuntimeCachePendingUpdate SMI > 2.b. Verify RuntimeCachePendingUpdate flag is cleared > 3. Perform read from RuntimeCache > 4. Release RuntimeCacheReadLock > >Proposed FlushRuntimeCachePendingUpdate SMI >-------------------------------------------- > 1. If RuntimeCachePendingUpdate flag is not set: > 1.a. Return > 2. Copy the data at RuntimeCachePendingOffset of >RuntimeCachePendingLength to > RuntimeCache > 3. Clear the RuntimeCachePendingUpdate flag > >Proposed SMM Write Flow >------------------------ > 1. Perform variable authentication and non-volatile write. If either fail= , > return an error to the caller. > 2. If RuntimeCacheReadLock is set then: > 2.a. Set RuntimeCachePendingUpdate flag > 2.b. Update RuntimeCachePendingOffset and >RuntimeCachePendingLength to > cover the a superset of the pending chunk (for simplicity, the > entire variable store is currently synchronized). >3. Else: > 3.a. Update RuntimeCache >4. Update SmmCache > - Note: RT read cannot occur during SMI processing since all cores ar= e > locked in SMM. > >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 > >Michael Kubacki (10): > MdeModulePkg/Variable: Consolidate common parsing functions > MdeModulePkg/Variable: Parameterize GetNextVariableInternal () stores > MdeModulePkg/Variable: Parameterize VARIABLE_INFO_ENTRY buffer > MdeModulePkg/Variable: Parameterize auth status in VariableParsing > MdeModulePkg/Variable: Add a file for NV variable functions > MdeModulePkg VariableInfo: Always consider RT DXE and SMM stats > MdeModulePkg/Variable: Add RT GetVariable() cache support > MdeModulePkg/Variable: Add RT GetNextVariableName() cache support > OvmfPkg: Disable variable runtime cache > MdeModulePkg: Enable variable runtime cache by default > > MdeModulePkg/MdeModulePkg.dec | = 12 + > OvmfPkg/OvmfPkgIa32.dsc | = 1 + > OvmfPkg/OvmfPkgIa32X64.dsc | = 1 + > OvmfPkg/OvmfPkgX64.dsc | = 1 + > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | >6 + > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | = 6 >+ > >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.i >nf | 20 +- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf >| 6 + > MdeModulePkg/Include/Guid/SmmVariableCommon.h | = 29 +- > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h | 1= 51 +-- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.h | >67 + > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h | 3= 47 >+++++ > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.h >| 51 + > MdeModulePkg/Application/VariableInfo/VariableInfo.c | = 37 +- > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 13= 73 >++++---------------- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableExLib.c | = 24 >+- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c | >334 +++++ > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c | 7= 86 >+++++++++++ > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c >| 153 +++ > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 1= 20 >+- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c >| 655 +++++++++- > 21 files changed, 2851 insertions(+), 1329 deletions(-) > create mode 100644 >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.h > create mode 100644 >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h > create mode 100644 >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.h > create mode 100644 >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c > create mode 100644 >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c > create mode 100644 >MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c > >-- >2.16.2.windows.1