Re: [PATCH V3 04/17] MdeModulePkg Variable: Not get NV PCD in VariableWriteServiceInitialize

["Wu, Hao A" <hao.a.wu@intel.com>]

Reviewed-by: Hao Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


> -----Original Message-----
> From: Zeng, Star
> Sent: Tuesday, January 15, 2019 6:29 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star; Wang, Jian J; Wu, Hao A
> Subject: [PATCH V3 04/17] MdeModulePkg Variable: Not get NV PCD in
> VariableWriteServiceInitialize
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1323
> Merge EmuVariable and Real variable driver.
> 
> Add macro NV_STORAGE_VARIABLE_BASE.
> Not get NV PCD in VariableWriteServiceInitialize, but in
> FtwNotificationEvent/SmmFtwNotificationEvent, then
> VariableWriteServiceInitialize could be not aware the NV
> storage is real or emulated.
> 
> This patch prepares for adding emulated variable NV mode
> support in VariableRuntimeDxe.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  .../Universal/Variable/RuntimeDxe/Variable.c         | 20 ++------------------
>  .../Universal/Variable/RuntimeDxe/Variable.h         |  9 +++++++--
>  .../Universal/Variable/RuntimeDxe/VariableDxe.c      | 13 ++++++++-----
>  .../Universal/Variable/RuntimeDxe/VariableSmm.c      | 15 ++++++++++-----
>  4 files changed, 27 insertions(+), 30 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index 0b675c8f36df..424f92a53757 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -3770,10 +3770,7 @@ InitRealNonVolatileVariableStore (
>      return EFI_OUT_OF_RESOURCES;
>    }
> 
> -  NvStorageBase = (EFI_PHYSICAL_ADDRESS) PcdGet64
> (PcdFlashNvStorageVariableBase64);
> -  if (NvStorageBase == 0) {
> -    NvStorageBase = (EFI_PHYSICAL_ADDRESS) PcdGet32
> (PcdFlashNvStorageVariableBase);
> -  }
> +  NvStorageBase = NV_STORAGE_VARIABLE_BASE;
>    ASSERT (NvStorageBase != 0);
> 
>    //
> @@ -4027,7 +4024,7 @@ FlushHobVariableToFlash (
>  }
> 
>  /**
> -  Initializes variable write service after FTW was ready.
> +  Initializes variable write service.
> 
>    @retval EFI_SUCCESS          Function successfully executed.
>    @retval Others               Fail to initialize the variable service.
> @@ -4041,23 +4038,10 @@ VariableWriteServiceInitialize (
>    EFI_STATUS                      Status;
>    UINTN                           Index;
>    UINT8                           Data;
> -  EFI_PHYSICAL_ADDRESS            VariableStoreBase;
> -  EFI_PHYSICAL_ADDRESS            NvStorageBase;
>    VARIABLE_ENTRY_PROPERTY         *VariableEntry;
> 
>    AcquireLockOnlyAtBootTime(&mVariableModuleGlobal-
> >VariableGlobal.VariableServicesLock);
> 
> -  NvStorageBase = (EFI_PHYSICAL_ADDRESS) PcdGet64
> (PcdFlashNvStorageVariableBase64);
> -  if (NvStorageBase == 0) {
> -    NvStorageBase = (EFI_PHYSICAL_ADDRESS) PcdGet32
> (PcdFlashNvStorageVariableBase);
> -  }
> -  VariableStoreBase = NvStorageBase + (mNvFvHeaderCache-
> >HeaderLength);
> -
> -  //
> -  // Let NonVolatileVariableBase point to flash variable store base directly
> after FTW ready.
> -  //
> -  mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase =
> VariableStoreBase;
> -
>    //
>    // Check if the free area is really free.
>    //
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> index 90507a8e64f6..d128d1a9c680 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
> @@ -2,7 +2,7 @@
>    The internal header file includes the common header files, defines
>    internal structure and functions used by Variable modules.
> 
> -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
> @@ -46,6 +46,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> 
>  #include "PrivilegePolymorphic.h"
> 
> +#define NV_STORAGE_VARIABLE_BASE (EFI_PHYSICAL_ADDRESS) \
> +                                   (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0 ? \
> +                                    PcdGet64 (PcdFlashNvStorageVariableBase64) : \
> +                                    PcdGet32 (PcdFlashNvStorageVariableBase))
> +
>  #define EFI_VARIABLE_ATTRIBUTES_MASK (EFI_VARIABLE_NON_VOLATILE
> | \
>                                        EFI_VARIABLE_BOOTSERVICE_ACCESS | \
>                                        EFI_VARIABLE_RUNTIME_ACCESS | \
> @@ -473,7 +478,7 @@ GetMaxVariableSize (
>    );
> 
>  /**
> -  Initializes variable write service after FVB was ready.
> +  Initializes variable write service.
> 
>    @retval EFI_SUCCESS          Function successfully executed.
>    @retval Others               Fail to initialize the variable service.
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> index f1304c9dbccc..30165ce4dc95 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> @@ -382,13 +382,17 @@ FtwNotificationEvent (
>      ASSERT (PcdGet32 (PcdFlashNvStorageVariableSize) <= FtwMaxBlockSize);
>    }
> 
> +  NvStorageVariableBase = NV_STORAGE_VARIABLE_BASE;
> +  VariableStoreBase = NvStorageVariableBase + mNvFvHeaderCache-
> >HeaderLength;
> +
> +  //
> +  // Let NonVolatileVariableBase point to flash variable store base directly
> after FTW ready.
> +  //
> +  mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase =
> VariableStoreBase;
> +
>    //
>    // Find the proper FVB protocol for variable.
>    //
> -  NvStorageVariableBase = (EFI_PHYSICAL_ADDRESS) PcdGet64
> (PcdFlashNvStorageVariableBase64);
> -  if (NvStorageVariableBase == 0) {
> -    NvStorageVariableBase = (EFI_PHYSICAL_ADDRESS) PcdGet32
> (PcdFlashNvStorageVariableBase);
> -  }
>    Status = GetFvbInfoByAddress (NvStorageVariableBase, NULL,
> &FvbProtocol);
>    if (EFI_ERROR (Status)) {
>      return ;
> @@ -398,7 +402,6 @@ FtwNotificationEvent (
>    //
>    // Mark the variable storage region of the FLASH as RUNTIME.
>    //
> -  VariableStoreBase   = NvStorageVariableBase + mNvFvHeaderCache-
> >HeaderLength;
>    VariableStoreLength = mNvVariableCache->Size;
>    BaseAddress = VariableStoreBase & (~EFI_PAGE_MASK);
>    Length      = VariableStoreLength + (VariableStoreBase - BaseAddress);
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> index 623badb0c755..2401ad912ef1 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
> @@ -14,7 +14,7 @@
>    VariableServiceSetVariable(), VariableServiceQueryVariableInfo(),
> ReclaimForOS(),
>    SmmVariableGetStatistics() should also do validation based on its own
> knowledge.
> 
> -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
> @@ -864,6 +864,7 @@ SmmFtwNotificationEvent (
>    )
>  {
>    EFI_STATUS                              Status;
> +  EFI_PHYSICAL_ADDRESS                    VariableStoreBase;
>    EFI_SMM_FIRMWARE_VOLUME_BLOCK_PROTOCOL  *FvbProtocol;
>    EFI_SMM_FAULT_TOLERANT_WRITE_PROTOCOL   *FtwProtocol;
>    EFI_PHYSICAL_ADDRESS                    NvStorageVariableBase;
> @@ -886,13 +887,17 @@ SmmFtwNotificationEvent (
>      ASSERT (PcdGet32 (PcdFlashNvStorageVariableSize) <= FtwMaxBlockSize);
>    }
> 
> +  NvStorageVariableBase = NV_STORAGE_VARIABLE_BASE;
> +  VariableStoreBase = NvStorageVariableBase + mNvFvHeaderCache-
> >HeaderLength;
> +
> +  //
> +  // Let NonVolatileVariableBase point to flash variable store base directly
> after FTW ready.
> +  //
> +  mVariableModuleGlobal->VariableGlobal.NonVolatileVariableBase =
> VariableStoreBase;
> +
>    //
>    // Find the proper FVB protocol for variable.
>    //
> -  NvStorageVariableBase = (EFI_PHYSICAL_ADDRESS) PcdGet64
> (PcdFlashNvStorageVariableBase64);
> -  if (NvStorageVariableBase == 0) {
> -    NvStorageVariableBase = (EFI_PHYSICAL_ADDRESS) PcdGet32
> (PcdFlashNvStorageVariableBase);
> -  }
>    Status = GetFvbInfoByAddress (NvStorageVariableBase, NULL,
> &FvbProtocol);
>    if (EFI_ERROR (Status)) {
>      return EFI_NOT_FOUND;
> --
> 2.7.0.windows.1