[edk2-devel] [PATCH V4 00/10] UEFI Variable SMI Reduction
Liming Gao
liming.gao at intel.com
Tue Oct 15 00:49:44 UTC 2019
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 at edk2.groups.io
>Cc: Bi, Dandan <dandan.bi at intel.com>; Ard Biesheuvel
><ard.biesheuvel at linaro.org>; Dong, Eric <eric.dong at intel.com>; Laszlo Ersek
><lersek at redhat.com>; Gao, Liming <liming.gao at intel.com>; Kinney, Michael
>D <michael.d.kinney at intel.com>; Ni, Ray <ray.ni at intel.com>; Wang, Jian J
><jian.j.wang at intel.com>; Wu, Hao A <hao.a.wu at intel.com>; Yao, Jiewen
><jiewen.yao at 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.bi at intel.com>
>Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>
>Cc: Eric Dong <eric.dong at intel.com>
>Cc: Laszlo Ersek <lersek at redhat.com>
>Cc: Liming Gao <liming.gao at intel.com>
>Cc: Michael D Kinney <michael.d.kinney at intel.com>
>Cc: Ray Ni <ray.ni at intel.com>
>Cc: Jian J Wang <jian.j.wang at intel.com>
>Cc: Hao A Wu <hao.a.wu at intel.com>
>Cc: Jiewen Yao <jiewen.yao at intel.com>
>Signed-off-by: Michael Kubacki <michael.a.kubacki at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list