From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <jian.j.wang@intel.com>
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 <edk2-devel@lists.01.org>; 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" <jian.j.wang@intel.com>
To: "Zeng, Star" <star.zeng@intel.com>, "edk2-devel@lists.01.org"
<edk2-devel@lists.01.org>
CC: "Dong, Eric" <eric.dong@intel.com>, "Yao, Jiewen" <jiewen.yao@intel.com>,
"Ni, Ruiyu" <ruiyu.ni@intel.com>, "Kinney, Michael D"
<michael.d.kinney@intel.com>
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: <D827630B58408649ACB04F44C510003624D3BA13@SHSMSX103.ccr.corp.intel.com>
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 <edk2-devel.lists.01.org>
List-Unsubscribe: <https://lists.01.org/mailman/options/edk2-devel>,
<mailto:edk2-devel-request@lists.01.org?subject=unsubscribe>
List-Archive: <http://lists.01.org/pipermail/edk2-devel/>
List-Post: <mailto:edk2-devel@lists.01.org>
List-Help: <mailto:edk2-devel-request@lists.01.org?subject=help>
List-Subscribe: <https://lists.01.org/mailman/listinfo/edk2-devel>,
<mailto:edk2-devel-request@lists.01.org?subject=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 <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;=
Ni,
> Ruiyu <ruiyu.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com=
>;
> Zeng, Star <star.zeng@intel.com>
> 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
> <star.zeng@intel.com>.
>=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 <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Y=
ao,
> Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> 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 <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
> 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