REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2220

V7 Changes:
 [PATCH V6 07/10] MdeModulePkg/Variable: Add RT GetVariable() cache support
 * Remove VariableRuntimeCache.c and VariableRuntimeCache.h from
   VariableSmmRuntimeDxe.inf since they are not needed to build the module.

V6 Changes:
 [PATCH V5 07/10] MdeModulePkg/Variable: Add RT GetVariable() cache support
 The most significant change is:
 * Free mVariableRuntimeHobCacheBuffer in CheckForRuntimeCacheSync () in
   VariableSmmRuntimeDxe.c with FreePages () instead of FreePool ().
 This issue was not found in earlier testing because on the initial set of
 platforms tested, the variable HOB flush was finished prior to the variable
 HOB runtime cache buffer being allocated so the FreePages () call was not
 executed.

 The remaining changes did not affect testing but are included for robustness:
 * Pass EFI_OPTIONAL_PTR for the DebugDisposition type in the EfiConvertPointer ()
   calls for mVariableRuntimeHobCacheBuffer, mVariableRuntimeNvCacheBuffer, and
   mVariableRuntimeVolatileCacheBuffer in VariableAddressChangeEvent () in
   VariableSmmRuntimeDxe.c as these buffers will not be allocated if the runtime
   cache is disabled.
 * In the SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT case in
   SmmVariableHandler () in VariableSmm.c, explicitly verify that
   VariableRuntimeHobCache.Store is not NULL in addition to checking that
   VariableGlobal.HobVariableBase is not set to zero (variable HOB is flushed)
   before writing to VariableRuntimeHobCache.Store.

V5 Changes:
 [PATCH V4 07/10] MdeModulePkg/Variable: Add RT GetVariable() cache support
 * Increased validation of the runtime buffers passed in the SMM comm buffer
   SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT structure to the
   SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT function in
   SmmVariableHandler () in VariableSmm.c.
 * Most notably, each runtime buffer given is checked to ensure its memory
   range does not overlap with SMRAM ranges via VariableSmmIsBufferOutsideSmmValid ().

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@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Michael Kubacki <michael.a.kubacki@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.inf |   18 +-
 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             |  195 ++-
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   |  655 +++++++++-
 21 files changed, 2924 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 (#49820): https://edk2.groups.io/g/devel/message/49820
Mute This Topic: https://groups.io/mt/40450233/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hello Michael,

On 11/01/19 18:34, Michael Kubacki wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2220
> 
> V7 Changes:
>  [PATCH V6 07/10] MdeModulePkg/Variable: Add RT GetVariable() cache support
>  * Remove VariableRuntimeCache.c and VariableRuntimeCache.h from
>    VariableSmmRuntimeDxe.inf since they are not needed to build the module.
> 
> V6 Changes:
>  [PATCH V5 07/10] MdeModulePkg/Variable: Add RT GetVariable() cache support
>  The most significant change is:
>  * Free mVariableRuntimeHobCacheBuffer in CheckForRuntimeCacheSync () in
>    VariableSmmRuntimeDxe.c with FreePages () instead of FreePool ().
>  This issue was not found in earlier testing because on the initial set of
>  platforms tested, the variable HOB flush was finished prior to the variable
>  HOB runtime cache buffer being allocated so the FreePages () call was not
>  executed.
> 
>  The remaining changes did not affect testing but are included for robustness:
>  * Pass EFI_OPTIONAL_PTR for the DebugDisposition type in the EfiConvertPointer ()
>    calls for mVariableRuntimeHobCacheBuffer, mVariableRuntimeNvCacheBuffer, and
>    mVariableRuntimeVolatileCacheBuffer in VariableAddressChangeEvent () in
>    VariableSmmRuntimeDxe.c as these buffers will not be allocated if the runtime
>    cache is disabled.
>  * In the SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT case in
>    SmmVariableHandler () in VariableSmm.c, explicitly verify that
>    VariableRuntimeHobCache.Store is not NULL in addition to checking that
>    VariableGlobal.HobVariableBase is not set to zero (variable HOB is flushed)
>    before writing to VariableRuntimeHobCache.Store.
> 
> V5 Changes:
>  [PATCH V4 07/10] MdeModulePkg/Variable: Add RT GetVariable() cache support
>  * Increased validation of the runtime buffers passed in the SMM comm buffer
>    SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT structure to the
>    SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT function in
>    SmmVariableHandler () in VariableSmm.c.
>  * Most notably, each runtime buffer given is checked to ensure its memory
>    range does not overlap with SMRAM ranges via VariableSmmIsBufferOutsideSmmValid ().
> 
> 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.

I gave my R-b for the OvmfPkg patch in the v4 posting:

https://edk2.groups.io/g/devel/message/48979

(alternative link:

http://mid.mail-archive.com/b89583ed-06ef-ccd2-2e29-d054f581ea6a@redhat.com
)

In the v5 posting -- assuming you had not changed that specific OvmfPkg
patch, relative to v4 -- you should have picked up my R-b, and carried
it forward ever since (to the present v7). Basically, do a git-rebase,
select the "reword" action for the patch, then cut&paste my R-b from my
v4 review email to the bottom of the commit message. Then every further
posting will contain it.

If there *have* been changes to the patch, relative to v4, then it's
indeed right to drop (or simply not pick up) my R-b. FWIW, the changelog
above does not suggest the particular patch has seen any changes since v4.

Thanks!
Laszlo

> 
> 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@intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Signed-off-by: Michael Kubacki <michael.a.kubacki@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.inf |   18 +-
>  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             |  195 ++-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   |  655 +++++++++-
>  21 files changed, 2924 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
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#49873): https://edk2.groups.io/g/devel/message/49873
Mute This Topic: https://groups.io/mt/40450233/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hi Laszlo,

I did not make any changes to the OvmfPkg patch and I forgot to carry forward the R-b.
I'll keep that in mind in the future.

For V7, I request the MdeModulePkg maintainers please add the R-b for the patches
not changed. If this is not acceptable, I will be happy to send out a patch series
with all of the R-b present.

Thanks,
Michael

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Friday, November 1, 2019 3:19 PM
> To: Kubacki, Michael A <michael.a.kubacki@intel.com>;
> devel@edk2.groups.io
> Cc: Bi, Dandan <dandan.bi@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Dong, Eric <eric.dong@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao
> A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [PATCH V7 00/10] UEFI Variable SMI Reduction
> 
> Hello Michael,
> 
> On 11/01/19 18:34, Michael Kubacki wrote:
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2220
> >
> > V7 Changes:
> >  [PATCH V6 07/10] MdeModulePkg/Variable: Add RT GetVariable() cache
> > support
> >  * Remove VariableRuntimeCache.c and VariableRuntimeCache.h from
> >    VariableSmmRuntimeDxe.inf since they are not needed to build the
> module.
> >
> > V6 Changes:
> >  [PATCH V5 07/10] MdeModulePkg/Variable: Add RT GetVariable() cache
> > support  The most significant change is:
> >  * Free mVariableRuntimeHobCacheBuffer in CheckForRuntimeCacheSync
> () in
> >    VariableSmmRuntimeDxe.c with FreePages () instead of FreePool ().
> >  This issue was not found in earlier testing because on the initial
> > set of  platforms tested, the variable HOB flush was finished prior to
> > the variable  HOB runtime cache buffer being allocated so the
> > FreePages () call was not  executed.
> >
> >  The remaining changes did not affect testing but are included for
> robustness:
> >  * Pass EFI_OPTIONAL_PTR for the DebugDisposition type in the
> EfiConvertPointer ()
> >    calls for mVariableRuntimeHobCacheBuffer,
> mVariableRuntimeNvCacheBuffer, and
> >    mVariableRuntimeVolatileCacheBuffer in VariableAddressChangeEvent ()
> in
> >    VariableSmmRuntimeDxe.c as these buffers will not be allocated if the
> runtime
> >    cache is disabled.
> >  * In the
> SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT
> case in
> >    SmmVariableHandler () in VariableSmm.c, explicitly verify that
> >    VariableRuntimeHobCache.Store is not NULL in addition to checking that
> >    VariableGlobal.HobVariableBase is not set to zero (variable HOB is
> flushed)
> >    before writing to VariableRuntimeHobCache.Store.
> >
> > V5 Changes:
> >  [PATCH V4 07/10] MdeModulePkg/Variable: Add RT GetVariable() cache
> > support
> >  * Increased validation of the runtime buffers passed in the SMM comm
> buffer
> >
> SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT
> structure to the
> >
> SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEXT
> function in
> >    SmmVariableHandler () in VariableSmm.c.
> >  * Most notably, each runtime buffer given is checked to ensure its
> memory
> >    range does not overlap with SMRAM ranges via
> VariableSmmIsBufferOutsideSmmValid ().
> >
> > 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.
> 
> I gave my R-b for the OvmfPkg patch in the v4 posting:
> 
> https://edk2.groups.io/g/devel/message/48979
> 
> (alternative link:
> 
> http://mid.mail-archive.com/b89583ed-06ef-ccd2-2e29-
> d054f581ea6a@redhat.com
> )
> 
> In the v5 posting -- assuming you had not changed that specific OvmfPkg
> patch, relative to v4 -- you should have picked up my R-b, and carried it
> forward ever since (to the present v7). Basically, do a git-rebase, select the
> "reword" action for the patch, then cut&paste my R-b from my
> v4 review email to the bottom of the commit message. Then every further
> posting will contain it.
> 
> If there *have* been changes to the patch, relative to v4, then it's indeed
> right to drop (or simply not pick up) my R-b. FWIW, the changelog above
> does not suggest the particular patch has seen any changes since v4.
> 
> Thanks!
> Laszlo
> 
> >
> > 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@intel.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Signed-off-by: Michael Kubacki <michael.a.kubacki@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 |   18 +-
> >
> 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             |
> 195 ++-
> >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
> c   |  655 +++++++++-
> >  21 files changed, 2924 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
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#49876): https://edk2.groups.io/g/devel/message/49876
Mute This Topic: https://groups.io/mt/40450233/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-