From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga02.intel.com (mga02.intel.com []) by mx.groups.io with SMTP id smtpd.web12.29704.1605496726136635553 for ; Sun, 15 Nov 2020 19:18:48 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=fail (domain: intel.com, ip: , mailfrom: w.sheng@intel.com) IronPort-SDR: VeUBtwK7VD6i16xnTpRYb3xHd45+zxMZW55BRosGFXdkE1mxTe9r/0giec6QnDqAHPndFsO5Fi Y0Bu/spN9+SQ== X-IronPort-AV: E=McAfee;i="6000,8403,9806"; a="157720789" X-IronPort-AV: E=Sophos;i="5.77,481,1596524400"; d="scan'208";a="157720789" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Nov 2020 19:18:48 -0800 IronPort-SDR: t0M0b/CBzb4mJKmynhv2tPso2IupWPb13I+J++OEmERb94SwA9A8pLOR75/xkb0+EmTFHSwGf0 ttlWncp1JGIg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,481,1596524400"; d="scan'208";a="475389822" Received: from shwdesssddpdwei.ccr.corp.intel.com ([10.239.157.46]) by orsmga004.jf.intel.com with ESMTP; 15 Nov 2020 19:18:45 -0800 From: "Sheng Wei" To: devel@edk2.groups.io Cc: Eric Dong , Ray Ni , Laszlo Ersek , Rahul Kumar , Jiewen Yao Subject: [PATCH v8 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: Reflect page table depth with page table address Date: Mon, 16 Nov 2020 11:18:33 +0800 Message-Id: <20201116031833.14324-3-w.sheng@intel.com> X-Mailer: git-send-email 2.16.2.windows.1 In-Reply-To: <20201116031833.14324-1-w.sheng@intel.com> References: <20201116031833.14324-1-w.sheng@intel.com> When trying to get page table base, if mInternalCr3 is zero, it will use the page table from CR3, and reflect the page table depth by CR4 LA57 bit. If mInternalCr3 is non zero, it will use the page table from mInternalCr3 and reflect the page table depth of mInternalCr3 at same time. In the case of X64, we use m5LevelPagingNeeded to reflect the depth of the page table. And in the case of IA32, it will not the page table depth information. This patch is a bug fix when enable CET feature with 5 level paging. The SMM page tables are allocated / initialized in PiCpuSmmEntry(). When CET is enabled, PiCpuSmmEntry() must further modify the attribute of shadow stack pages. This page table is not set to CR3 in PiCpuSmmEntry(). So the page table base address is set to mInternalCr3 for modifty the page table attribute. It could not use CR4 LA57 bit to reflect the page table depth for mInternalCr3. So we create a architecture-specific implementation GetPageTable() with 2 output parameters. One parameter is used to output the page table address. Another parameter is used to reflect if it is 5 level paging or not. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3015 Signed-off-by: Sheng Wei Cc: Eric Dong Cc: Ray Ni Cc: Laszlo Ersek Cc: Rahul Kumar Cc: Jiewen Yao Reviewed-by: Laszlo Ersek --- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 26 ++++++++++++- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 13 ++++--- UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 29 +++------------ UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 43 ++++++++++++++++++---- 4 files changed, 74 insertions(+), 37 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c index 2483f2ea84..9c8e2d15ac 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c @@ -28,6 +28,26 @@ EnableCet ( VOID ); +/** + Get page table base address and the depth of the page table. + + @param[out] Base Page table base address. + @param[out] FiveLevels TRUE means 5 level paging. FALSE means 4 level paging. +**/ +VOID +GetPageTable ( + OUT UINTN *Base, + OUT BOOLEAN *FiveLevels OPTIONAL + ) +{ + *Base = ((mInternalCr3 == 0) ? + (AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64) : + mInternalCr3); + if (FiveLevels != NULL) { + *FiveLevels = FALSE; + } +} + /** Create PageTable for SMM use. @@ -226,6 +246,7 @@ SetPageTableAttributes ( UINT64 *L1PageTable; UINT64 *L2PageTable; UINT64 *L3PageTable; + UINTN PageTableBase; BOOLEAN IsSplitted; BOOLEAN PageTableSplitted; BOOLEAN CetEnabled; @@ -268,9 +289,10 @@ SetPageTableAttributes ( DEBUG ((DEBUG_INFO, "Start...\n")); PageTableSplitted = FALSE; - L3PageTable = (UINT64 *)GetPageTableBase (); + GetPageTable (&PageTableBase, NULL); + L3PageTable = (UINT64 *)PageTableBase; - SmmSetMemoryAttributesEx ((EFI_PHYSICAL_ADDRESS)(UINTN)L3PageTable, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted); + SmmSetMemoryAttributesEx ((EFI_PHYSICAL_ADDRESS)PageTableBase, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted); PageTableSplitted = (PageTableSplitted || IsSplitted); for (Index3 = 0; Index3 < 4; Index3++) { diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h index 7fb3a2d9e4..b8aa9e1769 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h @@ -263,6 +263,7 @@ extern UINTN mMaxNumberOfCpus; extern UINTN mNumberOfCpus; extern EFI_SMM_CPU_PROTOCOL mSmmCpu; extern EFI_MM_MP_PROTOCOL mSmmMp; +extern UINTN mInternalCr3; /// /// The mode of the CPU at the time an SMI occurs @@ -942,13 +943,15 @@ SetPageTableAttributes ( ); /** - Return page table base. + Get page table base address and the depth of the page table. - @return page table base. + @param[out] Base Page table base address. + @param[out] FiveLevels TRUE means 5 level paging. FALSE means 4 level paging. **/ -UINTN -GetPageTableBase ( - VOID +VOID +GetPageTable ( + OUT UINTN *Base, + OUT BOOLEAN *FiveLevels OPTIONAL ); /** diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c index d67f036aea..766eb1246a 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c @@ -49,22 +49,6 @@ SetPageTableBase ( mInternalCr3 = Cr3; } -/** - Return page table base. - - @return page table base. -**/ -UINTN -GetPageTableBase ( - VOID - ) -{ - if (mInternalCr3 != 0) { - return mInternalCr3; - } - return (AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64); -} - /** Return length according to page attributes. @@ -131,21 +115,20 @@ GetPageTableEntry ( UINT64 *L3PageTable; UINT64 *L4PageTable; UINT64 *L5PageTable; - IA32_CR4 Cr4; + UINTN PageTableBase; BOOLEAN Enable5LevelPaging; + GetPageTable (&PageTableBase, &Enable5LevelPaging); + Index5 = ((UINTN)RShiftU64 (Address, 48)) & PAGING_PAE_INDEX_MASK; Index4 = ((UINTN)RShiftU64 (Address, 39)) & PAGING_PAE_INDEX_MASK; Index3 = ((UINTN)Address >> 30) & PAGING_PAE_INDEX_MASK; Index2 = ((UINTN)Address >> 21) & PAGING_PAE_INDEX_MASK; Index1 = ((UINTN)Address >> 12) & PAGING_PAE_INDEX_MASK; - Cr4.UintN = AsmReadCr4 (); - Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1); - if (sizeof(UINTN) == sizeof(UINT64)) { if (Enable5LevelPaging) { - L5PageTable = (UINT64 *)GetPageTableBase (); + L5PageTable = (UINT64 *)PageTableBase; if (L5PageTable[Index5] == 0) { *PageAttribute = PageNone; return NULL; @@ -153,7 +136,7 @@ GetPageTableEntry ( L4PageTable = (UINT64 *)(UINTN)(L5PageTable[Index5] & ~mAddressEncMask & PAGING_4K_ADDRESS_MASK_64); } else { - L4PageTable = (UINT64 *)GetPageTableBase (); + L4PageTable = (UINT64 *)PageTableBase; } if (L4PageTable[Index4] == 0) { *PageAttribute = PageNone; @@ -162,7 +145,7 @@ GetPageTableEntry ( L3PageTable = (UINT64 *)(UINTN)(L4PageTable[Index4] & ~mAddressEncMask & PAGING_4K_ADDRESS_MASK_64); } else { - L3PageTable = (UINT64 *)GetPageTableBase (); + L3PageTable = (UINT64 *)PageTableBase; } if (L3PageTable[Index3] == 0) { *PageAttribute = PageNone; diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c index 810985df20..cdc1fcefc5 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c @@ -104,6 +104,35 @@ Is5LevelPagingNeeded ( } } +/** + Get page table base address and the depth of the page table. + + @param[out] Base Page table base address. + @param[out] FiveLevels TRUE means 5 level paging. FALSE means 4 level paging. +**/ +VOID +GetPageTable ( + OUT UINTN *Base, + OUT BOOLEAN *FiveLevels OPTIONAL + ) +{ + IA32_CR4 Cr4; + + if (mInternalCr3 == 0) { + *Base = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64; + if (FiveLevels != NULL) { + Cr4.UintN = AsmReadCr4 (); + *FiveLevels = (BOOLEAN)(Cr4.Bits.LA57 == 1); + } + return; + } + + *Base = mInternalCr3; + if (FiveLevels != NULL) { + *FiveLevels = m5LevelPagingNeeded; + } +} + /** Set sub-entries number in entry. @@ -1111,15 +1140,12 @@ SetPageTableAttributes ( UINT64 *L3PageTable; UINT64 *L4PageTable; UINT64 *L5PageTable; + UINTN PageTableBase; BOOLEAN IsSplitted; BOOLEAN PageTableSplitted; BOOLEAN CetEnabled; - IA32_CR4 Cr4; BOOLEAN Enable5LevelPaging; - Cr4.UintN = AsmReadCr4 (); - Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1); - // // Don't mark page table memory as read-only if // - no restriction on access to non-SMRAM memory; or @@ -1163,9 +1189,12 @@ SetPageTableAttributes ( DEBUG ((DEBUG_INFO, "Start...\n")); PageTableSplitted = FALSE; L5PageTable = NULL; + + GetPageTable (&PageTableBase, &Enable5LevelPaging); + if (Enable5LevelPaging) { - L5PageTable = (UINT64 *)GetPageTableBase (); - SmmSetMemoryAttributesEx ((EFI_PHYSICAL_ADDRESS)(UINTN)L5PageTable, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted); + L5PageTable = (UINT64 *)PageTableBase; + SmmSetMemoryAttributesEx ((EFI_PHYSICAL_ADDRESS)PageTableBase, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted); PageTableSplitted = (PageTableSplitted || IsSplitted); } @@ -1176,7 +1205,7 @@ SetPageTableAttributes ( continue; } } else { - L4PageTable = (UINT64 *)GetPageTableBase (); + L4PageTable = (UINT64 *)PageTableBase; } SmmSetMemoryAttributesEx ((EFI_PHYSICAL_ADDRESS)(UINTN)L4PageTable, SIZE_4KB, EFI_MEMORY_RO, &IsSplitted); PageTableSplitted = (PageTableSplitted || IsSplitted); -- 2.16.2.windows.1