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=jian.j.wang@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 F34CF226CD784 for ; Wed, 11 Apr 2018 17:59:16 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Apr 2018 17:59:16 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,438,1517904000"; d="scan'208";a="36554386" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga002.fm.intel.com with ESMTP; 11 Apr 2018 17:59:16 -0700 Received: from fmsmsx113.amr.corp.intel.com (10.18.116.7) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 11 Apr 2018 17:59:16 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX113.amr.corp.intel.com (10.18.116.7) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 11 Apr 2018 17:59:15 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.151]) by shsmsx102.ccr.corp.intel.com ([169.254.2.184]) with mapi id 14.03.0319.002; Thu, 12 Apr 2018 08:59:13 +0800 From: "Wang, Jian J" To: "Zeng, Star" , "edk2-devel@lists.01.org" CC: "Dong, Eric" , "Yao, Jiewen" , "Ni, Ruiyu" , "Kinney, Michael D" Thread-Topic: [PATCH] MdeModulePkg/PiSmmIpl: fix non-executable SMM RAM Thread-Index: AQHT0Wsg58SyKaDU50+lmtdxBWN81KP8T+Pw Date: Thu, 12 Apr 2018 00:59:13 +0000 Message-ID: References: <20180411072955.6276-1-jian.j.wang@intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103BAA9C60@shsmsx102.ccr.corp.intel.com> In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103BAA9C60@shsmsx102.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNWU4MjA1ZWYtMGRlYS00ZWE4LWI1NDEtYTcyNGQwMzI2OTAyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiI2VmEzQmtmNVwvQUdRNW00Y1dvdUJjaUhNVkxEVWNwc2dHbndHb0xoRm5cL2JcL3JVNkg5UWY5MHA4cDRCNEJFdk9CIn0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdeModulePkg/PiSmmIpl: fix non-executable SMM RAM 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: Thu, 12 Apr 2018 00:59:17 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Yeah, you're right. I'll update the log text. Thanks for catching it. Regards, Jian > -----Original Message----- > From: Zeng, Star > Sent: Wednesday, April 11, 2018 4:00 PM > To: Wang, Jian J ; edk2-devel@lists.01.org > Cc: Dong, Eric ; Yao, Jiewen ;= Ni, > Ruiyu ; Kinney, Michael D ; > Zeng, Star > Subject: RE: [PATCH] MdeModulePkg/PiSmmIpl: fix non-executable SMM RAM >=20 > It should be exposed by 5b91bf82c67b586b9588cbe4bbffa1588f6b5926 + > 0c9f2cb10b7ddec56a3440e77219fd3ab1725e5c. >=20 > With this information updated in commit log, Reviewed-by: Star Zeng > . >=20 > Thanks, > Star > -----Original Message----- > From: Wang, Jian J > Sent: Wednesday, April 11, 2018 3:30 PM > To: edk2-devel@lists.01.org > Cc: Zeng, Star ; Dong, Eric ; Y= ao, > Jiewen ; Ni, Ruiyu ; Kinney, > Michael D > Subject: [PATCH] MdeModulePkg/PiSmmIpl: fix non-executable SMM RAM >=20 > This patch fixes an issue introduced by commit >=20 > 5b91bf82c67b586b9588cbe4bbffa1588f6b5926 >=20 > This issue will only happen if PcdDxeNxMemoryProtectionPolicy is > enabled for reserved memory, which will mark SMM RAM as NX (non- > executable) during DXE core initialization. SMM IPL driver will > unset the NX attribute for SMM RAM to allow loading and running > SMM core/drivers. >=20 > But above commit will fail the unset operation of the NX attribute > due to a fact that SMM RAM has zero cache attribute (MRC code always > sets 0 attribute for reserved memory), which will cause GCD internal > method ConverToCpuArchAttributes() to return 0 attribute which is > taken as invalid CPU paging attribute and skip the calling of > gCpu->SetMemoryAttributes(). >=20 > Commit 0c9f2cb10b7ddec56a3440e77219fd3ab1725e5c tries to fix compatible > issue but not this one. The solution is to make use of existing > functionality in PiSmmIpl to make sure one cache attribute is set > for SMM RAM. For performance consideration, PiSmmIpl will always > try to set SMM RAM to write-back. But there's a hole in the code > which will fail the setting write-back attribute because of no > corresponding cache capabilities. This patch will add necessary > cache capabilities before setting corresponding attributes. >=20 > Cc: Star Zeng > Cc: Eric Dong > Cc: Jiewen Yao > Cc: Ruiyu Ni > Cc: Michael D Kinney > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang > --- > MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 32 > ++++++++++++++++++++++++-------- > 1 file changed, 24 insertions(+), 8 deletions(-) >=20 > diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > index 94d671bd74..552220b4dd 100644 > --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > @@ -1617,6 +1617,21 @@ SmmIplEntry ( >=20 > GetSmramCacheRange (mCurrentSmramRange, &mSmramCacheBase, > &mSmramCacheSize); > // > + // Make sure we can change the cache attributes. > + // > + Status =3D gDS->GetMemorySpaceDescriptor ( > + mSmramCacheBase, > + &MemDesc > + ); > + if (!EFI_ERROR (Status) && > + (MemDesc.Capabilities & (EFI_MEMORY_WB | EFI_MEMORY_UC)) !=3D > (EFI_MEMORY_WB | EFI_MEMORY_UC)) { > + gDS->SetMemorySpaceCapabilities ( > + mSmramCacheBase, > + mSmramCacheSize, > + MemDesc.Capabilities | EFI_MEMORY_WB | EFI_MEMORY_UC > + ); > + } > + // > // If CPU AP is present, attempt to set SMRAM cacheability to WB and= clear > // XP if it's set. > // Note that it is expected that cacheability of SMRAM has been set = to WB if > CPU AP > @@ -1626,7 +1641,7 @@ SmmIplEntry ( > Status =3D gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOI= D > **)&CpuArch); > if (!EFI_ERROR (Status)) { > Status =3D gDS->SetMemorySpaceAttributes( > - mSmramCacheBase, > + mSmramCacheBase, > mSmramCacheSize, > EFI_MEMORY_WB > ); > @@ -1634,16 +1649,17 @@ SmmIplEntry ( > DEBUG ((DEBUG_WARN, "SMM IPL failed to set SMRAM window to > EFI_MEMORY_WB\n")); > } >=20 > - Status =3D gDS->GetMemorySpaceDescriptor( > - mCurrentSmramRange->PhysicalStart, > + Status =3D gDS->GetMemorySpaceDescriptor ( > + mSmramCacheBase, > &MemDesc > ); > if (!EFI_ERROR (Status) && (MemDesc.Attributes & EFI_MEMORY_XP) != =3D 0) { > - gDS->SetMemorySpaceAttributes ( > - mCurrentSmramRange->PhysicalStart, > - mCurrentSmramRange->PhysicalSize, > - MemDesc.Attributes & (~EFI_MEMORY_XP) > - ); > + Status =3D gDS->SetMemorySpaceAttributes ( > + mSmramCacheBase, > + mSmramCacheSize, > + MemDesc.Attributes & (~EFI_MEMORY_XP) > + ); > + ASSERT_EFI_ERROR (Status); > } > } > // > -- > 2.16.2.windows.1