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 <dandan...@intel.com>; Ard Biesheuvel ><ard.biesheu...@linaro.org>; Dong, Eric <eric.d...@intel.com>; Laszlo Ersek ><ler...@redhat.com>; Gao, Liming <liming....@intel.com>; Kinney, Michael >D <michael.d.kin...@intel.com>; Ni, Ray <ray...@intel.com>; Wang, Jian J ><jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com>; Yao, Jiewen ><jiewen....@intel.com> >Subject: [PATCH V4 00/10] UEFI Variable SMI Reduction > >REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2220 > >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 an >issue at > patch #7 or patch #8 in the series with no change in variable caching > behavior. >The > runtime cache variable logic would be applied explicitly in V4 patch #10. > >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 place > in > each variable driver in the SMM variable solution (VariableSmmRuntimeDxe > 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 VariableNonVolatile.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 regularly >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 execution >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 below. > >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 memory > buffer of EfiRuntimeServicesData type. The conclusion was that this change > does not fundamentally alter the attack surface. The UEFI variable Runtime > Services are invoked from ring 0 and the data already travels through ring > 0 buffers (such as the SMM communicate buffer) to reach the caller. Even > 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 buffer > contents such the tampered data is returned to the BSP caller. Or an > interrupt handler on the BSP may alter the communication buffer contents > before the data is returned to the caller. In summary, this was not found to > introduce any attack not possible today. > >2. VarCheckLib impact > > VarCheckLib plays a role in SetVariable () calls. This patch series only > 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 correctness. >11. Set OsIndications to stop at FW UI to verify cache load of non-volatile > 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 storage >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 updates >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 interrupt >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 are > locked in SMM. > >Cc: Dandan Bi <dandan...@intel.com> >Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> >Cc: Eric Dong <eric.d...@intel.com> >Cc: Laszlo Ersek <ler...@redhat.com> >Cc: Liming Gao <liming....@intel.com> >Cc: Michael D Kinney <michael.d.kin...@intel.com> >Cc: Ray Ni <ray...@intel.com> >Cc: Jian J Wang <jian.j.w...@intel.com> >Cc: Hao A Wu <hao.a...@intel.com> >Cc: Jiewen Yao <jiewen....@intel.com> >Signed-off-by: Michael Kubacki <michael.a.kuba...@intel.com> > >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 | 151 > +-- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.h | >67 + > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.h | 347 >+++++ > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.h >| 51 + > MdeModulePkg/Application/VariableInfo/VariableInfo.c | 37 +- > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 1373 >++++---------------- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableExLib.c | 24 >+- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableNonVolatile.c | >334 +++++ > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c | 786 >+++++++++++ > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c >| 153 +++ > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 120 >+- > 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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48964): https://edk2.groups.io/g/devel/message/48964 Mute This Topic: https://groups.io/mt/34540086/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
