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

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

Michael:
  Can the logic be simplified to only check this condition 'Attributes & EFI_VARIABLE_NON_VOLATILE) != 0'?

Thanks
Liming
>-----Original Message-----
>From: Kubacki, Michael A
>Sent: Monday, November 11, 2019 2:11 PM
>To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io
>Cc: 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: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix
>volatile variable RT cache update logic
>
>After a new volatile variable is written successfully, Runtime Services
>GetVariable () may return
>EFI_NOT_FOUND for that variable.
>
>Thanks,
>Michael
>
>> -----Original Message-----
>> From: Gao, Liming <liming.gao@intel.com>
>> Sent: Sunday, November 10, 2019 10:08 PM
>> To: devel@edk2.groups.io; Kubacki, Michael A
>> <michael.a.kubacki@intel.com>
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wang, Jian J
>> <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Liming
>> <liming.gao@intel.com>
>> Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix
>> volatile variable RT cache update logic
>>
>> 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.VariableRuntimeVolatileCa
>c
>> h
>> >e);
>> >-    } else {
>> >+    VolatileCacheInstance = &(mVariableModuleGlobal-
>> >>VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCa
>c
>> h
>> >e);
>> >+    if ((Variable->CurrPtr != NULL && !Variable->Volatile) ||
>> >+ (Attributes &
>> >EFI_VARIABLE_NON_VOLATILE) != 0) {
>> >       VolatileCacheInstance = &(mVariableModuleGlobal-
>> >>VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache);
>> >     }
>> >
>> >--
>> >2.16.2.windows.1
>> >
>> >
>> >
>>
>