From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=michael.d.kinney@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 776DC22526482 for ; Wed, 4 Apr 2018 13:28:03 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Apr 2018 13:28:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,408,1517904000"; d="scan'208";a="48009901" Received: from orsmsx110.amr.corp.intel.com ([10.22.240.8]) by orsmga002.jf.intel.com with ESMTP; 04 Apr 2018 13:28:02 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.67]) by ORSMSX110.amr.corp.intel.com ([169.254.10.11]) with mapi id 14.03.0319.002; Wed, 4 Apr 2018 13:28:01 -0700 From: "Kinney, Michael D" To: "Yao, Jiewen" , "Zeng, Star" , "edk2-devel@lists.01.org" , "Kinney, Michael D" CC: "Ni, Ruiyu" , Yi Li , "Dong, Eric" , Renhao Liang , "Gao, Liming" , Heyi Guo , "Zeng, Star" Thread-Topic: [edk2] [PATCH V2] MdeModulePkg/Gcd: Filter gCpu->SetMemoryAttributes() calls Thread-Index: AQHTy7ndX0sLzix33EGg/EPnJKn0EaPwt2SAgABXzyA= Date: Wed, 4 Apr 2018 20:28:01 +0000 Message-ID: References: <1522807708-75668-1-git-send-email-star.zeng@intel.com> <74D8A39837DF1E4DA445A8C0B3885C503AB339F7@shsmsx102.ccr.corp.intel.com> In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503AB339F7@shsmsx102.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.22.254.139] MIME-Version: 1.0 Subject: Re: [PATCH V2] MdeModulePkg/Gcd: Filter gCpu->SetMemoryAttributes() calls X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Apr 2018 20:28:03 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Star, Thanks for adding the comments Reviewed-by: Michael D Kinney Mike > -----Original Message----- > From: Yao, Jiewen > Sent: Wednesday, April 4, 2018 1:13 AM > To: Zeng, Star ; edk2- > devel@lists.01.org > Cc: Ni, Ruiyu ; Yi Li > ; Dong, Eric > ; Renhao Liang > ; Gao, Liming > ; Heyi Guo ; > Kinney, Michael D ; Zeng, > Star > Subject: RE: [edk2] [PATCH V2] MdeModulePkg/Gcd: Filter > gCpu->SetMemoryAttributes() calls >=20 > Thanks. >=20 > Reviewed-by: Jiewen.yao@intel.com >=20 > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel- > bounces@lists.01.org] On Behalf Of Star > > Zeng > > Sent: Wednesday, April 4, 2018 10:08 AM > > To: edk2-devel@lists.01.org > > Cc: Ni, Ruiyu ; Yi Li > ; Dong, Eric > > ; Renhao Liang > ; Gao, Liming > > ; Heyi Guo > ; Kinney, Michael D > > ; Zeng, Star > > > Subject: [edk2] [PATCH V2] MdeModulePkg/Gcd: Filter > > gCpu->SetMemoryAttributes() calls > > > > From: "Kinney, Michael D" > > > > > This patch fixes an issue with VlvTbltDevicePkg > introduced > > by commit 5b91bf82c67b586b9588cbe4bbffa1588f6b5926. > > > > The history is as below. > > To support heap guard feature, > > 14dde9e903bb9a719ebb8f3381da72b19509bc36 > > added support for SetMemorySpaceAttributes() to > handle page attributes, > > but after that, a combination of CPU arch attributes > and other attributes > > was not allowed anymore, for example, UC + RUNTIME. > It is a regression. > > Then 5b91bf82c67b586b9588cbe4bbffa1588f6b5926 was to > fix the regression, > > and we thought 0 CPU arch attributes may be used to > clear CPU arch > > attributes, so 0 CPU arch attributes was allowed to > be sent to > > gCpu->SetMemoryAttributes(). > > > > But some implementation of CPU driver may return > error for 0 CPU arch > > attributes. That fails the case that caller just > calls > > SetMemorySpaceAttributes() with none CPU arch > attributes (for example, > > RUNTIME), and the purpose of the case is not to clear > CPU arch attributes. > > > > This patch filters the call to gCpu- > >SetMemoryAttributes() > > if the requested attributes is 0. It also removes > the #define > > INVALID_CPU_ARCH_ATTRIBUTES that is no longer used. > > > > Cc: Heyi Guo > > Cc: Yi Li > > Cc: Renhao Liang > > Cc: Star Zeng > > Cc: Eric Dong > > Cc: Liming Gao > > Cc: Jian J Wang > > Cc: Ruiyu Ni > > Signed-off-by: Michael D Kinney > > > Signed-off-by: Star Zeng > > Contributed-under: TianoCore Contribution Agreement > 1.1 > > --- > > MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 25 > ++++++++++++++++++++++--- > > 1 file changed, 22 insertions(+), 3 deletions(-) > > > > diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > index 907245a3f512..31ddf9c3bb51 100644 > > --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > @@ -48,8 +48,6 @@ WITHOUT WARRANTIES OR > REPRESENTATIONS OF ANY > > KIND, EITHER EXPRESS OR IMPLIED. > > #define NONEXCLUSIVE_MEMORY_ATTRIBUTES > (EFI_MEMORY_XP | > > EFI_MEMORY_RP | \ > > > EFI_MEMORY_RO) > > > > -#define INVALID_CPU_ARCH_ATTRIBUTES 0xffffffff > > - > > // > > // Module Variables > > // > > @@ -873,7 +871,21 @@ CoreConvertSpace ( > > // Call CPU Arch Protocol to attempt to set > attributes on the range > > // > > CpuArchAttributes =3D ConverToCpuArchAttributes > (Attributes); > > - if (CpuArchAttributes !=3D > INVALID_CPU_ARCH_ATTRIBUTES) { > > + // > > + // CPU arch attributes include page attributes > and cache attributes. > > + // Only page attributes supports to be cleared, > but not cache attributes. > > + // Caller is expected to use > GetMemorySpaceDescriptor() to get the current > > + // attributes, AND/OR attributes, and then calls > > SetMemorySpaceAttributes() > > + // to set the new attributes. > > + // So 0 CPU arch attributes should not happen as > memory should always > > have > > + // a cache attribute (no matter UC or WB, etc). > > + // > > + // Here, 0 CPU arch attributes will be filtered > to be compatible with the > > + // case that caller just calls > SetMemorySpaceAttributes() with none CPU > > + // arch attributes (for example, RUNTIME) as the > purpose of the case is not > > + // to clear CPU arch attributes. > > + // > > + if (CpuArchAttributes !=3D 0) { > > if (gCpu =3D=3D NULL) { > > Status =3D EFI_NOT_AVAILABLE_YET; > > } else { > > @@ -936,6 +948,13 @@ CoreConvertSpace ( > > // Set attributes operation > > // > > case GCD_SET_ATTRIBUTES_MEMORY_OPERATION: > > + if (CpuArchAttributes =3D=3D 0) { > > + // > > + // Keep original CPU arch attributes when > caller just calls > > + // SetMemorySpaceAttributes() with none CPU > arch attributes (for > > example, RUNTIME). > > + // > > + Attributes |=3D (Entry->Attributes & > > (EXCLUSIVE_MEMORY_ATTRIBUTES | > NONEXCLUSIVE_MEMORY_ATTRIBUTES)); > > + } > > Entry->Attributes =3D Attributes; > > break; > > // > > -- > > 2.7.0.windows.1 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel