Re: [edk2-devel] [edk2-platforms] [PATCH v2 07/11] Silicon/NXP : Add MemoryInitPei Library

["Leif Lindholm" <leif.lindholm@linaro.org>]

On Thu, Nov 21, 2019 at 21:55:10 +0530, Meenakshi Aggarwal wrote:
> Add MemoryInitPei Library for NXP platforms.
> It has changes to get DRAM information from TFA.
> 

Only the feedback on the commit message has been addressed (but the
message is now fine). There were several more comments on this
patch. Please revisit and resubmit.

Best Regards,

Leif

> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> ---
>  Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf |  48 +++++++
>  Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c   | 139 ++++++++++++++++++++
>  2 files changed, 187 insertions(+)
> 
> diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
> new file mode 100644
> index 000000000000..806da6d9ab9a
> --- /dev/null
> +++ b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
> @@ -0,0 +1,48 @@
> +#/** @file
> +#
> +#  Copyright (c) 2011-2014, ARM Ltd. All rights reserved.<BR>
> +#  Copyright 2019 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = ArmMemoryInitPeiLib
> +  FILE_GUID                      = 55ddb6e0-70b5-11e0-b33e-0002a5d5c51b
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = MemoryInitPeiLib|SEC PEIM DXE_DRIVER
> +
> +[Sources]
> +  MemoryInitPeiLib.c
> +
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  Silicon/NXP/NxpQoriqLs.dec
> +
> +[LibraryClasses]
> +  DebugLib
> +  HobLib
> +  ArmMmuLib
> +  ArmPlatformLib
> +  PcdLib
> +
> +[Guids]
> +  gEfiMemoryTypeInformationGuid
> +
> +[FeaturePcd]
> +  gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob
> +
> +[Pcd]
> +  gArmTokenSpaceGuid.PcdSystemMemoryBase
> +  gArmTokenSpaceGuid.PcdSystemMemorySize
> +
> +[Depex]
> +  TRUE
> diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
> new file mode 100644
> index 000000000000..9889d5730261
> --- /dev/null
> +++ b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
> @@ -0,0 +1,139 @@
> +/** @file
> +*
> +*  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> +*
> +*  Copyright 2019 NXP
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#include <PiPei.h>
> +
> +#include <Library/ArmMmuLib.h>
> +#include <Library/ArmPlatformLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PcdLib.h>
> +
> +#include <DramInfo.h>
> +
> +VOID
> +BuildMemoryTypeInformationHob (
> +  VOID
> +  );
> +
> +VOID
> +InitMmu (
> +  IN ARM_MEMORY_REGION_DESCRIPTOR  *MemoryTable
> +  )
> +{
> +
> +  VOID                          *TranslationTableBase;
> +  UINTN                         TranslationTableSize;
> +  RETURN_STATUS                 Status;
> +
> +  //Note: Because we called PeiServicesInstallPeiMemory() before to call InitMmu() the MMU Page Table resides in
> +  //      DRAM (even at the top of DRAM as it is the first permanent memory allocation)
> +  Status = ArmConfigureMmu (MemoryTable, &TranslationTableBase, &TranslationTableSize);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Error: Failed to enable MMU\n"));
> +  }
> +}
> +
> +/*++
> +
> +Routine Description:
> +
> +
> +
> +Arguments:
> +
> +  FileHandle  - Handle of the file being invoked.
> +  PeiServices - Describes the list of possible PEI Services.
> +
> +Returns:
> +
> +  Status -  EFI_SUCCESS if the boot mode could be set
> +
> +--*/
> +EFI_STATUS
> +EFIAPI
> +MemoryPeim (
> +  IN EFI_PHYSICAL_ADDRESS               UefiMemoryBase,
> +  IN UINT64                             UefiMemorySize
> +  )
> +{
> +  ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
> +  EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> +  EFI_PEI_HOB_POINTERS         NextHob;
> +  BOOLEAN                      Found;
> +  DRAM_INFO                    DramInfo;
> +
> +  // Get Virtual Memory Map from the Platform Library
> +  ArmPlatformGetVirtualMemoryMap (&MemoryTable);
> +
> +  //
> +  // Ensure MemoryTable[0].Length which is size of DRAM has been set
> +  // by ArmPlatformGetVirtualMemoryMap ()
> +  //
> +  ASSERT (MemoryTable[0].Length != 0);
> +
> +  //
> +  // Now, the permanent memory has been installed, we can call AllocatePages()
> +  //
> +  ResourceAttributes = (
> +      EFI_RESOURCE_ATTRIBUTE_PRESENT |
> +      EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> +      EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
> +      EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
> +      EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
> +      EFI_RESOURCE_ATTRIBUTE_TESTED
> +  );
> +
> +  if (GetDramBankInfo (&DramInfo)) {
> +    DEBUG ((DEBUG_ERROR, "Failed to get DRAM information, exiting...\n"));
> +    return EFI_UNSUPPORTED;
> +  }
> +
> +  while (DramInfo.NumOfDrams--) {
> +    //
> +    // Check if the resource for the main system memory has been declared
> +    //
> +    Found = FALSE;
> +    NextHob.Raw = GetHobList ();
> +    while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) {
> +      if ((NextHob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) &&
> +          (DramInfo.DramRegion[DramInfo.NumOfDrams].BaseAddress >= NextHob.ResourceDescriptor->PhysicalStart) &&
> +          (NextHob.ResourceDescriptor->PhysicalStart + NextHob.ResourceDescriptor->ResourceLength <=
> +           DramInfo.DramRegion[DramInfo.NumOfDrams].BaseAddress + DramInfo.DramRegion[DramInfo.NumOfDrams].Size))
> +      {
> +        Found = TRUE;
> +        break;
> +      }
> +      NextHob.Raw = GET_NEXT_HOB (NextHob);
> +    }
> +
> +    if (!Found) {
> +      // Reserved the memory space occupied by the firmware volume
> +      BuildResourceDescriptorHob (
> +          EFI_RESOURCE_SYSTEM_MEMORY,
> +          ResourceAttributes,
> +          DramInfo.DramRegion[DramInfo.NumOfDrams].BaseAddress,
> +          DramInfo.DramRegion[DramInfo.NumOfDrams].Size
> +      );
> +    }
> +  }
> +
> +  // Build Memory Allocation Hob
> +  InitMmu (MemoryTable);
> +
> +  if (FeaturePcdGet (PcdPrePiProduceMemoryTypeInformationHob)) {
> +    // Optional feature that helps prevent EFI memory map fragmentation.
> +    BuildMemoryTypeInformationHob ();
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> -- 
> 1.9.1
> 
> 
> 
>