I agree, I will make the default to enable the runtime cache.

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kin...@intel.com>
> Sent: Thursday, October 3, 2019 3:01 PM
> To: Kubacki, Michael A <michael.a.kuba...@intel.com>; Wu, Hao A
> <hao.a...@intel.com>; devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kin...@intel.com>
> 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>; Ni, Ray
> <ray...@intel.com>; Wang, Jian J <jian.j.w...@intel.com>; Yao, Jiewen
> <jiewen....@intel.com>
> Subject: RE: [PATCH V2 7/9] MdeModulePkg/Variable: Add RT GetVariable()
> cache support
> 
> Michael,
> 
> Perhaps the FeaturePCD for #2 should be enabled by default
> so the platform DSC only needs to set this PCD for some
> validation tests.
> 
> Mike
> 
> 
> > -----Original Message-----
> > From: Kubacki, Michael A <michael.a.kuba...@intel.com>
> > Sent: Thursday, October 3, 2019 2:54 PM
> > To: Wu, Hao A <hao.a...@intel.com>; 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>; Yao, Jiewen
> > <jiewen....@intel.com>
> > Subject: RE: [PATCH V2 7/9] MdeModulePkg/Variable: Add
> > RT GetVariable() cache support
> >
> > #1 - The plan is to remove the polling entirely in V3.
> >
> > #2 - I'd prefer to take a definitive direction and
> > reduce validation and maintenance
> >         effort but you and Laszlo both requested this so
> > I'll add a FeaturePCD to control
> >         activation of the runtime cache in this patch
> > series. Perhaps this can be removed
> >         in the future.
> >
> > #3 - Will be done in V3.
> >
> > Other replies are inline.
> >
> > > -----Original Message-----
> > > From: Wu, Hao A <hao.a...@intel.com>
> > > Sent: Thursday, October 3, 2019 1:05 AM
> > > To: Kubacki, Michael A <michael.a.kuba...@intel.com>;
> > > 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>; Yao, Jiewen
> > <jiewen....@intel.com>
> > > Subject: RE: [PATCH V2 7/9] MdeModulePkg/Variable: Add
> > RT GetVariable()
> > > cache support
> > >
> > > Before any comment on the patch, since I am not
> > experienced in the
> > > Variable
> > > driver, I would like to ask for help from other
> > reviewers to look into this
> > > patch and provide feedbacks as well. Thanks in
> > advance.
> > >
> > > With the above fact, some comments provided below
> > maybe wrong. So
> > > please help
> > > to kindly correct me.
> > >
> > >
> > > Some general comments:
> > > 1. I am not sure if bringing the TimerLib dependency
> > (delay in acquiring the
> > >    runtime cache read lock) to variable driver (a
> > software driver for the most
> > >    part) is a good idea.
> > >
> > >    Hope other reviewers can provide some feedbacks for
> > this. Thanks in
> > > advance.
> > >
> > > 2. In my opinion, I prefer a switch can be provided
> > for platform owners to
> > >    choose between using the runtime cache and going
> > through SMM for
> > > GetVariable
> > >    (and for GetNextVariableName in the next patch as
> > well).
> > >
> > >    If platform owners feel uncomfortable with using
> > the runtime cache with
> > >    regard to the security perspective, they can switch
> > to the origin solution.
> > >
> > > 3. Please help to remove the 'EFIAPI' keyword for new
> > driver internal
> > > functions;
> > >
> > >
> > > Inline comments below:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Kubacki, Michael A
> > > > Sent: Saturday, September 28, 2019 9:47 AM
> > > > To: devel@edk2.groups.io
> > > > Cc: Bi, Dandan; Ard Biesheuvel; Dong, Eric; Laszlo
> > Ersek; Gao, Liming;
> > > Kinney,
> > > > Michael D; Ni, Ray; Wang, Jian J; Wu, Hao A; Yao,
> > Jiewen
> > > > Subject: [PATCH V2 7/9] MdeModulePkg/Variable: Add
> > RT GetVariable()
> > > > cache support
> > > >
> > > >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2220
> > > >
> > > > This change reduces SMIs for GetVariable () by
> > maintaining a
> > > > UEFI variable cache in Runtime DXE in addition to
> > the pre-
> > > > existing cache in SMRAM. When the Runtime Service
> > GetVariable()
> > > > is invoked, a Runtime DXE cache is used instead of
> > triggering an
> > > > SMI to VariableSmm. This can improve overall system
> > performance
> > > > by servicing variable read requests without
> > rendezvousing all
> > > > cores into SMM.
> > > >
> > > > The following are important points regarding this
> > change.
> > > >
> > > > 1. All of the non-volatile storage contents are
> > loaded into the
> > > >    cache upon driver load. This one time load
> > operation from storage
> > > >    is preferred as opposed to building the cache on
> > demand. An on-
> > > >    demand cache would require a fallback SMI to load
> > data into the
> > > >    cache as variables are requested.
> > > >
> > > > 2. SetVariable () requests will continue to always
> > trigger an SMI.
> > > >    This occurs regardless of whether the variable is
> > volatile or
> > > >    non-volatile.
> > > >
> > > > 3. Both volatile and non-volatile variables are
> > cached in a runtime
> > > >    buffer. As is the case in the current EDK II
> > variable driver, they
> > > >    continue to be cached in separate buffers.
> > > >
> > > > 4. The cache in Runtime DXE and SMM are intended to
> > be exact copies
> > > >    of one another. All SMM variable accesses only
> > return data from the
> > > >    SMM cache. The runtime caches are only updated
> > after the variable I/O
> > > >    operation is successful in SMM. The runtime
> > caches are only updated
> > > >    from SMM.
> > > >
> > > > 5. Synchronization mechanisms are in place to ensure
> > the runtime cache
> > > >    content integrity with the SMM cache. These may
> > result in updates to
> > > >    runtime cache that are the same in content but
> > different in offset and
> > > >    size from updates to the SMM cache.
> > > >
> > > > When using SMM variables, two caches will now be
> > present.
> > > > 1. "Runtime Cache" - Maintained in
> > VariableSmmRuntimeDxe. Used to
> > > > service
> > > >    Runtime Services GetVariable () and
> > GetNextVariableName () callers.
> > > > 2. "SMM Cache" - Maintained in VariableSmm to
> > service SMM GetVariable
> > > ()
> > > >    and GetNextVariableName () callers.
> > > >    a. This cache is retained so SMM modules do not
> > operate on data outside
> > > >       SMRAM.
> > > >
> > > > It is possible to view UEFI variable read and write
> > statistics by setting
> > > > the
> > gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatist
> > ics
> > > > FeaturePcd
> > > > to TRUE and using the VariableInfo UEFI application
> > in MdeModulePkg to
> > > > dump
> > > > variable statistics to the console. By doing so, a
> > user can view the number
> > > > of GetVariable () hits from the Runtime DXE variable
> > driver (Runtime Cache
> > > > hits) and the SMM variable driver (SMM Cache hits).
> > SMM Cache hits for
> > > > GetVariable () will occur when SMM modules invoke
> > GetVariable ().
> > > >
> > > > 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>
> > > > ---
> > > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRunti
> > meDxe.inf
> > > > |   2 +
> > > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.i
> > nf           |
> > > 2
> > > > +
> > > >
> > > >
> > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRu
> > ntimeDxe.i
> > > > nf |  31 +-
> > > >
> > > >
> > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStand
> > aloneMm.inf
> > > > |   2 +
> > > >  MdeModulePkg/Include/Guid/SmmVariableCommon.h
> > |  29 +-
> > > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> > |  39
> > > +-
> > > >
> > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRunti
> > meCache.h
> > > > |  47 ++
> > > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > |  44
> > > +-
> > > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRunti
> > meCache.c
> > > > | 153 +++++
> > > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > |
> > > 114
> > > > +++-
> > > >
> > > >
> > >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRu
> > ntimeDxe.
> > > > c   | 608 +++++++++++++++++---
> > > >  11 files changed, 966 insertions(+), 105
> > deletions(-)
> > > >
> > > > diff --git
> > > >
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRun
> > timeDxe.inf
> > > >
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRun
> > timeDxe.inf
> > > > index 08a5490787..ceea5d1ff9 100644
> > > > ---
> > > >
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRun
> > timeDxe.inf
> > > > +++
> > > >
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRun
> > timeDxe.inf
> > > > @@ -40,6 +40,8 @@
> > > >    VariableNonVolatile.h
> > > >    VariableParsing.c
> > > >    VariableParsing.h
> > > > +  VariableRuntimeCache.c
> > > > +  VariableRuntimeCache.h
> > >
> > >
> > > Per my understanding, the module specified by
> > VariableRuntimeDxe.inf
> > > does not
> > > involve SMM/SMI for variable services (like
> > GetVariable). It looks weird to
> > > me
> > > for this INF to include the newly introduced runtime
> > cache codes (below
> > > source
> > > header files):
> > >
> > > VariableRuntimeCache.c
> > > VariableRuntimeCache.h
> > >
> > >
> >
> > This is because Variable.c is common to the runtime DXE
> > and SMM variable
> > driver and it contains the code to update variable
> > caches. The runtime cache
> > synchronization function
> > (SynchronizeRuntimeVariableCache ()) will return
> > if the runtime cache pointer is NULL.
> >
> > > >    PrivilegePolymorphic.h
> > > >    Measurement.c
> > > >    TcgMorLockDxe.c
> > > > diff --git
> > > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > .inf
> > > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > .inf
> > > > index 6dc2721b81..bc3033588d 100644
> > > > ---
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > .inf
> > > > +++
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > .inf
> > > > @@ -49,6 +49,8 @@
> > > >    VariableNonVolatile.h
> > > >    VariableParsing.c
> > > >    VariableParsing.h
> > > > +  VariableRuntimeCache.c
> > > > +  VariableRuntimeCache.h
> > > >    VarCheck.c
> > > >    Variable.h
> > > >    PrivilegePolymorphic.h
> > > > diff --git
> > > >
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > RuntimeDx
> > > > e.inf
> > > >
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > RuntimeDx
> > > > e.inf
> > > > index 14894e6f13..70837ac6e0 100644
> > > > ---
> > > >
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > RuntimeDx
> > > > e.inf
> > > > +++
> > > >
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > RuntimeDx
> > > > e.inf
> > > > @@ -13,7 +13,7 @@
> > > >  #  may not be modified without authorization. If
> > platform fails to protect
> > > > these resources,
> > > >  #  the authentication service provided in this
> > driver will be broken, and the
> > > > behavior is undefined.
> > > >  #
> > > > -# Copyright (c) 2010 - 2017, Intel Corporation. All
> > rights reserved.<BR>
> > > > +# Copyright (c) 2010 - 2019, Intel Corporation. All
> > rights reserved.<BR>
> > > >  # SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >  #
> > > >  ##
> > > > @@ -39,6 +39,10 @@
> > > >    VariableSmmRuntimeDxe.c
> > > >    PrivilegePolymorphic.h
> > > >    Measurement.c
> > > > +  VariableParsing.c
> > > > +  VariableParsing.h
> > > > +  VariableRuntimeCache.c
> > > > +  VariableRuntimeCache.h
> > > >
> > > >  [Packages]
> > > >    MdePkg/MdePkg.dec
> > > > @@ -49,6 +53,7 @@
> > > >    BaseLib
> > > >    UefiBootServicesTableLib
> > > >    DebugLib
> > > > +  TimerLib
> > > >    UefiRuntimeLib
> > > >    DxeServicesTableLib
> > > >    UefiDriverEntryPoint
> > > > @@ -65,7 +70,29 @@
> > > >    gEdkiiVariableLockProtocolGuid                ##
> > PRODUCES
> > > >    gEdkiiVarCheckProtocolGuid                    ##
> > PRODUCES
> > > >
> > > > +[Pcd]
> > > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize
> > ##
> > > > CONSUMES
> > > > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize
> > > ##
> > > > CONSUMES
> > > > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariab
> > leSize
> > > > ## CONSUMES
> > > > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize
> > ##
> > > > CONSUMES
> > > > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize
> > ##
> > > > CONSUMES
> > > > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMaxUserNvVariableSpace
> > Size
> > > > ## CONSUMES
> > > > +
> > > >
> > >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdBoottimeReservedNvVari
> > ableSpace
> > > > Size  ## CONSUMES
> > >
> > >
> > > Not sure if the above PCDs are really needed by
> > VariableSmmRuntimeDxe.
> > >
> > >
> >
> > I will double check and remove any not required.
> >
> > > > +
> > > > +[FeaturePcd]
> > > > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatist
> > ics
> > > ##
> > > > CONSUMES
> > > > +
> > > >  [Guids]
> > > > +  ## PRODUCES             ## GUID # Signature of
> > Variable store header
> > > > +  ## CONSUMES             ## GUID # Signature of
> > Variable store header
> > > > +  ## SOMETIMES_PRODUCES   ## SystemTable
> > > > +  gEfiAuthenticatedVariableGuid
> > > > +
> > > > +  ## PRODUCES             ## GUID # Signature of
> > Variable store header
> > > > +  ## CONSUMES             ## GUID # Signature of
> > Variable store header
> > > > +  ## SOMETIMES_PRODUCES   ## SystemTable
> > > > +  gEfiVariableGuid
> > > > +
> > > >    gEfiEventVirtualAddressChangeGuid             ##
> > CONSUMES ## Event
> > > >    gEfiEventExitBootServicesGuid                 ##
> > CONSUMES ## Event
> > > >    ## CONSUMES ## GUID # Locate protocol
> > > > @@ -82,6 +109,8 @@
> > > >    ## SOMETIMES_CONSUMES   ## Variable:L"dbt"
> > > >    gEfiImageSecurityDatabaseGuid
> > > >
> > > > +  gEdkiiPiSmmCommunicationRegionTableGuid       ##
> > > > SOMETIMES_CONSUMES ## SystemTable
> > > > +
> > > >  [Depex]
> > > >    gEfiSmmCommunicationProtocolGuid
> > > >
> > > > diff --git
> > > >
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSta
> > ndaloneMm.i
> > > > nf
> > > >
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSta
> > ndaloneMm.
> > > > inf
> > > > index ca9d23ce9f..95c5310c0b 100644
> > > > ---
> > > >
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSta
> > ndaloneMm.i
> > > > nf
> > > > +++
> > > >
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSta
> > ndaloneMm.
> > > > inf
> > > > @@ -49,6 +49,8 @@
> > > >    VariableNonVolatile.h
> > > >    VariableParsing.c
> > > >    VariableParsing.h
> > > > +  VariableRuntimeCache.c
> > > > +  VariableRuntimeCache.h
> > > >    VarCheck.c
> > > >    Variable.h
> > > >    PrivilegePolymorphic.h
> > > > diff --git
> > a/MdeModulePkg/Include/Guid/SmmVariableCommon.h
> > > > b/MdeModulePkg/Include/Guid/SmmVariableCommon.h
> > > > index c527a59891..ceef44dfd2 100644
> > > > --- a/MdeModulePkg/Include/Guid/SmmVariableCommon.h
> > > > +++ b/MdeModulePkg/Include/Guid/SmmVariableCommon.h
> > > > @@ -1,7 +1,7 @@
> > > >  /** @file
> > > >    The file defined some common structures used for
> > communicating
> > > > between SMM variable module and SMM variable wrapper
> > module.
> > > >
> > > > -Copyright (c) 2011 - 2015, Intel Corporation. All
> > rights reserved.<BR>
> > > > +Copyright (c) 2011 - 2019, Intel Corporation. All
> > rights reserved.<BR>
> > > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > >  **/
> > > > @@ -9,6 +9,7 @@ SPDX-License-Identifier: BSD-2-
> > Clause-Patent
> > > >  #ifndef _SMM_VARIABLE_COMMON_H_
> > > >  #define _SMM_VARIABLE_COMMON_H_
> > > >
> > > > +#include <Guid/VariableFormat.h>
> > > >  #include <Protocol/VarCheck.h>
> > > >
> > > >  #define EFI_SMM_VARIABLE_WRITE_GUID \
> > > > @@ -66,6 +67,16 @@ typedef struct {
> > > >  #define
> > > >
> > SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET
> > 10
> > > >
> > > >  #define SMM_VARIABLE_FUNCTION_GET_PAYLOAD_SIZE
> > 11
> > > > +//
> > > > +// The payload for this function is
> > > >
> > SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT
> > > > +//
> > > > +#define
> > > >
> > SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEX
> > T
> > > > 12
> > > > +
> > > > +#define SMM_VARIABLE_FUNCTION_SYNC_RUNTIME_CACHE
> > > 13
> > > > +//
> > > > +// The payload for this function is
> > > > SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO
> > > > +//
> > > > +#define
> > SMM_VARIABLE_FUNCTION_GET_RUNTIME_CACHE_INFO
> > > > 14
> > > >
> > > >  ///
> > > >  /// Size of SMM communicate header, without
> > including the payload.
> > > > @@ -120,4 +131,20 @@ typedef struct {
> > > >    UINTN
> > VariablePayloadSize;
> > > >  } SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE;
> > > >
> > > > +typedef struct {
> > > > +  BOOLEAN                 *ReadLock;
> > > > +  BOOLEAN                 *PendingUpdate;
> > > > +  BOOLEAN                 *HobFlushComplete;
> > > > +  VARIABLE_STORE_HEADER   *RuntimeHobCache;
> > > > +  VARIABLE_STORE_HEADER   *RuntimeNvCache;
> > > > +  VARIABLE_STORE_HEADER   *RuntimeVolatileCache;
> > > > +}
> > > >
> > >
> >
> SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT;
> > > > +
> > > > +typedef struct {
> > > > +  UINTN                   TotalHobStorageSize;
> > > > +  UINTN                   TotalNvStorageSize;
> > > > +  UINTN                   TotalVolatileStorageSize;
> > > > +  BOOLEAN
> > AuthenticatedVariableUsage;
> > > > +} SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO;
> > > > +
> > > >  #endif // _SMM_VARIABLE_COMMON_H_
> > > > diff --git
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> > > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> > > > index fb574b2e32..b9723c0250 100644
> > > > ---
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> > > > +++
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> > > > @@ -57,6 +57,12 @@ SPDX-License-Identifier: BSD-2-
> > Clause-Patent
> > > >  ///
> > > >  #define ISO_639_2_ENTRY_SIZE    3
> > > >
> > > > +///
> > > > +/// The timeout to in 10us units to wait for the
> > > > +/// variable runtime cache read lock to be
> > acquired.
> > > > +///
> > > > +#define VARIABLE_RT_CACHE_READ_LOCK_TIMEOUT  200000
> > > > +
> > > >  typedef enum {
> > > >    VariableStoreTypeVolatile,
> > > >    VariableStoreTypeHob,
> > > > @@ -64,6 +70,21 @@ typedef enum {
> > > >    VariableStoreTypeMax
> > > >  } VARIABLE_STORE_TYPE;
> > > >
> > > > +typedef struct {
> > > > +  UINT32                  PendingUpdateOffset;
> > > > +  UINT32                  PendingUpdateLength;
> > > > +  VARIABLE_STORE_HEADER   *Store;
> > > > +} VARIABLE_RUNTIME_CACHE;
> > > > +
> > > > +typedef struct {
> > > > +  BOOLEAN                 *ReadLock;
> > > > +  BOOLEAN                 *PendingUpdate;
> > > > +  BOOLEAN                 *HobFlushComplete;
> > > > +  VARIABLE_RUNTIME_CACHE  VariableRuntimeHobCache;
> > > > +  VARIABLE_RUNTIME_CACHE  VariableRuntimeNvCache;
> > > > +  VARIABLE_RUNTIME_CACHE
> > VariableRuntimeVolatileCache;
> > > > +} VARIABLE_RUNTIME_CACHE_CONTEXT;
> > > > +
> > > >  typedef struct {
> > > >    VARIABLE_HEADER *CurrPtr;
> > > >    //
> > > > @@ -79,14 +100,16 @@ typedef struct {
> > > >  } VARIABLE_POINTER_TRACK;
> > > >
> > > >  typedef struct {
> > > > -  EFI_PHYSICAL_ADDRESS  HobVariableBase;
> > > > -  EFI_PHYSICAL_ADDRESS  VolatileVariableBase;
> > > > -  EFI_PHYSICAL_ADDRESS  NonVolatileVariableBase;
> > > > -  EFI_LOCK              VariableServicesLock;
> > > > -  UINT32                ReentrantState;
> > > > -  BOOLEAN               AuthFormat;
> > > > -  BOOLEAN               AuthSupport;
> > > > -  BOOLEAN               EmuNvMode;
> > > > +  EFI_PHYSICAL_ADDRESS            HobVariableBase;
> > > > +  EFI_PHYSICAL_ADDRESS
> > HobVariableBackupBase;
> > >
> > >
> > > I do not see any usage of the new field
> > "HobVariableBackupBase".
> > > Could you help to double confirm?
> > >
> > >
> >
> > You are correct. I removed usage of this variable before
> > sending the
> > patch series and the global variable here needs to be
> > cleaned up.
> >
> > > > +  EFI_PHYSICAL_ADDRESS
> > VolatileVariableBase;
> > > > +  EFI_PHYSICAL_ADDRESS
> > NonVolatileVariableBase;
> > > > +  VARIABLE_RUNTIME_CACHE_CONTEXT
> > VariableRuntimeCacheContext;
> > > > +  EFI_LOCK
> > VariableServicesLock;
> > > > +  UINT32                          ReentrantState;
> > > > +  BOOLEAN                         AuthFormat;
> > > > +  BOOLEAN                         AuthSupport;
> > > > +  BOOLEAN                         EmuNvMode;
> > > >  } VARIABLE_GLOBAL;
> > > >
> > > >  typedef struct {
> > > > diff --git
> > > >
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRun
> > timeCache.h
> > > >
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRun
> > timeCache.
> > > > h
> > > > new file mode 100644
> > > > index 0000000000..09b83eb215
> > > > --- /dev/null
> > > > +++
> > > >
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRun
> > timeCache.
> > > > h
> > > > @@ -0,0 +1,47 @@
> > > > +/** @file
> > > > +  The common variable volatile store routines
> > shared by the
> > > DXE_RUNTIME
> > > > variable
> > > > +  module and the DXE_SMM variable module.
> > > > +
> > > > +Copyright (c) 2019, Intel Corporation. All rights
> > reserved.<BR>
> > > > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > +
> > > > +**/
> > > > +
> > > > +#ifndef _VARIABLE_RUNTIME_CACHE_H_
> > > > +#define _VARIABLE_RUNTIME_CACHE_H_
> > > > +
> > > > +#include "Variable.h"
> > > > +
> > > > +/**
> > > > +  Copies any pending updates to runtime variable
> > caches.
> > > > +
> > > > +  @retval EFI_UNSUPPORTED         The volatile
> > store to be updated is not
> > > > initialized properly.
> > > > +  @retval EFI_SUCCESS             The volatile
> > store was updated successfully.
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +SynchronizeRuntimeVariableCacheEx (
> > > > +  VOID
> > > > +  );
> > > > +
> > > > +/**
> > > > +  Synchronizes the runtime variable caches with all
> > pending updates
> > > outside
> > > > runtime.
> > > > +
> > > > +  Ensures all conditions are met to maintain
> > coherency for runtime cache
> > > > updates.
> > > > +
> > > > +  @param[in] VariableRuntimeCache Variable runtime
> > cache structure for
> > > > the runtime cache being synchronized.
> > > > +  @param[in] Offset               Offset in bytes
> > to apply the update.
> > > > +  @param[in] Length               Length of data in
> > bytes of the update.
> > > > +
> > > > +  @retval EFI_UNSUPPORTED         The volatile
> > store to be updated is not
> > > > initialized properly.
> > > > +  @retval EFI_SUCCESS             The volatile
> > store was updated successfully.
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +SynchronizeRuntimeVariableCache (
> > > > +  IN  VARIABLE_RUNTIME_CACHE
> > *VariableRuntimeCache,
> > > > +  IN  UINTN                           Offset,
> > > > +  IN  UINTN                           Length
> > > > +  );
> > > > +
> > > > +#endif
> > > > diff --git
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > > > index 5da2354aa5..bb2fa3fc19 100644
> > > > ---
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > > > +++
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > > > @@ -25,6 +25,7 @@ SPDX-License-Identifier: BSD-2-
> > Clause-Patent
> > > >  #include "Variable.h"
> > > >  #include "VariableNonVolatile.h"
> > > >  #include "VariableParsing.h"
> > > > +#include "VariableRuntimeCache.h"
> > > >
> > > >  VARIABLE_MODULE_GLOBAL  *mVariableModuleGlobal;
> > > >
> > > > @@ -332,6 +333,12 @@ RecordVarErrorFlag (
> > > >        // Update the data in NV cache.
> > > >        //
> > > >        *VarErrFlag = TempFlag;
> > > > +      Status =  SynchronizeRuntimeVariableCache (
> > > > +                  &mVariableModuleGlobal-
> > > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeNvCache,
> > > > +                  (UINTN) VarErrFlag - (UINTN)
> > mNvVariableCache + (UINTN)
> > > > mVariableModuleGlobal-
> > >VariableGlobal.NonVolatileVariableBase,
> > > > +                  sizeof (TempFlag)
> > > > +                  );
> > > > +      ASSERT_EFI_ERROR (Status);
> > > >      }
> > > >    }
> > > >  }
> > > > @@ -755,12 +762,24 @@ Reclaim (
> > > >
> > > >  Done:
> > > >    if (IsVolatile || mVariableModuleGlobal-
> > >VariableGlobal.EmuNvMode) {
> > > > +    Status =  SynchronizeRuntimeVariableCache (
> > > > +                &mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeVolatileCach
> > > > e,
> > > > +                0,
> > > > +                VariableStoreHeader->Size
> > > > +                );
> > > > +    ASSERT_EFI_ERROR (Status);
> > > >      FreePool (ValidBuffer);
> > > >    } else {
> > > >      //
> > > >      // For NV variable reclaim, we use
> > mNvVariableCache as the buffer, so
> > > > copy the data back.
> > > >      //
> > > > -    CopyMem (mNvVariableCache, (UINT8
> > *)(UINTN)VariableBase,
> > > > VariableStoreHeader->Size);
> > > > +    CopyMem (mNvVariableCache, (UINT8 *) (UINTN)
> > VariableBase,
> > > > VariableStoreHeader->Size);
> > > > +    Status =  SynchronizeRuntimeVariableCache (
> > > > +                &(mVariableModuleGlobal-
> > > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeNvCache),
> > > > +                0,
> > > > +                VariableStoreHeader->Size
> > > > +                );
> > > > +    ASSERT_EFI_ERROR (Status);
> > > >    }
> > > >
> > > >    return Status;
> > > > @@ -1592,6 +1611,7 @@ UpdateVariable (
> > > >    VARIABLE_POINTER_TRACK              *Variable;
> > > >    VARIABLE_POINTER_TRACK              NvVariable;
> > > >    VARIABLE_STORE_HEADER
> > *VariableStoreHeader;
> > > > +  VARIABLE_RUNTIME_CACHE
> > *VolatileCacheInstance;
> > > >    UINT8
> > *BufferForMerge;
> > > >    UINTN
> > MergedBufSize;
> > > >    BOOLEAN                             DataReady;
> > > > @@ -2235,6 +2255,21 @@ UpdateVariable (
> > > >    }
> > > >
> > > >  Done:
> > > > +  if (!EFI_ERROR (Status)) {
> > > > +    if (Variable->Volatile) {
> > > > +      VolatileCacheInstance =
> > &(mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeVolatileCach
> > > > e);
> > > > +    } else {
> > > > +      VolatileCacheInstance =
> > &(mVariableModuleGlobal-
> > > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeNvCache);
> > > > +    }
> > > > +
> > > > +    Status =  SynchronizeRuntimeVariableCache (
> > > > +                VolatileCacheInstance,
> > > > +                0,
> > > > +                VolatileCacheInstance->Store->Size
> > > > +                );
> > > > +    ASSERT_EFI_ERROR (Status);
> > > > +  }
> > > > +
> > > >    return Status;
> > > >  }
> > > >
> > > > @@ -3409,6 +3444,12 @@ FlushHobVariableToFlash (
> > > >          ErrorFlag = TRUE;
> > > >        }
> > > >      }
> > > > +    Status =  SynchronizeRuntimeVariableCache (
> > > > +                &mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeHobCache,
> > > > +                0,
> > > > +                mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeHobCache.S
> > > > tore->Size
> > > > +                );
> > > > +    ASSERT_EFI_ERROR (Status);
> > > >      if (ErrorFlag) {
> > > >        //
> > > >        // We still have HOB variable(s) not flushed
> > in flash.
> > > > @@ -3419,6 +3460,7 @@ FlushHobVariableToFlash (
> > > >        // All HOB variables have been flushed in
> > flash.
> > > >        //
> > > >        DEBUG ((EFI_D_INFO, "Variable driver: all HOB
> > variables have been
> > > > flushed in flash.\n"));
> > > > +      *(mVariableModuleGlobal-
> > > >
> > >VariableGlobal.VariableRuntimeCacheContext.HobFlushComp
> > lete) =
> > > TRUE;
> > > >        if (!AtRuntime ()) {
> > > >          FreePool ((VOID *) VariableStoreHeader);
> > > >        }
> > > > diff --git
> > > >
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRun
> > timeCache.c
> > > >
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRun
> > timeCache.c
> > > > new file mode 100644
> > > > index 0000000000..2642d9b000
> > > > --- /dev/null
> > > > +++
> > > >
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRun
> > timeCache.c
> > > > @@ -0,0 +1,153 @@
> > > > +/** @file
> > > > +  The common variable volatile store routines
> > shared by the
> > > DXE_RUNTIME
> > > > variable
> > > > +  module and the DXE_SMM variable module.
> > > > +
> > > > +  Caution: This module requires additional review
> > when modified.
> > > > +  This driver will have external input - variable
> > data. They may be input in
> > > > SMM mode.
> > > > +  This external input must be validated carefully
> > to avoid security issue like
> > > > +  buffer overflow, integer overflow.
> > > > +
> > > > +Copyright (c) 2019, Intel Corporation. All rights
> > reserved.<BR>
> > > > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > +
> > > > +**/
> > > > +
> > > > +#include "VariableParsing.h"
> > > > +#include "VariableRuntimeCache.h"
> > > > +
> > > > +extern VARIABLE_MODULE_GLOBAL
> > *mVariableModuleGlobal;
> > > > +extern VARIABLE_STORE_HEADER    *mNvVariableCache;
> > > > +
> > > > +/**
> > > > +  Copies any pending updates to runtime variable
> > caches.
> > > > +
> > > > +  @retval EFI_UNSUPPORTED         The volatile
> > store to be updated is not
> > > > initialized properly.
> > > > +  @retval EFI_SUCCESS             The volatile
> > store was updated successfully.
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +SynchronizeRuntimeVariableCacheEx (
> > >
> > >
> > > It is not clear to me why this function is named as
> > the "Ex" version of function
> > > SynchronizeRuntimeVariableCache(). For me, this
> > function looks more like a
> > > basic
> > > version.
> > >
> > > I would suggest a name change for the functions
> > provided in file
> > > VariableRuntimeCache.c to better reflect their usage
> > model.
> > >
> > >
> >
> > I'll rename it in V3.
> >
> > > > +  VOID
> > > > +  )
> > > > +{
> > >
> > >
> > > I would recommend that at least a local variable
> > should be introduced to
> > > reduce
> > > the duplications of:
> > >
> > > "mVariableModuleGlobal-
> > >VariableGlobal.VariableRuntimeCacheContext"
> > >
> > > in this function in order to make it easier to read.
> > >
> > >
> >
> > I'll add it in V3.
> >
> > > > +  if (
> > > > +    mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeNvCache.St
> > > > ore == NULL ||
> > > > +    mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeVolatileCach
> > > > e.Store == NULL ||
> > > > +    mVariableModuleGlobal-
> > > >
> > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdat
> > e == NULL
> > > > +    ) {
> > > > +    return EFI_UNSUPPORTED;
> > > > +  }
> > > > +
> > > > +  if (*(mVariableModuleGlobal-
> > > >
> > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdat
> > e)) {
> > > > +    if (
> > > > +      mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeHobCache.S
> > > > tore != NULL &&
> > > > +      mVariableModuleGlobal-
> > >VariableGlobal.HobVariableBase > 0
> > > > +      ) {
> > > > +      CopyMem (
> > > > +        (VOID *) (
> > > > +          ((UINT8 *) (UINTN) mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeHobCache.S
> > > > tore) +
> > > > +          mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeHobCache.P
> > > > endingUpdateOffset
> > > > +          ),
> > > > +        (VOID *) (
> > > > +          ((UINT8 *) (UINTN) mVariableModuleGlobal-
> > > > >VariableGlobal.HobVariableBase) +
> > > > +          mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeHobCache.P
> > > > endingUpdateOffset
> > > > +          ),
> > > > +        mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeHobCache.P
> > > > endingUpdateLength
> > > > +        );
> > > > +      mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeHobCache.P
> > > > endingUpdateLength = 0;
> > > > +      mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeHobCache.P
> > > > endingUpdateOffset = 0;
> > > > +    }
> > > > +
> > > > +    CopyMem (
> > > > +      (VOID *) (
> > > > +        ((UINT8 *) (UINTN) mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeNvCache.St
> > > > ore) +
> > > > +        mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeNvCache.Pe
> > > > ndingUpdateOffset
> > > > +        ),
> > > > +      (VOID *) (
> > > > +        ((UINT8 *) (UINTN) mNvVariableCache) +
> > > > +        mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeNvCache.Pe
> > > > ndingUpdateOffset
> > > > +        ),
> > > > +      mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeNvCache.Pe
> > > > ndingUpdateLength
> > > > +      );
> > > > +    mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeNvCache.Pe
> > > > ndingUpdateLength = 0;
> > > > +    mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeNvCache.Pe
> > > > ndingUpdateOffset = 0;
> > > > +
> > > > +    CopyMem (
> > > > +      (VOID *) (
> > > > +        ((UINT8 *) (UINTN) mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeVolatileCach
> > > > e.Store) +
> > > > +        mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeVolatileCach
> > > > e.PendingUpdateOffset
> > > > +      ),
> > > > +      (VOID *) (
> > > > +        ((UINT8 *) (UINTN) mVariableModuleGlobal-
> > > > >VariableGlobal.VolatileVariableBase) +
> > > > +        mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeVolatileCach
> > > > e.PendingUpdateOffset
> > > > +        ),
> > > > +      mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeVolatileCach
> > > > e.PendingUpdateLength
> > > > +      );
> > > > +    mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeVolatileCach
> > > > e.PendingUpdateLength = 0;
> > > > +    mVariableModuleGlobal-
> > > >
> > >
> > >VariableGlobal.VariableRuntimeCacheContext.VariableRunt
> > imeVolatileCach
> > > > e.PendingUpdateOffset = 0;
> > > > +    *(mVariableModuleGlobal-
> > > >
> > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdat
> > e) = FALSE;
> > > > +  }
> > > > +
> > > > +  return EFI_SUCCESS;
> > > > +}
> > > > +
> > > > +/**
> > > > +  Synchronizes the runtime variable caches with all
> > pending updates
> > > outside
> > > > runtime.
> > > > +
> > > > +  Ensures all conditions are met to maintain
> > coherency for runtime cache
> > > > updates.
> > > > +
> > > > +  @param[in] VariableRuntimeCache Variable runtime
> > cache structure for
> > > > the runtime cache being synchronized.
> > > > +  @param[in] Offset               Offset in bytes
> > to apply the update.
> > > > +  @param[in] Length               Length of data in
> > bytes of the update.
> > > > +
> > > > +  @retval EFI_UNSUPPORTED         The volatile
> > store to be updated is not
> > > > initialized properly.
> > > > +  @retval EFI_SUCCESS             The volatile
> > store was updated successfully.
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +SynchronizeRuntimeVariableCache (
> > > > +  IN  VARIABLE_RUNTIME_CACHE
> > *VariableRuntimeCache,
> > > > +  IN  UINTN                           Offset,
> > > > +  IN  UINTN                           Length
> > > > +  )
> > > > +{
> > > > +  if (VariableRuntimeCache == NULL) {
> > > > +    return EFI_INVALID_PARAMETER;
> > > > +  } else if (VariableRuntimeCache->Store == NULL) {
> > > > +      // Runtime cache is not available yet at this
> > point,
> > > > +      // Return EFI_SUCCESS instead of
> > EFI_NOT_AVAILABLE_YET to let it
> > > > progress
> > > > +      return EFI_SUCCESS;
> > > > +  }
> > > > +
> > > > +  if (
> > > > +    mVariableModuleGlobal-
> > > >
> > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdat
> > e == NULL
> > > ||
> > > > +    mVariableModuleGlobal-
> > > > >VariableGlobal.VariableRuntimeCacheContext.ReadLock
> > == NULL
> > > > +    ) {
> > > > +    return EFI_UNSUPPORTED;
> > > > +  }
> > > > +
> > > > +  if (
> > > > +    *(mVariableModuleGlobal-
> > > >
> > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdat
> > e) &&
> > > > +    VariableRuntimeCache->PendingUpdateLength > 0
> > > > +    ) {
> > > > +    VariableRuntimeCache->PendingUpdateLength =
> > > > +      (UINT32) (
> > > > +        MAX (
> > > > +          (UINTN) (VariableRuntimeCache-
> > >PendingUpdateOffset +
> > > > VariableRuntimeCache->PendingUpdateLength),
> > > > +          Offset + Length
> > > > +        ) - MIN ((UINTN) VariableRuntimeCache-
> > >PendingUpdateOffset,
> > > Offset)
> > > > +      );
> > > > +    VariableRuntimeCache->PendingUpdateOffset =
> > > > +      (UINT32) MIN ((UINTN) VariableRuntimeCache-
> > > >PendingUpdateOffset,
> > > > Offset);
> > > > +  } else {
> > > > +    VariableRuntimeCache->PendingUpdateLength =
> > (UINT32) Length;
> > > > +    VariableRuntimeCache->PendingUpdateOffset =
> > (UINT32) Offset;
> > > > +  }
> > > > +  *(mVariableModuleGlobal-
> > > >
> > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdat
> > e) = TRUE;
> > > > +
> > > > +  if (*(mVariableModuleGlobal-
> > > >
> > >VariableGlobal.VariableRuntimeCacheContext.ReadLock) ==
> > FALSE) {
> > > > +    return SynchronizeRuntimeVariableCacheEx ();
> > > > +  }
> > > > +
> > > > +  return EFI_SUCCESS;
> > > > +}
> > > > diff --git
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > .c
> > > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > .c
> > > > index ce409f22a3..8d767f75ac 100644
> > > > ---
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > .c
> > > > +++
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > .c
> > > > @@ -31,6 +31,9 @@ SPDX-License-Identifier: BSD-2-
> > Clause-Patent
> > > >  #include <Guid/SmmVariableCommon.h>
> > > >  #include "Variable.h"
> > > >  #include "VariableParsing.h"
> > > > +#include "VariableRuntimeCache.h"
> > > > +
> > > > +extern VARIABLE_STORE_HEADER
> > *mNvVariableCache;
> > > >
> > > >  BOOLEAN
> > mAtRuntime              = FALSE;
> > > >  UINT8
> > *mVariableBufferPayload = NULL;
> > > > @@ -451,25 +454,29 @@ SmmVariableGetStatistics (
> > > >  EFI_STATUS
> > > >  EFIAPI
> > > >  SmmVariableHandler (
> > > > -  IN     EFI_HANDLE
> > DispatchHandle,
> > > > -  IN     CONST VOID
> > *RegisterContext,
> > > > -  IN OUT VOID
> > *CommBuffer,
> > > > -  IN OUT UINTN
> > *CommBufferSize
> > > > +  IN     EFI_HANDLE
> > DispatchHandle,
> > > > +  IN     CONST VOID
> > *RegisterContext,
> > > > +  IN OUT VOID
> > *CommBuffer,
> > > > +  IN OUT UINTN
> > *CommBufferSize
> > > >    )
> > > >  {
> > > > -  EFI_STATUS
> > Status;
> > > > -  SMM_VARIABLE_COMMUNICATE_HEADER
> > > > *SmmVariableFunctionHeader;
> > > > -  SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE
> > > > *SmmVariableHeader;
> > > > -  SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME
> > > > *GetNextVariableName;
> > > > -  SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO
> > > > *QueryVariableInfo;
> > > > -  SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE
> > > > *GetPayloadSize;
> > > > -  VARIABLE_INFO_ENTRY
> > *VariableInfo;
> > > > -  SMM_VARIABLE_COMMUNICATE_LOCK_VARIABLE
> > > *VariableToLock;
> > > > -
> > SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY
> > > > *CommVariableProperty;
> > > > -  UINTN
> > InfoSize;
> > > > -  UINTN
> > NameBufferSize;
> > > > -  UINTN
> > CommBufferPayloadSize;
> > > > -  UINTN
> > TempCommBufferSize;
> > > > +  EFI_STATUS
> > Status;
> > > > +  SMM_VARIABLE_COMMUNICATE_HEADER
> > > > *SmmVariableFunctionHeader;
> > > > +  SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE
> > > > *SmmVariableHeader;
> > > > +  SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME
> > > > *GetNextVariableName;
> > > > +  SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO
> > > > *QueryVariableInfo;
> > > > +  SMM_VARIABLE_COMMUNICATE_GET_PAYLOAD_SIZE
> > > > *GetPayloadSize;
> > > > +
> > > >
> > SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT
> > > > *RuntimeVariableCacheContext;
> > > > +  SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO
> > > > *GetRuntimeCacheInfo;
> > > > +  SMM_VARIABLE_COMMUNICATE_LOCK_VARIABLE
> > > > *VariableToLock;
> > > > +
> > SMM_VARIABLE_COMMUNICATE_VAR_CHECK_VARIABLE_PROPERTY
> > > > *CommVariableProperty;
> > > > +  VARIABLE_INFO_ENTRY
> > *VariableInfo;
> > > > +  VARIABLE_RUNTIME_CACHE_CONTEXT
> > > > *VariableCacheContext;
> > > > +  VARIABLE_STORE_HEADER
> > *VariableCache;
> > > > +  UINTN
> > InfoSize;
> > > > +  UINTN
> > NameBufferSize;
> > > > +  UINTN
> > CommBufferPayloadSize;
> > > > +  UINTN
> > TempCommBufferSize;
> > > >
> > > >    //
> > > >    // If input is invalid, stop processing this SMI
> > > > @@ -789,6 +796,79 @@ SmmVariableHandler (
> > > >                   );
> > > >        CopyMem (SmmVariableFunctionHeader->Data,
> > > mVariableBufferPayload,
> > > > CommBufferPayloadSize);
> > > >        break;
> > > > +    case
> > > >
> > >
> > SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEX
> > T:
> > > > +      if (CommBufferPayloadSize < sizeof
> > > >
> > >
> > (SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTE
> > XT)
> > > )
> > > > {
> > >
> > >
> > > The above check is not correct, I think it should be:
> > >
> > > if (CommBufferPayloadSize < sizeof
> > >
> >
> (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT
> > )
> > > ) {
> > >
> > > Please help to double confirm.
> > > Also, I recommend some security tests should be
> > performed to these new
> > > cases in
> > > the variable SMI handler.
> > >
> > >
> >
> > You're right. The wrong macro was simply copied.
> >
> > > > +        DEBUG ((DEBUG_ERROR,
> > "InitRuntimeVariableCacheContext: SMM
> > > > communication buffer size invalid!\n"));
> > > > +      } else if (mEndOfDxe) {
> > > > +        DEBUG ((DEBUG_ERROR,
> > "InitRuntimeVariableCacheContext: Cannot
> > > > init context after end of DXE!\n"));
> > > > +      } else {
> > > > +        RuntimeVariableCacheContext =
> > > >
> > >
> >
> (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT
> > > > *) SmmVariableFunctionHeader->Data;
> > >
> > >
> > > Not sure on this one:
> > >
> > > Do you think it is necessary to copy the contents in
> > the comm buffer to the
> > > pre-allocated SMM variable buffer payload
> > 'mVariableBufferPayload' to
> > > avoid
> > > TOCTOU issue? Since there are some tests (sort of, a
> > couple of ASSERTs)
> > > based
> > > on the comm buffer content.
> > >
> > >
> >
> > I understand the TOCTOU observation. But is this still a
> > concern with all the
> > cores rendezvoused in SMM prior to end of DXE?
> >
> > > > +        VariableCacheContext =
> > &mVariableModuleGlobal-
> > > > >VariableGlobal.VariableRuntimeCacheContext;
> > > > +
> > > > +        ASSERT (RuntimeVariableCacheContext-
> > >RuntimeVolatileCache !=
> > > > NULL);
> > > > +        ASSERT (RuntimeVariableCacheContext-
> > >RuntimeNvCache != NULL);
> > > > +        ASSERT (RuntimeVariableCacheContext-
> > >PendingUpdate != NULL);
> > > > +        ASSERT (RuntimeVariableCacheContext-
> > >ReadLock != NULL);
> > > > +        ASSERT (RuntimeVariableCacheContext-
> > >HobFlushComplete !=
> > > NULL);
> > > > +
> > > > +        VariableCacheContext-
> > >VariableRuntimeHobCache.Store      =
> > > > RuntimeVariableCacheContext->RuntimeHobCache;
> > > > +        VariableCacheContext-
> > >VariableRuntimeVolatileCache.Store =
> > > > RuntimeVariableCacheContext->RuntimeVolatileCache;
> > > > +        VariableCacheContext-
> > >VariableRuntimeNvCache.Store       =
> > > > RuntimeVariableCacheContext->RuntimeNvCache;
> > > > +        VariableCacheContext->PendingUpdate
> > =
> > > > RuntimeVariableCacheContext->PendingUpdate;
> > > > +        VariableCacheContext->ReadLock
> > =
> > > > RuntimeVariableCacheContext->ReadLock;
> > > > +        VariableCacheContext->HobFlushComplete
> > =
> > > > RuntimeVariableCacheContext->HobFlushComplete;
> > > > +
> > > > +        // Set up the intial pending request since
> > the RT cache needs to be in
> > > > sync with SMM cache
> > > > +        if (mVariableModuleGlobal-
> > >VariableGlobal.HobVariableBase == 0) {
> > > > +          VariableCacheContext-
> > > > >VariableRuntimeHobCache.PendingUpdateOffset = 0;
> > > > +          VariableCacheContext-
> > > > >VariableRuntimeHobCache.PendingUpdateLength = 0;
> > > > +        } else {
> > > > +          VariableCache = (VARIABLE_STORE_HEADER *)
> > (UINTN)
> > > > mVariableModuleGlobal-
> > >VariableGlobal.HobVariableBase;
> > > > +          VariableCacheContext-
> > > > >VariableRuntimeHobCache.PendingUpdateOffset = 0;
> > > > +          VariableCacheContext-
> > > > >VariableRuntimeHobCache.PendingUpdateLength =
> > (UINT32) ((UINTN)
> > > > GetEndPointer (VariableCache) - (UINTN)
> > VariableCache);
> > > > +          CopyGuid (&(VariableCacheContext-
> > > > >VariableRuntimeHobCache.Store->Signature),
> > &(VariableCache-
> > > > >Signature));
> > > > +        }
> > > > +        VariableCache = (VARIABLE_STORE_HEADER  *)
> > (UINTN)
> > > > mVariableModuleGlobal-
> > >VariableGlobal.VolatileVariableBase;
> > > > +        VariableCacheContext-
> > > > >VariableRuntimeVolatileCache.PendingUpdateOffset
> > = 0;
> > > > +        VariableCacheContext-
> > > > >VariableRuntimeVolatileCache.PendingUpdateLength
> > = (UINT32)
> > > ((UINTN)
> > > > GetEndPointer (VariableCache) - (UINTN)
> > VariableCache);
> > > > +        CopyGuid (&(VariableCacheContext-
> > > > >VariableRuntimeVolatileCache.Store->Signature),
> > &(VariableCache-
> > > > >Signature));
> > > > +
> > > > +        VariableCache = (VARIABLE_STORE_HEADER  *)
> > (UINTN)
> > > > mNvVariableCache;
> > > > +        VariableCacheContext-
> > > > >VariableRuntimeNvCache.PendingUpdateOffset = 0;
> > > > +        VariableCacheContext-
> > > > >VariableRuntimeNvCache.PendingUpdateLength =
> > (UINT32) ((UINTN)
> > > > GetEndPointer (VariableCache) - (UINTN)
> > VariableCache);
> > > > +        CopyGuid (&(VariableCacheContext-
> > > >VariableRuntimeNvCache.Store-
> > > > >Signature), &(VariableCache->Signature));
> > > > +
> > > > +        *(VariableCacheContext->PendingUpdate) =
> > TRUE;
> > > > +        *(VariableCacheContext->ReadLock) = FALSE;
> > > > +        *(VariableCacheContext->HobFlushComplete) =
> > FALSE;
> > > > +      }
> > > > +      Status = EFI_SUCCESS;
> > > > +      break;
> > > > +    case SMM_VARIABLE_FUNCTION_SYNC_RUNTIME_CACHE:
> > > > +      Status = SynchronizeRuntimeVariableCacheEx
> > ();
> > > > +      break;
> > > > +    case
> > SMM_VARIABLE_FUNCTION_GET_RUNTIME_CACHE_INFO:
> > > > +      if (CommBufferPayloadSize < sizeof
> > > > (SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO)) {
> > > > +        DEBUG ((DEBUG_ERROR, "GetRuntimeCacheInfo:
> > SMM
> > > communication
> > > > buffer size invalid!\n"));
> > > > +        return EFI_SUCCESS;
> > > > +      }
> > > > +      GetRuntimeCacheInfo =
> > > > (SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO *)
> > > > SmmVariableFunctionHeader->Data;
> > > > +
> > > > +      if (mVariableModuleGlobal-
> > >VariableGlobal.HobVariableBase > 0) {
> > > > +        VariableCache = (VARIABLE_STORE_HEADER *)
> > (UINTN)
> > > > mVariableModuleGlobal-
> > >VariableGlobal.HobVariableBase;
> > > > +        GetRuntimeCacheInfo->TotalHobStorageSize =
> > VariableCache->Size;
> > > > +      } else {
> > > > +        GetRuntimeCacheInfo->TotalHobStorageSize =
> > 0;
> > > > +      }
> > > > +
> > > > +      VariableCache = (VARIABLE_STORE_HEADER  *)
> > (UINTN)
> > > > mVariableModuleGlobal-
> > >VariableGlobal.VolatileVariableBase;
> > > > +      GetRuntimeCacheInfo->TotalVolatileStorageSize
> > = VariableCache-
> > > >Size;
> > > > +      VariableCache = (VARIABLE_STORE_HEADER  *)
> > (UINTN)
> > > > mNvVariableCache;
> > > > +      GetRuntimeCacheInfo->TotalNvStorageSize =
> > (UINTN) VariableCache-
> > > > >Size;
> > > > +      GetRuntimeCacheInfo-
> > >AuthenticatedVariableUsage =
> > > > mVariableModuleGlobal->VariableGlobal.AuthFormat;
> > > > +
> > > > +      Status = EFI_SUCCESS;
> > > > +      break;
> > > >
> > > >      default:
> > > >        Status = EFI_UNSUPPORTED;
> > > > diff --git
> > > >
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > RuntimeDx
> > > > e.c
> > > >
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > RuntimeDx
> > > > e.c
> > > > index 0a1888e5ef..46f69765a4 100644
> > > > ---
> > > >
> > >
> > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > RuntimeDx
> > > > e.c
> > > > +++
> > > >
> > >
> > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm
> > RuntimeDx
> > > > e.c
> > > > @@ -13,7 +13,7 @@
> > > >
> > > >    InitCommunicateBuffer() is really function to
> > check the variable data size.
> > > >
> > > > -Copyright (c) 2010 - 2017, Intel Corporation. All
> > rights reserved.<BR>
> > > > +Copyright (c) 2010 - 2019, Intel Corporation. All
> > rights reserved.<BR>
> > > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > >  **/
> > > > @@ -32,13 +32,16 @@ SPDX-License-Identifier: BSD-2-
> > Clause-Patent
> > > >  #include <Library/UefiRuntimeLib.h>
> > > >  #include <Library/BaseMemoryLib.h>
> > > >  #include <Library/DebugLib.h>
> > > > +#include <Library/TimerLib.h>
> > > >  #include <Library/UefiLib.h>
> > > >  #include <Library/BaseLib.h>
> > > >
> > > >  #include <Guid/EventGroup.h>
> > > > +#include <Guid/PiSmmCommunicationRegionTable.h>
> > > >  #include <Guid/SmmVariableCommon.h>
> > > >
> > > >  #include "PrivilegePolymorphic.h"
> > > > +#include "VariableParsing.h"
> > > >
> > > >  EFI_HANDLE                       mHandle
> > = NULL;
> > > >  EFI_SMM_VARIABLE_PROTOCOL       *mSmmVariable
> > = NULL;
> > > > @@ -46,8 +49,19 @@ EFI_EVENT
> > mVirtualAddressChangeEvent
> > > =
> > > > NULL;
> > > >  EFI_SMM_COMMUNICATION_PROTOCOL  *mSmmCommunication
> > =
> > > > NULL;
> > > >  UINT8                           *mVariableBuffer
> > = NULL;
> > > >  UINT8
> > *mVariableBufferPhysical    = NULL;
> > > > +VARIABLE_INFO_ENTRY             *mVariableInfo
> > = NULL;
> > > > +VARIABLE_STORE_HEADER
> > *mVariableRuntimeHobCacheBuffer
> > > =
> > > > NULL;
> > > > +VARIABLE_STORE_HEADER
> > *mVariableRuntimeNvCacheBuffer
> > > =
> > > > NULL;
> > > > +VARIABLE_STORE_HEADER
> > *mVariableRuntimeVolatileCacheBuffer
> > > > = NULL;
> > > >  UINTN
> > mVariableBufferSize;
> > > > +UINTN
> > mVariableRuntimeHobCacheBufferSize;
> > > > +UINTN
> > mVariableRuntimeNvCacheBufferSize;
> > > > +UINTN
> > mVariableRuntimeVolatileCacheBufferSize;
> > > >  UINTN
> > mVariableBufferPayloadSize;
> > > > +BOOLEAN
> > mVariableRuntimeCachePendingUpdate;
> > > > +BOOLEAN
> > mVariableRuntimeCacheReadLock;
> > > > +BOOLEAN
> > mVariableAuthFormat;
> > > > +BOOLEAN                          mHobFlushComplete;
> > > >  EFI_LOCK
> > mVariableServicesLock;
> > > >  EDKII_VARIABLE_LOCK_PROTOCOL     mVariableLock;
> > > >  EDKII_VAR_CHECK_PROTOCOL         mVarCheck;
> > > > @@ -107,6 +121,73 @@ ReleaseLockOnlyAtBootTime (
> > > >    }
> > > >  }
> > > >
> > > > +/**
> > > > +  Return TRUE if ExitBootServices () has been
> > called.
> > > > +
> > > > +  @retval TRUE If ExitBootServices () has been
> > called.
> > > > +**/
> > > > +BOOLEAN
> > > > +AtRuntime (
> > > > +  VOID
> > > > +  )
> > >
> > >
> > > I think we can either:
> > > 1. Use EfiAtRuntime() for VariableSmmRuntimeDxe
> > > 2. Move AtRuntime() to VariableParsing.c so that the
> > function can be shared
> > >    with VariableRuntimeDxe & VariableSmmRuntimeDxe.
> > And then update
> > > the
> > >    EfiAtRuntime() usages to AtRuntime() for
> > VariableSmmRuntimeDxe.
> > >
> > >
> >
> > #1 will work fine.
> >
> > > > +{
> > > > +  return EfiAtRuntime ();
> > > > +}
> > > > +
> > > > +/**
> > > > +  Initialize the variable cache buffer as an empty
> > variable store.
> > > > +
> > > > +  @param[out]     VariableCacheBuffer     A pointer
> > to pointer of a cache
> > > > variable store.
> > > > +  @param[in,out]  TotalVariableCacheSize  On input,
> > the minimum size
> > > > needed for the UEFI variable store cache
> > > > +                                          buffer
> > that is allocated. On output, the actual size of
> > > > the buffer allocated.
> > > > +                                          If
> > TotalVariableCacheSize is zero, a buffer will not be
> > > > allocated and the
> > > > +                                          function
> > will return with EFI_SUCCESS.
> > > > +
> > > > +  @retval EFI_SUCCESS             The variable
> > cache was allocated and
> > > initialized
> > > > successfully.
> > > > +  @retval EFI_INVALID_PARAMETER   A given pointer
> > is NULL or an invalid
> > > > variable store size was specified.
> > > > +  @retval EFI_OUT_OF_RESOURCES    Insufficient
> > resources are available
> > > to
> > > > allocate the variable store cache buffer.
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +EFIAPI
> > > > +InitVariableCache (
> > > > +  OUT    VARIABLE_STORE_HEADER
> > **VariableCacheBuffer,
> > > > +  IN OUT UINTN
> > *TotalVariableCacheSize
> > > > +  )
> > > > +{
> > > > +  VARIABLE_STORE_HEADER   *VariableCacheStorePtr;
> > > > +
> > > > +  if (TotalVariableCacheSize == NULL) {
> > > > +    return EFI_INVALID_PARAMETER;
> > > > +  }
> > > > +  if (*TotalVariableCacheSize == 0) {
> > > > +    return EFI_SUCCESS;
> > > > +  }
> > > > +  if (VariableCacheBuffer == NULL ||
> > *TotalVariableCacheSize < sizeof
> > > > (VARIABLE_STORE_HEADER)) {
> > > > +    return EFI_INVALID_PARAMETER;
> > > > +  }
> > > > +  *TotalVariableCacheSize = ALIGN_VALUE
> > (*TotalVariableCacheSize,
> > > sizeof
> > > > (UINT32));
> > > > +
> > > > +  //
> > > > +  // Allocate NV Storage Cache and initialize it to
> > all 1's (like an erased FV)
> > > > +  //
> > > > +  *VariableCacheBuffer =  (VARIABLE_STORE_HEADER *)
> > > > AllocateRuntimePages (
> > > > +                            EFI_SIZE_TO_PAGES
> > (*TotalVariableCacheSize)
> > > > +                            );
> > > > +  if (*VariableCacheBuffer == NULL) {
> > > > +    return EFI_OUT_OF_RESOURCES;
> > > > +  }
> > > > +  VariableCacheStorePtr = *VariableCacheBuffer;
> > > > +  SetMem32 ((VOID *) VariableCacheStorePtr,
> > *TotalVariableCacheSize,
> > > > (UINT32) 0xFFFFFFFF);
> > > > +
> > > > +  ZeroMem ((VOID *) VariableCacheStorePtr, sizeof
> > > > (VARIABLE_STORE_HEADER));
> > > > +  VariableCacheStorePtr->Size    = (UINT32)
> > *TotalVariableCacheSize;
> > > > +  VariableCacheStorePtr->Format  =
> > VARIABLE_STORE_FORMATTED;
> > > > +  VariableCacheStorePtr->State   =
> > VARIABLE_STORE_HEALTHY;
> > > > +
> > > > +  return EFI_SUCCESS;
> > > > +}
> > > > +
> > > >  /**
> > > >    Initialize the communicate buffer using DataSize
> > and Function.
> > > >
> > > > @@ -153,6 +234,69 @@ InitCommunicateBuffer (
> > > >  }
> > > >
> > > >
> > > > +/**
> > > > +  Gets a SMM communicate buffer from the
> > > > EDKII_PI_SMM_COMMUNICATION_REGION_TABLE installed as
> > an entry in
> > > > the UEFI
> > > > +  system configuration table. A generic SMM
> > communication buffer DXE
> > > > driver may install the table or a custom table
> > > > +  may be installed by a platform-specific driver.
> > > > +
> > > > +  The communicate size is:
> > SMM_COMMUNICATE_HEADER_SIZE +
> > > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE +
> > > > +                            DataSize.
> > > > +
> > > > +  @param[in,out]   CommBufferSize   On input, the
> > minimum size needed
> > > > for the communication buffer.
> > > > +                                    On output, the
> > SMM buffer size available at
> > > CommBuffer.
> > > > +  @param[out]      CommBuffer       A pointer to an
> > SMM communication
> > > > buffer pointer.
> > > > +
> > > > +  @retval EFI_SUCCESS               The
> > communication buffer was found
> > > > successfully.
> > > > +  @retval EFI_INVALID_PARAMETER     A given pointer
> > is NULL or the
> > > > CommBufferSize is zero.
> > > > +  @retval EFI_NOT_FOUND             The
> > > > EDKII_PI_SMM_COMMUNICATION_REGION_TABLE was not
> > found.
> > > > +  @retval EFI_OUT_OF_RESOURCES      A valid SMM
> > communicate buffer
> > > for
> > > > the requested size is not available.
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +GetCommunicateBuffer (
> > > > +  IN OUT  UINTN     *CommBufferSize,
> > > > +  OUT     UINT8     **CommBuffer
> > > > +  )
> > >
> > >
> > > Minor comment:
> > >
> > > I found that the consumers of the above function are:
> > > GetRuntimeCacheInfo()
> > > SendRuntimeVariableCacheContextToSmm()
> > >
> > > Both of them get called within SmmVariableReady() when
> > the SMM variable
> > > driver
> > > finished initialization. I am wondering if they can
> > simply use the pre-allocated
> > > comm buffer (via InitCommunicateBuffer() and using
> > 'mVariableBuffer'),
> > > instead
> > > of looking into the configuration table.
> > >
> > > In my opinion, this function can be dropped.
> > >
> > >
> >
> > I did that initially. It was recommended to use this
> > method.
> >
> > > > +{
> > > > +  EFI_STATUS                                Status;
> > > > +  EDKII_PI_SMM_COMMUNICATION_REGION_TABLE
> > > > *PiSmmCommunicationRegionTable;
> > > > +  EFI_MEMORY_DESCRIPTOR                     *Entry;
> > > > +  UINTN
> > EntrySize;
> > > > +  UINT32                                    Index;
> > > > +
> > > > +  if (CommBuffer == NULL || CommBufferSize == NULL
> > ||
> > > > *CommBufferSize == 0) {
> > > > +    return EFI_INVALID_PARAMETER;
> > > > +  }
> > > > +
> > > > +  Status = EfiGetSystemConfigurationTable (
> > > > +
> > &gEdkiiPiSmmCommunicationRegionTableGuid,
> > > > +             (VOID **)
> > &PiSmmCommunicationRegionTable
> > > > +             );
> > > > +  if (EFI_ERROR (Status) ||
> > PiSmmCommunicationRegionTable == NULL) {
> > > > +    return EFI_NOT_FOUND;
> > > > +  }
> > > > +
> > > > +  Entry = (EFI_MEMORY_DESCRIPTOR *)
> > > (PiSmmCommunicationRegionTable
> > > > + 1);
> > > > +  EntrySize = 0;
> > > > +  for (Index = 0; Index <
> > PiSmmCommunicationRegionTable-
> > > > >NumberOfEntries; Index++) {
> > > > +    if (Entry->Type == EfiConventionalMemory) {
> > > > +      EntrySize = EFI_PAGES_TO_SIZE ((UINTN) Entry-
> > >NumberOfPages);
> > > > +      if (EntrySize >= *CommBufferSize) {
> > > > +        break;
> > > > +      }
> > > > +    }
> > > > +    Entry = (EFI_MEMORY_DESCRIPTOR *) ((UINT8 *)
> > Entry +
> > > > PiSmmCommunicationRegionTable->DescriptorSize);
> > > > +  }
> > > > +
> > > > +  if (Index < PiSmmCommunicationRegionTable-
> > >NumberOfEntries) {
> > > > +    *CommBufferSize = EntrySize;
> > > > +    *CommBuffer = (UINT8 *) (UINTN) Entry-
> > >PhysicalStart;
> > > > +    return EFI_SUCCESS;
> > > > +  }
> > > > +
> > > > +  return EFI_OUT_OF_RESOURCES;
> > > > +}
> > > > +
> > > >  /**
> > > >    Send the data in communicate buffer to SMM.
> > > >
> > > > @@ -424,6 +568,171 @@ Done:
> > > >    return Status;
> > > >  }
> > > >
> > > > +/**
> > > > +  Signals SMM to synchronize any pending variable
> > updates with the
> > > > runtime cache(s).
> > > > +
> > > > +**/
> > > > +VOID
> > > > +EFIAPI
> > > > +SyncRuntimeCache (
> > > > +  VOID
> > > > +  )
> > > > +{
> > > > +  //
> > > > +  // Init the communicate buffer. The buffer data
> > size is:
> > > > +  // SMM_COMMUNICATE_HEADER_SIZE +
> > > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE.
> > > > +  //
> > > > +  InitCommunicateBuffer (NULL, 0,
> > > > SMM_VARIABLE_FUNCTION_SYNC_RUNTIME_CACHE);
> > > > +
> > > > +  //
> > > > +  // Send data to SMM.
> > > > +  //
> > > > +  SendCommunicateBuffer (0);
> > > > +}
> > > > +
> > > > +/**
> > > > +  Check whether a SMI must be triggered to retrieve
> > pending cache
> > > updates.
> > > > +
> > > > +  If the variable HOB was finished being flushed
> > since the last check for a
> > > > runtime cache update, this function
> > > > +  will prevent the HOB cache from being used for
> > future runtime cache
> > > hits.
> > > > +
> > > > +**/
> > > > +VOID
> > > > +EFIAPI
> > > > +CheckForRuntimeCacheSync (
> > > > +  VOID
> > > > +  )
> > > > +{
> > > > +  if (mVariableRuntimeCachePendingUpdate) {
> > > > +    SyncRuntimeCache ();
> > > > +  }
> > > > +  ASSERT (!mVariableRuntimeCachePendingUpdate);
> > > > +
> > > > +  //
> > > > +  // The HOB variable data may have finished being
> > flushed in the runtime
> > > > cache sync update
> > > > +  //
> > > > +  if (mHobFlushComplete &&
> > mVariableRuntimeHobCacheBuffer != NULL)
> > > {
> > > > +    if (!AtRuntime ()) {
> > > > +      FreePool (mVariableRuntimeHobCacheBuffer);
> > > > +    }
> > > > +    mVariableRuntimeHobCacheBuffer = NULL;
> > > > +  }
> > > > +}
> > > > +
> > > > +/**
> > > > +  This code finds variable in a volatile memory
> > store.
> > > > +
> > > > +  Caution: This function may receive untrusted
> > input.
> > > > +  The data size is external input, so this function
> > will validate it carefully to
> > > > avoid buffer overflow.
> > > > +
> > > > +  @param[in]      VariableName       Name of
> > Variable to be found.
> > > > +  @param[in]      VendorGuid         Variable
> > vendor GUID.
> > > > +  @param[out]     Attributes         Attribute
> > value of the variable found.
> > > > +  @param[in, out] DataSize           Size of Data
> > found. If size is less than the
> > > > +                                     data, this
> > value contains the required size.
> > > > +  @param[out]     Data               Data pointer.
> > > > +
> > > > +  @retval EFI_SUCCESS                Found the
> > specified variable.
> > > > +  @retval EFI_INVALID_PARAMETER      Invalid
> > parameter.
> > > > +  @retval EFI_NOT_FOUND              The specified
> > variable could not be
> > > found.
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +EFIAPI
> > > > +FindVariableInRuntimeCache (
> > > > +  IN      CHAR16
> > *VariableName,
> > > > +  IN      EFI_GUID
> > *VendorGuid,
> > > > +  OUT     UINT32
> > *Attributes OPTIONAL,
> > > > +  IN OUT  UINTN
> > *DataSize,
> > > > +  OUT     VOID                              *Data
> > OPTIONAL
> > > > +  )
> > > > +{
> > > > +  EFI_STATUS              Status;
> > > > +  UINTN                   DelayIndex;
> > > > +  UINTN                   TempDataSize;
> > > > +  VARIABLE_POINTER_TRACK  RtPtrTrack;
> > > > +  VARIABLE_STORE_TYPE     StoreType;
> > > > +  VARIABLE_STORE_HEADER
> > *VariableStoreList[VariableStoreTypeMax];
> > > > +
> > > > +  Status = EFI_NOT_FOUND;
> > > > +
> > > > +  if (VariableName == NULL || VendorGuid == NULL ||
> > DataSize == NULL) {
> > > > +    return EFI_INVALID_PARAMETER;
> > > > +  }
> > > > +
> > > > +  for (DelayIndex = 0;
> > mVariableRuntimeCacheReadLock && DelayIndex <
> > > > VARIABLE_RT_CACHE_READ_LOCK_TIMEOUT; DelayIndex++) {
> > > > +    MicroSecondDelay (10);
> > > > +  }
> > > > +  if (DelayIndex <
> > VARIABLE_RT_CACHE_READ_LOCK_TIMEOUT) {
> > > > +    ASSERT (!mVariableRuntimeCacheReadLock);
> > > > +
> > > > +    mVariableRuntimeCacheReadLock = TRUE;
> > > > +    CheckForRuntimeCacheSync ();
> > > > +
> > > > +    if (!mVariableRuntimeCachePendingUpdate) {
> > > > +      //
> > > > +      // 0: Volatile, 1: HOB, 2: Non-Volatile.
> > > > +      // The index and attributes mapping must be
> > kept in this order as
> > > > FindVariable
> > > > +      // makes use of this mapping to implement
> > search algorithm.
> > > > +      //
> > > > +      VariableStoreList[VariableStoreTypeVolatile]
> > =
> > > > mVariableRuntimeVolatileCacheBuffer;
> > > > +      VariableStoreList[VariableStoreTypeHob]
> > =
> > > > mVariableRuntimeHobCacheBuffer;
> > > > +      VariableStoreList[VariableStoreTypeNv]
> > =
> > > > mVariableRuntimeNvCacheBuffer;
> > > > +
> > > > +      for (StoreType = (VARIABLE_STORE_TYPE) 0;
> > StoreType <
> > > > VariableStoreTypeMax; StoreType++) {
> > > > +        if (VariableStoreList[StoreType] == NULL) {
> > > > +          continue;
> > > > +        }
> > > > +
> > > > +        RtPtrTrack.StartPtr = GetStartPointer
> > (VariableStoreList[StoreType]);
> > > > +        RtPtrTrack.EndPtr   = GetEndPointer
> > (VariableStoreList[StoreType]);
> > > > +        RtPtrTrack.Volatile = (BOOLEAN) (StoreType
> > ==
> > > > VariableStoreTypeVolatile);
> > > > +
> > > > +        Status = FindVariableEx (VariableName,
> > VendorGuid, FALSE,
> > > > &RtPtrTrack);
> > > > +        if (!EFI_ERROR (Status)) {
> > > > +          break;
> > > > +        }
> > > > +      }
> > > > +
> > > > +      if (!EFI_ERROR (Status)) {
> > > > +        //
> > > > +        // Get data size
> > > > +        //
> > > > +        TempDataSize = DataSizeOfVariable
> > (RtPtrTrack.CurrPtr);
> > > > +        ASSERT (TempDataSize != 0);
> > > > +
> > > > +        if (*DataSize >= TempDataSize) {
> > > > +          if (Data == NULL) {
> > > > +            Status = EFI_INVALID_PARAMETER;
> > > > +            goto Done;
> > > > +          }
> > > > +
> > > > +          CopyMem (Data, GetVariableDataPtr
> > (RtPtrTrack.CurrPtr),
> > > > TempDataSize);
> > > > +          if (Attributes != NULL) {
> > > > +            *Attributes = RtPtrTrack.CurrPtr-
> > >Attributes;
> > > > +          }
> > > > +
> > > > +          *DataSize = TempDataSize;
> > > > +
> > > > +          UpdateVariableInfo (VariableName,
> > VendorGuid,
> > > RtPtrTrack.Volatile,
> > > > TRUE, FALSE, FALSE, TRUE, &mVariableInfo);
> > > > +
> > > > +          Status = EFI_SUCCESS;
> > > > +          goto Done;
> > > > +        } else {
> > > > +          *DataSize = TempDataSize;
> > > > +          Status = EFI_BUFFER_TOO_SMALL;
> > > > +          goto Done;
> > > > +        }
> > > > +      }
> > > > +    }
> > > > +  }
> > > > +
> > > > +Done:
> > > > +  mVariableRuntimeCacheReadLock = FALSE;
> > >
> > >
> > > If timeout occurs when acquiring the read lock, should
> > this flag be set to
> > > FALSE
> > > in such case?
> > >
> >
> > Please see reply to patch #8.
> >
> > > Best Regards,
> > > Hao Wu
> > >
> > >
> > > > +
> > > > +  return Status;
> > > > +}
> > > > +
> > > >  /**
> > > >    This code finds variable in storage blocks
> > (Volatile or Non-Volatile).
> > > >
> > > > @@ -454,91 +763,21 @@ RuntimeServiceGetVariable (
> > > >    )
> > > >  {
> > > >    EFI_STATUS                                Status;
> > > > -  UINTN
> > PayloadSize;
> > > > -  SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE
> > > > *SmmVariableHeader;
> > > > -  UINTN
> > TempDataSize;
> > > > -  UINTN
> > VariableNameSize;
> > > >
> > > >    if (VariableName == NULL || VendorGuid == NULL ||
> > DataSize == NULL) {
> > > >      return EFI_INVALID_PARAMETER;
> > > >    }
> > > > -
> > > > -  TempDataSize          = *DataSize;
> > > > -  VariableNameSize      = StrSize (VariableName);
> > > > -  SmmVariableHeader     = NULL;
> > > > -
> > > > -  //
> > > > -  // If VariableName exceeds SMM payload limit.
> > Return failure
> > > > -  //
> > > > -  if (VariableNameSize > mVariableBufferPayloadSize
> > - OFFSET_OF
> > > > (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name)) {
> > > > -    return EFI_INVALID_PARAMETER;
> > > > -  }
> > > > -
> > > > -
> > AcquireLockOnlyAtBootTime(&mVariableServicesLock);
> > > > -
> > > > -  //
> > > > -  // Init the communicate buffer. The buffer data
> > size is:
> > > > -  // SMM_COMMUNICATE_HEADER_SIZE +
> > > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + PayloadSize.
> > > > -  //
> > > > -  if (TempDataSize > mVariableBufferPayloadSize -
> > OFFSET_OF
> > > > (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) -
> > > > VariableNameSize) {
> > > > -    //
> > > > -    // If output data buffer exceed SMM payload
> > limit. Trim output buffer to
> > > > SMM payload size
> > > > -    //
> > > > -    TempDataSize = mVariableBufferPayloadSize -
> > OFFSET_OF
> > > > (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) -
> > > > VariableNameSize;
> > > > -  }
> > > > -  PayloadSize = OFFSET_OF
> > > > (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) +
> > > > VariableNameSize + TempDataSize;
> > > > -
> > > > -  Status = InitCommunicateBuffer ((VOID
> > **)&SmmVariableHeader,
> > > > PayloadSize, SMM_VARIABLE_FUNCTION_GET_VARIABLE);
> > > > -  if (EFI_ERROR (Status)) {
> > > > -    goto Done;
> > > > -  }
> > > > -  ASSERT (SmmVariableHeader != NULL);
> > > > -
> > > > -  CopyGuid (&SmmVariableHeader->Guid, VendorGuid);
> > > > -  SmmVariableHeader->DataSize   = TempDataSize;
> > > > -  SmmVariableHeader->NameSize   = VariableNameSize;
> > > > -  if (Attributes == NULL) {
> > > > -    SmmVariableHeader->Attributes = 0;
> > > > -  } else {
> > > > -    SmmVariableHeader->Attributes = *Attributes;
> > > > -  }
> > > > -  CopyMem (SmmVariableHeader->Name, VariableName,
> > > > SmmVariableHeader->NameSize);
> > > > -
> > > > -  //
> > > > -  // Send data to SMM.
> > > > -  //
> > > > -  Status = SendCommunicateBuffer (PayloadSize);
> > > > -
> > > > -  //
> > > > -  // Get data from SMM.
> > > > -  //
> > > > -  if (Status == EFI_SUCCESS || Status ==
> > EFI_BUFFER_TOO_SMALL) {
> > > > -    //
> > > > -    // SMM CommBuffer DataSize can be a trimed
> > value
> > > > -    // Only update DataSize when needed
> > > > -    //
> > > > -    *DataSize = SmmVariableHeader->DataSize;
> > > > -  }
> > > > -  if (Attributes != NULL) {
> > > > -    *Attributes = SmmVariableHeader->Attributes;
> > > > -  }
> > > > -
> > > > -  if (EFI_ERROR (Status)) {
> > > > -    goto Done;
> > > > -  }
> > > > -
> > > > -  if (Data != NULL) {
> > > > -    CopyMem (Data, (UINT8 *)SmmVariableHeader->Name
> > +
> > > > SmmVariableHeader->NameSize, SmmVariableHeader-
> > >DataSize);
> > > > -  } else {
> > > > -    Status = EFI_INVALID_PARAMETER;
> > > > +  if (VariableName[0] == 0) {
> > > > +    return EFI_NOT_FOUND;
> > > >    }
> > > >
> > > > -Done:
> > > > +  AcquireLockOnlyAtBootTime
> > (&mVariableServicesLock);
> > > > +  Status =  FindVariableInRuntimeCache
> > (VariableName, VendorGuid,
> > > > Attributes, DataSize, Data);
> > > >    ReleaseLockOnlyAtBootTime
> > (&mVariableServicesLock);
> > > > +
> > > >    return Status;
> > > >  }
> > > >
> > > > -
> > > >  /**
> > > >    This code Finds the Next available variable.
> > > >
> > > > @@ -870,6 +1109,17 @@ OnReadyToBoot (
> > > >    //
> > > >    SendCommunicateBuffer (0);
> > > >
> > > > +  //
> > > > +  // Install the system configuration table for
> > variable info data captured
> > > > +  //
> > > > +  if (FeaturePcdGet (PcdVariableCollectStatistics))
> > {
> > > > +    if (mVariableAuthFormat) {
> > > > +      gBS->InstallConfigurationTable
> > (&gEfiAuthenticatedVariableGuid,
> > > > mVariableInfo);
> > > > +    } else {
> > > > +      gBS->InstallConfigurationTable
> > (&gEfiVariableGuid, mVariableInfo);
> > > > +    }
> > > > +  }
> > > > +
> > > >    gBS->CloseEvent (Event);
> > > >  }
> > > >
> > > > @@ -893,6 +1143,9 @@ VariableAddressChangeEvent (
> > > >  {
> > > >    EfiConvertPointer (0x0, (VOID **)
> > &mVariableBuffer);
> > > >    EfiConvertPointer (0x0, (VOID **)
> > &mSmmCommunication);
> > > > +  EfiConvertPointer (0x0, (VOID **)
> > &mVariableRuntimeHobCacheBuffer);
> > > > +  EfiConvertPointer (0x0, (VOID **)
> > &mVariableRuntimeNvCacheBuffer);
> > > > +  EfiConvertPointer (0x0, (VOID **)
> > > > &mVariableRuntimeVolatileCacheBuffer);
> > > >  }
> > > >
> > > >  /**
> > > > @@ -969,6 +1222,173 @@ Done:
> > > >    return Status;
> > > >  }
> > > >
> > > > +/**
> > > > +  This code gets information needed from SMM for
> > runtime cache
> > > > initialization.
> > > > +
> > > > +  @param[out] TotalHobStorageSize         Output
> > pointer for the total HOB
> > > > storage size in bytes.
> > > > +  @param[out] TotalNvStorageSize          Output
> > pointer for the total non-
> > > > volatile storage size in bytes.
> > > > +  @param[out] TotalVolatileStorageSize    Output
> > pointer for the total
> > > > volatile storage size in bytes.
> > > > +  @param[out] AuthenticatedVariableUsage  Output
> > pointer that indicates
> > > if
> > > > authenticated variables are to be used.
> > > > +
> > > > +  @retval EFI_SUCCESS                     Retrieved
> > the size successfully.
> > > > +  @retval EFI_INVALID_PARAMETER
> > TotalNvStorageSize parameter is
> > > > NULL.
> > > > +  @retval EFI_OUT_OF_RESOURCES            Could not
> > allocate a
> > > CommBuffer.
> > > > +  @retval Others                          Could not
> > retrieve the size successfully.;
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +EFIAPI
> > > > +GetRuntimeCacheInfo (
> > > > +  OUT UINTN
> > *TotalHobStorageSize,
> > > > +  OUT UINTN
> > *TotalNvStorageSize,
> > > > +  OUT UINTN
> > *TotalVolatileStorageSize,
> > > > +  OUT BOOLEAN
> > *AuthenticatedVariableUsage
> > > > +  )
> > > > +{
> > > > +  EFI_STATUS
> > Status;
> > > > +  SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO
> > > > *SmmGetRuntimeCacheInfo;
> > > > +  EFI_SMM_COMMUNICATE_HEADER
> > > > *SmmCommunicateHeader;
> > > > +  SMM_VARIABLE_COMMUNICATE_HEADER
> > > > *SmmVariableFunctionHeader;
> > > > +  UINTN
> > CommSize;
> > > > +  UINTN
> > CommBufferSize;
> > > > +  UINT8
> > *CommBuffer;
> > > > +
> > > > +  SmmGetRuntimeCacheInfo = NULL;
> > > > +  CommBuffer = NULL;
> > > > +
> > > > +  if (TotalHobStorageSize == NULL ||
> > TotalNvStorageSize == NULL ||
> > > > TotalVolatileStorageSize == NULL ||
> > AuthenticatedVariableUsage == NULL)
> > > {
> > > > +    return EFI_INVALID_PARAMETER;
> > > > +  }
> > > > +
> > > > +  AcquireLockOnlyAtBootTime
> > (&mVariableServicesLock);
> > > > +
> > > > +  CommSize = SMM_COMMUNICATE_HEADER_SIZE +
> > > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + sizeof
> > > > (SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO);
> > > > +  CommBufferSize = CommSize;
> > > > +  Status = GetCommunicateBuffer (&CommBufferSize,
> > &CommBuffer);
> > > > +  if (EFI_ERROR (Status)) {
> > > > +    goto Done;
> > > > +  }
> > > > +  if (CommBuffer == NULL) {
> > > > +    Status = EFI_OUT_OF_RESOURCES;
> > > > +    goto Done;
> > > > +  }
> > > > +  ZeroMem (CommBuffer, CommBufferSize);
> > > > +
> > > > +  SmmCommunicateHeader =
> > (EFI_SMM_COMMUNICATE_HEADER *)
> > > > CommBuffer;
> > > > +  CopyGuid (&SmmCommunicateHeader->HeaderGuid,
> > > > &gEfiSmmVariableProtocolGuid);
> > > > +  SmmCommunicateHeader->MessageLength =
> > > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + sizeof
> > > > (SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO);
> > > > +
> > > > +  SmmVariableFunctionHeader =
> > > > (SMM_VARIABLE_COMMUNICATE_HEADER *)
> > SmmCommunicateHeader-
> > > > >Data;
> > > > +  SmmVariableFunctionHeader->Function =
> > > > SMM_VARIABLE_FUNCTION_GET_RUNTIME_CACHE_INFO;
> > > > +  SmmGetRuntimeCacheInfo =
> > > > (SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO *)
> > > > SmmVariableFunctionHeader->Data;
> > > > +
> > > > +  //
> > > > +  // Send data to SMM.
> > > > +  //
> > > > +  Status = mSmmCommunication->Communicate
> > (mSmmCommunication,
> > > > CommBuffer, &CommSize);
> > > > +  ASSERT_EFI_ERROR (Status);
> > > > +  if (CommSize <=
> > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) {
> > > > +    Status = EFI_BAD_BUFFER_SIZE;
> > > > +    goto Done;
> > > > +  }
> > > > +
> > > > +  Status = SmmVariableFunctionHeader->ReturnStatus;
> > > > +  if (EFI_ERROR (Status)) {
> > > > +    goto Done;
> > > > +  }
> > > > +
> > > > +  //
> > > > +  // Get data from SMM.
> > > > +  //
> > > > +  *TotalHobStorageSize = SmmGetRuntimeCacheInfo-
> > > >TotalHobStorageSize;
> > > > +  *TotalNvStorageSize = SmmGetRuntimeCacheInfo-
> > >TotalNvStorageSize;
> > > > +  *TotalVolatileStorageSize =
> > SmmGetRuntimeCacheInfo-
> > > > >TotalVolatileStorageSize;
> > > > +  *AuthenticatedVariableUsage =
> > SmmGetRuntimeCacheInfo-
> > > > >AuthenticatedVariableUsage;
> > > > +
> > > > +Done:
> > > > +  ReleaseLockOnlyAtBootTime
> > (&mVariableServicesLock);
> > > > +  return Status;
> > > > +}
> > > > +
> > > > +/**
> > > > +  Sends the runtime variable cache context
> > information to SMM.
> > > > +
> > > > +  @retval EFI_SUCCESS               Retrieved the
> > size successfully.
> > > > +  @retval EFI_INVALID_PARAMETER
> > TotalNvStorageSize parameter is
> > > > NULL.
> > > > +  @retval EFI_OUT_OF_RESOURCES      Could not
> > allocate a CommBuffer.
> > > > +  @retval Others                    Could not
> > retrieve the size successfully.;
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +EFIAPI
> > > > +SendRuntimeVariableCacheContextToSmm (
> > > > +  VOID
> > > > +  )
> > > > +{
> > > > +  EFI_STATUS
> > Status;
> > > > +
> > > >
> > SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT
> > > > *SmmRuntimeVarCacheContext;
> > > > +  EFI_SMM_COMMUNICATE_HEADER
> > > > *SmmCommunicateHeader;
> > > > +  SMM_VARIABLE_COMMUNICATE_HEADER
> > > > *SmmVariableFunctionHeader;
> > > > +  UINTN
> > CommSize;
> > > > +  UINTN
> > CommBufferSize;
> > > > +  UINT8
> > *CommBuffer;
> > > > +
> > > > +  SmmRuntimeVarCacheContext = NULL;
> > > > +  CommBuffer = NULL;
> > > > +
> > > > +  AcquireLockOnlyAtBootTime
> > (&mVariableServicesLock);
> > > > +
> > > > +  //
> > > > +  // Init the communicate buffer. The buffer data
> > size is:
> > > > +  // SMM_COMMUNICATE_HEADER_SIZE +
> > > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + sizeof
> > > >
> > >
> >
> (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT
> > )
> > > ;
> > > > +  //
> > > > +  CommSize = SMM_COMMUNICATE_HEADER_SIZE +
> > > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + sizeof
> > > >
> > >
> >
> (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT
> > )
> > > ;
> > > > +  CommBufferSize = CommSize;
> > > > +  Status = GetCommunicateBuffer (&CommBufferSize,
> > &CommBuffer);
> > > > +  if (EFI_ERROR (Status)) {
> > > > +    goto Done;
> > > > +  }
> > > > +  if (CommBuffer == NULL) {
> > > > +    Status = EFI_OUT_OF_RESOURCES;
> > > > +    goto Done;
> > > > +  }
> > > > +  ZeroMem (CommBuffer, CommBufferSize);
> > > > +
> > > > +  SmmCommunicateHeader =
> > (EFI_SMM_COMMUNICATE_HEADER *)
> > > > CommBuffer;
> > > > +  CopyGuid (&SmmCommunicateHeader->HeaderGuid,
> > > > &gEfiSmmVariableProtocolGuid);
> > > > +  SmmCommunicateHeader->MessageLength =
> > > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + sizeof
> > > >
> > >
> >
> (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT
> > )
> > > ;
> > > > +
> > > > +  SmmVariableFunctionHeader =
> > > > (SMM_VARIABLE_COMMUNICATE_HEADER *)
> > SmmCommunicateHeader-
> > > > >Data;
> > > > +  SmmVariableFunctionHeader->Function =
> > > >
> > >
> > SMM_VARIABLE_FUNCTION_INIT_RUNTIME_VARIABLE_CACHE_CONTEX
> > T;
> > > > +  SmmRuntimeVarCacheContext =
> > > >
> > >
> >
> (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT
> > > > *) SmmVariableFunctionHeader->Data;
> > > > +
> > > > +  SmmRuntimeVarCacheContext->RuntimeHobCache =
> > > > mVariableRuntimeHobCacheBuffer;
> > > > +  SmmRuntimeVarCacheContext->RuntimeVolatileCache =
> > > > mVariableRuntimeVolatileCacheBuffer;
> > > > +  SmmRuntimeVarCacheContext->RuntimeNvCache =
> > > > mVariableRuntimeNvCacheBuffer;
> > > > +  SmmRuntimeVarCacheContext->PendingUpdate =
> > > > &mVariableRuntimeCachePendingUpdate;
> > > > +  SmmRuntimeVarCacheContext->ReadLock =
> > > > &mVariableRuntimeCacheReadLock;
> > > > +  SmmRuntimeVarCacheContext->HobFlushComplete =
> > > > &mHobFlushComplete;
> > > > +
> > > > +  //
> > > > +  // Send data to SMM.
> > > > +  //
> > > > +  Status = mSmmCommunication->Communicate
> > (mSmmCommunication,
> > > > CommBuffer, &CommSize);
> > > > +  ASSERT_EFI_ERROR (Status);
> > > > +  if (CommSize <=
> > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE) {
> > > > +    Status = EFI_BAD_BUFFER_SIZE;
> > > > +    goto Done;
> > > > +  }
> > > > +
> > > > +  Status = SmmVariableFunctionHeader->ReturnStatus;
> > > > +  if (EFI_ERROR (Status)) {
> > > > +    goto Done;
> > > > +  }
> > > > +
> > > > +Done:
> > > > +  ReleaseLockOnlyAtBootTime
> > (&mVariableServicesLock);
> > > > +  return Status;
> > > > +}
> > > > +
> > > >  /**
> > > >    Initialize variable service and install Variable
> > Architectural protocol.
> > > >
> > > > @@ -985,7 +1405,7 @@ SmmVariableReady (
> > > >  {
> > > >    EFI_STATUS                                Status;
> > > >
> > > > -  Status = gBS->LocateProtocol
> > (&gEfiSmmVariableProtocolGuid, NULL,
> > > > (VOID **)&mSmmVariable);
> > > > +  Status = gBS->LocateProtocol
> > (&gEfiSmmVariableProtocolGuid, NULL,
> > > > (VOID **) &mSmmVariable);
> > > >    if (EFI_ERROR (Status)) {
> > > >      return;
> > > >    }
> > > > @@ -1007,6 +1427,40 @@ SmmVariableReady (
> > > >    //
> > > >    mVariableBufferPhysical = mVariableBuffer;
> > > >
> > > > +  //
> > > > +  // Allocate runtime variable cache memory
> > buffers.
> > > > +  //
> > > > +  Status =  GetRuntimeCacheInfo (
> > > > +              &mVariableRuntimeHobCacheBufferSize,
> > > > +              &mVariableRuntimeNvCacheBufferSize,
> > > > +
> > &mVariableRuntimeVolatileCacheBufferSize,
> > > > +              &mVariableAuthFormat
> > > > +              );
> > > > +  if (!EFI_ERROR (Status)) {
> > > > +    Status = InitVariableCache
> > (&mVariableRuntimeHobCacheBuffer,
> > > > &mVariableRuntimeHobCacheBufferSize);
> > > > +    if (!EFI_ERROR (Status)) {
> > > > +      Status = InitVariableCache
> > (&mVariableRuntimeNvCacheBuffer,
> > > > &mVariableRuntimeNvCacheBufferSize);
> > > > +      if (!EFI_ERROR (Status)) {
> > > > +        Status = InitVariableCache
> > (&mVariableRuntimeVolatileCacheBuffer,
> > > > &mVariableRuntimeVolatileCacheBufferSize);
> > > > +        if (!EFI_ERROR (Status)) {
> > > > +          Status = InitVariableParsing
> > (mVariableAuthFormat);
> > > > +          ASSERT_EFI_ERROR (Status);
> > > > +
> > > > +          Status =
> > SendRuntimeVariableCacheContextToSmm ();
> > > > +          if (!EFI_ERROR (Status)) {
> > > > +            SyncRuntimeCache ();
> > > > +          }
> > > > +        }
> > > > +      }
> > > > +    }
> > > > +    if (EFI_ERROR (Status)) {
> > > > +      mVariableRuntimeHobCacheBuffer = NULL;
> > > > +      mVariableRuntimeNvCacheBuffer = NULL;
> > > > +      mVariableRuntimeVolatileCacheBuffer = NULL;
> > > > +    }
> > > > +  }
> > > > +  ASSERT_EFI_ERROR (Status);
> > > > +
> > > >    gRT->GetVariable         =
> > RuntimeServiceGetVariable;
> > > >    gRT->GetNextVariableName =
> > RuntimeServiceGetNextVariableName;
> > > >    gRT->SetVariable         =
> > RuntimeServiceSetVariable;
> > > > --
> > > > 2.16.2.windows.1
> > >
> >
> 


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

View/Reply Online (#48463): https://edk2.groups.io/g/devel/message/48463
Mute This Topic: https://groups.io/mt/34318592/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to