Cc'ing Ben On 09/17/15 10:09, Star Zeng wrote: > Take the range in the resource descriptor HOB for the memory region described > by the PHIT as higher priority if it is big enough. It can make the memory bin > allocated to be at the same memory region with PHIT that has more better > compatibility > to avoid memory fragmentation for some code practices assume and allocate <4G > ACPI memory. > > Also let the minimal memory size needed include the total memory bin size > needed to > make sure memory bin could be allocated successfully. > > V2 corrects a comment in original code and a missing > MINIMUM_INITIAL_MEMORY_SIZE updated > to MinimalMemorySizeNeeded in V1. > > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Liming Gao <liming....@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Star Zeng <star.z...@intel.com> > Reviewed-by: Jiewen Yao <jiewen....@intel.com> > --- > MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 163 > +++++++++++++++++++++++----------------- > 1 file changed, 95 insertions(+), 68 deletions(-)
I gave this patch a light review (albeit after the fact :)), and I think it's good. I think it captures what Ben wanted. For OVMF the "fallback" loop (ie. the loop that is a fallback *now*) has always been inactive (because we add the high memory as untested), but I agree that this priority is preferable. Anyway I regression-tested this with OVMF, and it seems okay. Thanks also for addressing my comment remark about EfiFreeMemoryTop <-> EfiMemoryTop in <http://thread.gmane.org/gmane.comp.bios.edk2.devel/1023/focus=1124>. Another good thing added in the patch: the logging of the initial region that was found, on the EFI_D_INFO level. This log message at the beginning of DXE plays *very nicely* together with the debug message printed by PeiInstallPeiMemory(), in "MdeModulePkg/Core/Pei/Memory/MemoryServices.c". One can now verify from the debug log that DXE's initial region falls within the permanent PEI RAM. For example, this OVMF guest that I just used for testing, printed: PeiInstallPeiMemory MemoryBegin 0xBBFBE000, MemoryLength 0x4042000 [...] CoreInitializeMemoryServices: BaseAddress - 0xBBFE0000 Length - 0x3F1E000 MinimalMemorySizeNeeded - 0x10F4000 So the permanent PEI RAM is [0xBBFBE000 .. 0xC0000000), and the initial DXE region is a proper sub-interval of that, [0xBBFE0000 .. BFEFE000). Thanks! Laszlo > diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > index 77586a9..c276962 100644 > --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > @@ -2003,6 +2003,30 @@ > CoreConvertResourceDescriptorHobAttributesToCapabilities ( > return Capabilities; > } > > +/** > + Calculate total memory bin size neeeded. > + > + @return The total memory bin size neeeded. > + > +**/ > +UINT64 > +CalculateTotalMemoryBinSizeNeeded ( > + VOID > + ) > +{ > + UINTN Index; > + UINT64 TotalSize; > + > + // > + // Loop through each memory type in the order specified by the > gMemoryTypeInformation[] array > + // > + TotalSize = 0; > + for (Index = 0; gMemoryTypeInformation[Index].Type != EfiMaxMemoryType; > Index++) { > + TotalSize += LShiftU64 (gMemoryTypeInformation[Index].NumberOfPages, > EFI_PAGE_SHIFT); > + } > + > + return TotalSize; > +} > > /** > External function. Initializes memory services based on the memory > @@ -2044,7 +2068,8 @@ CoreInitializeMemoryServices ( > UINT64 TestedMemoryLength; > EFI_PHYSICAL_ADDRESS HighAddress; > EFI_HOB_GUID_TYPE *GuidHob; > - UINT32 ReservedCodePageNumber; > + UINT32 ReservedCodePageNumber; > + UINT64 MinimalMemorySizeNeeded; > > // > // Point at the first HOB. This must be the PHIT HOB. > @@ -2098,9 +2123,13 @@ CoreInitializeMemoryServices ( > } > > // > + // Include the total memory bin size needed to make sure memory bin could > be allocated successfully. > + // > + MinimalMemorySizeNeeded = MINIMUM_INITIAL_MEMORY_SIZE + > CalculateTotalMemoryBinSizeNeeded (); > + > + // > // Find the Resource Descriptor HOB that contains PHIT range > EfiFreeMemoryBottom..EfiFreeMemoryTop > // > - Length = 0; > Found = FALSE; > for (Hob.Raw = *HobStart; !END_OF_HOB_LIST(Hob); Hob.Raw = > GET_NEXT_HOB(Hob)) { > // > @@ -2138,19 +2167,19 @@ CoreInitializeMemoryServices ( > Found = TRUE; > > // > - // Compute range between PHIT EfiFreeMemoryTop and the end of the > Resource Descriptor HOB > + // Compute range between PHIT EfiMemoryTop and the end of the Resource > Descriptor HOB > // > Attributes = PhitResourceHob->ResourceAttribute; > BaseAddress = PageAlignAddress (PhitHob->EfiMemoryTop); > Length = PageAlignLength (ResourceHob->PhysicalStart + > ResourceHob->ResourceLength - BaseAddress); > - if (Length < MINIMUM_INITIAL_MEMORY_SIZE) { > + if (Length < MinimalMemorySizeNeeded) { > // > // If that range is not large enough to intialize the DXE Core, then > // Compute range between PHIT EfiFreeMemoryBottom and PHIT > EfiFreeMemoryTop > // > BaseAddress = PageAlignAddress (PhitHob->EfiFreeMemoryBottom); > Length = PageAlignLength (PhitHob->EfiFreeMemoryTop - > BaseAddress); > - if (Length < MINIMUM_INITIAL_MEMORY_SIZE) { > + if (Length < MinimalMemorySizeNeeded) { > // > // If that range is not large enough to intialize the DXE Core, then > // Compute range between the start of the Resource Descriptor HOB > and the start of the HOB List > @@ -2168,81 +2197,79 @@ CoreInitializeMemoryServices ( > ASSERT (Found); > > // > - // 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. > + // Take the range in the resource descriptor HOB for the memory region > described > + // by the PHIT as higher priority if it is big enough. It can make the > memory bin > + // allocated to be at the same memory region with PHIT that has more > better compatibility > + // to avoid memory fragmentation for some code practices assume and > allocate <4G ACPI memory. > // > - HighAddress = MAX_ADDRESS; > - for (Hob.Raw = *HobStart; !END_OF_HOB_LIST(Hob); Hob.Raw = > GET_NEXT_HOB(Hob)) { > - // > - // Skip the Resource Descriptor HOB that contains the PHIT > - // > - if (Hob.ResourceDescriptor == PhitResourceHob) { > - continue; > - } > + if (Length < MinimalMemorySizeNeeded) { > // > - // Skip all HOBs except Resource Descriptor HOBs > + // 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. > // > - if (GET_HOB_TYPE (Hob) != EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) { > - continue; > - } > + HighAddress = MAX_ADDRESS; > + for (Hob.Raw = *HobStart; !END_OF_HOB_LIST(Hob); Hob.Raw = > GET_NEXT_HOB(Hob)) { > + // > + // Skip the Resource Descriptor HOB that contains the PHIT > + // > + if (Hob.ResourceDescriptor == PhitResourceHob) { > + continue; > + } > + // > + // Skip all HOBs except Resource Descriptor HOBs > + // > + if (GET_HOB_TYPE (Hob) != EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) { > + continue; > + } > > - // > - // Skip Resource Descriptor HOBs that do not describe tested system > memory below MAX_ADDRESS > - // > - ResourceHob = Hob.ResourceDescriptor; > - if (ResourceHob->ResourceType != EFI_RESOURCE_SYSTEM_MEMORY) { > - continue; > - } > - if ((ResourceHob->ResourceAttribute & MEMORY_ATTRIBUTE_MASK) != > TESTED_MEMORY_ATTRIBUTES) { > - continue; > - } > - if ((ResourceHob->PhysicalStart + ResourceHob->ResourceLength) > > (EFI_PHYSICAL_ADDRESS)MAX_ADDRESS) { > - continue; > - } > - > - // > - // Skip Resource Descriptor HOBs that are below a previously found > Resource Descriptor HOB > - // > - if (HighAddress != (EFI_PHYSICAL_ADDRESS)MAX_ADDRESS && > ResourceHob->PhysicalStart <= HighAddress) { > - continue; > - } > + // > + // Skip Resource Descriptor HOBs that do not describe tested system > memory below MAX_ADDRESS > + // > + ResourceHob = Hob.ResourceDescriptor; > + if (ResourceHob->ResourceType != EFI_RESOURCE_SYSTEM_MEMORY) { > + continue; > + } > + if ((ResourceHob->ResourceAttribute & MEMORY_ATTRIBUTE_MASK) != > TESTED_MEMORY_ATTRIBUTES) { > + continue; > + } > + if ((ResourceHob->PhysicalStart + ResourceHob->ResourceLength) > > (EFI_PHYSICAL_ADDRESS)MAX_ADDRESS) { > + continue; > + } > > - // > - // Skip Resource Descriptor HOBs that are not large enough to initilize > the DXE Core > - // > - TestedMemoryBaseAddress = PageAlignAddress (ResourceHob->PhysicalStart); > - TestedMemoryLength = PageAlignLength (ResourceHob->PhysicalStart + > ResourceHob->ResourceLength - TestedMemoryBaseAddress); > - if (TestedMemoryLength < MINIMUM_INITIAL_MEMORY_SIZE) { > - continue; > + // > + // Skip Resource Descriptor HOBs that are below a previously found > Resource Descriptor HOB > + // > + if (HighAddress != (EFI_PHYSICAL_ADDRESS)MAX_ADDRESS && > ResourceHob->PhysicalStart <= HighAddress) { > + continue; > + } > + > + // > + // Skip Resource Descriptor HOBs that are not large enough to > initilize the DXE Core > + // > + TestedMemoryBaseAddress = PageAlignAddress > (ResourceHob->PhysicalStart); > + TestedMemoryLength = PageAlignLength (ResourceHob->PhysicalStart > + ResourceHob->ResourceLength - TestedMemoryBaseAddress); > + if (TestedMemoryLength < MinimalMemorySizeNeeded) { > + continue; > + } > + > + // > + // Save the range described by the Resource Descriptor that is large > enough to initilize the DXE Core > + // > + BaseAddress = TestedMemoryBaseAddress; > + Length = TestedMemoryLength; > + Attributes = ResourceHob->ResourceAttribute; > + HighAddress = ResourceHob->PhysicalStart; > } > - > - // > - // Save the Resource Descriptor HOB context that is large enough to > initilize the DXE Core > - // > - MaxMemoryBaseAddress = TestedMemoryBaseAddress; > - MaxMemoryLength = TestedMemoryLength; > - MaxMemoryAttributes = ResourceHob->ResourceAttribute; > - HighAddress = ResourceHob->PhysicalStart; > } > > - // > - // If Length is not large enough to initialize the DXE Core or a Resource > - // Descriptor HOB was found above the PHIT HOB that is large enough to > initialize > - // the DXE Core, then use the range described by the Resource Descriptor > - // HOB that was found above the PHIT HOB. > - // > - if ((Length < MINIMUM_INITIAL_MEMORY_SIZE) || > - (MaxMemoryBaseAddress > BaseAddress && MaxMemoryLength >= > MINIMUM_INITIAL_MEMORY_SIZE)) { > - BaseAddress = MaxMemoryBaseAddress; > - Length = MaxMemoryLength; > - Attributes = MaxMemoryAttributes; > - } > + DEBUG ((EFI_D_INFO, "CoreInitializeMemoryServices:\n")); > + DEBUG ((EFI_D_INFO, " BaseAddress - 0x%lx Length - 0x%lx > MinimalMemorySizeNeeded - 0x%lx\n", BaseAddress, Length, > MinimalMemorySizeNeeded)); > > // > // If no memory regions are found that are big enough to initialize the > DXE core, then ASSERT(). > // > - ASSERT (Length >= MINIMUM_INITIAL_MEMORY_SIZE); > + ASSERT (Length >= MinimalMemorySizeNeeded); > > // > // Convert the Resource HOB Attributes to an EFI Memory Capabilities mask > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
