From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (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 D3D5981C65 for ; Wed, 23 Nov 2016 06:54:52 -0800 (PST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP; 23 Nov 2016 06:54:52 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,538,1473145200"; d="scan'208";a="789943903" Received: from jyao1-mobl.ccr.corp.intel.com ([10.254.215.70]) by FMSMGA003.fm.intel.com with ESMTP; 23 Nov 2016 06:54:50 -0800 From: Jiewen Yao To: edk2-devel@lists.01.org Cc: Laszlo Ersek , Jeff Fan , Michael D Kinney Date: Wed, 23 Nov 2016 22:54:46 +0800 Message-Id: <1479912886-38896-1-git-send-email-jiewen.yao@intel.com> X-Mailer: git-send-email 2.7.4.windows.1 Subject: [PATCH] UefiCpuPkg/PiSmmCpu: Fixed #double fault on #page fault. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Nov 2016 14:54:52 -0000 This patch fixes https://bugzilla.tianocore.org/show_bug.cgi?id=246 Previously, when SMM exception happens after EndOfDxe, with StackGuard enabled on IA32, the #double fault exception is reported instead of #page fault. Root cause is below: Current EDKII SMM page protection will lock GDT. If IA32 stack guard is enabled, the page fault handler will do task switch. This task switch need write busy flag in GDT, and write TSS. However, the GDT and TSS is locked at that time, so the double fault happens. This issue does not exist on X64, or IA32 without StackGuard. In this enhanced solution, we create a special GDT whose CS/DS is at read-only page, and whose TSS segment is at read-write page. We also put TSS into a read-write page. The detail is at UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c. Then the normal code can being protected, because it does not touch TSS. When page fault happens, the TSS region can be written. This fix is IA32 StackGuard specific. Cc: Laszlo Ersek Cc: Jeff Fan Cc: Michael D Kinney Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jiewen Yao --- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S | 2 +- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm | 2 +- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 2 +- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.S | 4 +- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.asm | 4 +- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm | 4 +- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 207 ++++++++++++++++---- UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 7 +- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 78 +++++++- UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 48 ----- UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 67 ++++++- 11 files changed, 311 insertions(+), 114 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S index 0c07558..e27080d 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S @@ -41,7 +41,7 @@ ASM_GLOBAL ASM_PFX(gSmiHandlerIdtr) .equ PROTECT_MODE_CS, 0x08 .equ PROTECT_MODE_DS, 0x20 -.equ TSS_SEGMENT, 0x40 +.equ TSS_SEGMENT, 0x1000 .text diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm index eda1708..e1ab470 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm @@ -36,7 +36,7 @@ DSC_OTHERSEG EQU 20 PROTECT_MODE_CS EQU 08h PROTECT_MODE_DS EQU 20h -TSS_SEGMENT EQU 40h +TSS_SEGMENT EQU 1000h SmiRendezvous PROTO C CpuSmmDebugEntry PROTO C diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm index d50a317..ccfb0d4 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm @@ -32,7 +32,7 @@ %define PROTECT_MODE_CS 0x8 %define PROTECT_MODE_DS 0x20 -%define TSS_SEGMENT 0x40 +%define TSS_SEGMENT 0x1000 extern ASM_PFX(SmiRendezvous) extern ASM_PFX(FeaturePcdGet (PcdCpuSmmStackGuard)) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.S b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.S index cf5ef82..30dd25e 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.S +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.S @@ -99,8 +99,8 @@ ExceptionTssSeg: .equ CODE_SEL, CodeSeg32 - NullSeg .equ DATA_SEL, DataSeg32 - NullSeg -.equ TSS_SEL, TssSeg - NullSeg -.equ EXCEPTION_TSS_SEL, ExceptionTssSeg - NullSeg +.equ TSS_SEL, 0x1000 +.equ EXCEPTION_TSS_SEL, 0x1008 # IA32 TSS fields .equ TSS_ESP0, 4 diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.asm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.asm index 7b162f8..740e4a5 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.asm +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.asm @@ -102,8 +102,8 @@ ExceptionTssSeg LABEL QWORD CODE_SEL = offset CodeSeg32 - offset NullSeg DATA_SEL = offset DataSeg32 - offset NullSeg -TSS_SEL = offset TssSeg - offset NullSeg -EXCEPTION_TSS_SEL = offset ExceptionTssSeg - offset NullSeg +TSS_SEL EQU 1000h +EXCEPTION_TSS_SEL EQU 1008h IA32_TSS STRUC DW ? diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm index 4d58999..a6dcf47 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm @@ -98,8 +98,8 @@ ExceptionTssSeg: CODE_SEL equ CodeSeg32 - NullSeg DATA_SEL equ DataSeg32 - NullSeg -TSS_SEL equ TssSeg - NullSeg -EXCEPTION_TSS_SEL equ ExceptionTssSeg - NullSeg +%define TSS_SEL 0x1000 +%define EXCEPTION_TSS_SEL 0x1008 struc IA32_TSS resw 1 diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c index f4db6c8..8e25b49 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c @@ -43,17 +43,15 @@ InitializeIDTSmmStackGuard ( /** Initialize Gdt for all processors. - + @param[in] Cr3 CR3 value. - @param[out] GdtStepSize The step size for GDT table. - @return GdtBase for processor 0. - GdtBase for processor X is: GdtBase + (GdtStepSize * X) + @return A GdtArray to hold all base of Gdt Table for all processors. + GdtBase for processor X is: GdtArray[X] **/ -VOID * +UINTN * InitGdt ( - IN UINTN Cr3, - OUT UINTN *GdtStepSize + IN UINTN Cr3 ) { UINTN Index; @@ -62,6 +60,16 @@ InitGdt ( UINTN GdtTssTableSize; UINT8 *GdtTssTables; UINTN GdtTableStepSize; + UINTN *GdtArray; + UINTN GdtCountPerPage; + UINTN BundleCount; + UINTN BundleIndex; + UINT8 *BundleBase; + UINTN GdtIndex; + UINTN CountMax; + + GdtArray = AllocatePool (sizeof(UINTN) * gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus); + ASSERT (GdtArray != NULL); if (FeaturePcdGet (PcdCpuSmmStackGuard)) { // @@ -71,41 +79,92 @@ InitGdt ( // // - // Enlarge GDT to contain 2 TSS descriptors + // We must separate normal GDT entry from GDT_TSS entry. + // The formal could be read only, and the later must be readwrite. + // The final layout is below: + // + // |<------ Read Only Bundle ------>|<--------- Read Write Bundle -------------->| + // | | | + // +--------------------------------+============================================+--------------+==================+ + // | GDT(0) | GDT(1) | ... | GDT(n) | GDT_TSS(0) | GDT_TSS(1) | ... | GDT_TSS(n) | GDT(n+1) |...| GDT_TSS(n+1) |...| + // +--------------------------------+============================================+--------------+==================+ + // | | | + // |<------------ Gdt.Limit ------------>|<-TSS->| // - gcSmiGdtr.Limit += (UINT16)(2 * sizeof (IA32_SEGMENT_DESCRIPTOR)); - GdtTssTableSize = (gcSmiGdtr.Limit + 1 + TSS_SIZE * 2 + 7) & ~7; // 8 bytes aligned - mGdtBufferSize = GdtTssTableSize * gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus; - GdtTssTables = (UINT8*)AllocateCodePages (EFI_SIZE_TO_PAGES (mGdtBufferSize)); + GdtTableStepSize = (2 * sizeof (IA32_SEGMENT_DESCRIPTOR) + TSS_SIZE * 2 + 7) & ~7; // 8 bytes aligned + if (GdtTableStepSize < (UINTN)(gcSmiGdtr.Limit + 1)) { + GdtTableStepSize = gcSmiGdtr.Limit + 1; + } + GdtCountPerPage = EFI_PAGE_SIZE / GdtTableStepSize; + BundleCount = (gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus + GdtCountPerPage - 1) / GdtCountPerPage; + mGdtBufferSize = EFI_PAGES_TO_SIZE(BundleCount * 2); + + GdtTssTables = (UINT8*)AllocateCodePages (BundleCount * 2); ASSERT (GdtTssTables != NULL); + ZeroMem ((VOID *)GdtTssTables, EFI_PAGES_TO_SIZE(BundleCount * 2)); mGdtBuffer = (UINTN)GdtTssTables; - GdtTableStepSize = GdtTssTableSize; - for (Index = 0; Index < gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus; Index++) { - CopyMem (GdtTssTables + GdtTableStepSize * Index, (VOID*)(UINTN)gcSmiGdtr.Base, gcSmiGdtr.Limit + 1 + TSS_SIZE * 2); - // - // Fixup TSS descriptors - // - TssBase = (UINTN)(GdtTssTables + GdtTableStepSize * Index + gcSmiGdtr.Limit + 1); - GdtDescriptor = (IA32_SEGMENT_DESCRIPTOR *)(TssBase) - 2; - GdtDescriptor->Bits.BaseLow = (UINT16)TssBase; - GdtDescriptor->Bits.BaseMid = (UINT8)(TssBase >> 16); - GdtDescriptor->Bits.BaseHigh = (UINT8)(TssBase >> 24); - - TssBase += TSS_SIZE; - GdtDescriptor++; - GdtDescriptor->Bits.BaseLow = (UINT16)TssBase; - GdtDescriptor->Bits.BaseMid = (UINT8)(TssBase >> 16); - GdtDescriptor->Bits.BaseHigh = (UINT8)(TssBase >> 24); - // - // Fixup TSS segments - // - // ESP as known good stack - // - *(UINTN *)(TssBase + TSS_IA32_ESP_OFFSET) = mSmmStackArrayBase + EFI_PAGE_SIZE + Index * mSmmStackSize; - *(UINT32 *)(TssBase + TSS_IA32_CR3_OFFSET) = Cr3; + Index = 0; + for (BundleIndex = 0; BundleIndex < BundleCount; BundleIndex++) { + CountMax = GdtCountPerPage; + if (BundleIndex == BundleCount - 1) { + CountMax = gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus - GdtCountPerPage * BundleIndex; + } + BundleBase = GdtTssTables + EFI_PAGE_SIZE * 2 * BundleIndex; + for (GdtIndex = 0; GdtIndex < CountMax; GdtIndex++, Index++) { + GdtArray[Index] = (UINTN)BundleBase + GdtTableStepSize * GdtIndex; + // + // Copy Basic GDT + // + CopyMem ( + (VOID *)GdtArray[Index], + (VOID*)(UINTN)gcSmiGdtr.Base, + gcSmiGdtr.Limit + 1 + ); + // + // Copy GDT TSS segment + // + CopyMem ( + (VOID *)(GdtArray[Index] + EFI_PAGE_SIZE), + (VOID*)((UINTN)gcSmiGdtr.Base + gcSmiGdtr.Limit + 1), + sizeof(IA32_SEGMENT_DESCRIPTOR) * 2 + ); + // + // Copy TSS + // + TssBase = (UINTN)(GdtArray[Index] + EFI_PAGE_SIZE + sizeof(IA32_SEGMENT_DESCRIPTOR) * 2); + CopyMem ( + (VOID *)(TssBase), + (VOID*)((UINTN)gcSmiGdtr.Base + gcSmiGdtr.Limit + 1 + sizeof(IA32_SEGMENT_DESCRIPTOR) * 2), + TSS_SIZE * 2 + ); + // + // Fixup TSS descriptors + // + GdtDescriptor = (IA32_SEGMENT_DESCRIPTOR *)(TssBase) - 2; + GdtDescriptor->Bits.BaseLow = (UINT16)TssBase; + GdtDescriptor->Bits.BaseMid = (UINT8)(TssBase >> 16); + GdtDescriptor->Bits.BaseHigh = (UINT8)(TssBase >> 24); + + TssBase += TSS_SIZE; + GdtDescriptor++; + GdtDescriptor->Bits.BaseLow = (UINT16)TssBase; + GdtDescriptor->Bits.BaseMid = (UINT8)(TssBase >> 16); + GdtDescriptor->Bits.BaseHigh = (UINT8)(TssBase >> 24); + // + // Fixup TSS segments + // + // ESP as known good stack + // + *(UINTN *)(TssBase + TSS_IA32_ESP_OFFSET) = mSmmStackArrayBase + EFI_PAGE_SIZE + Index * mSmmStackSize; + *(UINT32 *)(TssBase + TSS_IA32_CR3_OFFSET) = Cr3; + } } + // + // Enlarge GDT to contain 1 page plus 2 TSS descriptors + // + gcSmiGdtr.Limit = (UINT16)(EFI_PAGE_SIZE + 2 * sizeof (IA32_SEGMENT_DESCRIPTOR) - 1); } else { // // Just use original table, AllocatePage and copy them here to make sure GDTs are covered in page memory. @@ -119,11 +178,83 @@ InitGdt ( for (Index = 0; Index < gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus; Index++) { CopyMem (GdtTssTables + GdtTableStepSize * Index, (VOID*)(UINTN)gcSmiGdtr.Base, gcSmiGdtr.Limit + 1); + GdtArray[Index] = (UINTN)GdtTssTables + GdtTableStepSize * Index; + } + } + + return GdtArray; +} + +/** + This function sets GDT/IDT buffer to be RO and XP. +**/ +VOID +PatchGdtIdtMap ( + VOID + ) +{ + EFI_PHYSICAL_ADDRESS BaseAddress; + UINTN Size; + UINTN Index; + UINTN BundleCount; + + // + // GDT + // + DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - GDT:\n")); + + if (FeaturePcdGet (PcdCpuSmmStackGuard)) { + BaseAddress = mGdtBuffer; + Size = ALIGN_VALUE(mGdtBufferSize, SIZE_4KB); + SmmSetMemoryAttributes ( + BaseAddress, + Size, + EFI_MEMORY_XP + ); + BundleCount = EFI_SIZE_TO_PAGES(mGdtBufferSize)/2; + for (Index = 0; Index < BundleCount; Index++) { + SmmSetMemoryAttributes ( + BaseAddress + EFI_PAGE_SIZE * Index * 2, + EFI_PAGE_SIZE, + EFI_MEMORY_RO + ); + SmmClearMemoryAttributes ( + BaseAddress + EFI_PAGE_SIZE * (Index * 2 + 1), + EFI_PAGE_SIZE, + EFI_MEMORY_RO + ); } + } else { + BaseAddress = mGdtBuffer; + Size = ALIGN_VALUE(mGdtBufferSize, SIZE_4KB); + SmmSetMemoryAttributes ( + BaseAddress, + Size, + EFI_MEMORY_RO + ); + SmmSetMemoryAttributes ( + BaseAddress, + Size, + EFI_MEMORY_XP + ); } + // + // IDT + // + DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - IDT:\n")); - *GdtStepSize = GdtTableStepSize; - return GdtTssTables; + BaseAddress = gcSmiIdtr.Base; + Size = ALIGN_VALUE(gcSmiIdtr.Limit + 1, SIZE_4KB); + SmmSetMemoryAttributes ( + BaseAddress, + Size, + EFI_MEMORY_RO + ); + SmmSetMemoryAttributes ( + BaseAddress, + Size, + EFI_MEMORY_XP + ); } /** diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c index 9b8db90..591a18c 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c @@ -1377,8 +1377,7 @@ InitializeMpServiceData ( UINTN Index; MTRR_SETTINGS *Mtrr; PROCESSOR_SMM_DESCRIPTOR *Psd; - UINT8 *GdtTssTables; - UINTN GdtTableStepSize; + UINTN *GdtArray; // // Allocate memory for all locks and semaphores @@ -1407,7 +1406,7 @@ InitializeMpServiceData ( // Cr3 = SmmInitPageTable (); - GdtTssTables = InitGdt (Cr3, &GdtTableStepSize); + GdtArray = InitGdt (Cr3); // // Initialize PROCESSOR_SMM_DESCRIPTOR for each CPU @@ -1415,7 +1414,7 @@ InitializeMpServiceData ( for (Index = 0; Index < mMaxNumberOfCpus; Index++) { Psd = (PROCESSOR_SMM_DESCRIPTOR *)(VOID *)(UINTN)(mCpuHotPlugData.SmBase[Index] + SMM_PSD_OFFSET); CopyMem (Psd, &gcPsd, sizeof (gcPsd)); - Psd->SmmGdtPtr = (UINT64)(UINTN)(GdtTssTables + GdtTableStepSize * Index); + Psd->SmmGdtPtr = (UINT64)(UINTN)GdtArray[Index]; Psd->SmmGdtSize = gcSmiGdtr.Limit + 1; // diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h index abe5cc6..9e00022 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h @@ -512,15 +512,21 @@ InitializeIDTSmmStackGuard ( Initialize Gdt for all processors. @param[in] Cr3 CR3 value. - @param[out] GdtStepSize The step size for GDT table. - @return GdtBase for processor 0. - GdtBase for processor X is: GdtBase + (GdtStepSize * X) + @return A GdtArray to hold all base of Gdt Table for all processors. + GdtBase for processor X is: GdtArray[X] **/ -VOID * +UINTN * InitGdt ( - IN UINTN Cr3, - OUT UINTN *GdtStepSize + IN UINTN Cr3 + ); + +/** + This function sets GDT/IDT buffer to be RO and XP. +**/ +VOID +PatchGdtIdtMap ( + VOID ); /** @@ -596,6 +602,66 @@ SmmBlockingStartupThisAp ( ); /** + This function sets the attributes for the memory region specified by BaseAddress and + Length from their current attributes to the attributes specified by Attributes. + + @param[in] BaseAddress The physical address that is the start address of a memory region. + @param[in] Length The size in bytes of the memory region. + @param[in] Attributes The bit mask of attributes to set for the memory region. + + @retval EFI_SUCCESS The attributes were set for the memory region. + @retval EFI_ACCESS_DENIED The attributes for the memory resource range specified by + BaseAddress and Length cannot be modified. + @retval EFI_INVALID_PARAMETER Length is zero. + Attributes specified an illegal combination of attributes that + cannot be set together. + @retval EFI_OUT_OF_RESOURCES There are not enough system resources to modify the attributes of + the memory resource range. + @retval EFI_UNSUPPORTED The processor does not support one or more bytes of the memory + resource range specified by BaseAddress and Length. + The bit mask of attributes is not support for the memory resource + range specified by BaseAddress and Length. + +**/ +EFI_STATUS +EFIAPI +SmmSetMemoryAttributes ( + IN EFI_PHYSICAL_ADDRESS BaseAddress, + IN UINT64 Length, + IN UINT64 Attributes + ); + +/** + This function clears the attributes for the memory region specified by BaseAddress and + Length from their current attributes to the attributes specified by Attributes. + + @param[in] BaseAddress The physical address that is the start address of a memory region. + @param[in] Length The size in bytes of the memory region. + @param[in] Attributes The bit mask of attributes to clear for the memory region. + + @retval EFI_SUCCESS The attributes were cleared for the memory region. + @retval EFI_ACCESS_DENIED The attributes for the memory resource range specified by + BaseAddress and Length cannot be modified. + @retval EFI_INVALID_PARAMETER Length is zero. + Attributes specified an illegal combination of attributes that + cannot be set together. + @retval EFI_OUT_OF_RESOURCES There are not enough system resources to modify the attributes of + the memory resource range. + @retval EFI_UNSUPPORTED The processor does not support one or more bytes of the memory + resource range specified by BaseAddress and Length. + The bit mask of attributes is not support for the memory resource + range specified by BaseAddress and Length. + +**/ +EFI_STATUS +EFIAPI +SmmClearMemoryAttributes ( + IN EFI_PHYSICAL_ADDRESS BaseAddress, + IN UINT64 Length, + IN UINT64 Attributes + ); + +/** Initialize MP synchronization data. **/ diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c index c85e025..fc440e4 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c @@ -750,54 +750,6 @@ PatchSmmSaveStateMap ( } /** - This function sets GDT/IDT buffer to be RO and XP. -**/ -VOID -PatchGdtIdtMap ( - VOID - ) -{ - EFI_PHYSICAL_ADDRESS BaseAddress; - UINTN Size; - - // - // GDT - // - DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - GDT:\n")); - - BaseAddress = mGdtBuffer; - Size = ALIGN_VALUE(mGdtBufferSize, SIZE_4KB); - SmmSetMemoryAttributes ( - BaseAddress, - Size, - EFI_MEMORY_RO - ); - SmmSetMemoryAttributes ( - BaseAddress, - Size, - EFI_MEMORY_XP - ); - - // - // IDT - // - DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - IDT:\n")); - - BaseAddress = gcSmiIdtr.Base; - Size = ALIGN_VALUE(gcSmiIdtr.Limit + 1, SIZE_4KB); - SmmSetMemoryAttributes ( - BaseAddress, - Size, - EFI_MEMORY_RO - ); - SmmSetMemoryAttributes ( - BaseAddress, - Size, - EFI_MEMORY_XP - ); -} - -/** This function sets memory attribute according to MemoryAttributesTable. **/ VOID diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c index 9fc00c1..baeec4a 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c @@ -42,15 +42,13 @@ InitializeIDTSmmStackGuard ( Initialize Gdt for all processors. @param[in] Cr3 CR3 value. - @param[out] GdtStepSize The step size for GDT table. - @return GdtBase for processor 0. - GdtBase for processor X is: GdtBase + (GdtStepSize * X) + @return A GdtArray to hold all base of Gdt Table for all processors. + GdtBase for processor X is: GdtArray[X] **/ -VOID * +UINTN * InitGdt ( - IN UINTN Cr3, - OUT UINTN *GdtStepSize + IN UINTN Cr3 ) { UINTN Index; @@ -59,6 +57,10 @@ InitGdt ( UINTN GdtTssTableSize; UINT8 *GdtTssTables; UINTN GdtTableStepSize; + UINTN *GdtArray; + + GdtArray = AllocatePool (sizeof(UINTN) * gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus); + ASSERT (GdtArray != NULL); // // For X64 SMM, we allocate separate GDT/TSS for each CPUs to avoid TSS load contention @@ -89,10 +91,58 @@ InitGdt ( // *(UINTN *)(TssBase + TSS_X64_IST1_OFFSET) = (mSmmStackArrayBase + EFI_PAGE_SIZE + Index * mSmmStackSize); } + GdtArray[Index] = (UINTN)GdtTssTables + GdtTableStepSize * Index; } - *GdtStepSize = GdtTableStepSize; - return GdtTssTables; + return GdtArray; +} + +/** + This function sets GDT/IDT buffer to be RO and XP. +**/ +VOID +PatchGdtIdtMap ( + VOID + ) +{ + EFI_PHYSICAL_ADDRESS BaseAddress; + UINTN Size; + + // + // GDT + // + DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - GDT:\n")); + + BaseAddress = mGdtBuffer; + Size = ALIGN_VALUE(mGdtBufferSize, SIZE_4KB); + SmmSetMemoryAttributes ( + BaseAddress, + Size, + EFI_MEMORY_RO + ); + SmmSetMemoryAttributes ( + BaseAddress, + Size, + EFI_MEMORY_XP + ); + + // + // IDT + // + DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - IDT:\n")); + + BaseAddress = gcSmiIdtr.Base; + Size = ALIGN_VALUE(gcSmiIdtr.Limit + 1, SIZE_4KB); + SmmSetMemoryAttributes ( + BaseAddress, + Size, + EFI_MEMORY_RO + ); + SmmSetMemoryAttributes ( + BaseAddress, + Size, + EFI_MEMORY_XP + ); } /** @@ -154,4 +204,3 @@ TransferApToSafeState ( ASSERT (FALSE); } - -- 2.7.4.windows.1