public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Kubacki, Michael A" <michael.a.kubacki@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>,
	"Wu, Hao A" <hao.a.wu@intel.com>,
	"devel@edk2.groups.io" <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>,
	"Ni, Ray" <ray.ni@intel.com>,
	"Wang, Jian J" <jian.j.wang@intel.com>,
	"Yao, Jiewen" <jiewen.yao@intel.com>
Subject: Re: [PATCH V2 7/9] MdeModulePkg/Variable: Add RT GetVariable() cache support
Date: Fri, 4 Oct 2019 16:48:08 +0000	[thread overview]
Message-ID: <DM6PR11MB38343D44109E8215779022D0B59E0@DM6PR11MB3834.namprd11.prod.outlook.com> (raw)
In-Reply-To: <d0c57218-5ff4-3875-f98e-bde154199808@redhat.com>

> On 10/04/19 01:31, Kubacki, Michael A wrote:
> > I agree, I will make the default to enable the runtime cache.
> 
> I've just made a request for the opposite :) , before reading this part
> of the thread.
> 
> Let me revise my request then, seeing the above preference. From the
> following three options:
> 
> (1) set DEC default to FALSE,
> 
> (2) set the DEC default to TRUE, but set the PCD in OvmfPkg DSC files to
> FALSE,
> 
> (3) set the DEC default to TRUE, and inherit it in OvmfPkg,
> 
> I'd ask for (2) in the short or mid term, and entertain (3) in the long
> term (dependent on the upstream kernel patch I linked elsewhere in the
> thread:
> <http://mid.mail-archive.com/20191003100712.31045-1-
> javierm@redhat.com>).
> 
> 
> With the PCD available in the DEC file, I agree that downstream OVMF
> packages could easily be built with the PCD set to FALSE, e.g. on the
> "build" command line, regardless of the upstream OvmfPkg DSC setting.
> Given that, option (2) might appear superfluous.
> 
> However, I'd like upstream OvmfPkg's DSC setting to reflect that as yet,
> there is no alternative to GetVariable(), for exercising the SMM stack
> built into OVMF without side effects to the variable store. Once the
> kernel patch is merged and QueryVariableInfo() becomes *recommendable*
> as a practical substitute for GetVariable(), we can switch upstream
> OvmfPkg from option (2) to option (3).
> 
> Does that sound acceptable?
> 

I have no problem setting the PCD to FALSE in all the OvmfPkg DSC files. 

> Thanks!
> Laszlo
> 
> 
> >
> >> -----Original Message-----
> >> From: Kinney, Michael D <michael.d.kinney@intel.com>
> >> Sent: Thursday, October 3, 2019 3:01 PM
> >> To: Kubacki, Michael A <michael.a.kubacki@intel.com>; Wu, Hao A
> >> <hao.a.wu@intel.com>; devel@edk2.groups.io; Kinney, Michael D
> >> <michael.d.kinney@intel.com>
> >> Cc: Bi, Dandan <dandan.bi@intel.com>; Ard Biesheuvel
> >> <ard.biesheuvel@linaro.org>; Dong, Eric <eric.dong@intel.com>; Laszlo
> Ersek
> >> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>; Ni, Ray
> >> <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen
> >> <jiewen.yao@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.kubacki@intel.com>
> >>> Sent: Thursday, October 3, 2019 2:54 PM
> >>> To: Wu, Hao A <hao.a.wu@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>; Laszlo Ersek <lersek@redhat.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>; Yao, Jiewen
> >>> <jiewen.yao@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.wu@intel.com>
> >>>> Sent: Thursday, October 3, 2019 1:05 AM
> >>>> 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>; Laszlo Ersek
> >>>> <lersek@redhat.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>; Yao, Jiewen
> >>> <jiewen.yao@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.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>
> >>>>> ---
> >>>>>
> >>> 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
> >>>>
> >>>
> >>
> >


  reply	other threads:[~2019-10-04 16:48 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-28  1:47 [PATCH V2 0/9] UEFI Variable SMI Reduction Kubacki, Michael A
2019-09-28  1:47 ` [PATCH V2 1/9] MdeModulePkg/Variable: Consolidate common parsing functions Kubacki, Michael A
2019-10-03  8:03   ` Wu, Hao A
2019-10-03 17:35     ` Kubacki, Michael A
2019-10-08  2:11       ` Wu, Hao A
2019-10-08 21:53         ` Kubacki, Michael A
2019-10-08  6:07   ` Wang, Jian J
2019-10-08 22:00     ` Kubacki, Michael A
2019-09-28  1:47 ` [PATCH V2 2/9] MdeModulePkg/Variable: Parameterize GetNextVariableEx() store list Kubacki, Michael A
2019-10-03  8:03   ` Wu, Hao A
2019-10-03 18:04     ` Kubacki, Michael A
2019-09-28  1:47 ` [PATCH V2 3/9] MdeModulePkg/Variable: Parameterize VARIABLE_INFO_ENTRY buffer Kubacki, Michael A
2019-10-03  8:03   ` Wu, Hao A
2019-10-03 18:05     ` Kubacki, Michael A
2019-10-08  2:11       ` [edk2-devel] " Wu, Hao A
2019-10-08 21:49         ` Kubacki, Michael A
2019-09-28  1:47 ` [PATCH V2 4/9] MdeModulePkg/Variable: Add local auth status in VariableParsing Kubacki, Michael A
2019-10-03  8:04   ` [edk2-devel] " Wu, Hao A
2019-10-03 18:35     ` Kubacki, Michael A
2019-10-16  7:55       ` Wu, Hao A
2019-10-16 16:37         ` Kubacki, Michael A
2019-10-17  1:00           ` Wu, Hao A
2019-09-28  1:47 ` [PATCH V2 5/9] MdeModulePkg/Variable: Add a file for NV variable functions Kubacki, Michael A
2019-10-03  8:04   ` Wu, Hao A
2019-10-03 18:43     ` Kubacki, Michael A
2019-09-28  1:47 ` [PATCH V2 6/9] MdeModulePkg VariableInfo: Always consider RT DXE and SMM stats Kubacki, Michael A
2019-10-03  8:04   ` Wu, Hao A
2019-09-28  1:47 ` [PATCH V2 7/9] MdeModulePkg/Variable: Add RT GetVariable() cache support Kubacki, Michael A
2019-10-03  8:04   ` Wu, Hao A
2019-10-03 11:00     ` Laszlo Ersek
2019-10-03 20:53       ` Kubacki, Michael A
2019-10-03 21:53     ` Kubacki, Michael A
2019-10-03 22:01       ` Michael D Kinney
2019-10-03 23:31         ` Kubacki, Michael A
2019-10-04  6:50           ` Laszlo Ersek
2019-10-04 16:48             ` Kubacki, Michael A [this message]
2019-10-04  6:38       ` Laszlo Ersek
2019-10-04 16:48         ` Kubacki, Michael A
2019-10-08  2:12       ` Wu, Hao A
2019-09-28  1:47 ` [PATCH V2 8/9] MdeModulePkg/Variable: Add RT GetNextVariableName() " Kubacki, Michael A
2019-10-03  8:04   ` Wu, Hao A
2019-10-03 18:52     ` Kubacki, Michael A
2019-10-03 18:59       ` [edk2-devel] " Andrew Fish
2019-10-03 20:12         ` Kubacki, Michael A
2019-09-28  1:47 ` [PATCH V2 9/9] MdeModulePkg/VariableSmm: Remove unused SMI handler functions Kubacki, Michael A

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR11MB38343D44109E8215779022D0B59E0@DM6PR11MB3834.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox