On 2019/1/15 17:58, Laszlo Ersek wrote:
On 01/15/19 09:04, Wang, Jian J wrote:
Star,
Just a tiny comment below. With it's addressed,
Reviewed-by: Jian J Wang <[email protected]>
-----Original Message-----
From: Zeng, Star
Sent: Monday, January 14, 2019 11:20 PM
To: [email protected]
Cc: Zeng, Star <[email protected]>; Wang, Jian J <[email protected]>;
Wu, Hao A <[email protected]>; Laszlo Ersek <[email protected]>
Subject: [PATCH V2 06/15] MdeModulePkg Variable: Remove CacheOffset in
UpdateVariable()
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 <[email protected]>
Cc: Hao Wu <[email protected]>
Cc: Laszlo Ersek <[email protected]>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <[email protected]>
---
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.
+ //
The first character of comment is not capitalized.
In this particular case, I agree that such a change can be added to the
patch. While it does not pertain to the actual work done here, the patch
itself is so small (which is a good thing!) that the comment
capitalization would not cause confusion.
Still I suggest mentioning it briefly in the commit message too.
Make sense, I can do that.
Thanks,
Star
Thanks
Laszlo
+ 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.
--
2.7.0.windows.1
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
