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.w...@intel.com>
Cc: Hao Wu <hao.a...@intel.com>
Cc: Michael D Kinney <michael.d.kin...@intel.com>
Cc: Liming Gao <liming....@intel.com>
Cc: Ray Ni <ray...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.z...@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.


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to