Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix volatile variable RT cache update logic

["Liming Gao" <liming.gao@intel.com>]

Michael:
  What real issue is caused by this issue?

Thanks
Liming
>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Kubacki, Michael A
>Sent: Monday, November 11, 2019 1:16 PM
>To: devel@edk2.groups.io
>Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
><michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu,
>Hao A <hao.a.wu@intel.com>
>Subject: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix volatile
>variable RT cache update logic
>
>REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2333
>
>During a SetVariable () invocation, UpdateVariable () is called.
>UpdateVariable () contains logic to determine whether a volatile or
>non-volatile UEFI variable was set so the corresponding runtime
>cache can be updated to reflect the change. The current logic simply
>evaluates Variable->Volatile to determine which runtime cache should
>be updated.
>
>The problem is Variable->Volatile does not always reflect whether a
>volatile variable is being set. Variable->Volatile is set to TRUE
>only in the case a pre-existing variable is found in the volatile
>variable store. Therefore, the value is FALSE when a new volatile
>variable is written.
>
>This change updates the logic to take this into account. If a new
>variable is written successfully, the Attributes will accurately
>reflect whether the variable is non-volatile. If a pre-existing
>variable is modified, the Volatile field will reflect the type of
>variable (Attributes are not reliable; e.g. 0x0 indicates deletion).
>
>Cc: Liming Gao <liming.gao@intel.com>
>Cc: Michael D Kinney <michael.d.kinney@intel.com>
>Cc: Jian J Wang <jian.j.wang@intel.com>
>Cc: Hao A Wu <hao.a.wu@intel.com>
>Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
>---
> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
>diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>index 29d6aca993..75d33ff724 100644
>--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
>@@ -2296,9 +2296,8 @@ UpdateVariable (
>
> Done:
>   if (!EFI_ERROR (Status)) {
>-    if (Variable->Volatile) {
>-      VolatileCacheInstance = &(mVariableModuleGlobal-
>>VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCach
>e);
>-    } else {
>+    VolatileCacheInstance = &(mVariableModuleGlobal-
>>VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCach
>e);
>+    if ((Variable->CurrPtr != NULL && !Variable->Volatile) || (Attributes &
>EFI_VARIABLE_NON_VOLATILE) != 0) {
>       VolatileCacheInstance = &(mVariableModuleGlobal-
>>VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache);
>     }
>
>--
>2.16.2.windows.1
>
>
>