public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] UefiCpuPkg/PiSmmCpu: Fixed #double fault on #page fault.
@ 2016-11-23 14:54 Jiewen Yao
  2016-11-23 22:28 ` Laszlo Ersek
  0 siblings, 1 reply; 3+ messages in thread
From: Jiewen Yao @ 2016-11-23 14:54 UTC (permalink / raw)
  To: edk2-devel; +Cc: Laszlo Ersek, Jeff Fan, Michael D Kinney

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 <lersek@redhat.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
 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



^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-11-24  0:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-23 14:54 [PATCH] UefiCpuPkg/PiSmmCpu: Fixed #double fault on #page fault Jiewen Yao
2016-11-23 22:28 ` Laszlo Ersek
     [not found]   ` <ED077930C258884BBCB450DB737E6622486AB46E@shsmsx102.ccr.corp.intel.com>
2016-11-24  0:45     ` Yao, Jiewen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox