From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 E3C8481EBB for ; Wed, 23 Nov 2016 14:28:37 -0800 (PST) Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5E9D17DD06; Wed, 23 Nov 2016 22:28:37 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-97.phx2.redhat.com [10.3.116.97]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uANMSZr7006724; Wed, 23 Nov 2016 17:28:35 -0500 To: Jiewen Yao , edk2-devel@ml01.01.org References: <1479912886-38896-1-git-send-email-jiewen.yao@intel.com> Cc: Michael D Kinney , Jeff Fan From: Laszlo Ersek Message-ID: Date: Wed, 23 Nov 2016 23:28:34 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.0 MIME-Version: 1.0 In-Reply-To: <1479912886-38896-1-git-send-email-jiewen.yao@intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 23 Nov 2016 22:28:37 +0000 (UTC) Subject: Re: [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 22:28:38 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 11/23/16 15:54, Jiewen Yao wrote: > 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(-) I'm not volunteering for a review (I have no clue what a task switch is). I've done regression testing with Ia32 and Ia32X64 OVMF, including repeated suspend/resume. Regression-tested-by: Laszlo Ersek In addition, let me point out that my recent testing of the patch that improves the SMM exception messages: [edk2] [PATCH] UefiCpuPkg/PiSmmCpu: Correct exception message [edk2] [PATCH V3] UefiCpuPkg/PiSmmCpu: Correct exception message. covered the SMM stack overflow case, but -- as I stated explicitly -- only in the Ia32X64 build. See bullet (2) in . The reason is that the SMM stack overflow has occurred to us only when running the EnrollDefaultKeys.efi application, and for that app we only have a "production use" under Ia32X64, not Ia32. Anyway, I have now applied your [edk2] [PATCH V3] UefiCpuPkg/PiSmmCpu: Correct exception message. together with this patch, and attempted to trigger SMM stack overflow on both Ia32 and Ia32X64. - I confirm that the Ia32X64 case continues to work, like before: . For that, Regression-tested-by: Laszlo Ersek - Regarding the Ia32 case, which I haven't tested before, I couldn't trigger an SMM stack overflow with EnrollDefaultKeys.efi, even after reverting the patch that increased the SMM stack size in OVMF (0d0c245dfb14). So, I think I cannot give my Tested-by (I couldn't test the actual new functionality), just my Regression-tested-by (see above). Thanks Laszlo > 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); > } > > - >