[PATCH 0/5] refine Smm range code in BoardX58Ich10

[PATCH 0/5] refine Smm range code in BoardX58Ich10

[Zhiguang Liu]

In BoardX58Ich10 platform, two modules has hard-code about how SMM
range should be, and this causes a issue since PEI phase may change
SMM ranges now. This patch set refine Smm range related code.

Zhiguang Liu (5):
  SimicsOpenBoardPkg: Build gEfiSmmSmramMemoryGuid Hob in S3 path
  SimicsOpenBoardPkg: Move AcpiVariableGuid hob to MemDetect
  SimicsOpenBoardPkg: Use SmmAccessLib instead of SmmAccessPei.inf
  SimicsOpenBoardPkg: Use another SmmAccess Driver
  SimicsX58SktPkg: Remove unused Smm related modules

 .../BoardX58Ich10/OpenBoardPkg.dsc            |   9 +-
 .../BoardX58Ich10/OpenBoardPkg.fdf            |   1 -
 .../SimicsOpenBoardPkg/SimicsPei/MemDetect.c  | 124 +++----
 .../SimicsPei/SimicsPei.inf                   |   3 +
 .../SimicsX58SktPkg/SktUefiBootInclude.fdf    |   2 +-
 .../Smm/Access/SmmAccess2Dxe.c                | 148 --------
 .../Smm/Access/SmmAccess2Dxe.inf              |  54 ---
 .../SimicsX58SktPkg/Smm/Access/SmmAccessPei.c | 346 ------------------
 .../Smm/Access/SmmAccessPei.inf               |  65 ----
 .../Smm/Access/SmramInternal.c                | 200 ----------
 .../Smm/Access/SmramInternal.h                |  82 -----
 11 files changed, 64 insertions(+), 970 deletions(-)
 delete mode 100644 Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.c
 delete mode 100644 Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.inf
 delete mode 100644 Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
 delete mode 100644 Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
 delete mode 100644 Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.c
 delete mode 100644 Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.h

-- 
2.31.1.windows.1

[PATCH 1/5] SimicsOpenBoardPkg: Build gEfiSmmSmramMemoryGuid Hob in S3 path

[Zhiguang Liu]

gEfiSmmSmramMemoryGuid Hob is needed for SmmRelocation feature
even for S3 path. So in MemDetect.c, remove specical code path
for S3 about creating gEfiSmmSmramMemoryGuid Hob and adding some
memory descriptor, which does no harm in S3 path.

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 .../SimicsOpenBoardPkg/SimicsPei/MemDetect.c  | 107 +++++++-----------
 1 file changed, 42 insertions(+), 65 deletions(-)

diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
index 127afffc00..d80ac1d213 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
+++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
@@ -405,79 +405,56 @@ QemuInitializeRam (
   LowerMemorySize = GetSystemMemorySizeBelow4gb ();
   UpperMemorySize = GetSystemMemorySizeAbove4gb ();
 
-  if (mBootMode == BOOT_ON_S3_RESUME) {
-    //
-    // Create the following memory HOB as an exception on the S3 boot path.
-    //
-    // Normally we'd create memory HOBs only on the normal boot path. However,
-    // CpuMpPei specifically needs such a low-memory HOB on the S3 path as
-    // well, for "borrowing" a subset of it temporarily, for the AP startup
-    // vector.
-    //
-    // CpuMpPei saves the original contents of the borrowed area in permanent
-    // PEI RAM, in a backup buffer allocated with the normal PEI services.
-    // CpuMpPei restores the original contents ("returns" the borrowed area) at
-    // End-of-PEI. End-of-PEI in turn is emitted by S3Resume2Pei before
-    // transferring control to the OS's wakeup vector in the FACS.
-    //
-    // We expect any other PEIMs that "borrow" memory similarly to CpuMpPei to
-    // restore the original contents. Furthermore, we expect all such PEIMs
-    // (CpuMpPei included) to claim the borrowed areas by producing memory
-    // allocation HOBs, and to honor preexistent memory allocation HOBs when
-    // looking for an area to borrow.
-    //
-    AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
-  } else {
-    //
-    // Create memory HOBs
-    //
-    AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
+  //
+  // Create memory HOBs
+  //
+  AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
 
-    if (FeaturePcdGet (PcdSmmSmramRequire)) {
-      UINT32 TsegSize;
+  if (FeaturePcdGet (PcdSmmSmramRequire)) {
+    UINT32 TsegSize;
 
-      TsegSize = mX58TsegMbytes * SIZE_1MB;
-      AddMemoryRangeHob (BASE_1MB, LowerMemorySize - TsegSize);
-      AddReservedMemoryBaseSizeHob (LowerMemorySize - TsegSize, TsegSize,
-        TRUE);
+    TsegSize = mX58TsegMbytes * SIZE_1MB;
+    AddMemoryRangeHob (BASE_1MB, LowerMemorySize - TsegSize);
+    AddReservedMemoryBaseSizeHob (LowerMemorySize - TsegSize, TsegSize,
+      TRUE);
 
-	  BufferSize = sizeof(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK);
-	  SmramRanges = 1;
+    BufferSize = sizeof(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK);
+    SmramRanges = 1;
 
-      Hob.Raw = BuildGuidHob(
-          &gEfiSmmSmramMemoryGuid,
-          BufferSize
-      );
-      ASSERT(Hob.Raw);
-
-      SmramHobDescriptorBlock = (EFI_SMRAM_HOB_DESCRIPTOR_BLOCK *)(Hob.Raw);
-      SmramHobDescriptorBlock->NumberOfSmmReservedRegions = SmramRanges;
-
-      SmramIndex = 0;
-      for (Index = 0; Index < SmramRanges; Index++) {
-        //
-        // This is an SMRAM range, create an SMRAM descriptor
-        //
-        SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalStart = LowerMemorySize - TsegSize;
-        SmramHobDescriptorBlock->Descriptor[SmramIndex].CpuStart = LowerMemorySize - TsegSize;
-        SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalSize = TsegSize;
-        SmramHobDescriptorBlock->Descriptor[SmramIndex].RegionState = EFI_SMRAM_CLOSED | EFI_CACHEABLE;
-        SmramIndex++;
-      }
+    Hob.Raw = BuildGuidHob(
+        &gEfiSmmSmramMemoryGuid,
+        BufferSize
+    );
+    ASSERT(Hob.Raw);
 
-    } else {
-      AddMemoryRangeHob (BASE_1MB, LowerMemorySize);
-    }
+    SmramHobDescriptorBlock = (EFI_SMRAM_HOB_DESCRIPTOR_BLOCK *)(Hob.Raw);
+    SmramHobDescriptorBlock->NumberOfSmmReservedRegions = SmramRanges;
 
-    //
-    // If QEMU presents an E820 map, then create memory HOBs for the >=4GB RAM
-    // entries. Otherwise, create a single memory HOB with the flat >=4GB
-    // memory size read from the CMOS.
-    //
-    if (UpperMemorySize != 0) {
-      AddMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
+    SmramIndex = 0;
+    for (Index = 0; Index < SmramRanges; Index++) {
+      //
+      // This is an SMRAM range, create an SMRAM descriptor
+      //
+      SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalStart = LowerMemorySize - TsegSize;
+      SmramHobDescriptorBlock->Descriptor[SmramIndex].CpuStart = LowerMemorySize - TsegSize;
+      SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalSize = TsegSize;
+      SmramHobDescriptorBlock->Descriptor[SmramIndex].RegionState = EFI_SMRAM_CLOSED | EFI_CACHEABLE;
+      SmramIndex++;
     }
+
+  } else {
+    AddMemoryRangeHob (BASE_1MB, LowerMemorySize);
   }
+
+  //
+  // If QEMU presents an E820 map, then create memory HOBs for the >=4GB RAM
+  // entries. Otherwise, create a single memory HOB with the flat >=4GB
+  // memory size read from the CMOS.
+  //
+  if (UpperMemorySize != 0) {
+    AddMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
+  }
+
 }
 
 /**
-- 
2.31.1.windows.1

Re: [PATCH 1/5] SimicsOpenBoardPkg: Build gEfiSmmSmramMemoryGuid Hob in S3 path

[Ni, Ray]

Zhiguang,
Can you please keep the comments that explain why below 1MB memory resource
should be added for QSP platform?

Another question not related to your changes:
  why "AddMemoryRangeHob (BASE_1MB, LowerMemorySize);" is only called when
  PcdSmmSmramRequire is FALSE?

Thanks,
Ray

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Tuesday, April 25, 2023 3:03 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH 1/5] SimicsOpenBoardPkg: Build
> gEfiSmmSmramMemoryGuid Hob in S3 path
> 
> gEfiSmmSmramMemoryGuid Hob is needed for SmmRelocation feature
> even for S3 path. So in MemDetect.c, remove specical code path
> for S3 about creating gEfiSmmSmramMemoryGuid Hob and adding some
> memory descriptor, which does no harm in S3 path.
> 
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  .../SimicsOpenBoardPkg/SimicsPei/MemDetect.c  | 107 +++++++-----------
>  1 file changed, 42 insertions(+), 65 deletions(-)
> 
> diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> index 127afffc00..d80ac1d213 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> @@ -405,79 +405,56 @@ QemuInitializeRam (
>    LowerMemorySize = GetSystemMemorySizeBelow4gb ();
>    UpperMemorySize = GetSystemMemorySizeAbove4gb ();
> 
> -  if (mBootMode == BOOT_ON_S3_RESUME) {
> -    //
> -    // Create the following memory HOB as an exception on the S3 boot path.
> -    //
> -    // Normally we'd create memory HOBs only on the normal boot path.
> However,
> -    // CpuMpPei specifically needs such a low-memory HOB on the S3 path as
> -    // well, for "borrowing" a subset of it temporarily, for the AP startup
> -    // vector.
> -    //
> -    // CpuMpPei saves the original contents of the borrowed area in
> permanent
> -    // PEI RAM, in a backup buffer allocated with the normal PEI services.
> -    // CpuMpPei restores the original contents ("returns" the borrowed area)
> at
> -    // End-of-PEI. End-of-PEI in turn is emitted by S3Resume2Pei before
> -    // transferring control to the OS's wakeup vector in the FACS.
> -    //
> -    // We expect any other PEIMs that "borrow" memory similarly to
> CpuMpPei to
> -    // restore the original contents. Furthermore, we expect all such PEIMs
> -    // (CpuMpPei included) to claim the borrowed areas by producing
> memory
> -    // allocation HOBs, and to honor preexistent memory allocation HOBs
> when
> -    // looking for an area to borrow.
> -    //
> -    AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
> -  } else {
> -    //
> -    // Create memory HOBs
> -    //
> -    AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
> +  //
> +  // Create memory HOBs
> +  //
> +  AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
> 
> -    if (FeaturePcdGet (PcdSmmSmramRequire)) {
> -      UINT32 TsegSize;
> +  if (FeaturePcdGet (PcdSmmSmramRequire)) {
> +    UINT32 TsegSize;
> 
> -      TsegSize = mX58TsegMbytes * SIZE_1MB;
> -      AddMemoryRangeHob (BASE_1MB, LowerMemorySize - TsegSize);
> -      AddReservedMemoryBaseSizeHob (LowerMemorySize - TsegSize,
> TsegSize,
> -        TRUE);
> +    TsegSize = mX58TsegMbytes * SIZE_1MB;
> +    AddMemoryRangeHob (BASE_1MB, LowerMemorySize - TsegSize);
> +    AddReservedMemoryBaseSizeHob (LowerMemorySize - TsegSize,
> TsegSize,
> +      TRUE);
> 
> -	  BufferSize = sizeof(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK);
> -	  SmramRanges = 1;
> +    BufferSize = sizeof(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK);
> +    SmramRanges = 1;
> 
> -      Hob.Raw = BuildGuidHob(
> -          &gEfiSmmSmramMemoryGuid,
> -          BufferSize
> -      );
> -      ASSERT(Hob.Raw);
> -
> -      SmramHobDescriptorBlock = (EFI_SMRAM_HOB_DESCRIPTOR_BLOCK
> *)(Hob.Raw);
> -      SmramHobDescriptorBlock->NumberOfSmmReservedRegions =
> SmramRanges;
> -
> -      SmramIndex = 0;
> -      for (Index = 0; Index < SmramRanges; Index++) {
> -        //
> -        // This is an SMRAM range, create an SMRAM descriptor
> -        //
> -        SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalStart =
> LowerMemorySize - TsegSize;
> -        SmramHobDescriptorBlock->Descriptor[SmramIndex].CpuStart =
> LowerMemorySize - TsegSize;
> -        SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalSize =
> TsegSize;
> -        SmramHobDescriptorBlock->Descriptor[SmramIndex].RegionState =
> EFI_SMRAM_CLOSED | EFI_CACHEABLE;
> -        SmramIndex++;
> -      }
> +    Hob.Raw = BuildGuidHob(
> +        &gEfiSmmSmramMemoryGuid,
> +        BufferSize
> +    );
> +    ASSERT(Hob.Raw);
> 
> -    } else {
> -      AddMemoryRangeHob (BASE_1MB, LowerMemorySize);
> -    }
> +    SmramHobDescriptorBlock = (EFI_SMRAM_HOB_DESCRIPTOR_BLOCK
> *)(Hob.Raw);
> +    SmramHobDescriptorBlock->NumberOfSmmReservedRegions =
> SmramRanges;
> 
> -    //
> -    // If QEMU presents an E820 map, then create memory HOBs for
> the >=4GB RAM
> -    // entries. Otherwise, create a single memory HOB with the flat >=4GB
> -    // memory size read from the CMOS.
> -    //
> -    if (UpperMemorySize != 0) {
> -      AddMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
> +    SmramIndex = 0;
> +    for (Index = 0; Index < SmramRanges; Index++) {
> +      //
> +      // This is an SMRAM range, create an SMRAM descriptor
> +      //
> +      SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalStart =
> LowerMemorySize - TsegSize;
> +      SmramHobDescriptorBlock->Descriptor[SmramIndex].CpuStart =
> LowerMemorySize - TsegSize;
> +      SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalSize =
> TsegSize;
> +      SmramHobDescriptorBlock->Descriptor[SmramIndex].RegionState =
> EFI_SMRAM_CLOSED | EFI_CACHEABLE;
> +      SmramIndex++;
>      }
> +
> +  } else {
> +    AddMemoryRangeHob (BASE_1MB, LowerMemorySize);
>    }
> +
> +  //
> +  // If QEMU presents an E820 map, then create memory HOBs for
> the >=4GB RAM
> +  // entries. Otherwise, create a single memory HOB with the flat >=4GB
> +  // memory size read from the CMOS.
> +  //
> +  if (UpperMemorySize != 0) {
> +    AddMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
> +  }
> +
>  }
> 
>  /**
> --
> 2.31.1.windows.1

Re: [edk2-devel] [PATCH 1/5] SimicsOpenBoardPkg: Build gEfiSmmSmramMemoryGuid Hob in S3 path

[Ni, Ray]

Please ignore my 2nd question.
I saw " AddMemoryRangeHob (BASE_1MB, LowerMemorySize - TsegSize);" when
PcdSmmSmramRequire is TRUE.

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Tuesday, April 25, 2023 9:49 PM
> To: Liu, Zhiguang <zhiguang.liu@intel.com>; devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Subject: Re: [edk2-devel] [PATCH 1/5] SimicsOpenBoardPkg: Build
> gEfiSmmSmramMemoryGuid Hob in S3 path
> 
> Zhiguang,
> Can you please keep the comments that explain why below 1MB memory
> resource
> should be added for QSP platform?
> 
> Another question not related to your changes:
>   why "AddMemoryRangeHob (BASE_1MB, LowerMemorySize);" is only
> called when
>   PcdSmmSmramRequire is FALSE?
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: Liu, Zhiguang <zhiguang.liu@intel.com>
> > Sent: Tuesday, April 25, 2023 3:03 PM
> > To: devel@edk2.groups.io
> > Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Desimone, Nathaniel L
> > <nathaniel.l.desimone@intel.com>; Ni, Ray <ray.ni@intel.com>
> > Subject: [PATCH 1/5] SimicsOpenBoardPkg: Build
> > gEfiSmmSmramMemoryGuid Hob in S3 path
> >
> > gEfiSmmSmramMemoryGuid Hob is needed for SmmRelocation feature
> > even for S3 path. So in MemDetect.c, remove specical code path
> > for S3 about creating gEfiSmmSmramMemoryGuid Hob and adding some
> > memory descriptor, which does no harm in S3 path.
> >
> > Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> > ---
> >  .../SimicsOpenBoardPkg/SimicsPei/MemDetect.c  | 107 +++++++-----------
> >  1 file changed, 42 insertions(+), 65 deletions(-)
> >
> > diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> > b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> > index 127afffc00..d80ac1d213 100644
> > --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> > +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> > @@ -405,79 +405,56 @@ QemuInitializeRam (
> >    LowerMemorySize = GetSystemMemorySizeBelow4gb ();
> >    UpperMemorySize = GetSystemMemorySizeAbove4gb ();
> >
> > -  if (mBootMode == BOOT_ON_S3_RESUME) {
> > -    //
> > -    // Create the following memory HOB as an exception on the S3 boot
> path.
> > -    //
> > -    // Normally we'd create memory HOBs only on the normal boot path.
> > However,
> > -    // CpuMpPei specifically needs such a low-memory HOB on the S3 path
> as
> > -    // well, for "borrowing" a subset of it temporarily, for the AP startup
> > -    // vector.
> > -    //
> > -    // CpuMpPei saves the original contents of the borrowed area in
> > permanent
> > -    // PEI RAM, in a backup buffer allocated with the normal PEI services.
> > -    // CpuMpPei restores the original contents ("returns" the borrowed
> area)
> > at
> > -    // End-of-PEI. End-of-PEI in turn is emitted by S3Resume2Pei before
> > -    // transferring control to the OS's wakeup vector in the FACS.
> > -    //
> > -    // We expect any other PEIMs that "borrow" memory similarly to
> > CpuMpPei to
> > -    // restore the original contents. Furthermore, we expect all such PEIMs
> > -    // (CpuMpPei included) to claim the borrowed areas by producing
> > memory
> > -    // allocation HOBs, and to honor preexistent memory allocation HOBs
> > when
> > -    // looking for an area to borrow.
> > -    //
> > -    AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
> > -  } else {
> > -    //
> > -    // Create memory HOBs
> > -    //
> > -    AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
> > +  //
> > +  // Create memory HOBs
> > +  //
> > +  AddMemoryRangeHob (0, BASE_512KB + BASE_128KB);
> >
> > -    if (FeaturePcdGet (PcdSmmSmramRequire)) {
> > -      UINT32 TsegSize;
> > +  if (FeaturePcdGet (PcdSmmSmramRequire)) {
> > +    UINT32 TsegSize;
> >
> > -      TsegSize = mX58TsegMbytes * SIZE_1MB;
> > -      AddMemoryRangeHob (BASE_1MB, LowerMemorySize - TsegSize);
> > -      AddReservedMemoryBaseSizeHob (LowerMemorySize - TsegSize,
> > TsegSize,
> > -        TRUE);
> > +    TsegSize = mX58TsegMbytes * SIZE_1MB;
> > +    AddMemoryRangeHob (BASE_1MB, LowerMemorySize - TsegSize);
> > +    AddReservedMemoryBaseSizeHob (LowerMemorySize - TsegSize,
> > TsegSize,
> > +      TRUE);
> >
> > -	  BufferSize = sizeof(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK);
> > -	  SmramRanges = 1;
> > +    BufferSize = sizeof(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK);
> > +    SmramRanges = 1;
> >
> > -      Hob.Raw = BuildGuidHob(
> > -          &gEfiSmmSmramMemoryGuid,
> > -          BufferSize
> > -      );
> > -      ASSERT(Hob.Raw);
> > -
> > -      SmramHobDescriptorBlock = (EFI_SMRAM_HOB_DESCRIPTOR_BLOCK
> > *)(Hob.Raw);
> > -      SmramHobDescriptorBlock->NumberOfSmmReservedRegions =
> > SmramRanges;
> > -
> > -      SmramIndex = 0;
> > -      for (Index = 0; Index < SmramRanges; Index++) {
> > -        //
> > -        // This is an SMRAM range, create an SMRAM descriptor
> > -        //
> > -        SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalStart =
> > LowerMemorySize - TsegSize;
> > -        SmramHobDescriptorBlock->Descriptor[SmramIndex].CpuStart =
> > LowerMemorySize - TsegSize;
> > -        SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalSize =
> > TsegSize;
> > -        SmramHobDescriptorBlock->Descriptor[SmramIndex].RegionState =
> > EFI_SMRAM_CLOSED | EFI_CACHEABLE;
> > -        SmramIndex++;
> > -      }
> > +    Hob.Raw = BuildGuidHob(
> > +        &gEfiSmmSmramMemoryGuid,
> > +        BufferSize
> > +    );
> > +    ASSERT(Hob.Raw);
> >
> > -    } else {
> > -      AddMemoryRangeHob (BASE_1MB, LowerMemorySize);
> > -    }
> > +    SmramHobDescriptorBlock = (EFI_SMRAM_HOB_DESCRIPTOR_BLOCK
> > *)(Hob.Raw);
> > +    SmramHobDescriptorBlock->NumberOfSmmReservedRegions =
> > SmramRanges;
> >
> > -    //
> > -    // If QEMU presents an E820 map, then create memory HOBs for
> > the >=4GB RAM
> > -    // entries. Otherwise, create a single memory HOB with the flat >=4GB
> > -    // memory size read from the CMOS.
> > -    //
> > -    if (UpperMemorySize != 0) {
> > -      AddMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
> > +    SmramIndex = 0;
> > +    for (Index = 0; Index < SmramRanges; Index++) {
> > +      //
> > +      // This is an SMRAM range, create an SMRAM descriptor
> > +      //
> > +      SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalStart =
> > LowerMemorySize - TsegSize;
> > +      SmramHobDescriptorBlock->Descriptor[SmramIndex].CpuStart =
> > LowerMemorySize - TsegSize;
> > +      SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalSize =
> > TsegSize;
> > +      SmramHobDescriptorBlock->Descriptor[SmramIndex].RegionState =
> > EFI_SMRAM_CLOSED | EFI_CACHEABLE;
> > +      SmramIndex++;
> >      }
> > +
> > +  } else {
> > +    AddMemoryRangeHob (BASE_1MB, LowerMemorySize);
> >    }
> > +
> > +  //
> > +  // If QEMU presents an E820 map, then create memory HOBs for
> > the >=4GB RAM
> > +  // entries. Otherwise, create a single memory HOB with the flat >=4GB
> > +  // memory size read from the CMOS.
> > +  //
> > +  if (UpperMemorySize != 0) {
> > +    AddMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
> > +  }
> > +
> >  }
> >
> >  /**
> > --
> > 2.31.1.windows.1
> 
> 
> 
> 
> 

[PATCH 2/5] SimicsOpenBoardPkg: Move AcpiVariableGuid hob to MemDetect

[Zhiguang Liu]

Currently, MemDetect create gEfiSmmSmramMemoryGuid Hob containing one
descriptor, which should be updated later, when AcpiVariableGuid hob
use some buffer from SmRam. However, the Hob doesn't get updated, and
this is a bug.

Move the logic creating AcpiVariableGuid hob from PEIM SmmAccessPei.inf
to MemDetect, so that in the same file, it has both knowleage about
the smmram and the acpi data structure. So it can create the
gEfiSmmSmramMemoryGuid Hob containing two descriptors.

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 .../SimicsOpenBoardPkg/SimicsPei/MemDetect.c  | 36 +++++++++++--------
 .../SimicsPei/SimicsPei.inf                   |  1 +
 .../SimicsX58SktPkg/Smm/Access/SmmAccessPei.c |  8 -----
 .../Smm/Access/SmmAccessPei.inf               |  3 --
 4 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
index d80ac1d213..13ee415f40 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
+++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
@@ -391,11 +391,10 @@ QemuInitializeRam (
   UINT64                               LowerMemorySize;
   UINT64                               UpperMemorySize;
   UINTN                                 BufferSize;
-  UINT8                                 SmramIndex;
   UINT8                                 SmramRanges;
   EFI_PEI_HOB_POINTERS                  Hob;
   EFI_SMRAM_HOB_DESCRIPTOR_BLOCK        *SmramHobDescriptorBlock;
-  UINT8                                 Index;
+  VOID                                  *GuidHob;
 
   DEBUG ((EFI_D_INFO, "%a called\n", __FUNCTION__));
 
@@ -418,8 +417,8 @@ QemuInitializeRam (
     AddReservedMemoryBaseSizeHob (LowerMemorySize - TsegSize, TsegSize,
       TRUE);
 
-    BufferSize = sizeof(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK);
-    SmramRanges = 1;
+    SmramRanges = 2;
+    BufferSize = sizeof(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK) + (SmramRanges - 1) * sizeof(EFI_SMRAM_DESCRIPTOR);
 
     Hob.Raw = BuildGuidHob(
         &gEfiSmmSmramMemoryGuid,
@@ -430,18 +429,25 @@ QemuInitializeRam (
     SmramHobDescriptorBlock = (EFI_SMRAM_HOB_DESCRIPTOR_BLOCK *)(Hob.Raw);
     SmramHobDescriptorBlock->NumberOfSmmReservedRegions = SmramRanges;
 
-    SmramIndex = 0;
-    for (Index = 0; Index < SmramRanges; Index++) {
-      //
-      // This is an SMRAM range, create an SMRAM descriptor
-      //
-      SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalStart = LowerMemorySize - TsegSize;
-      SmramHobDescriptorBlock->Descriptor[SmramIndex].CpuStart = LowerMemorySize - TsegSize;
-      SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalSize = TsegSize;
-      SmramHobDescriptorBlock->Descriptor[SmramIndex].RegionState = EFI_SMRAM_CLOSED | EFI_CACHEABLE;
-      SmramIndex++;
-    }
+    //
+    // Create first SMRAM descriptor, which contains data structures used in S3 resume.
+    // One page is enough for the data structure
+    //
+    SmramHobDescriptorBlock->Descriptor[0].PhysicalStart = LowerMemorySize - TsegSize;
+    SmramHobDescriptorBlock->Descriptor[0].CpuStart = LowerMemorySize - TsegSize;
+    SmramHobDescriptorBlock->Descriptor[0].PhysicalSize = EFI_PAGE_SIZE;
+    SmramHobDescriptorBlock->Descriptor[0].RegionState = EFI_SMRAM_CLOSED | EFI_CACHEABLE | EFI_ALLOCATED;
+    GuidHob = BuildGuidHob (&gEfiAcpiVariableGuid, sizeof(EFI_SMRAM_DESCRIPTOR));
+    ASSERT (GuidHob != NULL);
+    CopyMem (GuidHob, &SmramHobDescriptorBlock->Descriptor[0], sizeof(EFI_SMRAM_DESCRIPTOR));
 
+    //
+    // Create second SMRAM descriptor, which is free and will be used by SMM foundation.
+    //
+    SmramHobDescriptorBlock->Descriptor[1].PhysicalStart = SmramHobDescriptorBlock->Descriptor[0].PhysicalStart + EFI_PAGE_SIZE;
+    SmramHobDescriptorBlock->Descriptor[1].CpuStart = SmramHobDescriptorBlock->Descriptor[0].CpuStart + EFI_PAGE_SIZE;
+    SmramHobDescriptorBlock->Descriptor[1].PhysicalSize = TsegSize - EFI_PAGE_SIZE;
+    SmramHobDescriptorBlock->Descriptor[1].RegionState = EFI_SMRAM_CLOSED | EFI_CACHEABLE;
   } else {
     AddMemoryRangeHob (BASE_1MB, LowerMemorySize);
   }
diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
index 710fa680be..618ad4075f 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
+++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
@@ -40,6 +40,7 @@
 [Guids]
   gEfiMemoryTypeInformationGuid
   gEfiSmmSmramMemoryGuid              ## CONSUMES
+  gEfiAcpiVariableGuid
 
 [LibraryClasses]
   BaseLib
diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
index c54026b4d1..d489cc7513 100644
--- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
+++ b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
@@ -241,7 +241,6 @@ SmmAccessPeiEntryPoint (
   EFI_STATUS           Status;
   UINTN                SmramMapSize;
   EFI_SMRAM_DESCRIPTOR SmramMap[DescIdxCount];
-  VOID                 *GuidHob;
 
   //
   // This module should only be included if SMRAM support is required.
@@ -322,13 +321,6 @@ SmmAccessPeiEntryPoint (
   }
   DEBUG_CODE_END ();
 
-  GuidHob = BuildGuidHob (&gEfiAcpiVariableGuid, sizeof SmramMap[DescIdxSmmS3ResumeState]);
-  if (GuidHob == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  CopyMem (GuidHob, &SmramMap[DescIdxSmmS3ResumeState], sizeof SmramMap[DescIdxSmmS3ResumeState]);
-
   //
   // We're done. The next step should succeed, but even if it fails, we can't
   // roll back the above BuildGuidHob() allocation, because PEI doesn't support
diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
index 2b6b14f437..3c71e64fe9 100644
--- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
+++ b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
@@ -38,9 +38,6 @@
   SimicsX58SktPkg/SktPkg.dec
   SimicsIch10Pkg/Ich10Pkg.dec
 
-[Guids]
-  gEfiAcpiVariableGuid
-
 [LibraryClasses]
   BaseLib
   BaseMemoryLib
-- 
2.31.1.windows.1

Re: [edk2-devel] [PATCH 2/5] SimicsOpenBoardPkg: Move AcpiVariableGuid hob to MemDetect

[Ni, Ray]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Zhiguang Liu
> Sent: Tuesday, April 25, 2023 3:03 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [edk2-devel] [PATCH 2/5] SimicsOpenBoardPkg: Move
> AcpiVariableGuid hob to MemDetect
> 
> Currently, MemDetect create gEfiSmmSmramMemoryGuid Hob containing
> one
> descriptor, which should be updated later, when AcpiVariableGuid hob
> use some buffer from SmRam. However, the Hob doesn't get updated, and
> this is a bug.
> 
> Move the logic creating AcpiVariableGuid hob from PEIM SmmAccessPei.inf
> to MemDetect, so that in the same file, it has both knowleage about
> the smmram and the acpi data structure. So it can create the
> gEfiSmmSmramMemoryGuid Hob containing two descriptors.
> 
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  .../SimicsOpenBoardPkg/SimicsPei/MemDetect.c  | 36 +++++++++++--------
>  .../SimicsPei/SimicsPei.inf                   |  1 +
>  .../SimicsX58SktPkg/Smm/Access/SmmAccessPei.c |  8 -----
>  .../Smm/Access/SmmAccessPei.inf               |  3 --
>  4 files changed, 22 insertions(+), 26 deletions(-)
> 
> diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> index d80ac1d213..13ee415f40 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> @@ -391,11 +391,10 @@ QemuInitializeRam (
>    UINT64                               LowerMemorySize;
>    UINT64                               UpperMemorySize;
>    UINTN                                 BufferSize;
> -  UINT8                                 SmramIndex;
>    UINT8                                 SmramRanges;
>    EFI_PEI_HOB_POINTERS                  Hob;
>    EFI_SMRAM_HOB_DESCRIPTOR_BLOCK        *SmramHobDescriptorBlock;
> -  UINT8                                 Index;
> +  VOID                                  *GuidHob;
> 
>    DEBUG ((EFI_D_INFO, "%a called\n", __FUNCTION__));
> 
> @@ -418,8 +417,8 @@ QemuInitializeRam (
>      AddReservedMemoryBaseSizeHob (LowerMemorySize - TsegSize,
> TsegSize,
>        TRUE);
> 
> -    BufferSize = sizeof(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK);
> -    SmramRanges = 1;
> +    SmramRanges = 2;
> +    BufferSize = sizeof(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK) +
> (SmramRanges - 1) * sizeof(EFI_SMRAM_DESCRIPTOR);
> 
>      Hob.Raw = BuildGuidHob(
>          &gEfiSmmSmramMemoryGuid,
> @@ -430,18 +429,25 @@ QemuInitializeRam (
>      SmramHobDescriptorBlock = (EFI_SMRAM_HOB_DESCRIPTOR_BLOCK
> *)(Hob.Raw);
>      SmramHobDescriptorBlock->NumberOfSmmReservedRegions =
> SmramRanges;
> 
> -    SmramIndex = 0;
> -    for (Index = 0; Index < SmramRanges; Index++) {
> -      //
> -      // This is an SMRAM range, create an SMRAM descriptor
> -      //
> -      SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalStart =
> LowerMemorySize - TsegSize;
> -      SmramHobDescriptorBlock->Descriptor[SmramIndex].CpuStart =
> LowerMemorySize - TsegSize;
> -      SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalSize =
> TsegSize;
> -      SmramHobDescriptorBlock->Descriptor[SmramIndex].RegionState =
> EFI_SMRAM_CLOSED | EFI_CACHEABLE;
> -      SmramIndex++;
> -    }
> +    //
> +    // Create first SMRAM descriptor, which contains data structures used in
> S3 resume.
> +    // One page is enough for the data structure
> +    //
> +    SmramHobDescriptorBlock->Descriptor[0].PhysicalStart =
> LowerMemorySize - TsegSize;
> +    SmramHobDescriptorBlock->Descriptor[0].CpuStart = LowerMemorySize -
> TsegSize;
> +    SmramHobDescriptorBlock->Descriptor[0].PhysicalSize = EFI_PAGE_SIZE;
> +    SmramHobDescriptorBlock->Descriptor[0].RegionState =
> EFI_SMRAM_CLOSED | EFI_CACHEABLE | EFI_ALLOCATED;
> +    GuidHob = BuildGuidHob (&gEfiAcpiVariableGuid,
> sizeof(EFI_SMRAM_DESCRIPTOR));
> +    ASSERT (GuidHob != NULL);
> +    CopyMem (GuidHob, &SmramHobDescriptorBlock->Descriptor[0],
> sizeof(EFI_SMRAM_DESCRIPTOR));
> 
> +    //
> +    // Create second SMRAM descriptor, which is free and will be used by
> SMM foundation.
> +    //
> +    SmramHobDescriptorBlock->Descriptor[1].PhysicalStart =
> SmramHobDescriptorBlock->Descriptor[0].PhysicalStart + EFI_PAGE_SIZE;
> +    SmramHobDescriptorBlock->Descriptor[1].CpuStart =
> SmramHobDescriptorBlock->Descriptor[0].CpuStart + EFI_PAGE_SIZE;
> +    SmramHobDescriptorBlock->Descriptor[1].PhysicalSize = TsegSize -
> EFI_PAGE_SIZE;
> +    SmramHobDescriptorBlock->Descriptor[1].RegionState =
> EFI_SMRAM_CLOSED | EFI_CACHEABLE;
>    } else {
>      AddMemoryRangeHob (BASE_1MB, LowerMemorySize);
>    }
> diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> index 710fa680be..618ad4075f 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> @@ -40,6 +40,7 @@
>  [Guids]
>    gEfiMemoryTypeInformationGuid
>    gEfiSmmSmramMemoryGuid              ## CONSUMES
> +  gEfiAcpiVariableGuid
> 
>  [LibraryClasses]
>    BaseLib
> diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
> b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
> index c54026b4d1..d489cc7513 100644
> --- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
> +++ b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
> @@ -241,7 +241,6 @@ SmmAccessPeiEntryPoint (
>    EFI_STATUS           Status;
>    UINTN                SmramMapSize;
>    EFI_SMRAM_DESCRIPTOR SmramMap[DescIdxCount];
> -  VOID                 *GuidHob;
> 
>    //
>    // This module should only be included if SMRAM support is required.
> @@ -322,13 +321,6 @@ SmmAccessPeiEntryPoint (
>    }
>    DEBUG_CODE_END ();
> 
> -  GuidHob = BuildGuidHob (&gEfiAcpiVariableGuid, sizeof
> SmramMap[DescIdxSmmS3ResumeState]);
> -  if (GuidHob == NULL) {
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -
> -  CopyMem (GuidHob, &SmramMap[DescIdxSmmS3ResumeState], sizeof
> SmramMap[DescIdxSmmS3ResumeState]);
> -
>    //
>    // We're done. The next step should succeed, but even if it fails, we can't
>    // roll back the above BuildGuidHob() allocation, because PEI doesn't
> support
> diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
> b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
> index 2b6b14f437..3c71e64fe9 100644
> --- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
> +++ b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
> @@ -38,9 +38,6 @@
>    SimicsX58SktPkg/SktPkg.dec
>    SimicsIch10Pkg/Ich10Pkg.dec
> 
> -[Guids]
> -  gEfiAcpiVariableGuid
> -
>  [LibraryClasses]
>    BaseLib
>    BaseMemoryLib
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 

[PATCH 3/5] SimicsOpenBoardPkg: Use SmmAccessLib instead of SmmAccessPei.inf

[Zhiguang Liu]

SmmAccessPei.inf is a PEIM we should deleted, here is the reason:
1. It programs registers MCH_TOLUD to set the Low Usable DRAM,
but reading LMCH_TOLUD always return zere in QSP platforms
2. It programs/reads MCH_TSEGMB to implemte some Smm Access service
such as open/close/lock. However, this reading LMCH_TOLUD also always
return zere in QSP platforms
3. It returns the hard-code Smm range information. However, there are
two improper things about this. One is that we already have the hard
code value about T-Seg base/size in MemDetect. The other Smm range
informaton is already saved in gEfiSmmSmramMemoryGuid Hob. No need
hard-code value.

So, this patch uses another way, calling PeiInstallSmmAccessPpi from
SmmAccessLib. The lib instance we choose will use the
gEfiSmmSmramMemoryGuid Hob information.
In a word, with the patch, we can avoid additional hard-code, and
avoid programing unimplemented registers.

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 .../SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc    | 7 +------
 .../SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf    | 1 -
 Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c  | 9 +++++++++
 .../Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf     | 2 ++
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
index 7b98baf764..fcae343146 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
+++ b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
@@ -142,6 +142,7 @@
   # Silicon Package
   #####################################
   ReportCpuHobLib|IntelSiliconPkg/Library/ReportCpuHobLib/ReportCpuHobLib.inf
+  SmmAccessLib|IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLib/PeiSmmAccessLib.inf
 
   #####################################
   # Platform Package
@@ -190,12 +191,6 @@
   #######################################
   # Silicon Initialization Package
   #######################################
-!if gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly == FALSE
-  $(SKT_PKG)/Smm/Access/SmmAccessPei.inf {
-    <LibraryClasses>
-      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
-  }
-!endif
 
   #####################################
   # Platform Package
diff --git a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
index 221706ae03..844f9b6dcf 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
+++ b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
@@ -165,7 +165,6 @@ INF  MinPlatformPkg/PlatformInit/SiliconPolicyPei/SiliconPolicyPeiPostMem.inf
 !include MinPlatformPkg/Include/Fdf/CoreSecurityPostMemoryInclude.fdf
 
 INF  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
-INF  $(SKT_PKG)/Smm/Access/SmmAccessPei.inf
 # S3 SMM PEI driver
 #INF  UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
 
diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
index 13ee415f40..f9a5487365 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
+++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
@@ -25,6 +25,7 @@
 #include <Library/CmosAccessLib.h>
 #include <SimicsPlatforms.h>
 #include <Guid/SmramMemoryReserve.h>
+#include <Library/SmmAccessLib.h>
 
 #include <CmosMap.h>
 
@@ -472,6 +473,8 @@ InitializeRamRegions (
   VOID
   )
 {
+  EFI_STATUS     Status;
+
   QemuInitializeRam ();
 
   if (mS3Supported && mBootMode != BOOT_ON_S3_RESUME) {
@@ -544,4 +547,10 @@ InitializeRamRegions (
         );
     }
   }
+
+  //
+  // Install EFI_PEI_MM_ACCESS_PPI for S3 resume case
+  //
+  Status = PeiInstallSmmAccessPpi ();
+  ASSERT_EFI_ERROR (Status);
 }
diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
index 618ad4075f..cdc30ad582 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
+++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
@@ -36,6 +36,7 @@
   SimicsX58SktPkg/SktPkg.dec
   SimicsIch10Pkg/Ich10Pkg.dec
   BoardModulePkg/BoardModulePkg.dec
+  IntelSiliconPkg/IntelSiliconPkg.dec
 
 [Guids]
   gEfiMemoryTypeInformationGuid
@@ -55,6 +56,7 @@
   MtrrLib
   PcdLib
   CmosAccessLib
+  SmmAccessLib
 
 [Pcd]
   gSimicsOpenBoardPkgTokenSpaceGuid.PcdSimicsPeiMemFvBase
-- 
2.31.1.windows.1

Re: [PATCH 3/5] SimicsOpenBoardPkg: Use SmmAccessLib instead of SmmAccessPei.inf

[Ni, Ray]

Can you fix some typos in the commit message?
With that, Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Tuesday, April 25, 2023 3:03 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH 3/5] SimicsOpenBoardPkg: Use SmmAccessLib instead of
> SmmAccessPei.inf
> 
> SmmAccessPei.inf is a PEIM we should deleted, here is the reason:
> 1. It programs registers MCH_TOLUD to set the Low Usable DRAM,
> but reading LMCH_TOLUD always return zere in QSP platforms
> 2. It programs/reads MCH_TSEGMB to implemte some Smm Access service
> such as open/close/lock. However, this reading LMCH_TOLUD also always
> return zere in QSP platforms
> 3. It returns the hard-code Smm range information. However, there are
> two improper things about this. One is that we already have the hard
> code value about T-Seg base/size in MemDetect. The other Smm range
> informaton is already saved in gEfiSmmSmramMemoryGuid Hob. No need
> hard-code value.
> 
> So, this patch uses another way, calling PeiInstallSmmAccessPpi from
> SmmAccessLib. The lib instance we choose will use the
> gEfiSmmSmramMemoryGuid Hob information.
> In a word, with the patch, we can avoid additional hard-code, and
> avoid programing unimplemented registers.
> 
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  .../SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc    | 7 +------
>  .../SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf    | 1 -
>  Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c  | 9
> +++++++++
>  .../Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf     | 2 ++
>  4 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> index 7b98baf764..fcae343146 100644
> ---
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> @@ -142,6 +142,7 @@
>    # Silicon Package
>    #####################################
> 
> ReportCpuHobLib|IntelSiliconPkg/Library/ReportCpuHobLib/ReportCpuHobLi
> b.inf
> +
> SmmAccessLib|IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLi
> b/PeiSmmAccessLib.inf
> 
>    #####################################
>    # Platform Package
> @@ -190,12 +191,6 @@
>    #######################################
>    # Silicon Initialization Package
>    #######################################
> -!if gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly == FALSE
> -  $(SKT_PKG)/Smm/Access/SmmAccessPei.inf {
> -    <LibraryClasses>
> -      PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
> -  }
> -!endif
> 
>    #####################################
>    # Platform Package
> diff --git
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
> index 221706ae03..844f9b6dcf 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf
> @@ -165,7 +165,6 @@ INF
> MinPlatformPkg/PlatformInit/SiliconPolicyPei/SiliconPolicyPeiPostMem.inf
>  !include MinPlatformPkg/Include/Fdf/CoreSecurityPostMemoryInclude.fdf
> 
>  INF  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> -INF  $(SKT_PKG)/Smm/Access/SmmAccessPei.inf
>  # S3 SMM PEI driver
>  #INF  UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
> 
> diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> index 13ee415f40..f9a5487365 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> @@ -25,6 +25,7 @@
>  #include <Library/CmosAccessLib.h>
>  #include <SimicsPlatforms.h>
>  #include <Guid/SmramMemoryReserve.h>
> +#include <Library/SmmAccessLib.h>
> 
>  #include <CmosMap.h>
> 
> @@ -472,6 +473,8 @@ InitializeRamRegions (
>    VOID
>    )
>  {
> +  EFI_STATUS     Status;
> +
>    QemuInitializeRam ();
> 
>    if (mS3Supported && mBootMode != BOOT_ON_S3_RESUME) {
> @@ -544,4 +547,10 @@ InitializeRamRegions (
>          );
>      }
>    }
> +
> +  //
> +  // Install EFI_PEI_MM_ACCESS_PPI for S3 resume case
> +  //
> +  Status = PeiInstallSmmAccessPpi ();
> +  ASSERT_EFI_ERROR (Status);
>  }
> diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> index 618ad4075f..cdc30ad582 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> @@ -36,6 +36,7 @@
>    SimicsX58SktPkg/SktPkg.dec
>    SimicsIch10Pkg/Ich10Pkg.dec
>    BoardModulePkg/BoardModulePkg.dec
> +  IntelSiliconPkg/IntelSiliconPkg.dec
> 
>  [Guids]
>    gEfiMemoryTypeInformationGuid
> @@ -55,6 +56,7 @@
>    MtrrLib
>    PcdLib
>    CmosAccessLib
> +  SmmAccessLib
> 
>  [Pcd]
>    gSimicsOpenBoardPkgTokenSpaceGuid.PcdSimicsPeiMemFvBase
> --
> 2.31.1.windows.1

[PATCH 4/5] SimicsOpenBoardPkg: Use another SmmAccess Driver

[Zhiguang Liu]

Because of the similiar reason I mentioned in last commit, the
SmmAccess2Dxe.inf driver should be deleted and the replacement
will avoid hard-code and use gEfiSmmSmramMemoryGuid Hob to get
Smm Range information.

This can fix an exsiting bug, when gSmmBaseHobGuid may allocate buffer
from smm range, and update gEfiSmmSmramMemoryGuid Hob. Current
driver will return hard-code smm range and the buffer used
by gSmmBaseHobGuid is marked as free range by mistake.

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 .../Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc     | 2 +-
 Silicon/Intel/SimicsX58SktPkg/SktUefiBootInclude.fdf            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
index fcae343146..64c3af2584 100644
--- a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
+++ b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
@@ -278,7 +278,7 @@
 !if gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly == FALSE
   $(PCH_PKG)/SmmControl/RuntimeDxe/SmmControl2Dxe.inf
   $(PCH_PKG)/Spi/Smm/PchSpiSmm.inf
-  $(SKT_PKG)/Smm/Access/SmmAccess2Dxe.inf
+  IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess.inf
   IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
 !endif
 
diff --git a/Silicon/Intel/SimicsX58SktPkg/SktUefiBootInclude.fdf b/Silicon/Intel/SimicsX58SktPkg/SktUefiBootInclude.fdf
index fdcb4fb9a7..ca3706578b 100644
--- a/Silicon/Intel/SimicsX58SktPkg/SktUefiBootInclude.fdf
+++ b/Silicon/Intel/SimicsX58SktPkg/SktUefiBootInclude.fdf
@@ -8,7 +8,7 @@
 ##
 
 !if gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly == FALSE
-  INF  $(SKT_PKG)/Smm/Access/SmmAccess2Dxe.inf
+  INF  IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess.inf
   INF  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
 !endif
 INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
-- 
2.31.1.windows.1

Re: [PATCH 4/5] SimicsOpenBoardPkg: Use another SmmAccess Driver

[Ni, Ray]

The code change looks good to me.

Can you refine your commit message a bit to explain what the bug is?

Thanks,
Ray

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Tuesday, April 25, 2023 3:03 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH 4/5] SimicsOpenBoardPkg: Use another SmmAccess Driver
> 
> Because of the similiar reason I mentioned in last commit, the
> SmmAccess2Dxe.inf driver should be deleted and the replacement
> will avoid hard-code and use gEfiSmmSmramMemoryGuid Hob to get
> Smm Range information.
> 
> This can fix an exsiting bug, when gSmmBaseHobGuid may allocate buffer
> from smm range, and update gEfiSmmSmramMemoryGuid Hob. Current
> driver will return hard-code smm range and the buffer used
> by gSmmBaseHobGuid is marked as free range by mistake.
> 
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  .../Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc     | 2 +-
>  Silicon/Intel/SimicsX58SktPkg/SktUefiBootInclude.fdf            | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> index fcae343146..64c3af2584 100644
> ---
> a/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> +++
> b/Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc
> @@ -278,7 +278,7 @@
>  !if gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly == FALSE
>    $(PCH_PKG)/SmmControl/RuntimeDxe/SmmControl2Dxe.inf
>    $(PCH_PKG)/Spi/Smm/PchSpiSmm.inf
> -  $(SKT_PKG)/Smm/Access/SmmAccess2Dxe.inf
> +  IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess.inf
>    IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
>  !endif
> 
> diff --git a/Silicon/Intel/SimicsX58SktPkg/SktUefiBootInclude.fdf
> b/Silicon/Intel/SimicsX58SktPkg/SktUefiBootInclude.fdf
> index fdcb4fb9a7..ca3706578b 100644
> --- a/Silicon/Intel/SimicsX58SktPkg/SktUefiBootInclude.fdf
> +++ b/Silicon/Intel/SimicsX58SktPkg/SktUefiBootInclude.fdf
> @@ -8,7 +8,7 @@
>  ##
> 
>  !if gMinPlatformPkgTokenSpaceGuid.PcdBootToShellOnly == FALSE
> -  INF  $(SKT_PKG)/Smm/Access/SmmAccess2Dxe.inf
> +  INF  IntelSiliconPkg/Feature/SmmAccess/SmmAccessDxe/SmmAccess.inf
>    INF  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
>  !endif
>  INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> --
> 2.31.1.windows.1

[PATCH 5/5] SimicsX58SktPkg: Remove unused Smm related modules

[Zhiguang Liu]

In last two commit, I replace the two SMM related modules, and now
no platform will use these two moduels. Remove them

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 .../Smm/Access/SmmAccess2Dxe.c                | 148 --------
 .../Smm/Access/SmmAccess2Dxe.inf              |  54 ---
 .../SimicsX58SktPkg/Smm/Access/SmmAccessPei.c | 338 ------------------
 .../Smm/Access/SmmAccessPei.inf               |  62 ----
 .../Smm/Access/SmramInternal.c                | 200 -----------
 .../Smm/Access/SmramInternal.h                |  82 -----
 6 files changed, 884 deletions(-)
 delete mode 100644 Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.c
 delete mode 100644 Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.inf
 delete mode 100644 Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
 delete mode 100644 Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
 delete mode 100644 Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.c
 delete mode 100644 Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.h

diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.c b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.c
deleted file mode 100644
index 5d3b2c14aa..0000000000
--- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.c
+++ /dev/null
@@ -1,148 +0,0 @@
-/** @file
-  A DXE_DRIVER providing SMRAM access by producing EFI_SMM_ACCESS2_PROTOCOL.
-
-  X58 TSEG is expected to have been verified and set up by the SmmAccessPei
-  driver.
-
-  Copyright (C) 2013, 2015, Red Hat, Inc.<BR>
-  Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#include <Library/DebugLib.h>
-#include <Library/PcdLib.h>
-#include <Library/UefiBootServicesTableLib.h>
-#include <Protocol/SmmAccess2.h>
-
-#include "SmramInternal.h"
-
-/**
-  Opens the SMRAM area to be accessible by a boot-service driver.
-
-  This function "opens" SMRAM so that it is visible while not inside of SMM.
-  The function should return EFI_UNSUPPORTED if the hardware does not support
-  hiding of SMRAM. The function should return EFI_DEVICE_ERROR if the SMRAM
-  configuration is locked.
-
-  @param[in] This           The EFI_SMM_ACCESS2_PROTOCOL instance.
-
-  @retval EFI_SUCCESS       The operation was successful.
-  @retval EFI_UNSUPPORTED   The system does not support opening and closing of
-                            SMRAM.
-  @retval EFI_DEVICE_ERROR  SMRAM cannot be opened, perhaps because it is
-                            locked.
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-SmmAccess2DxeOpen (
-  IN EFI_SMM_ACCESS2_PROTOCOL  *This
-  )
-{
-  return SmramAccessOpen (&This->LockState, &This->OpenState);
-}
-
-/**
-  Inhibits access to the SMRAM.
-
-  This function "closes" SMRAM so that it is not visible while outside of SMM.
-  The function should return EFI_UNSUPPORTED if the hardware does not support
-  hiding of SMRAM.
-
-  @param[in] This           The EFI_SMM_ACCESS2_PROTOCOL instance.
-
-  @retval EFI_SUCCESS       The operation was successful.
-  @retval EFI_UNSUPPORTED   The system does not support opening and closing of
-                            SMRAM.
-  @retval EFI_DEVICE_ERROR  SMRAM cannot be closed.
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-SmmAccess2DxeClose (
-  IN EFI_SMM_ACCESS2_PROTOCOL  *This
-  )
-{
-  return SmramAccessClose (&This->LockState, &This->OpenState);
-}
-
-/**
-  Inhibits access to the SMRAM.
-
-  This function prohibits access to the SMRAM region.  This function is usually
-  implemented such that it is a write-once operation.
-
-  @param[in] This          The EFI_SMM_ACCESS2_PROTOCOL instance.
-
-  @retval EFI_SUCCESS      The device was successfully locked.
-  @retval EFI_UNSUPPORTED  The system does not support locking of SMRAM.
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-SmmAccess2DxeLock (
-  IN EFI_SMM_ACCESS2_PROTOCOL  *This
-  )
-{
-  return SmramAccessLock (&This->LockState, &This->OpenState);
-}
-
-/**
-  Queries the memory controller for the possible regions that will support
-  SMRAM.
-
-  @param[in]     This           The EFI_SMM_ACCESS2_PROTOCOL instance.
-  @param[in,out] SmramMapSize   A pointer to the size, in bytes, of the
-                                SmramMemoryMap buffer.
-  @param[in,out] SmramMap       A pointer to the buffer in which firmware
-                                places the current memory map.
-
-  @retval EFI_SUCCESS           The chipset supported the given resource.
-  @retval EFI_BUFFER_TOO_SMALL  The SmramMap parameter was too small.  The
-                                current buffer size needed to hold the memory
-                                map is returned in SmramMapSize.
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-SmmAccess2DxeGetCapabilities (
-  IN CONST EFI_SMM_ACCESS2_PROTOCOL  *This,
-  IN OUT UINTN                       *SmramMapSize,
-  IN OUT EFI_SMRAM_DESCRIPTOR        *SmramMap
-  )
-{
-  return SmramAccessGetCapabilities (This->LockState, This->OpenState,
-           SmramMapSize, SmramMap);
-}
-
-//
-// LockState and OpenState will be filled in by the entry point.
-//
-STATIC EFI_SMM_ACCESS2_PROTOCOL mAccess2 = {
-  &SmmAccess2DxeOpen,
-  &SmmAccess2DxeClose,
-  &SmmAccess2DxeLock,
-  &SmmAccess2DxeGetCapabilities
-};
-
-//
-// Entry point of this driver.
-//
-EFI_STATUS
-EFIAPI
-SmmAccess2DxeEntryPoint (
-  IN EFI_HANDLE       ImageHandle,
-  IN EFI_SYSTEM_TABLE *SystemTable
-  )
-{
-  //
-  // This module should only be included if SMRAM support is required.
-  //
-  ASSERT (FeaturePcdGet (PcdSmmSmramRequire));
-
-  GetStates (&mAccess2.LockState, &mAccess2.OpenState);
-  return gBS->InstallMultipleProtocolInterfaces (&ImageHandle,
-                &gEfiSmmAccess2ProtocolGuid, &mAccess2,
-                NULL);
-}
diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.inf b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.inf
deleted file mode 100644
index eb8c8f93dd..0000000000
--- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.inf
+++ /dev/null
@@ -1,54 +0,0 @@
-## @file
-# A DXE_DRIVER providing SMRAM access by producing EFI_SMM_ACCESS2_PROTOCOL.
-#
-# X58 TSEG is expected to have been verified and set up by the SmmAccessPei
-# driver.
-#
-# Copyright (C) 2013, 2015, Red Hat, Inc.
-# Copyright (C) 2019, Intel Corporation. All rights reserved.<BR>
-#
-# SPDX-License-Identifier: BSD-2-Clause-Patent
-#
-##
-
-[Defines]
-  INF_VERSION                    = 0x00010005
-  BASE_NAME                      = SmmAccess2Dxe
-  FILE_GUID                      = AC95AD3D-4366-44BF-9A62-E4B29D7A2206
-  MODULE_TYPE                    = DXE_DRIVER
-  VERSION_STRING                 = 1.0
-  PI_SPECIFICATION_VERSION       = 0x00010400
-  ENTRY_POINT                    = SmmAccess2DxeEntryPoint
-
-#
-# The following information is for reference only and not required by the build tools.
-#
-#  VALID_ARCHITECTURES           = IA32 X64
-#
-
-[Sources]
-  SmmAccess2Dxe.c
-  SmramInternal.c
-  SmramInternal.h
-
-[Packages]
-  MdeModulePkg/MdeModulePkg.dec
-  MdePkg/MdePkg.dec
-  SimicsX58SktPkg/SktPkg.dec
-  SimicsIch10Pkg/Ich10Pkg.dec
-
-[LibraryClasses]
-  DebugLib
-  PcdLib
-  PciLib
-  UefiBootServicesTableLib
-  UefiDriverEntryPoint
-
-[Protocols]
-  gEfiSmmAccess2ProtocolGuid   ## PRODUCES
-
-[FeaturePcd]
-  gSimicsX58PkgTokenSpaceGuid.PcdSmmSmramRequire
-
-[Depex]
-  TRUE
diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
deleted file mode 100644
index d489cc7513..0000000000
--- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
+++ /dev/null
@@ -1,338 +0,0 @@
-/** @file
-  A PEIM with the following responsibilities:
-
-  - verify & configure the X58 TSEG in the entry point,
-  - provide SMRAM access by producing PEI_SMM_ACCESS_PPI,
-  - set aside the SMM_S3_RESUME_STATE object at the bottom of TSEG, and expose
-    it via the gEfiAcpiVariableGuid GUID HOB.
-
-  This PEIM runs from RAM, so we can write to variables with static storage
-  duration.
-
-  Copyright (C) 2013, 2015, Red Hat, Inc.<BR>
-  Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#include <Guid/AcpiS3Context.h>
-#include <Library/BaseLib.h>
-#include <Library/BaseMemoryLib.h>
-#include <Library/DebugLib.h>
-#include <Library/HobLib.h>
-#include <Library/IoLib.h>
-#include <Library/PcdLib.h>
-#include <Library/PciLib.h>
-#include <Library/PeiServicesLib.h>
-#include <Ppi/SmmAccess.h>
-
-#include <Register/X58Ich10.h>
-#include "SmramInternal.h"
-
-//
-// PEI_SMM_ACCESS_PPI implementation.
-//
-
-/**
-  Opens the SMRAM area to be accessible by a PEIM driver.
-
-  This function "opens" SMRAM so that it is visible while not inside of SMM.
-  The function should return EFI_UNSUPPORTED if the hardware does not support
-  hiding of SMRAM. The function should return EFI_DEVICE_ERROR if the SMRAM
-  configuration is locked.
-
-  @param  PeiServices            General purpose services available to every
-                                 PEIM.
-  @param  This                   The pointer to the SMM Access Interface.
-  @param  DescriptorIndex        The region of SMRAM to Open.
-
-  @retval EFI_SUCCESS            The region was successfully opened.
-  @retval EFI_DEVICE_ERROR       The region could not be opened because locked
-                                 by chipset.
-  @retval EFI_INVALID_PARAMETER  The descriptor index was out of bounds.
-
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-SmmAccessPeiOpen (
-  IN EFI_PEI_SERVICES                **PeiServices,
-  IN PEI_SMM_ACCESS_PPI              *This,
-  IN UINTN                           DescriptorIndex
-  )
-{
-  if (DescriptorIndex >= DescIdxCount) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  //
-  // According to current practice, DescriptorIndex is not considered at all,
-  // beyond validating it.
-  //
-  return SmramAccessOpen (&This->LockState, &This->OpenState);
-}
-
-/**
-  Inhibits access to the SMRAM.
-
-  This function "closes" SMRAM so that it is not visible while outside of SMM.
-  The function should return EFI_UNSUPPORTED if the hardware does not support
-  hiding of SMRAM.
-
-  @param  PeiServices              General purpose services available to every
-                                   PEIM.
-  @param  This                     The pointer to the SMM Access Interface.
-  @param  DescriptorIndex          The region of SMRAM to Close.
-
-  @retval EFI_SUCCESS              The region was successfully closed.
-  @retval EFI_DEVICE_ERROR         The region could not be closed because
-                                   locked by chipset.
-  @retval EFI_INVALID_PARAMETER    The descriptor index was out of bounds.
-
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-SmmAccessPeiClose (
-  IN EFI_PEI_SERVICES                **PeiServices,
-  IN PEI_SMM_ACCESS_PPI              *This,
-  IN UINTN                           DescriptorIndex
-  )
-{
-  if (DescriptorIndex >= DescIdxCount) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  //
-  // According to current practice, DescriptorIndex is not considered at all,
-  // beyond validating it.
-  //
-  return SmramAccessClose (&This->LockState, &This->OpenState);
-}
-
-/**
-  Inhibits access to the SMRAM.
-
-  This function prohibits access to the SMRAM region.  This function is usually
-  implemented such that it is a write-once operation.
-
-  @param  PeiServices              General purpose services available to every
-                                   PEIM.
-  @param  This                     The pointer to the SMM Access Interface.
-  @param  DescriptorIndex          The region of SMRAM to Close.
-
-  @retval EFI_SUCCESS            The region was successfully locked.
-  @retval EFI_DEVICE_ERROR       The region could not be locked because at
-                                 least one range is still open.
-  @retval EFI_INVALID_PARAMETER  The descriptor index was out of bounds.
-
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-SmmAccessPeiLock (
-  IN EFI_PEI_SERVICES                **PeiServices,
-  IN PEI_SMM_ACCESS_PPI              *This,
-  IN UINTN                           DescriptorIndex
-  )
-{
-  if (DescriptorIndex >= DescIdxCount) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  //
-  // According to current practice, DescriptorIndex is not considered at all,
-  // beyond validating it.
-  //
-  return SmramAccessLock (&This->LockState, &This->OpenState);
-}
-
-/**
-  Queries the memory controller for the possible regions that will support
-  SMRAM.
-
-  @param  PeiServices           General purpose services available to every
-                                PEIM.
-  @param This                   The pointer to the SmmAccessPpi Interface.
-  @param SmramMapSize           The pointer to the variable containing size of
-                                the buffer to contain the description
-                                information.
-  @param SmramMap               The buffer containing the data describing the
-                                Smram region descriptors.
-
-  @retval EFI_BUFFER_TOO_SMALL  The user did not provide a sufficient buffer.
-  @retval EFI_SUCCESS           The user provided a sufficiently-sized buffer.
-
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-SmmAccessPeiGetCapabilities (
-  IN EFI_PEI_SERVICES                **PeiServices,
-  IN PEI_SMM_ACCESS_PPI              *This,
-  IN OUT UINTN                       *SmramMapSize,
-  IN OUT EFI_SMRAM_DESCRIPTOR        *SmramMap
-  )
-{
-  return SmramAccessGetCapabilities (This->LockState, This->OpenState, SmramMapSize, SmramMap);
-}
-
-//
-// LockState and OpenState will be filled in by the entry point.
-//
-STATIC PEI_SMM_ACCESS_PPI mAccess = {
-  &SmmAccessPeiOpen,
-  &SmmAccessPeiClose,
-  &SmmAccessPeiLock,
-  &SmmAccessPeiGetCapabilities
-};
-
-
-STATIC EFI_PEI_PPI_DESCRIPTOR mPpiList[] = {
-  {
-    EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
-    &gPeiSmmAccessPpiGuid, &mAccess
-  }
-};
-
-
-//
-// Utility functions.
-//
-STATIC
-UINT8
-CmosRead8 (
-  IN UINT8  Index
-  )
-{
-  IoWrite8 (0x70, Index);
-  return IoRead8 (0x71);
-}
-
-STATIC
-UINT32
-GetSystemMemorySizeBelow4gb (
-  VOID
-  )
-{
-  UINT32 Cmos0x34;
-  UINT32 Cmos0x35;
-
-  Cmos0x34 = CmosRead8 (0x34);
-  Cmos0x35 = CmosRead8 (0x35);
-
-  return ((Cmos0x35 << 8 | Cmos0x34) << 16) + SIZE_16MB;
-}
-
-
-//
-// Entry point of this driver.
-//
-EFI_STATUS
-EFIAPI
-SmmAccessPeiEntryPoint (
-  IN       EFI_PEI_FILE_HANDLE  FileHandle,
-  IN CONST EFI_PEI_SERVICES     **PeiServices
-  )
-{
-  UINT16               HostBridgeDevId;
-  UINT32                EsmramcVal;
-  UINT32               TopOfLowRam, TopOfLowRamMb;
-  EFI_STATUS           Status;
-  UINTN                SmramMapSize;
-  EFI_SMRAM_DESCRIPTOR SmramMap[DescIdxCount];
-
-  //
-  // This module should only be included if SMRAM support is required.
-  //
-  ASSERT (FeaturePcdGet (PcdSmmSmramRequire));
-
-  //
-  // Verify if we're running on a X58 machine type.
-  //
-  HostBridgeDevId = PciRead16 (SIMICS_HOSTBRIDGE_DID);
-  if (HostBridgeDevId != INTEL_ICH10_DEVICE_ID) {
-    DEBUG ((EFI_D_ERROR, "%a: no SMRAM with host bridge DID=0x%04x; only "
-      "DID=0x%04x (X58) is supported\n", __FUNCTION__, HostBridgeDevId,
-      INTEL_ICH10_DEVICE_ID));
-    goto WrongConfig;
-  }
-
-  //
-  // Confirm if Simics supports SMRAM.
-  //
-  // With no support for it, the ESMRAMC (Extended System Management RAM
-  // Control) register reads as zero. If there is support, the cache-enable
-  // bits are hard-coded as 1 by Simics.
-  //
-
-  TopOfLowRam = GetSystemMemorySizeBelow4gb ();
-  ASSERT ((TopOfLowRam & (SIZE_1MB - 1)) == 0);
-  TopOfLowRamMb = TopOfLowRam >> 20;
-  DEBUG((EFI_D_INFO, "TopOfLowRam =0x%x; TopOfLowRamMb =0x%x \n", TopOfLowRam, TopOfLowRamMb));
-
-
-  //
-  // Set Top of Low Usable DRAM.
-  //
-  PciWrite32 (DRAMC_REGISTER_X58(MCH_TOLUD), TopOfLowRam);
-  DEBUG((EFI_D_INFO, "MCH_TOLUD =0x%x; \n", PciRead32(DRAMC_REGISTER_X58(MCH_TOLUD))));
-
-  //
-  // Set TSEG Memory Base.
-  //
-  EsmramcVal = (TopOfLowRamMb - FixedPcdGet8(PcdX58TsegMbytes)) << MCH_TSEGMB_MB_SHIFT;
-  //
-  // Set TSEG size, and disable TSEG visibility outside of SMM. Note that the
-  // T_EN bit has inverse meaning; when T_EN is set, then TSEG visibility is
-  // *restricted* to SMM.
-  //
-  EsmramcVal &= ~(UINT32)MCH_ESMRAMC_TSEG_MASK;
-  EsmramcVal |= FixedPcdGet8 (PcdX58TsegMbytes) == 8 ? MCH_ESMRAMC_TSEG_8MB :
-                FixedPcdGet8 (PcdX58TsegMbytes) == 2 ? MCH_ESMRAMC_TSEG_2MB :
-                MCH_ESMRAMC_TSEG_1MB;
-  EsmramcVal |= MCH_ESMRAMC_T_EN;
-  PciWrite32(DRAMC_REGISTER_X58(MCH_TSEGMB), EsmramcVal);
-  DEBUG((EFI_D_INFO, "MCH_TSEGMB =0x%x; \n", PciRead32(DRAMC_REGISTER_X58(MCH_TSEGMB))));
-  DEBUG((EFI_D_INFO, "MCH_TSEGMB_1 =0x%x; MCH_TSEGMB_2 =0x%x;\n", ((TopOfLowRamMb - FixedPcdGet8(PcdX58TsegMbytes)) << MCH_TSEGMB_MB_SHIFT), EsmramcVal));
-
-  //
-  // Create the GUID HOB and point it to the first SMRAM range.
-  //
-  GetStates (&mAccess.LockState, &mAccess.OpenState);
-  SmramMapSize = sizeof SmramMap;
-  Status = SmramAccessGetCapabilities (mAccess.LockState, mAccess.OpenState, &SmramMapSize, SmramMap);
-  ASSERT_EFI_ERROR (Status);
-
-  DEBUG_CODE_BEGIN ();
-  {
-    UINTN Count;
-    UINTN Idx;
-
-    Count = SmramMapSize / sizeof SmramMap[0];
-    DEBUG ((EFI_D_VERBOSE, "%a: SMRAM map follows, %d entries\n", __FUNCTION__, (INT32)Count));
-    DEBUG ((EFI_D_VERBOSE, "% 20a % 20a % 20a % 20a\n", "PhysicalStart(0x)",
-      "PhysicalSize(0x)", "CpuStart(0x)", "RegionState(0x)"));
-    for (Idx = 0; Idx < Count; ++Idx) {
-      DEBUG ((EFI_D_VERBOSE, "% 20Lx % 20Lx % 20Lx % 20Lx\n",
-        SmramMap[Idx].PhysicalStart, SmramMap[Idx].PhysicalSize,
-        SmramMap[Idx].CpuStart, SmramMap[Idx].RegionState));
-    }
-  }
-  DEBUG_CODE_END ();
-
-  //
-  // We're done. The next step should succeed, but even if it fails, we can't
-  // roll back the above BuildGuidHob() allocation, because PEI doesn't support
-  // releasing memory.
-  //
-  return PeiServicesInstallPpi (mPpiList);
-
-WrongConfig:
-  //
-  // We really don't want to continue in this case.
-  //
-  ASSERT (FALSE);
-  CpuDeadLoop ();
-  return EFI_UNSUPPORTED;
-}
diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
deleted file mode 100644
index 3c71e64fe9..0000000000
--- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
+++ /dev/null
@@ -1,62 +0,0 @@
-## @file
-# A PEIM with the following responsibilities:
-#
-# - provide SMRAM access by producing PEI_SMM_ACCESS_PPI,
-# - verify & configure the X58 TSEG in the entry point,
-# - set aside the SMM_S3_RESUME_STATE object at the bottom of TSEG, and expose
-#   it via the gEfiAcpiVariableGuid GUIDed HOB.
-#
-# Copyright (C) 2013, 2015, Red Hat, Inc.
-# Copyright (C) 2019, Intel Corporation. All rights reserved.<BR>
-#
-# SPDX-License-Identifier: BSD-2-Clause-Patent
-#
-##
-
-[Defines]
-  INF_VERSION                    = 0x00010005
-  BASE_NAME                      = SmmAccessPei
-  FILE_GUID                      = 6C0E75B4-B0B9-44D1-8210-3377D7B4E066
-  MODULE_TYPE                    = PEIM
-  VERSION_STRING                 = 1.0
-  ENTRY_POINT                    = SmmAccessPeiEntryPoint
-
-#
-# The following information is for reference only and not required by the build tools.
-#
-#  VALID_ARCHITECTURES           = IA32 X64
-#
-
-[Sources]
-  SmmAccessPei.c
-  SmramInternal.c
-  SmramInternal.h
-
-[Packages]
-  MdeModulePkg/MdeModulePkg.dec
-  MdePkg/MdePkg.dec
-  SimicsX58SktPkg/SktPkg.dec
-  SimicsIch10Pkg/Ich10Pkg.dec
-
-[LibraryClasses]
-  BaseLib
-  BaseMemoryLib
-  DebugLib
-  HobLib
-  IoLib
-  PcdLib
-  PciLib
-  PeiServicesLib
-  PeimEntryPoint
-
-[FeaturePcd]
-  gSimicsX58PkgTokenSpaceGuid.PcdSmmSmramRequire
-
-[FixedPcd]
-  gSimicsX58PkgTokenSpaceGuid.PcdX58TsegMbytes
-
-[Ppis]
-  gPeiSmmAccessPpiGuid           ## PRODUCES
-
-[Depex]
-  gEfiPeiMemoryDiscoveredPpiGuid
diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.c b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.c
deleted file mode 100644
index 4b5a92f602..0000000000
--- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.c
+++ /dev/null
@@ -1,200 +0,0 @@
-/** @file
-  Functions and types shared by the SMM accessor PEI and DXE modules.
-
-  Copyright (C) 2015, Red Hat, Inc.
-  Copyright (C) 2019, Intel Corporation. All rights reserved.<BR>
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#include <Guid/AcpiS3Context.h>
-#include <Register/X58Ich10.h>
-#include <Library/DebugLib.h>
-#include <Library/PciLib.h>
-
-#include "SmramInternal.h"
-
-BOOLEAN gLockState;
-BOOLEAN gOpenState;
-
-/**
-  Read the MCH_SMRAM and ESMRAMC registers, and update the LockState and
-  OpenState fields in the PEI_SMM_ACCESS_PPI / EFI_SMM_ACCESS2_PROTOCOL object,
-  from the D_LCK and T_EN bits.
-
-  PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL member functions can rely on
-  the LockState and OpenState fields being up-to-date on entry, and they need
-  to restore the same invariant on exit, if they touch the bits in question.
-
-  @param[out] LockState  Reflects the D_LCK bit on output; TRUE iff SMRAM is
-                         locked.
-  @param[out] OpenState  Reflects the inverse of the T_EN bit on output; TRUE
-                         iff SMRAM is open.
-**/
-VOID
-GetStates (
-  OUT BOOLEAN *LockState,
-  OUT BOOLEAN *OpenState
-)
-{
-  UINT8 EsmramcVal;
-
-  EsmramcVal = PciRead8(DRAMC_REGISTER_X58(MCH_TSEGMB));
-
-  *OpenState = !(EsmramcVal & MCH_ESMRAMC_T_EN);
-  *LockState = !*OpenState;
-
-  *OpenState = gOpenState;
-  *LockState = gLockState;
-}
-
-//
-// The functions below follow the PEI_SMM_ACCESS_PPI and
-// EFI_SMM_ACCESS2_PROTOCOL member declarations. The PeiServices and This
-// pointers are removed (TSEG doesn't depend on them), and so is the
-// DescriptorIndex parameter (TSEG doesn't support range-wise locking).
-//
-// The LockState and OpenState members that are common to both
-// PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL are taken and updated in
-// isolation from the rest of the (non-shared) members.
-//
-
-EFI_STATUS
-SmramAccessOpen (
-  OUT BOOLEAN *LockState,
-  OUT BOOLEAN *OpenState
-  )
-{
-
-  //
-  // Open TSEG by clearing T_EN.
-  //
-  PciAnd8(DRAMC_REGISTER_X58(MCH_TSEGMB),
-    (UINT8)((~(UINT32)MCH_ESMRAMC_T_EN) & 0xff));
-
-  gOpenState = TRUE;
-  gLockState = !gOpenState;
-
-  GetStates (LockState, OpenState);
-  if (!*OpenState) {
-    return EFI_DEVICE_ERROR;
-  }
-  return EFI_SUCCESS;
-}
-
-EFI_STATUS
-SmramAccessClose (
-  OUT BOOLEAN *LockState,
-  OUT BOOLEAN *OpenState
-  )
-{
-  //
-  // Close TSEG by setting T_EN.
-  //
-  PciOr8(DRAMC_REGISTER_X58(MCH_TSEGMB), MCH_ESMRAMC_T_EN);
-
-  gOpenState = FALSE;
-  gLockState = !gOpenState;
-
-  GetStates (LockState, OpenState);
-  if (*OpenState) {
-    return EFI_DEVICE_ERROR;
-  }
-  return EFI_SUCCESS;
-}
-
-EFI_STATUS
-SmramAccessLock (
-  OUT    BOOLEAN *LockState,
-  IN OUT BOOLEAN *OpenState
-  )
-{
-  if (*OpenState) {
-    return EFI_DEVICE_ERROR;
-  }
-
-  //
-  // Close & lock TSEG by setting T_EN and D_LCK.
-  //
-  PciOr8 (DRAMC_REGISTER_X58(MCH_TSEGMB), MCH_ESMRAMC_T_EN);
-
-  gOpenState = FALSE;
-  gLockState = !gOpenState;
-
-  GetStates (LockState, OpenState);
-  if (*OpenState || !*LockState) {
-    return EFI_DEVICE_ERROR;
-  }
-  return EFI_SUCCESS;
-}
-
-EFI_STATUS
-SmramAccessGetCapabilities (
-  IN BOOLEAN                  LockState,
-  IN BOOLEAN                  OpenState,
-  IN OUT UINTN                *SmramMapSize,
-  IN OUT EFI_SMRAM_DESCRIPTOR *SmramMap
-  )
-{
-  UINTN  OriginalSize;
-  UINT32 TsegMemoryBaseMb, TsegMemoryBase;
-  UINT64 CommonRegionState;
-  UINT8  TsegSizeBits;
-
-  OriginalSize  = *SmramMapSize;
-  *SmramMapSize = DescIdxCount * sizeof *SmramMap;
-  if (OriginalSize < *SmramMapSize) {
-    return EFI_BUFFER_TOO_SMALL;
-  }
-
-  //
-  // Read the TSEG Memory Base register.
-  //
-  TsegMemoryBaseMb = PciRead32(DRAMC_REGISTER_X58(MCH_TSEGMB));
-
-  TsegMemoryBaseMb = 0xDF800000;
-
-  TsegMemoryBase = (TsegMemoryBaseMb >> MCH_TSEGMB_MB_SHIFT) << 20;
-
-  //
-  // Precompute the region state bits that will be set for all regions.
-  //
-  CommonRegionState = (OpenState ? EFI_SMRAM_OPEN : EFI_SMRAM_CLOSED) |
-                      (LockState ? EFI_SMRAM_LOCKED : 0) |
-                      EFI_CACHEABLE;
-
-  //
-  // The first region hosts an SMM_S3_RESUME_STATE object. It is located at the
-  // start of TSEG. We round up the size to whole pages, and we report it as
-  // EFI_ALLOCATED, so that the SMM_CORE stays away from it.
-  //
-  SmramMap[DescIdxSmmS3ResumeState].PhysicalStart = TsegMemoryBase;
-  SmramMap[DescIdxSmmS3ResumeState].CpuStart      = TsegMemoryBase;
-  SmramMap[DescIdxSmmS3ResumeState].PhysicalSize  =
-    EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (sizeof (SMM_S3_RESUME_STATE)));
-  SmramMap[DescIdxSmmS3ResumeState].RegionState   =
-    CommonRegionState | EFI_ALLOCATED;
-
-  //
-  // Get the TSEG size bits from the ESMRAMC register.
-  //
-  TsegSizeBits = PciRead8 (DRAMC_REGISTER_X58(MCH_TSEGMB)) &
-                            MCH_ESMRAMC_TSEG_MASK;
-
-  TsegSizeBits = MCH_ESMRAMC_TSEG_8MB;
-
-  //
-  // The second region is the main one, following the first.
-  //
-  SmramMap[DescIdxMain].PhysicalStart =
-    SmramMap[DescIdxSmmS3ResumeState].PhysicalStart +
-    SmramMap[DescIdxSmmS3ResumeState].PhysicalSize;
-  SmramMap[DescIdxMain].CpuStart = SmramMap[DescIdxMain].PhysicalStart;
-  SmramMap[DescIdxMain].PhysicalSize =
-    (TsegSizeBits == MCH_ESMRAMC_TSEG_8MB ? SIZE_8MB :
-     TsegSizeBits == MCH_ESMRAMC_TSEG_2MB ? SIZE_2MB :
-     SIZE_1MB) - SmramMap[DescIdxSmmS3ResumeState].PhysicalSize;
-  SmramMap[DescIdxMain].RegionState = CommonRegionState;
-
-  return EFI_SUCCESS;
-}
diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.h b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.h
deleted file mode 100644
index 81180a9c8e..0000000000
--- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.h
+++ /dev/null
@@ -1,82 +0,0 @@
-/** @file
-  Functions and types shared by the SMM accessor PEI and DXE modules.
-
-  Copyright (C) 2015, Red Hat, Inc.
-  Copyright (C) 2019, Intel Corporation. All rights reserved.<BR>
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#include <Pi/PiMultiPhase.h>
-
-//
-// We'll have two SMRAM ranges.
-//
-// The first is a tiny one that hosts an SMM_S3_RESUME_STATE object, to be
-// filled in by the CPU SMM driver during normal boot, for the PEI instance of
-// the LockBox library (which will rely on the object during S3 resume).
-//
-// The other SMRAM range is the main one, for the SMM core and the SMM drivers.
-//
-typedef enum {
-  DescIdxSmmS3ResumeState = 0,
-  DescIdxMain             = 1,
-  DescIdxCount            = 2
-} DESCRIPTOR_INDEX;
-
-/**
-  Read the MCH_SMRAM and ESMRAMC registers, and update the LockState and
-  OpenState fields in the PEI_SMM_ACCESS_PPI / EFI_SMM_ACCESS2_PROTOCOL object,
-  from the D_LCK and T_EN bits.
-
-  PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL member functions can rely on
-  the LockState and OpenState fields being up-to-date on entry, and they need
-  to restore the same invariant on exit, if they touch the bits in question.
-
-  @param[out] LockState  Reflects the D_LCK bit on output; TRUE iff SMRAM is
-                         locked.
-  @param[out] OpenState  Reflects the inverse of the T_EN bit on output; TRUE
-                         iff SMRAM is open.
-**/
-VOID
-GetStates (
-  OUT BOOLEAN *LockState,
-  OUT BOOLEAN *OpenState
-  );
-
-//
-// The functions below follow the PEI_SMM_ACCESS_PPI and
-// EFI_SMM_ACCESS2_PROTOCOL member declarations. The PeiServices and This
-// pointers are removed (TSEG doesn't depend on them), and so is the
-// DescriptorIndex parameter (TSEG doesn't support range-wise locking).
-//
-// The LockState and OpenState members that are common to both
-// PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL are taken and updated in
-// isolation from the rest of the (non-shared) members.
-//
-
-EFI_STATUS
-SmramAccessOpen (
-  OUT BOOLEAN *LockState,
-  OUT BOOLEAN *OpenState
-  );
-
-EFI_STATUS
-SmramAccessClose (
-  OUT BOOLEAN *LockState,
-  OUT BOOLEAN *OpenState
-  );
-
-EFI_STATUS
-SmramAccessLock (
-  OUT    BOOLEAN *LockState,
-  IN OUT BOOLEAN *OpenState
-  );
-
-EFI_STATUS
-SmramAccessGetCapabilities (
-  IN BOOLEAN                  LockState,
-  IN BOOLEAN                  OpenState,
-  IN OUT UINTN                *SmramMapSize,
-  IN OUT EFI_SMRAM_DESCRIPTOR *SmramMap
-  );
-- 
2.31.1.windows.1

Re: [PATCH 5/5] SimicsX58SktPkg: Remove unused Smm related modules

[Ni, Ray]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Tuesday, April 25, 2023 3:03 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH 5/5] SimicsX58SktPkg: Remove unused Smm related
> modules
> 
> In last two commit, I replace the two SMM related modules, and now
> no platform will use these two moduels. Remove them
> 
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  .../Smm/Access/SmmAccess2Dxe.c                | 148 --------
>  .../Smm/Access/SmmAccess2Dxe.inf              |  54 ---
>  .../SimicsX58SktPkg/Smm/Access/SmmAccessPei.c | 338 ------------------
>  .../Smm/Access/SmmAccessPei.inf               |  62 ----
>  .../Smm/Access/SmramInternal.c                | 200 -----------
>  .../Smm/Access/SmramInternal.h                |  82 -----
>  6 files changed, 884 deletions(-)
>  delete mode 100644
> Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.c
>  delete mode 100644
> Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.inf
>  delete mode 100644
> Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
>  delete mode 100644
> Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
>  delete mode 100644
> Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.c
>  delete mode 100644
> Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.h
> 
> diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.c
> b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.c
> deleted file mode 100644
> index 5d3b2c14aa..0000000000
> --- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.c
> +++ /dev/null
> @@ -1,148 +0,0 @@
> -/** @file
> -  A DXE_DRIVER providing SMRAM access by producing
> EFI_SMM_ACCESS2_PROTOCOL.
> -
> -  X58 TSEG is expected to have been verified and set up by the
> SmmAccessPei
> -  driver.
> -
> -  Copyright (C) 2013, 2015, Red Hat, Inc.<BR>
> -  Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> -**/
> -
> -#include <Library/DebugLib.h>
> -#include <Library/PcdLib.h>
> -#include <Library/UefiBootServicesTableLib.h>
> -#include <Protocol/SmmAccess2.h>
> -
> -#include "SmramInternal.h"
> -
> -/**
> -  Opens the SMRAM area to be accessible by a boot-service driver.
> -
> -  This function "opens" SMRAM so that it is visible while not inside of SMM.
> -  The function should return EFI_UNSUPPORTED if the hardware does not
> support
> -  hiding of SMRAM. The function should return EFI_DEVICE_ERROR if the
> SMRAM
> -  configuration is locked.
> -
> -  @param[in] This           The EFI_SMM_ACCESS2_PROTOCOL instance.
> -
> -  @retval EFI_SUCCESS       The operation was successful.
> -  @retval EFI_UNSUPPORTED   The system does not support opening and
> closing of
> -                            SMRAM.
> -  @retval EFI_DEVICE_ERROR  SMRAM cannot be opened, perhaps because
> it is
> -                            locked.
> -**/
> -STATIC
> -EFI_STATUS
> -EFIAPI
> -SmmAccess2DxeOpen (
> -  IN EFI_SMM_ACCESS2_PROTOCOL  *This
> -  )
> -{
> -  return SmramAccessOpen (&This->LockState, &This->OpenState);
> -}
> -
> -/**
> -  Inhibits access to the SMRAM.
> -
> -  This function "closes" SMRAM so that it is not visible while outside of SMM.
> -  The function should return EFI_UNSUPPORTED if the hardware does not
> support
> -  hiding of SMRAM.
> -
> -  @param[in] This           The EFI_SMM_ACCESS2_PROTOCOL instance.
> -
> -  @retval EFI_SUCCESS       The operation was successful.
> -  @retval EFI_UNSUPPORTED   The system does not support opening and
> closing of
> -                            SMRAM.
> -  @retval EFI_DEVICE_ERROR  SMRAM cannot be closed.
> -**/
> -STATIC
> -EFI_STATUS
> -EFIAPI
> -SmmAccess2DxeClose (
> -  IN EFI_SMM_ACCESS2_PROTOCOL  *This
> -  )
> -{
> -  return SmramAccessClose (&This->LockState, &This->OpenState);
> -}
> -
> -/**
> -  Inhibits access to the SMRAM.
> -
> -  This function prohibits access to the SMRAM region.  This function is usually
> -  implemented such that it is a write-once operation.
> -
> -  @param[in] This          The EFI_SMM_ACCESS2_PROTOCOL instance.
> -
> -  @retval EFI_SUCCESS      The device was successfully locked.
> -  @retval EFI_UNSUPPORTED  The system does not support locking of
> SMRAM.
> -**/
> -STATIC
> -EFI_STATUS
> -EFIAPI
> -SmmAccess2DxeLock (
> -  IN EFI_SMM_ACCESS2_PROTOCOL  *This
> -  )
> -{
> -  return SmramAccessLock (&This->LockState, &This->OpenState);
> -}
> -
> -/**
> -  Queries the memory controller for the possible regions that will support
> -  SMRAM.
> -
> -  @param[in]     This           The EFI_SMM_ACCESS2_PROTOCOL instance.
> -  @param[in,out] SmramMapSize   A pointer to the size, in bytes, of the
> -                                SmramMemoryMap buffer.
> -  @param[in,out] SmramMap       A pointer to the buffer in which firmware
> -                                places the current memory map.
> -
> -  @retval EFI_SUCCESS           The chipset supported the given resource.
> -  @retval EFI_BUFFER_TOO_SMALL  The SmramMap parameter was too
> small.  The
> -                                current buffer size needed to hold the memory
> -                                map is returned in SmramMapSize.
> -**/
> -STATIC
> -EFI_STATUS
> -EFIAPI
> -SmmAccess2DxeGetCapabilities (
> -  IN CONST EFI_SMM_ACCESS2_PROTOCOL  *This,
> -  IN OUT UINTN                       *SmramMapSize,
> -  IN OUT EFI_SMRAM_DESCRIPTOR        *SmramMap
> -  )
> -{
> -  return SmramAccessGetCapabilities (This->LockState, This->OpenState,
> -           SmramMapSize, SmramMap);
> -}
> -
> -//
> -// LockState and OpenState will be filled in by the entry point.
> -//
> -STATIC EFI_SMM_ACCESS2_PROTOCOL mAccess2 = {
> -  &SmmAccess2DxeOpen,
> -  &SmmAccess2DxeClose,
> -  &SmmAccess2DxeLock,
> -  &SmmAccess2DxeGetCapabilities
> -};
> -
> -//
> -// Entry point of this driver.
> -//
> -EFI_STATUS
> -EFIAPI
> -SmmAccess2DxeEntryPoint (
> -  IN EFI_HANDLE       ImageHandle,
> -  IN EFI_SYSTEM_TABLE *SystemTable
> -  )
> -{
> -  //
> -  // This module should only be included if SMRAM support is required.
> -  //
> -  ASSERT (FeaturePcdGet (PcdSmmSmramRequire));
> -
> -  GetStates (&mAccess2.LockState, &mAccess2.OpenState);
> -  return gBS->InstallMultipleProtocolInterfaces (&ImageHandle,
> -                &gEfiSmmAccess2ProtocolGuid, &mAccess2,
> -                NULL);
> -}
> diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.inf
> b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.inf
> deleted file mode 100644
> index eb8c8f93dd..0000000000
> --- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccess2Dxe.inf
> +++ /dev/null
> @@ -1,54 +0,0 @@
> -## @file
> -# A DXE_DRIVER providing SMRAM access by producing
> EFI_SMM_ACCESS2_PROTOCOL.
> -#
> -# X58 TSEG is expected to have been verified and set up by the
> SmmAccessPei
> -# driver.
> -#
> -# Copyright (C) 2013, 2015, Red Hat, Inc.
> -# Copyright (C) 2019, Intel Corporation. All rights reserved.<BR>
> -#
> -# SPDX-License-Identifier: BSD-2-Clause-Patent
> -#
> -##
> -
> -[Defines]
> -  INF_VERSION                    = 0x00010005
> -  BASE_NAME                      = SmmAccess2Dxe
> -  FILE_GUID                      = AC95AD3D-4366-44BF-9A62-E4B29D7A2206
> -  MODULE_TYPE                    = DXE_DRIVER
> -  VERSION_STRING                 = 1.0
> -  PI_SPECIFICATION_VERSION       = 0x00010400
> -  ENTRY_POINT                    = SmmAccess2DxeEntryPoint
> -
> -#
> -# The following information is for reference only and not required by the
> build tools.
> -#
> -#  VALID_ARCHITECTURES           = IA32 X64
> -#
> -
> -[Sources]
> -  SmmAccess2Dxe.c
> -  SmramInternal.c
> -  SmramInternal.h
> -
> -[Packages]
> -  MdeModulePkg/MdeModulePkg.dec
> -  MdePkg/MdePkg.dec
> -  SimicsX58SktPkg/SktPkg.dec
> -  SimicsIch10Pkg/Ich10Pkg.dec
> -
> -[LibraryClasses]
> -  DebugLib
> -  PcdLib
> -  PciLib
> -  UefiBootServicesTableLib
> -  UefiDriverEntryPoint
> -
> -[Protocols]
> -  gEfiSmmAccess2ProtocolGuid   ## PRODUCES
> -
> -[FeaturePcd]
> -  gSimicsX58PkgTokenSpaceGuid.PcdSmmSmramRequire
> -
> -[Depex]
> -  TRUE
> diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
> b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
> deleted file mode 100644
> index d489cc7513..0000000000
> --- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
> +++ /dev/null
> @@ -1,338 +0,0 @@
> -/** @file
> -  A PEIM with the following responsibilities:
> -
> -  - verify & configure the X58 TSEG in the entry point,
> -  - provide SMRAM access by producing PEI_SMM_ACCESS_PPI,
> -  - set aside the SMM_S3_RESUME_STATE object at the bottom of TSEG, and
> expose
> -    it via the gEfiAcpiVariableGuid GUID HOB.
> -
> -  This PEIM runs from RAM, so we can write to variables with static storage
> -  duration.
> -
> -  Copyright (C) 2013, 2015, Red Hat, Inc.<BR>
> -  Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> -**/
> -
> -#include <Guid/AcpiS3Context.h>
> -#include <Library/BaseLib.h>
> -#include <Library/BaseMemoryLib.h>
> -#include <Library/DebugLib.h>
> -#include <Library/HobLib.h>
> -#include <Library/IoLib.h>
> -#include <Library/PcdLib.h>
> -#include <Library/PciLib.h>
> -#include <Library/PeiServicesLib.h>
> -#include <Ppi/SmmAccess.h>
> -
> -#include <Register/X58Ich10.h>
> -#include "SmramInternal.h"
> -
> -//
> -// PEI_SMM_ACCESS_PPI implementation.
> -//
> -
> -/**
> -  Opens the SMRAM area to be accessible by a PEIM driver.
> -
> -  This function "opens" SMRAM so that it is visible while not inside of SMM.
> -  The function should return EFI_UNSUPPORTED if the hardware does not
> support
> -  hiding of SMRAM. The function should return EFI_DEVICE_ERROR if the
> SMRAM
> -  configuration is locked.
> -
> -  @param  PeiServices            General purpose services available to every
> -                                 PEIM.
> -  @param  This                   The pointer to the SMM Access Interface.
> -  @param  DescriptorIndex        The region of SMRAM to Open.
> -
> -  @retval EFI_SUCCESS            The region was successfully opened.
> -  @retval EFI_DEVICE_ERROR       The region could not be opened because
> locked
> -                                 by chipset.
> -  @retval EFI_INVALID_PARAMETER  The descriptor index was out of bounds.
> -
> -**/
> -STATIC
> -EFI_STATUS
> -EFIAPI
> -SmmAccessPeiOpen (
> -  IN EFI_PEI_SERVICES                **PeiServices,
> -  IN PEI_SMM_ACCESS_PPI              *This,
> -  IN UINTN                           DescriptorIndex
> -  )
> -{
> -  if (DescriptorIndex >= DescIdxCount) {
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
> -  //
> -  // According to current practice, DescriptorIndex is not considered at all,
> -  // beyond validating it.
> -  //
> -  return SmramAccessOpen (&This->LockState, &This->OpenState);
> -}
> -
> -/**
> -  Inhibits access to the SMRAM.
> -
> -  This function "closes" SMRAM so that it is not visible while outside of SMM.
> -  The function should return EFI_UNSUPPORTED if the hardware does not
> support
> -  hiding of SMRAM.
> -
> -  @param  PeiServices              General purpose services available to every
> -                                   PEIM.
> -  @param  This                     The pointer to the SMM Access Interface.
> -  @param  DescriptorIndex          The region of SMRAM to Close.
> -
> -  @retval EFI_SUCCESS              The region was successfully closed.
> -  @retval EFI_DEVICE_ERROR         The region could not be closed because
> -                                   locked by chipset.
> -  @retval EFI_INVALID_PARAMETER    The descriptor index was out of
> bounds.
> -
> -**/
> -STATIC
> -EFI_STATUS
> -EFIAPI
> -SmmAccessPeiClose (
> -  IN EFI_PEI_SERVICES                **PeiServices,
> -  IN PEI_SMM_ACCESS_PPI              *This,
> -  IN UINTN                           DescriptorIndex
> -  )
> -{
> -  if (DescriptorIndex >= DescIdxCount) {
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
> -  //
> -  // According to current practice, DescriptorIndex is not considered at all,
> -  // beyond validating it.
> -  //
> -  return SmramAccessClose (&This->LockState, &This->OpenState);
> -}
> -
> -/**
> -  Inhibits access to the SMRAM.
> -
> -  This function prohibits access to the SMRAM region.  This function is usually
> -  implemented such that it is a write-once operation.
> -
> -  @param  PeiServices              General purpose services available to every
> -                                   PEIM.
> -  @param  This                     The pointer to the SMM Access Interface.
> -  @param  DescriptorIndex          The region of SMRAM to Close.
> -
> -  @retval EFI_SUCCESS            The region was successfully locked.
> -  @retval EFI_DEVICE_ERROR       The region could not be locked because at
> -                                 least one range is still open.
> -  @retval EFI_INVALID_PARAMETER  The descriptor index was out of bounds.
> -
> -**/
> -STATIC
> -EFI_STATUS
> -EFIAPI
> -SmmAccessPeiLock (
> -  IN EFI_PEI_SERVICES                **PeiServices,
> -  IN PEI_SMM_ACCESS_PPI              *This,
> -  IN UINTN                           DescriptorIndex
> -  )
> -{
> -  if (DescriptorIndex >= DescIdxCount) {
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
> -  //
> -  // According to current practice, DescriptorIndex is not considered at all,
> -  // beyond validating it.
> -  //
> -  return SmramAccessLock (&This->LockState, &This->OpenState);
> -}
> -
> -/**
> -  Queries the memory controller for the possible regions that will support
> -  SMRAM.
> -
> -  @param  PeiServices           General purpose services available to every
> -                                PEIM.
> -  @param This                   The pointer to the SmmAccessPpi Interface.
> -  @param SmramMapSize           The pointer to the variable containing size of
> -                                the buffer to contain the description
> -                                information.
> -  @param SmramMap               The buffer containing the data describing the
> -                                Smram region descriptors.
> -
> -  @retval EFI_BUFFER_TOO_SMALL  The user did not provide a sufficient
> buffer.
> -  @retval EFI_SUCCESS           The user provided a sufficiently-sized buffer.
> -
> -**/
> -STATIC
> -EFI_STATUS
> -EFIAPI
> -SmmAccessPeiGetCapabilities (
> -  IN EFI_PEI_SERVICES                **PeiServices,
> -  IN PEI_SMM_ACCESS_PPI              *This,
> -  IN OUT UINTN                       *SmramMapSize,
> -  IN OUT EFI_SMRAM_DESCRIPTOR        *SmramMap
> -  )
> -{
> -  return SmramAccessGetCapabilities (This->LockState, This->OpenState,
> SmramMapSize, SmramMap);
> -}
> -
> -//
> -// LockState and OpenState will be filled in by the entry point.
> -//
> -STATIC PEI_SMM_ACCESS_PPI mAccess = {
> -  &SmmAccessPeiOpen,
> -  &SmmAccessPeiClose,
> -  &SmmAccessPeiLock,
> -  &SmmAccessPeiGetCapabilities
> -};
> -
> -
> -STATIC EFI_PEI_PPI_DESCRIPTOR mPpiList[] = {
> -  {
> -    EFI_PEI_PPI_DESCRIPTOR_PPI |
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
> -    &gPeiSmmAccessPpiGuid, &mAccess
> -  }
> -};
> -
> -
> -//
> -// Utility functions.
> -//
> -STATIC
> -UINT8
> -CmosRead8 (
> -  IN UINT8  Index
> -  )
> -{
> -  IoWrite8 (0x70, Index);
> -  return IoRead8 (0x71);
> -}
> -
> -STATIC
> -UINT32
> -GetSystemMemorySizeBelow4gb (
> -  VOID
> -  )
> -{
> -  UINT32 Cmos0x34;
> -  UINT32 Cmos0x35;
> -
> -  Cmos0x34 = CmosRead8 (0x34);
> -  Cmos0x35 = CmosRead8 (0x35);
> -
> -  return ((Cmos0x35 << 8 | Cmos0x34) << 16) + SIZE_16MB;
> -}
> -
> -
> -//
> -// Entry point of this driver.
> -//
> -EFI_STATUS
> -EFIAPI
> -SmmAccessPeiEntryPoint (
> -  IN       EFI_PEI_FILE_HANDLE  FileHandle,
> -  IN CONST EFI_PEI_SERVICES     **PeiServices
> -  )
> -{
> -  UINT16               HostBridgeDevId;
> -  UINT32                EsmramcVal;
> -  UINT32               TopOfLowRam, TopOfLowRamMb;
> -  EFI_STATUS           Status;
> -  UINTN                SmramMapSize;
> -  EFI_SMRAM_DESCRIPTOR SmramMap[DescIdxCount];
> -
> -  //
> -  // This module should only be included if SMRAM support is required.
> -  //
> -  ASSERT (FeaturePcdGet (PcdSmmSmramRequire));
> -
> -  //
> -  // Verify if we're running on a X58 machine type.
> -  //
> -  HostBridgeDevId = PciRead16 (SIMICS_HOSTBRIDGE_DID);
> -  if (HostBridgeDevId != INTEL_ICH10_DEVICE_ID) {
> -    DEBUG ((EFI_D_ERROR, "%a: no SMRAM with host bridge DID=0x%04x;
> only "
> -      "DID=0x%04x (X58) is supported\n", __FUNCTION__, HostBridgeDevId,
> -      INTEL_ICH10_DEVICE_ID));
> -    goto WrongConfig;
> -  }
> -
> -  //
> -  // Confirm if Simics supports SMRAM.
> -  //
> -  // With no support for it, the ESMRAMC (Extended System Management
> RAM
> -  // Control) register reads as zero. If there is support, the cache-enable
> -  // bits are hard-coded as 1 by Simics.
> -  //
> -
> -  TopOfLowRam = GetSystemMemorySizeBelow4gb ();
> -  ASSERT ((TopOfLowRam & (SIZE_1MB - 1)) == 0);
> -  TopOfLowRamMb = TopOfLowRam >> 20;
> -  DEBUG((EFI_D_INFO, "TopOfLowRam =0x%x; TopOfLowRamMb =0x%x \n",
> TopOfLowRam, TopOfLowRamMb));
> -
> -
> -  //
> -  // Set Top of Low Usable DRAM.
> -  //
> -  PciWrite32 (DRAMC_REGISTER_X58(MCH_TOLUD), TopOfLowRam);
> -  DEBUG((EFI_D_INFO, "MCH_TOLUD =0x%x; \n",
> PciRead32(DRAMC_REGISTER_X58(MCH_TOLUD))));
> -
> -  //
> -  // Set TSEG Memory Base.
> -  //
> -  EsmramcVal = (TopOfLowRamMb - FixedPcdGet8(PcdX58TsegMbytes)) <<
> MCH_TSEGMB_MB_SHIFT;
> -  //
> -  // Set TSEG size, and disable TSEG visibility outside of SMM. Note that the
> -  // T_EN bit has inverse meaning; when T_EN is set, then TSEG visibility is
> -  // *restricted* to SMM.
> -  //
> -  EsmramcVal &= ~(UINT32)MCH_ESMRAMC_TSEG_MASK;
> -  EsmramcVal |= FixedPcdGet8 (PcdX58TsegMbytes) == 8 ?
> MCH_ESMRAMC_TSEG_8MB :
> -                FixedPcdGet8 (PcdX58TsegMbytes) == 2 ?
> MCH_ESMRAMC_TSEG_2MB :
> -                MCH_ESMRAMC_TSEG_1MB;
> -  EsmramcVal |= MCH_ESMRAMC_T_EN;
> -  PciWrite32(DRAMC_REGISTER_X58(MCH_TSEGMB), EsmramcVal);
> -  DEBUG((EFI_D_INFO, "MCH_TSEGMB =0x%x; \n",
> PciRead32(DRAMC_REGISTER_X58(MCH_TSEGMB))));
> -  DEBUG((EFI_D_INFO, "MCH_TSEGMB_1 =0x%x; MCH_TSEGMB_2
> =0x%x;\n", ((TopOfLowRamMb - FixedPcdGet8(PcdX58TsegMbytes)) <<
> MCH_TSEGMB_MB_SHIFT), EsmramcVal));
> -
> -  //
> -  // Create the GUID HOB and point it to the first SMRAM range.
> -  //
> -  GetStates (&mAccess.LockState, &mAccess.OpenState);
> -  SmramMapSize = sizeof SmramMap;
> -  Status = SmramAccessGetCapabilities (mAccess.LockState,
> mAccess.OpenState, &SmramMapSize, SmramMap);
> -  ASSERT_EFI_ERROR (Status);
> -
> -  DEBUG_CODE_BEGIN ();
> -  {
> -    UINTN Count;
> -    UINTN Idx;
> -
> -    Count = SmramMapSize / sizeof SmramMap[0];
> -    DEBUG ((EFI_D_VERBOSE, "%a: SMRAM map follows, %d entries\n",
> __FUNCTION__, (INT32)Count));
> -    DEBUG ((EFI_D_VERBOSE, "% 20a % 20a % 20a % 20a\n",
> "PhysicalStart(0x)",
> -      "PhysicalSize(0x)", "CpuStart(0x)", "RegionState(0x)"));
> -    for (Idx = 0; Idx < Count; ++Idx) {
> -      DEBUG ((EFI_D_VERBOSE, "% 20Lx % 20Lx % 20Lx % 20Lx\n",
> -        SmramMap[Idx].PhysicalStart, SmramMap[Idx].PhysicalSize,
> -        SmramMap[Idx].CpuStart, SmramMap[Idx].RegionState));
> -    }
> -  }
> -  DEBUG_CODE_END ();
> -
> -  //
> -  // We're done. The next step should succeed, but even if it fails, we can't
> -  // roll back the above BuildGuidHob() allocation, because PEI doesn't
> support
> -  // releasing memory.
> -  //
> -  return PeiServicesInstallPpi (mPpiList);
> -
> -WrongConfig:
> -  //
> -  // We really don't want to continue in this case.
> -  //
> -  ASSERT (FALSE);
> -  CpuDeadLoop ();
> -  return EFI_UNSUPPORTED;
> -}
> diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
> b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
> deleted file mode 100644
> index 3c71e64fe9..0000000000
> --- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
> +++ /dev/null
> @@ -1,62 +0,0 @@
> -## @file
> -# A PEIM with the following responsibilities:
> -#
> -# - provide SMRAM access by producing PEI_SMM_ACCESS_PPI,
> -# - verify & configure the X58 TSEG in the entry point,
> -# - set aside the SMM_S3_RESUME_STATE object at the bottom of TSEG,
> and expose
> -#   it via the gEfiAcpiVariableGuid GUIDed HOB.
> -#
> -# Copyright (C) 2013, 2015, Red Hat, Inc.
> -# Copyright (C) 2019, Intel Corporation. All rights reserved.<BR>
> -#
> -# SPDX-License-Identifier: BSD-2-Clause-Patent
> -#
> -##
> -
> -[Defines]
> -  INF_VERSION                    = 0x00010005
> -  BASE_NAME                      = SmmAccessPei
> -  FILE_GUID                      = 6C0E75B4-B0B9-44D1-8210-3377D7B4E066
> -  MODULE_TYPE                    = PEIM
> -  VERSION_STRING                 = 1.0
> -  ENTRY_POINT                    = SmmAccessPeiEntryPoint
> -
> -#
> -# The following information is for reference only and not required by the
> build tools.
> -#
> -#  VALID_ARCHITECTURES           = IA32 X64
> -#
> -
> -[Sources]
> -  SmmAccessPei.c
> -  SmramInternal.c
> -  SmramInternal.h
> -
> -[Packages]
> -  MdeModulePkg/MdeModulePkg.dec
> -  MdePkg/MdePkg.dec
> -  SimicsX58SktPkg/SktPkg.dec
> -  SimicsIch10Pkg/Ich10Pkg.dec
> -
> -[LibraryClasses]
> -  BaseLib
> -  BaseMemoryLib
> -  DebugLib
> -  HobLib
> -  IoLib
> -  PcdLib
> -  PciLib
> -  PeiServicesLib
> -  PeimEntryPoint
> -
> -[FeaturePcd]
> -  gSimicsX58PkgTokenSpaceGuid.PcdSmmSmramRequire
> -
> -[FixedPcd]
> -  gSimicsX58PkgTokenSpaceGuid.PcdX58TsegMbytes
> -
> -[Ppis]
> -  gPeiSmmAccessPpiGuid           ## PRODUCES
> -
> -[Depex]
> -  gEfiPeiMemoryDiscoveredPpiGuid
> diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.c
> b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.c
> deleted file mode 100644
> index 4b5a92f602..0000000000
> --- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.c
> +++ /dev/null
> @@ -1,200 +0,0 @@
> -/** @file
> -  Functions and types shared by the SMM accessor PEI and DXE modules.
> -
> -  Copyright (C) 2015, Red Hat, Inc.
> -  Copyright (C) 2019, Intel Corporation. All rights reserved.<BR>
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> -**/
> -
> -#include <Guid/AcpiS3Context.h>
> -#include <Register/X58Ich10.h>
> -#include <Library/DebugLib.h>
> -#include <Library/PciLib.h>
> -
> -#include "SmramInternal.h"
> -
> -BOOLEAN gLockState;
> -BOOLEAN gOpenState;
> -
> -/**
> -  Read the MCH_SMRAM and ESMRAMC registers, and update the LockState
> and
> -  OpenState fields in the PEI_SMM_ACCESS_PPI /
> EFI_SMM_ACCESS2_PROTOCOL object,
> -  from the D_LCK and T_EN bits.
> -
> -  PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL member
> functions can rely on
> -  the LockState and OpenState fields being up-to-date on entry, and they
> need
> -  to restore the same invariant on exit, if they touch the bits in question.
> -
> -  @param[out] LockState  Reflects the D_LCK bit on output; TRUE iff SMRAM
> is
> -                         locked.
> -  @param[out] OpenState  Reflects the inverse of the T_EN bit on output;
> TRUE
> -                         iff SMRAM is open.
> -**/
> -VOID
> -GetStates (
> -  OUT BOOLEAN *LockState,
> -  OUT BOOLEAN *OpenState
> -)
> -{
> -  UINT8 EsmramcVal;
> -
> -  EsmramcVal = PciRead8(DRAMC_REGISTER_X58(MCH_TSEGMB));
> -
> -  *OpenState = !(EsmramcVal & MCH_ESMRAMC_T_EN);
> -  *LockState = !*OpenState;
> -
> -  *OpenState = gOpenState;
> -  *LockState = gLockState;
> -}
> -
> -//
> -// The functions below follow the PEI_SMM_ACCESS_PPI and
> -// EFI_SMM_ACCESS2_PROTOCOL member declarations. The PeiServices
> and This
> -// pointers are removed (TSEG doesn't depend on them), and so is the
> -// DescriptorIndex parameter (TSEG doesn't support range-wise locking).
> -//
> -// The LockState and OpenState members that are common to both
> -// PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL are taken
> and updated in
> -// isolation from the rest of the (non-shared) members.
> -//
> -
> -EFI_STATUS
> -SmramAccessOpen (
> -  OUT BOOLEAN *LockState,
> -  OUT BOOLEAN *OpenState
> -  )
> -{
> -
> -  //
> -  // Open TSEG by clearing T_EN.
> -  //
> -  PciAnd8(DRAMC_REGISTER_X58(MCH_TSEGMB),
> -    (UINT8)((~(UINT32)MCH_ESMRAMC_T_EN) & 0xff));
> -
> -  gOpenState = TRUE;
> -  gLockState = !gOpenState;
> -
> -  GetStates (LockState, OpenState);
> -  if (!*OpenState) {
> -    return EFI_DEVICE_ERROR;
> -  }
> -  return EFI_SUCCESS;
> -}
> -
> -EFI_STATUS
> -SmramAccessClose (
> -  OUT BOOLEAN *LockState,
> -  OUT BOOLEAN *OpenState
> -  )
> -{
> -  //
> -  // Close TSEG by setting T_EN.
> -  //
> -  PciOr8(DRAMC_REGISTER_X58(MCH_TSEGMB), MCH_ESMRAMC_T_EN);
> -
> -  gOpenState = FALSE;
> -  gLockState = !gOpenState;
> -
> -  GetStates (LockState, OpenState);
> -  if (*OpenState) {
> -    return EFI_DEVICE_ERROR;
> -  }
> -  return EFI_SUCCESS;
> -}
> -
> -EFI_STATUS
> -SmramAccessLock (
> -  OUT    BOOLEAN *LockState,
> -  IN OUT BOOLEAN *OpenState
> -  )
> -{
> -  if (*OpenState) {
> -    return EFI_DEVICE_ERROR;
> -  }
> -
> -  //
> -  // Close & lock TSEG by setting T_EN and D_LCK.
> -  //
> -  PciOr8 (DRAMC_REGISTER_X58(MCH_TSEGMB), MCH_ESMRAMC_T_EN);
> -
> -  gOpenState = FALSE;
> -  gLockState = !gOpenState;
> -
> -  GetStates (LockState, OpenState);
> -  if (*OpenState || !*LockState) {
> -    return EFI_DEVICE_ERROR;
> -  }
> -  return EFI_SUCCESS;
> -}
> -
> -EFI_STATUS
> -SmramAccessGetCapabilities (
> -  IN BOOLEAN                  LockState,
> -  IN BOOLEAN                  OpenState,
> -  IN OUT UINTN                *SmramMapSize,
> -  IN OUT EFI_SMRAM_DESCRIPTOR *SmramMap
> -  )
> -{
> -  UINTN  OriginalSize;
> -  UINT32 TsegMemoryBaseMb, TsegMemoryBase;
> -  UINT64 CommonRegionState;
> -  UINT8  TsegSizeBits;
> -
> -  OriginalSize  = *SmramMapSize;
> -  *SmramMapSize = DescIdxCount * sizeof *SmramMap;
> -  if (OriginalSize < *SmramMapSize) {
> -    return EFI_BUFFER_TOO_SMALL;
> -  }
> -
> -  //
> -  // Read the TSEG Memory Base register.
> -  //
> -  TsegMemoryBaseMb =
> PciRead32(DRAMC_REGISTER_X58(MCH_TSEGMB));
> -
> -  TsegMemoryBaseMb = 0xDF800000;
> -
> -  TsegMemoryBase = (TsegMemoryBaseMb >> MCH_TSEGMB_MB_SHIFT)
> << 20;
> -
> -  //
> -  // Precompute the region state bits that will be set for all regions.
> -  //
> -  CommonRegionState = (OpenState ? EFI_SMRAM_OPEN :
> EFI_SMRAM_CLOSED) |
> -                      (LockState ? EFI_SMRAM_LOCKED : 0) |
> -                      EFI_CACHEABLE;
> -
> -  //
> -  // The first region hosts an SMM_S3_RESUME_STATE object. It is located at
> the
> -  // start of TSEG. We round up the size to whole pages, and we report it as
> -  // EFI_ALLOCATED, so that the SMM_CORE stays away from it.
> -  //
> -  SmramMap[DescIdxSmmS3ResumeState].PhysicalStart =
> TsegMemoryBase;
> -  SmramMap[DescIdxSmmS3ResumeState].CpuStart      = TsegMemoryBase;
> -  SmramMap[DescIdxSmmS3ResumeState].PhysicalSize  =
> -    EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (sizeof
> (SMM_S3_RESUME_STATE)));
> -  SmramMap[DescIdxSmmS3ResumeState].RegionState   =
> -    CommonRegionState | EFI_ALLOCATED;
> -
> -  //
> -  // Get the TSEG size bits from the ESMRAMC register.
> -  //
> -  TsegSizeBits = PciRead8 (DRAMC_REGISTER_X58(MCH_TSEGMB)) &
> -                            MCH_ESMRAMC_TSEG_MASK;
> -
> -  TsegSizeBits = MCH_ESMRAMC_TSEG_8MB;
> -
> -  //
> -  // The second region is the main one, following the first.
> -  //
> -  SmramMap[DescIdxMain].PhysicalStart =
> -    SmramMap[DescIdxSmmS3ResumeState].PhysicalStart +
> -    SmramMap[DescIdxSmmS3ResumeState].PhysicalSize;
> -  SmramMap[DescIdxMain].CpuStart =
> SmramMap[DescIdxMain].PhysicalStart;
> -  SmramMap[DescIdxMain].PhysicalSize =
> -    (TsegSizeBits == MCH_ESMRAMC_TSEG_8MB ? SIZE_8MB :
> -     TsegSizeBits == MCH_ESMRAMC_TSEG_2MB ? SIZE_2MB :
> -     SIZE_1MB) - SmramMap[DescIdxSmmS3ResumeState].PhysicalSize;
> -  SmramMap[DescIdxMain].RegionState = CommonRegionState;
> -
> -  return EFI_SUCCESS;
> -}
> diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.h
> b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.h
> deleted file mode 100644
> index 81180a9c8e..0000000000
> --- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmramInternal.h
> +++ /dev/null
> @@ -1,82 +0,0 @@
> -/** @file
> -  Functions and types shared by the SMM accessor PEI and DXE modules.
> -
> -  Copyright (C) 2015, Red Hat, Inc.
> -  Copyright (C) 2019, Intel Corporation. All rights reserved.<BR>
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> -**/
> -
> -#include <Pi/PiMultiPhase.h>
> -
> -//
> -// We'll have two SMRAM ranges.
> -//
> -// The first is a tiny one that hosts an SMM_S3_RESUME_STATE object, to
> be
> -// filled in by the CPU SMM driver during normal boot, for the PEI instance
> of
> -// the LockBox library (which will rely on the object during S3 resume).
> -//
> -// The other SMRAM range is the main one, for the SMM core and the SMM
> drivers.
> -//
> -typedef enum {
> -  DescIdxSmmS3ResumeState = 0,
> -  DescIdxMain             = 1,
> -  DescIdxCount            = 2
> -} DESCRIPTOR_INDEX;
> -
> -/**
> -  Read the MCH_SMRAM and ESMRAMC registers, and update the LockState
> and
> -  OpenState fields in the PEI_SMM_ACCESS_PPI /
> EFI_SMM_ACCESS2_PROTOCOL object,
> -  from the D_LCK and T_EN bits.
> -
> -  PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL member
> functions can rely on
> -  the LockState and OpenState fields being up-to-date on entry, and they
> need
> -  to restore the same invariant on exit, if they touch the bits in question.
> -
> -  @param[out] LockState  Reflects the D_LCK bit on output; TRUE iff SMRAM
> is
> -                         locked.
> -  @param[out] OpenState  Reflects the inverse of the T_EN bit on output;
> TRUE
> -                         iff SMRAM is open.
> -**/
> -VOID
> -GetStates (
> -  OUT BOOLEAN *LockState,
> -  OUT BOOLEAN *OpenState
> -  );
> -
> -//
> -// The functions below follow the PEI_SMM_ACCESS_PPI and
> -// EFI_SMM_ACCESS2_PROTOCOL member declarations. The PeiServices
> and This
> -// pointers are removed (TSEG doesn't depend on them), and so is the
> -// DescriptorIndex parameter (TSEG doesn't support range-wise locking).
> -//
> -// The LockState and OpenState members that are common to both
> -// PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL are taken
> and updated in
> -// isolation from the rest of the (non-shared) members.
> -//
> -
> -EFI_STATUS
> -SmramAccessOpen (
> -  OUT BOOLEAN *LockState,
> -  OUT BOOLEAN *OpenState
> -  );
> -
> -EFI_STATUS
> -SmramAccessClose (
> -  OUT BOOLEAN *LockState,
> -  OUT BOOLEAN *OpenState
> -  );
> -
> -EFI_STATUS
> -SmramAccessLock (
> -  OUT    BOOLEAN *LockState,
> -  IN OUT BOOLEAN *OpenState
> -  );
> -
> -EFI_STATUS
> -SmramAccessGetCapabilities (
> -  IN BOOLEAN                  LockState,
> -  IN BOOLEAN                  OpenState,
> -  IN OUT UINTN                *SmramMapSize,
> -  IN OUT EFI_SMRAM_DESCRIPTOR *SmramMap
> -  );
> --
> 2.31.1.windows.1