On 6 May 2016 at 06:35, John Stultz <[email protected]> wrote: > In the current edk2 code, as of 40a3f38f67cee > ("ArmPlatformPkg/MemoryInitPei: Check if the main System > Memory resource has been declared"), ArmPlatformGetVirtualMemoryMap() > is now called before the base system memory HOB is created in > MemoryPeim(). > > This causes the logic in HiKey's ArmPlatformGetVirtualMemoryMap() > which tries to chop up that base HOB to fail, as there's nothing > to chop up at that time. > > The calling code is smart enough to check if a base system memory > HOB is already present before creating one, so we can just set it > up ourselves here before we try to make our reservations. > > This avoids problems seen in the kernel with the current logic, > where none of the reservations were actually being made. > > Note: I'm a total newbie with this codebase (I just recently > figured out what HOB stood for). So feedback would be greatly > appreciated! >
Hi John, Thanks for the patch. What tree is this commit based on? > CC: Olivier Martin <[email protected]> > Cc: Haojian Zhuang <[email protected]> > Cc: Vishal Bhoj <[email protected]> > Signed-off-by: John Stultz <[email protected]> > --- > Platforms/Hisilicon/HiKey/Library/HiKeyLib/HiKeyMem.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/Platforms/Hisilicon/HiKey/Library/HiKeyLib/HiKeyMem.c > b/Platforms/Hisilicon/HiKey/Library/HiKeyLib/HiKeyMem.c > index a5ce05b..2d118e0 100644 > --- a/Platforms/Hisilicon/HiKey/Library/HiKeyLib/HiKeyMem.c > +++ b/Platforms/Hisilicon/HiKey/Library/HiKeyLib/HiKeyMem.c > @@ -89,6 +89,24 @@ ArmPlatformGetVirtualMemoryMap ( > > MemorySize = HiKeyInitMemorySize (); > This suggests that the PcdSystemMemorySize below may be different from the real memory size, but I cannot tell from the context. On systems with ARM Trusted Firmware running at EL3, there is usually some ad-hoc logic to figure out how much memory has been taken up by the secure firmware, and that either needs to be reflected in the PCD (if it's a dynamic PCD), or perhaps we should be using the MemorySize below? > + ResourceAttributes = ( > + EFI_RESOURCE_ATTRIBUTE_PRESENT | > + EFI_RESOURCE_ATTRIBUTE_INITIALIZED | > + EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE | > + EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE | > + EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE | > + EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE | > + EFI_RESOURCE_ATTRIBUTE_TESTED > + ); > + > + // Create initial Base Hob for system memory. > + BuildResourceDescriptorHob ( > + EFI_RESOURCE_SYSTEM_MEMORY, > + ResourceAttributes, > + PcdGet64 (PcdSystemMemoryBase), > + PcdGet64 (PcdSystemMemorySize) > + ); > + > NextHob.Raw = GetHobList (); > Count = sizeof (HiKeyReservedMemoryBuffer) / sizeof (struct > HiKeyReservedMemory); > while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, > NextHob.Raw)) != NULL) > -- > 1.9.1 > > _______________________________________________ > Linaro-uefi mailing list > [email protected] > https://lists.linaro.org/mailman/listinfo/linaro-uefi _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

