From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mx.groups.io with SMTP id smtpd.web12.1406.1573602048234504138 for ; Tue, 12 Nov 2019 15:40:48 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.24, mailfrom: michael.a.kubacki@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Nov 2019 15:40:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,298,1569308400"; d="scan'208";a="216203283" Received: from makuback-desk1.amr.corp.intel.com ([10.7.159.162]) by orsmga002.jf.intel.com with ESMTP; 12 Nov 2019 15:40:47 -0800 From: "Kubacki, Michael A" To: devel@edk2.groups.io Cc: Liming Gao , Michael D Kinney , Jian J Wang , Hao A Wu Subject: [PATCH V2 1/1] MdeModulePkg/Variable: Fix volatile variable RT cache update logic Date: Tue, 12 Nov 2019 15:40:38 -0800 Message-Id: <20191112234038.23092-1-michael.a.kubacki@intel.com> X-Mailer: git-send-email 2.16.2.windows.1 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). * Observable symptom: A volatile variable that was set successfully might return EFI_NOT_FOUND when the variable should be found. * The issue is a regression introduced to the variable services only when the variable runtime cache is enabled by the following PCD being set to TRUE: gEfiMdeModulePkgTokenSpaceGuid.PcdEnableVariableRuntimeCache * The issue was implemented in commit aab3b9b9a1 but the PCD was not set to TRUE by default enabling the issue until commit e07b7d024a. Fixes: aab3b9b9a1e5e1f3fa966fb1667fc3e6c47e7706 Cc: Liming Gao Cc: Michael D Kinney Cc: Jian J Wang Cc: Hao A Wu Signed-off-by: Michael Kubacki --- Notes: V2 changes: * Sets the runtime cache instance pointer that was updated in V1 using an if/else statement instead of just an if statement so it better aligns to the original code style. * Updates the commit message to include the simple symptom most commonly observed that this change fixes, an explicit description that this change is a regression to the variable services, that it only is present when the runtime variable cache is enabled, and relevant commit details. MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c index 29d6aca993..b0ee5e50d0 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c @@ -2296,10 +2296,10 @@ UpdateVariable ( Done: if (!EFI_ERROR (Status)) { - if (Variable->Volatile) { - VolatileCacheInstance = &(mVariableModuleGlobal->VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCache); - } else { + if ((Variable->CurrPtr != NULL && !Variable->Volatile) || (Attributes & EFI_VARIABLE_NON_VOLATILE) != 0) { VolatileCacheInstance = &(mVariableModuleGlobal->VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache); + } else { + VolatileCacheInstance = &(mVariableModuleGlobal->VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCache); } if (VolatileCacheInstance->Store != NULL) { -- 2.16.2.windows.1