Re: [PATCH V2 06/15] MdeModulePkg Variable: Remove CacheOffset in UpdateVariable()

[Laszlo Ersek <lersek@redhat.com>]

On 01/14/19 16:19, Star Zeng wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1323
> Merge EmuVariable and Real variable driver.
> 
> CacheOffset could be removed in UpdateVariable() after
> //
> // update the memory copy of Flash region.
> //
> CopyMem (
>   (UINT8 *)mNvVariableCache + CacheOffset,
>   (UINT8 *)NextVariable, VarSize
>    );
> 
> is moved to be before mVariableModuleGlobal->NonVolatileLastVariableOffset
> value is updated, like right before
> 
> mVariableModuleGlobal->NonVolatileLastVariableOffset +=
>   HEADER_ALIGN (VarSize);
> 
> 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>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> index 424f92a53757..4d524db30fec 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> @@ -2139,7 +2139,6 @@ UpdateVariable (
>    VARIABLE_POINTER_TRACK              *Variable;
>    VARIABLE_POINTER_TRACK              NvVariable;
>    VARIABLE_STORE_HEADER               *VariableStoreHeader;
> -  UINTN                               CacheOffset;
>    UINT8                               *BufferForMerge;
>    UINTN                               MergedBufSize;
>    BOOLEAN                             DataReady;
> @@ -2577,7 +2576,6 @@ UpdateVariable (
>      //
>      // Step 1:
>      //
> -    CacheOffset = mVariableModuleGlobal->NonVolatileLastVariableOffset;
>      Status = UpdateVariableStore (
>                 &mVariableModuleGlobal->VariableGlobal,
>                 FALSE,
> @@ -2643,6 +2641,11 @@ UpdateVariable (
>        goto Done;
>      }
>  
> +    //
> +    // update the memory copy of Flash region.
> +    //
> +    CopyMem ((UINT8 *)mNvVariableCache + mVariableModuleGlobal->NonVolatileLastVariableOffset, (UINT8 *)NextVariable, VarSize);
> +
>      mVariableModuleGlobal->NonVolatileLastVariableOffset += HEADER_ALIGN (VarSize);
>  
>      if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) != 0) {
> @@ -2653,10 +2656,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.
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo