From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: eric.dong@intel.com) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by groups.io with SMTP; Mon, 26 Aug 2019 18:43:37 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Aug 2019 18:43:36 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,435,1559545200"; d="scan'208";a="355617464" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga005.jf.intel.com with ESMTP; 26 Aug 2019 18:43:36 -0700 Received: from fmsmsx126.amr.corp.intel.com (10.18.125.43) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 26 Aug 2019 18:43:35 -0700 Received: from shsmsx108.ccr.corp.intel.com (10.239.4.97) by FMSMSX126.amr.corp.intel.com (10.18.125.43) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 26 Aug 2019 18:43:35 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.19]) by SHSMSX108.ccr.corp.intel.com ([169.254.8.163]) with mapi id 14.03.0439.000; Tue, 27 Aug 2019 09:43:33 +0800 From: "Dong, Eric" To: "Ni, Ray" , "devel@edk2.groups.io" CC: "Yao, Jiewen" , Laszlo Ersek Subject: Re: [PATCH 2/5] UefiCpuPkg/PiSmmCpu: Use new PCD PcdCpuSmmRestrictedMemoryAccess Thread-Topic: [PATCH 2/5] UefiCpuPkg/PiSmmCpu: Use new PCD PcdCpuSmmRestrictedMemoryAccess Thread-Index: AQHVW5byfhprLl1dRE2Oj6Q1CPfCRKcOOnUg Date: Tue, 27 Aug 2019 01:43:32 +0000 Message-ID: References: <20190825224513.171572-1-ray.ni@intel.com> <20190825224513.171572-3-ray.ni@intel.com> In-Reply-To: <20190825224513.171572-3-ray.ni@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: eric.dong@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Eric Dong > -----Original Message----- > From: Ni, Ray > Sent: Monday, August 26, 2019 6:45 AM > To: devel@edk2.groups.io > Cc: Dong, Eric ; Yao, Jiewen ; > Laszlo Ersek > Subject: [PATCH 2/5] UefiCpuPkg/PiSmmCpu: Use new PCD > PcdCpuSmmRestrictedMemoryAccess >=20 > The patch changes PiSmmCpu driver to consume PCD > PcdCpuSmmRestrictedMemoryAccess. > Because the behavior controlled by PcdCpuSmmStaticPageTable in original > code is not changed after switching to PcdCpuSmmRestrictedMemoryAccess. >=20 > The functionality is not impacted by this patch. >=20 > Signed-off-by: Ray Ni > Cc: Eric Dong > Cc: Jiewen Yao > Cc: Laszlo Ersek > --- > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 4 +- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 52 ++++++++++++-------- > 2 files changed, 34 insertions(+), 22 deletions(-) >=20 > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > index da0308c47f..b12b2691f8 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf > @@ -133,7 +133,6 @@ [Pcd] > gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugDataAddress ## > SOMETIMES_PRODUCES > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmCodeAccessCheckEnable ## > CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode ## > CONSUMES > - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStaticPageTable ## > CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmShadowStackSize ## > SOMETIMES_CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable ## > CONSUMES >=20 > gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask > ## CONSUMES > @@ -141,6 +140,9 @@ [Pcd] > gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask = ## > CONSUMES > gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask > ## CONSUMES >=20 > +[Pcd.X64] > + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmRestrictedMemoryAccess ## > CONSUMES > + > [Depex] > gEfiMpServiceProtocolGuid >=20 > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index d60c404a3d..7516f35055 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -15,7 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > LIST_ENTRY mPagePool =3D INITIALIZE_LIST_HEAD_V= ARIABLE > (mPagePool); > BOOLEAN m1GPageTableSupport =3D FALSE; > -BOOLEAN mCpuSmmStaticPageTable; > +BOOLEAN mCpuSmmRestrictedMemoryAccess; > BOOLEAN m5LevelPagingSupport; > X86_ASSEMBLY_PATCH_LABEL gPatch5LevelPagingSupport; >=20 > @@ -334,15 +334,15 @@ SmmInitPageTable ( > // > InitializeSpinLock (mPFLock); >=20 > - mCpuSmmStaticPageTable =3D PcdGetBool (PcdCpuSmmStaticPageTable); > - m1GPageTableSupport =3D Is1GPageSupport (); > - m5LevelPagingSupport =3D Is5LevelPagingSupport (); > - mPhysicalAddressBits =3D CalculateMaximumSupportAddress (); > + mCpuSmmRestrictedMemoryAccess =3D PcdGetBool > (PcdCpuSmmRestrictedMemoryAccess); > + m1GPageTableSupport =3D Is1GPageSupport (); > + m5LevelPagingSupport =3D Is5LevelPagingSupport (); > + mPhysicalAddressBits =3D CalculateMaximumSupportAddress (); > PatchInstructionX86 (gPatch5LevelPagingSupport, m5LevelPagingSupport, = 1); > - DEBUG ((DEBUG_INFO, "5LevelPaging Support - %d\n", > m5LevelPagingSupport)); > - DEBUG ((DEBUG_INFO, "1GPageTable Support - %d\n", > m1GPageTableSupport)); > - DEBUG ((DEBUG_INFO, "PcdCpuSmmStaticPageTable - %d\n", > mCpuSmmStaticPageTable)); > - DEBUG ((DEBUG_INFO, "PhysicalAddressBits - %d\n", > mPhysicalAddressBits)); > + DEBUG ((DEBUG_INFO, "5LevelPaging Support - %d\n", > m5LevelPagingSupport)); > + DEBUG ((DEBUG_INFO, "1GPageTable Support - %d\n", > m1GPageTableSupport)); > + DEBUG ((DEBUG_INFO, "PcdCpuSmmRestrictedMemoryAccess - %d\n", > mCpuSmmRestrictedMemoryAccess)); > + DEBUG ((DEBUG_INFO, "PhysicalAddressBits - %d\n", > mPhysicalAddressBits)); > // > // Generate PAE page table for the first 4GB memory space > // > @@ -385,7 +385,11 @@ SmmInitPageTable ( > PTEntry =3D Pml5Entry; > } >=20 > - if (mCpuSmmStaticPageTable) { > + if (mCpuSmmRestrictedMemoryAccess) { > + // > + // When access to non-SMRAM memory is restricted, create page table > + // that covers all memory space. > + // > SetStaticPageTable ((UINTN)PTEntry); > } else { > // > @@ -972,7 +976,7 @@ SmiPFHandler ( >=20 > PFAddress =3D AsmReadCr2 (); >=20 > - if (mCpuSmmStaticPageTable && (PFAddress >=3D LShiftU64 (1, > (mPhysicalAddressBits - 1)))) { > + if (mCpuSmmRestrictedMemoryAccess && (PFAddress >=3D LShiftU64 (1, > + (mPhysicalAddressBits - 1)))) { > DumpCpuContext (InterruptType, SystemContext); > DEBUG ((DEBUG_ERROR, "Do not support address 0x%lx by processor!\n", > PFAddress)); > CpuDeadLoop (); > @@ -1049,7 +1053,7 @@ SmiPFHandler ( > goto Exit; > } >=20 > - if (mCpuSmmStaticPageTable && IsSmmCommBufferForbiddenAddress > (PFAddress)) { > + if (mCpuSmmRestrictedMemoryAccess && > + IsSmmCommBufferForbiddenAddress (PFAddress)) { > DumpCpuContext (InterruptType, SystemContext); > DEBUG ((DEBUG_ERROR, "Access SMM communication forbidden address > (0x%lx)!\n", PFAddress)); > DEBUG_CODE ( > @@ -1100,26 +1104,26 @@ SetPageTableAttributes ( > Enable5LevelPaging =3D (BOOLEAN) (Cr4.Bits.LA57 =3D=3D 1); >=20 > // > - // Don't do this if > - // - no static page table; or > + // Don't mark page table memory as read-only if // - no restriction > + on access to non-SMRAM memory; or > // - SMM heap guard feature enabled; or > // BIT2: SMM page guard enabled > // BIT3: SMM pool guard enabled > // - SMM profile feature enabled > // > - if (!mCpuSmmStaticPageTable || > + if (!mCpuSmmRestrictedMemoryAccess || > ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) !=3D 0) || > FeaturePcdGet (PcdCpuSmmProfileEnable)) { > // > - // Static paging and heap guard could not be enabled at the same tim= e. > + // Restriction on access to non-SMRAM memory and heap guard could no= t > be enabled at the same time. > // > - ASSERT (!(mCpuSmmStaticPageTable && > + ASSERT (!(mCpuSmmRestrictedMemoryAccess && > (PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) !=3D = 0)); >=20 > // > - // Static paging and SMM profile could not be enabled at the same ti= me. > + // Restriction on access to non-SMRAM memory and SMM profile could n= ot > be enabled at the same time. > // > - ASSERT (!(mCpuSmmStaticPageTable && FeaturePcdGet > (PcdCpuSmmProfileEnable))); > + ASSERT (!(mCpuSmmRestrictedMemoryAccess && FeaturePcdGet > + (PcdCpuSmmProfileEnable))); > return ; > } >=20 > @@ -1223,7 +1227,10 @@ SaveCr2 ( > OUT UINTN *Cr2 > ) > { > - if (!mCpuSmmStaticPageTable) { > + if (!mCpuSmmRestrictedMemoryAccess) { > + // > + // On-demand paging is enabled when access to non-SMRAM is not > restricted. > + // > *Cr2 =3D AsmReadCr2 (); > } > } > @@ -1238,7 +1245,10 @@ RestoreCr2 ( > IN UINTN Cr2 > ) > { > - if (!mCpuSmmStaticPageTable) { > + if (!mCpuSmmRestrictedMemoryAccess) { > + // > + // On-demand paging is enabled when access to non-SMRAM is not > restricted. > + // > AsmWriteCr2 (Cr2); > } > } > -- > 2.21.0.windows.1