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.151; helo=mga17.intel.com; envelope-from=star.zeng@intel.com; receiver=edk2-devel@lists.01.org Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) (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 3829721E08160 for ; Mon, 2 Apr 2018 18:15:46 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 Apr 2018 18:15:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,398,1517904000"; d="scan'208";a="42892775" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga004.fm.intel.com with ESMTP; 02 Apr 2018 18:15:44 -0700 Received: from fmsmsx156.amr.corp.intel.com (10.18.116.74) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 2 Apr 2018 18:15:43 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx156.amr.corp.intel.com (10.18.116.74) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 2 Apr 2018 18:15:43 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.80]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.235]) with mapi id 14.03.0319.002; Tue, 3 Apr 2018 09:15:41 +0800 From: "Zeng, Star" To: "Kinney, Michael D" , Heyi Guo , "edk2-devel@lists.01.org" CC: Yi Li , Renhao Liang , "Dong, Eric" , "Gao, Liming" , "Wang, Jian J" , "Ni, Ruiyu" , "Zeng, Star" Thread-Topic: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion Thread-Index: AQHTxzbVMQvHVBZY20yNQkEJtf3o/KPtAMYQgACB6QCAALrAQIAABRcA Date: Tue, 3 Apr 2018 01:15:41 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103BA7549E@shsmsx102.ccr.corp.intel.com> References: <1522311590-104218-1-git-send-email-heyi.guo@linaro.org> <0C09AFA07DD0434D9E2A0C6AEB0483103BA75236@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103BA7547F@shsmsx102.ccr.corp.intel.com> In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103BA7547F@shsmsx102.ccr.corp.intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion 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: Tue, 03 Apr 2018 01:15:46 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Current gCpu->SetMemoryAttributes does not support getting memory attribute= s, and has no clear description about clearing memory attributes. I noticed we introduced SmmMemoryAttribute(MdeModulePkg\Include\Protocol\Sm= mMemoryAttribute.h) protocol for heap guard feature in SMM environment. Seemingly, we also need introduce MemoryAttribute protocol for DXE? Thanks, Star -----Original Message----- From: Zeng, Star=20 Sent: Tuesday, April 3, 2018 8:59 AM To: Kinney, Michael D ; Heyi Guo ; edk2-devel@lists.01.org Cc: Yi Li ; Renhao Liang ;= Dong, Eric ; Gao, Liming ; Wang= , Jian J ; Ni, Ruiyu ; Zeng, Sta= r Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion Mike, Agree the problem. We need support only RUNTIME attribute. We need support only cache attributes. We need support only page attributes. We need support RUNTIME + cache + page attributes. Do we need support the 0 Attributes case to clear page attributes? There was discussion about the 0 Attributes case at https://lists.01.org/pi= permail/edk2-devel/2018-March/023315.html. It came to the question that should the 0 Attributes case be handled by Set= MemoryAttributes() to clear the page attributes? Thanks, Star -----Original Message----- From: Kinney, Michael D Sent: Tuesday, April 3, 2018 5:43 AM To: Zeng, Star ; Heyi Guo ; edk2-= devel@lists.01.org; Kinney, Michael D Cc: Yi Li ; Renhao Liang ;= Dong, Eric ; Gao, Liming ; Wang= , Jian J ; Ni, Ruiyu Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion Star, This commit breaks Vlv2TbltDevicePkg. On this platform, there are 2 calls to=20 gDS->SetMemorySpaceAttributes() for an MMIO range to set only the EFI_MEMORY_RUNTIME bit. With this commit, ConverToCpuArchAttributes()returns 0, and 0 is passed to = gCpu->SetMemoryAttributes()that returns EFI_INVALID_PARAMETER on Vlv2TbltDe= vicePkg. Before this commit, ConverToCpuArchAttributes() returns INVALID_CPU_ARCH_AT= TRIBUTES which causes the call to gCpu->SetMemoryAttributes()to be skipped = so no error is generated. I think the right fix is to skip the call to=20 gCpu->SetMemoryAttributes() if CpuArchAttributes is 0 so a call that only sets the RUNTIME attribute can be supported along = with call the set both the RUNTIME bit and other cache related bits. I will send a patch soon with the proposed fix. Mike > -----Original Message----- > From: Zeng, Star > Sent: Sunday, April 1, 2018 10:59 PM > To: Heyi Guo ; edk2- devel@lists.01.org > Cc: Yi Li ; Renhao Liang=20 > ; Dong, Eric ; Kinney,=20 > Michael D ; Gao, Liming=20 > ; Wang, Jian J ; Ni,=20 > Ruiyu ; Zeng, Star > Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute=20 > conversion >=20 > Pushed at 5b91bf82c67b586b9588cbe4bbffa1588f6b5926. >=20 > Thanks, > Star > -----Original Message----- > From: Heyi Guo [mailto:heyi.guo@linaro.org] > Sent: Thursday, March 29, 2018 4:20 PM > To: edk2-devel@lists.01.org > Cc: Heyi Guo ; Yi Li ;=20 > Renhao Liang ; Zeng, Star=20 > ; Dong, Eric ; Kinney,=20 > Michael D ; Gao, Liming=20 > ; Wang, Jian J ; Ni,=20 > Ruiyu > Subject: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion >=20 > For gDS->SetMemorySpaceAttributes(), when user passes a combined=20 > memory attribute including CPU arch attribute and other attributes,=20 > like EFI_MEMORY_RUNTIME, > ConverToCpuArchAttributes() will return INVALID_CPU_ARCH_ATTRIBUTES=20 > and skip setting page/cache attribute for the specified memory space. >=20 > We don't see any reason to forbid combining CPU arch attributes and=20 > non-CPU-arch attributes when calling gDS- > >SetMemorySpaceAttributes(), so we remove the check code > in ConverToCpuArchAttributes(); the remaining code is enough to grab=20 > the interested bits for > Cpu->SetMemoryAttributes(). >=20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Heyi Guo > Signed-off-by: Yi Li > Signed-off-by: Renhao Liang > Cc: Star Zeng > Cc: Eric Dong > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Jian J Wang > Cc: Ruiyu Ni > --- > MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 5 ----- > 1 file changed, 5 deletions(-) >=20 > diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c=20 > b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c index > 77f4adb4bc01..907245a3f512 100644 > --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > @@ -673,11 +673,6 @@ ConverToCpuArchAttributes ( { > UINT64 CpuArchAttributes; >=20 > - if ((Attributes & ~(EXCLUSIVE_MEMORY_ATTRIBUTES | > - NONEXCLUSIVE_MEMORY_ATTRIBUTES)) > !=3D 0) { > - return INVALID_CPU_ARCH_ATTRIBUTES; > - } > - > CpuArchAttributes =3D Attributes & > NONEXCLUSIVE_MEMORY_ATTRIBUTES; >=20 > if ( (Attributes & EFI_MEMORY_UC) =3D=3D EFI_MEMORY_UC) { > -- > 2.7.4