On Fri, 2015-09-18 at 17:03 +0200, Laszlo Ersek wrote: > Cc'ing Ben Thanks, I saw it :) I just got caught up with a bunch of other things and haven't had a chance to look more closely.
I got legal approval to release code, so I'm hoping sometime in the next few weeks, to push out on github a slightly cleaned up version of my proof of concept ppc64 port. While doing that I will clean up my memory management and test this patch properly. I will also submit patches for dealing with non-1:1 mapping of MMIO in PCI land vs. GetBarAttributes(). Cheers, 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 <[email protected]> > > Cc: Liming Gao <[email protected]> > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Star Zeng <[email protected]> > > Reviewed-by: Jiewen Yao <[email protected]> > > --- > > 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 [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
