public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB
@ 2024-01-23 20:24 Michael D Kinney
  2024-01-24 14:17 ` Laszlo Ersek
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Michael D Kinney @ 2024-01-23 20:24 UTC (permalink / raw)
  To: devel; +Cc: Liming Gao, Aaron Li, Liu Yun, Andrew Fish

Provide an optional method for PEI to declare a specific address
range to use for the Memory Type Information bins. The current
algorithm uses heuristics that tends to place the Memory Type
Information bins in the same location, but memory configuration
changes across boots or algorithm changes across a firmware
updates could potentially change the Memory Type Information bin
location.

If the HOB List contains a Resource Descriptor HOB that
describes tested system memory and has an Owner GUID of
gEfiMemoryTypeInformationGuid, then use the address range
described by the Resource Descriptor HOB as the preferred
location of the Memory Type Information bins. If this HOB is
not detected, then the current behavior is preserved.

The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
is ignored for the following conditions:
* The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
  is smaller than the Memory Type Information bins.
* The HOB list contains more than one Resource Descriptor HOB
  with an owner GUID of gEfiMemoryTypeInformationGuid.
* The Resource Descriptor HOB with an Owner GUID of
  gEfiMemoryTypeInformationGuid is the same Resource Descriptor
  HOB that that describes the PHIT memory range.

Update the DxeMain initialization order to initialize GCD
services before any runtime allocations are performed.  This
is required to prevent runtime data fragmentation when the
UEFI System Table and UEFI Runtime Service Table is allocated.

Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Aaron Li <aaron.li@intel.com>
Cc: Liu Yun <yun.y.liu@intel.com>
Cc: Andrew Fish <afish@apple.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 MdeModulePkg/Core/Dxe/DxeMain.h         |   6 ++
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c |  23 +++---
 MdeModulePkg/Core/Dxe/Gcd/Gcd.c         |  72 ++++++++++++++++-
 MdeModulePkg/Core/Dxe/Mem/Page.c        | 101 ++++++++++++++++++++++++
 4 files changed, 190 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index 86a7be2f5188..53e26703f8c7 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -277,6 +277,12 @@ CoreInitializePool (
   VOID
   );
 
+VOID
+CoreSetMemoryTypeInformationRange (
+  IN EFI_PHYSICAL_ADDRESS  Start,
+  IN UINT64                Length
+  );
+
 /**
   Called to initialize the memory map and add descriptors to
   the current descriptor list.
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 0e0f9769b99d..17d510a287e5 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -276,6 +276,18 @@ DxeMain (
 
   MemoryProfileInit (HobStart);
 
+  //
+  // Start the Image Services.
+  //
+  Status = CoreInitializeImageServices (HobStart);
+  ASSERT_EFI_ERROR (Status);
+
+  //
+  // Initialize the Global Coherency Domain Services
+  //
+  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress, MemoryLength);
+  ASSERT_EFI_ERROR (Status);
+
   //
   // Allocate the EFI System Table and EFI Runtime Service Table from EfiRuntimeServicesData
   // Use the templates to initialize the contents of the EFI System Table and EFI Runtime Services Table
@@ -289,16 +301,9 @@ DxeMain (
   gDxeCoreST->RuntimeServices = gDxeCoreRT;
 
   //
-  // Start the Image Services.
+  // Update DXE Core Loaded Image Protocol with allocated UEFI System Table
   //
-  Status = CoreInitializeImageServices (HobStart);
-  ASSERT_EFI_ERROR (Status);
-
-  //
-  // Initialize the Global Coherency Domain Services
-  //
-  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress, MemoryLength);
-  ASSERT_EFI_ERROR (Status);
+  gDxeCoreLoadedImage->SystemTable = gDxeCoreST;
 
   //
   // Call constructor for all libraries
diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
index 792cd2e0af23..c450d1bf2531 100644
--- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
+++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
@@ -2238,6 +2238,8 @@ CoreInitializeMemoryServices (
   EFI_HOB_HANDOFF_INFO_TABLE   *PhitHob;
   EFI_HOB_RESOURCE_DESCRIPTOR  *ResourceHob;
   EFI_HOB_RESOURCE_DESCRIPTOR  *PhitResourceHob;
+  EFI_HOB_RESOURCE_DESCRIPTOR  *MemoryTypeInformationResourceHob;
+  UINTN                        Count;
   EFI_PHYSICAL_ADDRESS         BaseAddress;
   UINT64                       Length;
   UINT64                       Attributes;
@@ -2289,12 +2291,47 @@ CoreInitializeMemoryServices (
   //
   // See if a Memory Type Information HOB is available
   //
-  GuidHob = GetFirstGuidHob (&gEfiMemoryTypeInformationGuid);
+  MemoryTypeInformationResourceHob = NULL;
+  GuidHob                          = GetFirstGuidHob (&gEfiMemoryTypeInformationGuid);
   if (GuidHob != NULL) {
     EfiMemoryTypeInformation = GET_GUID_HOB_DATA (GuidHob);
     DataSize                 = GET_GUID_HOB_DATA_SIZE (GuidHob);
     if ((EfiMemoryTypeInformation != NULL) && (DataSize > 0) && (DataSize <= (EfiMaxMemoryType + 1) * sizeof (EFI_MEMORY_TYPE_INFORMATION))) {
       CopyMem (&gMemoryTypeInformation, EfiMemoryTypeInformation, DataSize);
+
+      //
+      // Look for Resource Descriptor HOB with a ResourceType of System Memory
+      // and an Owner GUID of gEfiMemoryTypeInformationGuid. If more than 1 is
+      // found, then set MemoryTypeInformationResourceHob to NULL.
+      //
+      Count = 0;
+      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB (Hob)) {
+        if (GET_HOB_TYPE (Hob) != EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
+          continue;
+        }
+
+        ResourceHob = Hob.ResourceDescriptor;
+        if (!CompareGuid (&ResourceHob->Owner, &gEfiMemoryTypeInformationGuid)) {
+          continue;
+        }
+
+        Count++;
+        if (ResourceHob->ResourceType != EFI_RESOURCE_SYSTEM_MEMORY) {
+          continue;
+        }
+
+        if ((ResourceHob->ResourceAttribute & MEMORY_ATTRIBUTE_MASK) != TESTED_MEMORY_ATTRIBUTES) {
+          continue;
+        }
+
+        if (ResourceHob->ResourceLength >= CalculateTotalMemoryBinSizeNeeded ()) {
+          MemoryTypeInformationResourceHob = ResourceHob;
+        }
+      }
+
+      if (Count > 1) {
+        MemoryTypeInformationResourceHob = NULL;
+      }
     }
   }
 
@@ -2344,6 +2381,15 @@ CoreInitializeMemoryServices (
     PhitResourceHob = ResourceHob;
     Found           = TRUE;
 
+    //
+    // If a Memory Type Information Resource HOB was found and is the same
+    // Resource HOB that describes the PHIT HOB, then ignore the Memory Type
+    // Information Resource HOB.
+    //
+    if (MemoryTypeInformationResourceHob == PhitResourceHob) {
+      MemoryTypeInformationResourceHob = NULL;
+    }
+
     //
     // Compute range between PHIT EfiMemoryTop and the end of the Resource Descriptor HOB
     //
@@ -2387,8 +2433,9 @@ CoreInitializeMemoryServices (
   if (Length < MinimalMemorySizeNeeded) {
     //
     // Search all the resource descriptor HOBs from the highest possible addresses down for a memory
-    // region that is big enough to initialize the DXE core.  Always skip the PHIT Resource HOB.
-    // The max address must be within the physically addressible range for the processor.
+    // region that is big enough to initialize the DXE core.  Always skip the PHIT Resource HOB
+    // and the Memory Type Information Resource HOB. The max address must be within the physically
+    // addressable range for the processor.
     //
     HighAddress = MAX_ALLOC_ADDRESS;
     for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB (Hob)) {
@@ -2399,6 +2446,13 @@ CoreInitializeMemoryServices (
         continue;
       }
 
+      //
+      // Skip the Resource Descriptor HOB that contains Memory Type Information bins
+      //
+      if (Hob.ResourceDescriptor == MemoryTypeInformationResourceHob) {
+        continue;
+      }
+
       //
       // Skip all HOBs except Resource Descriptor HOBs
       //
@@ -2466,6 +2520,18 @@ CoreInitializeMemoryServices (
     Capabilities = CoreConvertResourceDescriptorHobAttributesToCapabilities (EfiGcdMemoryTypeSystemMemory, Attributes);
   }
 
+  if (MemoryTypeInformationResourceHob != NULL) {
+    //
+    // If a Memory Type Information Resource HOB was found, then use the address
+    // range of the  Memory Type Information Resource HOB as the preferred
+    // address range for the Memory Type Information bins.
+    //
+    CoreSetMemoryTypeInformationRange (
+      MemoryTypeInformationResourceHob->PhysicalStart,
+      MemoryTypeInformationResourceHob->ResourceLength
+      );
+  }
+
   //
   // Declare the very first memory region, so the EFI Memory Services are available.
   //
diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index 6497af573353..458c62090265 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -532,6 +532,107 @@ CoreLoadingFixedAddressHook (
   return;
 }
 
+/**
+  Sets the preferred memory range to use for the Memory Type Information bins.
+  This service must be called before fist call to CoreAddMemoryDescriptor().
+
+  If the location of the Memory Type Information bins has already been
+  established or the size of the range provides is smaller than all the
+  Memory Type Information bins, then the range provides is not used.
+
+  @param  Start   The start address of the Memory Type Information range.
+  @param  Length  The size, in bytes, of the Memory Type Information range.
+**/
+VOID
+CoreSetMemoryTypeInformationRange (
+  IN EFI_PHYSICAL_ADDRESS  Start,
+  IN UINT64                Length
+  )
+{
+  EFI_PHYSICAL_ADDRESS  Top;
+  EFI_MEMORY_TYPE       Type;
+  UINTN                 Index;
+  UINTN                 Size;
+
+  //
+  // Return if Memory Type Information bin locations have already been set
+  //
+  if (mMemoryTypeInformationInitialized) {
+    return;
+  }
+
+  //
+  // Return if size of the Memory Type Information bins is greater than Length
+  //
+  Size = 0;
+  for (Index = 0; gMemoryTypeInformation[Index].Type != EfiMaxMemoryType; Index++) {
+    //
+    // Make sure the memory type in the gMemoryTypeInformation[] array is valid
+    //
+    Type = (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
+    if ((UINT32)Type > EfiMaxMemoryType) {
+      continue;
+    }
+
+    Size += EFI_PAGES_TO_SIZE (gMemoryTypeInformation[Index].NumberOfPages);
+  }
+
+  if (Size > Length) {
+    return;
+  }
+
+  //
+  // Loop through each memory type in the order specified by the
+  // gMemoryTypeInformation[] array
+  //
+  Top = Start + Length;
+  for (Index = 0; gMemoryTypeInformation[Index].Type != EfiMaxMemoryType; Index++) {
+    //
+    // Make sure the memory type in the gMemoryTypeInformation[] array is valid
+    //
+    Type = (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
+    if ((UINT32)Type > EfiMaxMemoryType) {
+      continue;
+    }
+
+    if (gMemoryTypeInformation[Index].NumberOfPages != 0) {
+      mMemoryTypeStatistics[Type].MaximumAddress = Top - 1;
+      Top                                       -= EFI_PAGES_TO_SIZE (gMemoryTypeInformation[Index].NumberOfPages);
+      mMemoryTypeStatistics[Type].BaseAddress    = Top;
+
+      //
+      // If the current base address is the lowest address so far, then update
+      // the default maximum address
+      //
+      if (mMemoryTypeStatistics[Type].BaseAddress < mDefaultMaximumAddress) {
+        mDefaultMaximumAddress = mMemoryTypeStatistics[Type].BaseAddress - 1;
+      }
+
+      mMemoryTypeStatistics[Type].NumberOfPages   = gMemoryTypeInformation[Index].NumberOfPages;
+      gMemoryTypeInformation[Index].NumberOfPages = 0;
+    }
+  }
+
+  //
+  // If the number of pages reserved for a memory type is 0, then all
+  // allocations for that type should be in the default range.
+  //
+  for (Type = (EFI_MEMORY_TYPE)0; Type < EfiMaxMemoryType; Type++) {
+    for (Index = 0; gMemoryTypeInformation[Index].Type != EfiMaxMemoryType; Index++) {
+      if (Type == (EFI_MEMORY_TYPE)gMemoryTypeInformation[Index].Type) {
+        mMemoryTypeStatistics[Type].InformationIndex = Index;
+      }
+    }
+
+    mMemoryTypeStatistics[Type].CurrentNumberOfPages = 0;
+    if (mMemoryTypeStatistics[Type].MaximumAddress == MAX_ALLOC_ADDRESS) {
+      mMemoryTypeStatistics[Type].MaximumAddress = mDefaultMaximumAddress;
+    }
+  }
+
+  mMemoryTypeInformationInitialized = TRUE;
+}
+
 /**
   Called to initialize the memory map and add descriptors to
   the current descriptor list.
-- 
2.40.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114229): https://edk2.groups.io/g/devel/message/114229
Mute This Topic: https://groups.io/mt/103918464/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB
  2024-01-23 20:24 [edk2-devel] [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB Michael D Kinney
@ 2024-01-24 14:17 ` Laszlo Ersek
  2024-01-24 17:39   ` Michael D Kinney
  2024-01-24 14:20 ` Laszlo Ersek
  2024-01-24 14:59 ` [edk2-devel] 回复: " gaoliming via groups.io
  2 siblings, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2024-01-24 14:17 UTC (permalink / raw)
  To: devel, michael.d.kinney; +Cc: Liming Gao, Aaron Li, Liu Yun, Andrew Fish

On 1/23/24 21:24, Michael D Kinney wrote:
> Provide an optional method for PEI to declare a specific address
> range to use for the Memory Type Information bins. The current
> algorithm uses heuristics that tends to place the Memory Type
> Information bins in the same location, but memory configuration
> changes across boots or algorithm changes across a firmware
> updates could potentially change the Memory Type Information bin
> location.
>
> If the HOB List contains a Resource Descriptor HOB that
> describes tested system memory and has an Owner GUID of
> gEfiMemoryTypeInformationGuid, then use the address range
> described by the Resource Descriptor HOB as the preferred
> location of the Memory Type Information bins. If this HOB is
> not detected, then the current behavior is preserved.
>
> The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
> is ignored for the following conditions:
> * The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
>   is smaller than the Memory Type Information bins.
> * The HOB list contains more than one Resource Descriptor HOB
>   with an owner GUID of gEfiMemoryTypeInformationGuid.
> * The Resource Descriptor HOB with an Owner GUID of
>   gEfiMemoryTypeInformationGuid is the same Resource Descriptor
>   HOB that that describes the PHIT memory range.

I feel that, while this GUID usage makes sense, it overloads the already
existing meanings of gEfiMemoryTypeInformationGuid.

We already use this GUID for two things:

(see "MdeModulePkg/Include/Guid/MemoryTypeInformation.h"):

- UEFI variable name space GUID for the "MemoryTypeInformation" variable

- HOB of type EFI_HOB_TYPE_GUID_EXTENSION, controlling the page counts
in the various memory type bins.

Now this new use would introduce a HOB of type
EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, where the Owner field is
gEfiMemoryTypeInformationGuid, and the range described by the
PhysicalStart and ResourceLength fields would accommodate the actual
bins. On one hand this new HOB is certainly related to the prior two
uses, but on the other hand, we'd now have to HOBs that used the "same
GUID" for different purposes. What distinguishes them is
EFI_HOB_TYPE_GUID_EXTENSION vs. EFI_HOB_TYPE_RESOURCE_DESCRIPTOR --
which I find a bit too obscure.

(1) So I'd either suggest generating a brand new GUID, *or* extending the
comments in "MdeModulePkg/Include/Guid/MemoryTypeInformation.h" with the
new usage:

> /** @file
>   This file defines:
>   * Memory Type Information GUID for HOB and Variable.
>   * Memory Type Information Variable Name.
>   * Memory Type Information GUID HOB data structure.
>
>   The memory type information HOB and variable can
>   be used to store the information for each memory type in Variable or HOB.
>
> Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/

Back to the patch:

On 1/23/24 21:24, Michael D Kinney wrote:
> Update the DxeMain initialization order to initialize GCD
> services before any runtime allocations are performed.  This
> is required to prevent runtime data fragmentation when the
> UEFI System Table and UEFI Runtime Service Table is allocated.

(2) Should this be a separate patch? (First patch in the series?) This seems
to effect behavior even if the new HOB is not present.

>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Aaron Li <aaron.li@intel.com>
> Cc: Liu Yun <yun.y.liu@intel.com>
> Cc: Andrew Fish <afish@apple.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  MdeModulePkg/Core/Dxe/DxeMain.h         |   6 ++
>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c |  23 +++---
>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c         |  72 ++++++++++++++++-
>  MdeModulePkg/Core/Dxe/Mem/Page.c        | 101 ++++++++++++++++++++++++
>  4 files changed, 190 insertions(+), 12 deletions(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
> index 86a7be2f5188..53e26703f8c7 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> @@ -277,6 +277,12 @@ CoreInitializePool (
>    VOID
>    );
>
> +VOID
> +CoreSetMemoryTypeInformationRange (
> +  IN EFI_PHYSICAL_ADDRESS  Start,
> +  IN UINT64                Length
> +  );
> +
>  /**
>    Called to initialize the memory map and add descriptors to
>    the current descriptor list.
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> index 0e0f9769b99d..17d510a287e5 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> @@ -276,6 +276,18 @@ DxeMain (
>
>    MemoryProfileInit (HobStart);
>
> +  //
> +  // Start the Image Services.
> +  //
> +  Status = CoreInitializeImageServices (HobStart);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  //
> +  // Initialize the Global Coherency Domain Services
> +  //
> +  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress, MemoryLength);
> +  ASSERT_EFI_ERROR (Status);
> +
>    //
>    // Allocate the EFI System Table and EFI Runtime Service Table from EfiRuntimeServicesData
>    // Use the templates to initialize the contents of the EFI System Table and EFI Runtime Services Table
> @@ -289,16 +301,9 @@ DxeMain (
>    gDxeCoreST->RuntimeServices = gDxeCoreRT;
>
>    //
> -  // Start the Image Services.
> +  // Update DXE Core Loaded Image Protocol with allocated UEFI System Table
>    //
> -  Status = CoreInitializeImageServices (HobStart);
> -  ASSERT_EFI_ERROR (Status);
> -
> -  //
> -  // Initialize the Global Coherency Domain Services
> -  //
> -  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress, MemoryLength);
> -  ASSERT_EFI_ERROR (Status);
> +  gDxeCoreLoadedImage->SystemTable = gDxeCoreST;
>
>    //
>    // Call constructor for all libraries

(3) The separate assignment to "gDxeCoreLoadedImage->SystemTable"
becomes necessary because CoreInitializeImageServices() is hoisted above

  gDxeCoreST = AllocateRuntimeCopyPool (sizeof (EFI_SYSTEM_TABLE), &mEfiSystemTableTemplate);

and therefore the assignment

  Image->Info.SystemTable = gDxeCoreST;

in CoreInitializeImageServices() becomes ineffective -- at that time,
gDxeCoreST is NULL, post-patch.

This double-write (overwrite) to the SystemTable field is confusing. I
propose (again, in said separate, first patch) the removal of the
now-ineffective assignment from CoreInitializeImageServices().


> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> index 792cd2e0af23..c450d1bf2531 100644
> --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> @@ -2238,6 +2238,8 @@ CoreInitializeMemoryServices (
>    EFI_HOB_HANDOFF_INFO_TABLE   *PhitHob;
>    EFI_HOB_RESOURCE_DESCRIPTOR  *ResourceHob;
>    EFI_HOB_RESOURCE_DESCRIPTOR  *PhitResourceHob;
> +  EFI_HOB_RESOURCE_DESCRIPTOR  *MemoryTypeInformationResourceHob;
> +  UINTN                        Count;
>    EFI_PHYSICAL_ADDRESS         BaseAddress;
>    UINT64                       Length;
>    UINT64                       Attributes;
> @@ -2289,12 +2291,47 @@ CoreInitializeMemoryServices (
>    //
>    // See if a Memory Type Information HOB is available
>    //
> -  GuidHob = GetFirstGuidHob (&gEfiMemoryTypeInformationGuid);
> +  MemoryTypeInformationResourceHob = NULL;
> +  GuidHob                          = GetFirstGuidHob (&gEfiMemoryTypeInformationGuid);
>    if (GuidHob != NULL) {
>      EfiMemoryTypeInformation = GET_GUID_HOB_DATA (GuidHob);
>      DataSize                 = GET_GUID_HOB_DATA_SIZE (GuidHob);
>      if ((EfiMemoryTypeInformation != NULL) && (DataSize > 0) && (DataSize <= (EfiMaxMemoryType + 1) * sizeof (EFI_MEMORY_TYPE_INFORMATION))) {
>        CopyMem (&gMemoryTypeInformation, EfiMemoryTypeInformation, DataSize);
> +
> +      //
> +      // Look for Resource Descriptor HOB with a ResourceType of System Memory
> +      // and an Owner GUID of gEfiMemoryTypeInformationGuid. If more than 1 is
> +      // found, then set MemoryTypeInformationResourceHob to NULL.
> +      //
> +      Count = 0;
> +      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB (Hob)) {
> +        if (GET_HOB_TYPE (Hob) != EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
> +          continue;
> +        }
> +
> +        ResourceHob = Hob.ResourceDescriptor;
> +        if (!CompareGuid (&ResourceHob->Owner, &gEfiMemoryTypeInformationGuid)) {
> +          continue;
> +        }
> +
> +        Count++;
> +        if (ResourceHob->ResourceType != EFI_RESOURCE_SYSTEM_MEMORY) {
> +          continue;
> +        }
> +
> +        if ((ResourceHob->ResourceAttribute & MEMORY_ATTRIBUTE_MASK) != TESTED_MEMORY_ATTRIBUTES) {
> +          continue;
> +        }
> +
> +        if (ResourceHob->ResourceLength >= CalculateTotalMemoryBinSizeNeeded ()) {
> +          MemoryTypeInformationResourceHob = ResourceHob;
> +        }
> +      }
> +
> +      if (Count > 1) {
> +        MemoryTypeInformationResourceHob = NULL;
> +      }
>      }
>    }
>
> @@ -2344,6 +2381,15 @@ CoreInitializeMemoryServices (
>      PhitResourceHob = ResourceHob;
>      Found           = TRUE;
>
> +    //
> +    // If a Memory Type Information Resource HOB was found and is the same
> +    // Resource HOB that describes the PHIT HOB, then ignore the Memory Type
> +    // Information Resource HOB.
> +    //
> +    if (MemoryTypeInformationResourceHob == PhitResourceHob) {
> +      MemoryTypeInformationResourceHob = NULL;
> +    }
> +
>      //
>      // Compute range between PHIT EfiMemoryTop and the end of the Resource Descriptor HOB
>      //
> @@ -2387,8 +2433,9 @@ CoreInitializeMemoryServices (
>    if (Length < MinimalMemorySizeNeeded) {
>      //
>      // Search all the resource descriptor HOBs from the highest possible addresses down for a memory
> -    // region that is big enough to initialize the DXE core.  Always skip the PHIT Resource HOB.
> -    // The max address must be within the physically addressible range for the processor.
> +    // region that is big enough to initialize the DXE core.  Always skip the PHIT Resource HOB
> +    // and the Memory Type Information Resource HOB. The max address must be within the physically
> +    // addressable range for the processor.
>      //
>      HighAddress = MAX_ALLOC_ADDRESS;
>      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB (Hob)) {
> @@ -2399,6 +2446,13 @@ CoreInitializeMemoryServices (
>          continue;
>        }
>
> +      //
> +      // Skip the Resource Descriptor HOB that contains Memory Type Information bins
> +      //
> +      if (Hob.ResourceDescriptor == MemoryTypeInformationResourceHob) {
> +        continue;
> +      }
> +
>        //
>        // Skip all HOBs except Resource Descriptor HOBs
>        //
> @@ -2466,6 +2520,18 @@ CoreInitializeMemoryServices (
>      Capabilities = CoreConvertResourceDescriptorHobAttributesToCapabilities (EfiGcdMemoryTypeSystemMemory, Attributes);
>    }
>
> +  if (MemoryTypeInformationResourceHob != NULL) {
> +    //
> +    // If a Memory Type Information Resource HOB was found, then use the address
> +    // range of the  Memory Type Information Resource HOB as the preferred
> +    // address range for the Memory Type Information bins.
> +    //
> +    CoreSetMemoryTypeInformationRange (
> +      MemoryTypeInformationResourceHob->PhysicalStart,
> +      MemoryTypeInformationResourceHob->ResourceLength
> +      );
> +  }
> +
>    //
>    // Declare the very first memory region, so the EFI Memory Services are available.
>    //

Hmm, I think I have a coarse understanding of the control flow here. If
we call CoreSetMemoryTypeInformationRange() here -- due to the new HOB
being available --, then "mMemoryTypeInformationInitialized" will be set
to TRUE. Then, the very first call to CoreAddMemoryDescriptor(), just
below the context quoted above, will see
"mMemoryTypeInformationInitialized" as TRUE, and avoid iterating through
the memory type information array. OK.

> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
> index 6497af573353..458c62090265 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> @@ -532,6 +532,107 @@ CoreLoadingFixedAddressHook (
>    return;
>  }
>
> +/**
> +  Sets the preferred memory range to use for the Memory Type Information bins.
> +  This service must be called before fist call to CoreAddMemoryDescriptor().
> +
> +  If the location of the Memory Type Information bins has already been
> +  established or the size of the range provides is smaller than all the
> +  Memory Type Information bins, then the range provides is not used.
> +
> +  @param  Start   The start address of the Memory Type Information range.
> +  @param  Length  The size, in bytes, of the Memory Type Information range.
> +**/
> +VOID
> +CoreSetMemoryTypeInformationRange (
> +  IN EFI_PHYSICAL_ADDRESS  Start,
> +  IN UINT64                Length
> +  )
> +{
> +  EFI_PHYSICAL_ADDRESS  Top;
> +  EFI_MEMORY_TYPE       Type;
> +  UINTN                 Index;
> +  UINTN                 Size;
> +
> +  //
> +  // Return if Memory Type Information bin locations have already been set
> +  //
> +  if (mMemoryTypeInformationInitialized) {
> +    return;
> +  }

(4) Under what circumstances can this condition evaluate to TRUE?

Should we perhaps

  ASSERT (!mMemoryTypeInformationInitialized);

instead?

> +
> +  //
> +  // Return if size of the Memory Type Information bins is greater than Length
> +  //
> +  Size = 0;
> +  for (Index = 0; gMemoryTypeInformation[Index].Type != EfiMaxMemoryType; Index++) {
> +    //
> +    // Make sure the memory type in the gMemoryTypeInformation[] array is valid
> +    //
> +    Type = (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> +    if ((UINT32)Type > EfiMaxMemoryType) {
> +      continue;
> +    }
> +
> +    Size += EFI_PAGES_TO_SIZE (gMemoryTypeInformation[Index].NumberOfPages);
> +  }
> +
> +  if (Size > Length) {
> +    return;
> +  }
> +
> +  //
> +  // Loop through each memory type in the order specified by the
> +  // gMemoryTypeInformation[] array
> +  //
> +  Top = Start + Length;
> +  for (Index = 0; gMemoryTypeInformation[Index].Type != EfiMaxMemoryType; Index++) {
> +    //
> +    // Make sure the memory type in the gMemoryTypeInformation[] array is valid
> +    //
> +    Type = (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> +    if ((UINT32)Type > EfiMaxMemoryType) {
> +      continue;
> +    }
> +
> +    if (gMemoryTypeInformation[Index].NumberOfPages != 0) {
> +      mMemoryTypeStatistics[Type].MaximumAddress = Top - 1;
> +      Top                                       -= EFI_PAGES_TO_SIZE (gMemoryTypeInformation[Index].NumberOfPages);
> +      mMemoryTypeStatistics[Type].BaseAddress    = Top;
> +
> +      //
> +      // If the current base address is the lowest address so far, then update
> +      // the default maximum address
> +      //
> +      if (mMemoryTypeStatistics[Type].BaseAddress < mDefaultMaximumAddress) {
> +        mDefaultMaximumAddress = mMemoryTypeStatistics[Type].BaseAddress - 1;
> +      }
> +
> +      mMemoryTypeStatistics[Type].NumberOfPages   = gMemoryTypeInformation[Index].NumberOfPages;
> +      gMemoryTypeInformation[Index].NumberOfPages = 0;
> +    }
> +  }
> +
> +  //
> +  // If the number of pages reserved for a memory type is 0, then all
> +  // allocations for that type should be in the default range.
> +  //
> +  for (Type = (EFI_MEMORY_TYPE)0; Type < EfiMaxMemoryType; Type++) {
> +    for (Index = 0; gMemoryTypeInformation[Index].Type != EfiMaxMemoryType; Index++) {
> +      if (Type == (EFI_MEMORY_TYPE)gMemoryTypeInformation[Index].Type) {
> +        mMemoryTypeStatistics[Type].InformationIndex = Index;
> +      }
> +    }
> +
> +    mMemoryTypeStatistics[Type].CurrentNumberOfPages = 0;
> +    if (mMemoryTypeStatistics[Type].MaximumAddress == MAX_ALLOC_ADDRESS) {
> +      mMemoryTypeStatistics[Type].MaximumAddress = mDefaultMaximumAddress;
> +    }
> +  }
> +
> +  mMemoryTypeInformationInitialized = TRUE;
> +}
> +
>  /**
>    Called to initialize the memory map and add descriptors to
>    the current descriptor list.

This part resembles (and seems to substitute for) the tail of said very
first CoreAddMemoryDescriptor() execution. There are some differences;
here we don't rely on CoreAllocatePages(). I don't understand either
piece of code enough for making a meaningful comparison. The general
control flow separation seems sane, modulo my question (4) above.

Thanks,
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114300): https://edk2.groups.io/g/devel/message/114300
Mute This Topic: https://groups.io/mt/103918464/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB
  2024-01-23 20:24 [edk2-devel] [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB Michael D Kinney
  2024-01-24 14:17 ` Laszlo Ersek
@ 2024-01-24 14:20 ` Laszlo Ersek
  2024-01-24 17:46   ` Michael D Kinney
  2024-01-24 14:59 ` [edk2-devel] 回复: " gaoliming via groups.io
  2 siblings, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2024-01-24 14:20 UTC (permalink / raw)
  To: devel, michael.d.kinney; +Cc: Liming Gao, Aaron Li, Liu Yun, Andrew Fish

On 1/23/24 21:24, Michael D Kinney wrote:
> Provide an optional method for PEI to declare a specific address
> range to use for the Memory Type Information bins. The current
> algorithm uses heuristics that tends to place the Memory Type
> Information bins in the same location, but memory configuration
> changes across boots or algorithm changes across a firmware
> updates could potentially change the Memory Type Information bin
> location.

(5) Why is such a rearrangement of the bins -- which is likely visible
in the UEFI memory map, too -- a problem?

Can we include a hint on that in the commit message?

I understand why it would be a problem across ACPI S4, but a memory
config change (DIMM addition/removal?), or a firmware update, seems to
require a full S5 power cycle.

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114301): https://edk2.groups.io/g/devel/message/114301
Mute This Topic: https://groups.io/mt/103918464/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* [edk2-devel] 回复: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB
  2024-01-23 20:24 [edk2-devel] [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB Michael D Kinney
  2024-01-24 14:17 ` Laszlo Ersek
  2024-01-24 14:20 ` Laszlo Ersek
@ 2024-01-24 14:59 ` gaoliming via groups.io
  2024-01-24 16:16   ` Laszlo Ersek
  2 siblings, 1 reply; 12+ messages in thread
From: gaoliming via groups.io @ 2024-01-24 14:59 UTC (permalink / raw)
  To: 'Michael D Kinney', devel
  Cc: 'Aaron Li', 'Liu Yun', 'Andrew Fish'

Mike:
 Current algorithm tries to reserve the top available memory for memory type
bin range. 
 If the platform uses new way to describe the memory range, should we
suggest the rule to still 
 use the top memory resource hob for the memory type bin range?

Thanks
Liming
> -----邮件原件-----
> 发件人: Michael D Kinney <michael.d.kinney@intel.com>
> 发送时间: 2024年1月24日 4:24
> 收件人: devel@edk2.groups.io
> 抄送: Liming Gao <gaoliming@byosoft.com.cn>; Aaron Li
> <aaron.li@intel.com>; Liu Yun <yun.y.liu@intel.com>; Andrew Fish
> <afish@apple.com>
> 主题: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin
> range from HOB
> 
> Provide an optional method for PEI to declare a specific address
> range to use for the Memory Type Information bins. The current
> algorithm uses heuristics that tends to place the Memory Type
> Information bins in the same location, but memory configuration
> changes across boots or algorithm changes across a firmware
> updates could potentially change the Memory Type Information bin
> location.
> 
> If the HOB List contains a Resource Descriptor HOB that
> describes tested system memory and has an Owner GUID of
> gEfiMemoryTypeInformationGuid, then use the address range
> described by the Resource Descriptor HOB as the preferred
> location of the Memory Type Information bins. If this HOB is
> not detected, then the current behavior is preserved.
> 
> The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
> is ignored for the following conditions:
> * The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
>   is smaller than the Memory Type Information bins.
> * The HOB list contains more than one Resource Descriptor HOB
>   with an owner GUID of gEfiMemoryTypeInformationGuid.
> * The Resource Descriptor HOB with an Owner GUID of
>   gEfiMemoryTypeInformationGuid is the same Resource Descriptor
>   HOB that that describes the PHIT memory range.
> 
> Update the DxeMain initialization order to initialize GCD
> services before any runtime allocations are performed.  This
> is required to prevent runtime data fragmentation when the
> UEFI System Table and UEFI Runtime Service Table is allocated.
> 
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Aaron Li <aaron.li@intel.com>
> Cc: Liu Yun <yun.y.liu@intel.com>
> Cc: Andrew Fish <afish@apple.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  MdeModulePkg/Core/Dxe/DxeMain.h         |   6 ++
>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c |  23 +++---
>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c         |  72 ++++++++++++++++-
>  MdeModulePkg/Core/Dxe/Mem/Page.c        | 101
> ++++++++++++++++++++++++
>  4 files changed, 190 insertions(+), 12 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
> b/MdeModulePkg/Core/Dxe/DxeMain.h
> index 86a7be2f5188..53e26703f8c7 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> @@ -277,6 +277,12 @@ CoreInitializePool (
>    VOID
>    );
> 
> +VOID
> +CoreSetMemoryTypeInformationRange (
> +  IN EFI_PHYSICAL_ADDRESS  Start,
> +  IN UINT64                Length
> +  );
> +
>  /**
>    Called to initialize the memory map and add descriptors to
>    the current descriptor list.
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> index 0e0f9769b99d..17d510a287e5 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> @@ -276,6 +276,18 @@ DxeMain (
> 
>    MemoryProfileInit (HobStart);
> 
> +  //
> +  // Start the Image Services.
> +  //
> +  Status = CoreInitializeImageServices (HobStart);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  //
> +  // Initialize the Global Coherency Domain Services
> +  //
> +  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress,
> MemoryLength);
> +  ASSERT_EFI_ERROR (Status);
> +
>    //
>    // Allocate the EFI System Table and EFI Runtime Service Table from
> EfiRuntimeServicesData
>    // Use the templates to initialize the contents of the EFI System Table
and
> EFI Runtime Services Table
> @@ -289,16 +301,9 @@ DxeMain (
>    gDxeCoreST->RuntimeServices = gDxeCoreRT;
> 
>    //
> -  // Start the Image Services.
> +  // Update DXE Core Loaded Image Protocol with allocated UEFI System
> Table
>    //
> -  Status = CoreInitializeImageServices (HobStart);
> -  ASSERT_EFI_ERROR (Status);
> -
> -  //
> -  // Initialize the Global Coherency Domain Services
> -  //
> -  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress,
> MemoryLength);
> -  ASSERT_EFI_ERROR (Status);
> +  gDxeCoreLoadedImage->SystemTable = gDxeCoreST;
> 
>    //
>    // Call constructor for all libraries
> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> index 792cd2e0af23..c450d1bf2531 100644
> --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> @@ -2238,6 +2238,8 @@ CoreInitializeMemoryServices (
>    EFI_HOB_HANDOFF_INFO_TABLE   *PhitHob;
>    EFI_HOB_RESOURCE_DESCRIPTOR  *ResourceHob;
>    EFI_HOB_RESOURCE_DESCRIPTOR  *PhitResourceHob;
> +  EFI_HOB_RESOURCE_DESCRIPTOR
> *MemoryTypeInformationResourceHob;
> +  UINTN                        Count;
>    EFI_PHYSICAL_ADDRESS         BaseAddress;
>    UINT64                       Length;
>    UINT64                       Attributes;
> @@ -2289,12 +2291,47 @@ CoreInitializeMemoryServices (
>    //
>    // See if a Memory Type Information HOB is available
>    //
> -  GuidHob = GetFirstGuidHob (&gEfiMemoryTypeInformationGuid);
> +  MemoryTypeInformationResourceHob = NULL;
> +  GuidHob                          = GetFirstGuidHob
> (&gEfiMemoryTypeInformationGuid);
>    if (GuidHob != NULL) {
>      EfiMemoryTypeInformation = GET_GUID_HOB_DATA (GuidHob);
>      DataSize                 = GET_GUID_HOB_DATA_SIZE (GuidHob);
>      if ((EfiMemoryTypeInformation != NULL) && (DataSize > 0) && (DataSize
> <= (EfiMaxMemoryType + 1) * sizeof (EFI_MEMORY_TYPE_INFORMATION))) {
>        CopyMem (&gMemoryTypeInformation, EfiMemoryTypeInformation,
> DataSize);
> +
> +      //
> +      // Look for Resource Descriptor HOB with a ResourceType of System
> Memory
> +      // and an Owner GUID of gEfiMemoryTypeInformationGuid. If more
> than 1 is
> +      // found, then set MemoryTypeInformationResourceHob to NULL.
> +      //
> +      Count = 0;
> +      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw =
> GET_NEXT_HOB (Hob)) {
> +        if (GET_HOB_TYPE (Hob) !=
> EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
> +          continue;
> +        }
> +
> +        ResourceHob = Hob.ResourceDescriptor;
> +        if (!CompareGuid (&ResourceHob->Owner,
> &gEfiMemoryTypeInformationGuid)) {
> +          continue;
> +        }
> +
> +        Count++;
> +        if (ResourceHob->ResourceType !=
> EFI_RESOURCE_SYSTEM_MEMORY) {
> +          continue;
> +        }
> +
> +        if ((ResourceHob->ResourceAttribute &
> MEMORY_ATTRIBUTE_MASK) != TESTED_MEMORY_ATTRIBUTES) {
> +          continue;
> +        }
> +
> +        if (ResourceHob->ResourceLength >=
> CalculateTotalMemoryBinSizeNeeded ()) {
> +          MemoryTypeInformationResourceHob = ResourceHob;
> +        }
> +      }
> +
> +      if (Count > 1) {
> +        MemoryTypeInformationResourceHob = NULL;
> +      }
>      }
>    }
> 
> @@ -2344,6 +2381,15 @@ CoreInitializeMemoryServices (
>      PhitResourceHob = ResourceHob;
>      Found           = TRUE;
> 
> +    //
> +    // If a Memory Type Information Resource HOB was found and is the
> same
> +    // Resource HOB that describes the PHIT HOB, then ignore the Memory
> Type
> +    // Information Resource HOB.
> +    //
> +    if (MemoryTypeInformationResourceHob == PhitResourceHob) {
> +      MemoryTypeInformationResourceHob = NULL;
> +    }
> +
>      //
>      // Compute range between PHIT EfiMemoryTop and the end of the
> Resource Descriptor HOB
>      //
> @@ -2387,8 +2433,9 @@ CoreInitializeMemoryServices (
>    if (Length < MinimalMemorySizeNeeded) {
>      //
>      // Search all the resource descriptor HOBs from the highest possible
> addresses down for a memory
> -    // region that is big enough to initialize the DXE core.  Always skip
the
> PHIT Resource HOB.
> -    // The max address must be within the physically addressible range
for
> the processor.
> +    // region that is big enough to initialize the DXE core.  Always skip
the
> PHIT Resource HOB
> +    // and the Memory Type Information Resource HOB. The max address
> must be within the physically
> +    // addressable range for the processor.
>      //
>      HighAddress = MAX_ALLOC_ADDRESS;
>      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw =
> GET_NEXT_HOB (Hob)) {
> @@ -2399,6 +2446,13 @@ CoreInitializeMemoryServices (
>          continue;
>        }
> 
> +      //
> +      // Skip the Resource Descriptor HOB that contains Memory Type
> Information bins
> +      //
> +      if (Hob.ResourceDescriptor == MemoryTypeInformationResourceHob)
> {
> +        continue;
> +      }
> +
>        //
>        // Skip all HOBs except Resource Descriptor HOBs
>        //
> @@ -2466,6 +2520,18 @@ CoreInitializeMemoryServices (
>      Capabilities =
> CoreConvertResourceDescriptorHobAttributesToCapabilities
> (EfiGcdMemoryTypeSystemMemory, Attributes);
>    }
> 
> +  if (MemoryTypeInformationResourceHob != NULL) {
> +    //
> +    // If a Memory Type Information Resource HOB was found, then use the
> address
> +    // range of the  Memory Type Information Resource HOB as the
> preferred
> +    // address range for the Memory Type Information bins.
> +    //
> +    CoreSetMemoryTypeInformationRange (
> +      MemoryTypeInformationResourceHob->PhysicalStart,
> +      MemoryTypeInformationResourceHob->ResourceLength
> +      );
> +  }
> +
>    //
>    // Declare the very first memory region, so the EFI Memory Services are
> available.
>    //
> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> index 6497af573353..458c62090265 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> @@ -532,6 +532,107 @@ CoreLoadingFixedAddressHook (
>    return;
>  }
> 
> +/**
> +  Sets the preferred memory range to use for the Memory Type Information
> bins.
> +  This service must be called before fist call to
CoreAddMemoryDescriptor().
> +
> +  If the location of the Memory Type Information bins has already been
> +  established or the size of the range provides is smaller than all the
> +  Memory Type Information bins, then the range provides is not used.
> +
> +  @param  Start   The start address of the Memory Type Information
> range.
> +  @param  Length  The size, in bytes, of the Memory Type Information
> range.
> +**/
> +VOID
> +CoreSetMemoryTypeInformationRange (
> +  IN EFI_PHYSICAL_ADDRESS  Start,
> +  IN UINT64                Length
> +  )
> +{
> +  EFI_PHYSICAL_ADDRESS  Top;
> +  EFI_MEMORY_TYPE       Type;
> +  UINTN                 Index;
> +  UINTN                 Size;
> +
> +  //
> +  // Return if Memory Type Information bin locations have already been
set
> +  //
> +  if (mMemoryTypeInformationInitialized) {
> +    return;
> +  }
> +
> +  //
> +  // Return if size of the Memory Type Information bins is greater than
> Length
> +  //
> +  Size = 0;
> +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
> EfiMaxMemoryType; Index++) {
> +    //
> +    // Make sure the memory type in the gMemoryTypeInformation[] array
> is valid
> +    //
> +    Type = (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> +    if ((UINT32)Type > EfiMaxMemoryType) {
> +      continue;
> +    }
> +
> +    Size += EFI_PAGES_TO_SIZE
> (gMemoryTypeInformation[Index].NumberOfPages);
> +  }
> +
> +  if (Size > Length) {
> +    return;
> +  }
> +
> +  //
> +  // Loop through each memory type in the order specified by the
> +  // gMemoryTypeInformation[] array
> +  //
> +  Top = Start + Length;
> +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
> EfiMaxMemoryType; Index++) {
> +    //
> +    // Make sure the memory type in the gMemoryTypeInformation[] array
> is valid
> +    //
> +    Type = (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> +    if ((UINT32)Type > EfiMaxMemoryType) {
> +      continue;
> +    }
> +
> +    if (gMemoryTypeInformation[Index].NumberOfPages != 0) {
> +      mMemoryTypeStatistics[Type].MaximumAddress = Top - 1;
> +      Top                                       -=
> EFI_PAGES_TO_SIZE (gMemoryTypeInformation[Index].NumberOfPages);
> +      mMemoryTypeStatistics[Type].BaseAddress    = Top;
> +
> +      //
> +      // If the current base address is the lowest address so far, then
> update
> +      // the default maximum address
> +      //
> +      if (mMemoryTypeStatistics[Type].BaseAddress <
> mDefaultMaximumAddress) {
> +        mDefaultMaximumAddress =
> mMemoryTypeStatistics[Type].BaseAddress - 1;
> +      }
> +
> +      mMemoryTypeStatistics[Type].NumberOfPages   =
> gMemoryTypeInformation[Index].NumberOfPages;
> +      gMemoryTypeInformation[Index].NumberOfPages = 0;
> +    }
> +  }
> +
> +  //
> +  // If the number of pages reserved for a memory type is 0, then all
> +  // allocations for that type should be in the default range.
> +  //
> +  for (Type = (EFI_MEMORY_TYPE)0; Type < EfiMaxMemoryType; Type++) {
> +    for (Index = 0; gMemoryTypeInformation[Index].Type !=
> EfiMaxMemoryType; Index++) {
> +      if (Type ==
> (EFI_MEMORY_TYPE)gMemoryTypeInformation[Index].Type) {
> +        mMemoryTypeStatistics[Type].InformationIndex = Index;
> +      }
> +    }
> +
> +    mMemoryTypeStatistics[Type].CurrentNumberOfPages = 0;
> +    if (mMemoryTypeStatistics[Type].MaximumAddress ==
> MAX_ALLOC_ADDRESS) {
> +      mMemoryTypeStatistics[Type].MaximumAddress =
> mDefaultMaximumAddress;
> +    }
> +  }
> +
> +  mMemoryTypeInformationInitialized = TRUE;
> +}
> +
>  /**
>    Called to initialize the memory map and add descriptors to
>    the current descriptor list.
> --
> 2.40.1.windows.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114308): https://edk2.groups.io/g/devel/message/114308
Mute This Topic: https://groups.io/mt/103933112/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] 回复: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB
  2024-01-24 14:59 ` [edk2-devel] 回复: " gaoliming via groups.io
@ 2024-01-24 16:16   ` Laszlo Ersek
  2024-01-24 17:52     ` Michael D Kinney
  0 siblings, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2024-01-24 16:16 UTC (permalink / raw)
  To: devel, gaoliming, 'Michael D Kinney'
  Cc: 'Aaron Li', 'Liu Yun', 'Andrew Fish'

On 1/24/24 15:59, gaoliming via groups.io wrote:
> Mike:
>  Current algorithm tries to reserve the top available memory for memory type
> bin range. 
>  If the platform uses new way to describe the memory range, should we
> suggest the rule to still 
>  use the top memory resource hob for the memory type bin range?

How would that work, technically? If the platform specifies a concrete
address range, how can that be reconciled with using the top of RAM?

Or do you mean documentation? I.e., a note for platform implementors
that they should point their new HOB to the top of RAM?

Thanks
Laszlo

> 
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: Michael D Kinney <michael.d.kinney@intel.com>
>> 发送时间: 2024年1月24日 4:24
>> 收件人: devel@edk2.groups.io
>> 抄送: Liming Gao <gaoliming@byosoft.com.cn>; Aaron Li
>> <aaron.li@intel.com>; Liu Yun <yun.y.liu@intel.com>; Andrew Fish
>> <afish@apple.com>
>> 主题: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin
>> range from HOB
>>
>> Provide an optional method for PEI to declare a specific address
>> range to use for the Memory Type Information bins. The current
>> algorithm uses heuristics that tends to place the Memory Type
>> Information bins in the same location, but memory configuration
>> changes across boots or algorithm changes across a firmware
>> updates could potentially change the Memory Type Information bin
>> location.
>>
>> If the HOB List contains a Resource Descriptor HOB that
>> describes tested system memory and has an Owner GUID of
>> gEfiMemoryTypeInformationGuid, then use the address range
>> described by the Resource Descriptor HOB as the preferred
>> location of the Memory Type Information bins. If this HOB is
>> not detected, then the current behavior is preserved.
>>
>> The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
>> is ignored for the following conditions:
>> * The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
>>   is smaller than the Memory Type Information bins.
>> * The HOB list contains more than one Resource Descriptor HOB
>>   with an owner GUID of gEfiMemoryTypeInformationGuid.
>> * The Resource Descriptor HOB with an Owner GUID of
>>   gEfiMemoryTypeInformationGuid is the same Resource Descriptor
>>   HOB that that describes the PHIT memory range.
>>
>> Update the DxeMain initialization order to initialize GCD
>> services before any runtime allocations are performed.  This
>> is required to prevent runtime data fragmentation when the
>> UEFI System Table and UEFI Runtime Service Table is allocated.
>>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Aaron Li <aaron.li@intel.com>
>> Cc: Liu Yun <yun.y.liu@intel.com>
>> Cc: Andrew Fish <afish@apple.com>
>> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
>> ---
>>  MdeModulePkg/Core/Dxe/DxeMain.h         |   6 ++
>>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c |  23 +++---
>>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c         |  72 ++++++++++++++++-
>>  MdeModulePkg/Core/Dxe/Mem/Page.c        | 101
>> ++++++++++++++++++++++++
>>  4 files changed, 190 insertions(+), 12 deletions(-)
>>
>> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
>> b/MdeModulePkg/Core/Dxe/DxeMain.h
>> index 86a7be2f5188..53e26703f8c7 100644
>> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
>> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
>> @@ -277,6 +277,12 @@ CoreInitializePool (
>>    VOID
>>    );
>>
>> +VOID
>> +CoreSetMemoryTypeInformationRange (
>> +  IN EFI_PHYSICAL_ADDRESS  Start,
>> +  IN UINT64                Length
>> +  );
>> +
>>  /**
>>    Called to initialize the memory map and add descriptors to
>>    the current descriptor list.
>> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
>> b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
>> index 0e0f9769b99d..17d510a287e5 100644
>> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
>> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
>> @@ -276,6 +276,18 @@ DxeMain (
>>
>>    MemoryProfileInit (HobStart);
>>
>> +  //
>> +  // Start the Image Services.
>> +  //
>> +  Status = CoreInitializeImageServices (HobStart);
>> +  ASSERT_EFI_ERROR (Status);
>> +
>> +  //
>> +  // Initialize the Global Coherency Domain Services
>> +  //
>> +  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress,
>> MemoryLength);
>> +  ASSERT_EFI_ERROR (Status);
>> +
>>    //
>>    // Allocate the EFI System Table and EFI Runtime Service Table from
>> EfiRuntimeServicesData
>>    // Use the templates to initialize the contents of the EFI System Table
> and
>> EFI Runtime Services Table
>> @@ -289,16 +301,9 @@ DxeMain (
>>    gDxeCoreST->RuntimeServices = gDxeCoreRT;
>>
>>    //
>> -  // Start the Image Services.
>> +  // Update DXE Core Loaded Image Protocol with allocated UEFI System
>> Table
>>    //
>> -  Status = CoreInitializeImageServices (HobStart);
>> -  ASSERT_EFI_ERROR (Status);
>> -
>> -  //
>> -  // Initialize the Global Coherency Domain Services
>> -  //
>> -  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress,
>> MemoryLength);
>> -  ASSERT_EFI_ERROR (Status);
>> +  gDxeCoreLoadedImage->SystemTable = gDxeCoreST;
>>
>>    //
>>    // Call constructor for all libraries
>> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
>> b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
>> index 792cd2e0af23..c450d1bf2531 100644
>> --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
>> +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
>> @@ -2238,6 +2238,8 @@ CoreInitializeMemoryServices (
>>    EFI_HOB_HANDOFF_INFO_TABLE   *PhitHob;
>>    EFI_HOB_RESOURCE_DESCRIPTOR  *ResourceHob;
>>    EFI_HOB_RESOURCE_DESCRIPTOR  *PhitResourceHob;
>> +  EFI_HOB_RESOURCE_DESCRIPTOR
>> *MemoryTypeInformationResourceHob;
>> +  UINTN                        Count;
>>    EFI_PHYSICAL_ADDRESS         BaseAddress;
>>    UINT64                       Length;
>>    UINT64                       Attributes;
>> @@ -2289,12 +2291,47 @@ CoreInitializeMemoryServices (
>>    //
>>    // See if a Memory Type Information HOB is available
>>    //
>> -  GuidHob = GetFirstGuidHob (&gEfiMemoryTypeInformationGuid);
>> +  MemoryTypeInformationResourceHob = NULL;
>> +  GuidHob                          = GetFirstGuidHob
>> (&gEfiMemoryTypeInformationGuid);
>>    if (GuidHob != NULL) {
>>      EfiMemoryTypeInformation = GET_GUID_HOB_DATA (GuidHob);
>>      DataSize                 = GET_GUID_HOB_DATA_SIZE (GuidHob);
>>      if ((EfiMemoryTypeInformation != NULL) && (DataSize > 0) && (DataSize
>> <= (EfiMaxMemoryType + 1) * sizeof (EFI_MEMORY_TYPE_INFORMATION))) {
>>        CopyMem (&gMemoryTypeInformation, EfiMemoryTypeInformation,
>> DataSize);
>> +
>> +      //
>> +      // Look for Resource Descriptor HOB with a ResourceType of System
>> Memory
>> +      // and an Owner GUID of gEfiMemoryTypeInformationGuid. If more
>> than 1 is
>> +      // found, then set MemoryTypeInformationResourceHob to NULL.
>> +      //
>> +      Count = 0;
>> +      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw =
>> GET_NEXT_HOB (Hob)) {
>> +        if (GET_HOB_TYPE (Hob) !=
>> EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
>> +          continue;
>> +        }
>> +
>> +        ResourceHob = Hob.ResourceDescriptor;
>> +        if (!CompareGuid (&ResourceHob->Owner,
>> &gEfiMemoryTypeInformationGuid)) {
>> +          continue;
>> +        }
>> +
>> +        Count++;
>> +        if (ResourceHob->ResourceType !=
>> EFI_RESOURCE_SYSTEM_MEMORY) {
>> +          continue;
>> +        }
>> +
>> +        if ((ResourceHob->ResourceAttribute &
>> MEMORY_ATTRIBUTE_MASK) != TESTED_MEMORY_ATTRIBUTES) {
>> +          continue;
>> +        }
>> +
>> +        if (ResourceHob->ResourceLength >=
>> CalculateTotalMemoryBinSizeNeeded ()) {
>> +          MemoryTypeInformationResourceHob = ResourceHob;
>> +        }
>> +      }
>> +
>> +      if (Count > 1) {
>> +        MemoryTypeInformationResourceHob = NULL;
>> +      }
>>      }
>>    }
>>
>> @@ -2344,6 +2381,15 @@ CoreInitializeMemoryServices (
>>      PhitResourceHob = ResourceHob;
>>      Found           = TRUE;
>>
>> +    //
>> +    // If a Memory Type Information Resource HOB was found and is the
>> same
>> +    // Resource HOB that describes the PHIT HOB, then ignore the Memory
>> Type
>> +    // Information Resource HOB.
>> +    //
>> +    if (MemoryTypeInformationResourceHob == PhitResourceHob) {
>> +      MemoryTypeInformationResourceHob = NULL;
>> +    }
>> +
>>      //
>>      // Compute range between PHIT EfiMemoryTop and the end of the
>> Resource Descriptor HOB
>>      //
>> @@ -2387,8 +2433,9 @@ CoreInitializeMemoryServices (
>>    if (Length < MinimalMemorySizeNeeded) {
>>      //
>>      // Search all the resource descriptor HOBs from the highest possible
>> addresses down for a memory
>> -    // region that is big enough to initialize the DXE core.  Always skip
> the
>> PHIT Resource HOB.
>> -    // The max address must be within the physically addressible range
> for
>> the processor.
>> +    // region that is big enough to initialize the DXE core.  Always skip
> the
>> PHIT Resource HOB
>> +    // and the Memory Type Information Resource HOB. The max address
>> must be within the physically
>> +    // addressable range for the processor.
>>      //
>>      HighAddress = MAX_ALLOC_ADDRESS;
>>      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw =
>> GET_NEXT_HOB (Hob)) {
>> @@ -2399,6 +2446,13 @@ CoreInitializeMemoryServices (
>>          continue;
>>        }
>>
>> +      //
>> +      // Skip the Resource Descriptor HOB that contains Memory Type
>> Information bins
>> +      //
>> +      if (Hob.ResourceDescriptor == MemoryTypeInformationResourceHob)
>> {
>> +        continue;
>> +      }
>> +
>>        //
>>        // Skip all HOBs except Resource Descriptor HOBs
>>        //
>> @@ -2466,6 +2520,18 @@ CoreInitializeMemoryServices (
>>      Capabilities =
>> CoreConvertResourceDescriptorHobAttributesToCapabilities
>> (EfiGcdMemoryTypeSystemMemory, Attributes);
>>    }
>>
>> +  if (MemoryTypeInformationResourceHob != NULL) {
>> +    //
>> +    // If a Memory Type Information Resource HOB was found, then use the
>> address
>> +    // range of the  Memory Type Information Resource HOB as the
>> preferred
>> +    // address range for the Memory Type Information bins.
>> +    //
>> +    CoreSetMemoryTypeInformationRange (
>> +      MemoryTypeInformationResourceHob->PhysicalStart,
>> +      MemoryTypeInformationResourceHob->ResourceLength
>> +      );
>> +  }
>> +
>>    //
>>    // Declare the very first memory region, so the EFI Memory Services are
>> available.
>>    //
>> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
>> b/MdeModulePkg/Core/Dxe/Mem/Page.c
>> index 6497af573353..458c62090265 100644
>> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
>> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
>> @@ -532,6 +532,107 @@ CoreLoadingFixedAddressHook (
>>    return;
>>  }
>>
>> +/**
>> +  Sets the preferred memory range to use for the Memory Type Information
>> bins.
>> +  This service must be called before fist call to
> CoreAddMemoryDescriptor().
>> +
>> +  If the location of the Memory Type Information bins has already been
>> +  established or the size of the range provides is smaller than all the
>> +  Memory Type Information bins, then the range provides is not used.
>> +
>> +  @param  Start   The start address of the Memory Type Information
>> range.
>> +  @param  Length  The size, in bytes, of the Memory Type Information
>> range.
>> +**/
>> +VOID
>> +CoreSetMemoryTypeInformationRange (
>> +  IN EFI_PHYSICAL_ADDRESS  Start,
>> +  IN UINT64                Length
>> +  )
>> +{
>> +  EFI_PHYSICAL_ADDRESS  Top;
>> +  EFI_MEMORY_TYPE       Type;
>> +  UINTN                 Index;
>> +  UINTN                 Size;
>> +
>> +  //
>> +  // Return if Memory Type Information bin locations have already been
> set
>> +  //
>> +  if (mMemoryTypeInformationInitialized) {
>> +    return;
>> +  }
>> +
>> +  //
>> +  // Return if size of the Memory Type Information bins is greater than
>> Length
>> +  //
>> +  Size = 0;
>> +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
>> EfiMaxMemoryType; Index++) {
>> +    //
>> +    // Make sure the memory type in the gMemoryTypeInformation[] array
>> is valid
>> +    //
>> +    Type = (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
>> +    if ((UINT32)Type > EfiMaxMemoryType) {
>> +      continue;
>> +    }
>> +
>> +    Size += EFI_PAGES_TO_SIZE
>> (gMemoryTypeInformation[Index].NumberOfPages);
>> +  }
>> +
>> +  if (Size > Length) {
>> +    return;
>> +  }
>> +
>> +  //
>> +  // Loop through each memory type in the order specified by the
>> +  // gMemoryTypeInformation[] array
>> +  //
>> +  Top = Start + Length;
>> +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
>> EfiMaxMemoryType; Index++) {
>> +    //
>> +    // Make sure the memory type in the gMemoryTypeInformation[] array
>> is valid
>> +    //
>> +    Type = (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
>> +    if ((UINT32)Type > EfiMaxMemoryType) {
>> +      continue;
>> +    }
>> +
>> +    if (gMemoryTypeInformation[Index].NumberOfPages != 0) {
>> +      mMemoryTypeStatistics[Type].MaximumAddress = Top - 1;
>> +      Top                                       -=
>> EFI_PAGES_TO_SIZE (gMemoryTypeInformation[Index].NumberOfPages);
>> +      mMemoryTypeStatistics[Type].BaseAddress    = Top;
>> +
>> +      //
>> +      // If the current base address is the lowest address so far, then
>> update
>> +      // the default maximum address
>> +      //
>> +      if (mMemoryTypeStatistics[Type].BaseAddress <
>> mDefaultMaximumAddress) {
>> +        mDefaultMaximumAddress =
>> mMemoryTypeStatistics[Type].BaseAddress - 1;
>> +      }
>> +
>> +      mMemoryTypeStatistics[Type].NumberOfPages   =
>> gMemoryTypeInformation[Index].NumberOfPages;
>> +      gMemoryTypeInformation[Index].NumberOfPages = 0;
>> +    }
>> +  }
>> +
>> +  //
>> +  // If the number of pages reserved for a memory type is 0, then all
>> +  // allocations for that type should be in the default range.
>> +  //
>> +  for (Type = (EFI_MEMORY_TYPE)0; Type < EfiMaxMemoryType; Type++) {
>> +    for (Index = 0; gMemoryTypeInformation[Index].Type !=
>> EfiMaxMemoryType; Index++) {
>> +      if (Type ==
>> (EFI_MEMORY_TYPE)gMemoryTypeInformation[Index].Type) {
>> +        mMemoryTypeStatistics[Type].InformationIndex = Index;
>> +      }
>> +    }
>> +
>> +    mMemoryTypeStatistics[Type].CurrentNumberOfPages = 0;
>> +    if (mMemoryTypeStatistics[Type].MaximumAddress ==
>> MAX_ALLOC_ADDRESS) {
>> +      mMemoryTypeStatistics[Type].MaximumAddress =
>> mDefaultMaximumAddress;
>> +    }
>> +  }
>> +
>> +  mMemoryTypeInformationInitialized = TRUE;
>> +}
>> +
>>  /**
>>    Called to initialize the memory map and add descriptors to
>>    the current descriptor list.
>> --
>> 2.40.1.windows.1
> 
> 
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114316): https://edk2.groups.io/g/devel/message/114316
Mute This Topic: https://groups.io/mt/103933112/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB
  2024-01-24 14:17 ` Laszlo Ersek
@ 2024-01-24 17:39   ` Michael D Kinney
  0 siblings, 0 replies; 12+ messages in thread
From: Michael D Kinney @ 2024-01-24 17:39 UTC (permalink / raw)
  To: Laszlo Ersek, devel@edk2.groups.io
  Cc: Liming Gao, Li, Aaron, Liu, Yun Y, Andrew Fish, Kinney, Michael D

Hi Laszlo,

Responses below.

Mike

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Wednesday, January 24, 2024 6:17 AM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>; Li, Aaron
> <aaron.li@intel.com>; Liu, Yun Y <yun.y.liu@intel.com>; Andrew Fish
> <afish@apple.com>
> Subject: Re: [edk2-devel] [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set
> MemoryTypeInfo bin range from HOB
> 
> On 1/23/24 21:24, Michael D Kinney wrote:
> > Provide an optional method for PEI to declare a specific address
> > range to use for the Memory Type Information bins. The current
> > algorithm uses heuristics that tends to place the Memory Type
> > Information bins in the same location, but memory configuration
> > changes across boots or algorithm changes across a firmware
> > updates could potentially change the Memory Type Information bin
> > location.
> >
> > If the HOB List contains a Resource Descriptor HOB that
> > describes tested system memory and has an Owner GUID of
> > gEfiMemoryTypeInformationGuid, then use the address range
> > described by the Resource Descriptor HOB as the preferred
> > location of the Memory Type Information bins. If this HOB is
> > not detected, then the current behavior is preserved.
> >
> > The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
> > is ignored for the following conditions:
> > * The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
> >   is smaller than the Memory Type Information bins.
> > * The HOB list contains more than one Resource Descriptor HOB
> >   with an owner GUID of gEfiMemoryTypeInformationGuid.
> > * The Resource Descriptor HOB with an Owner GUID of
> >   gEfiMemoryTypeInformationGuid is the same Resource Descriptor
> >   HOB that that describes the PHIT memory range.
> 
> I feel that, while this GUID usage makes sense, it overloads the already
> existing meanings of gEfiMemoryTypeInformationGuid.
> 
> We already use this GUID for two things:
> 
> (see "MdeModulePkg/Include/Guid/MemoryTypeInformation.h"):
> 
> - UEFI variable name space GUID for the "MemoryTypeInformation" variable
> 
> - HOB of type EFI_HOB_TYPE_GUID_EXTENSION, controlling the page counts
> in the various memory type bins.
> 
> Now this new use would introduce a HOB of type
> EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, where the Owner field is
> gEfiMemoryTypeInformationGuid, and the range described by the
> PhysicalStart and ResourceLength fields would accommodate the actual
> bins. On one hand this new HOB is certainly related to the prior two
> uses, but on the other hand, we'd now have to HOBs that used the "same
> GUID" for different purposes. What distinguishes them is
> EFI_HOB_TYPE_GUID_EXTENSION vs. EFI_HOB_TYPE_RESOURCE_DESCRIPTOR --
> which I find a bit too obscure.
> 
> (1) So I'd either suggest generating a brand new GUID, *or* extending
> the
> comments in "MdeModulePkg/Include/Guid/MemoryTypeInformation.h" with the
> new usage:

I will extend the description of the GUID in MemoryTypeInformation.h

> 
> > /** @file
> >   This file defines:
> >   * Memory Type Information GUID for HOB and Variable.
> >   * Memory Type Information Variable Name.
> >   * Memory Type Information GUID HOB data structure.
> >
> >   The memory type information HOB and variable can
> >   be used to store the information for each memory type in Variable or
> HOB.
> >
> > Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> 
> Back to the patch:
> 
> On 1/23/24 21:24, Michael D Kinney wrote:
> > Update the DxeMain initialization order to initialize GCD
> > services before any runtime allocations are performed.  This
> > is required to prevent runtime data fragmentation when the
> > UEFI System Table and UEFI Runtime Service Table is allocated.
> 
> (2) Should this be a separate patch? (First patch in the series?) This
> seems
> to effect behavior even if the new HOB is not present.

I did consider doing this as a separate patch because the change in the
order of init in DxeMain can be done independent of this new optional
feature.  I will break it out to make that clear and simpler to review.
The new feature does depend on this new order so it has to be the first
patch.

> 
> >
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Aaron Li <aaron.li@intel.com>
> > Cc: Liu Yun <yun.y.liu@intel.com>
> > Cc: Andrew Fish <afish@apple.com>
> > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > ---
> >  MdeModulePkg/Core/Dxe/DxeMain.h         |   6 ++
> >  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c |  23 +++---
> >  MdeModulePkg/Core/Dxe/Gcd/Gcd.c         |  72 ++++++++++++++++-
> >  MdeModulePkg/Core/Dxe/Mem/Page.c        | 101
> ++++++++++++++++++++++++
> >  4 files changed, 190 insertions(+), 12 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
> b/MdeModulePkg/Core/Dxe/DxeMain.h
> > index 86a7be2f5188..53e26703f8c7 100644
> > --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> > +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> > @@ -277,6 +277,12 @@ CoreInitializePool (
> >    VOID
> >    );
> >
> > +VOID
> > +CoreSetMemoryTypeInformationRange (
> > +  IN EFI_PHYSICAL_ADDRESS  Start,
> > +  IN UINT64                Length
> > +  );
> > +
> >  /**
> >    Called to initialize the memory map and add descriptors to
> >    the current descriptor list.
> > diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > index 0e0f9769b99d..17d510a287e5 100644
> > --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > @@ -276,6 +276,18 @@ DxeMain (
> >
> >    MemoryProfileInit (HobStart);
> >
> > +  //
> > +  // Start the Image Services.
> > +  //
> > +  Status = CoreInitializeImageServices (HobStart);
> > +  ASSERT_EFI_ERROR (Status);
> > +
> > +  //
> > +  // Initialize the Global Coherency Domain Services
> > +  //
> > +  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress,
> MemoryLength);
> > +  ASSERT_EFI_ERROR (Status);
> > +
> >    //
> >    // Allocate the EFI System Table and EFI Runtime Service Table from
> EfiRuntimeServicesData
> >    // Use the templates to initialize the contents of the EFI System
> Table and EFI Runtime Services Table
> > @@ -289,16 +301,9 @@ DxeMain (
> >    gDxeCoreST->RuntimeServices = gDxeCoreRT;
> >
> >    //
> > -  // Start the Image Services.
> > +  // Update DXE Core Loaded Image Protocol with allocated UEFI System
> Table
> >    //
> > -  Status = CoreInitializeImageServices (HobStart);
> > -  ASSERT_EFI_ERROR (Status);
> > -
> > -  //
> > -  // Initialize the Global Coherency Domain Services
> > -  //
> > -  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress,
> MemoryLength);
> > -  ASSERT_EFI_ERROR (Status);
> > +  gDxeCoreLoadedImage->SystemTable = gDxeCoreST;
> >
> >    //
> >    // Call constructor for all libraries
> 
> (3) The separate assignment to "gDxeCoreLoadedImage->SystemTable"
> becomes necessary because CoreInitializeImageServices() is hoisted above
> 
>   gDxeCoreST = AllocateRuntimeCopyPool (sizeof (EFI_SYSTEM_TABLE),
> &mEfiSystemTableTemplate);
> 
> and therefore the assignment
> 
>   Image->Info.SystemTable = gDxeCoreST;
> 
> in CoreInitializeImageServices() becomes ineffective -- at that time,
> gDxeCoreST is NULL, post-patch.
> 
> This double-write (overwrite) to the SystemTable field is confusing. I
> propose (again, in said separate, first patch) the removal of the
> now-ineffective assignment from CoreInitializeImageServices().


Agreed.  I will remove that ineffective assignment and do that in the
first patch.

> 
> 
> > diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > index 792cd2e0af23..c450d1bf2531 100644
> > --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > @@ -2238,6 +2238,8 @@ CoreInitializeMemoryServices (
> >    EFI_HOB_HANDOFF_INFO_TABLE   *PhitHob;
> >    EFI_HOB_RESOURCE_DESCRIPTOR  *ResourceHob;
> >    EFI_HOB_RESOURCE_DESCRIPTOR  *PhitResourceHob;
> > +  EFI_HOB_RESOURCE_DESCRIPTOR  *MemoryTypeInformationResourceHob;
> > +  UINTN                        Count;
> >    EFI_PHYSICAL_ADDRESS         BaseAddress;
> >    UINT64                       Length;
> >    UINT64                       Attributes;
> > @@ -2289,12 +2291,47 @@ CoreInitializeMemoryServices (
> >    //
> >    // See if a Memory Type Information HOB is available
> >    //
> > -  GuidHob = GetFirstGuidHob (&gEfiMemoryTypeInformationGuid);
> > +  MemoryTypeInformationResourceHob = NULL;
> > +  GuidHob                          = GetFirstGuidHob
> (&gEfiMemoryTypeInformationGuid);
> >    if (GuidHob != NULL) {
> >      EfiMemoryTypeInformation = GET_GUID_HOB_DATA (GuidHob);
> >      DataSize                 = GET_GUID_HOB_DATA_SIZE (GuidHob);
> >      if ((EfiMemoryTypeInformation != NULL) && (DataSize > 0) &&
> (DataSize <= (EfiMaxMemoryType + 1) * sizeof
> (EFI_MEMORY_TYPE_INFORMATION))) {
> >        CopyMem (&gMemoryTypeInformation, EfiMemoryTypeInformation,
> DataSize);
> > +
> > +      //
> > +      // Look for Resource Descriptor HOB with a ResourceType of
> System Memory
> > +      // and an Owner GUID of gEfiMemoryTypeInformationGuid. If more
> than 1 is
> > +      // found, then set MemoryTypeInformationResourceHob to NULL.
> > +      //
> > +      Count = 0;
> > +      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw =
> GET_NEXT_HOB (Hob)) {
> > +        if (GET_HOB_TYPE (Hob) != EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
> > +          continue;
> > +        }
> > +
> > +        ResourceHob = Hob.ResourceDescriptor;
> > +        if (!CompareGuid (&ResourceHob->Owner,
> &gEfiMemoryTypeInformationGuid)) {
> > +          continue;
> > +        }
> > +
> > +        Count++;
> > +        if (ResourceHob->ResourceType != EFI_RESOURCE_SYSTEM_MEMORY)
> {
> > +          continue;
> > +        }
> > +
> > +        if ((ResourceHob->ResourceAttribute & MEMORY_ATTRIBUTE_MASK)
> != TESTED_MEMORY_ATTRIBUTES) {
> > +          continue;
> > +        }
> > +
> > +        if (ResourceHob->ResourceLength >=
> CalculateTotalMemoryBinSizeNeeded ()) {
> > +          MemoryTypeInformationResourceHob = ResourceHob;
> > +        }
> > +      }
> > +
> > +      if (Count > 1) {
> > +        MemoryTypeInformationResourceHob = NULL;
> > +      }
> >      }
> >    }
> >
> > @@ -2344,6 +2381,15 @@ CoreInitializeMemoryServices (
> >      PhitResourceHob = ResourceHob;
> >      Found           = TRUE;
> >
> > +    //
> > +    // If a Memory Type Information Resource HOB was found and is the
> same
> > +    // Resource HOB that describes the PHIT HOB, then ignore the
> Memory Type
> > +    // Information Resource HOB.
> > +    //
> > +    if (MemoryTypeInformationResourceHob == PhitResourceHob) {
> > +      MemoryTypeInformationResourceHob = NULL;
> > +    }
> > +
> >      //
> >      // Compute range between PHIT EfiMemoryTop and the end of the
> Resource Descriptor HOB
> >      //
> > @@ -2387,8 +2433,9 @@ CoreInitializeMemoryServices (
> >    if (Length < MinimalMemorySizeNeeded) {
> >      //
> >      // Search all the resource descriptor HOBs from the highest
> possible addresses down for a memory
> > -    // region that is big enough to initialize the DXE core.  Always
> skip the PHIT Resource HOB.
> > -    // The max address must be within the physically addressible
> range for the processor.
> > +    // region that is big enough to initialize the DXE core.  Always
> skip the PHIT Resource HOB
> > +    // and the Memory Type Information Resource HOB. The max address
> must be within the physically
> > +    // addressable range for the processor.
> >      //
> >      HighAddress = MAX_ALLOC_ADDRESS;
> >      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw =
> GET_NEXT_HOB (Hob)) {
> > @@ -2399,6 +2446,13 @@ CoreInitializeMemoryServices (
> >          continue;
> >        }
> >
> > +      //
> > +      // Skip the Resource Descriptor HOB that contains Memory Type
> Information bins
> > +      //
> > +      if (Hob.ResourceDescriptor == MemoryTypeInformationResourceHob)
> {
> > +        continue;
> > +      }
> > +
> >        //
> >        // Skip all HOBs except Resource Descriptor HOBs
> >        //
> > @@ -2466,6 +2520,18 @@ CoreInitializeMemoryServices (
> >      Capabilities =
> CoreConvertResourceDescriptorHobAttributesToCapabilities
> (EfiGcdMemoryTypeSystemMemory, Attributes);
> >    }
> >
> > +  if (MemoryTypeInformationResourceHob != NULL) {
> > +    //
> > +    // If a Memory Type Information Resource HOB was found, then use
> the address
> > +    // range of the  Memory Type Information Resource HOB as the
> preferred
> > +    // address range for the Memory Type Information bins.
> > +    //
> > +    CoreSetMemoryTypeInformationRange (
> > +      MemoryTypeInformationResourceHob->PhysicalStart,
> > +      MemoryTypeInformationResourceHob->ResourceLength
> > +      );
> > +  }
> > +
> >    //
> >    // Declare the very first memory region, so the EFI Memory Services
> are available.
> >    //
> 
> Hmm, I think I have a coarse understanding of the control flow here. If
> we call CoreSetMemoryTypeInformationRange() here -- due to the new HOB
> being available --, then "mMemoryTypeInformationInitialized" will be set
> to TRUE. Then, the very first call to CoreAddMemoryDescriptor(), just
> below the context quoted above, will see
> "mMemoryTypeInformationInitialized" as TRUE, and avoid iterating through
> the memory type information array. OK.
> 
> > diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > index 6497af573353..458c62090265 100644
> > --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> > +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > @@ -532,6 +532,107 @@ CoreLoadingFixedAddressHook (
> >    return;
> >  }
> >
> > +/**
> > +  Sets the preferred memory range to use for the Memory Type
> Information bins.
> > +  This service must be called before fist call to
> CoreAddMemoryDescriptor().
> > +
> > +  If the location of the Memory Type Information bins has already
> been
> > +  established or the size of the range provides is smaller than all
> the
> > +  Memory Type Information bins, then the range provides is not used.
> > +
> > +  @param  Start   The start address of the Memory Type Information
> range.
> > +  @param  Length  The size, in bytes, of the Memory Type Information
> range.
> > +**/
> > +VOID
> > +CoreSetMemoryTypeInformationRange (
> > +  IN EFI_PHYSICAL_ADDRESS  Start,
> > +  IN UINT64                Length
> > +  )
> > +{
> > +  EFI_PHYSICAL_ADDRESS  Top;
> > +  EFI_MEMORY_TYPE       Type;
> > +  UINTN                 Index;
> > +  UINTN                 Size;
> > +
> > +  //
> > +  // Return if Memory Type Information bin locations have already
> been set
> > +  //
> > +  if (mMemoryTypeInformationInitialized) {
> > +    return;
> > +  }
> 
> (4) Under what circumstances can this condition evaluate to TRUE?
> 
> Should we perhaps
> 
>   ASSERT (!mMemoryTypeInformationInitialized);
> 
> instead?

I am just being defensive in case code in the future attempts to call this
service more than one time.  This service should only be called one time
and any attempt to call again are ignored.  This is similar to the existing
behavior where the first call to CoreAddMemoryDescriptor() determines
the location of the bins and additional calls to CoreAddMemoryDescriptor()
skip the bin allocation logic.

I am also trying to avoid use of ASSERT() when possible due to overuse.

It may be better to have a DEBUG_ERROR message for this condition, so the
log shows that an attempt to set the memory type information location 
again was ignored.

> 
> > +
> > +  //
> > +  // Return if size of the Memory Type Information bins is greater
> than Length
> > +  //
> > +  Size = 0;
> > +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
> EfiMaxMemoryType; Index++) {
> > +    //
> > +    // Make sure the memory type in the gMemoryTypeInformation[]
> array is valid
> > +    //
> > +    Type = (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> > +    if ((UINT32)Type > EfiMaxMemoryType) {
> > +      continue;
> > +    }
> > +
> > +    Size += EFI_PAGES_TO_SIZE
> (gMemoryTypeInformation[Index].NumberOfPages);
> > +  }
> > +
> > +  if (Size > Length) {
> > +    return;
> > +  }
> > +
> > +  //
> > +  // Loop through each memory type in the order specified by the
> > +  // gMemoryTypeInformation[] array
> > +  //
> > +  Top = Start + Length;
> > +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
> EfiMaxMemoryType; Index++) {
> > +    //
> > +    // Make sure the memory type in the gMemoryTypeInformation[]
> array is valid
> > +    //
> > +    Type = (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> > +    if ((UINT32)Type > EfiMaxMemoryType) {
> > +      continue;
> > +    }
> > +
> > +    if (gMemoryTypeInformation[Index].NumberOfPages != 0) {
> > +      mMemoryTypeStatistics[Type].MaximumAddress = Top - 1;
> > +      Top                                       -= EFI_PAGES_TO_SIZE
> (gMemoryTypeInformation[Index].NumberOfPages);
> > +      mMemoryTypeStatistics[Type].BaseAddress    = Top;
> > +
> > +      //
> > +      // If the current base address is the lowest address so far,
> then update
> > +      // the default maximum address
> > +      //
> > +      if (mMemoryTypeStatistics[Type].BaseAddress <
> mDefaultMaximumAddress) {
> > +        mDefaultMaximumAddress =
> mMemoryTypeStatistics[Type].BaseAddress - 1;
> > +      }
> > +
> > +      mMemoryTypeStatistics[Type].NumberOfPages   =
> gMemoryTypeInformation[Index].NumberOfPages;
> > +      gMemoryTypeInformation[Index].NumberOfPages = 0;
> > +    }
> > +  }
> > +
> > +  //
> > +  // If the number of pages reserved for a memory type is 0, then all
> > +  // allocations for that type should be in the default range.
> > +  //
> > +  for (Type = (EFI_MEMORY_TYPE)0; Type < EfiMaxMemoryType; Type++) {
> > +    for (Index = 0; gMemoryTypeInformation[Index].Type !=
> EfiMaxMemoryType; Index++) {
> > +      if (Type ==
> (EFI_MEMORY_TYPE)gMemoryTypeInformation[Index].Type) {
> > +        mMemoryTypeStatistics[Type].InformationIndex = Index;
> > +      }
> > +    }
> > +
> > +    mMemoryTypeStatistics[Type].CurrentNumberOfPages = 0;
> > +    if (mMemoryTypeStatistics[Type].MaximumAddress ==
> MAX_ALLOC_ADDRESS) {
> > +      mMemoryTypeStatistics[Type].MaximumAddress =
> mDefaultMaximumAddress;
> > +    }
> > +  }
> > +
> > +  mMemoryTypeInformationInitialized = TRUE;
> > +}
> > +
> >  /**
> >    Called to initialize the memory map and add descriptors to
> >    the current descriptor list.
> 
> This part resembles (and seems to substitute for) the tail of said very
> first CoreAddMemoryDescriptor() execution. There are some differences;
> here we don't rely on CoreAllocatePages(). I don't understand either
> piece of code enough for making a meaningful comparison. The general
> control flow separation seems sane, modulo my question (4) above.

I agree they are similar.  I did consider trying to consolidate the 
common logic to a single static helper function.  But there are differences
in the input conditions.  Current logic uses AllocatePages() to attempt
to allocate each bin.  And these allocations are not required to be
contiguous in memory.  They can be piecemeal extended as more memory is
registered until all the bins are allocated.  The memory type information
location from HOB is simpler because it is guaranteed to be a single
contiguous memory range.  This difference may a common helper function
much more challenging.

I was also concerned about inadvertently introducing a behavior change
when the memory type information location feature is not enabled.
Keeping the current code unmodified makes it easy from a review perspective
to know current behavior is preserved and allows a focus on reviewing
the new logic associated with the new feature.

> 
> Thanks,
> Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114345): https://edk2.groups.io/g/devel/message/114345
Mute This Topic: https://groups.io/mt/103918464/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB
  2024-01-24 14:20 ` Laszlo Ersek
@ 2024-01-24 17:46   ` Michael D Kinney
  2024-01-25 21:34     ` Laszlo Ersek
  0 siblings, 1 reply; 12+ messages in thread
From: Michael D Kinney @ 2024-01-24 17:46 UTC (permalink / raw)
  To: Laszlo Ersek, devel@edk2.groups.io
  Cc: Liming Gao, Li, Aaron, Liu, Yun Y, Andrew Fish, Kinney, Michael D

Hi Laszlo,

Yes.  I can add more details in the commit message.

The impact is for ACPI S4.  There are many reasons why the set of
HOBs passed into the DXE Core may change from boot to boot or that
allocations in the early DXE init phase should change where the
memory type information bins are allocated.

ACPI S4 does do a power cycle and it is possible to do FW updates
or FW setup config changes between an S4 save and S4 resume operation.

It is even possible for one OS to do S4 save.  Reboot the system
to boot a completely different OS.  Reboot again and do an S4 resume
of the original OS.

If a platform chooses to enable this feature, the number of scenarios
that an S4 resume can fail is reduced.

Mike

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Wednesday, January 24, 2024 6:21 AM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>; Li, Aaron
> <aaron.li@intel.com>; Liu, Yun Y <yun.y.liu@intel.com>; Andrew Fish
> <afish@apple.com>
> Subject: Re: [edk2-devel] [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set
> MemoryTypeInfo bin range from HOB
> 
> On 1/23/24 21:24, Michael D Kinney wrote:
> > Provide an optional method for PEI to declare a specific address
> > range to use for the Memory Type Information bins. The current
> > algorithm uses heuristics that tends to place the Memory Type
> > Information bins in the same location, but memory configuration
> > changes across boots or algorithm changes across a firmware
> > updates could potentially change the Memory Type Information bin
> > location.
> 
> (5) Why is such a rearrangement of the bins -- which is likely visible
> in the UEFI memory map, too -- a problem?
> 
> Can we include a hint on that in the commit message?
> 
> I understand why it would be a problem across ACPI S4, but a memory
> config change (DIMM addition/removal?), or a firmware update, seems to
> require a full S5 power cycle.
> 
> Thanks
> Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114346): https://edk2.groups.io/g/devel/message/114346
Mute This Topic: https://groups.io/mt/103918464/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] 回复: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB
  2024-01-24 16:16   ` Laszlo Ersek
@ 2024-01-24 17:52     ` Michael D Kinney
  2024-01-25  1:18       ` 回复: " gaoliming via groups.io
  0 siblings, 1 reply; 12+ messages in thread
From: Michael D Kinney @ 2024-01-24 17:52 UTC (permalink / raw)
  To: Laszlo Ersek, devel@edk2.groups.io, gaoliming@byosoft.com.cn
  Cc: Li, Aaron, Liu, Yun Y, 'Andrew Fish', Kinney, Michael D

Hi Liming,

The current algorithm allocates the bins from the top of the first
memory range given to the DXE Core.  This is not guaranteed to be
the top of RAM.  The heuristic will use the memory range from the
PHIT HOB if it is considered big enough.  If PHIT is not big enough
then a search of the HOBs is made for one that is considered big
enough near the top of memory that is tested.

Additional memory can be added or promoted from untested to tested
after DXE Core init and that added or promoted memory may have
a higher address than the initial memory given to the DXE Core.

With current heuristics, platforms have to carefully construct the
HOBs in PEIM phase to get the behavior where these bins are at the
top of RAM.

With this feature, it provides precise control over where the
bins are placed.

Mike

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Wednesday, January 24, 2024 8:17 AM
> To: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Li, Aaron <aaron.li@intel.com>; Liu, Yun Y <yun.y.liu@intel.com>;
> 'Andrew Fish' <afish@apple.com>
> Subject: Re: [edk2-devel] 回复: [Patch v2 1/1] MdeModulePkg/Core/Dxe:
> Set MemoryTypeInfo bin range from HOB
> 
> On 1/24/24 15:59, gaoliming via groups.io wrote:
> > Mike:
> >  Current algorithm tries to reserve the top available memory for
> memory type
> > bin range.
> >  If the platform uses new way to describe the memory range, should we
> > suggest the rule to still
> >  use the top memory resource hob for the memory type bin range?
> 
> How would that work, technically? If the platform specifies a concrete
> address range, how can that be reconciled with using the top of RAM?
> 
> Or do you mean documentation? I.e., a note for platform implementors
> that they should point their new HOB to the top of RAM?
> 
> Thanks
> Laszlo
> 
> >
> > Thanks
> > Liming
> >> -----邮件原件-----
> >> 发件人: Michael D Kinney <michael.d.kinney@intel.com>
> >> 发送时间: 2024年1月24日 4:24
> >> 收件人: devel@edk2.groups.io
> >> 抄送: Liming Gao <gaoliming@byosoft.com.cn>; Aaron Li
> >> <aaron.li@intel.com>; Liu Yun <yun.y.liu@intel.com>; Andrew Fish
> >> <afish@apple.com>
> >> 主题: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin
> >> range from HOB
> >>
> >> Provide an optional method for PEI to declare a specific address
> >> range to use for the Memory Type Information bins. The current
> >> algorithm uses heuristics that tends to place the Memory Type
> >> Information bins in the same location, but memory configuration
> >> changes across boots or algorithm changes across a firmware
> >> updates could potentially change the Memory Type Information bin
> >> location.
> >>
> >> If the HOB List contains a Resource Descriptor HOB that
> >> describes tested system memory and has an Owner GUID of
> >> gEfiMemoryTypeInformationGuid, then use the address range
> >> described by the Resource Descriptor HOB as the preferred
> >> location of the Memory Type Information bins. If this HOB is
> >> not detected, then the current behavior is preserved.
> >>
> >> The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
> >> is ignored for the following conditions:
> >> * The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
> >>   is smaller than the Memory Type Information bins.
> >> * The HOB list contains more than one Resource Descriptor HOB
> >>   with an owner GUID of gEfiMemoryTypeInformationGuid.
> >> * The Resource Descriptor HOB with an Owner GUID of
> >>   gEfiMemoryTypeInformationGuid is the same Resource Descriptor
> >>   HOB that that describes the PHIT memory range.
> >>
> >> Update the DxeMain initialization order to initialize GCD
> >> services before any runtime allocations are performed.  This
> >> is required to prevent runtime data fragmentation when the
> >> UEFI System Table and UEFI Runtime Service Table is allocated.
> >>
> >> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> >> Cc: Aaron Li <aaron.li@intel.com>
> >> Cc: Liu Yun <yun.y.liu@intel.com>
> >> Cc: Andrew Fish <afish@apple.com>
> >> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> >> ---
> >>  MdeModulePkg/Core/Dxe/DxeMain.h         |   6 ++
> >>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c |  23 +++---
> >>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c         |  72 ++++++++++++++++-
> >>  MdeModulePkg/Core/Dxe/Mem/Page.c        | 101
> >> ++++++++++++++++++++++++
> >>  4 files changed, 190 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
> >> b/MdeModulePkg/Core/Dxe/DxeMain.h
> >> index 86a7be2f5188..53e26703f8c7 100644
> >> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> >> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> >> @@ -277,6 +277,12 @@ CoreInitializePool (
> >>    VOID
> >>    );
> >>
> >> +VOID
> >> +CoreSetMemoryTypeInformationRange (
> >> +  IN EFI_PHYSICAL_ADDRESS  Start,
> >> +  IN UINT64                Length
> >> +  );
> >> +
> >>  /**
> >>    Called to initialize the memory map and add descriptors to
> >>    the current descriptor list.
> >> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> >> b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> >> index 0e0f9769b99d..17d510a287e5 100644
> >> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> >> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> >> @@ -276,6 +276,18 @@ DxeMain (
> >>
> >>    MemoryProfileInit (HobStart);
> >>
> >> +  //
> >> +  // Start the Image Services.
> >> +  //
> >> +  Status = CoreInitializeImageServices (HobStart);
> >> +  ASSERT_EFI_ERROR (Status);
> >> +
> >> +  //
> >> +  // Initialize the Global Coherency Domain Services
> >> +  //
> >> +  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress,
> >> MemoryLength);
> >> +  ASSERT_EFI_ERROR (Status);
> >> +
> >>    //
> >>    // Allocate the EFI System Table and EFI Runtime Service Table
> from
> >> EfiRuntimeServicesData
> >>    // Use the templates to initialize the contents of the EFI System
> Table
> > and
> >> EFI Runtime Services Table
> >> @@ -289,16 +301,9 @@ DxeMain (
> >>    gDxeCoreST->RuntimeServices = gDxeCoreRT;
> >>
> >>    //
> >> -  // Start the Image Services.
> >> +  // Update DXE Core Loaded Image Protocol with allocated UEFI
> System
> >> Table
> >>    //
> >> -  Status = CoreInitializeImageServices (HobStart);
> >> -  ASSERT_EFI_ERROR (Status);
> >> -
> >> -  //
> >> -  // Initialize the Global Coherency Domain Services
> >> -  //
> >> -  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress,
> >> MemoryLength);
> >> -  ASSERT_EFI_ERROR (Status);
> >> +  gDxeCoreLoadedImage->SystemTable = gDxeCoreST;
> >>
> >>    //
> >>    // Call constructor for all libraries
> >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> >> b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> >> index 792cd2e0af23..c450d1bf2531 100644
> >> --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> >> +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> >> @@ -2238,6 +2238,8 @@ CoreInitializeMemoryServices (
> >>    EFI_HOB_HANDOFF_INFO_TABLE   *PhitHob;
> >>    EFI_HOB_RESOURCE_DESCRIPTOR  *ResourceHob;
> >>    EFI_HOB_RESOURCE_DESCRIPTOR  *PhitResourceHob;
> >> +  EFI_HOB_RESOURCE_DESCRIPTOR
> >> *MemoryTypeInformationResourceHob;
> >> +  UINTN                        Count;
> >>    EFI_PHYSICAL_ADDRESS         BaseAddress;
> >>    UINT64                       Length;
> >>    UINT64                       Attributes;
> >> @@ -2289,12 +2291,47 @@ CoreInitializeMemoryServices (
> >>    //
> >>    // See if a Memory Type Information HOB is available
> >>    //
> >> -  GuidHob = GetFirstGuidHob (&gEfiMemoryTypeInformationGuid);
> >> +  MemoryTypeInformationResourceHob = NULL;
> >> +  GuidHob                          = GetFirstGuidHob
> >> (&gEfiMemoryTypeInformationGuid);
> >>    if (GuidHob != NULL) {
> >>      EfiMemoryTypeInformation = GET_GUID_HOB_DATA (GuidHob);
> >>      DataSize                 = GET_GUID_HOB_DATA_SIZE (GuidHob);
> >>      if ((EfiMemoryTypeInformation != NULL) && (DataSize > 0) &&
> (DataSize
> >> <= (EfiMaxMemoryType + 1) * sizeof (EFI_MEMORY_TYPE_INFORMATION))) {
> >>        CopyMem (&gMemoryTypeInformation, EfiMemoryTypeInformation,
> >> DataSize);
> >> +
> >> +      //
> >> +      // Look for Resource Descriptor HOB with a ResourceType of
> System
> >> Memory
> >> +      // and an Owner GUID of gEfiMemoryTypeInformationGuid. If more
> >> than 1 is
> >> +      // found, then set MemoryTypeInformationResourceHob to NULL.
> >> +      //
> >> +      Count = 0;
> >> +      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw =
> >> GET_NEXT_HOB (Hob)) {
> >> +        if (GET_HOB_TYPE (Hob) !=
> >> EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
> >> +          continue;
> >> +        }
> >> +
> >> +        ResourceHob = Hob.ResourceDescriptor;
> >> +        if (!CompareGuid (&ResourceHob->Owner,
> >> &gEfiMemoryTypeInformationGuid)) {
> >> +          continue;
> >> +        }
> >> +
> >> +        Count++;
> >> +        if (ResourceHob->ResourceType !=
> >> EFI_RESOURCE_SYSTEM_MEMORY) {
> >> +          continue;
> >> +        }
> >> +
> >> +        if ((ResourceHob->ResourceAttribute &
> >> MEMORY_ATTRIBUTE_MASK) != TESTED_MEMORY_ATTRIBUTES) {
> >> +          continue;
> >> +        }
> >> +
> >> +        if (ResourceHob->ResourceLength >=
> >> CalculateTotalMemoryBinSizeNeeded ()) {
> >> +          MemoryTypeInformationResourceHob = ResourceHob;
> >> +        }
> >> +      }
> >> +
> >> +      if (Count > 1) {
> >> +        MemoryTypeInformationResourceHob = NULL;
> >> +      }
> >>      }
> >>    }
> >>
> >> @@ -2344,6 +2381,15 @@ CoreInitializeMemoryServices (
> >>      PhitResourceHob = ResourceHob;
> >>      Found           = TRUE;
> >>
> >> +    //
> >> +    // If a Memory Type Information Resource HOB was found and is
> the
> >> same
> >> +    // Resource HOB that describes the PHIT HOB, then ignore the
> Memory
> >> Type
> >> +    // Information Resource HOB.
> >> +    //
> >> +    if (MemoryTypeInformationResourceHob == PhitResourceHob) {
> >> +      MemoryTypeInformationResourceHob = NULL;
> >> +    }
> >> +
> >>      //
> >>      // Compute range between PHIT EfiMemoryTop and the end of the
> >> Resource Descriptor HOB
> >>      //
> >> @@ -2387,8 +2433,9 @@ CoreInitializeMemoryServices (
> >>    if (Length < MinimalMemorySizeNeeded) {
> >>      //
> >>      // Search all the resource descriptor HOBs from the highest
> possible
> >> addresses down for a memory
> >> -    // region that is big enough to initialize the DXE core.  Always
> skip
> > the
> >> PHIT Resource HOB.
> >> -    // The max address must be within the physically addressible
> range
> > for
> >> the processor.
> >> +    // region that is big enough to initialize the DXE core.  Always
> skip
> > the
> >> PHIT Resource HOB
> >> +    // and the Memory Type Information Resource HOB. The max address
> >> must be within the physically
> >> +    // addressable range for the processor.
> >>      //
> >>      HighAddress = MAX_ALLOC_ADDRESS;
> >>      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw =
> >> GET_NEXT_HOB (Hob)) {
> >> @@ -2399,6 +2446,13 @@ CoreInitializeMemoryServices (
> >>          continue;
> >>        }
> >>
> >> +      //
> >> +      // Skip the Resource Descriptor HOB that contains Memory Type
> >> Information bins
> >> +      //
> >> +      if (Hob.ResourceDescriptor ==
> MemoryTypeInformationResourceHob)
> >> {
> >> +        continue;
> >> +      }
> >> +
> >>        //
> >>        // Skip all HOBs except Resource Descriptor HOBs
> >>        //
> >> @@ -2466,6 +2520,18 @@ CoreInitializeMemoryServices (
> >>      Capabilities =
> >> CoreConvertResourceDescriptorHobAttributesToCapabilities
> >> (EfiGcdMemoryTypeSystemMemory, Attributes);
> >>    }
> >>
> >> +  if (MemoryTypeInformationResourceHob != NULL) {
> >> +    //
> >> +    // If a Memory Type Information Resource HOB was found, then use
> the
> >> address
> >> +    // range of the  Memory Type Information Resource HOB as the
> >> preferred
> >> +    // address range for the Memory Type Information bins.
> >> +    //
> >> +    CoreSetMemoryTypeInformationRange (
> >> +      MemoryTypeInformationResourceHob->PhysicalStart,
> >> +      MemoryTypeInformationResourceHob->ResourceLength
> >> +      );
> >> +  }
> >> +
> >>    //
> >>    // Declare the very first memory region, so the EFI Memory
> Services are
> >> available.
> >>    //
> >> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> index 6497af573353..458c62090265 100644
> >> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> @@ -532,6 +532,107 @@ CoreLoadingFixedAddressHook (
> >>    return;
> >>  }
> >>
> >> +/**
> >> +  Sets the preferred memory range to use for the Memory Type
> Information
> >> bins.
> >> +  This service must be called before fist call to
> > CoreAddMemoryDescriptor().
> >> +
> >> +  If the location of the Memory Type Information bins has already
> been
> >> +  established or the size of the range provides is smaller than all
> the
> >> +  Memory Type Information bins, then the range provides is not used.
> >> +
> >> +  @param  Start   The start address of the Memory Type Information
> >> range.
> >> +  @param  Length  The size, in bytes, of the Memory Type Information
> >> range.
> >> +**/
> >> +VOID
> >> +CoreSetMemoryTypeInformationRange (
> >> +  IN EFI_PHYSICAL_ADDRESS  Start,
> >> +  IN UINT64                Length
> >> +  )
> >> +{
> >> +  EFI_PHYSICAL_ADDRESS  Top;
> >> +  EFI_MEMORY_TYPE       Type;
> >> +  UINTN                 Index;
> >> +  UINTN                 Size;
> >> +
> >> +  //
> >> +  // Return if Memory Type Information bin locations have already
> been
> > set
> >> +  //
> >> +  if (mMemoryTypeInformationInitialized) {
> >> +    return;
> >> +  }
> >> +
> >> +  //
> >> +  // Return if size of the Memory Type Information bins is greater
> than
> >> Length
> >> +  //
> >> +  Size = 0;
> >> +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
> >> EfiMaxMemoryType; Index++) {
> >> +    //
> >> +    // Make sure the memory type in the gMemoryTypeInformation[]
> array
> >> is valid
> >> +    //
> >> +    Type = (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> >> +    if ((UINT32)Type > EfiMaxMemoryType) {
> >> +      continue;
> >> +    }
> >> +
> >> +    Size += EFI_PAGES_TO_SIZE
> >> (gMemoryTypeInformation[Index].NumberOfPages);
> >> +  }
> >> +
> >> +  if (Size > Length) {
> >> +    return;
> >> +  }
> >> +
> >> +  //
> >> +  // Loop through each memory type in the order specified by the
> >> +  // gMemoryTypeInformation[] array
> >> +  //
> >> +  Top = Start + Length;
> >> +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
> >> EfiMaxMemoryType; Index++) {
> >> +    //
> >> +    // Make sure the memory type in the gMemoryTypeInformation[]
> array
> >> is valid
> >> +    //
> >> +    Type = (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> >> +    if ((UINT32)Type > EfiMaxMemoryType) {
> >> +      continue;
> >> +    }
> >> +
> >> +    if (gMemoryTypeInformation[Index].NumberOfPages != 0) {
> >> +      mMemoryTypeStatistics[Type].MaximumAddress = Top - 1;
> >> +      Top                                       -=
> >> EFI_PAGES_TO_SIZE (gMemoryTypeInformation[Index].NumberOfPages);
> >> +      mMemoryTypeStatistics[Type].BaseAddress    = Top;
> >> +
> >> +      //
> >> +      // If the current base address is the lowest address so far,
> then
> >> update
> >> +      // the default maximum address
> >> +      //
> >> +      if (mMemoryTypeStatistics[Type].BaseAddress <
> >> mDefaultMaximumAddress) {
> >> +        mDefaultMaximumAddress =
> >> mMemoryTypeStatistics[Type].BaseAddress - 1;
> >> +      }
> >> +
> >> +      mMemoryTypeStatistics[Type].NumberOfPages   =
> >> gMemoryTypeInformation[Index].NumberOfPages;
> >> +      gMemoryTypeInformation[Index].NumberOfPages = 0;
> >> +    }
> >> +  }
> >> +
> >> +  //
> >> +  // If the number of pages reserved for a memory type is 0, then
> all
> >> +  // allocations for that type should be in the default range.
> >> +  //
> >> +  for (Type = (EFI_MEMORY_TYPE)0; Type < EfiMaxMemoryType; Type++) {
> >> +    for (Index = 0; gMemoryTypeInformation[Index].Type !=
> >> EfiMaxMemoryType; Index++) {
> >> +      if (Type ==
> >> (EFI_MEMORY_TYPE)gMemoryTypeInformation[Index].Type) {
> >> +        mMemoryTypeStatistics[Type].InformationIndex = Index;
> >> +      }
> >> +    }
> >> +
> >> +    mMemoryTypeStatistics[Type].CurrentNumberOfPages = 0;
> >> +    if (mMemoryTypeStatistics[Type].MaximumAddress ==
> >> MAX_ALLOC_ADDRESS) {
> >> +      mMemoryTypeStatistics[Type].MaximumAddress =
> >> mDefaultMaximumAddress;
> >> +    }
> >> +  }
> >> +
> >> +  mMemoryTypeInformationInitialized = TRUE;
> >> +}
> >> +
> >>  /**
> >>    Called to initialize the memory map and add descriptors to
> >>    the current descriptor list.
> >> --
> >> 2.40.1.windows.1
> >
> >
> >
> >
> >
> > 
> >
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114347): https://edk2.groups.io/g/devel/message/114347
Mute This Topic: https://groups.io/mt/103933112/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* 回复: [edk2-devel] 回复: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB
  2024-01-24 17:52     ` Michael D Kinney
@ 2024-01-25  1:18       ` gaoliming via groups.io
  2024-01-25  1:39         ` Michael D Kinney
  0 siblings, 1 reply; 12+ messages in thread
From: gaoliming via groups.io @ 2024-01-25  1:18 UTC (permalink / raw)
  To: 'Kinney, Michael D', 'Laszlo Ersek', devel
  Cc: 'Li, Aaron', 'Liu, Yun Y', 'Andrew Fish'

Mike:
  I suggest to document the recommended usage of the specified memory type info bin range. I worry that someone configures the low memory range for the memory type info bin range. It may still cause S4 failure. 

Thanks
Liming
> -----邮件原件-----
> 发件人: Kinney, Michael D <michael.d.kinney@intel.com>
> 发送时间: 2024年1月25日 1:53
> 收件人: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io;
> gaoliming@byosoft.com.cn
> 抄送: Li, Aaron <aaron.li@intel.com>; Liu, Yun Y <yun.y.liu@intel.com>;
> 'Andrew Fish' <afish@apple.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> 主题: RE: [edk2-devel] 回复: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set
> MemoryTypeInfo bin range from HOB
> 
> Hi Liming,
> 
> The current algorithm allocates the bins from the top of the first
> memory range given to the DXE Core.  This is not guaranteed to be
> the top of RAM.  The heuristic will use the memory range from the
> PHIT HOB if it is considered big enough.  If PHIT is not big enough
> then a search of the HOBs is made for one that is considered big
> enough near the top of memory that is tested.
> 
> Additional memory can be added or promoted from untested to tested
> after DXE Core init and that added or promoted memory may have
> a higher address than the initial memory given to the DXE Core.
> 
> With current heuristics, platforms have to carefully construct the
> HOBs in PEIM phase to get the behavior where these bins are at the
> top of RAM.
> 
> With this feature, it provides precise control over where the
> bins are placed.
> 
> Mike
> 
> > -----Original Message-----
> > From: Laszlo Ersek <lersek@redhat.com>
> > Sent: Wednesday, January 24, 2024 8:17 AM
> > To: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Cc: Li, Aaron <aaron.li@intel.com>; Liu, Yun Y <yun.y.liu@intel.com>;
> > 'Andrew Fish' <afish@apple.com>
> > Subject: Re: [edk2-devel] 回复: [Patch v2 1/1] MdeModulePkg/Core/Dxe:
> > Set MemoryTypeInfo bin range from HOB
> >
> > On 1/24/24 15:59, gaoliming via groups.io wrote:
> > > Mike:
> > >  Current algorithm tries to reserve the top available memory for
> > memory type
> > > bin range.
> > >  If the platform uses new way to describe the memory range, should we
> > > suggest the rule to still
> > >  use the top memory resource hob for the memory type bin range?
> >
> > How would that work, technically? If the platform specifies a concrete
> > address range, how can that be reconciled with using the top of RAM?
> >
> > Or do you mean documentation? I.e., a note for platform implementors
> > that they should point their new HOB to the top of RAM?
> >
> > Thanks
> > Laszlo
> >
> > >
> > > Thanks
> > > Liming
> > >> -----邮件原件-----
> > >> 发件人: Michael D Kinney <michael.d.kinney@intel.com>
> > >> 发送时间: 2024年1月24日 4:24
> > >> 收件人: devel@edk2.groups.io
> > >> 抄送: Liming Gao <gaoliming@byosoft.com.cn>; Aaron Li
> > >> <aaron.li@intel.com>; Liu Yun <yun.y.liu@intel.com>; Andrew Fish
> > >> <afish@apple.com>
> > >> 主题: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo
> bin
> > >> range from HOB
> > >>
> > >> Provide an optional method for PEI to declare a specific address
> > >> range to use for the Memory Type Information bins. The current
> > >> algorithm uses heuristics that tends to place the Memory Type
> > >> Information bins in the same location, but memory configuration
> > >> changes across boots or algorithm changes across a firmware
> > >> updates could potentially change the Memory Type Information bin
> > >> location.
> > >>
> > >> If the HOB List contains a Resource Descriptor HOB that
> > >> describes tested system memory and has an Owner GUID of
> > >> gEfiMemoryTypeInformationGuid, then use the address range
> > >> described by the Resource Descriptor HOB as the preferred
> > >> location of the Memory Type Information bins. If this HOB is
> > >> not detected, then the current behavior is preserved.
> > >>
> > >> The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
> > >> is ignored for the following conditions:
> > >> * The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
> > >>   is smaller than the Memory Type Information bins.
> > >> * The HOB list contains more than one Resource Descriptor HOB
> > >>   with an owner GUID of gEfiMemoryTypeInformationGuid.
> > >> * The Resource Descriptor HOB with an Owner GUID of
> > >>   gEfiMemoryTypeInformationGuid is the same Resource Descriptor
> > >>   HOB that that describes the PHIT memory range.
> > >>
> > >> Update the DxeMain initialization order to initialize GCD
> > >> services before any runtime allocations are performed.  This
> > >> is required to prevent runtime data fragmentation when the
> > >> UEFI System Table and UEFI Runtime Service Table is allocated.
> > >>
> > >> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > >> Cc: Aaron Li <aaron.li@intel.com>
> > >> Cc: Liu Yun <yun.y.liu@intel.com>
> > >> Cc: Andrew Fish <afish@apple.com>
> > >> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > >> ---
> > >>  MdeModulePkg/Core/Dxe/DxeMain.h         |   6 ++
> > >>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c |  23 +++---
> > >>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c         |  72
> ++++++++++++++++-
> > >>  MdeModulePkg/Core/Dxe/Mem/Page.c        | 101
> > >> ++++++++++++++++++++++++
> > >>  4 files changed, 190 insertions(+), 12 deletions(-)
> > >>
> > >> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
> > >> b/MdeModulePkg/Core/Dxe/DxeMain.h
> > >> index 86a7be2f5188..53e26703f8c7 100644
> > >> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> > >> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> > >> @@ -277,6 +277,12 @@ CoreInitializePool (
> > >>    VOID
> > >>    );
> > >>
> > >> +VOID
> > >> +CoreSetMemoryTypeInformationRange (
> > >> +  IN EFI_PHYSICAL_ADDRESS  Start,
> > >> +  IN UINT64                Length
> > >> +  );
> > >> +
> > >>  /**
> > >>    Called to initialize the memory map and add descriptors to
> > >>    the current descriptor list.
> > >> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > >> b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > >> index 0e0f9769b99d..17d510a287e5 100644
> > >> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > >> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > >> @@ -276,6 +276,18 @@ DxeMain (
> > >>
> > >>    MemoryProfileInit (HobStart);
> > >>
> > >> +  //
> > >> +  // Start the Image Services.
> > >> +  //
> > >> +  Status = CoreInitializeImageServices (HobStart);
> > >> +  ASSERT_EFI_ERROR (Status);
> > >> +
> > >> +  //
> > >> +  // Initialize the Global Coherency Domain Services
> > >> +  //
> > >> +  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress,
> > >> MemoryLength);
> > >> +  ASSERT_EFI_ERROR (Status);
> > >> +
> > >>    //
> > >>    // Allocate the EFI System Table and EFI Runtime Service Table
> > from
> > >> EfiRuntimeServicesData
> > >>    // Use the templates to initialize the contents of the EFI System
> > Table
> > > and
> > >> EFI Runtime Services Table
> > >> @@ -289,16 +301,9 @@ DxeMain (
> > >>    gDxeCoreST->RuntimeServices = gDxeCoreRT;
> > >>
> > >>    //
> > >> -  // Start the Image Services.
> > >> +  // Update DXE Core Loaded Image Protocol with allocated UEFI
> > System
> > >> Table
> > >>    //
> > >> -  Status = CoreInitializeImageServices (HobStart);
> > >> -  ASSERT_EFI_ERROR (Status);
> > >> -
> > >> -  //
> > >> -  // Initialize the Global Coherency Domain Services
> > >> -  //
> > >> -  Status = CoreInitializeGcdServices (&HobStart, MemoryBaseAddress,
> > >> MemoryLength);
> > >> -  ASSERT_EFI_ERROR (Status);
> > >> +  gDxeCoreLoadedImage->SystemTable = gDxeCoreST;
> > >>
> > >>    //
> > >>    // Call constructor for all libraries
> > >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > >> b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > >> index 792cd2e0af23..c450d1bf2531 100644
> > >> --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > >> +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > >> @@ -2238,6 +2238,8 @@ CoreInitializeMemoryServices (
> > >>    EFI_HOB_HANDOFF_INFO_TABLE   *PhitHob;
> > >>    EFI_HOB_RESOURCE_DESCRIPTOR  *ResourceHob;
> > >>    EFI_HOB_RESOURCE_DESCRIPTOR  *PhitResourceHob;
> > >> +  EFI_HOB_RESOURCE_DESCRIPTOR
> > >> *MemoryTypeInformationResourceHob;
> > >> +  UINTN                        Count;
> > >>    EFI_PHYSICAL_ADDRESS         BaseAddress;
> > >>    UINT64                       Length;
> > >>    UINT64                       Attributes;
> > >> @@ -2289,12 +2291,47 @@ CoreInitializeMemoryServices (
> > >>    //
> > >>    // See if a Memory Type Information HOB is available
> > >>    //
> > >> -  GuidHob = GetFirstGuidHob (&gEfiMemoryTypeInformationGuid);
> > >> +  MemoryTypeInformationResourceHob = NULL;
> > >> +  GuidHob                          = GetFirstGuidHob
> > >> (&gEfiMemoryTypeInformationGuid);
> > >>    if (GuidHob != NULL) {
> > >>      EfiMemoryTypeInformation = GET_GUID_HOB_DATA (GuidHob);
> > >>      DataSize                 = GET_GUID_HOB_DATA_SIZE
> (GuidHob);
> > >>      if ((EfiMemoryTypeInformation != NULL) && (DataSize > 0) &&
> > (DataSize
> > >> <= (EfiMaxMemoryType + 1) * sizeof
> (EFI_MEMORY_TYPE_INFORMATION))) {
> > >>        CopyMem (&gMemoryTypeInformation,
> EfiMemoryTypeInformation,
> > >> DataSize);
> > >> +
> > >> +      //
> > >> +      // Look for Resource Descriptor HOB with a ResourceType of
> > System
> > >> Memory
> > >> +      // and an Owner GUID of gEfiMemoryTypeInformationGuid. If
> more
> > >> than 1 is
> > >> +      // found, then set MemoryTypeInformationResourceHob to
> NULL.
> > >> +      //
> > >> +      Count = 0;
> > >> +      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw
> =
> > >> GET_NEXT_HOB (Hob)) {
> > >> +        if (GET_HOB_TYPE (Hob) !=
> > >> EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
> > >> +          continue;
> > >> +        }
> > >> +
> > >> +        ResourceHob = Hob.ResourceDescriptor;
> > >> +        if (!CompareGuid (&ResourceHob->Owner,
> > >> &gEfiMemoryTypeInformationGuid)) {
> > >> +          continue;
> > >> +        }
> > >> +
> > >> +        Count++;
> > >> +        if (ResourceHob->ResourceType !=
> > >> EFI_RESOURCE_SYSTEM_MEMORY) {
> > >> +          continue;
> > >> +        }
> > >> +
> > >> +        if ((ResourceHob->ResourceAttribute &
> > >> MEMORY_ATTRIBUTE_MASK) != TESTED_MEMORY_ATTRIBUTES) {
> > >> +          continue;
> > >> +        }
> > >> +
> > >> +        if (ResourceHob->ResourceLength >=
> > >> CalculateTotalMemoryBinSizeNeeded ()) {
> > >> +          MemoryTypeInformationResourceHob = ResourceHob;
> > >> +        }
> > >> +      }
> > >> +
> > >> +      if (Count > 1) {
> > >> +        MemoryTypeInformationResourceHob = NULL;
> > >> +      }
> > >>      }
> > >>    }
> > >>
> > >> @@ -2344,6 +2381,15 @@ CoreInitializeMemoryServices (
> > >>      PhitResourceHob = ResourceHob;
> > >>      Found           = TRUE;
> > >>
> > >> +    //
> > >> +    // If a Memory Type Information Resource HOB was found and is
> > the
> > >> same
> > >> +    // Resource HOB that describes the PHIT HOB, then ignore the
> > Memory
> > >> Type
> > >> +    // Information Resource HOB.
> > >> +    //
> > >> +    if (MemoryTypeInformationResourceHob == PhitResourceHob) {
> > >> +      MemoryTypeInformationResourceHob = NULL;
> > >> +    }
> > >> +
> > >>      //
> > >>      // Compute range between PHIT EfiMemoryTop and the end of
> the
> > >> Resource Descriptor HOB
> > >>      //
> > >> @@ -2387,8 +2433,9 @@ CoreInitializeMemoryServices (
> > >>    if (Length < MinimalMemorySizeNeeded) {
> > >>      //
> > >>      // Search all the resource descriptor HOBs from the highest
> > possible
> > >> addresses down for a memory
> > >> -    // region that is big enough to initialize the DXE core.  Always
> > skip
> > > the
> > >> PHIT Resource HOB.
> > >> -    // The max address must be within the physically addressible
> > range
> > > for
> > >> the processor.
> > >> +    // region that is big enough to initialize the DXE core.  Always
> > skip
> > > the
> > >> PHIT Resource HOB
> > >> +    // and the Memory Type Information Resource HOB. The max
> address
> > >> must be within the physically
> > >> +    // addressable range for the processor.
> > >>      //
> > >>      HighAddress = MAX_ALLOC_ADDRESS;
> > >>      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw =
> > >> GET_NEXT_HOB (Hob)) {
> > >> @@ -2399,6 +2446,13 @@ CoreInitializeMemoryServices (
> > >>          continue;
> > >>        }
> > >>
> > >> +      //
> > >> +      // Skip the Resource Descriptor HOB that contains Memory Type
> > >> Information bins
> > >> +      //
> > >> +      if (Hob.ResourceDescriptor ==
> > MemoryTypeInformationResourceHob)
> > >> {
> > >> +        continue;
> > >> +      }
> > >> +
> > >>        //
> > >>        // Skip all HOBs except Resource Descriptor HOBs
> > >>        //
> > >> @@ -2466,6 +2520,18 @@ CoreInitializeMemoryServices (
> > >>      Capabilities =
> > >> CoreConvertResourceDescriptorHobAttributesToCapabilities
> > >> (EfiGcdMemoryTypeSystemMemory, Attributes);
> > >>    }
> > >>
> > >> +  if (MemoryTypeInformationResourceHob != NULL) {
> > >> +    //
> > >> +    // If a Memory Type Information Resource HOB was found, then
> use
> > the
> > >> address
> > >> +    // range of the  Memory Type Information Resource HOB as the
> > >> preferred
> > >> +    // address range for the Memory Type Information bins.
> > >> +    //
> > >> +    CoreSetMemoryTypeInformationRange (
> > >> +      MemoryTypeInformationResourceHob->PhysicalStart,
> > >> +      MemoryTypeInformationResourceHob->ResourceLength
> > >> +      );
> > >> +  }
> > >> +
> > >>    //
> > >>    // Declare the very first memory region, so the EFI Memory
> > Services are
> > >> available.
> > >>    //
> > >> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> > >> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > >> index 6497af573353..458c62090265 100644
> > >> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> > >> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > >> @@ -532,6 +532,107 @@ CoreLoadingFixedAddressHook (
> > >>    return;
> > >>  }
> > >>
> > >> +/**
> > >> +  Sets the preferred memory range to use for the Memory Type
> > Information
> > >> bins.
> > >> +  This service must be called before fist call to
> > > CoreAddMemoryDescriptor().
> > >> +
> > >> +  If the location of the Memory Type Information bins has already
> > been
> > >> +  established or the size of the range provides is smaller than all
> > the
> > >> +  Memory Type Information bins, then the range provides is not used.
> > >> +
> > >> +  @param  Start   The start address of the Memory Type
> Information
> > >> range.
> > >> +  @param  Length  The size, in bytes, of the Memory Type
> Information
> > >> range.
> > >> +**/
> > >> +VOID
> > >> +CoreSetMemoryTypeInformationRange (
> > >> +  IN EFI_PHYSICAL_ADDRESS  Start,
> > >> +  IN UINT64                Length
> > >> +  )
> > >> +{
> > >> +  EFI_PHYSICAL_ADDRESS  Top;
> > >> +  EFI_MEMORY_TYPE       Type;
> > >> +  UINTN                 Index;
> > >> +  UINTN                 Size;
> > >> +
> > >> +  //
> > >> +  // Return if Memory Type Information bin locations have already
> > been
> > > set
> > >> +  //
> > >> +  if (mMemoryTypeInformationInitialized) {
> > >> +    return;
> > >> +  }
> > >> +
> > >> +  //
> > >> +  // Return if size of the Memory Type Information bins is greater
> > than
> > >> Length
> > >> +  //
> > >> +  Size = 0;
> > >> +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
> > >> EfiMaxMemoryType; Index++) {
> > >> +    //
> > >> +    // Make sure the memory type in the gMemoryTypeInformation[]
> > array
> > >> is valid
> > >> +    //
> > >> +    Type =
> (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> > >> +    if ((UINT32)Type > EfiMaxMemoryType) {
> > >> +      continue;
> > >> +    }
> > >> +
> > >> +    Size += EFI_PAGES_TO_SIZE
> > >> (gMemoryTypeInformation[Index].NumberOfPages);
> > >> +  }
> > >> +
> > >> +  if (Size > Length) {
> > >> +    return;
> > >> +  }
> > >> +
> > >> +  //
> > >> +  // Loop through each memory type in the order specified by the
> > >> +  // gMemoryTypeInformation[] array
> > >> +  //
> > >> +  Top = Start + Length;
> > >> +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
> > >> EfiMaxMemoryType; Index++) {
> > >> +    //
> > >> +    // Make sure the memory type in the gMemoryTypeInformation[]
> > array
> > >> is valid
> > >> +    //
> > >> +    Type =
> (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> > >> +    if ((UINT32)Type > EfiMaxMemoryType) {
> > >> +      continue;
> > >> +    }
> > >> +
> > >> +    if (gMemoryTypeInformation[Index].NumberOfPages != 0) {
> > >> +      mMemoryTypeStatistics[Type].MaximumAddress = Top - 1;
> > >> +      Top                                       -=
> > >> EFI_PAGES_TO_SIZE
> (gMemoryTypeInformation[Index].NumberOfPages);
> > >> +      mMemoryTypeStatistics[Type].BaseAddress    = Top;
> > >> +
> > >> +      //
> > >> +      // If the current base address is the lowest address so far,
> > then
> > >> update
> > >> +      // the default maximum address
> > >> +      //
> > >> +      if (mMemoryTypeStatistics[Type].BaseAddress <
> > >> mDefaultMaximumAddress) {
> > >> +        mDefaultMaximumAddress =
> > >> mMemoryTypeStatistics[Type].BaseAddress - 1;
> > >> +      }
> > >> +
> > >> +      mMemoryTypeStatistics[Type].NumberOfPages   =
> > >> gMemoryTypeInformation[Index].NumberOfPages;
> > >> +      gMemoryTypeInformation[Index].NumberOfPages = 0;
> > >> +    }
> > >> +  }
> > >> +
> > >> +  //
> > >> +  // If the number of pages reserved for a memory type is 0, then
> > all
> > >> +  // allocations for that type should be in the default range.
> > >> +  //
> > >> +  for (Type = (EFI_MEMORY_TYPE)0; Type < EfiMaxMemoryType;
> Type++) {
> > >> +    for (Index = 0; gMemoryTypeInformation[Index].Type !=
> > >> EfiMaxMemoryType; Index++) {
> > >> +      if (Type ==
> > >> (EFI_MEMORY_TYPE)gMemoryTypeInformation[Index].Type) {
> > >> +        mMemoryTypeStatistics[Type].InformationIndex = Index;
> > >> +      }
> > >> +    }
> > >> +
> > >> +    mMemoryTypeStatistics[Type].CurrentNumberOfPages = 0;
> > >> +    if (mMemoryTypeStatistics[Type].MaximumAddress ==
> > >> MAX_ALLOC_ADDRESS) {
> > >> +      mMemoryTypeStatistics[Type].MaximumAddress =
> > >> mDefaultMaximumAddress;
> > >> +    }
> > >> +  }
> > >> +
> > >> +  mMemoryTypeInformationInitialized = TRUE;
> > >> +}
> > >> +
> > >>  /**
> > >>    Called to initialize the memory map and add descriptors to
> > >>    the current descriptor list.
> > >> --
> > >> 2.40.1.windows.1
> > >
> > >
> > >
> > >
> > >
> > > 
> > >
> > >





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114358): https://edk2.groups.io/g/devel/message/114358
Mute This Topic: https://groups.io/mt/103945537/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] 回复: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB
  2024-01-25  1:18       ` 回复: " gaoliming via groups.io
@ 2024-01-25  1:39         ` Michael D Kinney
  2024-01-26 15:50           ` 回复: " gaoliming via groups.io
  0 siblings, 1 reply; 12+ messages in thread
From: Michael D Kinney @ 2024-01-25  1:39 UTC (permalink / raw)
  To: devel@edk2.groups.io, gaoliming@byosoft.com.cn,
	'Laszlo Ersek'
  Cc: Li, Aaron, Liu, Yun Y, 'Andrew Fish', Kinney, Michael D

Hi Liming,

The exact range that may or may not support S4 resume may be OS dependent.
I am not sure.

In general, a UEFI OS should not be dependent on specific UEFI memory
types being above or below specific addresses.

The only S4 requirement I know of is landing the memory types at the
same address.

If you are aware of additional constraints for specific OSes, then
that is good information, but not sure where to put it.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
> via groups.io
> Sent: Wednesday, January 24, 2024 5:19 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; 'Laszlo Ersek'
> <lersek@redhat.com>; devel@edk2.groups.io
> Cc: Li, Aaron <aaron.li@intel.com>; Liu, Yun Y <yun.y.liu@intel.com>;
> 'Andrew Fish' <afish@apple.com>
> Subject: 回复: [edk2-devel] 回复: [Patch v2 1/1] MdeModulePkg/Core/Dxe:
> Set MemoryTypeInfo bin range from HOB
> 
> Mike:
>   I suggest to document the recommended usage of the specified memory
> type info bin range. I worry that someone configures the low memory
> range for the memory type info bin range. It may still cause S4 failure.
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Kinney, Michael D <michael.d.kinney@intel.com>
> > 发送时间: 2024年1月25日 1:53
> > 收件人: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io;
> > gaoliming@byosoft.com.cn
> > 抄送: Li, Aaron <aaron.li@intel.com>; Liu, Yun Y
> <yun.y.liu@intel.com>;
> > 'Andrew Fish' <afish@apple.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > 主题: RE: [edk2-devel] 回复: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set
> > MemoryTypeInfo bin range from HOB
> >
> > Hi Liming,
> >
> > The current algorithm allocates the bins from the top of the first
> > memory range given to the DXE Core.  This is not guaranteed to be
> > the top of RAM.  The heuristic will use the memory range from the
> > PHIT HOB if it is considered big enough.  If PHIT is not big enough
> > then a search of the HOBs is made for one that is considered big
> > enough near the top of memory that is tested.
> >
> > Additional memory can be added or promoted from untested to tested
> > after DXE Core init and that added or promoted memory may have
> > a higher address than the initial memory given to the DXE Core.
> >
> > With current heuristics, platforms have to carefully construct the
> > HOBs in PEIM phase to get the behavior where these bins are at the
> > top of RAM.
> >
> > With this feature, it provides precise control over where the
> > bins are placed.
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Laszlo Ersek <lersek@redhat.com>
> > > Sent: Wednesday, January 24, 2024 8:17 AM
> > > To: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Kinney, Michael
> D
> > > <michael.d.kinney@intel.com>
> > > Cc: Li, Aaron <aaron.li@intel.com>; Liu, Yun Y
> <yun.y.liu@intel.com>;
> > > 'Andrew Fish' <afish@apple.com>
> > > Subject: Re: [edk2-devel] 回复: [Patch v2 1/1]
> MdeModulePkg/Core/Dxe:
> > > Set MemoryTypeInfo bin range from HOB
> > >
> > > On 1/24/24 15:59, gaoliming via groups.io wrote:
> > > > Mike:
> > > >  Current algorithm tries to reserve the top available memory for
> > > memory type
> > > > bin range.
> > > >  If the platform uses new way to describe the memory range, should
> we
> > > > suggest the rule to still
> > > >  use the top memory resource hob for the memory type bin range?
> > >
> > > How would that work, technically? If the platform specifies a
> concrete
> > > address range, how can that be reconciled with using the top of RAM?
> > >
> > > Or do you mean documentation? I.e., a note for platform implementors
> > > that they should point their new HOB to the top of RAM?
> > >
> > > Thanks
> > > Laszlo
> > >
> > > >
> > > > Thanks
> > > > Liming
> > > >> -----邮件原件-----
> > > >> 发件人: Michael D Kinney <michael.d.kinney@intel.com>
> > > >> 发送时间: 2024年1月24日 4:24
> > > >> 收件人: devel@edk2.groups.io
> > > >> 抄送: Liming Gao <gaoliming@byosoft.com.cn>; Aaron Li
> > > >> <aaron.li@intel.com>; Liu Yun <yun.y.liu@intel.com>; Andrew Fish
> > > >> <afish@apple.com>
> > > >> 主题: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo
> > bin
> > > >> range from HOB
> > > >>
> > > >> Provide an optional method for PEI to declare a specific address
> > > >> range to use for the Memory Type Information bins. The current
> > > >> algorithm uses heuristics that tends to place the Memory Type
> > > >> Information bins in the same location, but memory configuration
> > > >> changes across boots or algorithm changes across a firmware
> > > >> updates could potentially change the Memory Type Information bin
> > > >> location.
> > > >>
> > > >> If the HOB List contains a Resource Descriptor HOB that
> > > >> describes tested system memory and has an Owner GUID of
> > > >> gEfiMemoryTypeInformationGuid, then use the address range
> > > >> described by the Resource Descriptor HOB as the preferred
> > > >> location of the Memory Type Information bins. If this HOB is
> > > >> not detected, then the current behavior is preserved.
> > > >>
> > > >> The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
> > > >> is ignored for the following conditions:
> > > >> * The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
> > > >>   is smaller than the Memory Type Information bins.
> > > >> * The HOB list contains more than one Resource Descriptor HOB
> > > >>   with an owner GUID of gEfiMemoryTypeInformationGuid.
> > > >> * The Resource Descriptor HOB with an Owner GUID of
> > > >>   gEfiMemoryTypeInformationGuid is the same Resource Descriptor
> > > >>   HOB that that describes the PHIT memory range.
> > > >>
> > > >> Update the DxeMain initialization order to initialize GCD
> > > >> services before any runtime allocations are performed.  This
> > > >> is required to prevent runtime data fragmentation when the
> > > >> UEFI System Table and UEFI Runtime Service Table is allocated.
> > > >>
> > > >> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > >> Cc: Aaron Li <aaron.li@intel.com>
> > > >> Cc: Liu Yun <yun.y.liu@intel.com>
> > > >> Cc: Andrew Fish <afish@apple.com>
> > > >> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > > >> ---
> > > >>  MdeModulePkg/Core/Dxe/DxeMain.h         |   6 ++
> > > >>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c |  23 +++---
> > > >>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c         |  72
> > ++++++++++++++++-
> > > >>  MdeModulePkg/Core/Dxe/Mem/Page.c        | 101
> > > >> ++++++++++++++++++++++++
> > > >>  4 files changed, 190 insertions(+), 12 deletions(-)
> > > >>
> > > >> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
> > > >> b/MdeModulePkg/Core/Dxe/DxeMain.h
> > > >> index 86a7be2f5188..53e26703f8c7 100644
> > > >> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> > > >> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> > > >> @@ -277,6 +277,12 @@ CoreInitializePool (
> > > >>    VOID
> > > >>    );
> > > >>
> > > >> +VOID
> > > >> +CoreSetMemoryTypeInformationRange (
> > > >> +  IN EFI_PHYSICAL_ADDRESS  Start,
> > > >> +  IN UINT64                Length
> > > >> +  );
> > > >> +
> > > >>  /**
> > > >>    Called to initialize the memory map and add descriptors to
> > > >>    the current descriptor list.
> > > >> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > > >> b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > > >> index 0e0f9769b99d..17d510a287e5 100644
> > > >> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > > >> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > > >> @@ -276,6 +276,18 @@ DxeMain (
> > > >>
> > > >>    MemoryProfileInit (HobStart);
> > > >>
> > > >> +  //
> > > >> +  // Start the Image Services.
> > > >> +  //
> > > >> +  Status = CoreInitializeImageServices (HobStart);
> > > >> +  ASSERT_EFI_ERROR (Status);
> > > >> +
> > > >> +  //
> > > >> +  // Initialize the Global Coherency Domain Services
> > > >> +  //
> > > >> +  Status = CoreInitializeGcdServices (&HobStart,
> MemoryBaseAddress,
> > > >> MemoryLength);
> > > >> +  ASSERT_EFI_ERROR (Status);
> > > >> +
> > > >>    //
> > > >>    // Allocate the EFI System Table and EFI Runtime Service Table
> > > from
> > > >> EfiRuntimeServicesData
> > > >>    // Use the templates to initialize the contents of the EFI
> System
> > > Table
> > > > and
> > > >> EFI Runtime Services Table
> > > >> @@ -289,16 +301,9 @@ DxeMain (
> > > >>    gDxeCoreST->RuntimeServices = gDxeCoreRT;
> > > >>
> > > >>    //
> > > >> -  // Start the Image Services.
> > > >> +  // Update DXE Core Loaded Image Protocol with allocated UEFI
> > > System
> > > >> Table
> > > >>    //
> > > >> -  Status = CoreInitializeImageServices (HobStart);
> > > >> -  ASSERT_EFI_ERROR (Status);
> > > >> -
> > > >> -  //
> > > >> -  // Initialize the Global Coherency Domain Services
> > > >> -  //
> > > >> -  Status = CoreInitializeGcdServices (&HobStart,
> MemoryBaseAddress,
> > > >> MemoryLength);
> > > >> -  ASSERT_EFI_ERROR (Status);
> > > >> +  gDxeCoreLoadedImage->SystemTable = gDxeCoreST;
> > > >>
> > > >>    //
> > > >>    // Call constructor for all libraries
> > > >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > > >> b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > > >> index 792cd2e0af23..c450d1bf2531 100644
> > > >> --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > > >> +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > > >> @@ -2238,6 +2238,8 @@ CoreInitializeMemoryServices (
> > > >>    EFI_HOB_HANDOFF_INFO_TABLE   *PhitHob;
> > > >>    EFI_HOB_RESOURCE_DESCRIPTOR  *ResourceHob;
> > > >>    EFI_HOB_RESOURCE_DESCRIPTOR  *PhitResourceHob;
> > > >> +  EFI_HOB_RESOURCE_DESCRIPTOR
> > > >> *MemoryTypeInformationResourceHob;
> > > >> +  UINTN                        Count;
> > > >>    EFI_PHYSICAL_ADDRESS         BaseAddress;
> > > >>    UINT64                       Length;
> > > >>    UINT64                       Attributes;
> > > >> @@ -2289,12 +2291,47 @@ CoreInitializeMemoryServices (
> > > >>    //
> > > >>    // See if a Memory Type Information HOB is available
> > > >>    //
> > > >> -  GuidHob = GetFirstGuidHob (&gEfiMemoryTypeInformationGuid);
> > > >> +  MemoryTypeInformationResourceHob = NULL;
> > > >> +  GuidHob                          = GetFirstGuidHob
> > > >> (&gEfiMemoryTypeInformationGuid);
> > > >>    if (GuidHob != NULL) {
> > > >>      EfiMemoryTypeInformation = GET_GUID_HOB_DATA (GuidHob);
> > > >>      DataSize                 = GET_GUID_HOB_DATA_SIZE
> > (GuidHob);
> > > >>      if ((EfiMemoryTypeInformation != NULL) && (DataSize > 0) &&
> > > (DataSize
> > > >> <= (EfiMaxMemoryType + 1) * sizeof
> > (EFI_MEMORY_TYPE_INFORMATION))) {
> > > >>        CopyMem (&gMemoryTypeInformation,
> > EfiMemoryTypeInformation,
> > > >> DataSize);
> > > >> +
> > > >> +      //
> > > >> +      // Look for Resource Descriptor HOB with a ResourceType of
> > > System
> > > >> Memory
> > > >> +      // and an Owner GUID of gEfiMemoryTypeInformationGuid. If
> > more
> > > >> than 1 is
> > > >> +      // found, then set MemoryTypeInformationResourceHob to
> > NULL.
> > > >> +      //
> > > >> +      Count = 0;
> > > >> +      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw
> > =
> > > >> GET_NEXT_HOB (Hob)) {
> > > >> +        if (GET_HOB_TYPE (Hob) !=
> > > >> EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
> > > >> +          continue;
> > > >> +        }
> > > >> +
> > > >> +        ResourceHob = Hob.ResourceDescriptor;
> > > >> +        if (!CompareGuid (&ResourceHob->Owner,
> > > >> &gEfiMemoryTypeInformationGuid)) {
> > > >> +          continue;
> > > >> +        }
> > > >> +
> > > >> +        Count++;
> > > >> +        if (ResourceHob->ResourceType !=
> > > >> EFI_RESOURCE_SYSTEM_MEMORY) {
> > > >> +          continue;
> > > >> +        }
> > > >> +
> > > >> +        if ((ResourceHob->ResourceAttribute &
> > > >> MEMORY_ATTRIBUTE_MASK) != TESTED_MEMORY_ATTRIBUTES) {
> > > >> +          continue;
> > > >> +        }
> > > >> +
> > > >> +        if (ResourceHob->ResourceLength >=
> > > >> CalculateTotalMemoryBinSizeNeeded ()) {
> > > >> +          MemoryTypeInformationResourceHob = ResourceHob;
> > > >> +        }
> > > >> +      }
> > > >> +
> > > >> +      if (Count > 1) {
> > > >> +        MemoryTypeInformationResourceHob = NULL;
> > > >> +      }
> > > >>      }
> > > >>    }
> > > >>
> > > >> @@ -2344,6 +2381,15 @@ CoreInitializeMemoryServices (
> > > >>      PhitResourceHob = ResourceHob;
> > > >>      Found           = TRUE;
> > > >>
> > > >> +    //
> > > >> +    // If a Memory Type Information Resource HOB was found and
> is
> > > the
> > > >> same
> > > >> +    // Resource HOB that describes the PHIT HOB, then ignore the
> > > Memory
> > > >> Type
> > > >> +    // Information Resource HOB.
> > > >> +    //
> > > >> +    if (MemoryTypeInformationResourceHob == PhitResourceHob) {
> > > >> +      MemoryTypeInformationResourceHob = NULL;
> > > >> +    }
> > > >> +
> > > >>      //
> > > >>      // Compute range between PHIT EfiMemoryTop and the end of
> > the
> > > >> Resource Descriptor HOB
> > > >>      //
> > > >> @@ -2387,8 +2433,9 @@ CoreInitializeMemoryServices (
> > > >>    if (Length < MinimalMemorySizeNeeded) {
> > > >>      //
> > > >>      // Search all the resource descriptor HOBs from the highest
> > > possible
> > > >> addresses down for a memory
> > > >> -    // region that is big enough to initialize the DXE core.
> Always
> > > skip
> > > > the
> > > >> PHIT Resource HOB.
> > > >> -    // The max address must be within the physically addressible
> > > range
> > > > for
> > > >> the processor.
> > > >> +    // region that is big enough to initialize the DXE core.
> Always
> > > skip
> > > > the
> > > >> PHIT Resource HOB
> > > >> +    // and the Memory Type Information Resource HOB. The max
> > address
> > > >> must be within the physically
> > > >> +    // addressable range for the processor.
> > > >>      //
> > > >>      HighAddress = MAX_ALLOC_ADDRESS;
> > > >>      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob); Hob.Raw =
> > > >> GET_NEXT_HOB (Hob)) {
> > > >> @@ -2399,6 +2446,13 @@ CoreInitializeMemoryServices (
> > > >>          continue;
> > > >>        }
> > > >>
> > > >> +      //
> > > >> +      // Skip the Resource Descriptor HOB that contains Memory
> Type
> > > >> Information bins
> > > >> +      //
> > > >> +      if (Hob.ResourceDescriptor ==
> > > MemoryTypeInformationResourceHob)
> > > >> {
> > > >> +        continue;
> > > >> +      }
> > > >> +
> > > >>        //
> > > >>        // Skip all HOBs except Resource Descriptor HOBs
> > > >>        //
> > > >> @@ -2466,6 +2520,18 @@ CoreInitializeMemoryServices (
> > > >>      Capabilities =
> > > >> CoreConvertResourceDescriptorHobAttributesToCapabilities
> > > >> (EfiGcdMemoryTypeSystemMemory, Attributes);
> > > >>    }
> > > >>
> > > >> +  if (MemoryTypeInformationResourceHob != NULL) {
> > > >> +    //
> > > >> +    // If a Memory Type Information Resource HOB was found, then
> > use
> > > the
> > > >> address
> > > >> +    // range of the  Memory Type Information Resource HOB as the
> > > >> preferred
> > > >> +    // address range for the Memory Type Information bins.
> > > >> +    //
> > > >> +    CoreSetMemoryTypeInformationRange (
> > > >> +      MemoryTypeInformationResourceHob->PhysicalStart,
> > > >> +      MemoryTypeInformationResourceHob->ResourceLength
> > > >> +      );
> > > >> +  }
> > > >> +
> > > >>    //
> > > >>    // Declare the very first memory region, so the EFI Memory
> > > Services are
> > > >> available.
> > > >>    //
> > > >> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> > > >> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > > >> index 6497af573353..458c62090265 100644
> > > >> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> > > >> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > > >> @@ -532,6 +532,107 @@ CoreLoadingFixedAddressHook (
> > > >>    return;
> > > >>  }
> > > >>
> > > >> +/**
> > > >> +  Sets the preferred memory range to use for the Memory Type
> > > Information
> > > >> bins.
> > > >> +  This service must be called before fist call to
> > > > CoreAddMemoryDescriptor().
> > > >> +
> > > >> +  If the location of the Memory Type Information bins has
> already
> > > been
> > > >> +  established or the size of the range provides is smaller than
> all
> > > the
> > > >> +  Memory Type Information bins, then the range provides is not
> used.
> > > >> +
> > > >> +  @param  Start   The start address of the Memory Type
> > Information
> > > >> range.
> > > >> +  @param  Length  The size, in bytes, of the Memory Type
> > Information
> > > >> range.
> > > >> +**/
> > > >> +VOID
> > > >> +CoreSetMemoryTypeInformationRange (
> > > >> +  IN EFI_PHYSICAL_ADDRESS  Start,
> > > >> +  IN UINT64                Length
> > > >> +  )
> > > >> +{
> > > >> +  EFI_PHYSICAL_ADDRESS  Top;
> > > >> +  EFI_MEMORY_TYPE       Type;
> > > >> +  UINTN                 Index;
> > > >> +  UINTN                 Size;
> > > >> +
> > > >> +  //
> > > >> +  // Return if Memory Type Information bin locations have
> already
> > > been
> > > > set
> > > >> +  //
> > > >> +  if (mMemoryTypeInformationInitialized) {
> > > >> +    return;
> > > >> +  }
> > > >> +
> > > >> +  //
> > > >> +  // Return if size of the Memory Type Information bins is
> greater
> > > than
> > > >> Length
> > > >> +  //
> > > >> +  Size = 0;
> > > >> +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
> > > >> EfiMaxMemoryType; Index++) {
> > > >> +    //
> > > >> +    // Make sure the memory type in the gMemoryTypeInformation[]
> > > array
> > > >> is valid
> > > >> +    //
> > > >> +    Type =
> > (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> > > >> +    if ((UINT32)Type > EfiMaxMemoryType) {
> > > >> +      continue;
> > > >> +    }
> > > >> +
> > > >> +    Size += EFI_PAGES_TO_SIZE
> > > >> (gMemoryTypeInformation[Index].NumberOfPages);
> > > >> +  }
> > > >> +
> > > >> +  if (Size > Length) {
> > > >> +    return;
> > > >> +  }
> > > >> +
> > > >> +  //
> > > >> +  // Loop through each memory type in the order specified by the
> > > >> +  // gMemoryTypeInformation[] array
> > > >> +  //
> > > >> +  Top = Start + Length;
> > > >> +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
> > > >> EfiMaxMemoryType; Index++) {
> > > >> +    //
> > > >> +    // Make sure the memory type in the gMemoryTypeInformation[]
> > > array
> > > >> is valid
> > > >> +    //
> > > >> +    Type =
> > (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> > > >> +    if ((UINT32)Type > EfiMaxMemoryType) {
> > > >> +      continue;
> > > >> +    }
> > > >> +
> > > >> +    if (gMemoryTypeInformation[Index].NumberOfPages != 0) {
> > > >> +      mMemoryTypeStatistics[Type].MaximumAddress = Top - 1;
> > > >> +      Top                                       -=
> > > >> EFI_PAGES_TO_SIZE
> > (gMemoryTypeInformation[Index].NumberOfPages);
> > > >> +      mMemoryTypeStatistics[Type].BaseAddress    = Top;
> > > >> +
> > > >> +      //
> > > >> +      // If the current base address is the lowest address so
> far,
> > > then
> > > >> update
> > > >> +      // the default maximum address
> > > >> +      //
> > > >> +      if (mMemoryTypeStatistics[Type].BaseAddress <
> > > >> mDefaultMaximumAddress) {
> > > >> +        mDefaultMaximumAddress =
> > > >> mMemoryTypeStatistics[Type].BaseAddress - 1;
> > > >> +      }
> > > >> +
> > > >> +      mMemoryTypeStatistics[Type].NumberOfPages   =
> > > >> gMemoryTypeInformation[Index].NumberOfPages;
> > > >> +      gMemoryTypeInformation[Index].NumberOfPages = 0;
> > > >> +    }
> > > >> +  }
> > > >> +
> > > >> +  //
> > > >> +  // If the number of pages reserved for a memory type is 0,
> then
> > > all
> > > >> +  // allocations for that type should be in the default range.
> > > >> +  //
> > > >> +  for (Type = (EFI_MEMORY_TYPE)0; Type < EfiMaxMemoryType;
> > Type++) {
> > > >> +    for (Index = 0; gMemoryTypeInformation[Index].Type !=
> > > >> EfiMaxMemoryType; Index++) {
> > > >> +      if (Type ==
> > > >> (EFI_MEMORY_TYPE)gMemoryTypeInformation[Index].Type) {
> > > >> +        mMemoryTypeStatistics[Type].InformationIndex = Index;
> > > >> +      }
> > > >> +    }
> > > >> +
> > > >> +    mMemoryTypeStatistics[Type].CurrentNumberOfPages = 0;
> > > >> +    if (mMemoryTypeStatistics[Type].MaximumAddress ==
> > > >> MAX_ALLOC_ADDRESS) {
> > > >> +      mMemoryTypeStatistics[Type].MaximumAddress =
> > > >> mDefaultMaximumAddress;
> > > >> +    }
> > > >> +  }
> > > >> +
> > > >> +  mMemoryTypeInformationInitialized = TRUE;
> > > >> +}
> > > >> +
> > > >>  /**
> > > >>    Called to initialize the memory map and add descriptors to
> > > >>    the current descriptor list.
> > > >> --
> > > >> 2.40.1.windows.1
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> 
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114360): https://edk2.groups.io/g/devel/message/114360
Mute This Topic: https://groups.io/mt/103945844/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB
  2024-01-24 17:46   ` Michael D Kinney
@ 2024-01-25 21:34     ` Laszlo Ersek
  0 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2024-01-25 21:34 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io
  Cc: Liming Gao, Li, Aaron, Liu, Yun Y, Andrew Fish

On 1/24/24 18:46, Kinney, Michael D wrote:
> Hi Laszlo,
> 
> Yes.  I can add more details in the commit message.
> 
> The impact is for ACPI S4.  There are many reasons why the set of
> HOBs passed into the DXE Core may change from boot to boot or that
> allocations in the early DXE init phase should change where the
> memory type information bins are allocated.
> 
> ACPI S4 does do a power cycle and it is possible to do FW updates
> or FW setup config changes between an S4 save and S4 resume operation.
> 
> It is even possible for one OS to do S4 save.  Reboot the system
> to boot a completely different OS.  Reboot again and do an S4 resume
> of the original OS.
> 
> If a platform chooses to enable this feature, the number of scenarios
> that an S4 resume can fail is reduced.

Awesome info, I couldn't have imagined. (I never use, or develop for,
ACPI S4.) Thanks!

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114461): https://edk2.groups.io/g/devel/message/114461
Mute This Topic: https://groups.io/mt/103918464/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* 回复: [edk2-devel] 回复: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB
  2024-01-25  1:39         ` Michael D Kinney
@ 2024-01-26 15:50           ` gaoliming via groups.io
  0 siblings, 0 replies; 12+ messages in thread
From: gaoliming via groups.io @ 2024-01-26 15:50 UTC (permalink / raw)
  To: 'Kinney, Michael D', devel, 'Laszlo Ersek'
  Cc: 'Li, Aaron', 'Liu, Yun Y', 'Andrew Fish'

Mike:
  OK. I agree this patch can assure the memory type be always allocated at the same address. I have no comments for this change.  

Thanks
Liming
> -----邮件原件-----
> 发件人: Kinney, Michael D <michael.d.kinney@intel.com>
> 发送时间: 2024年1月25日 9:39
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; 'Laszlo Ersek'
> <lersek@redhat.com>
> 抄送: Li, Aaron <aaron.li@intel.com>; Liu, Yun Y <yun.y.liu@intel.com>;
> 'Andrew Fish' <afish@apple.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> 主题: RE: [edk2-devel] 回复: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set
> MemoryTypeInfo bin range from HOB
> 
> Hi Liming,
> 
> The exact range that may or may not support S4 resume may be OS
> dependent.
> I am not sure.
> 
> In general, a UEFI OS should not be dependent on specific UEFI memory
> types being above or below specific addresses.
> 
> The only S4 requirement I know of is landing the memory types at the
> same address.
> 
> If you are aware of additional constraints for specific OSes, then
> that is good information, but not sure where to put it.
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> gaoliming
> > via groups.io
> > Sent: Wednesday, January 24, 2024 5:19 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>; 'Laszlo Ersek'
> > <lersek@redhat.com>; devel@edk2.groups.io
> > Cc: Li, Aaron <aaron.li@intel.com>; Liu, Yun Y <yun.y.liu@intel.com>;
> > 'Andrew Fish' <afish@apple.com>
> > Subject: 回复: [edk2-devel] 回复: [Patch v2 1/1]
> MdeModulePkg/Core/Dxe:
> > Set MemoryTypeInfo bin range from HOB
> >
> > Mike:
> >   I suggest to document the recommended usage of the specified memory
> > type info bin range. I worry that someone configures the low memory
> > range for the memory type info bin range. It may still cause S4 failure.
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: Kinney, Michael D <michael.d.kinney@intel.com>
> > > 发送时间: 2024年1月25日 1:53
> > > 收件人: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io;
> > > gaoliming@byosoft.com.cn
> > > 抄送: Li, Aaron <aaron.li@intel.com>; Liu, Yun Y
> > <yun.y.liu@intel.com>;
> > > 'Andrew Fish' <afish@apple.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > 主题: RE: [edk2-devel] 回复: [Patch v2 1/1] MdeModulePkg/Core/Dxe:
> Set
> > > MemoryTypeInfo bin range from HOB
> > >
> > > Hi Liming,
> > >
> > > The current algorithm allocates the bins from the top of the first
> > > memory range given to the DXE Core.  This is not guaranteed to be
> > > the top of RAM.  The heuristic will use the memory range from the
> > > PHIT HOB if it is considered big enough.  If PHIT is not big enough
> > > then a search of the HOBs is made for one that is considered big
> > > enough near the top of memory that is tested.
> > >
> > > Additional memory can be added or promoted from untested to tested
> > > after DXE Core init and that added or promoted memory may have
> > > a higher address than the initial memory given to the DXE Core.
> > >
> > > With current heuristics, platforms have to carefully construct the
> > > HOBs in PEIM phase to get the behavior where these bins are at the
> > > top of RAM.
> > >
> > > With this feature, it provides precise control over where the
> > > bins are placed.
> > >
> > > Mike
> > >
> > > > -----Original Message-----
> > > > From: Laszlo Ersek <lersek@redhat.com>
> > > > Sent: Wednesday, January 24, 2024 8:17 AM
> > > > To: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Kinney, Michael
> > D
> > > > <michael.d.kinney@intel.com>
> > > > Cc: Li, Aaron <aaron.li@intel.com>; Liu, Yun Y
> > <yun.y.liu@intel.com>;
> > > > 'Andrew Fish' <afish@apple.com>
> > > > Subject: Re: [edk2-devel] 回复: [Patch v2 1/1]
> > MdeModulePkg/Core/Dxe:
> > > > Set MemoryTypeInfo bin range from HOB
> > > >
> > > > On 1/24/24 15:59, gaoliming via groups.io wrote:
> > > > > Mike:
> > > > >  Current algorithm tries to reserve the top available memory for
> > > > memory type
> > > > > bin range.
> > > > >  If the platform uses new way to describe the memory range, should
> > we
> > > > > suggest the rule to still
> > > > >  use the top memory resource hob for the memory type bin range?
> > > >
> > > > How would that work, technically? If the platform specifies a
> > concrete
> > > > address range, how can that be reconciled with using the top of RAM?
> > > >
> > > > Or do you mean documentation? I.e., a note for platform implementors
> > > > that they should point their new HOB to the top of RAM?
> > > >
> > > > Thanks
> > > > Laszlo
> > > >
> > > > >
> > > > > Thanks
> > > > > Liming
> > > > >> -----邮件原件-----
> > > > >> 发件人: Michael D Kinney <michael.d.kinney@intel.com>
> > > > >> 发送时间: 2024年1月24日 4:24
> > > > >> 收件人: devel@edk2.groups.io
> > > > >> 抄送: Liming Gao <gaoliming@byosoft.com.cn>; Aaron Li
> > > > >> <aaron.li@intel.com>; Liu Yun <yun.y.liu@intel.com>; Andrew Fish
> > > > >> <afish@apple.com>
> > > > >> 主题: [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set
> MemoryTypeInfo
> > > bin
> > > > >> range from HOB
> > > > >>
> > > > >> Provide an optional method for PEI to declare a specific address
> > > > >> range to use for the Memory Type Information bins. The current
> > > > >> algorithm uses heuristics that tends to place the Memory Type
> > > > >> Information bins in the same location, but memory configuration
> > > > >> changes across boots or algorithm changes across a firmware
> > > > >> updates could potentially change the Memory Type Information bin
> > > > >> location.
> > > > >>
> > > > >> If the HOB List contains a Resource Descriptor HOB that
> > > > >> describes tested system memory and has an Owner GUID of
> > > > >> gEfiMemoryTypeInformationGuid, then use the address range
> > > > >> described by the Resource Descriptor HOB as the preferred
> > > > >> location of the Memory Type Information bins. If this HOB is
> > > > >> not detected, then the current behavior is preserved.
> > > > >>
> > > > >> The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
> > > > >> is ignored for the following conditions:
> > > > >> * The HOB with an Owner GUID of gEfiMemoryTypeInformationGuid
> > > > >>   is smaller than the Memory Type Information bins.
> > > > >> * The HOB list contains more than one Resource Descriptor HOB
> > > > >>   with an owner GUID of gEfiMemoryTypeInformationGuid.
> > > > >> * The Resource Descriptor HOB with an Owner GUID of
> > > > >>   gEfiMemoryTypeInformationGuid is the same Resource Descriptor
> > > > >>   HOB that that describes the PHIT memory range.
> > > > >>
> > > > >> Update the DxeMain initialization order to initialize GCD
> > > > >> services before any runtime allocations are performed.  This
> > > > >> is required to prevent runtime data fragmentation when the
> > > > >> UEFI System Table and UEFI Runtime Service Table is allocated.
> > > > >>
> > > > >> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > > >> Cc: Aaron Li <aaron.li@intel.com>
> > > > >> Cc: Liu Yun <yun.y.liu@intel.com>
> > > > >> Cc: Andrew Fish <afish@apple.com>
> > > > >> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > > > >> ---
> > > > >>  MdeModulePkg/Core/Dxe/DxeMain.h         |   6 ++
> > > > >>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c |  23 +++---
> > > > >>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c         |  72
> > > ++++++++++++++++-
> > > > >>  MdeModulePkg/Core/Dxe/Mem/Page.c        | 101
> > > > >> ++++++++++++++++++++++++
> > > > >>  4 files changed, 190 insertions(+), 12 deletions(-)
> > > > >>
> > > > >> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
> > > > >> b/MdeModulePkg/Core/Dxe/DxeMain.h
> > > > >> index 86a7be2f5188..53e26703f8c7 100644
> > > > >> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> > > > >> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> > > > >> @@ -277,6 +277,12 @@ CoreInitializePool (
> > > > >>    VOID
> > > > >>    );
> > > > >>
> > > > >> +VOID
> > > > >> +CoreSetMemoryTypeInformationRange (
> > > > >> +  IN EFI_PHYSICAL_ADDRESS  Start,
> > > > >> +  IN UINT64                Length
> > > > >> +  );
> > > > >> +
> > > > >>  /**
> > > > >>    Called to initialize the memory map and add descriptors to
> > > > >>    the current descriptor list.
> > > > >> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > > > >> b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > > > >> index 0e0f9769b99d..17d510a287e5 100644
> > > > >> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > > > >> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > > > >> @@ -276,6 +276,18 @@ DxeMain (
> > > > >>
> > > > >>    MemoryProfileInit (HobStart);
> > > > >>
> > > > >> +  //
> > > > >> +  // Start the Image Services.
> > > > >> +  //
> > > > >> +  Status = CoreInitializeImageServices (HobStart);
> > > > >> +  ASSERT_EFI_ERROR (Status);
> > > > >> +
> > > > >> +  //
> > > > >> +  // Initialize the Global Coherency Domain Services
> > > > >> +  //
> > > > >> +  Status = CoreInitializeGcdServices (&HobStart,
> > MemoryBaseAddress,
> > > > >> MemoryLength);
> > > > >> +  ASSERT_EFI_ERROR (Status);
> > > > >> +
> > > > >>    //
> > > > >>    // Allocate the EFI System Table and EFI Runtime Service Table
> > > > from
> > > > >> EfiRuntimeServicesData
> > > > >>    // Use the templates to initialize the contents of the EFI
> > System
> > > > Table
> > > > > and
> > > > >> EFI Runtime Services Table
> > > > >> @@ -289,16 +301,9 @@ DxeMain (
> > > > >>    gDxeCoreST->RuntimeServices = gDxeCoreRT;
> > > > >>
> > > > >>    //
> > > > >> -  // Start the Image Services.
> > > > >> +  // Update DXE Core Loaded Image Protocol with allocated UEFI
> > > > System
> > > > >> Table
> > > > >>    //
> > > > >> -  Status = CoreInitializeImageServices (HobStart);
> > > > >> -  ASSERT_EFI_ERROR (Status);
> > > > >> -
> > > > >> -  //
> > > > >> -  // Initialize the Global Coherency Domain Services
> > > > >> -  //
> > > > >> -  Status = CoreInitializeGcdServices (&HobStart,
> > MemoryBaseAddress,
> > > > >> MemoryLength);
> > > > >> -  ASSERT_EFI_ERROR (Status);
> > > > >> +  gDxeCoreLoadedImage->SystemTable = gDxeCoreST;
> > > > >>
> > > > >>    //
> > > > >>    // Call constructor for all libraries
> > > > >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > > > >> b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > > > >> index 792cd2e0af23..c450d1bf2531 100644
> > > > >> --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > > > >> +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> > > > >> @@ -2238,6 +2238,8 @@ CoreInitializeMemoryServices (
> > > > >>    EFI_HOB_HANDOFF_INFO_TABLE   *PhitHob;
> > > > >>    EFI_HOB_RESOURCE_DESCRIPTOR  *ResourceHob;
> > > > >>    EFI_HOB_RESOURCE_DESCRIPTOR  *PhitResourceHob;
> > > > >> +  EFI_HOB_RESOURCE_DESCRIPTOR
> > > > >> *MemoryTypeInformationResourceHob;
> > > > >> +  UINTN                        Count;
> > > > >>    EFI_PHYSICAL_ADDRESS         BaseAddress;
> > > > >>    UINT64                       Length;
> > > > >>    UINT64                       Attributes;
> > > > >> @@ -2289,12 +2291,47 @@ CoreInitializeMemoryServices (
> > > > >>    //
> > > > >>    // See if a Memory Type Information HOB is available
> > > > >>    //
> > > > >> -  GuidHob = GetFirstGuidHob (&gEfiMemoryTypeInformationGuid);
> > > > >> +  MemoryTypeInformationResourceHob = NULL;
> > > > >> +  GuidHob                          = GetFirstGuidHob
> > > > >> (&gEfiMemoryTypeInformationGuid);
> > > > >>    if (GuidHob != NULL) {
> > > > >>      EfiMemoryTypeInformation = GET_GUID_HOB_DATA
> (GuidHob);
> > > > >>      DataSize                 = GET_GUID_HOB_DATA_SIZE
> > > (GuidHob);
> > > > >>      if ((EfiMemoryTypeInformation != NULL) && (DataSize > 0) &&
> > > > (DataSize
> > > > >> <= (EfiMaxMemoryType + 1) * sizeof
> > > (EFI_MEMORY_TYPE_INFORMATION))) {
> > > > >>        CopyMem (&gMemoryTypeInformation,
> > > EfiMemoryTypeInformation,
> > > > >> DataSize);
> > > > >> +
> > > > >> +      //
> > > > >> +      // Look for Resource Descriptor HOB with a ResourceType of
> > > > System
> > > > >> Memory
> > > > >> +      // and an Owner GUID of gEfiMemoryTypeInformationGuid.
> If
> > > more
> > > > >> than 1 is
> > > > >> +      // found, then set MemoryTypeInformationResourceHob to
> > > NULL.
> > > > >> +      //
> > > > >> +      Count = 0;
> > > > >> +      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob);
> Hob.Raw
> > > =
> > > > >> GET_NEXT_HOB (Hob)) {
> > > > >> +        if (GET_HOB_TYPE (Hob) !=
> > > > >> EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) {
> > > > >> +          continue;
> > > > >> +        }
> > > > >> +
> > > > >> +        ResourceHob = Hob.ResourceDescriptor;
> > > > >> +        if (!CompareGuid (&ResourceHob->Owner,
> > > > >> &gEfiMemoryTypeInformationGuid)) {
> > > > >> +          continue;
> > > > >> +        }
> > > > >> +
> > > > >> +        Count++;
> > > > >> +        if (ResourceHob->ResourceType !=
> > > > >> EFI_RESOURCE_SYSTEM_MEMORY) {
> > > > >> +          continue;
> > > > >> +        }
> > > > >> +
> > > > >> +        if ((ResourceHob->ResourceAttribute &
> > > > >> MEMORY_ATTRIBUTE_MASK) != TESTED_MEMORY_ATTRIBUTES) {
> > > > >> +          continue;
> > > > >> +        }
> > > > >> +
> > > > >> +        if (ResourceHob->ResourceLength >=
> > > > >> CalculateTotalMemoryBinSizeNeeded ()) {
> > > > >> +          MemoryTypeInformationResourceHob = ResourceHob;
> > > > >> +        }
> > > > >> +      }
> > > > >> +
> > > > >> +      if (Count > 1) {
> > > > >> +        MemoryTypeInformationResourceHob = NULL;
> > > > >> +      }
> > > > >>      }
> > > > >>    }
> > > > >>
> > > > >> @@ -2344,6 +2381,15 @@ CoreInitializeMemoryServices (
> > > > >>      PhitResourceHob = ResourceHob;
> > > > >>      Found           = TRUE;
> > > > >>
> > > > >> +    //
> > > > >> +    // If a Memory Type Information Resource HOB was found and
> > is
> > > > the
> > > > >> same
> > > > >> +    // Resource HOB that describes the PHIT HOB, then ignore the
> > > > Memory
> > > > >> Type
> > > > >> +    // Information Resource HOB.
> > > > >> +    //
> > > > >> +    if (MemoryTypeInformationResourceHob == PhitResourceHob)
> {
> > > > >> +      MemoryTypeInformationResourceHob = NULL;
> > > > >> +    }
> > > > >> +
> > > > >>      //
> > > > >>      // Compute range between PHIT EfiMemoryTop and the end of
> > > the
> > > > >> Resource Descriptor HOB
> > > > >>      //
> > > > >> @@ -2387,8 +2433,9 @@ CoreInitializeMemoryServices (
> > > > >>    if (Length < MinimalMemorySizeNeeded) {
> > > > >>      //
> > > > >>      // Search all the resource descriptor HOBs from the highest
> > > > possible
> > > > >> addresses down for a memory
> > > > >> -    // region that is big enough to initialize the DXE core.
> > Always
> > > > skip
> > > > > the
> > > > >> PHIT Resource HOB.
> > > > >> -    // The max address must be within the physically addressible
> > > > range
> > > > > for
> > > > >> the processor.
> > > > >> +    // region that is big enough to initialize the DXE core.
> > Always
> > > > skip
> > > > > the
> > > > >> PHIT Resource HOB
> > > > >> +    // and the Memory Type Information Resource HOB. The max
> > > address
> > > > >> must be within the physically
> > > > >> +    // addressable range for the processor.
> > > > >>      //
> > > > >>      HighAddress = MAX_ALLOC_ADDRESS;
> > > > >>      for (Hob.Raw = *HobStart; !END_OF_HOB_LIST (Hob);
> Hob.Raw =
> > > > >> GET_NEXT_HOB (Hob)) {
> > > > >> @@ -2399,6 +2446,13 @@ CoreInitializeMemoryServices (
> > > > >>          continue;
> > > > >>        }
> > > > >>
> > > > >> +      //
> > > > >> +      // Skip the Resource Descriptor HOB that contains Memory
> > Type
> > > > >> Information bins
> > > > >> +      //
> > > > >> +      if (Hob.ResourceDescriptor ==
> > > > MemoryTypeInformationResourceHob)
> > > > >> {
> > > > >> +        continue;
> > > > >> +      }
> > > > >> +
> > > > >>        //
> > > > >>        // Skip all HOBs except Resource Descriptor HOBs
> > > > >>        //
> > > > >> @@ -2466,6 +2520,18 @@ CoreInitializeMemoryServices (
> > > > >>      Capabilities =
> > > > >> CoreConvertResourceDescriptorHobAttributesToCapabilities
> > > > >> (EfiGcdMemoryTypeSystemMemory, Attributes);
> > > > >>    }
> > > > >>
> > > > >> +  if (MemoryTypeInformationResourceHob != NULL) {
> > > > >> +    //
> > > > >> +    // If a Memory Type Information Resource HOB was found,
> then
> > > use
> > > > the
> > > > >> address
> > > > >> +    // range of the  Memory Type Information Resource HOB as
> the
> > > > >> preferred
> > > > >> +    // address range for the Memory Type Information bins.
> > > > >> +    //
> > > > >> +    CoreSetMemoryTypeInformationRange (
> > > > >> +      MemoryTypeInformationResourceHob->PhysicalStart,
> > > > >> +      MemoryTypeInformationResourceHob->ResourceLength
> > > > >> +      );
> > > > >> +  }
> > > > >> +
> > > > >>    //
> > > > >>    // Declare the very first memory region, so the EFI Memory
> > > > Services are
> > > > >> available.
> > > > >>    //
> > > > >> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> > > > >> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > > > >> index 6497af573353..458c62090265 100644
> > > > >> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> > > > >> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > > > >> @@ -532,6 +532,107 @@ CoreLoadingFixedAddressHook (
> > > > >>    return;
> > > > >>  }
> > > > >>
> > > > >> +/**
> > > > >> +  Sets the preferred memory range to use for the Memory Type
> > > > Information
> > > > >> bins.
> > > > >> +  This service must be called before fist call to
> > > > > CoreAddMemoryDescriptor().
> > > > >> +
> > > > >> +  If the location of the Memory Type Information bins has
> > already
> > > > been
> > > > >> +  established or the size of the range provides is smaller than
> > all
> > > > the
> > > > >> +  Memory Type Information bins, then the range provides is not
> > used.
> > > > >> +
> > > > >> +  @param  Start   The start address of the Memory Type
> > > Information
> > > > >> range.
> > > > >> +  @param  Length  The size, in bytes, of the Memory Type
> > > Information
> > > > >> range.
> > > > >> +**/
> > > > >> +VOID
> > > > >> +CoreSetMemoryTypeInformationRange (
> > > > >> +  IN EFI_PHYSICAL_ADDRESS  Start,
> > > > >> +  IN UINT64                Length
> > > > >> +  )
> > > > >> +{
> > > > >> +  EFI_PHYSICAL_ADDRESS  Top;
> > > > >> +  EFI_MEMORY_TYPE       Type;
> > > > >> +  UINTN                 Index;
> > > > >> +  UINTN                 Size;
> > > > >> +
> > > > >> +  //
> > > > >> +  // Return if Memory Type Information bin locations have
> > already
> > > > been
> > > > > set
> > > > >> +  //
> > > > >> +  if (mMemoryTypeInformationInitialized) {
> > > > >> +    return;
> > > > >> +  }
> > > > >> +
> > > > >> +  //
> > > > >> +  // Return if size of the Memory Type Information bins is
> > greater
> > > > than
> > > > >> Length
> > > > >> +  //
> > > > >> +  Size = 0;
> > > > >> +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
> > > > >> EfiMaxMemoryType; Index++) {
> > > > >> +    //
> > > > >> +    // Make sure the memory type in the
> gMemoryTypeInformation[]
> > > > array
> > > > >> is valid
> > > > >> +    //
> > > > >> +    Type =
> > > (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> > > > >> +    if ((UINT32)Type > EfiMaxMemoryType) {
> > > > >> +      continue;
> > > > >> +    }
> > > > >> +
> > > > >> +    Size += EFI_PAGES_TO_SIZE
> > > > >> (gMemoryTypeInformation[Index].NumberOfPages);
> > > > >> +  }
> > > > >> +
> > > > >> +  if (Size > Length) {
> > > > >> +    return;
> > > > >> +  }
> > > > >> +
> > > > >> +  //
> > > > >> +  // Loop through each memory type in the order specified by the
> > > > >> +  // gMemoryTypeInformation[] array
> > > > >> +  //
> > > > >> +  Top = Start + Length;
> > > > >> +  for (Index = 0; gMemoryTypeInformation[Index].Type !=
> > > > >> EfiMaxMemoryType; Index++) {
> > > > >> +    //
> > > > >> +    // Make sure the memory type in the
> gMemoryTypeInformation[]
> > > > array
> > > > >> is valid
> > > > >> +    //
> > > > >> +    Type =
> > > (EFI_MEMORY_TYPE)(gMemoryTypeInformation[Index].Type);
> > > > >> +    if ((UINT32)Type > EfiMaxMemoryType) {
> > > > >> +      continue;
> > > > >> +    }
> > > > >> +
> > > > >> +    if (gMemoryTypeInformation[Index].NumberOfPages != 0) {
> > > > >> +      mMemoryTypeStatistics[Type].MaximumAddress = Top - 1;
> > > > >> +      Top                                       -=
> > > > >> EFI_PAGES_TO_SIZE
> > > (gMemoryTypeInformation[Index].NumberOfPages);
> > > > >> +      mMemoryTypeStatistics[Type].BaseAddress    = Top;
> > > > >> +
> > > > >> +      //
> > > > >> +      // If the current base address is the lowest address so
> > far,
> > > > then
> > > > >> update
> > > > >> +      // the default maximum address
> > > > >> +      //
> > > > >> +      if (mMemoryTypeStatistics[Type].BaseAddress <
> > > > >> mDefaultMaximumAddress) {
> > > > >> +        mDefaultMaximumAddress =
> > > > >> mMemoryTypeStatistics[Type].BaseAddress - 1;
> > > > >> +      }
> > > > >> +
> > > > >> +      mMemoryTypeStatistics[Type].NumberOfPages   =
> > > > >> gMemoryTypeInformation[Index].NumberOfPages;
> > > > >> +      gMemoryTypeInformation[Index].NumberOfPages = 0;
> > > > >> +    }
> > > > >> +  }
> > > > >> +
> > > > >> +  //
> > > > >> +  // If the number of pages reserved for a memory type is 0,
> > then
> > > > all
> > > > >> +  // allocations for that type should be in the default range.
> > > > >> +  //
> > > > >> +  for (Type = (EFI_MEMORY_TYPE)0; Type < EfiMaxMemoryType;
> > > Type++) {
> > > > >> +    for (Index = 0; gMemoryTypeInformation[Index].Type !=
> > > > >> EfiMaxMemoryType; Index++) {
> > > > >> +      if (Type ==
> > > > >> (EFI_MEMORY_TYPE)gMemoryTypeInformation[Index].Type) {
> > > > >> +        mMemoryTypeStatistics[Type].InformationIndex = Index;
> > > > >> +      }
> > > > >> +    }
> > > > >> +
> > > > >> +    mMemoryTypeStatistics[Type].CurrentNumberOfPages = 0;
> > > > >> +    if (mMemoryTypeStatistics[Type].MaximumAddress ==
> > > > >> MAX_ALLOC_ADDRESS) {
> > > > >> +      mMemoryTypeStatistics[Type].MaximumAddress =
> > > > >> mDefaultMaximumAddress;
> > > > >> +    }
> > > > >> +  }
> > > > >> +
> > > > >> +  mMemoryTypeInformationInitialized = TRUE;
> > > > >> +}
> > > > >> +
> > > > >>  /**
> > > > >>    Called to initialize the memory map and add descriptors to
> > > > >>    the current descriptor list.
> > > > >> --
> > > > >> 2.40.1.windows.1
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> >
> >
> >
> >
> >
> > 
> >





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114619): https://edk2.groups.io/g/devel/message/114619
Mute This Topic: https://groups.io/mt/103978575/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

end of thread, other threads:[~2024-01-26 15:50 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-23 20:24 [edk2-devel] [Patch v2 1/1] MdeModulePkg/Core/Dxe: Set MemoryTypeInfo bin range from HOB Michael D Kinney
2024-01-24 14:17 ` Laszlo Ersek
2024-01-24 17:39   ` Michael D Kinney
2024-01-24 14:20 ` Laszlo Ersek
2024-01-24 17:46   ` Michael D Kinney
2024-01-25 21:34     ` Laszlo Ersek
2024-01-24 14:59 ` [edk2-devel] 回复: " gaoliming via groups.io
2024-01-24 16:16   ` Laszlo Ersek
2024-01-24 17:52     ` Michael D Kinney
2024-01-25  1:18       ` 回复: " gaoliming via groups.io
2024-01-25  1:39         ` Michael D Kinney
2024-01-26 15:50           ` 回复: " gaoliming via groups.io

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