Re: [PATCH 06/12] MdeModulePkg Variable: Add emulated variable NV mode support

["Zeng, Star" <star.zeng@intel.com>]

Hi Laszlo,

Yes, agree with the suggestions.
Two patches have been separated from this patch in V2 patch series at 
https://lists.01.org/pipermail/edk2-devel/2019-January/035015.html.


Thanks,
Star

On 2019/1/14 18:57, Laszlo Ersek wrote:
> Hi Star,
> 
> On 01/13/19 16:37, Star Zeng wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1323
>> Merge EmuVariable and Real variable driver.
>>
>> Add emulated variable NV mode support in real variable driver.
>> Platform can configure PcdEmuVariableNvModeEnable statically
>> (build time) or dynamically (boot time) to support emulated
>> variable NV mode.
>>
>> Then EmuVariableRuntimeDxe could be removed, the removal of
>> EmuVariableRuntimeDxe will be done after platforms are migrated
>> to use the merged variable driver.
>>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Hao Wu <hao.a.wu@intel.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Star Zeng <star.zeng@intel.com>
>> ---
>>   .../Universal/Variable/RuntimeDxe/Variable.c       | 319 +++++++++++++++------
>>   .../Universal/Variable/RuntimeDxe/Variable.h       |   1 +
>>   .../Universal/Variable/RuntimeDxe/VariableDxe.c    |  27 +-
>>   .../Variable/RuntimeDxe/VariableRuntimeDxe.inf     |   4 +-
>>   .../Universal/Variable/RuntimeDxe/VariableSmm.c    |  29 +-
>>   .../Universal/Variable/RuntimeDxe/VariableSmm.inf  |   4 +-
>>   6 files changed, 271 insertions(+), 113 deletions(-)
> 
> What I did for (briefly) reviewing this patch was the following. I
> fetched the branch you noted in the blurb, and the ran
> 
>    git show --color -W -b 9b509dea1227
> 
> Because, a large part of this patch just re-indents existent code, due
> to new conditionals.
> 
> My main goal with this review was to see whether "EmuNvMode==FALSE"
> would imply that the changes are a "no-op". Because OVMF, and the
> non-Xen ArmVirt DSC, will inherit the default FALSE setting for the PCD.
> So far, things look fine.
> 
> However, I noticed two small things that I believe would improve the
> readability of the patch. I suggest that you please split them out to
> separate patches, in the "preparation" section of the series. Namely:
> 
> 
>>
>> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>> index 424f92a53757..845276d891ae 100644
>> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>> @@ -34,6 +34,7 @@ VARIABLE_MODULE_GLOBAL  *mVariableModuleGlobal;
>>   
>>   ///
>>   /// Define a memory cache that improves the search performance for a variable.
>> +/// For EmuNvMode == TRUE, it will be equal to NonVolatileVariableBase.
>>   ///
>>   VARIABLE_STORE_HEADER  *mNvVariableCache      = NULL;
>>   
>> @@ -273,7 +274,7 @@ UpdateVariableStore (
>>     //
>>     // Check if the Data is Volatile.
>>     //
>> -  if (!Volatile) {
>> +  if (!Volatile && !mVariableModuleGlobal->VariableGlobal.EmuNvMode) {
>>       if (Fvb == NULL) {
>>         return EFI_UNSUPPORTED;
>>       }
>> @@ -296,17 +297,30 @@ UpdateVariableStore (
>>       // Data Pointer should point to the actual Address where data is to be
>>       // written.
>>       //
>> -    VolatileBase = (VARIABLE_STORE_HEADER *) ((UINTN) mVariableModuleGlobal->VariableGlobal.VolatileVariableBase);
>> -    if (SetByIndex) {
>> -      DataPtr += mVariableModuleGlobal->VariableGlobal.VolatileVariableBase;
>> -    }
>> +    if (Volatile) {
>> +      VolatileBase = (VARIABLE_STORE_HEADER *) ((UINTN) mVariableModuleGlobal->VariableGlobal.VolatileVariableBase);
>> +      if (SetByIndex) {
>> +        DataPtr += mVariableModuleGlobal->VariableGlobal.VolatileVariableBase;
>> +      }
>>   
>> -    if ((DataPtr + DataSize) > ((UINTN) ((UINT8 *) VolatileBase + VolatileBase->Size))) {
>> -      return EFI_OUT_OF_RESOURCES;
>> +      if ((DataPtr + DataSize) > ((UINTN) VolatileBase + VolatileBase->Size)) {
> 
> (1) Here the "if" statement is not just re-indented, but the controlling
> expression is also modified. The change looks OK to me (and I understand
> why you were motivated to simplify it); however, it would be better to
> split this change to a separate refactoring patch. That patch will be
> simple to review in isolation, and in turn it will make this patch
> easier to read, especially with "git show -b".
> 
>> +        return EFI_OUT_OF_RESOURCES;
>> +      }
>> +    } else {
>> +      //
>> +      // Emulated non-volatile variable mode.
>> +      //
>> +      if (SetByIndex) {
>> +        DataPtr += (UINTN) mNvVariableCache;
>> +      }
>> +
>> +      if ((DataPtr + DataSize) > ((UINTN) mNvVariableCache + mNvVariableCache->Size)) {
>> +        return EFI_OUT_OF_RESOURCES;
>> +      }
>>       }
>>   
>>       //
>> -    // If Volatile Variable just do a simple mem copy.
>> +    // If Volatile/Emulated Non-volatile Variable just do a simple mem copy.
>>       //
>>       CopyMem ((UINT8 *)(UINTN)DataPtr, Buffer, DataSize);
>>       return EFI_SUCCESS;
>> @@ -987,7 +1001,7 @@ Reclaim (
>>     CommonUserVariableTotalSize = 0;
>>     HwErrVariableTotalSize  = 0;
>>   
>> -  if (IsVolatile) {
>> +  if (IsVolatile || mVariableModuleGlobal->VariableGlobal.EmuNvMode) {
>>       //
>>       // Start Pointers for the variable.
>>       //
>> @@ -1155,13 +1169,21 @@ Reclaim (
>>       CurrPtr += NewVariableSize;
>>     }
>>   
>> -  if (IsVolatile) {
>> +  if (IsVolatile || mVariableModuleGlobal->VariableGlobal.EmuNvMode) {
>>       //
>> -    // If volatile variable store, just copy valid buffer.
>> +    // If volatile/emulated non-volatile variable store, just copy valid buffer.
>>       //
>>       SetMem ((UINT8 *) (UINTN) VariableBase, VariableStoreHeader->Size, 0xff);
>>       CopyMem ((UINT8 *) (UINTN) VariableBase, ValidBuffer, (UINTN) CurrPtr - (UINTN) ValidBuffer);
>>       *LastVariableOffset = (UINTN) CurrPtr - (UINTN) ValidBuffer;
>> +    if (!IsVolatile) {
>> +      //
>> +      // Emulated non-volatile variable mode.
>> +      //
>> +      mVariableModuleGlobal->HwErrVariableTotalSize = HwErrVariableTotalSize;
>> +      mVariableModuleGlobal->CommonVariableTotalSize = CommonVariableTotalSize;
>> +      mVariableModuleGlobal->CommonUserVariableTotalSize = CommonUserVariableTotalSize;
>> +    }
>>       Status  = EFI_SUCCESS;
>>     } else {
>>       //
>> @@ -1200,7 +1222,7 @@ Reclaim (
>>     }
>>   
>>   Done:
>> -  if (IsVolatile) {
>> +  if (IsVolatile || mVariableModuleGlobal->VariableGlobal.EmuNvMode) {
>>       FreePool (ValidBuffer);
>>     } else {
>>       //
>> @@ -2139,7 +2161,6 @@ UpdateVariable (
>>     VARIABLE_POINTER_TRACK              *Variable;
>>     VARIABLE_POINTER_TRACK              NvVariable;
>>     VARIABLE_STORE_HEADER               *VariableStoreHeader;
>> -  UINTN                               CacheOffset;
>>     UINT8                               *BufferForMerge;
>>     UINTN                               MergedBufSize;
>>     BOOLEAN                             DataReady;
>> @@ -2148,7 +2169,7 @@ UpdateVariable (
>>     BOOLEAN                             IsCommonUserVariable;
>>     AUTHENTICATED_VARIABLE_HEADER       *AuthVariable;
>>   
>> -  if (mVariableModuleGlobal->FvbInstance == NULL) {
>> +  if (mVariableModuleGlobal->FvbInstance == NULL && !mVariableModuleGlobal->VariableGlobal.EmuNvMode) {
>>       //
>>       // The FVB protocol is not ready, so the EFI_VARIABLE_WRITE_ARCH_PROTOCOL is not installed.
>>       //
>> @@ -2567,80 +2588,105 @@ UpdateVariable (
>>         }
>>         goto Done;
>>       }
>> -    //
>> -    // Four steps
>> -    // 1. Write variable header
>> -    // 2. Set variable state to header valid
>> -    // 3. Write variable data
>> -    // 4. Set variable state to valid
>> -    //
>> -    //
>> -    // Step 1:
>> -    //
>> -    CacheOffset = mVariableModuleGlobal->NonVolatileLastVariableOffset;
>> -    Status = UpdateVariableStore (
>> -               &mVariableModuleGlobal->VariableGlobal,
>> -               FALSE,
>> -               TRUE,
>> -               Fvb,
>> -               mVariableModuleGlobal->NonVolatileLastVariableOffset,
>> -               (UINT32) GetVariableHeaderSize (),
>> -               (UINT8 *) NextVariable
>> -               );
>>   
>> -    if (EFI_ERROR (Status)) {
>> -      goto Done;
>> -    }
>> +    if (!mVariableModuleGlobal->VariableGlobal.EmuNvMode) {
>> +      //
>> +      // Four steps
>> +      // 1. Write variable header
>> +      // 2. Set variable state to header valid
>> +      // 3. Write variable data
>> +      // 4. Set variable state to valid
>> +      //
>> +      //
>> +      // Step 1:
>> +      //
>> +      Status = UpdateVariableStore (
>> +                 &mVariableModuleGlobal->VariableGlobal,
>> +                 FALSE,
>> +                 TRUE,
>> +                 Fvb,
>> +                 mVariableModuleGlobal->NonVolatileLastVariableOffset,
>> +                 (UINT32) GetVariableHeaderSize (),
>> +                 (UINT8 *) NextVariable
>> +                 );
>>   
>> -    //
>> -    // Step 2:
>> -    //
>> -    NextVariable->State = VAR_HEADER_VALID_ONLY;
>> -    Status = UpdateVariableStore (
>> -               &mVariableModuleGlobal->VariableGlobal,
>> -               FALSE,
>> -               TRUE,
>> -               Fvb,
>> -               mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF (VARIABLE_HEADER, State),
>> -               sizeof (UINT8),
>> -               &NextVariable->State
>> -               );
>> +      if (EFI_ERROR (Status)) {
>> +        goto Done;
>> +      }
>>   
>> -    if (EFI_ERROR (Status)) {
>> -      goto Done;
>> -    }
>> -    //
>> -    // Step 3:
>> -    //
>> -    Status = UpdateVariableStore (
>> -               &mVariableModuleGlobal->VariableGlobal,
>> -               FALSE,
>> -               TRUE,
>> -               Fvb,
>> -               mVariableModuleGlobal->NonVolatileLastVariableOffset + GetVariableHeaderSize (),
>> -               (UINT32) (VarSize - GetVariableHeaderSize ()),
>> -               (UINT8 *) NextVariable + GetVariableHeaderSize ()
>> -               );
>> +      //
>> +      // Step 2:
>> +      //
>> +      NextVariable->State = VAR_HEADER_VALID_ONLY;
>> +      Status = UpdateVariableStore (
>> +                 &mVariableModuleGlobal->VariableGlobal,
>> +                 FALSE,
>> +                 TRUE,
>> +                 Fvb,
>> +                 mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF (VARIABLE_HEADER, State),
>> +                 sizeof (UINT8),
>> +                 &NextVariable->State
>> +                 );
>>   
>> -    if (EFI_ERROR (Status)) {
>> -      goto Done;
>> -    }
>> -    //
>> -    // Step 4:
>> -    //
>> -    NextVariable->State = VAR_ADDED;
>> -    Status = UpdateVariableStore (
>> -               &mVariableModuleGlobal->VariableGlobal,
>> -               FALSE,
>> -               TRUE,
>> -               Fvb,
>> -               mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF (VARIABLE_HEADER, State),
>> -               sizeof (UINT8),
>> -               &NextVariable->State
>> -               );
>> +      if (EFI_ERROR (Status)) {
>> +        goto Done;
>> +      }
>> +      //
>> +      // Step 3:
>> +      //
>> +      Status = UpdateVariableStore (
>> +                 &mVariableModuleGlobal->VariableGlobal,
>> +                 FALSE,
>> +                 TRUE,
>> +                 Fvb,
>> +                 mVariableModuleGlobal->NonVolatileLastVariableOffset + GetVariableHeaderSize (),
>> +                 (UINT32) (VarSize - GetVariableHeaderSize ()),
>> +                 (UINT8 *) NextVariable + GetVariableHeaderSize ()
>> +                 );
>>   
>> -    if (EFI_ERROR (Status)) {
>> -      goto Done;
>> +      if (EFI_ERROR (Status)) {
>> +        goto Done;
>> +      }
>> +      //
>> +      // Step 4:
>> +      //
>> +      NextVariable->State = VAR_ADDED;
>> +      Status = UpdateVariableStore (
>> +                 &mVariableModuleGlobal->VariableGlobal,
>> +                 FALSE,
>> +                 TRUE,
>> +                 Fvb,
>> +                 mVariableModuleGlobal->NonVolatileLastVariableOffset + OFFSET_OF (VARIABLE_HEADER, State),
>> +                 sizeof (UINT8),
>> +                 &NextVariable->State
>> +                 );
>> +
>> +      if (EFI_ERROR (Status)) {
>> +        goto Done;
>> +      }
>> +
>> +      //
>> +      // update the memory copy of Flash region.
>> +      //
>> +      CopyMem ((UINT8 *)mNvVariableCache + mVariableModuleGlobal->NonVolatileLastVariableOffset, (UINT8 *)NextVariable, VarSize);
> 
> (2) For this function too, please write a separate "prep" patch, that:
> 
> (a) removes the "CacheOffset" variable, and
> 
> (b) moves the CopyMem() call that currently uses "CacheOffset" to its
> final location -- i.e., just above
> 
>    NonVolatileLastVariableOffset += HEADER_ALIGN (VarSize);
> 
> -- and final parameter list as well. So that *this* patch need at most
> reindent the function call, ultimately.
> 
> Again, such a patch should not be hard to review in isolation, and it
> will improve the readability of this patch a lot.
> 
> 
> In summary, I *think* the current changes are all fine, from the
> perspective anyway that I mention at the top; but a reviewer's
> confidence is higher if the patch is easier to read.
> 
> Thanks,
> Laszlo
> 
> 
>> +    } else {
>> +      //
>> +      // Emulated non-volatile variable mode.
>> +      //
>> +      NextVariable->State = VAR_ADDED;
>> +      Status = UpdateVariableStore (
>> +                 &mVariableModuleGlobal->VariableGlobal,
>> +                 FALSE,
>> +                 TRUE,
>> +                 Fvb,
>> +                 mVariableModuleGlobal->NonVolatileLastVariableOffset,
>> +                 (UINT32) VarSize,
>> +                 (UINT8 *) NextVariable
>> +                 );
>> +
>> +      if (EFI_ERROR (Status)) {
>> +        goto Done;
>> +      }
>>       }
>>   
>>       mVariableModuleGlobal->NonVolatileLastVariableOffset += HEADER_ALIGN (VarSize);
>> @@ -2653,10 +2699,6 @@ UpdateVariable (
>>           mVariableModuleGlobal->CommonUserVariableTotalSize += HEADER_ALIGN (VarSize);
>>         }
>>       }
>> -    //
>> -    // update the memory copy of Flash region.
>> -    //
>> -    CopyMem ((UINT8 *)mNvVariableCache + CacheOffset, (UINT8 *)NextVariable, VarSize);
>>     } else {
>>       //
>>       // Create a volatile variable.
>> @@ -3877,6 +3919,93 @@ InitRealNonVolatileVariableStore (
>>   }
>>   
>>   /**
>> +  Init emulated non-volatile variable store.
>> +
>> +  @param[out] VariableStoreBase Output pointer to emulated non-volatile variable store base.
>> +
>> +  @retval EFI_SUCCESS           Function successfully executed.
>> +  @retval EFI_OUT_OF_RESOURCES  Fail to allocate enough memory resource.
>> +
>> +**/
>> +EFI_STATUS
>> +InitEmuNonVolatileVariableStore (
>> +  EFI_PHYSICAL_ADDRESS  *VariableStoreBase
>> +  )
>> +{
>> +  VARIABLE_STORE_HEADER *VariableStore;
>> +  UINT32                VariableStoreLength;
>> +  BOOLEAN               FullyInitializeStore;
>> +  UINT32                HwErrStorageSize;
>> +
>> +  FullyInitializeStore = TRUE;
>> +
>> +  VariableStoreLength = PcdGet32 (PcdVariableStoreSize);
>> +  ASSERT (sizeof (VARIABLE_STORE_HEADER) <= VariableStoreLength);
>> +
>> +  //
>> +  // Allocate memory for variable store.
>> +  //
>> +  if (PcdGet64 (PcdEmuVariableNvStoreReserved) == 0) {
>> +    VariableStore = (VARIABLE_STORE_HEADER *) AllocateRuntimePool (VariableStoreLength);
>> +    if (VariableStore == NULL) {
>> +      return EFI_OUT_OF_RESOURCES;
>> +    }
>> +  } else {
>> +    //
>> +    // A memory location has been reserved for the NV variable store.  Certain
>> +    // platforms may be able to preserve a memory range across system resets,
>> +    // thereby providing better NV variable emulation.
>> +    //
>> +    VariableStore =
>> +      (VARIABLE_STORE_HEADER *)(VOID*)(UINTN)
>> +        PcdGet64 (PcdEmuVariableNvStoreReserved);
>> +    if ((VariableStore->Size == VariableStoreLength) &&
>> +        (CompareGuid (&VariableStore->Signature, &gEfiAuthenticatedVariableGuid) ||
>> +         CompareGuid (&VariableStore->Signature, &gEfiVariableGuid)) &&
>> +        (VariableStore->Format == VARIABLE_STORE_FORMATTED) &&
>> +        (VariableStore->State == VARIABLE_STORE_HEALTHY)) {
>> +      DEBUG((
>> +        DEBUG_INFO,
>> +        "Variable Store reserved at %p appears to be valid\n",
>> +        VariableStore
>> +        ));
>> +      FullyInitializeStore = FALSE;
>> +    }
>> +  }
>> +
>> +  if (FullyInitializeStore) {
>> +    SetMem (VariableStore, VariableStoreLength, 0xff);
>> +    //
>> +    // Use gEfiAuthenticatedVariableGuid for potential auth variable support.
>> +    //
>> +    CopyGuid (&VariableStore->Signature, &gEfiAuthenticatedVariableGuid);
>> +    VariableStore->Size       = VariableStoreLength;
>> +    VariableStore->Format     = VARIABLE_STORE_FORMATTED;
>> +    VariableStore->State      = VARIABLE_STORE_HEALTHY;
>> +    VariableStore->Reserved   = 0;
>> +    VariableStore->Reserved1  = 0;
>> +  }
>> +
>> +  *VariableStoreBase = (EFI_PHYSICAL_ADDRESS) (UINTN) VariableStore;
>> +
>> +  HwErrStorageSize = PcdGet32 (PcdHwErrStorageSize);
>> +
>> +  //
>> +  // Note that in EdkII variable driver implementation, Hardware Error Record type variable
>> +  // is stored with common variable in the same NV region. So the platform integrator should
>> +  // ensure that the value of PcdHwErrStorageSize is less than the value of
>> +  // (VariableStoreLength - sizeof (VARIABLE_STORE_HEADER)).
>> +  //
>> +  ASSERT (HwErrStorageSize < (VariableStoreLength - sizeof (VARIABLE_STORE_HEADER)));
>> +
>> +  mVariableModuleGlobal->CommonVariableSpace = ((UINTN) VariableStoreLength - sizeof (VARIABLE_STORE_HEADER) - HwErrStorageSize);
>> +  mVariableModuleGlobal->CommonMaxUserVariableSpace = mVariableModuleGlobal->CommonVariableSpace;
>> +  mVariableModuleGlobal->CommonRuntimeVariableSpace = mVariableModuleGlobal->CommonVariableSpace;
>> +
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +/**
>>     Init non-volatile variable store.
>>   
>>     @retval EFI_SUCCESS           Function successfully executed.
>> @@ -3895,9 +4024,19 @@ InitNonVolatileVariableStore (
>>     UINTN                                 VariableSize;
>>     EFI_STATUS                            Status;
>>   
>> -  Status = InitRealNonVolatileVariableStore (&VariableStoreBase);
>> -  if (EFI_ERROR (Status)) {
>> -    return Status;
>> +  if (!PcdGetBool (PcdEmuVariableNvModeEnable)) {
>> +    Status = InitRealNonVolatileVariableStore (&VariableStoreBase);
>> +    if (EFI_ERROR (Status)) {
>> +      return Status;
>> +    }
>> +    mVariableModuleGlobal->VariableGlobal.EmuNvMode = FALSE;
>> +  } else {
>> +    Status = InitEmuNonVolatileVariableStore (&VariableStoreBase);
>> +    if (EFI_ERROR (Status)) {
>> +      return Status;
>> +    }
>> +    mVariableModuleGlobal->VariableGlobal.EmuNvMode = TRUE;
>> +    DEBUG ((DEBUG_INFO, "Variable driver will work at emulated non-volatile variable mode!\n"));
>>     }
>>   
>>     mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase = VariableStoreBase;
>> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
>> index 566e7268d187..fdc92b2b017c 100644
>> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
>> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
>> @@ -92,6 +92,7 @@ typedef struct {
>>     UINT32                ReentrantState;
>>     BOOLEAN               AuthFormat;
>>     BOOLEAN               AuthSupport;
>> +  BOOLEAN               EmuNvMode;
>>   } VARIABLE_GLOBAL;
>>   
>>   typedef struct {
>> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
>> index 5131e6f351e4..7d5c8b3f842d 100644
>> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
>> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
>> @@ -533,16 +533,23 @@ VariableServiceInitialize (
>>                     );
>>     ASSERT_EFI_ERROR (Status);
>>   
>> -  //
>> -  // Register FtwNotificationEvent () notify function.
>> -  //
>> -  EfiCreateProtocolNotifyEvent (
>> -    &gEfiFaultTolerantWriteProtocolGuid,
>> -    TPL_CALLBACK,
>> -    FtwNotificationEvent,
>> -    (VOID *)SystemTable,
>> -    &mFtwRegistration
>> -    );
>> +  if (!PcdGetBool (PcdEmuVariableNvModeEnable)) {
>> +    //
>> +    // Register FtwNotificationEvent () notify function.
>> +    //
>> +    EfiCreateProtocolNotifyEvent (
>> +      &gEfiFaultTolerantWriteProtocolGuid,
>> +      TPL_CALLBACK,
>> +      FtwNotificationEvent,
>> +      (VOID *)SystemTable,
>> +      &mFtwRegistration
>> +      );
>> +  } else {
>> +    //
>> +    // Emulated non-volatile variable mode does not depend on FVB and FTW.
>> +    //
>> +    VariableWriteServiceInitializeDxe ();
>> +  }
>>   
>>     Status = gBS->CreateEventEx (
>>                     EVT_NOTIFY_SIGNAL,
>> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>> index 7ef8a97f5d6a..273067753c25 100644
>> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>> @@ -9,7 +9,7 @@
>>   #  This external input must be validated carefully to avoid security issues such as
>>   #  buffer overflow or integer overflow.
>>   #
>> -# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>> +# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>>   # This program and the accompanying materials
>>   # are licensed and made available under the terms and conditions of the BSD License
>>   # which accompanies this distribution. The full text of the license may be found at
>> @@ -131,6 +131,8 @@ [Pcd]
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdMaxUserNvVariableSpaceSize           ## CONSUMES
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdBoottimeReservedNvVariableSpaceSize  ## CONSUMES
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdReclaimVariableSpaceAtEndOfDxe  ## CONSUMES
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable         ## SOMETIMES_CONSUMES
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved      ## SOMETIMES_CONSUMES
>>   
>>   [FeaturePcd]
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics  ## CONSUMES # statistic the information of variable.
>> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
>> index e63af812534e..d47493c891e5 100644
>> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
>> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
>> @@ -1034,17 +1034,24 @@ VariableServiceInitialize (
>>                       );
>>     ASSERT_EFI_ERROR (Status);
>>   
>> -  //
>> -  // Register FtwNotificationEvent () notify function.
>> -  //
>> -  Status = gSmst->SmmRegisterProtocolNotify (
>> -                    &gEfiSmmFaultTolerantWriteProtocolGuid,
>> -                    SmmFtwNotificationEvent,
>> -                    &SmmFtwRegistration
>> -                    );
>> -  ASSERT_EFI_ERROR (Status);
>> -
>> -  SmmFtwNotificationEvent (NULL, NULL, NULL);
>> +  if (!PcdGetBool (PcdEmuVariableNvModeEnable)) {
>> +    //
>> +    // Register FtwNotificationEvent () notify function.
>> +    //
>> +    Status = gSmst->SmmRegisterProtocolNotify (
>> +                      &gEfiSmmFaultTolerantWriteProtocolGuid,
>> +                      SmmFtwNotificationEvent,
>> +                      &SmmFtwRegistration
>> +                      );
>> +    ASSERT_EFI_ERROR (Status);
>> +
>> +    SmmFtwNotificationEvent (NULL, NULL, NULL);
>> +  } else {
>> +    //
>> +    // Emulated non-volatile variable mode does not depend on FVB and FTW.
>> +    //
>> +    VariableWriteServiceInitializeSmm ();
>> +  }
>>   
>>     return EFI_SUCCESS;
>>   }
>> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
>> index db7d220e06df..30d446d06e0d 100644
>> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
>> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
>> @@ -18,7 +18,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 - 2016, Intel Corporation. All rights reserved.<BR>
>> +# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
>>   # This program and the accompanying materials
>>   # are licensed and made available under the terms and conditions of the BSD License
>>   # which accompanies this distribution. The full text of the license may be found at
>> @@ -133,6 +133,8 @@ [Pcd]
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdMaxUserNvVariableSpaceSize           ## CONSUMES
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdBoottimeReservedNvVariableSpaceSize  ## CONSUMES
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdReclaimVariableSpaceAtEndOfDxe   ## CONSUMES
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable          ## SOMETIMES_CONSUMES
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved       ## SOMETIMES_CONSUMES
>>   
>>   [FeaturePcd]
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics        ## CONSUMES  # statistic the information of variable.
>>