.../Universal/Variable/RuntimeDxe/Variable.c | 45 ++----------------- 1 file changed, 4 insertions(+), 41 deletions(-)
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4597
When creating a new variable, skip marking VAR_HEADER_VALID_ONLY so that
variable header + data update can be merged into one flash write. This
will greatly reduce the time taken for updating a variable and thus
increase performance. Removing VAR_HEADER_VALID_ONLY marking doesn't
have any function impact since it's not used by current code to detect
variable header + data corruption.
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Gao Cheng <gao.cheng@intel.com>
---
.../Universal/Variable/RuntimeDxe/Variable.c | 45 ++-----------------
1 file changed, 4 insertions(+), 41 deletions(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 7a1331255b..3c360481f4 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -2168,11 +2168,9 @@ UpdateVariable (
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
+ // Two steps
+ // 1. Write variable header and data
+ // 2. Set variable state to valid
//
//
// Step 1:
@@ -2183,7 +2181,7 @@ UpdateVariable (
TRUE,
Fvb,
mVariableModuleGlobal->NonVolatileLastVariableOffset,
- (UINT32)GetVariableHeaderSize (AuthFormat),
+ (UINT32)VarSize,
(UINT8 *)NextVariable
);
@@ -2194,41 +2192,6 @@ UpdateVariable (
//
// 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 3:
- //
- Status = UpdateVariableStore (
- &mVariableModuleGlobal->VariableGlobal,
- FALSE,
- TRUE,
- Fvb,
- mVariableModuleGlobal->NonVolatileLastVariableOffset + GetVariableHeaderSize (AuthFormat),
- (UINT32)(VarSize - GetVariableHeaderSize (AuthFormat)),
- (UINT8 *)NextVariable + GetVariableHeaderSize (AuthFormat)
- );
-
- if (EFI_ERROR (Status)) {
- goto Done;
- }
-
- //
- // Step 4:
- //
NextVariable->State = VAR_ADDED;
Status = UpdateVariableStore (
&mVariableModuleGlobal->VariableGlobal,
--
2.42.0.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111193): https://edk2.groups.io/g/devel/message/111193
Mute This Topic: https://groups.io/mt/102579876/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Cheng: I think this change is fine. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn> Thanks Liming > -----邮件原件----- > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Gao > 发送时间: 2023年11月14日 16:24 > 收件人: devel@edk2.groups.io > 抄送: Gao Cheng <gao.cheng@intel.com>; Liming Gao > <gaoliming@byosoft.com.cn>; Ray Ni <ray.ni@intel.com> > 主题: [edk2-devel] [PATCH] MdeModulePkg/Variable: Merge variable header > + data update into one step > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4597 > > When creating a new variable, skip marking VAR_HEADER_VALID_ONLY so > that > variable header + data update can be merged into one flash write. This > will greatly reduce the time taken for updating a variable and thus > increase performance. Removing VAR_HEADER_VALID_ONLY marking doesn't > have any function impact since it's not used by current code to detect > variable header + data corruption. > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Ray Ni <ray.ni@intel.com> > Signed-off-by: Gao Cheng <gao.cheng@intel.com> > --- > .../Universal/Variable/RuntimeDxe/Variable.c | 45 ++----------------- > 1 file changed, 4 insertions(+), 41 deletions(-) > > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > index 7a1331255b..3c360481f4 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > @@ -2168,11 +2168,9 @@ UpdateVariable ( > > 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 > + // Two steps > + // 1. Write variable header and data > + // 2. Set variable state to valid > // > // > // Step 1: > @@ -2183,7 +2181,7 @@ UpdateVariable ( > TRUE, > Fvb, > > mVariableModuleGlobal->NonVolatileLastVariableOffset, > - (UINT32)GetVariableHeaderSize (AuthFormat), > + (UINT32)VarSize, > (UINT8 *)NextVariable > ); > > @@ -2194,41 +2192,6 @@ UpdateVariable ( > // > // 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 3: > - // > - Status = UpdateVariableStore ( > - &mVariableModuleGlobal->VariableGlobal, > - FALSE, > - TRUE, > - Fvb, > - > mVariableModuleGlobal->NonVolatileLastVariableOffset + > GetVariableHeaderSize (AuthFormat), > - (UINT32)(VarSize - GetVariableHeaderSize > (AuthFormat)), > - (UINT8 *)NextVariable + GetVariableHeaderSize > (AuthFormat) > - ); > - > - if (EFI_ERROR (Status)) { > - goto Done; > - } > - > - // > - // Step 4: > - // > NextVariable->State = VAR_ADDED; > Status = UpdateVariableStore ( > > &mVariableModuleGlobal->VariableGlobal, > -- > 2.42.0.windows.2 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111332): https://edk2.groups.io/g/devel/message/111332 Mute This Topic: https://groups.io/mt/102639131/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
+Jiewen (I seem to remember Jiewen co-authored a whitepaper on edk2 variables; I could be wrong) one more comment below: On 11/14/23 09:23, Gao wrote: > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4597 > > When creating a new variable, skip marking VAR_HEADER_VALID_ONLY so that > variable header + data update can be merged into one flash write. This > will greatly reduce the time taken for updating a variable and thus > increase performance. Removing VAR_HEADER_VALID_ONLY marking doesn't > have any function impact since it's not used by current code to detect > variable header + data corruption. That would have been my question; thanks for spelling it out in advance (in the commit message). I wouldn't like to start reviewing this right now, but if you don't get a review in a week or so, feel free to ping me; I'll try to dig into it. Laszlo > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Ray Ni <ray.ni@intel.com> > Signed-off-by: Gao Cheng <gao.cheng@intel.com> > --- > .../Universal/Variable/RuntimeDxe/Variable.c | 45 ++----------------- > 1 file changed, 4 insertions(+), 41 deletions(-) > > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > index 7a1331255b..3c360481f4 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > @@ -2168,11 +2168,9 @@ UpdateVariable ( > > 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 > + // Two steps > + // 1. Write variable header and data > + // 2. Set variable state to valid > // > // > // Step 1: > @@ -2183,7 +2181,7 @@ UpdateVariable ( > TRUE, > Fvb, > mVariableModuleGlobal->NonVolatileLastVariableOffset, > - (UINT32)GetVariableHeaderSize (AuthFormat), > + (UINT32)VarSize, > (UINT8 *)NextVariable > ); > > @@ -2194,41 +2192,6 @@ UpdateVariable ( > // > // 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 3: > - // > - Status = UpdateVariableStore ( > - &mVariableModuleGlobal->VariableGlobal, > - FALSE, > - TRUE, > - Fvb, > - mVariableModuleGlobal->NonVolatileLastVariableOffset + GetVariableHeaderSize (AuthFormat), > - (UINT32)(VarSize - GetVariableHeaderSize (AuthFormat)), > - (UINT8 *)NextVariable + GetVariableHeaderSize (AuthFormat) > - ); > - > - if (EFI_ERROR (Status)) { > - goto Done; > - } > - > - // > - // Step 4: > - // > NextVariable->State = VAR_ADDED; > Status = UpdateVariableStore ( > &mVariableModuleGlobal->VariableGlobal, -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111204): https://edk2.groups.io/g/devel/message/111204 Mute This Topic: https://groups.io/mt/102579876/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.