[edk2-devel] [PATCH 15/19] Platform/NXP/LS1043ARDB: introduce PEI Phase

Leif Lindholm leif at nuviainc.com
Wed Feb 12 20:24:53 UTC 2020


On Fri, Feb 07, 2020 at 18:13:24 +0530, Pankaj Bansal wrote:
> Added PEI phase to LS1043ARDB.

Needs more detail.

> Signed-off-by: Pankaj Bansal <pankaj.bansal at nxp.com>
> ---
>  Platform/NXP/FVRules.fdf.inc                  |  60 ++++---
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc  |  13 --
>  Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf  |  19 ++-
>  .../AArch64/ArmPlatformHelper.S               |  46 +++++
>  .../ArmPlatformLib/AArch64/NxpQoriqLsHelper.S |  31 ----
>  .../Library/ArmPlatformLib/ArmPlatformLib.c   |  86 +++++-----
>  .../Library/ArmPlatformLib/ArmPlatformLib.inf |  19 ++-
>  .../ArmPlatformLib/ArmPlatformLibMem.c        |  27 ++-
>  Platform/NXP/LS1043aRdbPkg/VarStore.fdf.inc   |  91 ++++++++++
>  .../MemoryInitPeiLib.c                        | 160 +++++++++---------
>  .../MemoryInitPeiLib.h                        |   1 -
>  .../MemoryInitPeiLib.inf                      |  24 +--
>  .../Library/PlatformPeiLib/PlatformPeiLib.c   |  30 ++++
>  .../Library/PlatformPeiLib/PlatformPeiLib.inf |  50 ++++++
>  Silicon/NXP/NxpQoriqLs.dec                    |  21 +--
>  Silicon/NXP/NxpQoriqLs.dsc.inc                |  67 +++++---
>  16 files changed, 481 insertions(+), 264 deletions(-)
>  create mode 100644 Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/ArmPlatformHelper.S
>  delete mode 100644 Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/NxpQoriqLsHelper.S
>  create mode 100644 Platform/NXP/LS1043aRdbPkg/VarStore.fdf.inc
>  rename Silicon/NXP/Library/{MemoryInitPei => MemoryInitPeiLib}/MemoryInitPeiLib.c (82%)
>  rename Silicon/NXP/Library/{MemoryInitPei => MemoryInitPeiLib}/MemoryInitPeiLib.h (94%)
>  rename Silicon/NXP/Library/{MemoryInitPei => MemoryInitPeiLib}/MemoryInitPeiLib.inf (80%)
>  create mode 100644 Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.c
>  create mode 100644 Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf
> 
> diff --git a/Platform/NXP/FVRules.fdf.inc b/Platform/NXP/FVRules.fdf.inc
> index c9fba65dae..598262e045 100644
> --- a/Platform/NXP/FVRules.fdf.inc
> +++ b/Platform/NXP/FVRules.fdf.inc
> @@ -1,8 +1,8 @@
> -#  FvRules.fdf.inc
>  #
> -#  Rules for creating FD.
> -#
> -#  Copyright 2017-2019 NXP
> +#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> +#  Copyright (c) 2014-2016, Linaro Limited. All rights reserved.
> +#  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> +#  Copyright 2017-2020 NXP
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -16,40 +16,49 @@
>  #
>  ################################################################################
>  
> +
> +############################################################################
> +# Example of a DXE_DRIVER FFS file with a Checksum encapsulation section   #
> +############################################################################
> +#
> +#[Rule.Common.DXE_DRIVER]
> +#  FILE DRIVER = $(NAMED_GUID) {
> +#    DXE_DEPEX    DXE_DEPEX               Optional $(INF_OUTPUT)/$(MODULE_NAME).depex
> +#    COMPRESS PI_STD {
> +#      GUIDED {
> +#        PE32     PE32                    $(INF_OUTPUT)/$(MODULE_NAME).efi
> +#        UI       STRING="$(MODULE_NAME)" Optional
> +#        VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
> +#      }
> +#    }
> +#  }
> +#
> +############################################################################
> +
>  [Rule.Common.SEC]
> -  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
> -    TE  TE    Align = 32                $(INF_OUTPUT)/$(MODULE_NAME).efi
> +  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED FIXED {
> +    TE  TE Align = Auto                 $(INF_OUTPUT)/$(MODULE_NAME).efi
>    }
>  
>  [Rule.Common.PEI_CORE]
> -  FILE PEI_CORE = $(NAMED_GUID) {
> -    TE     TE                           $(INF_OUTPUT)/$(MODULE_NAME).efi
> +  FILE PEI_CORE = $(NAMED_GUID) FIXED {
> +    TE     TE Align = Auto              $(INF_OUTPUT)/$(MODULE_NAME).efi
>      UI     STRING ="$(MODULE_NAME)" Optional
>    }
>  
>  [Rule.Common.PEIM]
> -  FILE PEIM = $(NAMED_GUID) {
> +  FILE PEIM = $(NAMED_GUID) FIXED {
>       PEI_DEPEX PEI_DEPEX Optional       $(INF_OUTPUT)/$(MODULE_NAME).depex
> -     PE32      PE32                     $(INF_OUTPUT)/$(MODULE_NAME).efi
> +     TE       TE Align = Auto           $(INF_OUTPUT)/$(MODULE_NAME).efi
>       UI       STRING="$(MODULE_NAME)" Optional
>    }
>  
> -[Rule.Common.PEIM.TIANOCOMPRESSED]
> -  FILE PEIM = $(NAMED_GUID) DEBUG_MYTOOLS_IA32 {
> -    PEI_DEPEX PEI_DEPEX Optional        $(INF_OUTPUT)/$(MODULE_NAME).depex
> -    GUIDED A31280AD-481E-41B6-95E8-127F4C984779 PROCESSING_REQUIRED = TRUE {
> -      PE32      PE32                    $(INF_OUTPUT)/$(MODULE_NAME).efi
> -      UI        STRING="$(MODULE_NAME)" Optional
> -    }
> -  }
> -
>  [Rule.Common.DXE_CORE]
>    FILE DXE_CORE = $(NAMED_GUID) {
>      PE32     PE32                       $(INF_OUTPUT)/$(MODULE_NAME).efi
>      UI       STRING="$(MODULE_NAME)" Optional
>    }
>  
> -
>  [Rule.Common.UEFI_DRIVER]
>    FILE DRIVER = $(NAMED_GUID) {
>      DXE_DEPEX    DXE_DEPEX              Optional $(INF_OUTPUT)/$(MODULE_NAME).depex
> @@ -62,6 +71,8 @@
>      DXE_DEPEX    DXE_DEPEX              Optional $(INF_OUTPUT)/$(MODULE_NAME).depex
>      PE32         PE32                   $(INF_OUTPUT)/$(MODULE_NAME).efi
>      UI           STRING="$(MODULE_NAME)" Optional
> +    RAW          ACPI  Optional               |.acpi
> +    RAW          ASL   Optional               |.aml
>    }
>  
>  [Rule.Common.DXE_RUNTIME_DRIVER]
> @@ -73,7 +84,7 @@
>  
>  [Rule.Common.UEFI_APPLICATION]
>    FILE APPLICATION = $(NAMED_GUID) {
> -    UI     STRING ="$(MODULE_NAME)" Optional
> +    UI     STRING ="$(MODULE_NAME)"     Optional
>      PE32   PE32                         $(INF_OUTPUT)/$(MODULE_NAME).efi
>    }
>  
> @@ -91,3 +102,10 @@
>      UI        STRING="$(MODULE_NAME)" Optional
>      VERSION   STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
>    }
> +
> +[Rule.Common.USER_DEFINED.ACPITABLE]
> +  FILE FREEFORM = $(NAMED_GUID) {
> +    RAW       ACPI                    |.acpi
> +    RAW       ASL                     |.aml
> +    UI        STRING="$(MODULE_NAME)" Optional
> +  }
> diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> index 4bc7f6ef97..c9f828668f 100644
> --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
> @@ -32,19 +32,6 @@
>  
>  [PcdsFixedAtBuild.common]
>  
> -  #
> -  # LS1043a board Specific PCDs
> -  # XX (DRAM - Region 1 2GB)
> -  # (NOR - IFC Region 1 512MB)
> -  gArmTokenSpaceGuid.PcdSystemMemoryBase|0x80000000
> -  gArmTokenSpaceGuid.PcdSystemMemorySize|0x7BE00000
> -  gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x02000000
> -
> -  #
> -  # Board Specific Pcds
> -  #
> -  gNxpQoriqLsTokenSpaceGuid.PcdPlatformFreqDiv|0x1
> -
>    #
>    # RTC Pcds
>    #
> diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
> index 8d66f36d74..22391899b7 100644
> --- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
> +++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
> @@ -3,7 +3,7 @@
>  #  FLASH layout file for LS1043a board.
>  #
>  #  Copyright (c) 2016, Freescale Ltd. All rights reserved.
> -#  Copyright 2017-2019 NXP
> +#  Copyright 2017-2020 NXP
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -24,10 +24,10 @@
>  
>  [FD.LS1043ARDB_EFI]
>  BaseAddress   = 0x82000000|gArmTokenSpaceGuid.PcdFdBaseAddress  #The base address of the FLASH Device.
> -Size          = 0x000ED000|gArmTokenSpaceGuid.PcdFdSize         #The size in bytes of the FLASH Device
> +Size          = 0x00100000|gArmTokenSpaceGuid.PcdFdSize         #The size in bytes of the FLASH Device
>  ErasePolarity = 1
>  BlockSize     = 0x1
> -NumBlocks     = 0xED000
> +NumBlocks     = 0x100000
>  
>  ################################################################################
>  #
> @@ -44,10 +44,11 @@ NumBlocks     = 0xED000
>  # RegionType <FV, DATA, or FILE>
>  #
>  ################################################################################
> -0x00000000|0x000ED000
> +0x00000000|0x00100000
>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>  FV = FVMAIN_COMPACT
>  
> +!include VarStore.fdf.inc
>  !include Platform/NXP/FVRules.fdf.inc
>  ################################################################################
>  #
> @@ -158,7 +159,15 @@ READ_STATUS        = TRUE
>  READ_LOCK_CAP      = TRUE
>  READ_LOCK_STATUS   = TRUE
>  
> -  INF ArmPlatformPkg/PrePi/PeiUniCore.inf
> +  INF ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> +  INF MdeModulePkg/Core/Pei/PeiMain.inf
> +  INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
> +  INF MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> +  INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> +  INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
> +  INF ArmPkg/Drivers/CpuPei/CpuPei.inf
> +  INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> +  INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>  
>    FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>      SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
> diff --git a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/ArmPlatformHelper.S b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/ArmPlatformHelper.S
> new file mode 100644
> index 0000000000..d1b9f1debb
> --- /dev/null
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/ArmPlatformHelper.S
> @@ -0,0 +1,46 @@
> +//
> +//  Copyright (c) 2012-2013, ARM Limited. All rights reserved.
> +//
> +//  SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +//
> +
> +#include <AsmMacroIoLibV8.h>
> +#include <Library/ArmLib.h>
> +
> +ASM_FUNC(ArmPlatformPeiBootAction)
> +  ret
> +
> +//UINTN
> +//ArmPlatformGetCorePosition (
> +//  IN UINTN MpId
> +//  );
> +// With this function: CorePos = (ClusterId * 4) + CoreId
> +ASM_FUNC(ArmPlatformGetCorePosition)
> +  and   x1, x0, #ARM_CORE_MASK
> +  and   x0, x0, #ARM_CLUSTER_MASK
> +  add   x0, x1, x0, LSR #6
> +  ret
> +
> +//UINTN
> +//ArmPlatformGetPrimaryCoreMpId (
> +//  VOID
> +//  );
> +ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
> +  MOV32  (w0, FixedPcdGet32 (PcdArmPrimaryCore))
> +  ret
> +
> +//UINTN
> +//ArmPlatformIsPrimaryCore (
> +//  IN UINTN MpId
> +//  );
> +ASM_FUNC(ArmPlatformIsPrimaryCore)
> +  MOV32  (w1, FixedPcdGet32 (PcdArmPrimaryCoreMask))
> +  and   x0, x0, x1
> +  MOV32  (w1, FixedPcdGet32 (PcdArmPrimaryCore))
> +  cmp   w0, w1
> +  mov   x0, #1
> +  mov   x1, #0
> +  csel  x0, x0, x1, eq
> +  ret
> +
> diff --git a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/NxpQoriqLsHelper.S b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/NxpQoriqLsHelper.S
> deleted file mode 100644
> index dfbf73675a..0000000000
> --- a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/AArch64/NxpQoriqLsHelper.S
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -#  @file
> -#
> -#  Copyright (c) 2012-2013, ARM Limited. All rights reserved.
> -#  Copyright 2017, 2020 NXP
> -#
> -#  SPDX-License-Identifier: BSD-2-Clause-Patent
> -#
> -
> -#include <AsmMacroIoLibV8.h>
> -#include <AutoGen.h>
> -
> -.text
> -.align 2
> -
> -GCC_ASM_IMPORT(ArmReadMpidr)
> -
> -ASM_FUNC(ArmPlatformIsPrimaryCore)
> -  tst x0, #3
> -  cset x0, eq
> -  ret
> -
> -ASM_FUNC(ArmPlatformPeiBootAction)
> -EL1_OR_EL2(x0)
> -1:
> -2:
> -  ret
> -
> -ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
> -  MOV32 (x0, FixedPcdGet32(PcdArmPrimaryCore))
> -  ldrh   w0, [x0]
> -  ret
> diff --git a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> index 821d413a3e..f1d67d6c7d 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.c
> @@ -1,8 +1,4 @@
> -/** ArmPlatformLib.c
> -*
> -*  Contains board initialization functions.
> -*
> -*  Based on BeagleBoardPkg/Library/BeagleBoardLib/BeagleBoard.c
> +/** @file
>  *
>  *  Copyright (c) 2011-2012, ARM Limited. All rights reserved.
>  *  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
> @@ -19,19 +15,18 @@
>  #include <Library/SocLib.h>
>  #include <Ppi/NxpPlatformGetClock.h>
>  
> -/**
> -  Return the current Boot Mode
> -
> -  This function returns the boot reason on the platform
> +ARM_CORE_INFO mArmPlatformMpCoreInfoTable[] = {
> +  {
> +    // Cluster 0, Core 0
> +    0, 0,
>  
> -**/
> -EFI_BOOT_MODE
> -ArmPlatformGetBootMode (
> -  VOID
> -  )
> -{
> -  return BOOT_WITH_FULL_CONFIGURATION;
> -}
> +    // MP Core MailBox Set/Get/Clear Addresses and Clear Value
> +    (EFI_PHYSICAL_ADDRESS)0,
> +    (EFI_PHYSICAL_ADDRESS)0,
> +    (EFI_PHYSICAL_ADDRESS)0,
> +    (UINT64)0xFFFFFFFF
> +  },
> +};
>  
>  /**
>    Get the clocks supplied by Platform(Board) to NXP Layerscape SOC
> @@ -77,11 +72,29 @@ NxpPlatformGetClock(
>  }
>  
>  /**
> - Placeholder for Platform Initialization
> +  Return the current Boot Mode
> +
> +  This function returns the boot reason on the platform
> +
>  **/
> -EFI_STATUS
> +EFI_BOOT_MODE
> +ArmPlatformGetBootMode (
> +  VOID
> +  )
> +{
> +  return BOOT_WITH_FULL_CONFIGURATION;
> +}
> +
> +/**
> +  Initialize controllers that must setup in the normal world
> +
> +  This function is called by the ArmPlatformPkg/PrePi or ArmPlatformPkg/PlatformPei
> +  in the PEI phase.
> +
> +**/
> +RETURN_STATUS
>  ArmPlatformInitialize (
> -  IN  UINTN   MpId
> +  IN  UINTN                     MpId

Would prefer if formating changes were left until actually needed,
and/or broken out into separate patch.

>    )
>  {
>    SocInit ();
> @@ -89,29 +102,19 @@ ArmPlatformInitialize (
>    return EFI_SUCCESS;
>  }
>  
> -ARM_CORE_INFO LS1043aMpCoreInfoCTA53x4[] = {
> -  {
> -    // Cluster 0, Core 0
> -    0x0, 0x0,
> -
> -    // MP Core MailBox Set/Get/Clear Addresses and Clear Value
> -    (EFI_PHYSICAL_ADDRESS)0,
> -    (EFI_PHYSICAL_ADDRESS)0,
> -    (EFI_PHYSICAL_ADDRESS)0,
> -    (UINT64)0xFFFFFFFF
> -  },
> -};
> -
>  EFI_STATUS
>  PrePeiCoreGetMpCoreInfo (
>    OUT UINTN                   *CoreCount,
>    OUT ARM_CORE_INFO           **ArmCoreTable
>    )
>  {
> -  *CoreCount    = sizeof (LS1043aMpCoreInfoCTA53x4) / sizeof (ARM_CORE_INFO);
> -  *ArmCoreTable = LS1043aMpCoreInfoCTA53x4;
> -
> -  return EFI_SUCCESS;
> +  if (ArmIsMpCore()) {
> +    *CoreCount    = sizeof (mArmPlatformMpCoreInfoTable) / sizeof (ARM_CORE_INFO);
> +    *ArmCoreTable = mArmPlatformMpCoreInfoTable;
> +    return EFI_SUCCESS;
> +  } else {
> +    return EFI_UNSUPPORTED;
> +  }
>  }
>  
>  ARM_MP_CORE_INFO_PPI mMpCoreInfoPpi = { PrePeiCoreGetMpCoreInfo };
> @@ -125,6 +128,7 @@ EFI_PEI_PPI_DESCRIPTOR      gPlatformPpiTable[] = {
>    }
>  };
>  
> +

Spurious blank line addition.

>  VOID
>  ArmPlatformGetPlatformPpiList (
>    OUT UINTN                   *PpiListSize,
> @@ -135,11 +139,3 @@ ArmPlatformGetPlatformPpiList (
>    *PpiList = gPlatformPpiTable;
>  }
>  
> -
> -UINTN
> -ArmPlatformGetCorePosition (
> -  IN UINTN MpId
> -  )
> -{
> -  return 1;
> -}
> diff --git a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> index 8b79fd7490..beb94fba64 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> @@ -1,24 +1,24 @@
> -#  @file
> -#
> +#/* @file

I don't see the point of this change? For doxygen?
If so, shouldn't it be /** ?
Regardless, it does not form part of "added PEI for", so belongs in
a different patch.

>  #  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
>  #  Copyright 2017, 2019-2020 NXP
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> +#*/
>  
>  [Defines]
>    INF_VERSION                    = 0x0001001A
> -  BASE_NAME                      = PlatformLib
> +  BASE_NAME                      = ArmPlatformLib

This does not look like the patch this hunk belongs in.

>    FILE_GUID                      = 736343a0-1d96-11e0-aaaa-0002a5d5c51b
>    MODULE_TYPE                    = BASE
>    VERSION_STRING                 = 1.0
>    LIBRARY_CLASS                  = ArmPlatformLib
>  
>  [Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec

Please insert packages sorted alphabetically.

>    ArmPkg/ArmPkg.dec
>    ArmPlatformPkg/ArmPlatformPkg.dec
> -  EmbeddedPkg/EmbeddedPkg.dec
> -  MdePkg/MdePkg.dec
>    Silicon/NXP/NxpQoriqLs.dec
>    Silicon/NXP/Chassis2/Chassis2.dec
>    Silicon/NXP/Chassis2/LS1043A/LS1043A.dec
> @@ -30,13 +30,14 @@
>    DebugLib
>  
>  [Sources.common]
> -  AArch64/NxpQoriqLsHelper.S    | GCC
> +  AArch64/ArmPlatformHelper.S    | GCC
>    ArmPlatformLibMem.c
>    ArmPlatformLib.c
>  
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
> +  gArmTokenSpaceGuid.PcdArmPrimaryCore
> +
>  [Ppis]
>    gArmMpCoreInfoPpiGuid
>  
> -[FixedPcd]
> -  gArmTokenSpaceGuid.PcdArmPrimaryCore
> -  gArmPlatformTokenSpaceGuid.PcdCoreCount
> diff --git a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> index 3a72c8bdd8..822afb2188 100644
> --- a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c
> @@ -1,8 +1,4 @@
> -/** NxpQoriqLsMem.c
> -*
> -*  Board memory specific Library.
> -*
> -*  Based on BeagleBoardPkg/Library/BeagleBoardLib/BeagleBoardMem.c
> +/** @file
>  *
>  *  Copyright (c) 2011, ARM Limited. All rights reserved.
>  *  Copyright (c) 2016, Freescale Semiconductor, Inc. All rights reserved.
> @@ -14,10 +10,11 @@
>  
>  #include <Library/ArmPlatformLib.h>
>  #include <Library/DebugLib.h>
> -#include <Library/PcdLib.h>
> +#include <Library/HobLib.h>

Please insert include files sorted alphabetically.

>  #include <Library/MemoryAllocationLib.h>
>  #include <Soc.h>
>  
> +// Number of Virtual Memory Map Descriptors

This is not part of "added PEI for", belongs in a different patch.

>  #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          25
>  
>  /**
> @@ -25,28 +22,28 @@
>  
>    This Virtual Memory Map is used by MemoryInitPei Module to initialize the MMU on your platform.
>  
> -  @param  VirtualMemoryMap     Array of ARM_MEMORY_REGION_DESCRIPTOR describing a Physical-to-
> -                               Virtual Memory mapping. This array must be ended by a zero-filled
> -                               entry
> +  @param[out]   VirtualMemoryMap    Array of ARM_MEMORY_REGION_DESCRIPTOR describing a Physical-to-
> +                                    Virtual Memory mapping. This array must be ended by a zero-filled
> +                                    entry

Nor does this improvement.

>  
>  **/
> -

(Or this, but at least deleted lines don't confuse git blame/praise output.)

>  VOID
>  ArmPlatformGetVirtualMemoryMap (
> -  IN  ARM_MEMORY_REGION_DESCRIPTOR **VirtualMemoryMap
> +  IN ARM_MEMORY_REGION_DESCRIPTOR** VirtualMemoryMap

Not this whitespace juggling.

>    )
>  {
> -  UINTN                            Index;
> -  ARM_MEMORY_REGION_DESCRIPTOR     *VirtualMemoryTable;
> +  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
> +  UINT32                        Index;

Nor this one.

>  
>    Index = 0;
>  
>    ASSERT (VirtualMemoryMap != NULL);
>  
> -  VirtualMemoryTable = (ARM_MEMORY_REGION_DESCRIPTOR*)AllocatePages (
> -          EFI_SIZE_TO_PAGES (sizeof (ARM_MEMORY_REGION_DESCRIPTOR) * MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS));
> +  VirtualMemoryTable = AllocatePool (sizeof (ARM_MEMORY_REGION_DESCRIPTOR) *
> +                                     MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
>  
>    if (VirtualMemoryTable == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a: Error: Failed AllocatePool()\n", __FUNCTION__));
>      return;
>    }
>  
> diff --git a/Platform/NXP/LS1043aRdbPkg/VarStore.fdf.inc b/Platform/NXP/LS1043aRdbPkg/VarStore.fdf.inc
> new file mode 100644
> index 0000000000..c6cb3339d9
> --- /dev/null
> +++ b/Platform/NXP/LS1043aRdbPkg/VarStore.fdf.inc

Also confused over how this file addition forms part of "added PEI...".

> @@ -0,0 +1,91 @@
> +## @file
> +#  FDF include file with FD definition that defines an empty variable store.
> +#
> +#  Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.
> +#  Copyright (C) 2014, Red Hat, Inc.
> +#  Copyright (c) 2016, Linaro, Ltd. All rights reserved.
> +#  Copyright (c) 2016, Freescale Semiconductor. All rights reserved.
> +#  Copyright 2017-2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[FD.LS1043ARDBNV_EFI]
> +BaseAddress = 0x60500000
> +Size = 0x000C0000
> +ErasePolarity = 1
> +BlockSize = 0x1
> +NumBlocks = 0xC0000
> +
> +#
> +# Place NV Storage just above Platform Data Base
> +#
> +DEFINE NVRAM_AREA_VARIABLE_BASE                = 0x00000000
> +DEFINE NVRAM_AREA_VARIABLE_SIZE                = 0x00040000
> +DEFINE FTW_WORKING_BASE                        = $(NVRAM_AREA_VARIABLE_BASE) + $(NVRAM_AREA_VARIABLE_SIZE)
> +DEFINE FTW_WORKING_SIZE                        = 0x00040000
> +DEFINE FTW_SPARE_BASE                          = $(FTW_WORKING_BASE) + $(FTW_WORKING_SIZE)
> +DEFINE FTW_SPARE_SIZE                          = 0x00040000
> +
> +#############################################################################
> +# LS1043ARDB NVRAM Area
> +# LS1043ARDB NVRAM Area contains: Variable + FTW Working + FTW Spare
> +#############################################################################
> +
> +
> +$(NVRAM_AREA_VARIABLE_BASE)|$(NVRAM_AREA_VARIABLE_SIZE)
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> +#NV_VARIABLE_STORE
> +DATA = {
> +  ## This is the EFI_FIRMWARE_VOLUME_HEADER
> +  # ZeroVector []
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  # FileSystemGuid: gEfiSystemNvDataFvGuid         =
> +  #   { 0xFFF12B8D, 0x7696, 0x4C8B,
> +  #     { 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50 }}
> +  0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
> +  0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
> +  # FvLength: Flash Size : 0x4000000
> +  0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00,
> +  # Signature "_FVH"       # Attributes
> +  0x5f, 0x46, 0x56, 0x48, 0x36, 0x0E, 0x00, 0x00,
> +  # HeaderLength # CheckSum # ExtHeaderOffset #Reserved #Revision
> +  0x48, 0x00, 0x08, 0xA6, 0x00, 0x00, 0x00, 0x02,
> +  # Blockmap[0]: 0x4000 Blocks * 0x1000 Bytes / Block = SIZE_64MB
> +  0x00, 0x40, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00,
> +  # Blockmap[1]: End
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  ## This is the VARIABLE_STORE_HEADER
> +  # It is compatible with SECURE_BOOT_ENABLE == FALSE as well.
> +  # Signature: gEfiVariableGuid =
> +  #   { 0xddcf3616, 0x3275, 0x4164,
> +  #     { 0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d }}
> +  0x16, 0x36, 0xcf, 0xdd, 0x75, 0x32, 0x64, 0x41,
> +  0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d,
> +  # Size: 0x40000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) -
> +  #         0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x3ffb8
> +  # This can speed up the Variable Dispatch a bit.
> +  0xB8, 0xFF, 0x03, 0x00,
> +  # FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
> +  0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +}
> +
> +$(FTW_WORKING_BASE)|$(FTW_WORKING_SIZE)
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
> +#NV_FTW_WORKING
> +DATA = {
> +  # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature = gEdkiiWorkingBlockSignatureGuid         =
> +  #  { 0x9e58292b, 0x7c68, 0x497d, { 0xa0, 0xce, 0x65,  0x0, 0xfd, 0x9f, 0x1b, 0x95 }}
> +  0x2b, 0x29, 0x58, 0x9e, 0x68, 0x7c, 0x7d, 0x49,
> +  0xa0, 0xce, 0x65,  0x0, 0xfd, 0x9f, 0x1b, 0x95,
> +  # Crc:UINT32            #WorkingBlockValid:1, WorkingBlockInvalid:1, Reserved
> +  0x5b, 0xe7, 0xc6, 0x86, 0xFE, 0xFF, 0xFF, 0xFF,
> +  # WriteQueueSize: UINT64
> +  0xE0, 0xFF, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00
> +}
> +
> +$(FTW_SPARE_BASE)|$(FTW_SPARE_SIZE)
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
> +#NV_FTW_SPARE
> diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> similarity index 82%
> rename from Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
> rename to Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> index eb1983bdbc..3d04e05283 100644
> --- a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.c
> +++ b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> @@ -1,7 +1,6 @@
>  /** @file
>  *
>  *  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> -*

Why?

>  *  Copyright 2019-2020 NXP
>  *
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -10,42 +9,18 @@
>  
>  #include <PiPei.h>
>  
> +#include <Library/ArmSmcLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>

Please insert include files alphabetically sorted.

>  #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 <Library/ArmSmcLib.h>
>  
>  #include "MemoryInitPeiLib.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"));
> -  }
> -}
> -
>  STATIC
>  UINTN
>  CalculateReservedMemBase (
> @@ -62,7 +37,7 @@ CalculateReservedMemBase (
>    UINTN                 ReservedBaseAddress;
>    INTN                  Index2;
>  
> -  ReservedMemAlignment = FixedPcdGet64 (PcdReservedMemAlignment);
> +  ReservedMemAlignment = PcdGet64 (PcdReservedMemAlignment);
>    //
>    // Compute alignment bit mask
>    //
> @@ -125,56 +100,26 @@ CalculateReservedMemBase (
>    }
>  }
>  
> -/*++
> -
> -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
> +RETURN_STATUS
>  EFIAPI
> -MemoryPeim (
> -  IN EFI_PHYSICAL_ADDRESS               UefiMemoryBase,
> -  IN UINT64                             UefiMemorySize
> +MemoryInitPeiLibConstructor (
> +  VOID
>    )
>  {
> -  ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
> -  ARM_SMC_ARGS                 ArmSmcArgs;
> -  INT32                        Index;
> -  UINTN                        DramSize;
> -  UINTN                        BaseAddress;
> -  UINTN                        Size;
> -  UINTN                        Top;
> +  ARM_SMC_ARGS                ArmSmcArgs;
> +  INT32                       Index;
> +  UINTN                       DramSize;
> +  UINTN                       BaseAddress;
> +  UINTN                       Size;
> +  UINTN                       Top;

This block indentation change looks fairly superfluous.

>    // 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;
> -  UINTN                        FdBase;
> -  UINTN                        FdTop;
> -  BOOLEAN                      FoundSystemMem;
> -
> -  // 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);
> +  DRAM_REGION_INFO            DramRegions[MAX_DRAM_REGIONS + 1];
> +  EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes;
> +  UINTN                       FdBase;
> +  UINTN                       FdTop;
> +  BOOLEAN                     FoundSystemMem;

As does this block indentation change, which really messes with patch clarity.

>  
> -  //
> -  // Now, the permanent memory has been installed, we can call AllocatePages()
> -  //
>    ResourceAttributes = (
>        EFI_RESOURCE_ATTRIBUTE_PRESENT |
>        EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> @@ -214,16 +159,17 @@ MemoryPeim (
>    ASSERT (!DramSize);
>  
>    // Get the reserved memory size from non volatile storage
> -  Size = FixedPcdGet64 (PcdReservedMemSize);
> +  Size = PcdGet64 (PcdReservedMemSize);
>    if (Size) {
>      BaseAddress = CalculateReservedMemBase (DramRegions, Index, Size);
>      if (BaseAddress) {
>        DEBUG ((DEBUG_INFO, "ReservedMem: start 0x%lx, size 0x%lx\n", BaseAddress, Size));
> +      PcdSet64S (PcdReservedMemBase, BaseAddress);
>      }
>    }
>  
> -  FdBase = (UINTN)FixedPcdGet64 (PcdFdBaseAddress);
> -  FdTop = FdBase + (UINTN)FixedPcdGet32 (PcdFdSize);
> +  FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);
> +  FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);
>  
>    // Declare memory regios to system
>    for (Index = MAX_DRAM_REGIONS; Index >= 0; Index--) {
> @@ -266,8 +212,8 @@ MemoryPeim (
>          );
>        };
>        // Mark the memory covering the Firmware Device as boot services data
> -      BuildMemoryAllocationHob (FixedPcdGet64 (PcdFdBaseAddress),
> -                                FixedPcdGet32 (PcdFdSize),
> +      BuildMemoryAllocationHob (PcdGet64 (PcdFdBaseAddress),
> +                                PcdGet32 (PcdFdSize),
>                                  EfiBootServicesData);
>      }else {
>        BuildResourceDescriptorHob (
> @@ -287,16 +233,72 @@ MemoryPeim (
>      Top = DramRegions[Index].BaseAddress + DramRegions[Index].Size;
>  
>      if (FdBase >= BaseAddress && FdTop <= Top) {
> -      Size -= (UINTN)FixedPcdGet32 (PcdFdSize);
> +      Size -= (UINTN)PcdGet32 (PcdFdSize);
>      }
>  
>      if (Size >= FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)) {
>        FoundSystemMem = TRUE;
> +      PcdSet64S (PcdSystemMemoryBase, BaseAddress);
> +      PcdSet64S (PcdSystemMemorySize, Size);
>      }
>    }
>  
>    ASSERT (FoundSystemMem);
>  
> +  return EFI_SUCCESS;
> +}
> +
> +VOID
> +BuildMemoryTypeInformationHob (
> +  VOID
> +  );
> +

Why has this prototype moved down here?

> +STATIC
> +VOID
> +InitMmu (

And this whole function definition?

> +  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;
> +
> +  // Get Virtual Memory Map from the Platform Library
> +  ArmPlatformGetVirtualMemoryMap (&MemoryTable);
> +
>    // Build Memory Allocation Hob
>    InitMmu (MemoryTable);
>  
> diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.h
> similarity index 94%
> rename from Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h
> rename to Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.h
> index e563b2ba8d..c3c42ed8a3 100644
> --- a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.h
> +++ b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.h
> @@ -21,4 +21,3 @@ typedef struct {
>  } DRAM_REGION_INFO;
>  
>  #endif
> -

Does not belong in this patch.

> diff --git a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
> similarity index 80%
> rename from Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
> rename to Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
> index 9adddcaf8c..4d932444a3 100644
> --- a/Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
> +++ b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
> @@ -8,32 +8,32 @@
>  #**/
>  
>  [Defines]
> -  INF_VERSION                    = 0x0001001A
> +  INF_VERSION                    = 0x00010005

No.

>    BASE_NAME                      = ArmMemoryInitPeiLib
>    FILE_GUID                      = 55ddb6e0-70b5-11e0-b33e-0002a5d5c51b
>    MODULE_TYPE                    = BASE
>    VERSION_STRING                 = 1.0
> -  LIBRARY_CLASS                  = MemoryInitPeiLib|SEC PEIM DXE_DRIVER
> +  LIBRARY_CLASS                  = MemoryInitPeiLib|PEIM
> +  CONSTRUCTOR                    = MemoryInitPeiLibConstructor
>  
>  [Sources]
>    MemoryInitPeiLib.c
>  
> -
>  [Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec

Please insert packages sorted alphabetically.

>    ArmPkg/ArmPkg.dec
>    ArmPlatformPkg/ArmPlatformPkg.dec
> -  EmbeddedPkg/EmbeddedPkg.dec
> -  MdeModulePkg/MdeModulePkg.dec
> -  MdePkg/MdePkg.dec
>    Silicon/NXP/NxpQoriqLs.dec
>  
>  [LibraryClasses]
> -  ArmMmuLib
> -  ArmPlatformLib
>    DebugLib
>    HobLib
> -  PcdLib
> +  ArmMmuLib
> +  ArmPlatformLib

Please insert library classes sorted alphabetically.

>    ArmSmcLib
> +  PcdLib
>  
>  [Guids]
>    gEfiMemoryTypeInformationGuid
> @@ -44,13 +44,15 @@
>  [FixedPcd]
>    gArmTokenSpaceGuid.PcdFdBaseAddress
>    gArmTokenSpaceGuid.PcdFdSize
> +

Why?

>    gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize
> -  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemSize
> -  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemAlignment
>  
>  [Pcd]
>    gArmTokenSpaceGuid.PcdSystemMemoryBase
>    gArmTokenSpaceGuid.PcdSystemMemorySize
> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemSize
> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemAlignment
> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemBase

Please insert alphabetically sorted.

>  
>  [Depex]
>    TRUE
> diff --git a/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.c b/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.c
> new file mode 100644
> index 0000000000..f64e564469
> --- /dev/null
> +++ b/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.c
> @@ -0,0 +1,30 @@
> +/** @file
> +*
> +*  Copyright (c) 2011-2014, ARM Limited. All rights reserved.
> +*  Copyright 2020 NXP
> +*
> +*  SPDX-License-Identifier: BSD-2-Clause-Patent
> +*
> +**/
> +
> +#include <PiPei.h>
> +
> +#include <Library/HobLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PcdLib.h>
> +
> +#define XPRINT(x) PRINT(x)
> +#define PRINT(x) #x

?

> +
> +EFI_STATUS
> +EFIAPI
> +PlatformPeim (
> +  VOID
> +  )
> +{
> +  BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
> +  DEBUG ((DEBUG_INIT, "Edk2 version is %a\n", XPRINT (WORKSPACE_GIT_VERSION)));
> +  DEBUG ((DEBUG_INIT, "Edk2 platforms version is %a\n", XPRINT (PACKAGES_PATH_GIT_VERSION)));

I am confused - what information are these trying to provide?

> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf b/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf
> new file mode 100644
> index 0000000000..b0481dece0
> --- /dev/null
> +++ b/Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf
> @@ -0,0 +1,50 @@
> +#/** @file
> +#
> +#  Copyright (c) 2011-2012, ARM Limited. All rights reserved.
> +#  Copyright 2020 NXP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005

Please bump version.

> +  BASE_NAME                      = ArmPlatformPeiLib
> +  FILE_GUID                      = 49d37060-70b5-11e0-aa2d-0002a5d5c51b

GUIDs are supposed to be globally unique.
You can generate new ones from (for example)
https://www.guidgenerator.com/online-guid-generator.aspx
.

> +  MODULE_TYPE                    = PEIM
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = PlatformPeiLib
> +
> +[BuildOptions]
> +  GCC:*_*_*_CC_FLAGS = -DWORKSPACE_GIT_VERSION="$(WORKSPACE_GIT_VERSION)"
> +  GCC:*_*_*_CC_FLAGS = -DPACKAGES_PATH_GIT_VERSION="$(PACKAGES_PATH_GIT_VERSION)"
> +
> +[Sources]
> +  PlatformPeiLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  Silicon/NXP/NxpQoriqLs.dec

Please insert packages sorted alphabetically.

> +
> +[LibraryClasses]
> +  DebugLib
> +  HobLib
> +  PcdLib
> +
> +[Ppis]
> +  gEfiPeiMasterBootModePpiGuid                  # PPI ALWAYS_PRODUCED
> +  gEfiPeiBootInRecoveryModePpiGuid              # PPI SOMETIMES_PRODUCED

Please insert Ppis sorted alphabetically.

> +
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdFdBaseAddress
> +  gArmTokenSpaceGuid.PcdFdSize
> +
> +  gArmTokenSpaceGuid.PcdFvBaseAddress
> +  gArmTokenSpaceGuid.PcdFvSize
> +
> +[depex]
> +  TRUE
> diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
> index c327e738cc..0a90334c02 100644
> --- a/Silicon/NXP/NxpQoriqLs.dec
> +++ b/Silicon/NXP/NxpQoriqLs.dec
> @@ -27,21 +27,12 @@
>    gNxpQoriqLsTokenSpaceGuid      = {0x98657342, 0x4aee, 0x4fc6, {0xbc, 0xb5, 0xff, 0x45, 0xb7, 0xa8, 0x71, 0xf2}}
>    gNxpNonDiscoverableI2cMasterGuid = { 0x5f2c099c, 0x54a3, 0x4dd4, {0x9e, 0xc5, 0xe9, 0x12, 0x8c, 0x36, 0x81, 0x6a}}
>  
> -[PcdsFixedAtBuild.common]
> -  #
> -  # Platform PCDs
> -  #
> -  gNxpQoriqLsTokenSpaceGuid.PcdPlatformFreqDiv|0x0|UINT32|0x00000250
> -
> -  #
> -  # Pcds to support Big Endian IPs
> -  #
> -  gNxpQoriqLsTokenSpaceGuid.PcdGurBigEndian|FALSE|BOOLEAN|0x0000311
> -
> -  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemSize|0x0|UINT64|0x00000315
> -  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemAlignment|0x0|UINT64|0x00000316
> +[PcdsDynamic,PcdsPatchableInModule]
> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemBase|0x0|UINT64|0x00000001
> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemSize|0x0|UINT64|0x00000002
> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemAlignment|0x0|UINT64|0x00000003
>  
>  [PcdsFeatureFlag]
> -  gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|FALSE|BOOLEAN|0x00000317
> -  gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian|FALSE|BOOLEAN|0x00000318
> +  gNxpQoriqLsTokenSpaceGuid.PcdI2cErratumA009203|FALSE|BOOLEAN|0x00000004
> +  gNxpQoriqLsTokenSpaceGuid.PcdDcfgBigEndian|FALSE|BOOLEAN|0x00000005

This reassigning of tokens should happen in a separate patch.

/
    Leif

>  
> diff --git a/Silicon/NXP/NxpQoriqLs.dsc.inc b/Silicon/NXP/NxpQoriqLs.dsc.inc
> index 94d3e53a04..9922686304 100644
> --- a/Silicon/NXP/NxpQoriqLs.dsc.inc
> +++ b/Silicon/NXP/NxpQoriqLs.dsc.inc
> @@ -93,29 +93,35 @@
>    CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
>    NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
>    ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
> +  UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
>  
>    I2cLib|Silicon/NXP/Library/I2cLib/I2cLib.inf
>    ResetSystemLib|ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf
>    IoAccessLib|Silicon/NXP/Library/IoAccessLib/IoAccessLib.inf
> +  PlatformPeiLib|Silicon/NXP/Library/PlatformPeiLib/PlatformPeiLib.inf
> +  MemoryInitPeiLib|Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
>  
>  [LibraryClasses.common.SEC]
>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> -  UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
> -  ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
> -  LzmaDecompressLib|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
> -  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
> -  HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
> -  PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
> -  MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
> -  PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
> -  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> -  MemoryInitPeiLib|Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
>  
> -  # 1/123 faster than Stm or Vstm version
> -  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +  DebugAgentLib|ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymbolsBaseLib.inf
> +  HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
> +  PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> +  PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
> +  MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
> +
> +[LibraryClasses.common.PEI_CORE]
> +  PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
> +  HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
> +  PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> +  MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
> +  PeiCoreEntryPoint|MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
> +  PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
> +  ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf
> +  ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
> +  OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>  
> -  # Uncomment to turn on GDB stub in SEC.
> -  #DebugAgentLib|EmbeddedPkg/Library/GdbDebugAgent/GdbDebugAgent.inf
> +  PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
>  
>  [LibraryClasses.common.PEIM]
>    PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
> @@ -124,14 +130,16 @@
>    PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
>    HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>    MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
> +  PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
> +  ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf
>    ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
> +  OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
>  
>  [LibraryClasses.common.DXE_CORE]
>    HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
>    MemoryAllocationLib|MdeModulePkg/Library/DxeCoreMemoryAllocationLib/DxeCoreMemoryAllocationLib.inf
>    DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
>    ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
> -  UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
>    DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>    PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
>  
> @@ -139,7 +147,6 @@
>    DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>    SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
>    PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
> -  MemoryInitPeiLib|Silicon/NXP/Library/MemoryInitPei/MemoryInitPeiLib.inf
>  
>  [LibraryClasses.common.UEFI_APPLICATION]
>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> @@ -204,6 +211,10 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution|640
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution|480
>  
> +  gArmTokenSpaceGuid.PcdSystemMemoryBase|0x0
> +  gArmTokenSpaceGuid.PcdSystemMemorySize|0x0
> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemBase|0x0
> +
>  [PcdsDynamicHii.common.DEFAULT]
>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|10
>  
> @@ -214,7 +225,6 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>    gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
> -  gArmPlatformTokenSpaceGuid.PcdCoreCount|1 # Only one core
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|1000000
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|2000000
>    gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|1000000
> @@ -224,6 +234,12 @@
>    gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0
>    gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320
>  
> +  ## Base of DRAM
> +  ## since TFA puts Fd at 0x2000000 offset from DRAM base, we can use this space
> +  ## for temporary ram
> +  gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x80000000
> +  gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x02000000
> +
>  !if $(TARGET) == RELEASE
>    gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x27
>    gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x81000001
> @@ -281,13 +297,26 @@
>  ################################################################################
>  [Components.common]
>    #
> -  # SEC
> +  # PEI Phase modules
>    #
> -  ArmPlatformPkg/PrePi/PeiUniCore.inf
> +  ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> +
> +  MdeModulePkg/Core/Pei/PeiMain.inf
>    MdeModulePkg/Universal/PCD/Pei/Pcd.inf {
>      <LibraryClasses>
>        PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>    }
> +  MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
> +  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> +
> +  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
> +  ArmPkg/Drivers/CpuPei/CpuPei.inf
> +  ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> +
> +  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
> +    <LibraryClasses>
> +      NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
> +  }
>  
>    #
>    # DXE
> -- 
> 2.17.1
> 

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

View/Reply Online (#54303): https://edk2.groups.io/g/devel/message/54303
Mute This Topic: https://groups.io/mt/71046339/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list