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

Reply via email to