From: "Kubacki, Michael A" <michael.a.kubacki@intel.com>
To: "Wang, Jian J" <jian.j.wang@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>,
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>, "Wu, Hao A" <hao.a.wu@intel.com>,
"Yao, Jiewen" <jiewen.yao@intel.com>
Subject: Re: [edk2-devel] [PATCH V4 07/10] MdeModulePkg/Variable: Add RT GetVariable() cache support
Date: Thu, 17 Oct 2019 01:24:53 +0000 [thread overview]
Message-ID: <DM6PR11MB383499CE5A7DBBDD4CCF2017B56D0@DM6PR11MB3834.namprd11.prod.outlook.com> (raw)
In-Reply-To: <D827630B58408649ACB04F44C510003625992EAD@SHSMSX107.ccr.corp.intel.com>
The "Data" parameter is marked OPTIONAL in FindVariableInRuntimeCache () for essentially the same reason
it is labeled OPTIONAL in the GetVariable () API in the UEFI specification. Callers expect that they can pass a
NULL as an actual parameter for Data and get back the size of a buffer needed for the given variable name and
GUID.
I addressed the implementation of AtRuntime () in VariableSmmRuntimeDxe.c alongside EfiAtRuntime () calls in
the file in another V4 reply. The conclusion is AtRuntime () is called by functions in VariableParsing.c which due
to its generic nature is linked against VariableSmmRuntimeDxe and VariableSmm. VariableSmm cannot call
EfiAtRuntime (). There's various ways to twist this but most I've considered are really cosmetic tradeoffs.
Jian, you left it open to me as to whether the buffers should be freed in SmmVariableReady () in VariableSmmRuntimeDxe.c
in the failure case. During implementation, I initially thought the platform would roughly "earmark" a fixed amount of
overall EfiRuntimeServicesData memory (typical value + some threshold) for S4 memory map consistency so it's not
consuming memory not already accounted for. I was also not aware of any harmful effects to not freeing the buffers,
but I did mean to reconsider this in the future. I still don't have enough justification to form a strong opinion either way,
so please let me know if you think it's necessary.
Thanks,
Michael
> -----Original Message-----
> From: Wang, Jian J <jian.j.wang@intel.com>
> Sent: Tuesday, October 15, 2019 11:55 PM
> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Kubacki,
> Michael A <michael.a.kubacki@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>; Kinney, Michael
> D <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: RE: [edk2-devel] [PATCH V4 07/10] MdeModulePkg/Variable: Add
> RT GetVariable() cache support
>
> The comments are for VariableRuntimeCache.c only.
>
> Regards,
> Jian
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wang,
> Jian
> > J
> > Sent: Wednesday, October 16, 2019 2:46 PM
> > To: Kubacki, Michael A <michael.a.kubacki@intel.com>;
> devel@edk2.groups.io
> > Cc: Bi, Dandan <dandan.bi@intel.com>; Ard Biesheuvel
> > <ard.biesheuvel@linaro.org>; Dong, Eric <eric.dong@intel.com>; 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>; Wu, Hao A
> > <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> > Subject: Re: [edk2-devel] [PATCH V4 07/10] MdeModulePkg/Variable: Add
> RT
> > GetVariable() cache support
> >
> > Hi Michael,
> >
> > Please see my inline comments.
> >
> > > -----Original Message-----
> > > From: Kubacki, Michael A <michael.a.kubacki@intel.com>
> > > Sent: Tuesday, October 15, 2019 7:30 AM
> > > To: 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>; Wu, Hao A <hao.a.wu@intel.com>; Yao,
> Jiewen
> > > <jiewen.yao@intel.com>
> > > Subject: [PATCH V4 07/10] 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 runtime cache can be disabled with by setting the FeaturePCD
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache
> > > to FALSE. If the PCD is set to FALSE, the runtime cache will not be
> > > used and an SMI will be triggered for Runtime Service
> > > GetVariable () and GetNextVariableName () invocations.
> > >
> > > The following are important points regarding the behavior of the
> > > variable drivers when the variable runtime cache is enabled.
> > >
> > > 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 with runtime cache enabled, 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.
> > >
> > > Because a race condition can occur if an SMI occurs during the execution
> > > of runtime code reading from the runtime cache, a runtime cache read
> lock
> > > is introduced that explicitly moves pending updates from SMM to the
> runtime
> > > cache if an SMM update occurs while the runtime cache is locked. Note
> that
> > > it is not expected a Runtime services call will interrupt SMM processing
> > > since all CPU cores rendezvous in SMM.
> > >
> > > It is possible to view UEFI variable read and write statistics by setting
> > > the gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics
> > 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/MdeModulePkg.dec | 12 +
> > >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> |
> > 2
> > > +
> > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
> | 2 +
> > >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.i
> nf
> > |
> > > 20 +-
> > >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> |
> > > 2 +
> > > MdeModulePkg/Include/Guid/SmmVariableCommon.h | 29
> +-
> > > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h | 32
> +-
> > >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.h
> |
> > > 51 ++
> > > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 50
> +-
> > >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c
> |
> > > 153 ++++++
> > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c |
> 114
> > > ++++-
> > >
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
> c
> > |
> > > 512 +++++++++++++++++++-
> > > 12 files changed, 938 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > > b/MdeModulePkg/MdeModulePkg.dec
> > > index 59b8c21713..a00835cb84 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > @@ -641,6 +641,18 @@
> > > # @Prompt Enable Device Path From Text support.
> > >
> > >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdDevicePathSupportDevicePathFrom
> Text
> > > |TRUE|BOOLEAN|0x00010038
> > >
> > > + ## Indicates if the UEFI variable runtime cache should be enabled.
> > > + # This setting only applies if SMM variables are enabled. When
> enabled, all
> > > variable
> > > + # data for Runtime Service GetVariable () and GetNextVariableName
> () calls
> > is
> > > retrieved
> > > + # from a runtime data buffer referred to as the "runtime cache". An
> SMI is
> > not
> > > triggered
> > > + # at all for these requests. Variables writes still trigger an SMI. This can
> > > greatly
> > > + # reduce overall system SMM usage as most boots tend to issue far
> more
> > > variable reads
> > > + # than writes.<BR><BR>
> > > + # TRUE - The UEFI variable runtime cache is enabled.<BR>
> > > + # FALSE - The UEFI variable runtime cache is disabled.<BR>
> > > + # @Prompt Enable the UEFI variable runtime cache.
> > > +
> > >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache|FALS
> E|B
> > > OOLEAN|0x00010039
> > > +
> > > ## Indicates if the statistics about variable usage will be collected. This
> > > information is
> > > # stored as a vendor configuration table into the EFI system table.
> > > # Set this PCD to TRUE to use VariableInfo application in
> > > MdeModulePkg\Application directory to get
> > > diff --git
> > >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> > > index 08a5490787..ceea5d1ff9 100644
> > > ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> > > +++
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> > > @@ -40,6 +40,8 @@
> > > VariableNonVolatile.h
> > > VariableParsing.c
> > > VariableParsing.h
> > > + VariableRuntimeCache.c
> > > + VariableRuntimeCache.h
> > > 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/VariableSmmRuntimeDx
> e.inf
> > >
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.inf
> > > index 14894e6f13..b5a779a233 100644
> > > ---
> > >
> >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.inf
> > > +++
> > >
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> 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
> > > @@ -65,7 +69,21 @@
> > > gEdkiiVariableLockProtocolGuid ## PRODUCES
> > > gEdkiiVarCheckProtocolGuid ## PRODUCES
> > >
> > > +[FeaturePcd]
> > > + gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache
> > > ## CONSUMES
> > > + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics
> ##
> > > 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
> > > diff --git
> > >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> nf
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.
> inf
> > > index f8a3742959..6e17f6cdf5 100644
> > > ---
> > >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.i
> nf
> > > +++
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.
> 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_CONTEXT
> > 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..0b2bb6ae66 100644
> > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> > > @@ -64,6 +64,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 +94,15 @@ 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 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/VariableRuntimeCache.h
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.
> h
> > > new file mode 100644
> > > index 0000000000..f9804a1d69
> > > --- /dev/null
> > > +++
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.
> h
> > > @@ -0,0 +1,51 @@
> > > +/** @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
> > > +FlushPendingRuntimeVariableCacheUpdates (
> > > + VOID
> > > + );
> > > +
> > > +/**
> > > + Synchronizes the runtime variable caches with all pending updates
> outside
> > > runtime.
> > > +
> > > + Ensures all conditions are met to maintain coherency for runtime cache
> > > updates. This function will attempt
> > > + to write the given update (and any other pending updates) if the
> ReadLock is
> > > available. Otherwise, the
> > > + update is added as a pending update for the given variable store and it
> will
> > be
> > > flushed to the runtime cache
> > > + at the next opportunity the ReadLock is available.
> > > +
> > > + @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_SUCCESS The update was added as a pending
> update
> > > successfully. If the variable runtime
> > > + cache ReadLock was available, the runtime cache was
> > > updated successfully.
> > > + @retval EFI_UNSUPPORTED The volatile store to be updated is not
> > > initialized properly.
> > > +
> > > +**/
> > > +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 0bd2f22e1a..29d6aca993 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.VariableRuntimeNvCache,
> > > + (UINTN) VarErrFlag - (UINTN) mNvVariableCache + (UINTN)
> > > mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase,
> > > + sizeof (TempFlag)
> > > + );
> > > + ASSERT_EFI_ERROR (Status);
> > > }
> > > }
> > > }
> > > @@ -766,12 +773,24 @@ Reclaim (
> > >
> > > Done:
> > > if (IsVolatile || mVariableModuleGlobal->VariableGlobal.EmuNvMode)
> {
> > > + Status = SynchronizeRuntimeVariableCache (
> > > + &mVariableModuleGlobal-
> > >
> >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCach
> 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.VariableRuntimeNvCache,
> > > + 0,
> > > + VariableStoreHeader->Size
> > > + );
> > > + ASSERT_EFI_ERROR (Status);
> > > }
> > >
> > > return Status;
> > > @@ -1614,6 +1633,7 @@ UpdateVariable (
> > > VARIABLE_POINTER_TRACK *Variable;
> > > VARIABLE_POINTER_TRACK NvVariable;
> > > VARIABLE_STORE_HEADER *VariableStoreHeader;
> > > + VARIABLE_RUNTIME_CACHE *VolatileCacheInstance;
> > > UINT8 *BufferForMerge;
> > > UINTN MergedBufSize;
> > > BOOLEAN DataReady;
> > > @@ -2275,6 +2295,23 @@ UpdateVariable (
> > > }
> > >
> > > Done:
> > > + if (!EFI_ERROR (Status)) {
> > > + if (Variable->Volatile) {
> > > + VolatileCacheInstance = &(mVariableModuleGlobal-
> > >
> >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCach
> e
> > );
> > > + } else {
> > > + VolatileCacheInstance = &(mVariableModuleGlobal-
> > >
> >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache);
> > > + }
> > > +
> > > + if (VolatileCacheInstance->Store != NULL) {
> > > + Status = SynchronizeRuntimeVariableCache (
> > > + VolatileCacheInstance,
> > > + 0,
> > > + VolatileCacheInstance->Store->Size
> > > + );
> > > + ASSERT_EFI_ERROR (Status);
> > > + }
> > > + }
> > > +
> > > return Status;
> > > }
> > >
> > > @@ -3200,6 +3237,14 @@ FlushHobVariableToFlash (
> > > ErrorFlag = TRUE;
> > > }
> > > }
> > > + if (mVariableModuleGlobal-
> > >
> >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeHobCache.S
> to
> > r
> > > e != NULL) {
> > > + Status = SynchronizeRuntimeVariableCache (
> > > + &mVariableModuleGlobal-
> > >
> >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeHobCache,
> > > + 0,
> > > + mVariableModuleGlobal-
> > >
> >VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeHobCache.S
> to
> > r
> > > e->Size
> > > + );
> > > + ASSERT_EFI_ERROR (Status);
> > > + }
> > > if (ErrorFlag) {
> > > //
> > > // We still have HOB variable(s) not flushed in flash.
> > > @@ -3210,6 +3255,9 @@ FlushHobVariableToFlash (
> > > // All HOB variables have been flushed in flash.
> > > //
> > > DEBUG ((EFI_D_INFO, "Variable driver: all HOB variables have been
> flushed
> > > in flash.\n"));
> > > + if (mVariableModuleGlobal-
> > > >VariableGlobal.VariableRuntimeCacheContext.HobFlushComplete !=
> NULL) {
> > > + *(mVariableModuleGlobal-
> > > >VariableGlobal.VariableRuntimeCacheContext.HobFlushComplete) =
> TRUE;
> > > + }
> > > if (!AtRuntime ()) {
> > > FreePool ((VOID *) VariableStoreHeader);
> > > }
> > > diff --git
> > >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c
> > > new file mode 100644
> > > index 0000000000..bc93cc07d2
> > > --- /dev/null
> > > +++
> >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeCache.c
> > > @@ -0,0 +1,153 @@
> > > +/** @file
> > > + Functions related to managing the UEFI variable runtime cache. This file
> > > should only include functions
> > > + used by the SMM UEFI variable driver.
> > > +
> > > + 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
> > > +FlushPendingRuntimeVariableCacheUpdates (
> > > + VOID
> > > + )
> > > +{
> > > + VARIABLE_RUNTIME_CACHE_CONTEXT
> *VariableRuntimeCacheContext;
> > > +
> > > + VariableRuntimeCacheContext = &mVariableModuleGlobal-
> > > >VariableGlobal.VariableRuntimeCacheContext;
> > > +
> > > + if (VariableRuntimeCacheContext->VariableRuntimeNvCache.Store ==
> NULL
> > > ||
> > > + VariableRuntimeCacheContext-
> >VariableRuntimeVolatileCache.Store ==
> > > NULL ||
> > > + VariableRuntimeCacheContext->PendingUpdate == NULL) {
> > > + return EFI_UNSUPPORTED;
> > > + }
> > > +
> > > + if (*(VariableRuntimeCacheContext->PendingUpdate)) {
> > > + if (VariableRuntimeCacheContext->VariableRuntimeHobCache.Store
> !=
> > NULL
> > > &&
> > > + mVariableModuleGlobal->VariableGlobal.HobVariableBase > 0) {
> > > + CopyMem (
> > > + (VOID *) (
> > > + ((UINT8 *) (UINTN) VariableRuntimeCacheContext-
> > > >VariableRuntimeHobCache.Store) +
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeHobCache.PendingUpdateOffset
> > > + ),
> > > + (VOID *) (
> > > + ((UINT8 *) (UINTN) mVariableModuleGlobal-
> > > >VariableGlobal.HobVariableBase) +
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeHobCache.PendingUpdateOffset
> > > + ),
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeHobCache.PendingUpdateLength
> > > + );
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeHobCache.PendingUpdateLength = 0;
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeHobCache.PendingUpdateOffset = 0;
> > > + }
> > > +
> > > + CopyMem (
> > > + (VOID *) (
> > > + ((UINT8 *) (UINTN) VariableRuntimeCacheContext-
> > > >VariableRuntimeNvCache.Store) +
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeNvCache.PendingUpdateOffset
> > > + ),
> > > + (VOID *) (
> > > + ((UINT8 *) (UINTN) mNvVariableCache) +
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeNvCache.PendingUpdateOffset
> > > + ),
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeNvCache.PendingUpdateLength
> > > + );
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeNvCache.PendingUpdateLength = 0;
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeNvCache.PendingUpdateOffset = 0;
> > > +
> > > + CopyMem (
> > > + (VOID *) (
> > > + ((UINT8 *) (UINTN) VariableRuntimeCacheContext-
> > > >VariableRuntimeVolatileCache.Store) +
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeVolatileCache.PendingUpdateOffset
> > > + ),
> > > + (VOID *) (
> > > + ((UINT8 *) (UINTN) mVariableModuleGlobal-
> > > >VariableGlobal.VolatileVariableBase) +
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeVolatileCache.PendingUpdateOffset
> > > + ),
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeVolatileCache.PendingUpdateLength
> > > + );
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeVolatileCache.PendingUpdateLength = 0;
> > > + VariableRuntimeCacheContext-
> > > >VariableRuntimeVolatileCache.PendingUpdateOffset = 0;
> > > + *(VariableRuntimeCacheContext->PendingUpdate) = 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. This function will attempt
> > > + to write the given update (and any other pending updates) if the
> ReadLock is
> > > available. Otherwise, the
> > > + update is added as a pending update for the given variable store and it
> will
> > be
> > > flushed to the runtime cache
> > > + at the next opportunity the ReadLock is available.
> > > +
> > > + @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_SUCCESS The update was added as a pending
> update
> > > successfully. If the variable runtime
> > > + cache ReadLock was available, the runtime cache was
> > > updated successfully.
> > > + @retval EFI_UNSUPPORTED The volatile store to be updated is not
> > > initialized properly.
> > > +
> > > +**/
> > > +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) {
> > > + // The runtime cache may not be active or allocated yet.
> > > + // In either case, return EFI_SUCCESS instead of
> EFI_NOT_AVAILABLE_YET.
> > > + return EFI_SUCCESS;
> > > + }
> > > +
> > > + if (mVariableModuleGlobal-
> > > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdate == NULL
> ||
> > > + mVariableModuleGlobal-
> > > >VariableGlobal.VariableRuntimeCacheContext.ReadLock == NULL) {
> > > + return EFI_UNSUPPORTED;
> > > + }
> > > +
> > > + if (*(mVariableModuleGlobal-
> > > >VariableGlobal.VariableRuntimeCacheContext.PendingUpdate) &&
> > > + 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.PendingUpdate) = TRUE;
> > > +
> > > + if (*(mVariableModuleGlobal-
> > > >VariableGlobal.VariableRuntimeCacheContext.ReadLock) == FALSE) {
> > > + return FlushPendingRuntimeVariableCacheUpdates ();
> > > + }
> > > +
> > > + return EFI_SUCCESS;
> > > +}
> > > diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> > > index 5e24bc4a62..45814b8996 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_CONTEXT:
> > > + if (CommBufferPayloadSize < sizeof
> > >
> (SMM_VARIABLE_COMMUNICATE_RUNTIME_VARIABLE_CACHE_CONTEXT)
> ) {
> > > + 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;
> > > + 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 = FlushPendingRuntimeVariableCacheUpdates ();
> > > + 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/VariableSmmRuntimeDx
> e.c
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> > > index 0a1888e5ef..e236ddff33 100644
> > > ---
> > >
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> e.c
> > > +++
> > >
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
> 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
> > >
> > > **/
> > > @@ -39,6 +39,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > > #include <Guid/SmmVariableCommon.h>
> > >
> > > #include "PrivilegePolymorphic.h"
> > > +#include "VariableParsing.h"
> > >
> > > EFI_HANDLE mHandle = NULL;
> > > EFI_SMM_VARIABLE_PROTOCOL *mSmmVariable = NULL;
> > > @@ -46,8 +47,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 +119,72 @@ ReleaseLockOnlyAtBootTime (
> > > }
> > > }
> > >
> > > +/**
> > > + Return TRUE if ExitBootServices () has been called.
> > > +
> > > + @retval TRUE If ExitBootServices () has been called. FALSE if
> > ExitBootServices
> > > () has not been called.
> > > +**/
> > > +BOOLEAN
> > > +AtRuntime (
> > > + VOID
> > > + )
> > > +{
> > > + return EfiAtRuntime ();
> > > +}
> > > +
> >
> > AtRuntime() is kept here. But EfiAtRuntime() is still used elsewhere in this
> file.
> >
> > > +/**
> > > + 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
> > > +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.
> > >
> > > @@ -425,7 +503,169 @@ Done:
> > > }
> > >
> > > /**
> > > - This code finds variable in storage blocks (Volatile or Non-Volatile).
> > > + Signals SMM to synchronize any pending variable updates with the
> runtime
> > > cache(s).
> > > +
> > > +**/
> > > +VOID
> > > +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
> > > +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 (!EfiAtRuntime ()) {
> > > + FreePool (mVariableRuntimeHobCacheBuffer);
> > > + }
> > > + mVariableRuntimeHobCacheBuffer = NULL;
> > > + }
> > > +}
> > > +
> > > +/**
> > > + Finds the given variable in a runtime cache variable 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
> > > +FindVariableInRuntimeCache (
> > > + IN CHAR16 *VariableName,
> > > + IN EFI_GUID *VendorGuid,
> > > + OUT UINT32 *Attributes OPTIONAL,
> > > + IN OUT UINTN *DataSize,
> > > + OUT VOID *Data OPTIONAL
> >
> > I'm not sure the newly added OPTIONAL to last parameter 'Data' is correct.
> > Per my understanding on below code, 'Data' is always required when
> > mVariableRuntimeCachePendingUpdate is FALSE. In other words, 'Data'
> > passing with NULL has no meaning. In my opinion it's should not be
> > 'optional'.
> >
> > > + )
> > > +{
> > > + EFI_STATUS Status;
> > > + 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;
> > > + }
> > > +
> > > + //
> > > + // The UEFI specification restricts Runtime Services callers from
> invoking the
> > > same or certain other Runtime Service
> > > + // functions prior to completion and return from a previous Runtime
> Service
> > > call. These restrictions prevent
> > > + // a GetVariable () or GetNextVariable () call from being issued until a
> prior
> > call
> > > has returned. The runtime
> > > + // cache read lock should always be free when entering this function.
> > > + //
> > > + 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,
> > > mVariableAuthFormat);
> > > + if (!EFI_ERROR (Status)) {
> > > + break;
> > > + }
> > > + }
> > > +
> > > + if (!EFI_ERROR (Status)) {
> > > + //
> > > + // Get data size
> > > + //
> > > + TempDataSize = DataSizeOfVariable (RtPtrTrack.CurrPtr,
> > > mVariableAuthFormat);
> > > + ASSERT (TempDataSize != 0);
> > > +
> > > + if (*DataSize >= TempDataSize) {
> > > + if (Data == NULL) {
> > > + Status = EFI_INVALID_PARAMETER;
> > > + goto Done;
> > > + }
> > > +
> > > + CopyMem (Data, GetVariableDataPtr (RtPtrTrack.CurrPtr,
> > > mVariableAuthFormat), 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;
> > > +
> > > + return Status;
> > > +}
> > > +
> > > +/**
> > > + Finds the given variable in a variable store in SMM.
> > >
> > > Caution: This function may receive untrusted input.
> > > The data size is external input, so this function will validate it carefully to
> > avoid
> > > buffer overflow.
> > > @@ -437,20 +677,18 @@ Done:
> > > 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_SUCCESS Find the specified variable.
> > > - @retval EFI_NOT_FOUND Not found.
> > > - @retval EFI_BUFFER_TO_SMALL DataSize is too small for the result.
> > > + @retval EFI_NOT_FOUND The specified variable could not be
> found.
> > >
> > > **/
> > > EFI_STATUS
> > > -EFIAPI
> > > -RuntimeServiceGetVariable (
> > > +FindVariableInSmm (
> > > IN CHAR16 *VariableName,
> > > IN EFI_GUID *VendorGuid,
> > > OUT UINT32 *Attributes OPTIONAL,
> > > IN OUT UINTN *DataSize,
> > > - OUT VOID *Data
> > > + OUT VOID *Data OPTIONAL
> > > )
> > > {
> > > EFI_STATUS Status;
> > > @@ -474,8 +712,6 @@ RuntimeServiceGetVariable (
> > > 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.
> > > @@ -488,7 +724,7 @@ RuntimeServiceGetVariable (
> > > }
> > > PayloadSize = OFFSET_OF
> > > (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE, Name) +
> > > VariableNameSize + TempDataSize;
> > >
> > > - Status = InitCommunicateBuffer ((VOID **)&SmmVariableHeader,
> > PayloadSize,
> > > SMM_VARIABLE_FUNCTION_GET_VARIABLE);
> > > + Status = InitCommunicateBuffer ((VOID **) &SmmVariableHeader,
> > > PayloadSize, SMM_VARIABLE_FUNCTION_GET_VARIABLE);
> > > if (EFI_ERROR (Status)) {
> > > goto Done;
> > > }
> > > @@ -534,11 +770,58 @@ RuntimeServiceGetVariable (
> > > }
> > >
> > > Done:
> > > + return Status;
> > > +}
> > > +
> > > +/**
> > > + This code finds variable in storage blocks (Volatile or Non-Volatile).
> > > +
> > > + 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_INVALID_PARAMETER Invalid parameter.
> > > + @retval EFI_SUCCESS Find the specified variable.
> > > + @retval EFI_NOT_FOUND Not found.
> > > + @retval EFI_BUFFER_TO_SMALL DataSize is too small for the result.
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +EFIAPI
> > > +RuntimeServiceGetVariable (
> > > + IN CHAR16 *VariableName,
> > > + IN EFI_GUID *VendorGuid,
> > > + OUT UINT32 *Attributes OPTIONAL,
> > > + IN OUT UINTN *DataSize,
> > > + OUT VOID *Data
> > > + )
> > > +{
> > > + EFI_STATUS Status;
> > > +
> > > + if (VariableName == NULL || VendorGuid == NULL || DataSize ==
> NULL) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > + if (VariableName[0] == 0) {
> > > + return EFI_NOT_FOUND;
> > > + }
> > > +
> > > + AcquireLockOnlyAtBootTime (&mVariableServicesLock);
> > > + if (FeaturePcdGet (PcdEnableVariableRuntimeCache)) {
> > > + Status = FindVariableInRuntimeCache (VariableName, VendorGuid,
> > Attributes,
> > > DataSize, Data);
> > > + } else {
> > > + Status = FindVariableInSmm (VariableName, VendorGuid, Attributes,
> > DataSize,
> > > Data);
> > > + }
> > > ReleaseLockOnlyAtBootTime (&mVariableServicesLock);
> > > +
> > > return Status;
> > > }
> > >
> > > -
> > > /**
> > > This code Finds the Next available variable.
> > >
> > > @@ -870,6 +1153,17 @@ OnReadyToBoot (
> > > //
> > > SendCommunicateBuffer (0);
> > >
> > > + //
> > > + // Install the system configuration table for variable info data captured
> > > + //
> > > + if (FeaturePcdGet (PcdEnableVariableRuntimeCache) &&
> FeaturePcdGet
> > > (PcdVariableCollectStatistics)) {
> > > + if (mVariableAuthFormat) {
> > > + gBS->InstallConfigurationTable (&gEfiAuthenticatedVariableGuid,
> > > mVariableInfo);
> > > + } else {
> > > + gBS->InstallConfigurationTable (&gEfiVariableGuid, mVariableInfo);
> > > + }
> > > + }
> > > +
> > > gBS->CloseEvent (Event);
> > > }
> > >
> > > @@ -893,6 +1187,9 @@ VariableAddressChangeEvent (
> > > {
> > > EfiConvertPointer (0x0, (VOID **) &mVariableBuffer);
> > > EfiConvertPointer (0x0, (VOID **) &mSmmCommunication);
> > > + EfiConvertPointer (0x0, (VOID **)
> &mVariableRuntimeHobCacheBuffer);
> > > + EfiConvertPointer (0x0, (VOID **)
> &mVariableRuntimeNvCacheBuffer);
> > > + EfiConvertPointer (0x0, (VOID **)
> &mVariableRuntimeVolatileCacheBuffer);
> > > }
> > >
> > > /**
> > > @@ -969,6 +1266,159 @@ 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 The memory resources
> needed for
> > a
> > > CommBuffer are not available.
> > > + @retval Others Could not retrieve the size successfully.
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +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;
> > > + UINT8 *CommBuffer;
> > > +
> > > + SmmGetRuntimeCacheInfo = NULL;
> > > + CommBuffer = mVariableBuffer;
> > > +
> > > + if (TotalHobStorageSize == NULL || TotalNvStorageSize == NULL ||
> > > TotalVolatileStorageSize == NULL || AuthenticatedVariableUsage ==
> NULL) {
> > > + return EFI_INVALID_PARAMETER;
> > > + }
> > > +
> > > + if (CommBuffer == NULL) {
> > > + return EFI_OUT_OF_RESOURCES;
> > > + }
> > > +
> > > + AcquireLockOnlyAtBootTime (&mVariableServicesLock);
> > > +
> > > + CommSize = SMM_COMMUNICATE_HEADER_SIZE +
> > > SMM_VARIABLE_COMMUNICATE_HEADER_SIZE + sizeof
> > > (SMM_VARIABLE_COMMUNICATE_GET_RUNTIME_CACHE_INFO);
> > > + ZeroMem (CommBuffer, CommSize);
> > > +
> > > + 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 The memory resources needed
> for a
> > > CommBuffer are not available.
> > > + @retval Others Could not retrieve the size successfully.;
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +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;
> > > + UINT8 *CommBuffer;
> > > +
> > > + SmmRuntimeVarCacheContext = NULL;
> > > + CommBuffer = mVariableBuffer;
> > > +
> > > + if (CommBuffer == NULL) {
> > > + return EFI_OUT_OF_RESOURCES;
> > > + }
> > > +
> > > + 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)
> ;
> > > + ZeroMem (CommBuffer, CommSize);
> > > +
> > > + 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_CONTEXT;
> > > + 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 +1435,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 +1457,42 @@ SmmVariableReady (
> > > //
> > > mVariableBufferPhysical = mVariableBuffer;
> > >
> > > + if (FeaturePcdGet (PcdEnableVariableRuntimeCache)) {
> > > + DEBUG ((DEBUG_INFO, "Variable driver runtime cache is
> enabled.\n"));
> > > + //
> > > + // 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 = SendRuntimeVariableCacheContextToSmm ();
> > > + if (!EFI_ERROR (Status)) {
> > > + SyncRuntimeCache ();
> > > + }
> > > + }
> > > + }
> > > + }
> > > + if (EFI_ERROR (Status)) {
> > > + mVariableRuntimeHobCacheBuffer = NULL;
> > > + mVariableRuntimeNvCacheBuffer = NULL;
> > > + mVariableRuntimeVolatileCacheBuffer = NULL;
> > > + }
> > > + }
> >
> > Above code uses a little bit deep of nested if-statement, and is a little bit
> harder
> > to
> > read. What about flatten it? For example, one of the ways is using short-
> > circuiting
> > expression in one if-statement. Using 'goto' is another option.
> >
> > if (GetRuntimeCacheInfo (&mVariableRuntimeHobCacheBufferSize,
> > &mVariableRuntimeNvCacheBufferSize,
> > &mVariableRuntimeVolatileCacheBufferSize,
> > &mVariableAuthFormat) == EFI_SUCCESS &&
> > InitVariableCache (&mVariableRuntimeHobCacheBuffer,
> > &mVariableRuntimeHobCacheBufferSize) == EFI_SUCCESS
> &&
> > InitVariableCache (&mVariableRuntimeNvCacheBuffer,
> > &mVariableRuntimeNvCacheBufferSize) == EFI_SUCCESS &&
> > InitVariableCache (&mVariableRuntimeVolatileCacheBuffer,
> > &mVariableRuntimeVolatileCacheBufferSize) == EFI_SUCCESS
> &&
> > SendRuntimeVariableCacheContextToSmm () == EFI_SUCCESS) {
> > SyncRuntimeCache ();
> > } else {
> > Status = EFI_OUT_OF_RESOURCES;
> > mVariableRuntimeHobCacheBuffer = NULL;
> > mVariableRuntimeNvCacheBuffer = NULL;
> > mVariableRuntimeVolatileCacheBuffer = NULL;
> > }
> >
> > Your code is not wrong, except that the memory already allocated is not
> freed
> > due to later
> > error. But maybe there's no need to free in such fatal situation. Anyway,
> you
> > don't have
> > to do above change. I'll leave it to you to make decision.
> >
> > Regards,
> > Jian
> > > + ASSERT_EFI_ERROR (Status);
> > > + } else {
> > > + DEBUG ((DEBUG_INFO, "Variable driver runtime cache is
> disabled.\n"));
> > > + }
> > > +
> > > gRT->GetVariable = RuntimeServiceGetVariable;
> > > gRT->GetNextVariableName = RuntimeServiceGetNextVariableName;
> > > gRT->SetVariable = RuntimeServiceSetVariable;
> > > --
> > > 2.16.2.windows.1
> >
> >
> >
>
next prev parent reply other threads:[~2019-10-17 1:24 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-14 23:29 [PATCH V4 00/10] UEFI Variable SMI Reduction Kubacki, Michael A
2019-10-14 23:29 ` [PATCH V4 01/10] MdeModulePkg/Variable: Consolidate common parsing functions Kubacki, Michael A
2019-10-16 7:52 ` Wu, Hao A
2019-10-14 23:29 ` [PATCH V4 02/10] MdeModulePkg/Variable: Parameterize GetNextVariableInternal () stores Kubacki, Michael A
2019-10-16 7:53 ` Wu, Hao A
2019-10-14 23:29 ` [PATCH V4 03/10] MdeModulePkg/Variable: Parameterize VARIABLE_INFO_ENTRY buffer Kubacki, Michael A
2019-10-16 7:54 ` Wu, Hao A
2019-10-14 23:29 ` [PATCH V4 04/10] MdeModulePkg/Variable: Parameterize auth status in VariableParsing Kubacki, Michael A
2019-10-17 1:01 ` Wu, Hao A
2019-10-17 1:41 ` Kubacki, Michael A
2019-10-17 1:49 ` Wu, Hao A
2019-10-14 23:29 ` [PATCH V4 05/10] MdeModulePkg/Variable: Add a file for NV variable functions Kubacki, Michael A
2019-10-16 7:55 ` Wu, Hao A
2019-10-14 23:29 ` [PATCH V4 06/10] MdeModulePkg VariableInfo: Always consider RT DXE and SMM stats Kubacki, Michael A
2019-10-16 7:56 ` Wu, Hao A
2019-10-14 23:29 ` [PATCH V4 07/10] MdeModulePkg/Variable: Add RT GetVariable() cache support Kubacki, Michael A
2019-10-16 6:46 ` Wang, Jian J
[not found] ` <15CE0DB2DE3EB613.1607@groups.io>
2019-10-16 6:54 ` [edk2-devel] " Wang, Jian J
2019-10-17 1:24 ` Kubacki, Michael A [this message]
2019-10-17 1:47 ` Wang, Jian J
2019-10-16 7:56 ` Wu, Hao A
2019-10-16 16:44 ` Kubacki, Michael A
2019-10-17 14:23 ` Wang, Jian J
2019-10-17 17:44 ` Kubacki, Michael A
2019-10-14 23:29 ` [PATCH V4 08/10] MdeModulePkg/Variable: Add RT GetNextVariableName() " Kubacki, Michael A
2019-10-16 7:56 ` Wu, Hao A
2019-10-14 23:30 ` [PATCH V4 09/10] OvmfPkg: Disable variable runtime cache Kubacki, Michael A
2019-10-15 7:32 ` Laszlo Ersek
2019-10-14 23:30 ` [PATCH V4 10/10] MdeModulePkg: Enable variable runtime cache by default Kubacki, Michael A
2019-10-15 7:33 ` Laszlo Ersek
2019-10-16 7:57 ` Wu, Hao A
2019-10-15 0:49 ` [PATCH V4 00/10] UEFI Variable SMI Reduction Liming Gao
2019-10-15 16:15 ` 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=DM6PR11MB383499CE5A7DBBDD4CCF2017B56D0@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