1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH 0/5] refine Smm range code in BoardX58Ich10
  4. View patch

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

Zhiguang Liu posted 5 patches 2 years, 9 months ago
There is a newer version of this series
[edk2-devel] [PATCH 1/5] SimicsOpenBoardPkg: Build gEfiSmmSmramMemoryGuid Hob in S3 path
Posted by Zhiguang Liu 2 years, 9 months ago 
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103529): https://edk2.groups.io/g/devel/message/103529
Mute This Topic: https://groups.io/mt/98488191/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/5] SimicsOpenBoardPkg: Build gEfiSmmSmramMemoryGuid Hob in S3 path
Posted by Ni, Ray 2 years, 9 months ago 
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103541): https://edk2.groups.io/g/devel/message/103541
Mute This Topic: https://groups.io/mt/98488191/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.