From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web10.9227.1573548833164329403 for ; Tue, 12 Nov 2019 00:53:53 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Qw5BvoX5; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1573548832; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=lrJZnyE7KPbliS8y1eoFJdfb5u+a5Dqxq2G4Zi9P8vY=; b=Qw5BvoX5RypKvXxZ1FDajV32YvCigrxXTXQHxzWiLckBIYXd0ZAHUg/Osb76S4IFH45JS/ WrA2IuzWIJ6zoYbf5EVyNuhOQ+75Lu7PetSpbhHtwOvH6b+hnAnMafhMA0VI7a5Jcxr3Y8 dR43C7ZPL0ALczZpCxErEjp7yV+JOZw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-6-sBLCzbfzNGS1wB82WKbaCw-1; Tue, 12 Nov 2019 03:53:48 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 36271800C61; Tue, 12 Nov 2019 08:53:47 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-207.ams2.redhat.com [10.36.116.207]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8E262100EBB8; Tue, 12 Nov 2019 08:53:45 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix volatile variable RT cache update logic To: devel@edk2.groups.io, michael.a.kubacki@intel.com, "Gao, Liming" Cc: "Kinney, Michael D" , "Wang, Jian J" , "Wu, Hao A" References: <20191111051620.37748-1-michael.a.kubacki@intel.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E53C580@SHSMSX104.ccr.corp.intel.com> From: "Laszlo Ersek" Message-ID: <537d3003-3bb8-5996-a81f-dfbaf00c93c9@redhat.com> Date: Tue, 12 Nov 2019 09:53:44 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-MC-Unique: sBLCzbfzNGS1wB82WKbaCw-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable Hello Michael, On 11/11/19 07:11, Kubacki, Michael A wrote: > After a new volatile variable is written successfully, Runtime Services = GetVariable () may return > EFI_NOT_FOUND for that variable. I've read , up to comment #1. Can you please update the commit message, with the following information: - the symptom that you describe above (in response to Liming's question), - whether this issue is a regression introduced by the earlier work for TianoCore#2220, - if so, then: whether the issue affects a platform even if the platform sets "PcdEnableVariableRuntimeCache" to false - alternatively, whether this issue is a preexistent one, which has been *exposed* only, by the work for TianoCore#2220, - in case this is a regression genuinely introduced by the work for TianoCore#2220, can you please add a "Fixes: " line near the end of the commit message, to identify the specific commit that introduced the regression? Regarding the Bugzilla ticket (TianoCore#2333): if the issue is a regression, then: - please set the "regression" keyword, in the Keywords field, - furthermore, please add "2220" to the See Also field. Thanks! Laszlo >> -----Original Message----- >> From: Gao, Liming >> Sent: Sunday, November 10, 2019 10:08 PM >> To: devel@edk2.groups.io; Kubacki, Michael A >> >> Cc: Kinney, Michael D ; Wang, Jian J >> ; Wu, Hao A ; Gao, Liming >> >> 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 ; Kinney, Michael D >>> ; Wang, Jian J ; Wu= , >>> Hao A >>> 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=3D2333 >>> >>> 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 b= e >>> 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 >>> Cc: Michael D Kinney >>> Cc: Jian J Wang >>> Cc: Hao A Wu >>> Signed-off-by: Michael Kubacki >>> --- >>> 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 =3D &(mVariableModuleGlobal- >>>> VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCac >> h >>> e); >>> - } else { >>> + VolatileCacheInstance =3D &(mVariableModuleGlobal- >>>> VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileCac >> h >>> e); >>> + if ((Variable->CurrPtr !=3D NULL && !Variable->Volatile) || >>> + (Attributes & >>> EFI_VARIABLE_NON_VOLATILE) !=3D 0) { >>> VolatileCacheInstance =3D &(mVariableModuleGlobal- >>>> VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeNvCache); >>> } >>> >>> -- >>> 2.16.2.windows.1 >>> >>> >>> >> >=20 >=20 >=20 >=20