[edk2-devel] [PATCH 13/19] Silicon/NXP: Move RAM retrieval from SocLib

Pankaj Bansal posted 19 patches 6 years ago
Only 18 patches received!
[edk2-devel] [PATCH 13/19] Silicon/NXP: Move RAM retrieval from SocLib
Posted by Pankaj Bansal 6 years ago
RAM retrieval using SMC commands is common to all Layerscape SOCs.
Therefore, move it to commom MemoryInit Pei Lib.

Also added provision to reserve a portion of RAM.

Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---
 Silicon/NXP/Include/DramInfo.h                |  38 ---
 .../Library/MemoryInitPei/MemoryInitPeiLib.c  | 235 +++++++++++++++---
 .../Library/MemoryInitPei/MemoryInitPeiLib.h  |  24 ++
 .../MemoryInitPei/MemoryInitPeiLib.inf        |   8 +
 Silicon/NXP/Library/SocLib/Chassis.c          |  67 -----
 Silicon/NXP/NxpQoriqLs.dec                    |   7 +-
 6 files changed, 239 insertions(+), 140 deletions(-)
 delete mode 100644 Silicon/NXP/Include/DramInfo.h
 create mode 100644 Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h

diff --git a/Silicon/NXP/Include/DramInfo.h b/Silicon/NXP/Include/DramInfo.h
deleted file mode 100644
index a934aaeff1..0000000000
--- a/Silicon/NXP/Include/DramInfo.h
+++ /dev/null
@@ -1,38 +0,0 @@
-/** @file
-*  Header defining the structure for Dram Information
-*
-*  Copyright 2019 NXP
-*
-*  SPDX-License-Identifier: BSD-2-Clause-Patent
-*
-**/
-
-#ifndef DRAM_INFO_H_
-#define DRAM_INFO_H_
-
-#include <Uefi/UefiBaseType.h>
-
-#define SMC_DRAM_BANK_INFO          (0xC200FF12)
-
-typedef struct {
-  UINTN            BaseAddress;
-  UINTN            Size;
-} DRAM_REGION_INFO;
-
-typedef struct {
-  UINT32            NumOfDrams;
-  UINT32            Reserved;
-  DRAM_REGION_INFO  DramRegion[3];
-} DRAM_INFO;
-
-EFI_STATUS
-GetDramBankInfo (
-  IN OUT DRAM_INFO *DramInfo
-  );
-
-VOID
-UpdateDpaaDram (
-  IN OUT DRAM_INFO *DramInfo
-  );
-
-#endif /* DRAM_INFO_H_ */
diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
index 3ea7736786..eb1983bdbc 100644
--- a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
+++ b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
@@ -17,8 +17,10 @@
 #include <Library/HobLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
+#include <Library/ArmSmcLib.h>
+
+#include "MemoryInitPeiLib.h"
 
-#include <DramInfo.h>
 
 VOID
 BuildMemoryTypeInformationHob (
@@ -44,6 +46,85 @@ InitMmu (
   }
 }
 
+STATIC
+UINTN
+CalculateReservedMemBase (
+  IN DRAM_REGION_INFO *DramRegions,
+  IN UINT32           NumRegions,
+  IN UINTN            ReservedMemSize
+)
+{
+  UINTN                 ReservedMemAlignment;
+  INTN                  Index;
+  EFI_PHYSICAL_ADDRESS  AlignmentMask;
+  UINTN                 RegionBaseAddress;
+  UINTN                 RegionSize;
+  UINTN                 ReservedBaseAddress;
+  INTN                  Index2;
+
+  ReservedMemAlignment = FixedPcdGet64 (PcdReservedMemAlignment);
+  //
+  // Compute alignment bit mask
+  //
+  if (ReservedMemAlignment) {
+    AlignmentMask = LShiftU64 (1, LowBitSet64(ReservedMemAlignment)) - 1;
+  } else {
+    AlignmentMask = 0;
+  }
+  Index = NumRegions;
+  while (Index--) {
+    RegionBaseAddress = DramRegions[Index].BaseAddress;
+    RegionSize = DramRegions[Index].Size;
+
+    if (ReservedMemSize > RegionSize) {
+      continue;
+    }
+
+    ReservedBaseAddress = (RegionBaseAddress + RegionSize - ReservedMemSize) & (~AlignmentMask);
+    if (ReservedBaseAddress < RegionBaseAddress) {
+      continue;
+    }
+
+    // found the region from which reserved mem is to be carved out
+    // Need to modify the region size and create/delete region if need be
+    RegionSize -= ReservedMemSize;
+    if (!RegionSize) {
+      for (Index2 = Index; Index2 < NumRegions; Index2++) {
+        CopyMem (&DramRegions[Index2], &DramRegions[Index2 + 1], sizeof (DRAM_REGION_INFO));
+      }
+      break;
+    }
+
+    if (ReservedBaseAddress - RegionBaseAddress) {
+      DramRegions[Index].Size = ReservedBaseAddress - RegionBaseAddress;
+      RegionSize -= DramRegions[Index].Size;
+    } else {
+      DramRegions[Index].BaseAddress = ReservedBaseAddress + ReservedMemSize;
+      DramRegions[Index].Size = RegionSize;
+      RegionSize = 0;
+    }
+
+    if (!RegionSize) {
+      break;
+    }
+
+    for (Index2 = NumRegions; Index2 > (Index + 1); Index2--) {
+      CopyMem (&DramRegions[Index2], &DramRegions[Index2 - 1], sizeof (DRAM_REGION_INFO));
+    }
+    DramRegions[Index2].BaseAddress = ReservedBaseAddress + ReservedMemSize;
+    DramRegions[Index2].Size = RegionSize;
+    RegionSize = 0;
+
+    break;
+  }
+
+  if (Index == -1) {
+    return 0;
+  } else {
+    return ReservedBaseAddress;
+  }
+}
+
 /*++
 
 Routine Description:
@@ -68,10 +149,19 @@ MemoryPeim (
   )
 {
   ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
+  ARM_SMC_ARGS                 ArmSmcArgs;
+  INT32                        Index;
+  UINTN                        DramSize;
+  UINTN                        BaseAddress;
+  UINTN                        Size;
+  UINTN                        Top;
+  // Extra region gets created if we want to reserve a memory region and that creates a memory hole
+  // because of alignement requirements
+  DRAM_REGION_INFO             DramRegions[MAX_DRAM_REGIONS + 1];
   EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
-  EFI_PEI_HOB_POINTERS         NextHob;
-  BOOLEAN                      Found;
-  DRAM_INFO                    DramInfo;
+  UINTN                        FdBase;
+  UINTN                        FdTop;
+  BOOLEAN                      FoundSystemMem;
 
   // Get Virtual Memory Map from the Platform Library
   ArmPlatformGetVirtualMemoryMap (&MemoryTable);
@@ -86,48 +176,127 @@ MemoryPeim (
   // 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
+      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;
-  }
+  FoundSystemMem = FALSE;
+  ZeroMem (DramRegions, sizeof (DramRegions));
+
+  Index = -1;
+  do {
+    ArmSmcArgs.Arg0 = SMC_DRAM_BANK_INFO;
+    ArmSmcArgs.Arg1 = Index++;
 
-  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;
+    ArmCallSmc (&ArmSmcArgs);
+    ASSERT (!(ArmSmcArgs.Arg0 && !Index));
+    if (!Index) {
+      DramSize = ArmSmcArgs.Arg1;
+    } else {
+      if (!ArmSmcArgs.Arg0) {
+        BaseAddress = ArmSmcArgs.Arg1;
+        Size = ArmSmcArgs.Arg2;
+        ASSERT (BaseAddress && Size);
+
+        DramRegions[Index - 1].BaseAddress = BaseAddress;
+        DramRegions[Index - 1].Size = Size;
+        DramSize -= Size;
+
+        DEBUG ((DEBUG_INFO, "bank[%d]: start 0x%lx, size 0x%lx\n", Index, BaseAddress, Size));
       }
-      NextHob.Raw = GET_NEXT_HOB (NextHob);
+    }
+  } while (DramSize && Index < MAX_DRAM_REGIONS);
+
+  ASSERT (!DramSize);
+
+  // Get the reserved memory size from non volatile storage
+  Size = FixedPcdGet64 (PcdReservedMemSize);
+  if (Size) {
+    BaseAddress = CalculateReservedMemBase (DramRegions, Index, Size);
+    if (BaseAddress) {
+      DEBUG ((DEBUG_INFO, "ReservedMem: start 0x%lx, size 0x%lx\n", BaseAddress, Size));
+    }
+  }
+
+  FdBase = (UINTN)FixedPcdGet64 (PcdFdBaseAddress);
+  FdTop = FdBase + (UINTN)FixedPcdGet32 (PcdFdSize);
+
+  // Declare memory regios to system
+  for (Index = MAX_DRAM_REGIONS; Index >= 0; Index--) {
+    if (!DramRegions[Index].Size) {
+      continue;
     }
 
-    if (!Found) {
-      // Reserved the memory space occupied by the firmware volume
+    BaseAddress = DramRegions[Index].BaseAddress;
+    Top = DramRegions[Index].BaseAddress + DramRegions[Index].Size;
+
+    // EDK2 does not have the concept of boot firmware copied into DRAM. To avoid the DXE
+    // core to overwrite this area we must create a memory allocation HOB for the region,
+    // but this only works if we split off the underlying resource descriptor as well.
+    if (FdBase >= BaseAddress && FdTop <= Top) {
+      // Update Size
+      Size = FdBase - BaseAddress;
+      if (Size) {
+        BuildResourceDescriptorHob (
+          EFI_RESOURCE_SYSTEM_MEMORY,
+          ResourceAttributes,
+          BaseAddress,
+          Size
+        );
+      }
+      // create the System Memory HOB for the firmware
       BuildResourceDescriptorHob (
         EFI_RESOURCE_SYSTEM_MEMORY,
         ResourceAttributes,
-        DramInfo.DramRegion[DramInfo.NumOfDrams].BaseAddress,
-        DramInfo.DramRegion[DramInfo.NumOfDrams].Size
+        FdBase,
+        PcdGet32 (PcdFdSize)
+      );
+      // Create the System Memory HOB for the remaining region (top of the FD)s
+      Size = Top - FdTop;
+      if (Size) {
+        BuildResourceDescriptorHob (
+          EFI_RESOURCE_SYSTEM_MEMORY,
+          ResourceAttributes,
+          FdTop,
+          Size
+        );
+      };
+      // Mark the memory covering the Firmware Device as boot services data
+      BuildMemoryAllocationHob (FixedPcdGet64 (PcdFdBaseAddress),
+                                FixedPcdGet32 (PcdFdSize),
+                                EfiBootServicesData);
+    }else {
+      BuildResourceDescriptorHob (
+          EFI_RESOURCE_SYSTEM_MEMORY,
+          ResourceAttributes,
+          DramRegions[Index].BaseAddress,
+          DramRegions[Index].Size
       );
     }
+
+    if (FoundSystemMem) {
+      continue;
+    }
+
+    BaseAddress = DramRegions[Index].BaseAddress;
+    Size = DramRegions[Index].Size;
+    Top = DramRegions[Index].BaseAddress + DramRegions[Index].Size;
+
+    if (FdBase >= BaseAddress && FdTop <= Top) {
+      Size -= (UINTN)FixedPcdGet32 (PcdFdSize);
+    }
+
+    if (Size >= FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)) {
+      FoundSystemMem = TRUE;
+    }
   }
 
+  ASSERT (FoundSystemMem);
+
   // Build Memory Allocation Hob
   InitMmu (MemoryTable);
 
diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h
new file mode 100644
index 0000000000..e563b2ba8d
--- /dev/null
+++ b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h
@@ -0,0 +1,24 @@
+/** @file
+*
+*  Copyright 2020 NXP
+*
+*  SPDX-License-Identifier: BSD-2-Clause-Patent
+*
+**/
+
+#ifndef _MEMORY_INIT_PEI_LIB_H_
+#define _MEMORY_INIT_PEI_LIB_H_
+
+#include <Uefi.h>
+
+// Specifies the Maximum regions onto which DDR memory can be mapped in a Platform
+#define MAX_DRAM_REGIONS            3
+#define SMC_DRAM_BANK_INFO          (0xC200FF12)
+
+typedef struct {
+  UINTN            BaseAddress;
+  UINTN            Size;
+} DRAM_REGION_INFO;
+
+#endif
+
diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
index a5bd39415d..9adddcaf8c 100644
--- a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
+++ b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
@@ -33,6 +33,7 @@
   DebugLib
   HobLib
   PcdLib
+  ArmSmcLib
 
 [Guids]
   gEfiMemoryTypeInformationGuid
@@ -40,6 +41,13 @@
 [FeaturePcd]
   gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob
 
+[FixedPcd]
+  gArmTokenSpaceGuid.PcdFdBaseAddress
+  gArmTokenSpaceGuid.PcdFdSize
+  gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize
+  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemSize
+  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemAlignment
+
 [Pcd]
   gArmTokenSpaceGuid.PcdSystemMemoryBase
   gArmTokenSpaceGuid.PcdSystemMemorySize
diff --git a/Silicon/NXP/Library/SocLib/Chassis.c b/Silicon/NXP/Library/SocLib/Chassis.c
index 847331a631..1ef99e8de2 100644
--- a/Silicon/NXP/Library/SocLib/Chassis.c
+++ b/Silicon/NXP/Library/SocLib/Chassis.c
@@ -22,7 +22,6 @@
 #include <Library/PrintLib.h>
 #include <Library/SerialPortLib.h>
 
-#include <DramInfo.h>
 #include "NxpChassis.h"
 
 UINT32
@@ -75,69 +74,3 @@ SmmuInit (
   MmioWrite32 ((UINTN)SMMU_REG_NSCR0, Value);
 }
 
-UINTN
-GetDramSize (
-  IN VOID
-  )
-{
-  ARM_SMC_ARGS  ArmSmcArgs;
-
-  ArmSmcArgs.Arg0 = SMC_DRAM_BANK_INFO;
-  ArmSmcArgs.Arg1 = -1;
-
-  ArmCallSmc (&ArmSmcArgs);
-
-  if (ArmSmcArgs.Arg0) {
-    return 0;
-  } else {
-    return ArmSmcArgs.Arg1;
-  }
-}
-
-EFI_STATUS
-GetDramBankInfo (
-  IN OUT DRAM_INFO *DramInfo
-  )
-{
-  ARM_SMC_ARGS  ArmSmcArgs;
-  UINT32        I;
-  UINTN         DramSize;
-
-  DramSize = GetDramSize ();
-  DEBUG ((DEBUG_INFO, "DRAM Total Size 0x%lx \n", DramSize));
-
-  // Ensure DramSize has been set
-  ASSERT (DramSize != 0);
-
-  I = 0;
-
-  do {
-    ArmSmcArgs.Arg0 = SMC_DRAM_BANK_INFO;
-    ArmSmcArgs.Arg1 = I;
-
-    ArmCallSmc (&ArmSmcArgs);
-    if (ArmSmcArgs.Arg0) {
-      if (I > 0) {
-        break;
-      } else {
-        ASSERT (ArmSmcArgs.Arg0 == 0);
-      }
-    }
-
-    DramInfo->DramRegion[I].BaseAddress = ArmSmcArgs.Arg1;
-    DramInfo->DramRegion[I].Size = ArmSmcArgs.Arg2;
-
-    DramSize -= DramInfo->DramRegion[I].Size;
-
-    DEBUG ((DEBUG_INFO, "bank[%d]: start 0x%lx, size 0x%lx\n",
-      I, DramInfo->DramRegion[I].BaseAddress, DramInfo->DramRegion[I].Size));
-
-    I++;
-  } while (DramSize);
-
-  DramInfo->NumOfDrams = I;
-
-  DEBUG ((DEBUG_INFO, "Number Of DRAM in system %d \n", DramInfo->NumOfDrams));
-
-  return EFI_SUCCESS;
-}
diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
index 4f14cc9848..c327e738cc 100644
--- a/Silicon/NXP/NxpQoriqLs.dec
+++ b/Silicon/NXP/NxpQoriqLs.dec
@@ -38,7 +38,10 @@
   #
   gNxpQoriqLsTokenSpaceGuid.PcdGurBigEndian|FALSE|BOOLEAN|0x0000311
 
+  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemSize|0x0|UINT64|0x00000315
+  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemAlignment|0x0|UINT64|0x00000316
+
 [PcdsFeatureFlag]
-  gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|FALSE|BOOLEAN|0x00000315
-  gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian|FALSE|BOOLEAN|0x00000316
+  gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|FALSE|BOOLEAN|0x00000317
+  gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian|FALSE|BOOLEAN|0x00000318
 
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54019): https://edk2.groups.io/g/devel/message/54019
Mute This Topic: https://groups.io/mt/71046335/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 13/19] Silicon/NXP: Move RAM retrieval from SocLib
Posted by Leif Lindholm 5 years, 12 months ago
On Fri, Feb 07, 2020 at 18:13:22 +0530, Pankaj Bansal wrote:
> RAM retrieval using SMC commands is common to all Layerscape SOCs.
> Therefore, move it to commom MemoryInit Pei Lib.
> 
> Also added provision to reserve a portion of RAM.

"Also" is an indicator of something that should be a separate patch.
That patch could explain the use case for the reservation function.

> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> ---
>  Silicon/NXP/Include/DramInfo.h                |  38 ---
>  .../Library/MemoryInitPei/MemoryInitPeiLib.c  | 235 +++++++++++++++---
>  .../Library/MemoryInitPei/MemoryInitPeiLib.h  |  24 ++
>  .../MemoryInitPei/MemoryInitPeiLib.inf        |   8 +
>  Silicon/NXP/Library/SocLib/Chassis.c          |  67 -----
>  Silicon/NXP/NxpQoriqLs.dec                    |   7 +-
>  6 files changed, 239 insertions(+), 140 deletions(-)
>  delete mode 100644 Silicon/NXP/Include/DramInfo.h
>  create mode 100644 Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h
> 
> diff --git a/Silicon/NXP/Include/DramInfo.h b/Silicon/NXP/Include/DramInfo.h
> deleted file mode 100644
> index a934aaeff1..0000000000
> --- a/Silicon/NXP/Include/DramInfo.h
> +++ /dev/null
> @@ -1,38 +0,0 @@
> -/** @file
> -*  Header defining the structure for Dram Information
> -*
> -*  Copyright 2019 NXP
> -*
> -*  SPDX-License-Identifier: BSD-2-Clause-Patent
> -*
> -**/
> -
> -#ifndef DRAM_INFO_H_
> -#define DRAM_INFO_H_
> -
> -#include <Uefi/UefiBaseType.h>
> -
> -#define SMC_DRAM_BANK_INFO          (0xC200FF12)
> -
> -typedef struct {
> -  UINTN            BaseAddress;
> -  UINTN            Size;
> -} DRAM_REGION_INFO;
> -
> -typedef struct {
> -  UINT32            NumOfDrams;
> -  UINT32            Reserved;
> -  DRAM_REGION_INFO  DramRegion[3];
> -} DRAM_INFO;
> -
> -EFI_STATUS
> -GetDramBankInfo (
> -  IN OUT DRAM_INFO *DramInfo
> -  );
> -
> -VOID
> -UpdateDpaaDram (
> -  IN OUT DRAM_INFO *DramInfo
> -  );
> -
> -#endif /* DRAM_INFO_H_ */
> diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
> index 3ea7736786..eb1983bdbc 100644
> --- a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
> +++ b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
> @@ -17,8 +17,10 @@
>  #include <Library/HobLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/PcdLib.h>
> +#include <Library/ArmSmcLib.h>

Please sort includes alphabetically.

> +
> +#include "MemoryInitPeiLib.h"
>  
> -#include <DramInfo.h>
>  
>  VOID
>  BuildMemoryTypeInformationHob (
> @@ -44,6 +46,85 @@ InitMmu (
>    }
>  }
>  
> +STATIC
> +UINTN
> +CalculateReservedMemBase (
> +  IN DRAM_REGION_INFO *DramRegions,
> +  IN UINT32           NumRegions,
> +  IN UINTN            ReservedMemSize
> +)
> +{
> +  UINTN                 ReservedMemAlignment;
> +  INTN                  Index;
> +  EFI_PHYSICAL_ADDRESS  AlignmentMask;
> +  UINTN                 RegionBaseAddress;
> +  UINTN                 RegionSize;
> +  UINTN                 ReservedBaseAddress;
> +  INTN                  Index2;
> +
> +  ReservedMemAlignment = FixedPcdGet64 (PcdReservedMemAlignment);
> +  //
> +  // Compute alignment bit mask
> +  //
> +  if (ReservedMemAlignment) {
> +    AlignmentMask = LShiftU64 (1, LowBitSet64(ReservedMemAlignment)) - 1;
> +  } else {
> +    AlignmentMask = 0;
> +  }
> +  Index = NumRegions;
> +  while (Index--) {
> +    RegionBaseAddress = DramRegions[Index].BaseAddress;
> +    RegionSize = DramRegions[Index].Size;
> +
> +    if (ReservedMemSize > RegionSize) {
> +      continue;
> +    }
> +
> +    ReservedBaseAddress = (RegionBaseAddress + RegionSize - ReservedMemSize) & (~AlignmentMask);
> +    if (ReservedBaseAddress < RegionBaseAddress) {
> +      continue;
> +    }
> +
> +    // found the region from which reserved mem is to be carved out
> +    // Need to modify the region size and create/delete region if need be
> +    RegionSize -= ReservedMemSize;
> +    if (!RegionSize) {
> +      for (Index2 = Index; Index2 < NumRegions; Index2++) {
> +        CopyMem (&DramRegions[Index2], &DramRegions[Index2 + 1], sizeof (DRAM_REGION_INFO));
> +      }
> +      break;
> +    }
> +
> +    if (ReservedBaseAddress - RegionBaseAddress) {
> +      DramRegions[Index].Size = ReservedBaseAddress - RegionBaseAddress;
> +      RegionSize -= DramRegions[Index].Size;
> +    } else {
> +      DramRegions[Index].BaseAddress = ReservedBaseAddress + ReservedMemSize;
> +      DramRegions[Index].Size = RegionSize;
> +      RegionSize = 0;
> +    }
> +
> +    if (!RegionSize) {
> +      break;
> +    }
> +
> +    for (Index2 = NumRegions; Index2 > (Index + 1); Index2--) {
> +      CopyMem (&DramRegions[Index2], &DramRegions[Index2 - 1], sizeof (DRAM_REGION_INFO));
> +    }
> +    DramRegions[Index2].BaseAddress = ReservedBaseAddress + ReservedMemSize;
> +    DramRegions[Index2].Size = RegionSize;
> +    RegionSize = 0;
> +
> +    break;
> +  }
> +
> +  if (Index == -1) {
> +    return 0;
> +  } else {
> +    return ReservedBaseAddress;
> +  }
> +}
> +
>  /*++
>  
>  Routine Description:
> @@ -68,10 +149,19 @@ MemoryPeim (
>    )
>  {
>    ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
> +  ARM_SMC_ARGS                 ArmSmcArgs;
> +  INT32                        Index;
> +  UINTN                        DramSize;
> +  UINTN                        BaseAddress;
> +  UINTN                        Size;
> +  UINTN                        Top;
> +  // Extra region gets created if we want to reserve a memory region and that creates a memory hole
> +  // because of alignement requirements
> +  DRAM_REGION_INFO             DramRegions[MAX_DRAM_REGIONS + 1];
>    EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> -  EFI_PEI_HOB_POINTERS         NextHob;
> -  BOOLEAN                      Found;
> -  DRAM_INFO                    DramInfo;
> +  UINTN                        FdBase;
> +  UINTN                        FdTop;
> +  BOOLEAN                      FoundSystemMem;
>  
>    // Get Virtual Memory Map from the Platform Library
>    ArmPlatformGetVirtualMemoryMap (&MemoryTable);
> @@ -86,48 +176,127 @@ MemoryPeim (
>    // 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
> +      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

This unrelated indentation change is wrong.

>    );
>  
> -  if (GetDramBankInfo (&DramInfo)) {
> -    DEBUG ((DEBUG_ERROR, "Failed to get DRAM information, exiting...\n"));
> -    return EFI_UNSUPPORTED;
> -  }
> +  FoundSystemMem = FALSE;
> +  ZeroMem (DramRegions, sizeof (DramRegions));
> +
> +  Index = -1;
> +  do {
> +    ArmSmcArgs.Arg0 = SMC_DRAM_BANK_INFO;
> +    ArmSmcArgs.Arg1 = Index++;

This loop would be nicer in a helper function (which would also make
the diff more clear).

>  
> -  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;
> +    ArmCallSmc (&ArmSmcArgs);
> +    ASSERT (!(ArmSmcArgs.Arg0 && !Index));
> +    if (!Index) {
> +      DramSize = ArmSmcArgs.Arg1;
> +    } else {
> +      if (!ArmSmcArgs.Arg0) {
> +        BaseAddress = ArmSmcArgs.Arg1;
> +        Size = ArmSmcArgs.Arg2;
> +        ASSERT (BaseAddress && Size);
> +
> +        DramRegions[Index - 1].BaseAddress = BaseAddress;
> +        DramRegions[Index - 1].Size = Size;
> +        DramSize -= Size;
> +
> +        DEBUG ((DEBUG_INFO, "bank[%d]: start 0x%lx, size 0x%lx\n", Index, BaseAddress, Size));
>        }
> -      NextHob.Raw = GET_NEXT_HOB (NextHob);
> +    }
> +  } while (DramSize && Index < MAX_DRAM_REGIONS);
> +
> +  ASSERT (!DramSize);
> +
> +  // Get the reserved memory size from non volatile storage
> +  Size = FixedPcdGet64 (PcdReservedMemSize);

FixedPcd reads from non-volatile storage?

> +  if (Size) {
> +    BaseAddress = CalculateReservedMemBase (DramRegions, Index, Size);
> +    if (BaseAddress) {
> +      DEBUG ((DEBUG_INFO, "ReservedMem: start 0x%lx, size 0x%lx\n", BaseAddress, Size));
> +    }
> +  }
> +
> +  FdBase = (UINTN)FixedPcdGet64 (PcdFdBaseAddress);
> +  FdTop = FdBase + (UINTN)FixedPcdGet32 (PcdFdSize);
> +
> +  // Declare memory regios to system
> +  for (Index = MAX_DRAM_REGIONS; Index >= 0; Index--) {
> +    if (!DramRegions[Index].Size) {
> +      continue;
>      }
>  
> -    if (!Found) {
> -      // Reserved the memory space occupied by the firmware volume
> +    BaseAddress = DramRegions[Index].BaseAddress;
> +    Top = DramRegions[Index].BaseAddress + DramRegions[Index].Size;
> +
> +    // EDK2 does not have the concept of boot firmware copied into DRAM. To avoid the DXE

Which boot firmware are we talking about here?

> +    // core to overwrite this area we must create a memory allocation HOB for the region,
> +    // but this only works if we split off the underlying resource descriptor as well.
> +    if (FdBase >= BaseAddress && FdTop <= Top) {
> +      // Update Size
> +      Size = FdBase - BaseAddress;
> +      if (Size) {
> +        BuildResourceDescriptorHob (
> +          EFI_RESOURCE_SYSTEM_MEMORY,
> +          ResourceAttributes,
> +          BaseAddress,
> +          Size
> +        );
> +      }
> +      // create the System Memory HOB for the firmware
>        BuildResourceDescriptorHob (
>          EFI_RESOURCE_SYSTEM_MEMORY,
>          ResourceAttributes,
> -        DramInfo.DramRegion[DramInfo.NumOfDrams].BaseAddress,
> -        DramInfo.DramRegion[DramInfo.NumOfDrams].Size
> +        FdBase,
> +        PcdGet32 (PcdFdSize)
> +      );
> +      // Create the System Memory HOB for the remaining region (top of the FD)s
> +      Size = Top - FdTop;
> +      if (Size) {
> +        BuildResourceDescriptorHob (
> +          EFI_RESOURCE_SYSTEM_MEMORY,
> +          ResourceAttributes,
> +          FdTop,
> +          Size
> +        );
> +      };
> +      // Mark the memory covering the Firmware Device as boot services data
> +      BuildMemoryAllocationHob (FixedPcdGet64 (PcdFdBaseAddress),
> +                                FixedPcdGet32 (PcdFdSize),
> +                                EfiBootServicesData);
> +    }else {

Space after } 

> +      BuildResourceDescriptorHob (
> +          EFI_RESOURCE_SYSTEM_MEMORY,

Incorrect indentation.

> +          ResourceAttributes,
> +          DramRegions[Index].BaseAddress,
> +          DramRegions[Index].Size
>        );
>      }
> +
> +    if (FoundSystemMem) {
> +      continue;
> +    }
> +
> +    BaseAddress = DramRegions[Index].BaseAddress;
> +    Size = DramRegions[Index].Size;
> +    Top = DramRegions[Index].BaseAddress + DramRegions[Index].Size;
> +
> +    if (FdBase >= BaseAddress && FdTop <= Top) {
> +      Size -= (UINTN)FixedPcdGet32 (PcdFdSize);
> +    }
> +
> +    if (Size >= FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)) {
> +      FoundSystemMem = TRUE;
> +    }
>    }
>  
> +  ASSERT (FoundSystemMem);
> +
>    // Build Memory Allocation Hob
>    InitMmu (MemoryTable);
>  
> diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h
> new file mode 100644
> index 0000000000..e563b2ba8d
> --- /dev/null
> +++ b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h
> @@ -0,0 +1,24 @@
> +/** @file
> +*
> +*  Copyright 2020 NXP
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#ifndef _MEMORY_INIT_PEI_LIB_H_
> +#define _MEMORY_INIT_PEI_LIB_H_

Please drop leading _ from include guards.

> +
> +#include <Uefi.h>
> +
> +// Specifies the Maximum regions onto which DDR memory can be mapped in a Platform
> +#define MAX_DRAM_REGIONS            3
> +#define SMC_DRAM_BANK_INFO          (0xC200FF12)
> +
> +typedef struct {
> +  UINTN            BaseAddress;
> +  UINTN            Size;
> +} DRAM_REGION_INFO;
> +
> +#endif
> +
> diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
> index a5bd39415d..9adddcaf8c 100644
> --- a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
> +++ b/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
> @@ -33,6 +33,7 @@
>    DebugLib
>    HobLib
>    PcdLib
> +  ArmSmcLib

Please sort library classes alphabetically.

>  
>  [Guids]
>    gEfiMemoryTypeInformationGuid
> @@ -40,6 +41,13 @@
>  [FeaturePcd]
>    gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob
>  
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdFdBaseAddress
> +  gArmTokenSpaceGuid.PcdFdSize
> +  gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize

Please sort fixedpcds alphabetically.

> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemSize
> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemAlignment
> +
>  [Pcd]
>    gArmTokenSpaceGuid.PcdSystemMemoryBase
>    gArmTokenSpaceGuid.PcdSystemMemorySize
> diff --git a/Silicon/NXP/Library/SocLib/Chassis.c b/Silicon/NXP/Library/SocLib/Chassis.c
> index 847331a631..1ef99e8de2 100644
> --- a/Silicon/NXP/Library/SocLib/Chassis.c
> +++ b/Silicon/NXP/Library/SocLib/Chassis.c
> @@ -22,7 +22,6 @@
>  #include <Library/PrintLib.h>
>  #include <Library/SerialPortLib.h>
>  
> -#include <DramInfo.h>
>  #include "NxpChassis.h"
>  
>  UINT32
> @@ -75,69 +74,3 @@ SmmuInit (
>    MmioWrite32 ((UINTN)SMMU_REG_NSCR0, Value);
>  }
>  
> -UINTN
> -GetDramSize (
> -  IN VOID
> -  )
> -{
> -  ARM_SMC_ARGS  ArmSmcArgs;
> -
> -  ArmSmcArgs.Arg0 = SMC_DRAM_BANK_INFO;
> -  ArmSmcArgs.Arg1 = -1;
> -
> -  ArmCallSmc (&ArmSmcArgs);
> -
> -  if (ArmSmcArgs.Arg0) {
> -    return 0;
> -  } else {
> -    return ArmSmcArgs.Arg1;
> -  }
> -}
> -
> -EFI_STATUS
> -GetDramBankInfo (
> -  IN OUT DRAM_INFO *DramInfo
> -  )
> -{
> -  ARM_SMC_ARGS  ArmSmcArgs;
> -  UINT32        I;
> -  UINTN         DramSize;
> -
> -  DramSize = GetDramSize ();
> -  DEBUG ((DEBUG_INFO, "DRAM Total Size 0x%lx \n", DramSize));
> -
> -  // Ensure DramSize has been set
> -  ASSERT (DramSize != 0);
> -
> -  I = 0;
> -
> -  do {
> -    ArmSmcArgs.Arg0 = SMC_DRAM_BANK_INFO;
> -    ArmSmcArgs.Arg1 = I;
> -
> -    ArmCallSmc (&ArmSmcArgs);
> -    if (ArmSmcArgs.Arg0) {
> -      if (I > 0) {
> -        break;
> -      } else {
> -        ASSERT (ArmSmcArgs.Arg0 == 0);
> -      }
> -    }
> -
> -    DramInfo->DramRegion[I].BaseAddress = ArmSmcArgs.Arg1;
> -    DramInfo->DramRegion[I].Size = ArmSmcArgs.Arg2;
> -
> -    DramSize -= DramInfo->DramRegion[I].Size;
> -
> -    DEBUG ((DEBUG_INFO, "bank[%d]: start 0x%lx, size 0x%lx\n",
> -      I, DramInfo->DramRegion[I].BaseAddress, DramInfo->DramRegion[I].Size));
> -
> -    I++;
> -  } while (DramSize);
> -
> -  DramInfo->NumOfDrams = I;
> -
> -  DEBUG ((DEBUG_INFO, "Number Of DRAM in system %d \n", DramInfo->NumOfDrams));
> -
> -  return EFI_SUCCESS;
> -}
> diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
> index 4f14cc9848..c327e738cc 100644
> --- a/Silicon/NXP/NxpQoriqLs.dec
> +++ b/Silicon/NXP/NxpQoriqLs.dec
> @@ -38,7 +38,10 @@
>    #
>    gNxpQoriqLsTokenSpaceGuid.PcdGurBigEndian|FALSE|BOOLEAN|0x0000311
>  
> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemSize|0x0|UINT64|0x00000315
> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemAlignment|0x0|UINT64|0x00000316
> +
>  [PcdsFeatureFlag]
> -  gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|FALSE|BOOLEAN|0x00000315
> -  gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian|FALSE|BOOLEAN|0x00000316
> +  gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|FALSE|BOOLEAN|0x00000317
> +  gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian|FALSE|BOOLEAN|0x00000318

And this hunk shows why it's a good idea to put your Pcds into groups
with gaps in between the tokens. The tokens should *not* be changing
throughout a single set, and only in unusual circumstances anyway.

/
    Leif

>  
> -- 
> 2.17.1
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54203): https://edk2.groups.io/g/devel/message/54203
Mute This Topic: https://groups.io/mt/71046335/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-