public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, gaoliming@byosoft.com.cn,
	'Michael D Kinney' <michael.d.kinney@intel.com>
Cc: 'Aaron Li' <aaron.li@intel.com>, 'Liu Yun' <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
Date: Wed, 24 Jan 2024 17:16:47 +0100	[thread overview]
Message-ID: <bdd579ac-8287-187a-cac2-b9a66b927058@redhat.com> (raw)
In-Reply-To: <01d301da4ed5$f3527940$d9f76bc0$@byosoft.com.cn>

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]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2024-01-24 16:16 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bdd579ac-8287-187a-cac2-b9a66b927058@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox