From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mx.groups.io with SMTP id smtpd.web12.4400.1573520141426422202 for ; Mon, 11 Nov 2019 16:55:41 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=O2DX45uA; spf=pass (domain: intel.com, ip: 134.134.136.100, mailfrom: michael.a.kubacki@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Nov 2019 16:55:40 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,294,1569308400"; d="scan'208";a="213897811" Received: from orsmsx101.amr.corp.intel.com ([10.22.225.128]) by fmsmga001.fm.intel.com with ESMTP; 11 Nov 2019 16:55:39 -0800 Received: from orsmsx122.amr.corp.intel.com (10.22.225.227) by ORSMSX101.amr.corp.intel.com (10.22.225.128) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 11 Nov 2019 16:55:39 -0800 Received: from ORSEDG001.ED.cps.intel.com (10.7.248.4) by ORSMSX122.amr.corp.intel.com (10.22.225.227) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 11 Nov 2019 16:55:38 -0800 Received: from NAM03-CO1-obe.outbound.protection.outlook.com (104.47.40.50) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 11 Nov 2019 16:55:38 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QXcTxlmNk+vCsBWHh7xozfmjls0t0oGjoC14ywUUVdRwEizf5KL284dpsWyYrGTS/z66ERKbr778CvI+Q9b/iTHQBhLXMMIuF0Q2V5QBttA8QOUuk2Xtofq0uQqe72avN67xjIjcar2AzdwB7o7LqCoy+PUHQQC+7sYIntX1yctG1xKxQ8nFGnk34tZWxjgb1JqOEZoebVwGmAoxEyz4IrSQwwO9f2AmHiWvHW0ItmOUxBDzxtbQOP3kv/oNAB/lxs81vVAz1UvLh9J3DxtSNbngcr47zS5Mv6Hj2UMxJOZbUOVW/DoVWtv/Kw46h2VYNaQB8h4x5ynfYM+TnWh7hw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=4Lo5pcaTDt3wEsA0JiePjMdfWooOCvVmQzl2p5JM+Ak=; b=fmVzT/xFdZBOQ6cvovXAXbLTWpVJ+CNx2ngvuYGrXo1yuTM9K3RHjEqBzj32dexAJkAqBTMI0ZjxBSF8u0NRSwum8qab7ItPU8tX+BUywYKLMRUnp/+edMgh6Tnb4w0QGjkDrocWhVnI6xP/+6Srh4BowJax0X3brnSyU4mYsU/vIXrGPlKxbaURqljPQVMWJAvA1l++A1ntg+naWu1ICsCH7IJ6WuGcU3d12g0sARPl4YN0ebJ4et1ipxObqq90zbz4dFXLJdNTeaY6bwT4rSx9Tt03TyjnjmodUeV781AXMICwiHi75KmylF844tXLMRRvkp28Ab+mNNe8W1Fjww== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=4Lo5pcaTDt3wEsA0JiePjMdfWooOCvVmQzl2p5JM+Ak=; b=O2DX45uAhAtYXyw1VhSmJAhBWSFTEYZYlb1C93BiH0s6hybJRNJ7pl+6dM6tRCkyv858pKMzgYH0EPOkiflxhgoFKpqMgM996U+jy8AlZtwV6okjnnBDHY/DTfd5+QnNzKL48leOEBHVHka3Emn9ThSXTC8Ac+Ol8RbkxTKEPSk= Received: from BY5PR11MB4484.namprd11.prod.outlook.com (52.132.254.155) by BY5PR11MB4136.namprd11.prod.outlook.com (10.255.163.158) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2430.22; Tue, 12 Nov 2019 00:55:36 +0000 Received: from BY5PR11MB4484.namprd11.prod.outlook.com ([fe80::c1ef:6e0b:90c8:55e2]) by BY5PR11MB4484.namprd11.prod.outlook.com ([fe80::c1ef:6e0b:90c8:55e2%7]) with mapi id 15.20.2430.027; Tue, 12 Nov 2019 00:55:36 +0000 From: "Kubacki, Michael A" To: "Gao, Liming" , "devel@edk2.groups.io" CC: "Kinney, Michael D" , "Wang, Jian J" , "Wu, Hao A" Subject: Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix volatile variable RT cache update logic Thread-Topic: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix volatile variable RT cache update logic Thread-Index: AQHVmE8xR9YeOpMBmUyU1dFIoHwO4qeFe/awgAAApDCAATHbAIAAAOVg Date: Tue, 12 Nov 2019 00:55:35 +0000 Message-ID: References: <20191111051620.37748-1-michael.a.kubacki@intel.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E53C580@SHSMSX104.ccr.corp.intel.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E53CC7C@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E53CC7C@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZmRkYzdmYWQtOTdlYi00M2Q3LWI4M2EtMDdlY2U0N2Y5ZDEzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiMHBqNGJiVlwvV2FMblBNMmZ2cjBBK3VZYjVKNXlVMUg0RHMzM2VpOXIxd3V3YUNpZmt6T01WdmZveXlWbVQxYlwvIn0= dlp-reaction: no-action dlp-version: 11.2.0.6 authentication-results: spf=none (sender IP is ) smtp.mailfrom=michael.a.kubacki@intel.com; x-originating-ip: [134.134.136.217] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 6403c97f-3876-4f9b-6fde-08d7670b076e x-ms-traffictypediagnostic: BY5PR11MB4136: x-ms-exchange-purlcount: 4 x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 021975AE46 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(396003)(346002)(376002)(39860400002)(366004)(136003)(13464003)(189003)(199004)(55016002)(53546011)(33656002)(6506007)(66556008)(14444005)(66066001)(76116006)(76176011)(4326008)(15650500001)(7696005)(6436002)(476003)(102836004)(107886003)(66446008)(446003)(486006)(256004)(26005)(66946007)(64756008)(6246003)(66476007)(186003)(71200400001)(71190400001)(9686003)(6306002)(11346002)(81166006)(74316002)(110136005)(2906002)(966005)(8676002)(14454004)(316002)(478600001)(305945005)(229853002)(7736002)(81156014)(52536014)(25786009)(99286004)(2501003)(86362001)(54906003)(5660300002)(6116002)(3846002)(8936002);DIR:OUT;SFP:1102;SCL:1;SRVR:BY5PR11MB4136;H:BY5PR11MB4484.namprd11.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: YpmT0X6RHlyjuuczHCaHIL/AOYvB1uLRulpYQDMuZqSb9Bkj37B6bDoDKpLYfBa9d9bY9AL1bPgY1oTUco6Nu5n5mXjMfUlWpTafJuqHENiQPlMZDRqpHHIkK8Am70SofC+0NmlXVWeUgtI3niXeF2nsGIexxZP89AotAUIMb5s5ux+2TxtP/F1/KRrNGcS43hU6rr5QlkMMeT0BSDy6ApXDUCnpNokpJnIVhkyt1/kYST0wQvIkqomm75hiInq0x4wj306fpc8mLRN+cT8QKiyGcWYdwVfVuGF79NxLGlbcR//0BpZoWPG5MSeBO5ggC1KPgpcuZjmUl+FbdB6UN3ge+2hh8HhOrWfnhjNL8U+MjxL/beBhFC5uACOcPw4hixkbMKBOG43j0toMeTAnjGEnHEgpG7rC0Ms93NADMbg7vDV1BSuzIIU5F0yDTV8pt1ri4GsSJHxfbcSlifE6hmLglhfCVms12syKQkj7ik4= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 6403c97f-3876-4f9b-6fde-08d7670b076e X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Nov 2019 00:55:35.5997 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: OOahVtMDhw9EyZzWvquQf1FUbyEDKIupwntny5VmdwRUzwnn5A3YqYMbEs1RvTTLed9oDN9soNb9KPmDU/UuepiksV4Ibcd9X/ldPFkeois= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR11MB4136 Return-Path: michael.a.kubacki@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable In the case a UEFI variable is deleted, the Attributes can be zero (Attribu= tes are marked OPTIONAL in the API). So in that case, the Attributes parameter alone does not accurately reflec= t the type of variable. The function handles this fairly cryptically. The condition you specified is used in th= e function, for example on the following line: https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/Varia= ble/RuntimeDxe/Variable.c#L2024 but I believe that is only valid because the deletion case was handled ear= lier in the function followed by a goto Done so this code would be skipped over in the case of a pre-existing= variable which would have the ->Volatile field set properly because it would have been set depending upo= n which variable store buffer=20 the variable was found in. At this point in the flow (after the Done label= ), I believe this logic is required to generically determine the variable volatility status. Thanks, Michael > -----Original Message----- > From: Gao, Liming > Sent: Monday, November 11, 2019 4:25 PM > To: Kubacki, Michael A ; > devel@edk2.groups.io > Cc: Kinney, Michael D ; Wang, Jian J > ; Wu, Hao A > Subject: RE: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Fix > volatile variable RT cache update logic >=20 > Michael: > Can the logic be simplified to only check this condition 'Attributes & > EFI_VARIABLE_NON_VOLATILE) !=3D 0'? >=20 > Thanks > Liming > >-----Original Message----- > >From: Kubacki, Michael A > >Sent: Monday, November 11, 2019 2:11 PM > >To: Gao, Liming ; devel@edk2.groups.io > >Cc: Kinney, Michael D ; Wang, Jian J > >; Wu, Hao A > >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 > >> 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 > >> >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 > >> >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.VariableRuntimeVolatileC > >> >>a > >c > >> h > >> >e); > >> >- } else { > >> >+ VolatileCacheInstance =3D &(mVariableModuleGlobal- > >> > >>VariableGlobal.VariableRuntimeCacheContext.VariableRuntimeVolatileC > >> >>a > >c > >> 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