Reviewed-by: Eric Dong <eric.d...@intel.com>
> -----Original Message-----
> From: Ni, Ray
> Sent: Monday, August 26, 2019 6:45 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.d...@intel.com>; Yao, Jiewen <jiewen....@intel.com>;
> Laszlo Ersek <ler...@redhat.com>
> Subject: [PATCH 2/5] UefiCpuPkg/PiSmmCpu: Use new PCD
> PcdCpuSmmRestrictedMemoryAccess
>
> 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.
>
> The functionality is not impacted by this patch.
>
> Signed-off-by: Ray Ni <ray...@intel.com>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Jiewen Yao <jiewen....@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 4 +-
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 52 ++++++++++++--------
> 2 files changed, 34 insertions(+), 22 deletions(-)
>
> 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
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask
> ## CONSUMES
> @@ -141,6 +140,9 @@ [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ##
> CONSUMES
> gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask
> ## CONSUMES
>
> +[Pcd.X64]
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmRestrictedMemoryAccess ##
> CONSUMES
> +
> [Depex]
> gEfiMpServiceProtocolGuid
>
> 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
>
> LIST_ENTRY mPagePool = INITIALIZE_LIST_HEAD_VARIABLE
> (mPagePool);
> BOOLEAN m1GPageTableSupport = FALSE;
> -BOOLEAN mCpuSmmStaticPageTable;
> +BOOLEAN mCpuSmmRestrictedMemoryAccess;
> BOOLEAN m5LevelPagingSupport;
> X86_ASSEMBLY_PATCH_LABEL gPatch5LevelPagingSupport;
>
> @@ -334,15 +334,15 @@ SmmInitPageTable (
> //
> InitializeSpinLock (mPFLock);
>
> - mCpuSmmStaticPageTable = PcdGetBool (PcdCpuSmmStaticPageTable);
> - m1GPageTableSupport = Is1GPageSupport ();
> - m5LevelPagingSupport = Is5LevelPagingSupport ();
> - mPhysicalAddressBits = CalculateMaximumSupportAddress ();
> + mCpuSmmRestrictedMemoryAccess = PcdGetBool
> (PcdCpuSmmRestrictedMemoryAccess);
> + m1GPageTableSupport = Is1GPageSupport ();
> + m5LevelPagingSupport = Is5LevelPagingSupport ();
> + mPhysicalAddressBits = 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 = Pml5Entry;
> }
>
> - 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 (
>
> PFAddress = AsmReadCr2 ();
>
> - if (mCpuSmmStaticPageTable && (PFAddress >= LShiftU64 (1,
> (mPhysicalAddressBits - 1)))) {
> + if (mCpuSmmRestrictedMemoryAccess && (PFAddress >= 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;
> }
>
> - 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 = (BOOLEAN) (Cr4.Bits.LA57 == 1);
>
> //
> - // 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)) != 0) ||
> FeaturePcdGet (PcdCpuSmmProfileEnable)) {
> //
> - // Static paging and heap guard could not be enabled at the same time.
> + // Restriction on access to non-SMRAM memory and heap guard could not
> be enabled at the same time.
> //
> - ASSERT (!(mCpuSmmStaticPageTable &&
> + ASSERT (!(mCpuSmmRestrictedMemoryAccess &&
> (PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0));
>
> //
> - // Static paging and SMM profile could not be enabled at the same time.
> + // Restriction on access to non-SMRAM memory and SMM profile could not
> be enabled at the same time.
> //
> - ASSERT (!(mCpuSmmStaticPageTable && FeaturePcdGet
> (PcdCpuSmmProfileEnable)));
> + ASSERT (!(mCpuSmmRestrictedMemoryAccess && FeaturePcdGet
> + (PcdCpuSmmProfileEnable)));
> return ;
> }
>
> @@ -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 = 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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46396): https://edk2.groups.io/g/devel/message/46396
Mute This Topic: https://groups.io/mt/33027104/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
